Skip to content

feat(sdk-coin-stx): accept a configured PoX-5 staking contract address #10058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Doddanna17 wants to merge 1 commit into from
+152 −9
Closed

feat(sdk-coin-stx): accept a configured PoX-5 staking contract address #10058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions modules/sdk-coin-stx/src/lib/pox5Builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,13 @@ export class Pox5Builder extends ContractBuilder {
}

public static isValidContractCall(coinConfig: Readonly<CoinConfig>, payload: ContractCallPayload): boolean {
return (
(coinConfig.network as BitgoStacksNetwork).stakingContractAddress === addressToString(payload.contractAddress) &&
payload.contractName.content === CONTRACT_NAME_POX5
);
const network = coinConfig.network as BitgoStacksNetwork;
const address = addressToString(payload.contractAddress);
// Canonical pox-5, or a deployment-configured non-canonical pox-5 (same standard name), so the
// tx routes to this builder (and keeps its pox-5-specific behaviour) without a caller allowlist.
const isPox5Address =
network.stakingContractAddress === address || (network.allowedStakingContractAddresses ?? []).includes(address);
return isPox5Address && payload.contractName.content === CONTRACT_NAME_POX5;
}

registerForBond(params: Pox5RegisterForBondParams): this {
Expand Down
6 changes: 5 additions & 1 deletion modules/sdk-coin-stx/src/lib/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ export function isValidMemo(memo: string): boolean {
/**
* Checks for valid contract address
*
* Accepts the canonical staking / send-many-memo / sbtc-withdrawal addresses plus any
* deployment-configured `network.allowedStakingContractAddresses` (e.g. a non-canonical pox-5).
*
* @param {string} addr - contract deployer address
* @param {BitgoStacksNetwork} network - network object
* @returns {boolean} - the validation result
Expand All @@ -288,7 +291,8 @@ export function isValidContractAddress(addr: string, network: BitgoStacksNetwork
return (
addr === network.stakingContractAddress ||
addr === network.sendmanymemoContractAddress ||
addr === network.sbtcWithdrawalContractAddress
addr === network.sbtcWithdrawalContractAddress ||
(network.allowedStakingContractAddresses ?? []).includes(addr)

@claude claude Bot Oct 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Every mainnet consumer will now build and parse pox-4 and send-many-memo calls to the third-party test deployer, not just its pox-5. isValidContractAddress trusts the configured address on its own. ContractBuilder.isContractPairValid then accepts any canonical name at any address that passes. Fix: scope the trust to the (pox-5, configured address) pair, checking it in isContractPairValid and Pox5Builder.isValidContractCall, and leave isValidContractAddress address-only for the canonical contracts.

Why this was flagged

The new clause at utils.ts:295 adds SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY to every check done through isValidContractAddress. ContractBuilder.contractAddress (contractBuilder.ts:108) therefore accepts it. isContractPairValid (contractBuilder.ts:66-73) returns true when isCanonicalName && isCanonicalAddress, and canonical names include pox-4 and send-many-memo. So a send-many-memo or pox-4 call to that deployer builds, and through factory.from(hex) it is rehydrated and explained like a trusted BitGo contract. On base it was rejected with 'Invalid contract address'. That deployer is an outside Stacks Foundation test key and can deploy a contract under any name, such as a send-many-memo that sends funds elsewhere. Explain/sign paths would show such a call as a normal send-many or stacking call. The doc comment's own @ obligation note says the address is trusted for every Stacks transaction. The PR describes the change as pox-5 only, and the test edit in contractBuilder.ts (swapping in a new dummy address) shows that cross-name pairing now happens.

Verification: On mainnet, a build or factory.from(hex) parse of a contract call whose deployer is SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY and whose name is pox-4 or send-many-memo passes. contractBuilder.ts:66-73 isContractPairValid returns true whenever isCanonicalName && isCanonicalAddress. On the base commit, line 110 throws "Invalid contract address" for this deployer.

);
}

Expand Down
75 changes: 71 additions & 4 deletions modules/sdk-coin-stx/test/unit/transactionBuilder/contractBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ describe('Stacks: Contract Builder', function () {
describe('Stx Contract call Builder', () => {
const factory = new StxLib.TransactionBuilderFactory(coins.get(coinNameTest));
const factoryProd = new StxLib.TransactionBuilderFactory(coins.get(coinName));
// Deployment-configured non-canonical pox-5 address (Stacks Foundation test contract) set on the
// mainnet `Stx` statics network as `allowedStakingContractAddresses`.
const CONFIGURED_STAKING_ADDRESS = 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY';

const initTxBuilder = () => {
const txBuilder = factory.getContractBuilder();
Expand Down Expand Up @@ -240,10 +243,11 @@ describe('Stacks: Contract Builder', function () {

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
// A dummy address that is NOT the network-configured pox-5 (otherwise the canonical `pox-5`
// name would legitimately pair with it) — keeps the allowlist pair check under test.
const dummyAddress = 'SP2T758K6T2YRKG9Q0TJ16B6FP5QQREWZSESRS0PY';
builder.allowedStakingContracts([{ name: 'pox-5-dummy-v1', address: dummyAddress }]);
builder.contractAddress(dummyAddress); // allowlisted address first
(() => builder.contractName('pox-5')).should.throw(/not an allowlisted pair/);
});

Expand Down Expand Up @@ -288,6 +292,69 @@ describe('Stacks: Contract Builder', function () {
);
});

it('accepts a deployment-configured staking address on the canonical pox-5 name (no allowlist)', async () => {
// The network statics carry the configured address, so a canonical pox-5 call targeting it
// builds/parses with no caller-supplied allowlist.
const builder = factoryProd.getContractBuilder();
builder.fee({ fee: '180' });
builder.nonce(0);
builder.contractAddress(CONFIGURED_STAKING_ADDRESS);
builder.contractName('pox-5');
builder.functionName('register-for-bond');
builder.functionArgs([
{ type: 'uint128', val: '210' },
{ type: 'principal', val: `${CONFIGURED_STAKING_ADDRESS}.signer-manager` },
{ type: 'uint128', val: '1005000' },
{ type: 'optional' },
{ type: 'optional' },
]);
builder.fromPubKey(testData.TX_SENDER.pub);
builder.numberSignatures(1);

const tx = await builder.build();
tx.toJson().payload.contractAddress.should.equal(CONFIGURED_STAKING_ADDRESS);
tx.toJson().payload.contractName.should.equal('pox-5');
});

it('rehydrates a configured-address pox-5 call with NO allowlist (OVC path)', async () => {
// The key behaviour: `factory.from(hex)` — used by sign / explain / fee-rebuild — must parse
// a register-for-bond targeting the configured address without any allowlist, so the offline
// vault console needs no code change.
const builder = factoryProd.getContractBuilder();
builder.fee({ fee: '180' });
builder.nonce(0);
builder.contractAddress(CONFIGURED_STAKING_ADDRESS);
builder.contractName('pox-5');
builder.functionName('register-for-bond');
builder.functionArgs([
{ type: 'uint128', val: '210' },
{ type: 'principal', val: `${CONFIGURED_STAKING_ADDRESS}.signer-manager` },
{ type: 'uint128', val: '1005000' },
{ type: 'optional' },
{ type: 'optional' },
]);
builder.fromPubKey(testData.TX_SENDER.pub);
builder.numberSignatures(1);
const tx = await builder.build();

// Routes to the pox-5 builder (parity) with no allowlist supplied.
should.equal(
StxLib.Pox5Builder.isValidContractCall(
coins.get(coinName),
(tx as StxLib.Transaction).stxTransaction.payload as any
),
true
);

const reparsed = factoryProd.from(tx.toBroadcastFormat());
reparsed.should.be.an.instanceOf(StxLib.Pox5Builder);
reparsed.fromPubKey(testData.TX_SENDER.pub);
reparsed.numberSignatures(1);
const payload = (await reparsed.build()).toJson().payload as any;
should.deepEqual(payload.contractAddress, CONFIGURED_STAKING_ADDRESS);
should.deepEqual(payload.contractName, 'pox-5');
});

it('a signed contract call with args', async () => {
const builder = initTxBuilder();
builder.functionArgs([
Expand Down
34 changes: 34 additions & 0 deletions modules/sdk-coin-stx/test/unit/transactionBuilder/pox5Builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,40 @@ describe('Stacks: PoX-5 Builder', function () {
);
});

it('accepts a deployment-configured non-canonical pox-5 address during factory routing', async () => {
// Mainnet `Stx` statics carry the Stacks Foundation test contract under
// `allowedStakingContractAddresses`, so a pox-5 call to it routes to the PoX-5 builder with no
// caller allowlist.
const factoryProd = new StxLib.TransactionBuilderFactory(coins.get('stx'));
const configuredAddress = 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY';
const builder = configure(factoryProd.getPox5Builder());
builder.contractAddress(configuredAddress);
builder.registerForBond({
bondIndex: 210,
signerManager,
amountUstx: '1005000',
lockup: { kind: 'btc', unlockBytes: '00', outputs: [validLockupOutput()] },
});
const tx = await builder.build();
const payload = (tx as StxLib.Transaction).stxTransaction.payload as any;

should.equal(StxLib.Pox5Builder.isValidContractCall(coins.get('stx'), payload), true);
// an unknown address is still rejected
should.equal(
StxLib.Pox5Builder.isValidContractCall(coins.get('stx'), {
...payload,
contractAddress: createAddress(testData.ACCOUNT_1.address),
}),
false
);

// No allowlist: the raw tx rehydrates back through the PoX-5 builder.
const reparsed = factoryProd.from(tx.toBroadcastFormat());
reparsed.should.be.an.instanceOf(StxLib.Pox5Builder);
reparsed.fromPubKey(testData.TX_SENDER.pub);
should.equal((await reparsed.build()).toBroadcastFormat(), tx.toBroadcastFormat());
});

it('builds sBTC registration using response error', async () => {
const builder = configure(factory.getPox5Builder());
builder.registerForBond({
Expand Down
21 changes: 21 additions & 0 deletions modules/sdk-coin-stx/test/unit/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
uintCV,
} from '@stacks/transactions';
import BigNum from 'bn.js';
import { coins } from '@bitgo/statics';
import * as testData from './resources';
import { StxLib } from '../../src';

Expand Down Expand Up @@ -123,6 +124,26 @@ describe('Stx util library', function () {
});
});

describe('isValidContractAddress', function () {
const network = coins.get('stx').network as any;
const configuredAddress = 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY';

it('accepts the canonical staking address and a deployment-configured address', function () {
Utils.isValidContractAddress(network.stakingContractAddress, network).should.be.true();
Utils.isValidContractAddress(network.sendmanymemoContractAddress, network).should.be.true();
Utils.isValidContractAddress(configuredAddress, network).should.be.true();
});

it('rejects an address that is neither canonical nor configured', function () {
Utils.isValidContractAddress('SP2T758K6T2YRKG9Q0TJ16B6FP5QQREWZSESRS0PY', network).should.be.false();
});

it('does not accept the configured address on a network that does not carry it', function () {
const testnet = coins.get('tstx').network as any;
Utils.isValidContractAddress(configuredAddress, testnet).should.be.false();
});
});

describe('amount', function () {
it('valid amount', function () {
Utils.isValidAmount('10').should.be.true();
Expand Down
14 changes: 14 additions & 0 deletions modules/statics/src/networks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,16 @@ export interface StacksNetwork extends AccountNetwork {
readonly chainId?: number;
/** Transaction version byte. Omitted for the stock Stacks mainnet/testnet deployments. */
readonly transactionVersion?: number;
/**
* Additional staking contract deployer addresses this network accepts, in addition to the
* canonical `stakingContractAddress`. Deployment-specific and temporary: used to allow a
* non-canonical pox-5 deployment (e.g. the Stacks Foundation test contract) without the caller
* passing an allowlist. Empty/omitted by default, so behaviour is unchanged unless configured.
*
* @obligation remove once the test deployment is retired — an address listed here is trusted for
* every Stacks transaction on this network
*/
readonly allowedStakingContractAddresses?: ReadonlyArray<string>;
}

// eslint-disable-next-line @typescript-eslint/no-empty-interface
Expand Down Expand Up @@ -1362,6 +1372,10 @@ class Stx extends Mainnet implements StacksNetwork {
sendmanymemoContractAddress = 'SP3FBR2AGK5H9QBDH3EEN6DF8EK8JY7RX8QJ5SVTE';
stakingContractAddress = 'SP000000000000000000002Q6VF78';
sbtcWithdrawalContractAddress = 'SM3VDXK3WZZSA84XXFKAFAF15NNZX32CTSG82JFQ4';
// Temporary: the Stacks Foundation pox-5 test contract (standard name `pox-5`, non-canonical boot
// address). Allows transactions targeting it to parse/build without a caller-supplied allowlist.
// Remove with the test.
allowedStakingContractAddresses = ['SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY'];
Comment on lines +1375 to +1378

@claude claude Bot Oct 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Every mainnet Stx consumer will build, parse and route real-fund pox-5 staking calls to a third-party test deployer, with no opt-in and no expiry. The constant sits on the shared mainnet Stx network, so Pox5Builder.isValidContractCall and factory.from(hex) treat SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY.pox-5 as canonical pox-5 for every package. Fix: limit the trust to deployments that ask for it. For example, keep the network field empty by default and let a consumer such as the vault turn it on through config, or add a hard expiry such as a block-height or date check, while keeping the caller allowlist override.

Why this was flagged

The mainnet Stx class at networks.ts:1378 sets allowedStakingContractAddresses = ['SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY']. Every package that imports @ bitgo/statics gets this value, not only the staking service and the vault. Pox5Builder.isValidContractCall (pox5Builder.ts:174-176) now sends any tx whose payload targets that address with name pox-5 to the Pox5Builder. ContractBuilder.contractAddress accepts it through isValidContractAddress (utils.ts:295), so a mainnet register-for-bond or stake that locks user STX in a test contract builds, explains and signs as a normal pox-5 staking call. On the base branch that call is rejected with 'Invalid contract address' unless the caller passes allowedStakingContracts, which keeps the trust deployment-scoped. The PR description calls this an accepted trade-off. That note does not bound the exposure: nothing in the code ties the entry to an expiry, environment or caller.

Verification: Trigger: any mainnet consumer of @ bitgo/statics builds or parses a contract call whose deployer is SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY. networks.ts:1378 puts the address on the shared mainnet Stx class. utils.isValidContractAddress (utils.ts:295) accepts the address for any contract name. It is then accepted where the base rejected it with 'Invalid contract address'.

}

class StxTestnet extends Testnet implements StacksNetwork {
Expand Down
Loading