[Doddanna17]

Context

The Stacks Foundation PoX-5 test contract is deployed with the standard name pox-5 at a non-canonical boot address (SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY). The SDK's Utils.isValidContractAddress is address-only, so a register-for-bond targeting it is rejected on every parse unless the caller supplies the staking allowlist.

The offline vault signs custody wallets and re-parses the tx to explain/sign, so it needs to accept the address — but the allowlist must not come from the transaction, or the check would no longer constrain whoever produced it.

Change

Pin the allowance in the network statics (compiled into the SDK), as a pair with an expiry:

File	Change
modules/statics/src/networks.ts	StacksNetwork.allowedStakingContracts?: ReadonlyArray<{ name; address; expiresAt? }>; mainnet Stx pins { name: 'pox-5', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY', expiresAt: '2026-11-02' }
modules/sdk-coin-stx/src/lib/utils.ts	adds isExpired(); isValidContractAddress stays address-only
modules/sdk-coin-stx/src/lib/contractBuilder.ts	isContractPairValid (and contractAddress) consult the caller pairs plus the active pinned pairs
modules/sdk-coin-stx/src/lib/pox5Builder.ts	isValidContractCall accepts the canonical address or a matching non-expired pinned pair

The caller-supplied allowedStakingContracts remains a working override.

Why the address is pinned, not taken from the transaction

An earlier revision read the allowlist from coinSpecific on the exported tx. That was rejected: the allowlist and txHex then arrive in the same object, so a compromised build service could point the tx at its own address and supply a matching allowlist — the check became self-certifying.

Here the address is compiled into the SDK. A transaction can never supply or redirect it: the SDK only ever accepts the one pinned pair, so a spoofed export cannot make the vault sign a call to any other contract.

Scope, and why it is bounded by time

The pin is on the shared Stx network, so it applies to every Stx consumer until it expires. That is a deliberate, time-bounded trade: the address is pinned and self-expiring, and it only ever admits the Foundation's test contract — never an arbitrary one.

After 2026-11-02 the entry is ignored and the SDK reverts to canonical-only, failing closed:

expiry passes -> entry filtered out
  build:  contractAddress(SP3JVSE...) -> throws "Invalid contract address"
  parse:  factory.from(hex)           -> ParseTransactionError
  Pox5Builder.isValidContractCall      -> false -> plain ContractBuilder -> same throw

Only the register path uses it

The target resolution is register-only (wallet-platform):

contractAddress: isPox5RegisterForBond
  ? pox5StakingContractAddress ?? stakingContractAddress
  : stakingContractAddress

So transfers, pox-4 stack-stx/delegate-stx, the canonical pox-5 register, send-many, sBTC withdraw and FT transfers all keep the existing contract and flow. The SDK change only makes the pinned address acceptable — it never redirects anything.

Commits

SHA	Subject
95df8aa13a	feat(sdk-coin-stx): accept a configured PoX-5 staking contract pair

Tests

• pox-5 at the pinned address → accepted (via isContractPairValid and Pox5Builder.isValidContractCall), including factory.from(hex) with no caller allowlist
• pox-4 at the pinned address → rejected; send-many-memo at the pinned address → rejected
• unknown address → rejected
• expiry: expired → rejected (/Invalid contract address/); future and absent → accepted
• canonical pox-5 / pox-4 → still accepted
• caller-supplied allowlist override → still works
• isValidContractAddress remains address-only
• the new tests use no bare any (real ContractCallPayloads where possible, typed casts otherwise)

The expiry tests inject their own pairs (expiresAt: '2999-01-01' / '2020-01-01') rather than reading the shipped date, so they don't break when 2026-11-02 passes.

cd modules/statics      && yarn tsc --build --incremental .
cd modules/sdk-coin-stx && BITGOJS_TEST_PASSWORD=test yarn unit-test   # 350 passing, 0 failing
cd modules/statics      && BITGOJS_TEST_PASSWORD=test yarn unit-test   # 41258 passing, 0 failing

Related

• SI-2089 (parent) · SI-2098 (staking-service reads) · SI-2026 (vault)
• Supersedes feat(sdk-coin-stx): accept a configured PoX-5 staking contract address #10058 (address-only statics)
• Consumer: the vault needs only a dependency bump (sdk-coin-stx + statics) — no code change

[linear-code]

SI-2089

[github-actions]

⚠️ Unit tests are failing on Node 26.x (Current release line, non-blocking). This is not an LTS version yet, so it does not block merge, but it signals an incompatibility to fix before Node 26.x becomes LTS.

View run

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[Doddanna17]

@claude review

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[Doddanna17]

Verify finding — addressed for the allowance; the public-keys gap is deliberately out of scope.

verifyTransaction now forwards the prebuild's coinSpecific to its internal explain call, so it resolves the allowance exactly like explainTransaction and signTransaction:

const explainedTx = await this.explainTransaction({
  txHex: rawTx,
  feeInfo: { fee: '' },
  coinSpecific: (txPrebuild as { coinSpecific?: ExplainTransactionOptions['coinSpecific'] })?.coinSpecific,
});

The test asserts the effect: with the prebuild allowance, verify now gets past parsing and fails only on the pre-existing gap (supply at least 1 public key); without it, it still fails at parsing (fail-closed).

We are deliberately not changing the public-keys handling. verifyTransaction calls explain without publicKeys, and a contract-call build requires at least one — but:

• that is pre-existing (true on master),
• it is unrelated to the allowance (address vs keys),
• and it would mean changing key handling in a shared verification path, which we do not want to ride along with this change.

So: the allowance is aligned across verify/explain/sign; the keys gap stands as-is and can be raised separately if hot-wallet contract-call verification is ever needed. The register-for-bond flow itself is unaffected — custody uses explainTransaction + signTransaction, both of which resolve the allowance (covered by tests).

[Doddanna17]

Addressed — pushed in fa56ae9716.

verifyTransaction now forwards the prebuild's coinSpecific to its internal explain call, so it resolves the staking allowance exactly like explainTransaction and signTransaction:

const explainedTx = await this.explainTransaction({
  txHex: rawTx,
  feeInfo: { fee: '' },
  coinSpecific: (txPrebuild as { coinSpecific?: ExplainTransactionOptions['coinSpecific'] })?.coinSpecific,
});

Tests cover it both ways:

• with the prebuild allowance → verify gets past parsing (the finding is fixed) and fails only on the pre-existing keys gap;
• without it → still fails at parsing (fail-closed).

The public-keys handling is deliberately unchanged — verifyTransaction still calls explain without publicKeys. That gap is pre-existing (true on master), unrelated to the allowance (address vs keys), and changing key handling in a shared verification path is out of scope here. The register-for-bond flow is unaffected: custody uses explainTransaction + signTransaction, both of which resolve the allowance (asserted by tests).

Current state: 1 commit, fa56ae9716, diff limited to modules/sdk-coin-stx — no modules/statics change, no keys change. 350 sdk-coin-stx / 41258 statics tests passing; prettier + eslint + commitlint clean.

[Doddanna17]

@claude review

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[claude]

Findings marked 🟡 are optional suggestions and need no follow-up push.

Still open from earlier reviews (1):

• 🔴 modules/sdk-coin-stx/src/stx.ts:299 — A compromised or spoofed build service can now get the SDK to sign a staking call to any contract, because the same pay…

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

[Doddanna17]

Accepted — the finding is correct, and the PR is reworked to fix it.

You're right that taking the allowlist from coinSpecific made the check self-certifying: the allowlist and txHex arrive in the same object, so a spoofed build service could aim the tx at its own address and supply a matching allowlist. Pushed in 99f449c093.

The address is now pinned in the SDK, not supplied by the transaction. It lives in the network statics and is the only non-canonical pair the SDK will accept:

// modules/statics/src/networks.ts — mainnet Stx
allowedStakingContracts = [
  { name: 'pox-5', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY', expiresAt: '2026-11-02' },
];

isContractPairValid and Pox5Builder.isValidContractCall consult that pinned list; isValidContractAddress stays address-only. A transaction can never supply or redirect the address, so a compromised producer can no longer get the vault to sign a call to any contract — the only address the SDK will admit is the pinned one.

Scope is deliberately global and time-bounded. The pin sits on the shared Stx network, so it applies to every Stx consumer until it expires. That is the explicit trade for keeping the vault unchanged: the address is pinned and self-expiring, and it only ever admits the Foundation's test contract. Past 2026-11-02 the entry is filtered out and the SDK reverts to canonical-only, failing closed (Invalid contract address / ParseTransactionError).

Only the register path uses it. wallet-platform resolves the register target only when it is a register-for-bond (isPox5RegisterForBond ? configured : canonical), so transfers, pox-4 stacking, the canonical pox-5 register, send-many, sBTC withdraw and FT transfers all keep the existing contract and flow. The SDK change only makes the pinned address acceptable; it never redirects anything.

Tests: pair accepted (incl. from(hex) with no caller allowlist), pox-4 / send-many-memo at that address rejected, expiry both directions (expired → throws; future/absent → accepted), canonical and caller-supplied override intact. The expiry tests inject their own pairs rather than reading the shipped date, so they don't break when the date passes.

If you'd rather have per-deployment scoping than a global time-bounded pin, that requires the vault to hold the allowlist — i.e. a vault change and release — since the signer's only no-change knowledge is its bundled SDK. Happy to take that route if you prefer it.

[Doddanna17]

@claude review and share the approval status as comment.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

[Vijay-Jagannathan]

Design is sound: the pair-scoped, SDK-pinned allowance fixes the self-certifying problem from the earlier coinSpecific approach, and isValidContractAddress staying address-only is the right call.

One blocking item: the PR says the shipped date can't break the tests, but three tests still read the real mainnet pin and the real clock. They fail from 2026-11-02T00:00Z with no code change. Details inline. Everything else is optional.

[Doddanna17]

All three blocking items fixed, plus the shipped-entry gap — pushed in 95df8aa13a.

1. Time bomb in contractBuilder.ts (the pox-4 / send-many-memo rejections). Both now build from an injected non-expired pair:

new StxLib.TransactionBuilderFactory(
  withPairs([{ name: 'pox-5', address: CONFIGURED_ADDRESS, expiresAt: '2999-01-01' }])
).getContractBuilder();

so the assertion stays the pair check (/not an allowlisted pair/) and never flips to Invalid contract address once the shipped pin expires.

2. Time bomb in pox5Builder.ts. The configured-pair assertions now use an injected activeConfig (expiresAt: '2999-01-01') instead of coins.get('stx'). The canonical-address assertion still uses the shipped config, which is correct — canonical doesn't depend on the pin. Remaining factoryProd usages in the file are canonical / unknown-address assertions only.

3. Shipped-entry gap — closed. Added a test that asserts the shipped mainnet statics entry, so a typo in the pinned address is caught:

should.deepEqual(
  (mainnet.allowedStakingContracts ?? []).map((c) => ({ name: c.name, address: c.address })),
  [{ name: 'pox-5', address: 'SP3JVSE4ZB5GMDJP5XBPX7ZB1BG3QDFW7RCEQJDMY' }]
);

It asserts only the name/address, so it doesn't depend on the expiresAt value or the clock. (351 passing now.)

4. Optional — all pox-5 functions admitted. Noted in the statics comment: the pinned pair admits every function Pox5Builder supports at that address (stake, unstake, update-bond-registration, …), not only register-for-bond; the register-only restriction lives in wallet-platform. Intentional for this test deployment, now stated explicitly.

5. Optional — statics comment. Updated to record the removal obligation (with the SI-2089 removal ticket), that the deployer SP3JVSE… is the Stacks Foundation's test deployment, and that '2026-11-02' parses as 00:00Z so the allowance ends at the start of Nov 2.

sdk-coin-stx  351 passing, 0 failing
statics       41258 passing, 0 failing

[Vijay-Jagannathan]

Re-review of 95df8aa. Both blocking items from my earlier review are fixed:

• The pox-4 and send-many-memo rejection tests in contractBuilder.ts now use withPairs([... expiresAt: '2999-01-01']), so they still assert the pair check after the shipped pin expires.
• pox5Builder.ts now builds activeConfig and expiredConfig from injected pairs. Only the canonical assertion uses coins.get('stx'), and that doesn't depend on the pin.
• A new test in util.ts pins the shipped name and address, so a typo in networks.ts would fail. It deliberately ignores expiresAt and the clock.
• The statics comment now documents the all-pox-5-functions scope and the removal obligation.

CI is green, including unit tests on Node 22, 24 and 26. I did not run the tests locally.

One non-blocking nit inline: a comment in networks.ts no longer matches the code.