From 0c7950cb442c016a95dff877f95dd5fc37d808fa Mon Sep 17 00:00:00 2001 From: Doddanna17 Date: Fri, 23 Oct 2026 15:52:14 +0530 Subject: [PATCH] feat(sdk-coin-stx): allow a caller-supplied staking contract The contract builder only accepts the canonical pox-4 / pox-5 / send-many-memo contracts: contractName must be in VALID_STAKING_CONTRACT_NAMES and contractAddress must be one of the network's contract addresses. That blocks building for a non-canonical staking contract (e.g. a test/dummy pox-5, or a future pox version) without an SDK change. Add an opt-in allowlist of staking contracts ({ name, address } pairs) that the caller supplies. Empty by default, so the canonical set and every existing flow is unchanged. It is threaded through every path that builds or rebuilds a contract call: - ContractBuilder takes the allowlist (constructor) and accepts a name/address only when they match one allowlisted pair (canonical set otherwise); - TransactionBuilderFactory.from(hex, { allowedStakingContracts }) passes it to getContractBuilder; - Stx.signTransaction / explainTransaction (and the recovery rebuild) pass the coin's setAllowedStakingContracts(...) value, so a rehydrated transaction re-validates against the same rules as the build path. Validation is therefore unchanged: the canonical checks run on every path (including initBuilder), and only an explicitly allowlisted contract is accepted in addition. A contract name that is allowlisted is validated against the canonical pox-5 function set (the dummy is an ABI-identical copy). Ticket: SI-2106 --- .../sdk-coin-stx/src/lib/contractBuilder.ts | 94 ++++++++++++-- .../src/lib/transactionBuilderFactory.ts | 17 ++- modules/sdk-coin-stx/src/stx.ts | 6 +- modules/sdk-coin-stx/src/types.ts | 7 + .../transactionBuilder/contractBuilder.ts | 122 +++++++++++++++++- 5 files changed, 230 insertions(+), 16 deletions(-) diff --git a/modules/sdk-coin-stx/src/lib/contractBuilder.ts b/modules/sdk-coin-stx/src/lib/contractBuilder.ts index b160bd5362e..ea225bd73ed 100644 --- a/modules/sdk-coin-stx/src/lib/contractBuilder.ts +++ b/modules/sdk-coin-stx/src/lib/contractBuilder.ts @@ -18,7 +18,7 @@ import { Transaction } from './transaction'; import { contractPrincipalCVFromString, isValidAddress, standardPrincipalCVFromString } from './utils'; import { ClarityValueJson } from './iface'; import { Utils } from '.'; -import { CONTRACT_NAME_SENDMANY, VALID_STAKING_CONTRACT_NAMES } from './constants'; +import { CONTRACT_NAME_SENDMANY, VALID_POX5_CONTRACT_FUNCTION_NAMES, VALID_STAKING_CONTRACT_NAMES } from './constants'; import { AbstractContractBuilder } from './abstractContractBuilder'; export class ContractBuilder extends AbstractContractBuilder { @@ -30,13 +30,64 @@ export class ContractBuilder extends AbstractContractBuilder { ClarityType.BoolFalse, ]); - constructor(_coinConfig: Readonly) { + /** + * Caller-supplied staking contracts this builder accepts **in addition to** the canonical ones + * (pox-4 / pox-5 / send-many-memo). Empty by default, so behaviour is unchanged unless a caller + * opts in. Lets a deployment target a different staking contract (e.g. a test/dummy pox-5, or a + * future pox version) without an SDK change — the caller passes the name/address it is configured + * with. Threaded in via the constructor so every path that rebuilds a transaction + * (`factory.from(hex, { allowedStakingContracts })`) re-validates against the same rules. + */ + private _allowedStakingContracts: ReadonlyArray<{ name: string; address: string }>; + + constructor( + _coinConfig: Readonly, + allowedStakingContracts: ReadonlyArray<{ name: string; address: string }> = [] + ) { super(_coinConfig); + this._allowedStakingContracts = allowedStakingContracts; + } + + allowedStakingContracts(contracts: ReadonlyArray<{ name: string; address: string }>): this { + const list = contracts ?? []; + const names = new Set(); + for (const contract of list) { + if (names.has(contract.name)) { + throw new InvalidParameterValueError(`Duplicate staking contract name: ${contract.name}`); + } + names.add(contract.name); + } + this._allowedStakingContracts = list; + return this; + } + + /** True when the current (name, address) is a canonical pair or an explicitly allowlisted pair. */ + private isContractPairValid(): boolean { + const isCanonicalName = + VALID_STAKING_CONTRACT_NAMES.includes(this._contractName) || this._contractName === CONTRACT_NAME_SENDMANY; + const isCanonicalAddress = Utils.isValidContractAddress( + this._contractAddress, + this._coinConfig.network as BitgoStacksNetwork + ); + if (isCanonicalName && isCanonicalAddress) { + return true; + } + return this._allowedStakingContracts.some( + (contract) => contract.name === this._contractName && contract.address === this._contractAddress + ); + } + + /** True when the name belongs to an allowlisted staking contract. */ + private isAllowedStakingContractName(name: string): boolean { + return this._allowedStakingContracts.some((contract) => contract.name === name); } initBuilder(tx: Transaction): void { super.initBuilder(tx); - // Retro compatibility, checks parameters. + // Retro compatibility, checks parameters — run unchanged on every path. The allowlist is threaded + // into this builder (constructor / `factory.from(hex, { allowedStakingContracts })`), so an + // allowlisted contract re-validates against the same rules as on the build path, and a + // non-allowlisted one is still rejected (no relaxation). this.contractAddress(this._contractAddress); this.contractName(this._contractName); this.functionName(this._functionName); @@ -54,10 +105,16 @@ export class ContractBuilder extends AbstractContractBuilder { if (!isValidAddress(address)) { throw new InvalidParameterValueError('Invalid address'); } - if (!Utils.isValidContractAddress(address, this._coinConfig.network as BitgoStacksNetwork)) { + const isCanonicalAddress = Utils.isValidContractAddress(address, this._coinConfig.network as BitgoStacksNetwork); + const isAllowedAddress = this._allowedStakingContracts.some((contract) => contract.address === address); + if (!isCanonicalAddress && !isAllowedAddress) { throw new InvalidParameterValueError('Invalid contract address'); } this._contractAddress = address; + // Order-independent: if the name is already set, the (name, address) pair must be valid. + if (this._contractName && !this.isContractPairValid()) { + throw new InvalidParameterValueError('Contract name and address are not an allowlisted pair'); + } return this; } @@ -71,13 +128,26 @@ export class ContractBuilder extends AbstractContractBuilder { if (name.length === 0) { throw new InvalidParameterValueError('Invalid name'); } - if (!VALID_STAKING_CONTRACT_NAMES.includes(name) && name !== CONTRACT_NAME_SENDMANY) { + const isCanonicalName = VALID_STAKING_CONTRACT_NAMES.includes(name) || name === CONTRACT_NAME_SENDMANY; + if (!isCanonicalName && !this.isAllowedStakingContractName(name)) { throw new InvalidParameterValueError('Only pox-4, pox-5, and send-many-memo contracts supported'); } - if (this._functionName && !Utils.isValidContractFunctionName(this._functionName, name)) { - throw new InvalidParameterValueError(`${this._functionName} is not supported contract function name`); - } this._contractName = name; + // Order-independent: if the address is already set, the (name, address) pair must be valid. + if (this._contractAddress && !this.isContractPairValid()) { + throw new InvalidParameterValueError('Contract name and address are not an allowlisted pair'); + } + // A caller-allowlisted contract (e.g. the dummy pox-5) is ABI-identical to the canonical pox-5, + // so its functions are validated against the pox-5 set instead of being skipped. (A contract + // with a genuinely different ABI would need to carry its own function list.) + if (this._functionName) { + const validFunction = this.isAllowedStakingContractName(name) + ? VALID_POX5_CONTRACT_FUNCTION_NAMES.includes(this._functionName) + : Utils.isValidContractFunctionName(this._functionName, name); + if (!validFunction) { + throw new InvalidParameterValueError(`${this._functionName} is not supported contract function name`); + } + } return this; } @@ -91,7 +161,13 @@ export class ContractBuilder extends AbstractContractBuilder { if (name.length === 0) { throw new InvalidParameterValueError('Invalid name'); } - if (!Utils.isValidContractFunctionName(name, this._contractName)) { + if (this.isAllowedStakingContractName(this._contractName)) { + // Caller-allowlisted contract: validate against the canonical pox-5 function set (ABI-identical + // copy) rather than skipping the check. + if (!VALID_POX5_CONTRACT_FUNCTION_NAMES.includes(name)) { + throw new InvalidParameterValueError(`${name} is not supported contract function name`); + } + } else if (!Utils.isValidContractFunctionName(name, this._contractName)) { throw new InvalidParameterValueError(`${name} is not supported contract function name`); } this._functionName = name; diff --git a/modules/sdk-coin-stx/src/lib/transactionBuilderFactory.ts b/modules/sdk-coin-stx/src/lib/transactionBuilderFactory.ts index 96716a24713..5dd2264457f 100644 --- a/modules/sdk-coin-stx/src/lib/transactionBuilderFactory.ts +++ b/modules/sdk-coin-stx/src/lib/transactionBuilderFactory.ts @@ -22,7 +22,10 @@ export class TransactionBuilderFactory extends BaseTransactionBuilderFactory { } /** @inheritdoc */ - from(raw: string): TransactionBuilder { + from( + raw: string, + params?: { allowedStakingContracts?: ReadonlyArray<{ name: string; address: string }> } + ): TransactionBuilder { this.validateRawTransaction(raw); try { const tx = this.parseTransaction(raw); @@ -42,7 +45,7 @@ export class TransactionBuilderFactory extends BaseTransactionBuilderFactory { if (Pox5Builder.isValidContractCall(this._coinConfig, tx.stxTransaction.payload)) { return this.getPox5Builder(tx); } - return this.getContractBuilder(tx); + return this.getContractBuilder(tx, params?.allowedStakingContracts); default: throw new InvalidTransactionError('Invalid transaction'); } @@ -69,8 +72,14 @@ export class TransactionBuilderFactory extends BaseTransactionBuilderFactory { return TransactionBuilderFactory.initializeBuilder(new TransferBuilder(this._coinConfig), tx); } - getContractBuilder(tx?: Transaction): ContractBuilder { - return TransactionBuilderFactory.initializeBuilder(new ContractBuilder(this._coinConfig), tx); + getContractBuilder( + tx?: Transaction, + allowedStakingContracts?: ReadonlyArray<{ name: string; address: string }> + ): ContractBuilder { + return TransactionBuilderFactory.initializeBuilder( + new ContractBuilder(this._coinConfig, allowedStakingContracts), + tx + ); } getSendmanyBuilder(tx?: Transaction): SendmanyBuilder { diff --git a/modules/sdk-coin-stx/src/stx.ts b/modules/sdk-coin-stx/src/stx.ts index d8131c75445..f8654d0a31f 100644 --- a/modules/sdk-coin-stx/src/stx.ts +++ b/modules/sdk-coin-stx/src/stx.ts @@ -283,7 +283,9 @@ export class Stx extends BaseCoin { */ async signTransaction(params: StxSignTransactionOptions): Promise { const factory = new StxLib.TransactionBuilderFactory(coins.get(this.getChain())); - const txBuilder = factory.from(params.txPrebuild.txHex); + const txBuilder = factory.from(params.txPrebuild.txHex, { + allowedStakingContracts: params.allowedStakingContracts, + }); const prvKeys = params.prv instanceof Array ? params.prv : [params.prv]; prvKeys.forEach((prv) => txBuilder.sign({ key: prv })); if (params.pubKeys) txBuilder.fromPubKey(params.pubKeys); @@ -316,7 +318,7 @@ export class Stx extends BaseCoin { } const factory = new StxLib.TransactionBuilderFactory(coins.get(this.getChain())); - const txBuilder = factory.from(txHex); + const txBuilder = factory.from(txHex, { allowedStakingContracts: params.allowedStakingContracts }); if (params.publicKeys !== undefined) { txBuilder.fromPubKey(params.publicKeys); diff --git a/modules/sdk-coin-stx/src/types.ts b/modules/sdk-coin-stx/src/types.ts index bd8409d3b1b..f6aef0c5721 100644 --- a/modules/sdk-coin-stx/src/types.ts +++ b/modules/sdk-coin-stx/src/types.ts @@ -29,6 +29,11 @@ export interface ExplainTransactionOptions { }; publicKeys?: string[]; feeInfo: TransactionFee; + /** + * Non-canonical staking contracts to accept while re-parsing (e.g. a dummy pox-5), passed + * explicitly so no shared/instance state is involved. Empty/omitted ⇒ canonical-only. + */ + allowedStakingContracts?: ReadonlyArray<{ name: string; address: string }>; } export interface StxSignTransactionOptions extends SignTransactionOptions { @@ -36,6 +41,8 @@ export interface StxSignTransactionOptions extends SignTransactionOptions { prv: string | string[]; pubKeys?: string[]; numberSignature?: number; + /** @see ExplainTransactionOptions.allowedStakingContracts */ + allowedStakingContracts?: ReadonlyArray<{ name: string; address: string }>; } export interface TransactionPrebuild extends BaseTransactionPrebuild { diff --git a/modules/sdk-coin-stx/test/unit/transactionBuilder/contractBuilder.ts b/modules/sdk-coin-stx/test/unit/transactionBuilder/contractBuilder.ts index dece7d9c268..01a2f1ecacb 100644 --- a/modules/sdk-coin-stx/test/unit/transactionBuilder/contractBuilder.ts +++ b/modules/sdk-coin-stx/test/unit/transactionBuilder/contractBuilder.ts @@ -2,7 +2,7 @@ import assert from 'assert'; import should from 'should'; import BigNum from 'bn.js'; import { STACKS_TESTNET, STACKS_MAINNET } from '@stacks/network'; -import { InvalidParameterValueError, TransactionType } from '@bitgo/sdk-core'; +import { InvalidParameterValueError, ParseTransactionError, TransactionType } from '@bitgo/sdk-core'; import { bufferCV, noneCV, someCV, standardPrincipalCV, tupleCV, uintCV, intCV } from '@stacks/transactions'; import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; import { BitGoAPI } from '@bitgo/sdk-api'; @@ -168,6 +168,126 @@ describe('Stacks: Contract Builder', function () { txJson.payload.functionArgs.length.should.equal(5); }); + it('an unsigned caller-allowlisted non-canonical pox-5 (dummy) contract call transaction', async () => { + // The caller supplies the staking contract it is configured for — no SDK change needed. + const builder = factoryProd.getContractBuilder(); + builder.allowedStakingContracts([ + { name: 'pox-5-dummy-v1', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY' }, + ]); + builder.fee({ fee: '180' }); + builder.nonce(0); + builder.contractAddress('SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY'); + builder.contractName('pox-5-dummy-v1'); + builder.functionName('register-for-bond'); + builder.functionArgs([ + { type: 'uint128', val: '210' }, + { type: 'principal', val: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY.signer-manager-stackslabs-dummy-v1' }, + { type: 'uint128', val: '1005000' }, + { type: 'optional' }, + { type: 'optional' }, + ]); + builder.fromPubKey(testData.TX_SENDER.pub); + builder.numberSignatures(1); + + const tx = await builder.build(); + const txJson = tx.toJson(); + should.deepEqual(txJson.payload.contractName, 'pox-5-dummy-v1'); + should.deepEqual(txJson.payload.contractAddress, 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY'); + should.deepEqual(txJson.payload.functionName, 'register-for-bond'); + }); + + it('rehydrates an allowlisted non-canonical contract call (sign / explain / fee-rebuild path)', async () => { + // `signTransaction` / `explainTransaction` / the fee re-build all go through `factory.from(hex)`, + // which re-creates the builder — re-parsing must not reject the allowlisted contract. + const allowlist = [{ name: 'pox-5-dummy-v1', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY' }]; + const builder = factoryProd.getContractBuilder(); + builder.allowedStakingContracts(allowlist); + builder.fee({ fee: '180' }); + builder.nonce(0); + builder.contractAddress('SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY'); + builder.contractName('pox-5-dummy-v1'); + builder.functionName('register-for-bond'); + builder.functionArgs([ + { type: 'uint128', val: '210' }, + { type: 'principal', val: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY.signer-manager-stackslabs-dummy-v1' }, + { type: 'uint128', val: '1005000' }, + { type: 'optional' }, + { type: 'optional' }, + ]); + builder.fromPubKey(testData.TX_SENDER.pub); + builder.numberSignatures(1); + const tx = await builder.build(); + + const reparsed = factoryProd.from(tx.toBroadcastFormat(), { allowedStakingContracts: allowlist }); + reparsed.fromPubKey(testData.TX_SENDER.pub); + reparsed.numberSignatures(1); + const payload = (await reparsed.build()).toJson().payload; + should.deepEqual(payload.contractAddress, 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY'); + should.deepEqual(payload.contractName, 'pox-5-dummy-v1'); + + // Without the allowlist the rebuild is rejected — canonical validation is preserved (no relaxation). + (() => factoryProd.from(tx.toBroadcastFormat())).should.throw(ParseTransactionError); + }); + + it('rejects an allowlisted name followed by a canonical address (order-independent pair check)', function () { + const builder = factoryProd.getContractBuilder(); + builder.allowedStakingContracts([ + { name: 'pox-5-dummy-v1', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY' }, + ]); + builder.contractName('pox-5-dummy-v1'); // allowlisted name first + (() => builder.contractAddress('SP000000000000000000002Q6VF78')).should.throw(/not an allowlisted pair/); + }); + + it('rejects an allowlisted address followed by a canonical name (order-independent pair check)', function () { + const builder = factoryProd.getContractBuilder(); + builder.allowedStakingContracts([ + { name: 'pox-5-dummy-v1', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY' }, + ]); + builder.contractAddress('SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY'); // allowlisted address first + (() => builder.contractName('pox-5')).should.throw(/not an allowlisted pair/); + }); + + it('rejects duplicate allowlisted contract names', function () { + const builder = factoryProd.getContractBuilder(); + (() => + builder.allowedStakingContracts([ + { name: 'pox-5-dummy-v1', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY' }, + { name: 'pox-5-dummy-v1', address: 'SP000000000000000000002Q6VF78' }, + ])).should.throw(/Duplicate staking contract name/); + }); + + it('rejects an allowlisted name paired with a different address', function () { + const builder = factoryProd.getContractBuilder(); + builder.allowedStakingContracts([ + { name: 'pox-5-dummy-v1', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY' }, + ]); + builder.contractAddress('SP000000000000000000002Q6VF78'); // canonical address, wrong pair + (() => builder.contractName('pox-5-dummy-v1')).should.throw(/not an allowlisted pair/); + }); + + it('rejects a function that is not in the pox-5 set for an allowlisted contract', function () { + const builder = factoryProd.getContractBuilder(); + builder.allowedStakingContracts([ + { name: 'pox-5-dummy-v1', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY' }, + ]); + builder.contractAddress('SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY'); + builder.contractName('pox-5-dummy-v1'); + // `stack-stx` is a pox-4 function, not part of the pox-5 ABI the dummy mirrors. + (() => builder.functionName('stack-stx')).should.throw(/is not supported contract function name/); + }); + + it('rejects a contract name that is not allowlisted', function () { + const builder = factory.getContractBuilder(); + (() => builder.contractName('some-other-contract')).should.throw(/Only pox-4, pox-5, and send-many-memo/); + }); + + it('rejects a contract address that is not allowlisted', function () { + const builder = factory.getContractBuilder(); + (() => builder.contractAddress('SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY')).should.throw( + /Invalid contract address/ + ); + }); + it('a signed contract call with args', async () => { const builder = initTxBuilder(); builder.functionArgs([