From f5eb879c14e2713097279a4e8cd426e2ad844604 Mon Sep 17 00:00:00 2001 From: bitgobot Date: Sat, 3 Oct 2026 11:46:08 +0000 Subject: [PATCH 1/2] fix(sdk-core): keep intent tokenName for token-wallet TSS signing 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 --- modules/sdk-coin-sol/test/unit/sol.ts | 80 +++++++++++++++++++ .../src/bitgo/utils/tss/recipientUtils.ts | 38 +++++++-- .../unit/bitgo/utils/tss/recipientUtils.ts | 56 +++++++++++++ 3 files changed, 169 insertions(+), 5 deletions(-) diff --git a/modules/sdk-coin-sol/test/unit/sol.ts b/modules/sdk-coin-sol/test/unit/sol.ts index 5a0618140f..08be013b77 100644 --- a/modules/sdk-coin-sol/test/unit/sol.ts +++ b/modules/sdk-coin-sol/test/unit/sol.ts @@ -25,6 +25,7 @@ import { MPCSweepTxs, MPCTx, MPCTxs, + resolveTssVerifyTransactionOptions, signRecoveryEddsaMPCv2, PrebuildAndSignTransactionOptions, TransactionPrebuild, @@ -1051,6 +1052,85 @@ describe('SOL:', function () { }); }); + describe('TSS signing-time verification of token wallet withdrawals (WCI-1723)', () => { + // Signing path for token wallets: eddsaMPCv2.signTxRequest resolves recipients + // from the persisted intent and passes baseCoin.getChain() as chainName — for a + // token wallet that chain name is the token itself (e.g. 'sol:usdt'). populateIntent + // stores amount.symbol = 'sol:usdt' because sendMany on a token wallet does not + // include tokenName, so the symbol must be preserved as tokenName for + // verifyTransaction to derive the recipient's associated token account. + let bitgoMainnet: TestBitGoAPI; + let solMainnet: Sol; + + const recipient = 'E7Z6pFfUhjx2dFjdB9Ws2KnKepXoq62TeF5uaCVSvqQV'; + const walletRoot = '4DujymUFbQ8GBKtAwAZrQ6QqpvtBEivL48h4ta2oJGd2'; + const usdtAmount = '40000000000'; + + const buildUsdtTokenTransferTx = async (): Promise => { + const txBuilder = getBuilderFactory('sol').getTokenTransferBuilder(); + txBuilder.nonce(blockHash); + txBuilder.sender(walletRoot); + txBuilder.send({ address: recipient, amount: usdtAmount, tokenName: 'sol:usdt' }); + const tx = await txBuilder.build(); + return tx.toBroadcastFormat(); + }; + + const tokenWalletTxRequest = (recipientAddress: string): TxRequest => + ({ + txRequestId: 'wci1723-txreq', + walletId: 'wci1723-wallet', + intent: { + intentType: 'payment', + recipients: [{ address: { address: recipientAddress }, amount: { value: usdtAmount, symbol: 'sol:usdt' } }], + }, + } as unknown as TxRequest); + + const tokenWallet = () => + new Wallet(bitgoMainnet, solMainnet, { + id: 'wci1723-wallet', + coin: 'sol:usdt', + coinSpecific: { rootAddress: walletRoot }, + multisigType: 'tss', + }); + + before(function () { + bitgoMainnet = TestBitGo.decorate(BitGoAPI, { env: 'mock' }); + bitgoMainnet.safeRegister('sol', Sol.createInstance); + bitgoMainnet.initializeTestVars(); + solMainnet = bitgoMainnet.coin('sol') as Sol; + }); + + it('verifies a sol:usdt withdrawal signed from a token wallet intent', async function () { + const txHex = await buildUsdtTokenTransferTx(); + const options = resolveTssVerifyTransactionOptions(tokenWalletTxRequest(recipient), txHex, undefined, 'sol:usdt'); + should.exist(options.txParams?.recipients?.[0].tokenName); + options.txParams?.recipients?.[0].tokenName?.should.equal('sol:usdt'); + const validTransaction = await solMainnet.verifyTransaction({ + ...options, + wallet: tokenWallet(), + walletType: 'tss', + } as any); + validTransaction.should.equal(true); + }); + + it('still rejects a sol:usdt withdrawal with a tampered intent recipient', async function () { + const txHex = await buildUsdtTokenTransferTx(); + const options = resolveTssVerifyTransactionOptions( + tokenWalletTxRequest('8KfDrb6cd4AM7TywFbgRtfr5ZB2auV6TfLF9hqE7BbFA'), + txHex, + undefined, + 'sol:usdt' + ); + await solMainnet + .verifyTransaction({ + ...options, + wallet: tokenWallet(), + walletType: 'tss', + } as any) + .should.be.rejectedWith('Tx outputs does not match with expected txParams recipients'); + }); + }); + describe('getAmountBasedOnEndianness', () => { let originalArch: string; diff --git a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts index c167386b08..e7a4d406f9 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts @@ -1,4 +1,5 @@ import * as t from 'io-ts'; +import { CoinNotDefinedError, coins } from '@bitgo/statics'; import { TransactionParams, VerifyTransactionOptions } from '../../baseCoin'; import { InvalidTransactionError } from '../../errors'; import { PopulatedIntent, TxRequest } from './baseTypes'; @@ -105,6 +106,26 @@ export const NO_RECIPIENT_TX_TYPES = new Set([ 'authorize', ]); +/** + * Whether chainName identifies a token coin rather than a native chain. + * + * For token wallets (e.g. a sol:usdt wallet) baseCoin.getChain() returns the token + * name itself, so an intent recipient whose amount.symbol equals chainName is + * still a token transfer and the symbol must be kept as tokenName. Native wallets + * keep the strict symbol !== chainName behavior. Chain names that are not in + * statics (e.g. dynamic/AMS tokens) resolve to false. + */ +function isTokenChainName(chainName: string): boolean { + try { + return coins.get(chainName).isToken; + } catch (e) { + if (e instanceof CoinNotDefinedError) { + return false; + } + throw e; + } +} + /** * Resolves the effective txParams for TSS signing recipient verification. * @@ -113,7 +134,9 @@ export const NO_RECIPIENT_TX_TYPES = new Set([ * mapped to ITransactionRecipient shape when txParams.recipients is absent. * * tokenName is derived from tokenData.tokenName when present, otherwise from - * amount.symbol when chainName is provided and symbol differs from it. + * amount.symbol when chainName is provided and the symbol is not the wallet's + * native asset: for native wallets that means symbol !== chainName, while for + * token wallets chainName is the token itself so the symbol is always kept. * * Staking intents (BSC delegate/undelegate, CELO stake/unstake, etc.) are * identified generically by the presence of `stakingRequestId` on the intent — @@ -125,21 +148,26 @@ export const NO_RECIPIENT_TX_TYPES = new Set([ * * @param txRequest - the transaction request containing the persisted intent * @param txParams - the caller-supplied transaction parameters (may be undefined) - * @param chainName - the base chain name (e.g. 'sol', 'tsol') used to exclude - * native-coin transfers from tokenName; pass baseCoin.getChain() + * @param chainName - the wallet's chain name (baseCoin.getChain()); for token + * wallets this is the token name itself (e.g. 'sol:usdt') */ export function resolveEffectiveTxParams( txRequest: TxRequest, txParams: TransactionParams | undefined, chainName?: string ): TransactionParams { + // Resolved once per call: a token wallet's chainName (e.g. 'sol:usdt') is itself + // a token, so recipient symbols equal to chainName still identify token transfers. + const chainNameIsToken = chainName !== undefined && isTokenChainName(chainName); + const intentRecipients = (txRequest.intent as PopulatedIntent)?.recipients?.map((intentRecipient) => { // Prefer tokenData.tokenName; fall back to amount.symbol when chainName is - // provided and differs from it. When absent, skip the symbol fallback. + // provided and the symbol is not the wallet's native asset. When absent, + // skip the symbol fallback. const { symbol } = intentRecipient.amount; const tokenName = intentRecipient.tokenData?.tokenName || - (chainName !== undefined && symbol && symbol !== chainName ? symbol : undefined); + (chainName !== undefined && symbol && (symbol !== chainName || chainNameIsToken) ? symbol : undefined); return { address: intentRecipient.address.address, amount: intentRecipient.amount.value, diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts index 1c93d7ca99..08f88f0075 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts @@ -362,6 +362,62 @@ describe('recipientUtils', function () { assert.strictEqual(result.recipients?.[0].tokenName, undefined); }); + it('keeps tokenName when symbol equals chainName for a token wallet (WCI-1723)', function () { + // A sol:usdt token wallet: populateIntent stores amount.symbol = 'sol:usdt' + // because sendMany on a token wallet does not include tokenName, and the + // EdDSA signers pass baseCoin.getChain() = 'sol:usdt' as chainName. + // The symbol is the token name and must be kept so verifyTransaction can + // derive the recipient's associated token account. + const txRequest = makeTxRequest({ + intent: { + intentType: 'payment', + recipients: [ + { + address: { address: 'E7Z6pFfUhjx2dFjdB9Ws2KnKepXoq62TeF5uaCVSvqQV' }, + amount: { value: '40000000000', symbol: 'sol:usdt' }, + }, + ], + } as any, + }); + const result = resolveEffectiveTxParams(txRequest, {}, 'sol:usdt'); + assert.strictEqual(result.recipients?.length, 1); + assert.strictEqual(result.recipients?.[0].tokenName, 'sol:usdt'); + }); + + it('keeps tokenName when symbol equals chainName for a testnet token wallet (WCI-1723)', function () { + const txRequest = makeTxRequest({ + intent: { + intentType: 'payment', + recipients: [ + { + address: { address: 'E7Z6pFfUhjx2dFjdB9Ws2KnKepXoq62TeF5uaCVSvqQV' }, + amount: { value: '1000000', symbol: 'tsol:usdc' }, + }, + ], + } as any, + }); + const result = resolveEffectiveTxParams(txRequest, {}, 'tsol:usdc'); + assert.strictEqual(result.recipients?.[0].tokenName, 'tsol:usdc'); + }); + + it('does not set tokenName for a chainName that is not in statics and equals symbol', function () { + // Dynamic/AMS token wallets are not in the statics registry, so a symbol + // equal to chainName cannot be distinguished from a native transfer. + const txRequest = makeTxRequest({ + intent: { + intentType: 'payment', + recipients: [ + { + address: { address: 'E7Z6pFfUhjx2dFjdB9Ws2KnKepXoq62TeF5uaCVSvqQV' }, + amount: { value: '1000', symbol: 'sol:some-dynamic-token' }, + }, + ], + } as any, + }); + const result = resolveEffectiveTxParams(txRequest, {}, 'sol:some-dynamic-token'); + assert.strictEqual(result.recipients?.[0].tokenName, undefined); + }); + it('prefers tokenData.tokenName over amount.symbol (uses distinct values to verify)', function () { const txRequest = makeTxRequest({ intent: { From 14a4902fec5b886e32029f146a7bf8b9fad559fd Mon Sep 17 00:00:00 2001 From: bitgobot Date: Sat, 3 Oct 2026 13:26:11 +0000 Subject: [PATCH 2/2] docs(sdk-core): correct token registry notes in recipientUtils 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 --- modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts | 9 ++++++--- .../test/unit/bitgo/utils/tss/recipientUtils.ts | 10 +++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts index e7a4d406f9..d322c78cfd 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts @@ -112,8 +112,10 @@ export const NO_RECIPIENT_TX_TYPES = new Set([ * For token wallets (e.g. a sol:usdt wallet) baseCoin.getChain() returns the token * name itself, so an intent recipient whose amount.symbol equals chainName is * still a token transfer and the symbol must be kept as tokenName. Native wallets - * keep the strict symbol !== chainName behavior. Chain names that are not in - * statics (e.g. dynamic/AMS tokens) resolve to false. + * keep the strict symbol !== chainName behavior. The statics coin map covers both + * statically listed tokens and runtime-registered (AMS) tokens — + * GlobalCoinFactory.registerToken adds them to the same map — so names never + * registered anywhere are the only ones that resolve to false. */ function isTokenChainName(chainName: string): boolean { try { @@ -235,7 +237,8 @@ const ConsolidateIntent = t.intersection([ * @param txRequest - the transaction request containing the persisted intent * @param txHex - the unsigned transaction to verify * @param txParams - the caller-supplied transaction parameters (may be undefined) - * @param chainName - the base chain name; pass baseCoin.getChain() + * @param chainName - the wallet's chain name; pass baseCoin.getChain() (the token + * name itself for token wallets) */ export function resolveTssVerifyTransactionOptions( txRequest: TxRequest, diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts index 08f88f0075..578a2dcfe3 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts @@ -400,21 +400,21 @@ describe('recipientUtils', function () { assert.strictEqual(result.recipients?.[0].tokenName, 'tsol:usdc'); }); - it('does not set tokenName for a chainName that is not in statics and equals symbol', function () { - // Dynamic/AMS token wallets are not in the statics registry, so a symbol - // equal to chainName cannot be distinguished from a native transfer. + it('does not set tokenName for a chainName that is not registered and equals symbol', function () { + // A name registered nowhere (statics or the runtime/AMS registry) cannot + // be distinguished from a native transfer, so the symbol stays dropped. const txRequest = makeTxRequest({ intent: { intentType: 'payment', recipients: [ { address: { address: 'E7Z6pFfUhjx2dFjdB9Ws2KnKepXoq62TeF5uaCVSvqQV' }, - amount: { value: '1000', symbol: 'sol:some-dynamic-token' }, + amount: { value: '1000', symbol: 'sol:some-unregistered-token' }, }, ], } as any, }); - const result = resolveEffectiveTxParams(txRequest, {}, 'sol:some-dynamic-token'); + const result = resolveEffectiveTxParams(txRequest, {}, 'sol:some-unregistered-token'); assert.strictEqual(result.recipients?.[0].tokenName, undefined); });