From 765df326a52237322fd76813b787b902d420cf08 Mon Sep 17 00:00:00 2001 From: Prabhsharan Singh Date: Thu, 21 May 2026 20:16:17 +0530 Subject: [PATCH] feat(abstract-eth): override getSignablePayload for ETH coin classes Override `getSignablePayload` on `AbstractEthLikeCoin` to return the keccak256 hash of the unsigned transaction (via `getMessageToSign(true)`) rather than raw serialized bytes. This exposes the correct bytes AKM needs for its external POST /sign endpoint. Changes: - Add `getSignablePayload(): Buffer` to `EthLikeTransactionData` interface and implement it in `EthTransactionData` using `tx.getMessageToSign(true)` - Add `get signablePayload(): Buffer` to the `Transaction` class, delegating to `EthTransactionData.getSignablePayload()` - Override `getSignablePayload(serializedTx)` in `AbstractEthLikeCoin`, rebuilding via the transaction builder and returning `tx.signablePayload` - Add unit tests covering Legacy and EIP1559 transaction types, and empty-transaction error handling Ticket: CGD-1083 --- .../abstract-eth/src/abstractEthLikeCoin.ts | 10 +++- modules/abstract-eth/src/ethLikeToken.ts | 5 ++ modules/abstract-eth/src/lib/iface.ts | 5 ++ modules/abstract-eth/src/lib/transaction.ts | 8 +++ modules/abstract-eth/src/lib/types.ts | 4 ++ modules/abstract-eth/test/unit/transaction.ts | 15 ++++++ modules/sdk-coin-bsc/test/unit/bscToken.ts | 12 +++++ modules/sdk-coin-eth/test/unit/eth.ts | 54 +++++++++++++++++++ modules/sdk-coin-eth/test/unit/transaction.ts | 24 +++++++++ 9 files changed, 136 insertions(+), 1 deletion(-) diff --git a/modules/abstract-eth/src/abstractEthLikeCoin.ts b/modules/abstract-eth/src/abstractEthLikeCoin.ts index b8e75995b7..4192ba52c6 100644 --- a/modules/abstract-eth/src/abstractEthLikeCoin.ts +++ b/modules/abstract-eth/src/abstractEthLikeCoin.ts @@ -27,7 +27,7 @@ import { } from '@bitgo/sdk-core'; import BigNumber from 'bignumber.js'; -import { isValidEthAddress, KeyPair as EthKeyPair, TransactionBuilder } from './lib'; +import { isValidEthAddress, KeyPair as EthKeyPair, Transaction as EthTransaction, TransactionBuilder } from './lib'; import { VerifyEthAddressOptions } from './abstractEthLikeNewCoins'; import { auditEcdsaPrivateKey } from '@bitgo/sdk-lib-mpc'; @@ -230,6 +230,14 @@ export abstract class AbstractEthLikeCoin extends BaseCoin { }; } + /** @inheritDoc */ + async getSignablePayload(serializedTx: string): Promise { + const txBuilder = this.getTransactionBuilder(); + txBuilder.from(serializedTx); + const tx = (await txBuilder.build()) as EthTransaction; + return tx.signablePayload; + } + /** * Create a new transaction builder for the current chain * @return a new transaction builder diff --git a/modules/abstract-eth/src/ethLikeToken.ts b/modules/abstract-eth/src/ethLikeToken.ts index 6e3e09d79c..eb14626a82 100644 --- a/modules/abstract-eth/src/ethLikeToken.ts +++ b/modules/abstract-eth/src/ethLikeToken.ts @@ -399,6 +399,11 @@ export class EthLikeToken extends AbstractEthLikeNewCoins { return txPrebuild.coin === this.tokenConfig.coin && txPrebuild.token === this.tokenConfig.type; } + /** @inheritDoc */ + async getSignablePayload(serializedTx: string): Promise { + return Buffer.from(serializedTx); + } + /** * Create a new transaction builder for the current chain * @return a new transaction builder diff --git a/modules/abstract-eth/src/lib/iface.ts b/modules/abstract-eth/src/lib/iface.ts index a97a67eb0f..21f482051c 100644 --- a/modules/abstract-eth/src/lib/iface.ts +++ b/modules/abstract-eth/src/lib/iface.ts @@ -91,6 +91,11 @@ export interface EthLikeTransactionData { * Return the hex string serialization of this transaction */ toSerialized(): string; + + /** + * Return the keccak256 hash of the unsigned transaction — the 32-byte digest an external signer must sign + */ + getSignablePayload(): Buffer; } export interface SignatureParts { diff --git a/modules/abstract-eth/src/lib/transaction.ts b/modules/abstract-eth/src/lib/transaction.ts index 221ccad20f..971211e4ed 100644 --- a/modules/abstract-eth/src/lib/transaction.ts +++ b/modules/abstract-eth/src/lib/transaction.ts @@ -169,6 +169,14 @@ export class Transaction extends BaseTransaction { this._signatures.push(toStringSig({ v: txData.v!, r: txData.r!, s: txData.s! })); } + /** @inheritdoc */ + get signablePayload(): Buffer { + if (!this._transactionData) { + throw new InvalidTransactionError('No transaction data to sign'); + } + return this._transactionData.getSignablePayload(); + } + /** @inheritdoc */ toBroadcastFormat(): string { if (this._transactionData) { diff --git a/modules/abstract-eth/src/lib/types.ts b/modules/abstract-eth/src/lib/types.ts index 993d79d640..707a7ca0e3 100644 --- a/modules/abstract-eth/src/lib/types.ts +++ b/modules/abstract-eth/src/lib/types.ts @@ -91,6 +91,10 @@ export class EthTransactionData implements EthLikeTransactionData { this.tx = this.tx.sign(privateKey); } + getSignablePayload(): Buffer { + return Buffer.from(this.tx.getMessageToSign(true)); + } + /** @inheritdoc */ toJson(): TxData { const result: BaseTxData = { diff --git a/modules/abstract-eth/test/unit/transaction.ts b/modules/abstract-eth/test/unit/transaction.ts index e259beaff5..b41cf336c2 100644 --- a/modules/abstract-eth/test/unit/transaction.ts +++ b/modules/abstract-eth/test/unit/transaction.ts @@ -57,5 +57,20 @@ export function runTransactionTests(coinName: string, testData: any, common: Com should.equal(tx.toBroadcastFormat(), testData.ENCODED_TRANSACTION); }); }); + + describe('signablePayload', () => { + it('should throw on an empty transaction', () => { + const tx = getTransaction(); + should.throws(() => tx.signablePayload); + }); + + it('should return a 32-byte keccak256 hash for an unsigned transaction', () => { + const tx = getTransaction(); + tx.setTransactionData(testData.TXDATA); + const payload = tx.signablePayload; + payload.should.be.instanceof(Buffer); + payload.length.should.equal(32); + }); + }); }); } diff --git a/modules/sdk-coin-bsc/test/unit/bscToken.ts b/modules/sdk-coin-bsc/test/unit/bscToken.ts index da88dd6b7e..2ac9068eb2 100644 --- a/modules/sdk-coin-bsc/test/unit/bscToken.ts +++ b/modules/sdk-coin-bsc/test/unit/bscToken.ts @@ -29,4 +29,16 @@ describe('Bsc Token:', function () { bscTokenCoin.network.should.equal('Testnet'); bscTokenCoin.decimalPlaces.should.equal(18); }); + + describe('getSignablePayload', function () { + // Tokens are out of scope for CGD-1083; EthLikeToken intentionally preserves + // the pre-PR behavior of returning the raw serialized tx bytes rather than + // the keccak256 hash returned by the parent chain coin. + it('should return the raw serialized bytes unchanged', async function () { + const serializedTx = + '0xf86b808504a817c80082520894eeaf0f05f37891ab4a21208b105a0687d12c5af7880de0b6b3a76400008025a0'; + const payload = await bscTokenCoin.getSignablePayload(serializedTx); + payload.should.deepEqual(Buffer.from(serializedTx)); + }); + }); }); diff --git a/modules/sdk-coin-eth/test/unit/eth.ts b/modules/sdk-coin-eth/test/unit/eth.ts index 0f34dd082b..d801cf24fa 100644 --- a/modules/sdk-coin-eth/test/unit/eth.ts +++ b/modules/sdk-coin-eth/test/unit/eth.ts @@ -2593,4 +2593,58 @@ describe('ETH:', function () { }); }); }); + + describe('getSignablePayload', function () { + let coin: Teth; + + before(function () { + coin = bitgo.coin('teth') as Teth; + }); + + it('should return a 32-byte keccak256 hash for a Legacy transaction', async function () { + const txBuilder = getBuilder('teth') as TransactionBuilder; + txBuilder.type(TransactionType.Send); + txBuilder.fee({ fee: '10000000000', gasLimit: '7000000' }); + txBuilder.counter(1); + txBuilder.contract('0x8Ce59c2d1702844F8EdED451AA103961bC37B4e8'); + const transferBuilder = txBuilder.transfer() as TransferBuilder; + transferBuilder + .coin('teth') + .expirationTime(Math.floor(Date.now() / 1000) + 3600) + .amount('100000') + .to('0xeeaf0F05f37891ab4a21208B105A0687d12c5aF7') + .contractSequenceId(1); + const tx = await txBuilder.build(); + const serializedTx = tx.toBroadcastFormat(); + + const payload = await coin.getSignablePayload(serializedTx); + assert.ok(Buffer.isBuffer(payload)); + assert.strictEqual(payload.length, 32); + }); + + it('should return a 32-byte keccak256 hash for an EIP1559 transaction', async function () { + const txBuilder = getBuilder('teth') as TransactionBuilder; + txBuilder.type(TransactionType.Send); + txBuilder.fee({ + fee: '280000000000', + gasLimit: '7000000', + eip1559: { maxFeePerGas: '7593123', maxPriorityFeePerGas: '150' }, + }); + txBuilder.counter(1); + txBuilder.contract('0x8Ce59c2d1702844F8EdED451AA103961bC37B4e8'); + const transferBuilder = txBuilder.transfer() as TransferBuilder; + transferBuilder + .coin('teth') + .expirationTime(Math.floor(Date.now() / 1000) + 3600) + .amount('100000') + .to('0xeeaf0F05f37891ab4a21208B105A0687d12c5aF7') + .contractSequenceId(1); + const tx = await txBuilder.build(); + const serializedTx = tx.toBroadcastFormat(); + + const payload = await coin.getSignablePayload(serializedTx); + assert.ok(Buffer.isBuffer(payload)); + assert.strictEqual(payload.length, 32); + }); + }); }); diff --git a/modules/sdk-coin-eth/test/unit/transaction.ts b/modules/sdk-coin-eth/test/unit/transaction.ts index fcc9ba37c5..1bc426b31d 100644 --- a/modules/sdk-coin-eth/test/unit/transaction.ts +++ b/modules/sdk-coin-eth/test/unit/transaction.ts @@ -61,4 +61,28 @@ describe('ETH Transaction', () => { }); }); }); + + describe('signablePayload', () => { + it('should throw on an empty transaction', () => { + const tx = getTransaction(); + assert.throws(() => tx.signablePayload); + }); + + testParams.map(([txnType, txData]) => { + it(`should return the keccak256 signing hash for a ${txnType} transaction`, () => { + const tx = getTransaction(txData); + const payload = tx.signablePayload; + assert.ok(Buffer.isBuffer(payload)); + assert.strictEqual(payload.length, 32); + // txData.id is set from getMessageToSign() in toJson() — must match the signing payload + assert.strictEqual('0x' + payload.toString('hex'), txData.id); + }); + }); + + it('should produce distinct payloads for Legacy and EIP1559 transactions', () => { + const legacy = getTransaction(testData.LEGACY_TXDATA).signablePayload; + const eip1559 = getTransaction(testData.EIP1559_TXDATA).signablePayload; + assert.notStrictEqual(legacy.toString('hex'), eip1559.toString('hex')); + }); + }); });