Skip to content

Commit db2c9f6

Browse files
Marzooqacursoragent
andcommitted
refactor(sdk-core): gate serializedTxHex TSS verify via CoinFeature
Replace hard-coded icp/bsc/xdc family checks with CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX on BSC, XDC, and ICP (and their tokens), so new coins can opt in via statics config. Ticket: WCI-1169 Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 67c754e commit db2c9f6

7 files changed

Lines changed: 61 additions & 20 deletions

File tree

modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
import { BaseEcdsaUtils } from './base';
5252
import { EncryptionVersion, IRequestTracer } from '../../../../api';
5353
import { resolveEffectiveTxParams } from '../recipientUtils';
54+
import { shouldVerifyWithSerializedTxHex } from '../serializedTxHexVerify';
5455

5556
const encryptNShare = ECDSAMethods.encryptNShare;
5657

@@ -813,13 +814,9 @@ export class EcdsaUtils extends BaseEcdsaUtils {
813814

814815
// For some coins, signableHex is not a parseable transaction. Pass
815816
// serializedTxHex so verifyTransaction can decode the full tx bytes.
816-
// - ICP: signableHex is a hash; serializedTxHex is the CBOR-encoded tx.
817-
// - BSC/XDC (legacy EIP-155): signableHex is RLP(..., chainId, 0, 0), which
818-
// fails ethereumjs fromSerializedTx EIP-155 v validation; serializedTxHex
819-
// is the unsigned broadcast form and parses cleanly.
820-
// For other coins, verification is typically done using just the signableHex.
821-
const coinFamily = this.baseCoin.getConfig().family;
822-
if (coinFamily === 'icp' || coinFamily === 'bsc' || coinFamily === 'xdc') {
817+
// Gated by CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX (ICP hash digests,
818+
// BSC/XDC legacy EIP-155 RLP with v=chainId, etc.).
819+
if (shouldVerifyWithSerializedTxHex(this.baseCoin)) {
823820
await this.baseCoin.verifyTransaction({
824821
txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex },
825822
txParams: resolveEffectiveTxParams(txRequest, params.txParams),

modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
SignableTransaction,
5252
} from '../baseTypes';
5353
import { shouldUsePreHashedSignable } from '../preHashedSignable';
54+
import { shouldVerifyWithSerializedTxHex } from '../serializedTxHexVerify';
5455
import { BaseEcdsaUtils } from './base';
5556
import { EcdsaMPCv2KeyGenSendFn, KeyGenSenderForEnterprise } from './ecdsaMPCv2KeyGenSender';
5657
import { envRequiresBitgoPubGpgKeyConfig, isBitgoMpcPubKey } from '../../../tss/bitgoPubKeys';
@@ -945,18 +946,12 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
945946

946947
// For some coins, signableHex is not a parseable transaction. Pass
947948
// serializedTxHex so verifyTransaction can decode the full tx bytes.
948-
// - ICP: signableHex is a hash; serializedTxHex is the CBOR-encoded tx.
949+
// - CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX: ICP hash digests,
950+
// BSC/XDC legacy EIP-155 RLP with v=chainId, etc.
949951
// - Avalanche atomic (FLRP/FLR cross-chain): signableHex is SHA-256(txBody);
950952
// serializedTxHex is the PVM/EVM atomic tx (codec prefix 0x0000).
951-
// - BSC/XDC (legacy EIP-155): signableHex is RLP(..., chainId, 0, 0), which
952-
// fails ethereumjs fromSerializedTx EIP-155 v validation; serializedTxHex
953-
// is the unsigned broadcast form and parses cleanly.
954953
// For other coins, signableHex IS the unsigned transaction (e.g. EIP-1559 RLP).
955-
const coinFamily = this.baseCoin.getConfig().family;
956-
const isIcp = coinFamily === 'icp';
957-
const isLegacyEip155Evm = coinFamily === 'bsc' || coinFamily === 'xdc';
958-
const isPreHashed = shouldUsePreHashedSignable(this.baseCoin, unsignedTx);
959-
if (isIcp || isPreHashed || isLegacyEip155Evm) {
954+
if (shouldVerifyWithSerializedTxHex(this.baseCoin) || shouldUsePreHashedSignable(this.baseCoin, unsignedTx)) {
960955
await this.baseCoin.verifyTransaction({
961956
txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex },
962957
txParams: resolveEffectiveTxParams(txRequest, params.txParams),
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { CoinFeature } from '@bitgo/statics';
2+
3+
import { IBaseCoin } from '../../baseCoin';
4+
5+
/**
6+
* Returns true when TSS verifyTransaction should decode serializedTxHex instead of
7+
* signableHex (e.g. ICP hash digests, or legacy EIP-155 RLP that fails ethereumjs
8+
* fromSerializedTx). Controlled via CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX.
9+
*/
10+
export function shouldVerifyWithSerializedTxHex(coin: IBaseCoin): boolean {
11+
return coin.getConfig().features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX);
12+
}

modules/statics/src/account.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
COSMOS_SIDECHAIN_FEATURES,
1111
ERC7984_TOKEN_FEATURES,
1212
TEMPO_FEATURES,
13+
XDC_TOKEN_FEATURES,
1314
} from './coinFeatures';
1415

1516
/**
@@ -3201,7 +3202,7 @@ export function xdcErc20(
32013202
decimalPlaces: number,
32023203
contractAddress: string,
32033204
asset: UnderlyingAsset,
3204-
features: CoinFeature[] = AccountCoin.DEFAULT_FEATURES,
3205+
features: CoinFeature[] = XDC_TOKEN_FEATURES,
32053206
prefix = '',
32063207
suffix: string = name.toUpperCase(),
32073208
network: AccountNetwork = Networks.main.xdc,
@@ -3248,7 +3249,7 @@ export function txdcErc20(
32483249
decimalPlaces: number,
32493250
contractAddress: string,
32503251
asset: UnderlyingAsset,
3251-
features: CoinFeature[] = AccountCoin.DEFAULT_FEATURES,
3252+
features: CoinFeature[] = XDC_TOKEN_FEATURES,
32523253
prefix = '',
32533254
suffix: string = name.toUpperCase(),
32543255
network: AccountNetwork = Networks.test.xdc,

modules/statics/src/base.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,13 @@ export enum CoinFeature {
617617
* behavior. MUST only be set together with TOKENIZED_EQUITY.
618618
*/
619619
BITGO_TOKENIZED_EQUITY = 'bitgo-tokenized-equity',
620+
621+
/**
622+
* TSS transaction verification should use serializedTxHex rather than signableHex.
623+
* Needed when signableHex is not a parseable transaction (e.g. ICP hash digest, or
624+
* legacy EIP-155 RLP with v=chainId that fails ethereumjs fromSerializedTx).
625+
*/
626+
TSS_VERIFY_USE_SERIALIZED_TX_HEX = 'tss-verify-use-serialized-tx-hex',
620627
}
621628

622629
/**

modules/statics/src/coinFeatures.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,15 +324,22 @@ export const BSC_FEATURES = [
324324
CoinFeature.BULK_TRANSACTION,
325325
CoinFeature.SHARED_EVM_MESSAGE_SIGNING,
326326
CoinFeature.ERC20_BULK_TRANSACTION,
327+
CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX,
328+
];
329+
export const BSC_TOKEN_FEATURES = [
330+
...ACCOUNT_COIN_DEFAULT_FEATURES,
331+
CoinFeature.BULK_TRANSACTION,
332+
CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX,
327333
];
328-
export const BSC_TOKEN_FEATURES = [...ACCOUNT_COIN_DEFAULT_FEATURES, CoinFeature.BULK_TRANSACTION];
329334
export const BSC_TOKEN_FEATURES_EXCLUDE_SINGAPORE = [
330335
...ACCOUNT_COIN_DEFAULT_FEATURES_EXCLUDE_SINGAPORE,
331336
CoinFeature.BULK_TRANSACTION,
337+
CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX,
332338
];
333339
export const BSC_TOKEN_FEATURES_EXCLUDE_MENA_FZE = [
334340
...ACCOUNT_COIN_DEFAULT_FEATURES_EXCLUDE_MENA_FZE,
335341
CoinFeature.BULK_TRANSACTION,
342+
CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX,
336343
];
337344
export const STX_FEATURES = [
338345
...ACCOUNT_COIN_DEFAULT_FEATURES,
@@ -658,6 +665,7 @@ export const ICP_FEATURES = [
658665
CoinFeature.SUPPORTS_TOKENS,
659666
CoinFeature.SHA256_WITH_ECDSA_TSS,
660667
CoinFeature.REBUILD_ON_CUSTODY_SIGNING,
668+
CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX,
661669
];
662670

663671
export const STARKNET_FEATURES = [
@@ -758,7 +766,12 @@ export const VET_TOKEN_FEATURES = VET_FEATURES.filter((feature) => feature !== C
758766

759767
export const EVM_NON_EIP1559_FEATURES = [...EVM_FEATURES.filter((feature) => feature !== CoinFeature.EIP1559)];
760768

761-
export const XDC_FEATURES = [...EVM_NON_EIP1559_FEATURES, CoinFeature.ERC20_BULK_TRANSACTION];
769+
export const XDC_FEATURES = [
770+
...EVM_NON_EIP1559_FEATURES,
771+
CoinFeature.ERC20_BULK_TRANSACTION,
772+
CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX,
773+
];
774+
export const XDC_TOKEN_FEATURES = [...ACCOUNT_COIN_DEFAULT_FEATURES, CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX];
762775

763776
export const SGB_FEATURES = [...EVM_FEATURES, CoinFeature.ERC20_BULK_TRANSACTION];
764777

modules/statics/test/unit/coins.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,6 +1273,22 @@ describe('ERC20 Bulk Transaction Feature', () => {
12731273
});
12741274
});
12751275

1276+
describe('TSS Verify Use Serialized Tx Hex Feature', () => {
1277+
it('should have TSS_VERIFY_USE_SERIALIZED_TX_HEX for coins whose signableHex is not parseable', () => {
1278+
const coinsNeedingSerializedTxHexVerify = ['bsc', 'tbsc', 'xdc', 'txdc', 'icp', 'ticp'];
1279+
coinsNeedingSerializedTxHexVerify.forEach((coinName) => {
1280+
const coin = coins.get(coinName);
1281+
coin.features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true);
1282+
});
1283+
});
1284+
1285+
it('should have TSS_VERIFY_USE_SERIALIZED_TX_HEX on BSC and XDC tokens', () => {
1286+
coins.get('bsc:busd').features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true);
1287+
coins.get('xdc:usdc').features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true);
1288+
coins.get('txdc:tmt').features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true);
1289+
});
1290+
});
1291+
12761292
describe('Custody Bulk Withdrawal Features', () => {
12771293
it('should have CUSTODY_BULK_TRANSACTION feature for appropriate coins', () => {
12781294
const custodyBulkWithdrawalCoins = [

0 commit comments

Comments
 (0)