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..d322c78cfd 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,28 @@ 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. 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 { + 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 +136,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 +150,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, @@ -207,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 1c93d7ca99..578a2dcfe3 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 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-unregistered-token' }, + }, + ], + } as any, + }); + const result = resolveEffectiveTxParams(txRequest, {}, 'sol:some-unregistered-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: {