From 3762bc66536bde23eb657c59be48bea53e6dacf5 Mon Sep 17 00:00:00 2001 From: Marzooqa Naeema Kather Date: Mon, 31 Aug 2026 13:13:32 +0530 Subject: [PATCH] fix(abstract-eth,sdk-core): use coin chain ID when deriving signableHex (WCI-1398) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unsigned EVM transactions have v=0 — no chain ID in the serialized bytes. TransactionFactory.fromSerializedData without an explicit Common defaults to mainnet (chain ID 1), while the server computes signableHex with the correct chain ID (e.g. 97 for BSC testnet), causing a false-positive tamper error on every signing flow. Fix: pass getCustomChainCommon(this.getChainId()) when parsing the serialized tx so the EIP-155 preimage is derived with the correct chain ID. For coins without CoinFeature.EIP1559 (BSC, XDC), pin the Common hardfork to petersburg so type-2 txs cannot be decoded as EIP-1559. Re-enable the assertSignableConsistency call in ecdsaMPCv2. TICKET: WCI-1398 Co-authored-by: Cursor --- .../src/abstractEthLikeNewCoins.ts | 8 ++++- modules/sdk-coin-bsc/test/unit/bsc.ts | 30 +++++++++++++++--- modules/sdk-coin-xdc/test/unit/xdc.ts | 31 +++++++++++++++---- .../src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts | 4 +++ 4 files changed, 61 insertions(+), 12 deletions(-) diff --git a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts index 75110f12d4..c353f958f6 100644 --- a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts +++ b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts @@ -3179,7 +3179,13 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem * /** @inheritdoc CoinWithSignableConsistency */ assertSignableConsistency(serializedTxHex: string, signableHex: string): void { - const ethTx = TransactionFactory.fromSerializedData(toBuffer(addHexPrefix(serializedTxHex))); + const common = AbstractEthLikeNewCoins.getCustomChainCommon(this.getChainId()); + // getCustomChainCommon defaults to the london hardfork, which is only valid for chains + // that activated EIP-1559. Chains without it (e.g. BSC, XDC) sign under petersburg. + if (!this.staticsCoin?.features.includes(CoinFeature.EIP1559)) { + common.setHardfork(optionalDeps.EthCommon.Hardfork.Petersburg); + } + const ethTx = TransactionFactory.fromSerializedData(toBuffer(addHexPrefix(serializedTxHex)), { common }); const derivedSignableHex = ethTx instanceof FeeMarketEIP1559Transaction ? ethTx.getMessageToSign(false).toString('hex') diff --git a/modules/sdk-coin-bsc/test/unit/bsc.ts b/modules/sdk-coin-bsc/test/unit/bsc.ts index 29ed2d2ae8..6badbedd83 100644 --- a/modules/sdk-coin-bsc/test/unit/bsc.ts +++ b/modules/sdk-coin-bsc/test/unit/bsc.ts @@ -3,10 +3,11 @@ import 'should'; import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; import { BitGoAPI } from '@bitgo/sdk-api'; import { TransactionType, Wallet } from '@bitgo/sdk-core'; -import { Transaction as LegacyTransaction } from '@ethereumjs/tx'; +import { FeeMarketEIP1559Transaction, Transaction as LegacyTransaction } from '@ethereumjs/tx'; import { RLP } from '@ethereumjs/rlp'; import { bufArrToArr } from 'ethereumjs-util'; +import { AbstractEthLikeNewCoins } from '@bitgo/abstract-eth'; import { Bsc, Tbsc } from '../../src/index'; import { TransactionBuilder } from '../../src/lib'; import { getBuilder } from './getBuilder'; @@ -242,15 +243,16 @@ describe('Native BNB', function () { return stripHex(tx.toBroadcastFormat()); } - function deriveSignableHex(serializedTxHex: string): string { - const legacyTx = LegacyTransaction.fromSerializedTx(Buffer.from(serializedTxHex, 'hex')); + function deriveSignableHex(serializedTxHex: string, coin: Tbsc): string { + const common = AbstractEthLikeNewCoins.getCustomChainCommon(coin.getChainId()); + const legacyTx = LegacyTransaction.fromSerializedTx(Buffer.from(serializedTxHex, 'hex'), { common }); return Buffer.from(RLP.encode(bufArrToArr(legacyTx.getMessageToSign(false)))).toString('hex'); } it('should pass when signableHex is consistent with serializedTxHex', async function () { const coin = bitgo.coin('tbsc') as Tbsc; const serializedTxHex = await buildSerializedTxHex(recipientAddress); - const signableHex = deriveSignableHex(serializedTxHex); + const signableHex = deriveSignableHex(serializedTxHex, coin); coin.assertSignableConsistency(serializedTxHex, signableHex); }); @@ -258,11 +260,29 @@ describe('Native BNB', function () { const coin = bitgo.coin('tbsc') as Tbsc; const benignSerializedTxHex = await buildSerializedTxHex(recipientAddress); const maliciousSerializedTxHex = await buildSerializedTxHex(wrongAddress); - const tamperedSignableHex = deriveSignableHex(maliciousSerializedTxHex); + const tamperedSignableHex = deriveSignableHex(maliciousSerializedTxHex, coin); (() => coin.assertSignableConsistency(benignSerializedTxHex, tamperedSignableHex)).should.throw( 'signableHex is inconsistent with serializedTxHex: possible server tampering' ); }); + + it('should reject an EIP-1559 typed tx since BSC is pinned to the petersburg hardfork', function () { + const coin = bitgo.coin('tbsc') as Tbsc; + const typedTx = FeeMarketEIP1559Transaction.fromTxData( + { + nonce: 1, + maxFeePerGas: 10, + maxPriorityFeePerGas: 1, + gasLimit: 21000, + to: recipientAddress, + value: 1000, + }, + { common: AbstractEthLikeNewCoins.getCustomChainCommon(coin.getChainId()) } + ); + (() => coin.assertSignableConsistency(stripHex(typedTx.serialize().toString('hex')), '')).should.throw( + /EIP-1559 not enabled on Common/ + ); + }); }); }); }); diff --git a/modules/sdk-coin-xdc/test/unit/xdc.ts b/modules/sdk-coin-xdc/test/unit/xdc.ts index 2e30248e6d..f45db142b9 100644 --- a/modules/sdk-coin-xdc/test/unit/xdc.ts +++ b/modules/sdk-coin-xdc/test/unit/xdc.ts @@ -4,11 +4,11 @@ import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; import { BitGoAPI } from '@bitgo/sdk-api'; import { Xdc, Txdc } from '../../src/index'; -import { UnsignedSweepTxMPCv2 } from '@bitgo/abstract-eth'; +import { AbstractEthLikeNewCoins, UnsignedSweepTxMPCv2 } from '@bitgo/abstract-eth'; import { mockDataUnsignedSweep, mockDataNonBitGoRecovery } from '../resources'; import nock from 'nock'; import { common, TransactionType, Wallet } from '@bitgo/sdk-core'; -import { Transaction } from '@ethereumjs/tx'; +import { FeeMarketEIP1559Transaction, Transaction } from '@ethereumjs/tx'; import { RLP } from '@ethereumjs/rlp'; import { stripHexPrefix } from '@ethereumjs/util'; import { bufArrToArr } from 'ethereumjs-util'; @@ -186,15 +186,16 @@ describe('xdc', function () { return stripHex(tx.toBroadcastFormat()); } - function deriveSignableHex(serializedTxHex: string): string { - const legacyTx = Transaction.fromSerializedTx(Buffer.from(serializedTxHex, 'hex')); + function deriveSignableHex(serializedTxHex: string, coin: Txdc): string { + const common = AbstractEthLikeNewCoins.getCustomChainCommon(coin.getChainId()); + const legacyTx = Transaction.fromSerializedTx(Buffer.from(serializedTxHex, 'hex'), { common }); return Buffer.from(RLP.encode(bufArrToArr(legacyTx.getMessageToSign(false)))).toString('hex'); } it('should pass when signableHex is consistent with serializedTxHex', async function () { const coin = bitgo.coin('txdc') as Txdc; const serializedTxHex = await buildSerializedTxHex(recipientAddress); - const signableHex = deriveSignableHex(serializedTxHex); + const signableHex = deriveSignableHex(serializedTxHex, coin); coin.assertSignableConsistency(serializedTxHex, signableHex); }); @@ -202,11 +203,29 @@ describe('xdc', function () { const coin = bitgo.coin('txdc') as Txdc; const benignSerializedTxHex = await buildSerializedTxHex(recipientAddress); const maliciousSerializedTxHex = await buildSerializedTxHex(wrongAddress); - const tamperedSignableHex = deriveSignableHex(maliciousSerializedTxHex); + const tamperedSignableHex = deriveSignableHex(maliciousSerializedTxHex, coin); (() => coin.assertSignableConsistency(benignSerializedTxHex, tamperedSignableHex)).should.throw( 'signableHex is inconsistent with serializedTxHex: possible server tampering' ); }); + + it('should reject an EIP-1559 typed tx since XDC is pinned to the petersburg hardfork', function () { + const coin = bitgo.coin('txdc') as Txdc; + const typedTx = FeeMarketEIP1559Transaction.fromTxData( + { + nonce: 1, + maxFeePerGas: 10, + maxPriorityFeePerGas: 1, + gasLimit: 21000, + to: recipientAddress, + value: 1000, + }, + { common: AbstractEthLikeNewCoins.getCustomChainCommon(coin.getChainId()) } + ); + (() => coin.assertSignableConsistency(stripHex(typedTx.serialize().toString('hex')), '')).should.throw( + /EIP-1559 not enabled on Common/ + ); + }); }); }); }); 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 478107d3ef..787f0cf143 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts @@ -52,6 +52,7 @@ import { } from '../baseTypes'; import { shouldUsePreHashedSignable } from '../preHashedSignable'; import { shouldVerifyWithSerializedTxHex } from '../serializedTxHexVerify'; +import { isCoinWithSignableConsistency } from '../signableConsistency'; import { BaseEcdsaUtils } from './base'; import { EcdsaMPCv2KeyGenSendFn, KeyGenSenderForEnterprise } from './ecdsaMPCv2KeyGenSender'; import { envRequiresBitgoPubGpgKeyConfig, isBitgoMpcPubKey } from '../../../tss/bitgoPubKeys'; @@ -958,6 +959,9 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils { wallet: this.wallet, walletType: this.wallet.multisigType(), }); + if (shouldVerifyWithSerializedTxHex(this.baseCoin) && isCoinWithSignableConsistency(this.baseCoin)) { + this.baseCoin.assertSignableConsistency(unsignedTx.serializedTxHex, unsignedTx.signableHex); + } } else { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.signableHex },