From aa030413f9f441298d07064ec5823cfff04a7877 Mon Sep 17 00:00:00 2001 From: Luis Covarrubias Date: Fri, 14 Nov 2025 15:52:32 -0800 Subject: [PATCH] feat: verify rbfTxId matches decoded transaction id Add validation to confirm the provided rbfTxId matches the ID of the transaction being replaced. This prevents potential discrepancies and ensures the proper transaction is being modified during RBF operations. Co-authored-by: llm-git Ticket: BTC-2785 TICKET: BTC-2785 --- .../fixedScript/parseTransaction.ts | 13 ++- .../test/unit/parseTransaction.ts | 79 ++++++++++++++++++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index 28879a88f2..6f73978423 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -65,7 +65,18 @@ export async function parseTransaction( if (txParams.rbfTxIds) { assert(txParams.rbfTxIds.length === 1); - const txToBeReplaced = await wallet.getTransaction({ txHash: txParams.rbfTxIds[0], includeRbf: true }); + const rbfTxId = txParams.rbfTxIds[0]; + + // validate the fetched transaction matches the provided rbfTxId + const txToBeReplaced = await wallet.getTransaction({ txHash: rbfTxId, includeRbf: true }); + const decodedRbfTransactionId = (await coin.explainTransaction({ txHex: txToBeReplaced.txHex })).id; + + if (decodedRbfTransactionId !== rbfTxId) { + throw new Error( + `The provided rbfTxId ${rbfTxId} does not match the decoded transaction id ${decodedRbfTransactionId}` + ); + } + expectedOutputs = txToBeReplaced.outputs.flatMap( (output: { valueString: string; address?: string; wallet?: string }) => { // For self-sends, the walletId will be the same as the wallet's id diff --git a/modules/abstract-utxo/test/unit/parseTransaction.ts b/modules/abstract-utxo/test/unit/parseTransaction.ts index f746c9794b..c95a0319a8 100644 --- a/modules/abstract-utxo/test/unit/parseTransaction.ts +++ b/modules/abstract-utxo/test/unit/parseTransaction.ts @@ -6,7 +6,7 @@ import { Wallet, UnexpectedAddressError, VerificationOptions } from '@bitgo/sdk- import { UtxoWallet, Output, TransactionParams } from '../../src'; import type { TransactionExplanation } from '../../src/transaction/fixedScript/explainTransaction'; -import { getUtxoCoin } from './util'; +import { getUtxoCoin, getUtxoWallet } from './util'; describe('Parse Transaction', function () { const coin = getUtxoCoin('tbtc'); @@ -123,4 +123,81 @@ describe('Parse Transaction', function () { recipients: [{ address: externalAddress, amount: outputAmount }], }); }); + + describe('RBF Transaction ID Validation', function () { + let rbfWallet: Wallet; + let stubExplain: sinon.SinonStub; + + beforeEach(function () { + rbfWallet = getUtxoWallet(coin, { + id: '5b34252f1bf349930e34020a', + coin: 'tbtc', + keys: ['5b3424f91bf349930e340175', '5b3424f91bf349930e340176', '5b3424f91bf349930e340177'], + }); + }); + + afterEach(function () { + if (stubExplain) { + stubExplain.restore(); + } + sinon.restore(); + }); + + it('should throw error when decoded transaction ID does not match rbfTxId', async function () { + const providedRbfTxId = 'tx-to-be-replaced'; + const decodedTxId = 'actual-decoded-tx-id'; + + // Stub wallet.getTransaction + sinon.stub(rbfWallet, 'getTransaction').resolves({ + id: providedRbfTxId, + txHex: '0100000001', + outputs: [ + { + address: '2MzQwSSnBHWHqSAqtTVQ6v47XtaisrJa1Vc', + value: 1000000, + valueString: '1000000', + wallet: 'some-other-wallet-id', + }, + ], + }); + + stubExplain = sinon.stub(coin, 'explainTransaction'); + stubExplain.onCall(0).resolves({ + id: 'new-tx-id', + outputs: [] as Output[], + changeOutputs: [] as Output[], + } as TransactionExplanation); + + // Second call: decoding the old RBF transaction with mismatched ID + stubExplain.onCall(1).resolves({ + id: decodedTxId, // Different from providedRbfTxId + outputs: [] as Output[], + changeOutputs: [] as Output[], + } as TransactionExplanation); + + try { + await coin.parseTransaction({ + txParams: { + rbfTxIds: [providedRbfTxId], + }, + txPrebuild: { txHex: '0100000001' }, + wallet: rbfWallet as unknown as UtxoWallet, + verification: { + disableNetworking: true, + keychains: { + user: { id: '0', pub: 'aaa', type: 'independent' }, + backup: { id: '1', pub: 'bbb', type: 'independent' }, + bitgo: { id: '2', pub: 'ccc', type: 'independent' }, + }, + }, + }); + assert.fail('Should have thrown an error'); + } catch (error) { + assert.ok(error instanceof Error); + // Verify the error message matches the expected validation error + const expectedMessage = `The provided rbfTxId ${providedRbfTxId} does not match the decoded transaction id ${decodedTxId}`; + assert.strictEqual(error.message, expectedMessage); + } + }); + }); });