Skip to content

fix(sdk-coin-sol): resolve SOL token payment recipients during TSS verify-before-sign - #9883

Closed
rishikeshdadam136 wants to merge 1 commit into
masterfrom
WCN-2952-followup/eddsa-sol-token-payment-verify
Closed

rishikeshdadam136 wants to merge 1 commit into
masterfrom
WCN-2952-followup/eddsa-sol-token-payment-verify

Conversation

@rishikeshdadam136

@rishikeshdadam136 rishikeshdadam136 commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Problem

SOL SPL token payment withdrawals (and manual sweeps-to-self) fail to sign with:

Tx outputs does not match with expected txParams recipients

Observed in production (MPCv2 TSS hot SOL wallet, web SDK): user creates a payment intent with a native recipient address, clicks Sign — the SDK's pre-sign verification throws and no signature share is ever submitted. Platform logs show zero server-side errors: the transaction builds and rebuilds fine; only the client-side gate rejects it.

Blast radius: any SPL token, any TSS SOL wallet (MPCv1 + MPCv2 on current bundles), signing without explicit txParams — UI Sign, or any API/SDK signer that omits txParams. The current UI and retail pins (@bitgo-beta/sdk-core 8.2.1-beta.2079, published Oct 1) contain the enforcement, so the payment path is broken in production until this ships.

Root cause

On Solana, a token payment delivers funds to the recipient's associated token account (ATA) — a different address than the native wallet address the user entered. The pre-sign check must therefore re-derive the ATA client-side to prove the output belongs to the intended recipient, which requires resolving the token mint from the recipient's tokenName.

The UI signs without txParams, so verification uses intent-fallback recipients (resolveEffectiveTxParams): { address: <native>, amount, tokenName: 'sol:usdt' } — no tokenAddress, no programId. When the name→mint lookup fails (unprefixed tokenData.tokenName, tokens outside the statics map, or a runtime where the statics lookup is unavailable), the mint cannot be resolved and the check treats every legitimate SOL token payment as a mismatch.

History: this enforcement was added to the EdDSA sign path in WCN-196 (a70114f, Jun 11), reverted Jun 25 (96658f1) for breaking signing, and re-introduced by WCN-2113 (PR #9813, Sep 24; first customer failures Sep 28). WCN-2952 (PR #9878) fixed the same regression for consolidate intents by verifying sweep-to-base-address instead — payment intents still take the unchanged recipients comparison, which is the gap this PR closes.

Changes

sdk-coin-sol — verifyTransaction recipients check

  1. Chain-prefixed name retry — when the bare tokenName does not resolve, retry ${chain}:${tokenName} (usdt → sol:usdt); wallet-platform persists tokenData.tokenName without the prefix on some intent types.
  2. Output-mint fallback — when the recipient's tokenName is a genuine token NAME (not a mint address) that still does not resolve and no tokenAddress is carried, take the mint from the explained output (the token actually moved — explainTransaction falls back to the mint as tokenName via useTokenAddressTokenName) and prove the output is the recipient's ATA for that mint.
  3. Total-amount check — per-asset totals were grouped by the raw tokenName string on both sides, so spelling differences (usdt vs sol:usdt) produced a second, different failure (Tx total amount does not match with expected total amount field). Both sides now group by canonical asset key (resolved mint), and a recipient whose name is unresolvable adopts the output-side key.

sdk-core — carry token identity onto intent-fallback recipients

  1. resolveEffectiveTxParams now maps tokenAddress (from recipient-level tokenAddress — persisted by wallet-platform on SOL consolidateToken intents — or tokenData.tokenContractAddress for EVM-style token intents) and programId (recipient-level tokenProgramId) onto fallback recipients. IntentRecipient declares these optional fields. When the data is present, the check gets the mint directly and never depends on a name lookup.

Security analysis

  • Unsupported-token rule preserved: if the recipient's tokenName is a mint address, the output-mint fallback is gated off (!isValidAddress(recipientFromUser.tokenName)) — the tx side never vouches for the token, and an explicit tokenAddress is still required. The existing rejection tests for that flow are unchanged and green.
  • Ownership always proven: whatever the mint source, the output must equal the ATA derived for the intent recipient's address — a transaction paying any other token account is rejected (new test).
  • Name→mint binding: in a healthy runtime the name lookup resolves and the fallback never runs; it only engages when client-side resolution is impossible, where the alternative is rejecting 100% of legitimate SOL token sends. The platform builds the tx from the intent server-side, so the tx mint is the intent's token by construction.
  • No change to transaction building, signing, broadcasting, amount comparison, or other coins (TRX/NEAR/etc. share the sign flow, not this check's SOL logic).

Test plan

New tests (all red on master, green with this PR):

  • should succeed to verify token transaction when recipient tokenName is not chain-prefixed
  • should succeed to verify token transaction for a token name outside the statics map when the explained output carries the mint
  • should fail to verify token transaction when the output token account belongs to a different owner (fallback still proves ownership)
  • sdk-core: recipient-level SPL token identity (tokenAddress/tokenProgramId) and tokenData.tokenContractAddress are carried onto fallback recipients

Unchanged and green:

  • All 9 pre-existing token-verification tests, including the security rejections (wrong tokenName, wrong amounts, wrong native address, unsupported token without tokenAddress, wrong Token-2022 programId)
  • Full verify suite in sdk-coin-sol/test/unit/sol.ts: 56 passing
  • sdk-core recipientUtils suite: 40 passing

Rollout

Fix reaches users via: merge → next @bitgo-beta/sdk-core + @bitgo-beta/sdk-coin-sol publish → bump in bitgo-ui (apps/app) and bitgo-retail (apps/retail-web) → deploy. Note the beta line is cut from a release branch (WCN-2952 merged Oct 1 13:15 UTC yet the same-day 19:50 beta did not contain it) — verify the fix is present in the published tarball before declaring victory.

Interim workaround for affected customers: sign via API with explicit txParams.recipients including tokenName + tokenAddress + programId — caller-supplied recipients take precedence and pass on all affected versions.

Related: WCN-2113 (PR #9813 — enforcement re-introduction), WCN-2952 (PR #9878 — consolidation counterpart), WCN-196 (original enforcement + revert).

@linear-code

linear-code Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

WCN-2952

@rishikeshdadam136
rishikeshdadam136 force-pushed the WCN-2952-followup/eddsa-sol-token-payment-verify branch from 80ce9d3 to ebdf11b Compare October 3, 2026 11:48
…rify-before-sign

WCN-2113 re-introduced the pre-sign recipient verification in the EdDSA TSS
sign flows. For SOL SPL payment intents the UI signs without txParams, so
verification compares intent-fallback recipients (native address + tokenName
from amount.symbol) against the explained transaction output (the
recipient's associated token account). When the recipient's token name does
not resolve in the statics map - unprefixed tokenData.tokenName ('usdt'),
tokens registered outside the statics map, or a runtime where the statics
lookup is unavailable - the mint cannot be resolved and every SOL token
payment sign fails with 'Tx outputs does not match with expected txParams
recipients'.

- verifyTransaction: retry the token lookup with the chain-prefixed
spelling ('usdt' -> 'sol:usdt'); when a recipient's tokenName is a genuine
token NAME that still does not resolve and no tokenAddress is carried, use
the mint from the explained output (the token actually moved) and prove the
output is the recipient's associated token account for that mint.
Recipients whose tokenName IS a mint address (unsupported-token shape) keep
requiring an explicit tokenAddress.

- verifyTransaction total-amount check: group per-asset totals by canonical
asset key (resolved mint) on both the recipient and output sides so the
name-spelling fallbacks above do not break the totals comparison.

- sdk-core resolveEffectiveTxParams: carry recipient-level
tokenAddress/tokenProgramId (SOL consolidateToken intents) and
tokenData.tokenContractAddress (EVM-style token intents) onto
intent-fallback recipients.

- sdk-core IntentRecipient: declare the optional tokenAddress/tokenProgramId
fields wallet-platform persists on SOL consolidateToken intent recipients.

Related: WCN-2113 (verification re-introduction), WCN-2952 (consolidation
follow-up, PR #9878). This is the payment-intent counterpart: SOL token
payment signing is broken for MPCv2 TSS wallets signing without explicit
txParams.

TICKET: WCN-2952
@rishikeshdadam136
rishikeshdadam136 force-pushed the WCN-2952-followup/eddsa-sol-token-payment-verify branch from ebdf11b to a16ab0f Compare October 3, 2026 12:03
@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

@rishikeshdadam136

Copy link
Copy Markdown
Contributor Author

@claude can you review this pr

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @rishikeshdadam136's task —— View job


PR Review for SOL Token Payment Verification Fix

Todo List:

  • Read repository guidelines from CLAUDE.md
  • Analyze core changes in sol.ts verifyTransaction method
  • Review test coverage and quality in sol test files
  • Examine sdk-core changes (baseTypes.ts, recipientUtils.ts)
  • Check for potential bugs, security issues, and edge cases
  • Provide comprehensive review feedback

Review Summary

This PR successfully addresses the SOL token payment verification issue for MPCv2 TSS wallets when signing without explicit txParams. The implementation includes thoughtful fallback mechanisms and proper test coverage.

Code Quality Assessment: ✅ APPROVED

Key Changes Analysis

1. SOL Token Name Resolution Enhancement (sol.ts:667-684)

✅ Excellent Implementation

  • Chain-prefixed fallback: Properly retries token lookup with sol: prefix when bare token name fails
  • Output-mint fallback: Smart use of explained transaction output as source of truth for unresolvable tokens
  • Security boundary: Correctly restricts mint fallback to genuine token NAMEs (not mint addresses) to prevent tx-side vouching for unsupported tokens

2. Total Amount Verification Improvement (sol.ts:756-796)

✅ Robust Solution

  • Canonical asset keys: Groups amounts by resolved mint address instead of token name
  • Consistent fallback logic: Mirrors the recipient verification approach for asset key resolution
  • Endianness handling: Properly applies s390x fix to output amounts

3. SDK-Core Integration (baseTypes.ts, recipientUtils.ts)

✅ Clean Design

  • Optional fields: Well-documented tokenAddress and tokenProgramId fields on IntentRecipient
  • Cross-platform support: Handles both SOL-style (tokenAddress) and EVM-style (tokenContractAddress) token intents
  • Backward compatibility: Optional fields preserve existing API contracts

Security Analysis

✅ Security Controls Maintained

  1. Address validation: Still requires isValidAddress() checks for all addresses
  2. ATA verification: Maintains proof that output address is the recipient's associated token account
  3. Ownership validation: Prevents accepting token accounts owned by different addresses
  4. Mint address restrictions: Only allows mint fallback for genuine token names, not mint addresses

✅ No Security Vulnerabilities Identified

  • Input sanitization is properly maintained
  • No potential for injection attacks
  • Appropriate error handling for edge cases

Test Coverage Assessment

✅ Comprehensive Test Suite

  1. Chain-prefixed fallback test: Validates usdc → sol:usdc retry logic
  2. Unsupported token test: Tests mint fallback for tokens outside statics map
  3. Negative test case: Ensures rejection when output belongs to different owner
  4. SDK-core tests: Covers both SOL and EVM token identity propagation

Code Style & Standards

✅ Follows Repository Guidelines

  • Consistent with existing code patterns
  • Proper TypeScript typing
  • Clear variable naming and documentation
  • Follows conventional commit format

Minor Observations

📝 Documentation Excellence

The inline comments explaining the fallback logic are exceptionally clear and help future maintainers understand the complex token resolution flow.

📝 Error Handling

The existing error message "Tx outputs does not match with expected txParams recipients" remains appropriate and provides clear feedback when verification fails.

Performance Considerations

✅ Efficient Implementation

  • Token lookups are performed only when needed
  • Caching is used appropriately in consolidation verification
  • No unnecessary network calls or computations

Recommendation: MERGE APPROVED ✅

This PR successfully solves the reported issue with a well-engineered solution that:

  • Maintains backward compatibility
  • Includes proper security controls
  • Has comprehensive test coverage
  • Follows established code patterns
  • Addresses edge cases appropriately

The implementation demonstrates deep understanding of the SOL token ecosystem and TSS verification requirements. The fallback mechanisms are elegantly designed and properly tested.


CI Status: ⚠️ Node 26.x unit test failure is noted as non-blocking (not LTS version yet)

@rishikeshdadam136 rishikeshdadam136 changed the title fix(sdk-coin-sol): resolve SOL token payment recipients during TSS ve… fix(sdk-coin-sol): resolve SOL token payment recipients during TSS verify-before-sign Oct 3, 2026
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.

1 participant