Skip to content

Commit a180e39

Browse files
MarzooqaMarzooqa
committed
fix(bsc): remove weak verifyTssTransaction override, inherit base class
Remove the stub verifyTssTransaction overrides from Bsc and BscToken that unconditionally returned true after only shallow presence checks. Both classes now inherit AbstractEthLikeNewCoins.verifyTssTransaction, which decodes txHex and validates native-BNB and BEP-20 transfers against the declared recipients. The original override was added in commit 3d29436 (COIN-3222, May 2025) to unblock a txHex-decoding crash in the transaction builder. That issue is no longer present, so the bypass can be removed. Add regression tests confirming: - Native BNB TSS transfer with matching recipient passes - Native BNB TSS transfer with mismatched recipient throws - BEP-20 TSS token transfer with matching calldata recipient passes - BEP-20 TSS token transfer with mismatched calldata recipient throws Ticket: WCI-1169 Session-Id: 4ade7ff7-085e-476d-b280-f4d3dd01c105 Task-Id: 9e3fa483-2a00-47db-aa04-c64b6f003a77
1 parent f7b8d3a commit a180e39

3 files changed

Lines changed: 124 additions & 70 deletions

File tree

‎modules/sdk-coin-bsc/src/bsc.ts‎

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,9 @@ import {
55
MPCAlgorithm,
66
MultisigType,
77
multisigTypes,
8-
NO_RECIPIENT_TX_TYPES,
98
} from '@bitgo/sdk-core';
109
import { BaseCoin as StaticsBaseCoin, coins } from '@bitgo/statics';
11-
import {
12-
AbstractEthLikeNewCoins,
13-
recoveryBlockchainExplorerQuery,
14-
VerifyEthTransactionOptions,
15-
} from '@bitgo/abstract-eth';
10+
import { AbstractEthLikeNewCoins, recoveryBlockchainExplorerQuery } from '@bitgo/abstract-eth';
1611
import { TransactionBuilder } from './lib';
1712

1813
export class Bsc extends AbstractEthLikeNewCoins {
@@ -63,35 +58,4 @@ export class Bsc extends AbstractEthLikeNewCoins {
6358
return await recoveryBlockchainExplorerQuery(query, explorerUrl as string, apiToken);
6459
}
6560

66-
/**
67-
* Verify if a tss transaction is valid
68-
*
69-
* @param {VerifyEthTransactionOptions} params
70-
* @param {TransactionParams} params.txParams - params object passed to send
71-
* @param {TransactionPrebuild} params.txPrebuild - prebuild object returned by server
72-
* @param {Wallet} params.wallet - Wallet object to obtain keys to verify against
73-
* @returns {boolean}
74-
*/
75-
async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
76-
const { txParams, txPrebuild, wallet } = params;
77-
if (
78-
!txParams?.recipients &&
79-
!(
80-
txParams.prebuildTx?.consolidateId ||
81-
txParams.stakingRequestId ||
82-
txParams.prebuildTx?.stakingRequestId ||
83-
(txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type))
84-
)
85-
) {
86-
throw new Error(`missing txParams`);
87-
}
88-
if (!wallet || !txPrebuild) {
89-
throw new Error(`missing params`);
90-
}
91-
if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) {
92-
throw new Error(`tx cannot be both a batch and hop transaction`);
93-
}
94-
95-
return true;
96-
}
9761
}

‎modules/sdk-coin-bsc/src/bscToken.ts‎

Lines changed: 2 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
*/
44

55
import { EthLikeTokenConfig, coins } from '@bitgo/statics';
6-
import { BitGoBase, CoinConstructor, NamedCoinConstructor, MPCAlgorithm, NO_RECIPIENT_TX_TYPES } from '@bitgo/sdk-core';
7-
import { CoinNames, EthLikeToken, VerifyEthTransactionOptions } from '@bitgo/abstract-eth';
6+
import { BitGoBase, CoinConstructor, NamedCoinConstructor, MPCAlgorithm } from '@bitgo/sdk-core';
7+
import { CoinNames, EthLikeToken } from '@bitgo/abstract-eth';
88
import { TransactionBuilder } from './lib';
99

1010
export { EthLikeTokenConfig };
@@ -43,35 +43,4 @@ export class BscToken extends EthLikeToken {
4343
getFullName(): string {
4444
return 'Bsc Token';
4545
}
46-
/**
47-
* Verify if a tss transaction is valid
48-
*
49-
* @param {VerifyEthTransactionOptions} params
50-
* @param {TransactionParams} params.txParams - params object passed to send
51-
* @param {TransactionPrebuild} params.txPrebuild - prebuild object returned by server
52-
* @param {Wallet} params.wallet - Wallet object to obtain keys to verify against
53-
* @returns {boolean}
54-
*/
55-
async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
56-
const { txParams, txPrebuild, wallet } = params;
57-
if (
58-
!txParams?.recipients &&
59-
!(
60-
txParams.prebuildTx?.consolidateId ||
61-
txParams.stakingRequestId ||
62-
txParams.prebuildTx?.stakingRequestId ||
63-
(txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type))
64-
)
65-
) {
66-
throw new Error(`missing txParams`);
67-
}
68-
if (!wallet || !txPrebuild) {
69-
throw new Error(`missing params`);
70-
}
71-
if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) {
72-
throw new Error(`tx cannot be both a batch and hop transaction`);
73-
}
74-
75-
return true;
76-
}
7746
}

‎modules/sdk-coin-bsc/test/unit/bsc.ts‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@ import 'should';
22

