diff --git a/modules/sdk-coin-sol/test/unit/sol.ts b/modules/sdk-coin-sol/test/unit/sol.ts index 4e85ebc4a2e..aca3af71a22 100644 --- a/modules/sdk-coin-sol/test/unit/sol.ts +++ b/modules/sdk-coin-sol/test/unit/sol.ts @@ -27,6 +27,7 @@ import { MPCTx, MPCTxs, RecoveryProviderError, + resolveEffectiveTxParams, signRecoveryEddsaMPCv2, PrebuildAndSignTransactionOptions, TransactionPrebuild, @@ -1010,6 +1011,81 @@ describe('SOL:', function () { .should.be.rejectedWith('Tx outputs does not match with expected txParams recipients'); }); + describe('MPCv2 sign-path token transfer verification', () => { + // Mirrors EddsaMPCv2Utils.signRequestBase: when the caller passes no txParams + // (UI Sign / re-sign / pending-approval paths), recipients are rebuilt from the + // persisted intent via resolveEffectiveTxParams with baseCoin.getChain() as + // chainName, then handed to verifyTransaction. + const feePayerWallet = new Wallet(bitgo, basecoin, { + id: '5b34252f1bf349930e34020a00000000', + coin: 'tsol', + keys: [ + '5b3424f91bf349930e34017500000000', + '5b3424f91bf349930e34017600000000', + '5b3424f91bf349930e34017700000000', + ], + coinSpecific: { + rootAddress: '4DujymUFbQ8GBKtAwAZrQ6QqpvtBEivL48h4ta2oJGd2', + }, + multisigType: 'tss', + }); + const intentTxRequest = (recipientAddress: string, symbol: string): TxRequest => + ({ + txRequestId: 'token-transfer-txreq', + walletId: 'token-transfer-wallet', + intent: { + intentType: 'payment', + recipients: [{ address: { address: recipientAddress }, amount: { value: '1', symbol } }], + }, + } as unknown as TxRequest); + + it('verifies a token transfer from an intent with an unprefixed token name', async function () { + const txParams = resolveEffectiveTxParams( + intentTxRequest('AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvg', 'usdc'), + undefined, + 'tsol' + ); + const txPrebuild = newTxPrebuildTokenTransfer(); + const validTransaction = await basecoin.verifyTransaction({ + txParams, + txPrebuild, + wallet: feePayerWallet, + } as unknown as SolVerifyTransactionOptions); + validTransaction.should.equal(true); + }); + + it('verifies a token wallet withdrawal whose intent symbol equals chainName', async function () { + const txParams = resolveEffectiveTxParams( + intentTxRequest('AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvg', 'tsol:usdc'), + undefined, + 'tsol:usdc' + ); + const txPrebuild = newTxPrebuildTokenTransfer(); + const validTransaction = await basecoin.verifyTransaction({ + txParams, + txPrebuild, + wallet: feePayerWallet, + } as unknown as SolVerifyTransactionOptions); + validTransaction.should.equal(true); + }); + + it('still rejects a token transfer with a tampered intent recipient', async function () { + const txParams = resolveEffectiveTxParams( + intentTxRequest('AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvX', 'usdc'), + undefined, + 'tsol' + ); + const txPrebuild = newTxPrebuildTokenTransfer(); + await basecoin + .verifyTransaction({ + txParams, + txPrebuild, + wallet: feePayerWallet, + } as unknown as SolVerifyTransactionOptions) + .should.be.rejectedWith('Tx outputs does not match with expected txParams recipients'); + }); + }); + it('should succeed to verify token transaction for unsupported token when tokenAddress and programId are provided', async function () { // Use an address that is NOT in the static token map as the unsupported mint const unsupportedMintAddress = resources.stakeAccount.pub; // '3c5emUWjViFqT72LxQYec8gkU8ZtmfKKXHvGgJNUBdYx' diff --git a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts index 275bff0d15c..ce78c14ca08 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts @@ -1,3 +1,4 @@ +import { CoinNotDefinedError, coins } from '@bitgo/statics'; import { TransactionParams } from '../../baseCoin'; import { InvalidTransactionError } from '../../errors'; import { PopulatedIntent, TxRequest } from './baseTypes'; @@ -104,6 +105,53 @@ export const NO_RECIPIENT_TX_TYPES = new Set([ 'authorize', ]); +/** + * The statics-registered spelling of name when it identifies a token coin, + * undefined when it is unknown or not a token. coin lookups throw + * CoinNotDefinedError for unregistered names, which is not an error here: + * callers treat it as "not resolvable". + */ +function lookupTokenName(name: string): string | undefined { + try { + return coins.get(name).isToken ? name : undefined; + } catch (e) { + if (e instanceof CoinNotDefinedError) { + return undefined; + } + throw e; + } +} + +/** + * 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 { + return lookupTokenName(chainName) !== undefined; +} + +/** + * The statics-registered spelling of a token name, retrying unprefixed names with + * the chain prefix ('usdc' -> 'tsol:usdc'). wallet-platform persists some intent + * token names without the prefix; coin-level verification resolves tokens by their + * registered name, so an unprefixed spelling leaves the mint unresolvable. Names + * that resolve nowhere are passed through unchanged so verification fails closed + * at the coin layer rather than guessing. + */ +function resolveTokenName(tokenName: string, chainName: string): string { + // The chain-scoped spelling is tried first: it is the unambiguous reading of a + // name on this wallet's chain, while a bare name may belong to a different + // chain's token (statics registers mainnet aliases like 'usdc'). + return lookupTokenName(`${chainName}:${tokenName}`) ?? lookupTokenName(tokenName) ?? tokenName; +} + /** * Resolves the effective txParams for TSS signing recipient verification. * @@ -112,7 +160,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 — @@ -124,21 +174,31 @@ 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 = + const rawTokenName = intentRecipient.tokenData?.tokenName || - (chainName !== undefined && symbol && symbol !== chainName ? symbol : undefined); + (chainName !== undefined && symbol && (symbol !== chainName || chainNameIsToken) ? symbol : undefined); + // Canonicalize against statics when rebuilding on behalf of the chain-aware + // EdDSA MPCv2 sign path: only that caller passes chainName, and coin-level + // verification needs the registered spelling to resolve the mint. + const tokenName = + rawTokenName && chainName !== undefined ? resolveTokenName(rawTokenName, chainName) : rawTokenName; 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 6a9a4da0e94..f48121b3dda 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts @@ -1,7 +1,7 @@ import assert from 'assert'; import { NO_RECIPIENT_TX_TYPES, resolveEffectiveTxParams } from '../../../../../src/bitgo/utils/tss/recipientUtils'; import { InvalidTransactionError } from '../../../../../src/bitgo/errors'; -import { PopulatedIntent, TxRequest } from '../../../../../src/bitgo/utils/tss/baseTypes'; +import { PopulatedIntent, TokenType, TxRequest } from '../../../../../src/bitgo/utils/tss/baseTypes'; function makeTxRequest(overrides: Partial = {}): TxRequest { return { @@ -109,6 +109,111 @@ describe('recipientUtils', function () { assert.strictEqual(result.recipients?.[0].amount, '500'); }); + describe('tokenName derivation for chain-aware callers (EdDSA MPCv2 sign path)', function () { + const paymentIntent = (recipient: { + address: string; + value: string; + symbol: string; + tokenData?: { tokenType: TokenType; tokenQuantity: string; tokenName: string }; + }): TxRequest => + makeTxRequest({ + intent: { + intentType: 'payment', + recipients: [ + { + address: { address: recipient.address }, + amount: { value: recipient.value, symbol: recipient.symbol }, + tokenData: recipient.tokenData, + }, + ], + } as PopulatedIntent, + }); + + it('canonicalizes an unprefixed tokenData.tokenName using the chain prefix', function () { + const txRequest = paymentIntent({ + address: 'AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvg', + value: '1', + symbol: 'tsol', + tokenData: { tokenType: TokenType.ERC20, tokenQuantity: '1', tokenName: 'usdc' }, + }); + const result = resolveEffectiveTxParams(txRequest, undefined, 'tsol'); + assert.strictEqual(result.recipients?.[0].tokenName, 'tsol:usdc'); + }); + + it('canonicalizes an unprefixed amount.symbol using the chain prefix', function () { + const txRequest = paymentIntent({ + address: 'AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvg', + value: '1', + symbol: 'usdc', + }); + const result = resolveEffectiveTxParams(txRequest, undefined, 'tsol'); + assert.strictEqual(result.recipients?.[0].tokenName, 'tsol:usdc'); + }); + + it('keeps a token wallet symbol that equals chainName', function () { + const txRequest = paymentIntent({ + address: 'AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvg', + value: '1', + symbol: 'tsol:usdc', + }); + const result = resolveEffectiveTxParams(txRequest, undefined, 'tsol:usdc'); + assert.strictEqual(result.recipients?.[0].tokenName, 'tsol:usdc'); + }); + + it('keeps an already-registered tokenName unchanged', function () { + const txRequest = paymentIntent({ + address: 'AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvg', + value: '1', + symbol: 'tsol', + tokenData: { tokenType: TokenType.ERC20, tokenQuantity: '1', tokenName: 'tsol:usdc' }, + }); + const result = resolveEffectiveTxParams(txRequest, undefined, 'tsol'); + assert.strictEqual(result.recipients?.[0].tokenName, 'tsol:usdc'); + }); + + it('leaves native transfers without tokenName', function () { + const txRequest = paymentIntent({ + address: 'AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvg', + value: '1', + symbol: 'tsol', + }); + const result = resolveEffectiveTxParams(txRequest, undefined, 'tsol'); + assert.strictEqual(result.recipients?.[0].tokenName, undefined); + }); + + it('passes through token names that resolve nowhere', function () { + const txRequest = paymentIntent({ + address: 'AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvg', + value: '1', + symbol: 'tsol', + tokenData: { tokenType: TokenType.ERC20, tokenQuantity: '1', tokenName: 'not-a-registered-token' }, + }); + const result = resolveEffectiveTxParams(txRequest, undefined, 'tsol'); + assert.strictEqual(result.recipients?.[0].tokenName, 'not-a-registered-token'); + }); + + it('does not set tokenName for an unregistered chainName that equals symbol', function () { + const txRequest = paymentIntent({ + address: 'AF5H6vBkFnJuVqChRPgPQ4JRcQ5Gk25HBFhQQkyojmvg', + value: '1', + symbol: 'tsol:some-unregistered-token', + }); + const result = resolveEffectiveTxParams(txRequest, undefined, 'tsol:some-unregistered-token'); + assert.strictEqual(result.recipients?.[0].tokenName, undefined); + }); + + it('leaves tokenData.tokenName unchanged when chainName is absent (legacy ECDSA callers)', function () { + const txRequest = paymentIntent({ + address: '0xabc', + value: '1', + symbol: 'eth', + tokenData: { tokenType: TokenType.ERC20, tokenQuantity: '1', tokenName: 'eth:usdc' }, + }); + const result = resolveEffectiveTxParams(txRequest, {}); + assert.strictEqual(result.recipients?.[0].tokenName, 'eth:usdc'); + }); + }); + it('resolves txType from intent.intentType when txParams.type is absent', function () { const txRequest = makeTxRequest({ intent: { intentType: 'consolidate' } as any }); const result = resolveEffectiveTxParams(txRequest, {});