diff --git a/modules/sdk-coin-bsc/src/bsc.ts b/modules/sdk-coin-bsc/src/bsc.ts index 9b3e255500..c4239fc45d 100644 --- a/modules/sdk-coin-bsc/src/bsc.ts +++ b/modules/sdk-coin-bsc/src/bsc.ts @@ -1,18 +1,6 @@ -import { - BaseCoin, - BitGoBase, - common, - MPCAlgorithm, - MultisigType, - multisigTypes, - NO_RECIPIENT_TX_TYPES, -} from '@bitgo/sdk-core'; +import { BaseCoin, BitGoBase, common, MPCAlgorithm, MultisigType, multisigTypes } from '@bitgo/sdk-core'; import { BaseCoin as StaticsBaseCoin, coins } from '@bitgo/statics'; -import { - AbstractEthLikeNewCoins, - recoveryBlockchainExplorerQuery, - VerifyEthTransactionOptions, -} from '@bitgo/abstract-eth'; +import { AbstractEthLikeNewCoins, recoveryBlockchainExplorerQuery } from '@bitgo/abstract-eth'; import { TransactionBuilder } from './lib'; export class Bsc extends AbstractEthLikeNewCoins { @@ -62,36 +50,4 @@ export class Bsc extends AbstractEthLikeNewCoins { const explorerUrl = common.Environments[this.bitgo.getEnv()].bscscanBaseUrl; return await recoveryBlockchainExplorerQuery(query, explorerUrl as string, apiToken); } - - /** - * Verify if a tss transaction is valid - * - * @param {VerifyEthTransactionOptions} params - * @param {TransactionParams} params.txParams - params object passed to send - * @param {TransactionPrebuild} params.txPrebuild - prebuild object returned by server - * @param {Wallet} params.wallet - Wallet object to obtain keys to verify against - * @returns {boolean} - */ - async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { - const { txParams, txPrebuild, wallet } = params; - if ( - !txParams?.recipients && - !( - txParams.prebuildTx?.consolidateId || - txParams.stakingRequestId || - txParams.prebuildTx?.stakingRequestId || - (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)) - ) - ) { - throw new Error(`missing txParams`); - } - if (!wallet || !txPrebuild) { - throw new Error(`missing params`); - } - if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) { - throw new Error(`tx cannot be both a batch and hop transaction`); - } - - return true; - } } diff --git a/modules/sdk-coin-bsc/src/bscToken.ts b/modules/sdk-coin-bsc/src/bscToken.ts index b2923cd137..28f7f659ba 100644 --- a/modules/sdk-coin-bsc/src/bscToken.ts +++ b/modules/sdk-coin-bsc/src/bscToken.ts @@ -3,8 +3,8 @@ */ import { EthLikeTokenConfig, coins } from '@bitgo/statics'; -import { BitGoBase, CoinConstructor, NamedCoinConstructor, MPCAlgorithm, NO_RECIPIENT_TX_TYPES } from '@bitgo/sdk-core'; -import { CoinNames, EthLikeToken, VerifyEthTransactionOptions } from '@bitgo/abstract-eth'; +import { BitGoBase, CoinConstructor, MPCAlgorithm, NamedCoinConstructor } from '@bitgo/sdk-core'; +import { CoinNames, EthLikeToken } from '@bitgo/abstract-eth'; import { TransactionBuilder } from './lib'; export { EthLikeTokenConfig }; @@ -43,35 +43,4 @@ export class BscToken extends EthLikeToken { getFullName(): string { return 'Bsc Token'; } - /** - * Verify if a tss transaction is valid - * - * @param {VerifyEthTransactionOptions} params - * @param {TransactionParams} params.txParams - params object passed to send - * @param {TransactionPrebuild} params.txPrebuild - prebuild object returned by server - * @param {Wallet} params.wallet - Wallet object to obtain keys to verify against - * @returns {boolean} - */ - async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { - const { txParams, txPrebuild, wallet } = params; - if ( - !txParams?.recipients && - !( - txParams.prebuildTx?.consolidateId || - txParams.stakingRequestId || - txParams.prebuildTx?.stakingRequestId || - (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)) - ) - ) { - throw new Error(`missing txParams`); - } - if (!wallet || !txPrebuild) { - throw new Error(`missing params`); - } - if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) { - throw new Error(`tx cannot be both a batch and hop transaction`); - } - - return true; - } } diff --git a/modules/sdk-coin-bsc/test/unit/bsc.ts b/modules/sdk-coin-bsc/test/unit/bsc.ts index 6c80d6047b..debb7ffd42 100644 --- a/modules/sdk-coin-bsc/test/unit/bsc.ts +++ b/modules/sdk-coin-bsc/test/unit/bsc.ts @@ -2,8 +2,18 @@ import 'should'; import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; import { BitGoAPI } from '@bitgo/sdk-api'; +import { TransactionType, Wallet } from '@bitgo/sdk-core'; import { Bsc, Tbsc } from '../../src/index'; +import { TransactionBuilder } from '../../src/lib'; +import { getBuilder } from './getBuilder'; + +/** Encode ERC-20 transfer(address,uint256) calldata without ethereumjs-abi. */ +function encodeErc20Transfer(to: string, amount: string): string { + const address = to.toLowerCase().replace(/^0x/, '').padStart(64, '0'); + const value = BigInt(amount).toString(16).padStart(64, '0'); + return `0xa9059cbb${address}${value}`; +} const bitgo: TestBitGoAPI = TestBitGo.decorate(BitGoAPI, { env: 'test' }); @@ -39,4 +49,178 @@ describe('Native BNB', function () { tbsc.allowsAccountConsolidations().should.equal(true); }); }); + + describe('verifyTssTransaction', function () { + const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; + const wrongAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; + const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; + const transferAmount = '1000000000000000000'; + + it('should accept a native BNB transfer where txHex matches declared recipient', async function () { + const coin = bitgo.coin('tbsc') as Tbsc; + + const txBuilder = getBuilder('tbsc') as TransactionBuilder; + txBuilder.type(TransactionType.SingleSigSend); + txBuilder.fee({ fee: '10', gasLimit: '21000' }); + txBuilder.counter(1); + txBuilder.contract(recipientAddress); + txBuilder.value(transferAmount); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + const result = await coin.verifyTssTransaction({ + txParams: { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: transferAmount }], + } as any, + txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any, + wallet, + }); + result.should.equal(true); + }); + + it('should reject a native BNB transfer when txHex recipient does not match declared recipient', async function () { + const coin = bitgo.coin('tbsc') as Tbsc; + + const txBuilder = getBuilder('tbsc') as TransactionBuilder; + txBuilder.type(TransactionType.SingleSigSend); + txBuilder.fee({ fee: '10', gasLimit: '21000' }); + txBuilder.counter(1); + txBuilder.contract(wrongAddress); + txBuilder.value(transferAmount); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + await coin + .verifyTssTransaction({ + txParams: { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: transferAmount }], + } as any, + txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any, + wallet, + }) + .should.be.rejectedWith('destination address does not match with the recipient address'); + }); + + it('should accept a BEP-20 token transfer where calldata matches declared recipient', async function () { + const coin = bitgo.coin('tbsc') as Tbsc; + + const erc20TransferData = encodeErc20Transfer(recipientAddress, '10000000'); + + const txBuilder = getBuilder('tbsc') as TransactionBuilder; + txBuilder.type(TransactionType.ContractCall); + txBuilder.fee({ fee: '10', gasLimit: '60000' }); + txBuilder.counter(1); + txBuilder.contract(tokenContractAddress); + txBuilder.data(erc20TransferData); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + const result = await coin.verifyTssTransaction({ + txParams: { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '10000000' }], + } as any, + txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any, + wallet, + }); + result.should.equal(true); + }); + + it('should reject a BEP-20 token transfer when calldata recipient does not match declared recipient', async function () { + const coin = bitgo.coin('tbsc') as Tbsc; + + const erc20TransferData = encodeErc20Transfer(wrongAddress, '10000000'); + + const txBuilder = getBuilder('tbsc') as TransactionBuilder; + txBuilder.type(TransactionType.ContractCall); + txBuilder.fee({ fee: '10', gasLimit: '60000' }); + txBuilder.counter(1); + txBuilder.contract(tokenContractAddress); + txBuilder.data(erc20TransferData); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + await coin + .verifyTssTransaction({ + txParams: { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '10000000' }], + } as any, + txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any, + wallet, + }) + .should.be.rejectedWith('destination address does not match with the recipient address'); + }); + + it('should accept a BEP-20 token transfer using WalletConnect recipients[0].data flow', async function () { + const coin = bitgo.coin('tbsc') as Tbsc; + + // txHex sends to recipientAddress; recipients[0].data encodes the same intent + const erc20TransferData = encodeErc20Transfer(recipientAddress, '10000000'); + + const txBuilder = getBuilder('tbsc') as TransactionBuilder; + txBuilder.type(TransactionType.ContractCall); + txBuilder.fee({ fee: '10', gasLimit: '60000' }); + txBuilder.counter(1); + txBuilder.contract(tokenContractAddress); + txBuilder.data(erc20TransferData); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + const result = await coin.verifyTssTransaction({ + txParams: { + type: 'transfer', + // WalletConnect passes the intended calldata in recipients[0].data + recipients: [{ address: tokenContractAddress, amount: '0', data: erc20TransferData }], + } as any, + txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any, + wallet, + }); + result.should.equal(true); + }); + + it('should reject a BEP-20 token transfer using WalletConnect flow when calldata recipient is tampered', async function () { + const coin = bitgo.coin('tbsc') as Tbsc; + + // txHex sends to wrongAddress (tampered) + const tamperedData = encodeErc20Transfer(wrongAddress, '10000000'); + + const txBuilder = getBuilder('tbsc') as TransactionBuilder; + txBuilder.type(TransactionType.ContractCall); + txBuilder.fee({ fee: '10', gasLimit: '60000' }); + txBuilder.counter(1); + txBuilder.contract(tokenContractAddress); + txBuilder.data(tamperedData); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + // recipients[0].data declares the correct recipient (recipientAddress) + const correctData = encodeErc20Transfer(recipientAddress, '10000000'); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + await coin + .verifyTssTransaction({ + txParams: { + type: 'transfer', + recipients: [{ address: tokenContractAddress, amount: '0', data: correctData }], + } as any, + txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any, + wallet, + }) + .should.be.rejectedWith('destination address does not match with the recipient address'); + }); + }); }); diff --git a/modules/sdk-coin-xdc/src/xdc.ts b/modules/sdk-coin-xdc/src/xdc.ts index 1c3d9445e0..b6c124a48b 100644 --- a/modules/sdk-coin-xdc/src/xdc.ts +++ b/modules/sdk-coin-xdc/src/xdc.ts @@ -1,12 +1,4 @@ -import { - BaseCoin, - BitGoBase, - common, - MPCAlgorithm, - MultisigType, - multisigTypes, - NO_RECIPIENT_TX_TYPES, -} from '@bitgo/sdk-core'; +import { BaseCoin, BitGoBase, common, MPCAlgorithm, MultisigType, multisigTypes } from '@bitgo/sdk-core'; import { BaseCoin as StaticsBaseCoin, coins } from '@bitgo/statics'; import { AbstractEthLikeNewCoins, @@ -14,7 +6,6 @@ import { UnsignedSweepTxMPCv2, RecoverOptions, OfflineVaultTxInfo, - VerifyEthTransactionOptions, } from '@bitgo/abstract-eth'; import { TransactionBuilder } from './lib'; @@ -55,35 +46,4 @@ export class Xdc extends AbstractEthLikeNewCoins { const explorerUrl = common.Environments[this.bitgo.getEnv()].xdcExplorerBaseUrl; return await recoveryBlockchainExplorerQuery(query, explorerUrl as string, apiToken); } - /** - * Verify if a tss transaction is valid - * - * @param {VerifyEthTransactionOptions} params - * @param {TransactionParams} params.txParams - params object passed to send - * @param {TransactionPrebuild} params.txPrebuild - prebuild object returned by server - * @param {Wallet} params.wallet - Wallet object to obtain keys to verify against - * @returns {boolean} - */ - async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { - const { txParams, txPrebuild, wallet } = params; - if ( - !txParams?.recipients && - !( - txParams.prebuildTx?.consolidateId || - txParams.stakingRequestId || - txParams.prebuildTx?.stakingRequestId || - (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)) - ) - ) { - throw new Error(`missing txParams`); - } - if (!wallet || !txPrebuild) { - throw new Error(`missing params`); - } - if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) { - throw new Error(`tx cannot be both a batch and hop transaction`); - } - - return true; - } } diff --git a/modules/sdk-coin-xdc/src/xdcToken.ts b/modules/sdk-coin-xdc/src/xdcToken.ts index 29b0931971..3e6d39c7df 100644 --- a/modules/sdk-coin-xdc/src/xdcToken.ts +++ b/modules/sdk-coin-xdc/src/xdcToken.ts @@ -2,20 +2,8 @@ * @prettier */ import { EthLikeTokenConfig, coins } from '@bitgo/statics'; -import { - BitGoBase, - CoinConstructor, - NamedCoinConstructor, - common, - MPCAlgorithm, - NO_RECIPIENT_TX_TYPES, -} from '@bitgo/sdk-core'; -import { - CoinNames, - EthLikeToken, - recoveryBlockchainExplorerQuery, - VerifyEthTransactionOptions, -} from '@bitgo/abstract-eth'; +import { BitGoBase, CoinConstructor, NamedCoinConstructor, common, MPCAlgorithm } from '@bitgo/sdk-core'; +import { CoinNames, EthLikeToken, recoveryBlockchainExplorerQuery } from '@bitgo/abstract-eth'; import { TransactionBuilder } from './lib'; export { EthLikeTokenConfig }; @@ -64,36 +52,4 @@ export class XdcToken extends EthLikeToken { getMPCAlgorithm(): MPCAlgorithm { return 'ecdsa'; } - - /** - * Verify if a tss transaction is valid - * - * @param {VerifyEthTransactionOptions} params - * @param {TransactionParams} params.txParams - params object passed to send - * @param {TransactionPrebuild} params.txPrebuild - prebuild object returned by server - * @param {Wallet} params.wallet - Wallet object to obtain keys to verify against - * @returns {boolean} - */ - async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { - const { txParams, txPrebuild, wallet } = params; - if ( - !txParams?.recipients && - !( - txParams.prebuildTx?.consolidateId || - txParams.stakingRequestId || - txParams.prebuildTx?.stakingRequestId || - (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)) - ) - ) { - throw new Error(`missing txParams`); - } - if (!wallet || !txPrebuild) { - throw new Error(`missing params`); - } - if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) { - throw new Error(`tx cannot be both a batch and hop transaction`); - } - - return true; - } } diff --git a/modules/sdk-coin-xdc/test/unit/xdc.ts b/modules/sdk-coin-xdc/test/unit/xdc.ts index b067e6beb0..98cd23ce75 100644 --- a/modules/sdk-coin-xdc/test/unit/xdc.ts +++ b/modules/sdk-coin-xdc/test/unit/xdc.ts @@ -7,10 +7,20 @@ import { Xdc, Txdc } from '../../src/index'; import { UnsignedSweepTxMPCv2 } from '@bitgo/abstract-eth'; import { mockDataUnsignedSweep, mockDataNonBitGoRecovery } from '../resources'; import nock from 'nock'; -import { common } from '@bitgo/sdk-core'; +import { common, TransactionType, Wallet } from '@bitgo/sdk-core'; import { Transaction } from '@ethereumjs/tx'; import { stripHexPrefix } from '@ethereumjs/util'; +import { TransactionBuilder } from '../../src/lib'; +import { getBuilder } from './getBuilder'; + +/** Encode ERC-20 transfer(address,uint256) calldata without ethereumjs-abi. */ +function encodeErc20Transfer(to: string, amount: string): string { + const address = to.toLowerCase().replace(/^0x/, '').padStart(64, '0'); + const value = BigInt(amount).toString(16).padStart(64, '0'); + return `0xa9059cbb${address}${value}`; +} + const bitgo: TestBitGoAPI = TestBitGo.decorate(BitGoAPI, { env: 'test' }); describe('xdc', function () { @@ -45,6 +55,119 @@ describe('xdc', function () { txdc.allowsAccountConsolidations().should.equal(false); }); }); + + describe('verifyTssTransaction', function () { + const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; + const wrongAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; + const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; + const transferAmount = '1000000000000000000'; + + it('should accept a native XDC transfer where txHex matches declared recipient', async function () { + const coin = bitgo.coin('txdc') as Txdc; + + const txBuilder = getBuilder('txdc') as TransactionBuilder; + txBuilder.type(TransactionType.SingleSigSend); + txBuilder.fee({ fee: '10', gasLimit: '21000' }); + txBuilder.counter(1); + txBuilder.contract(recipientAddress); + txBuilder.value(transferAmount); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + const result = await coin.verifyTssTransaction({ + txParams: { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: transferAmount }], + } as any, + txPrebuild: { txHex, coin: 'txdc', walletId: 'fakeWalletId' } as any, + wallet, + }); + result.should.equal(true); + }); + + it('should reject a native XDC transfer when txHex recipient does not match declared recipient', async function () { + const coin = bitgo.coin('txdc') as Txdc; + + const txBuilder = getBuilder('txdc') as TransactionBuilder; + txBuilder.type(TransactionType.SingleSigSend); + txBuilder.fee({ fee: '10', gasLimit: '21000' }); + txBuilder.counter(1); + txBuilder.contract(wrongAddress); + txBuilder.value(transferAmount); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + await coin + .verifyTssTransaction({ + txParams: { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: transferAmount }], + } as any, + txPrebuild: { txHex, coin: 'txdc', walletId: 'fakeWalletId' } as any, + wallet, + }) + .should.be.rejectedWith('destination address does not match with the recipient address'); + }); + + it('should accept an ERC-20 token transfer where calldata matches declared recipient', async function () { + const coin = bitgo.coin('txdc') as Txdc; + + const erc20TransferData = encodeErc20Transfer(recipientAddress, '10000000'); + + const txBuilder = getBuilder('txdc') as TransactionBuilder; + txBuilder.type(TransactionType.ContractCall); + txBuilder.fee({ fee: '10', gasLimit: '60000' }); + txBuilder.counter(1); + txBuilder.contract(tokenContractAddress); + txBuilder.data(erc20TransferData); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + const result = await coin.verifyTssTransaction({ + txParams: { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '10000000' }], + } as any, + txPrebuild: { txHex, coin: 'txdc', walletId: 'fakeWalletId' } as any, + wallet, + }); + result.should.equal(true); + }); + + it('should reject an ERC-20 token transfer when calldata recipient does not match declared recipient', async function () { + const coin = bitgo.coin('txdc') as Txdc; + + const erc20TransferData = encodeErc20Transfer(wrongAddress, '10000000'); + + const txBuilder = getBuilder('txdc') as TransactionBuilder; + txBuilder.type(TransactionType.ContractCall); + txBuilder.fee({ fee: '10', gasLimit: '60000' }); + txBuilder.counter(1); + txBuilder.contract(tokenContractAddress); + txBuilder.data(erc20TransferData); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); + + const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } }); + + await coin + .verifyTssTransaction({ + txParams: { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '10000000' }], + } as any, + txPrebuild: { txHex, coin: 'txdc', walletId: 'fakeWalletId' } as any, + wallet, + }) + .should.be.rejectedWith('destination address does not match with the recipient address'); + }); + }); }); describe('Build Unsigned Sweep for Self-Custody Cold Wallets - (MPCv2)', function () { diff --git a/modules/sdk-coin-xdc/test/unit/xdcToken.ts b/modules/sdk-coin-xdc/test/unit/xdcToken.ts index d59f2ce913..337b371094 100644 --- a/modules/sdk-coin-xdc/test/unit/xdcToken.ts +++ b/modules/sdk-coin-xdc/test/unit/xdcToken.ts @@ -1,10 +1,18 @@ import 'should'; import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; import { BitGoAPI } from '@bitgo/sdk-api'; -import { IWallet } from '@bitgo/sdk-core'; +import { TransactionType, Wallet } from '@bitgo/sdk-core'; import { register, XdcToken } from '../../src'; -import { mockTokenTransferData } from '../resources'; +import { TransactionBuilder } from '../../src/lib'; +import { getBuilder } from './getBuilder'; + +/** Encode ERC-20 transfer(address,uint256) calldata without ethereumjs-abi. */ +function encodeErc20Transfer(to: string, amount: string): string { + const address = to.toLowerCase().replace(/^0x/, '').padStart(64, '0'); + const value = BigInt(amount).toString(16).padStart(64, '0'); + return `0xa9059cbb${address}${value}`; +} describe('XDC Token:', function () { let bitgo: TestBitGoAPI; @@ -121,186 +129,91 @@ describe('XDC Token:', function () { }); describe('verifyTssTransaction', function () { - it('should return true for valid token transfer params', async function () { - const token = bitgo.coin('txdc:tmt') as XdcToken; - const mockWallet = {} as unknown as IWallet; + const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; + const wrongAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; + const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; - const result = await token.verifyTssTransaction({ - txParams: { - recipients: [ - { - address: mockTokenTransferData.recipientAddress, - amount: mockTokenTransferData.tokenAmount, - }, - ], - }, - txPrebuild: mockTokenTransferData.txPrebuild as unknown as Parameters< - typeof token.verifyTssTransaction - >[0]['txPrebuild'], - wallet: mockWallet, - }); - - result.should.equal(true); - }); - - it('should return true for transferToken type without recipients', async function () { + it('should accept an ERC-20 transfer where calldata matches declared recipient', async function () { const token = bitgo.coin('txdc:tmt') as XdcToken; - const mockWallet = {} as unknown as IWallet; - const result = await token.verifyTssTransaction({ - txParams: { - type: 'transferToken', - }, - txPrebuild: mockTokenTransferData.txPrebuild as unknown as Parameters< - typeof token.verifyTssTransaction - >[0]['txPrebuild'], - wallet: mockWallet, - }); + const erc20TransferData = encodeErc20Transfer(recipientAddress, '10000000'); - result.should.equal(true); - }); + const txBuilder = getBuilder('txdc') as TransactionBuilder; + txBuilder.type(TransactionType.ContractCall); + txBuilder.fee({ fee: '10', gasLimit: '60000' }); + txBuilder.counter(1); + txBuilder.contract(tokenContractAddress); + txBuilder.data(erc20TransferData); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); - it('should return true for tokenApproval type without recipients', async function () { - const token = bitgo.coin('txdc:tmt') as XdcToken; - const mockWallet = {} as unknown as IWallet; + const wallet = new Wallet(bitgo, token, { coinSpecific: { baseAddress: recipientAddress } }); const result = await token.verifyTssTransaction({ txParams: { - type: 'tokenApproval', - }, - txPrebuild: mockTokenTransferData.txPrebuild as unknown as Parameters< - typeof token.verifyTssTransaction - >[0]['txPrebuild'], - wallet: mockWallet, + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '10000000' }], + } as any, + txPrebuild: { txHex, coin: 'txdc:tmt', walletId: 'fakeWalletId' } as any, + wallet, }); - result.should.equal(true); }); - it('should return true for consolidate type without recipients', async function () { + it('should reject an ERC-20 transfer when calldata recipient does not match declared recipient', async function () { const token = bitgo.coin('txdc:tmt') as XdcToken; - const mockWallet = {} as unknown as IWallet; - const result = await token.verifyTssTransaction({ - txParams: { - type: 'consolidate', - }, - txPrebuild: mockTokenTransferData.txPrebuild as unknown as Parameters< - typeof token.verifyTssTransaction - >[0]['txPrebuild'], - wallet: mockWallet, - }); + const erc20TransferData = encodeErc20Transfer(wrongAddress, '10000000'); - result.should.equal(true); - }); + const txBuilder = getBuilder('txdc') as TransactionBuilder; + txBuilder.type(TransactionType.ContractCall); + txBuilder.fee({ fee: '10', gasLimit: '60000' }); + txBuilder.counter(1); + txBuilder.contract(tokenContractAddress); + txBuilder.data(erc20TransferData); + const tx = await txBuilder.build(); + const txHex = tx.toBroadcastFormat(); - it('should throw error when txParams.recipients is missing and no valid type', async function () { - const token = bitgo.coin('txdc:tmt') as XdcToken; - const mockWallet = {} as unknown as IWallet; - - await token - .verifyTssTransaction({ - txParams: {}, - txPrebuild: mockTokenTransferData.txPrebuild as unknown as Parameters< - typeof token.verifyTssTransaction - >[0]['txPrebuild'], - wallet: mockWallet, - }) - .should.be.rejectedWith('missing txParams'); - }); - - it('should throw error when wallet is missing', async function () { - const token = bitgo.coin('txdc:tmt') as XdcToken; + const wallet = new Wallet(bitgo, token, { coinSpecific: { baseAddress: recipientAddress } }); await token .verifyTssTransaction({ txParams: { - recipients: [ - { - address: mockTokenTransferData.recipientAddress, - amount: mockTokenTransferData.tokenAmount, - }, - ], - }, - txPrebuild: mockTokenTransferData.txPrebuild as unknown as Parameters< - typeof token.verifyTssTransaction - >[0]['txPrebuild'], - wallet: undefined as unknown as IWallet, + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '10000000' }], + } as any, + txPrebuild: { txHex, coin: 'txdc:tmt', walletId: 'fakeWalletId' } as any, + wallet, }) - .should.be.rejectedWith('missing params'); + .should.be.rejectedWith('destination address does not match with the recipient address'); }); - it('should throw error when txPrebuild is missing', async function () { + it('should throw error when txParams.recipients is missing and no valid type', async function () { const token = bitgo.coin('txdc:tmt') as XdcToken; - const mockWallet = {} as unknown as IWallet; + const wallet = new Wallet(bitgo, token, { coinSpecific: { baseAddress: recipientAddress } }); await token .verifyTssTransaction({ - txParams: { - recipients: [ - { - address: mockTokenTransferData.recipientAddress, - amount: mockTokenTransferData.tokenAmount, - }, - ], - }, - txPrebuild: undefined as unknown as Parameters[0]['txPrebuild'], - wallet: mockWallet, + txParams: {}, + txPrebuild: { txHex: '0x00', coin: 'txdc:tmt', walletId: 'fakeWalletId' } as any, + wallet, }) - .should.be.rejectedWith('missing params'); + .should.be.rejectedWith('missing txParams'); }); - it('should throw error for batch + hop transaction', async function () { + it('should throw error when wallet is missing', async function () { const token = bitgo.coin('txdc:tmt') as XdcToken; - const mockWallet = {} as unknown as IWallet; await token .verifyTssTransaction({ txParams: { - hop: true, - recipients: [ - { address: '0x1111111111111111111111111111111111111111', amount: '1000' }, - { address: '0x2222222222222222222222222222222222222222', amount: '2000' }, - ], - }, - txPrebuild: mockTokenTransferData.txPrebuild as unknown as Parameters< - typeof token.verifyTssTransaction - >[0]['txPrebuild'], - wallet: mockWallet, + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '10000000' }], + } as any, + txPrebuild: { txHex: '0x00', coin: 'txdc:tmt', walletId: 'fakeWalletId' } as any, + wallet: undefined as unknown as Wallet, }) - .should.be.rejectedWith('tx cannot be both a batch and hop transaction'); - }); - - it('should not throw EIP155 error when verifying token transaction', async function () { - // This test ensures that verifyTssTransaction does NOT parse the txHex - // which would fail with "Incompatible EIP155-based V" error - const token = bitgo.coin('txdc:tmt') as XdcToken; - const mockWallet = {} as unknown as IWallet; - - // Use the signableHex (with v=51) which would fail if parsed - const txPrebuildWithSignableHex = { - ...mockTokenTransferData.txPrebuild, - txHex: mockTokenTransferData.signableHex, - }; - - // This should NOT throw EIP155 error because verifyTssTransaction - // does not parse the transaction - const result = await token.verifyTssTransaction({ - txParams: { - recipients: [ - { - address: mockTokenTransferData.recipientAddress, - amount: mockTokenTransferData.tokenAmount, - }, - ], - }, - txPrebuild: txPrebuildWithSignableHex as unknown as Parameters< - typeof token.verifyTssTransaction - >[0]['txPrebuild'], - wallet: mockWallet, - }); - - result.should.equal(true); + .should.be.rejectedWith('missing params'); }); }); }); diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts index 945535c837..62438e8125 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts @@ -51,6 +51,7 @@ import { import { BaseEcdsaUtils } from './base'; import { EncryptionVersion, IRequestTracer } from '../../../../api'; import { resolveEffectiveTxParams } from '../recipientUtils'; +import { shouldVerifyWithSerializedTxHex } from '../serializedTxHexVerify'; const encryptNShare = ECDSAMethods.encryptNShare; @@ -811,11 +812,11 @@ export class EcdsaUtils extends BaseEcdsaUtils { ); } - // For ICP transactions, the HSM signs the serializedTxHex, while the user signs the signableHex separately. - // Verification cannot be performed directly on the signableHex alone. However, we can parse the serializedTxHex - // to regenerate the signableHex and compare it against the provided value for verification. - // In contrast, for other coin families, verification is typically done using just the signableHex. - if (this.baseCoin.getConfig().family === 'icp') { + // For some coins, signableHex is not a parseable transaction. Pass + // serializedTxHex so verifyTransaction can decode the full tx bytes. + // Gated by CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX (ICP hash digests, + // BSC/XDC legacy EIP-155 RLP with v=chainId, etc.). + if (shouldVerifyWithSerializedTxHex(this.baseCoin)) { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex }, txParams: resolveEffectiveTxParams(txRequest, params.txParams), diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts index 9d8fb1c426..478107d3ef 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts @@ -51,6 +51,7 @@ import { SignableTransaction, } from '../baseTypes'; import { shouldUsePreHashedSignable } from '../preHashedSignable'; +import { shouldVerifyWithSerializedTxHex } from '../serializedTxHexVerify'; import { BaseEcdsaUtils } from './base'; import { EcdsaMPCv2KeyGenSendFn, KeyGenSenderForEnterprise } from './ecdsaMPCv2KeyGenSender'; import { envRequiresBitgoPubGpgKeyConfig, isBitgoMpcPubKey } from '../../../tss/bitgoPubKeys'; @@ -943,16 +944,14 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils { ); } - // For ICP and Avalanche atomic transactions, signableHex is a digest (not - // a parseable transaction). Pass serializedTxHex so verifyTransaction can - // parse the full transaction bytes. - // - ICP: signableHex is a hash; serializedTxHex is the CBOR-encoded tx. + // For some coins, signableHex is not a parseable transaction. Pass + // serializedTxHex so verifyTransaction can decode the full tx bytes. + // - CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX: ICP hash digests, + // BSC/XDC legacy EIP-155 RLP with v=chainId, etc. // - Avalanche atomic (FLRP/FLR cross-chain): signableHex is SHA-256(txBody); // serializedTxHex is the PVM/EVM atomic tx (codec prefix 0x0000). - // For all other coins, signableHex IS the unsigned transaction (e.g. RLP bytes). - const isIcp = this.baseCoin.getConfig().family === 'icp'; - const isPreHashed = shouldUsePreHashedSignable(this.baseCoin, unsignedTx); - if (isIcp || isPreHashed) { + // For other coins, signableHex IS the unsigned transaction (e.g. EIP-1559 RLP). + if (shouldVerifyWithSerializedTxHex(this.baseCoin) || shouldUsePreHashedSignable(this.baseCoin, unsignedTx)) { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex }, txParams: resolveEffectiveTxParams(txRequest, params.txParams), diff --git a/modules/sdk-core/src/bitgo/utils/tss/serializedTxHexVerify.ts b/modules/sdk-core/src/bitgo/utils/tss/serializedTxHexVerify.ts new file mode 100644 index 0000000000..2a0509914c --- /dev/null +++ b/modules/sdk-core/src/bitgo/utils/tss/serializedTxHexVerify.ts @@ -0,0 +1,12 @@ +import { CoinFeature } from '@bitgo/statics'; + +import { IBaseCoin } from '../../baseCoin'; + +/** + * Returns true when TSS verifyTransaction should decode serializedTxHex instead of + * signableHex (e.g. ICP hash digests, or legacy EIP-155 RLP that fails ethereumjs + * fromSerializedTx). Controlled via CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX. + */ +export function shouldVerifyWithSerializedTxHex(coin: IBaseCoin): boolean { + return coin.getConfig().features?.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX) ?? false; +} diff --git a/modules/statics/src/account.ts b/modules/statics/src/account.ts index 8dcc543af2..5cbc9929d3 100644 --- a/modules/statics/src/account.ts +++ b/modules/statics/src/account.ts @@ -10,6 +10,7 @@ import { COSMOS_SIDECHAIN_FEATURES, ERC7984_TOKEN_FEATURES, TEMPO_FEATURES, + XDC_TOKEN_FEATURES, } from './coinFeatures'; /** @@ -3201,7 +3202,7 @@ export function xdcErc20( decimalPlaces: number, contractAddress: string, asset: UnderlyingAsset, - features: CoinFeature[] = AccountCoin.DEFAULT_FEATURES, + features: CoinFeature[] = XDC_TOKEN_FEATURES, prefix = '', suffix: string = name.toUpperCase(), network: AccountNetwork = Networks.main.xdc, @@ -3248,7 +3249,7 @@ export function txdcErc20( decimalPlaces: number, contractAddress: string, asset: UnderlyingAsset, - features: CoinFeature[] = AccountCoin.DEFAULT_FEATURES, + features: CoinFeature[] = XDC_TOKEN_FEATURES, prefix = '', suffix: string = name.toUpperCase(), network: AccountNetwork = Networks.test.xdc, diff --git a/modules/statics/src/base.ts b/modules/statics/src/base.ts index b02217f3c8..8934b3a844 100644 --- a/modules/statics/src/base.ts +++ b/modules/statics/src/base.ts @@ -617,6 +617,13 @@ export enum CoinFeature { * behavior. MUST only be set together with TOKENIZED_EQUITY. */ BITGO_TOKENIZED_EQUITY = 'bitgo-tokenized-equity', + + /** + * TSS transaction verification should use serializedTxHex rather than signableHex. + * Needed when signableHex is not a parseable transaction (e.g. ICP hash digest, or + * legacy EIP-155 RLP with v=chainId that fails ethereumjs fromSerializedTx). + */ + TSS_VERIFY_USE_SERIALIZED_TX_HEX = 'tss-verify-use-serialized-tx-hex', } /** diff --git a/modules/statics/src/coinFeatures.ts b/modules/statics/src/coinFeatures.ts index b60ef68d45..899a2a7254 100644 --- a/modules/statics/src/coinFeatures.ts +++ b/modules/statics/src/coinFeatures.ts @@ -324,15 +324,22 @@ export const BSC_FEATURES = [ CoinFeature.BULK_TRANSACTION, CoinFeature.SHARED_EVM_MESSAGE_SIGNING, CoinFeature.ERC20_BULK_TRANSACTION, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, +]; +export const BSC_TOKEN_FEATURES = [ + ...ACCOUNT_COIN_DEFAULT_FEATURES, + CoinFeature.BULK_TRANSACTION, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, ]; -export const BSC_TOKEN_FEATURES = [...ACCOUNT_COIN_DEFAULT_FEATURES, CoinFeature.BULK_TRANSACTION]; export const BSC_TOKEN_FEATURES_EXCLUDE_SINGAPORE = [ ...ACCOUNT_COIN_DEFAULT_FEATURES_EXCLUDE_SINGAPORE, CoinFeature.BULK_TRANSACTION, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, ]; export const BSC_TOKEN_FEATURES_EXCLUDE_MENA_FZE = [ ...ACCOUNT_COIN_DEFAULT_FEATURES_EXCLUDE_MENA_FZE, CoinFeature.BULK_TRANSACTION, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, ]; export const STX_FEATURES = [ ...ACCOUNT_COIN_DEFAULT_FEATURES, @@ -658,6 +665,7 @@ export const ICP_FEATURES = [ CoinFeature.SUPPORTS_TOKENS, CoinFeature.SHA256_WITH_ECDSA_TSS, CoinFeature.REBUILD_ON_CUSTODY_SIGNING, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, ]; export const STARKNET_FEATURES = [ @@ -758,7 +766,12 @@ export const VET_TOKEN_FEATURES = VET_FEATURES.filter((feature) => feature !== C export const EVM_NON_EIP1559_FEATURES = [...EVM_FEATURES.filter((feature) => feature !== CoinFeature.EIP1559)]; -export const XDC_FEATURES = [...EVM_NON_EIP1559_FEATURES, CoinFeature.ERC20_BULK_TRANSACTION]; +export const XDC_FEATURES = [ + ...EVM_NON_EIP1559_FEATURES, + CoinFeature.ERC20_BULK_TRANSACTION, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, +]; +export const XDC_TOKEN_FEATURES = [...ACCOUNT_COIN_DEFAULT_FEATURES, CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX]; export const SGB_FEATURES = [...EVM_FEATURES, CoinFeature.ERC20_BULK_TRANSACTION]; diff --git a/modules/statics/test/unit/coins.ts b/modules/statics/test/unit/coins.ts index 3bde589874..488a9c66cd 100644 --- a/modules/statics/test/unit/coins.ts +++ b/modules/statics/test/unit/coins.ts @@ -1273,6 +1273,22 @@ describe('ERC20 Bulk Transaction Feature', () => { }); }); +describe('TSS Verify Use Serialized Tx Hex Feature', () => { + it('should have TSS_VERIFY_USE_SERIALIZED_TX_HEX for coins whose signableHex is not parseable', () => { + const coinsNeedingSerializedTxHexVerify = ['bsc', 'tbsc', 'xdc', 'txdc', 'icp', 'ticp']; + coinsNeedingSerializedTxHexVerify.forEach((coinName) => { + const coin = coins.get(coinName); + coin.features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true); + }); + }); + + it('should have TSS_VERIFY_USE_SERIALIZED_TX_HEX on BSC and XDC tokens', () => { + coins.get('bsc:busd').features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true); + coins.get('xdc:usdc').features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true); + coins.get('txdc:tmt').features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true); + }); +}); + describe('Custody Bulk Withdrawal Features', () => { it('should have CUSTODY_BULK_TRANSACTION feature for appropriate coins', () => { const custodyBulkWithdrawalCoins = [