33
import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test';
44
import { BitGoAPI } from '@bitgo/sdk-api';
5+
import { TransactionType, Wallet } from '@bitgo/sdk-core';
6+
import EthereumAbi from 'ethereumjs-abi';
57

68
import { Bsc, Tbsc } from '../../src/index';
9+
import { TransactionBuilder } from '../../src/lib';
10+
import { getBuilder } from './getBuilder';
711

812
const bitgo: TestBitGoAPI = TestBitGo.decorate(BitGoAPI, { env: 'test' });
913

@@ -39,4 +43,121 @@ describe('Native BNB', function () {
3943
tbsc.allowsAccountConsolidations().should.equal(true);
4044
});
4145
});
46+
47+
describe('verifyTssTransaction', function () {
48+
const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be';
49+
const wrongAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6';
50+
const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
51+
const transferAmount = '1000000000000000000';
52+
53+
it('should accept a native BNB transfer where txHex matches declared recipient', async function () {
54+
const coin = bitgo.coin('tbsc') as Tbsc;
55+
56+
const txBuilder = getBuilder('tbsc') as TransactionBuilder;
57+
txBuilder.type(TransactionType.SingleSigSend);
58+
txBuilder.fee({ fee: '10', gasLimit: '21000' });
59+
txBuilder.counter(1);
60+
txBuilder.contract(recipientAddress);
61+
txBuilder.value(transferAmount);
62+
const tx = await txBuilder.build();
63+
const txHex = tx.toBroadcastFormat();
64+
65+
const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
66+
67+
const result = await coin.verifyTssTransaction({
68+
txParams: {
69+
type: 'transfer',
70+
recipients: [{ address: recipientAddress, amount: transferAmount }],
71+
} as any,
72+
txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
73+
wallet,
74+
});
75+
result.should.equal(true);
76+
});
77+
78+
it('should reject a native BNB transfer when txHex recipient does not match declared recipient', async function () {
79+
const coin = bitgo.coin('tbsc') as Tbsc;
80+
81+
const txBuilder = getBuilder('tbsc') as TransactionBuilder;
82+
txBuilder.type(TransactionType.SingleSigSend);
83+
txBuilder.fee({ fee: '10', gasLimit: '21000' });
84+
txBuilder.counter(1);
85+
txBuilder.contract(wrongAddress);
86+
txBuilder.value(transferAmount);
87+
const tx = await txBuilder.build();
88+
const txHex = tx.toBroadcastFormat();
89+
90+
const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
91+
92+
await coin
93+
.verifyTssTransaction({
94+
txParams: {
95+
type: 'transfer',
96+
recipients: [{ address: recipientAddress, amount: transferAmount }],
97+
} as any,
98+
txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
99+
wallet,
100+
})
101+
.should.be.rejectedWith('destination address does not match with the recipient address');
102+
});
103+
104+
it('should accept a BEP-20 token transfer where calldata matches declared recipient', async function () {
105+
const coin = bitgo.coin('tbsc') as Tbsc;
106+
107+
const methodId = EthereumAbi.methodID('transfer', ['address', 'uint256']);
108+
const encodedParams = EthereumAbi.rawEncode(['address', 'uint256'], [recipientAddress, '10000000']);
109+
const erc20TransferData = '0x' + Buffer.concat([methodId, encodedParams]).toString('hex');
110+
111+
const txBuilder = getBuilder('tbsc') as TransactionBuilder;
112+
txBuilder.type(TransactionType.ContractCall);
113+
txBuilder.fee({ fee: '10', gasLimit: '60000' });
114+
txBuilder.counter(1);
115+
txBuilder.contract(tokenContractAddress);
116+
txBuilder.data(erc20TransferData);
117+
const tx = await txBuilder.build();
118+
const txHex = tx.toBroadcastFormat();
119+
120+
const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
121+
122+
const result = await coin.verifyTssTransaction({
123+
txParams: {
124+
type: 'transfer',
125+
recipients: [{ address: recipientAddress, amount: '10000000' }],
126+
} as any,
127+
txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
128+
wallet,
129+
});
130+
result.should.equal(true);
131+
});
132+
133+
it('should reject a BEP-20 token transfer when calldata recipient does not match declared recipient', async function () {
134+
const coin = bitgo.coin('tbsc') as Tbsc;
135+
136+
const methodId = EthereumAbi.methodID('transfer', ['address', 'uint256']);
137+
const encodedParams = EthereumAbi.rawEncode(['address', 'uint256'], [wrongAddress, '10000000']);
138+
const erc20TransferData = '0x' + Buffer.concat([methodId, encodedParams]).toString('hex');
139+
140+
const txBuilder = getBuilder('tbsc') as TransactionBuilder;
141+
txBuilder.type(TransactionType.ContractCall);
142+
txBuilder.fee({ fee: '10', gasLimit: '60000' });
143+
txBuilder.counter(1);
144+
txBuilder.contract(tokenContractAddress);
145+
txBuilder.data(erc20TransferData);
146+
const tx = await txBuilder.build();
147+
const txHex = tx.toBroadcastFormat();
148+
149+
const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
150+
151+
await coin
152+
.verifyTssTransaction({
153+
txParams: {
154+
type: 'transfer',
155+
recipients: [{ address: recipientAddress, amount: '10000000' }],
156+
} as any,
157+
txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
158+
wallet,
159+
})
160+
.should.be.rejectedWith('destination address does not match with the recipient address');
161+
});
162+
});
42163
});

0 commit comments

Comments
 (0)