Skip to content

fix(sdk-core): keep intent tokenName for token-wallet TSS signing - #9884

Draft
ralph-bitgo[bot] wants to merge 2 commits into
masterfrom
WCI-1723-sol-tss-recipient-verify
Draft

ralph-bitgo[bot] wants to merge 2 commits into
masterfrom
WCI-1723-sol-tss-recipient-verify

Conversation

@ralph-bitgo

@ralph-bitgo ralph-bitgo Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Description

SOL:USDT / SOL:USDC withdrawals and SOL consolidations from the affected SOL TSS
hot wallet failed UI signing with Tx outputs does not match with expected txParams recipients, leaving every txRequest stuck in pendingDelivery,
unsigned, with no txid (customer blocked; Case376984).

Root cause (token withdrawals). On a token wallet, baseCoin.getChain()
returns the token name itself (e.g. sol:usdt). populateIntent therefore
persists amount.symbol = 'sol:usdt' (sendMany on a token wallet carries no
recipient tokenName), but at signing time resolveEffectiveTxParams treated a
symbol equal to chainName as a native transfer and dropped the tokenName
fallback. Sol.verifyTransaction then had no mint to derive the recipient's
associated token account from, and rejected the transaction.

Fix. resolveEffectiveTxParams
(modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts) now keeps
amount.symbol as tokenName when chainName is itself a statics-registered
token — i.e. the wallet is a token wallet. Native wallets keep the strict
symbol !== chainName rule; names registered nowhere (neither statics nor the
runtime/AMS registry that GlobalCoinFactory feeds into it) are left unchanged
rather than guessed, so the fail-closed posture is preserved.

Consolidations were already fixed on master by WCN-2952 (sweep-to-base-address
verification); this PR closes the remaining token-withdrawal path.

Pending requests. The six listed txRequests carry valid intents and unsigned
transactions, and verification runs at signing time in the SDK — so once the UI
runs a SDK build containing this fix (plus WCN-2952 for the consolidations), they
can be signed as-is and do not need to be rejected and recreated.

Issue Number

Ticket: WCI-1723

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • New regression tests in
    modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts: token wallet
    (sol:usdt / tsol:usdc with chainName = symbol) keeps tokenName;
    native transfer (symbol === chainName === 'tsol') still drops it; chain
    name not in statics still drops it; all RED against the pre-fix code, GREEN
    after.
  • New end-to-end tests in modules/sdk-coin-sol/test/unit/sol.ts: a mainnet
    sol:usdt withdrawal resolved through resolveTssVerifyTransactionOptions
    with a production-shaped intent (symbol = token chain, no tokenData) passes
    verifyTransaction; a tampered intent recipient is still rejected with the
    exact production error.
  • Suites: sdk-coin-sol 790 passing; sdk-core 890 passing; bitgo tssUtils
    subsets match the master baseline (11 pre-existing failures: missing native
    sodium functions and pre-existing assertions, identical on master).
  • sdk-coin-sol and sdk-core clean builds (tsc) green; eslint clean;
    commitlint passes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My code compiles correctly for both Node and Browser environments
  • I have commented my code, particularly in hard-to-understand areas
  • My commits follow Conventional Commits and I have properly described any BREAKING CHANGES
  • The ticket or github issue was included in the commit message as a reference
  • I have made corresponding changes to the documentation and on any new/updated functions and/or methods - jsdoc
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Ticket: WCI-1723

@linear-code

linear-code Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

WCI-1723

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ 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

What changed:
- resolveEffectiveTxParams no longer drops amount.symbol as tokenName
  when it equals chainName: a statics-registered chainName that is itself
  a token (sol:usdt, sol:usdc, ...) proves the wallet is a token wallet,
  so the intent symbol always identifies a token transfer. Native wallets
  keep the strict symbol !== chainName behavior, and chain names absent
  from statics (dynamic/AMS tokens) are left unchanged to avoid guessing.
Why:
Signing a SOL:USDT/SOL:USDC withdrawal or consolidation from a token
wallet in the UI failed with 'Tx outputs does not match with expected
txParams recipients', leaving every request stuck pendingDelivery and
blocking the customer (WCI-1723). populateIntent stores
amount.symbol = baseCoin.getChain() because sendMany on a token wallet
carries no tokenName, so at signing time the symbol matched chainName,
the tokenName fallback was skipped, and verifyTransaction could not
derive the recipient's associated token account to compare outputs.

Ticket: WCI-1723
Session-Id: b7c7cfa9-c342-4d89-a7e4-8dae52576077
Task-Id: 43b26db5-5b57-41d7-94f3-5a3ffb828801
@ralph-bitgo
ralph-bitgo Bot force-pushed the WCI-1723-sol-tss-recipient-verify branch from 2ae815e to f5eb879 Compare October 3, 2026 12:04
What changed:
- isTokenChainName doc and the unregistered-name test comment no longer
  claim dynamic/AMS tokens resolve to false: GlobalCoinFactory.registerToken
  inserts runtime tokens into the same statics coin map that
  isTokenChainName queries, so registered AMS tokens are detected as tokens;
  only names registered nowhere stay false. Also aligned the
  resolveTssVerifyTransactionOptions chainName doc with the token-wallet
  reality (chain name is the token name itself for token wallets).

Why:
The comments shipped with the WCI-1723 fix described the registry
incorrectly and could mislead a future maintainer into thinking AMS token
wallets are unsupported by the fix when they are in fact covered.

Ticket: WCI-1723
Session-Id: b7c7cfa9-c342-4d89-a7e4-8dae52576077
Task-Id: 43b26db5-5b57-41d7-94f3-5a3ffb828801

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants