From f009cfb729a9543a3d3d88102b4a10db541898c6 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Thu, 30 Apr 2026 10:58:46 +0200 Subject: [PATCH] feat(abstract-utxo): prefer txHexPsbt over txHex in pending approvals When approving a pending approval, use txHexPsbt (PSBT-lite) over txHex if present. This preserves richer transaction metadata (nonWitnessUtxo, redeemScript, BIP-32 paths) that another user's send already fixed in the correct format. Regular /tx/build responses continue to use txHex; txHexPsbt only appears in pending approval coinSpecific data. Issue: BTC-2650 Co-authored-by: llm-git --- modules/abstract-utxo/src/abstractUtxoCoin.ts | 10 ++- .../fixedScript/parseTransaction.ts | 5 +- .../test/unit/parseTransaction.ts | 38 ++++++++- .../test/unit/pendingApprovalTxHexPsbt.ts | 79 +++++++++++++++++++ .../sdk-core/src/bitgo/baseCoin/iBaseCoin.ts | 5 ++ 5 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 modules/abstract-utxo/test/unit/pendingApprovalTxHexPsbt.ts diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 6248c96e08..4296357d17 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -259,6 +259,12 @@ export interface TransactionPrebuild e txInfo?: TransactionInfo; blockHeight?: number; decodeWith?: SdkBackend; + /** + * PSBT-lite hex present only in pending approval flows, where another user's send fixed the format. + * Not set in regular /tx/build responses (where the caller controls the build parameters). + * Preferred over txHex when present, as it carries richer data (nonWitnessUtxo, redeemScript, BIP-32 paths). + */ + txHexPsbt?: string; } export interface TransactionParams extends BaseTransactionParams { @@ -647,9 +653,11 @@ export abstract class AbstractUtxoCoin decodeTransactionFromPrebuild(prebuild: { txHex?: string; txBase64?: string; + /** See TransactionPrebuild.txHexPsbt — only present in pending approval flows. */ + txHexPsbt?: string; decodeWith?: string; }): DecodedTransaction { - const string = prebuild.txHex ?? prebuild.txBase64; + const string = prebuild.txHexPsbt ?? prebuild.txHex ?? prebuild.txBase64; if (!string) { throw new Error('missing required txHex or txBase64 property'); } diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index f5ff447050..c776cf5883 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -165,7 +165,8 @@ export async function parseTransaction( const keychainArray: Triple = toKeychainTriple(keychains); - if (_.isUndefined(txPrebuild.txHex)) { + const effectiveTxHex = txPrebuild.txHexPsbt ?? txPrebuild.txHex; + if (_.isUndefined(effectiveTxHex)) { throw new Error('missing required txPrebuild property txHex'); } @@ -207,7 +208,7 @@ export async function parseTransaction( // obtain all outputs const explanation: TransactionExplanation = await coin.explainTransaction({ - txHex: txPrebuild.txHex, + txHex: effectiveTxHex, txInfo: txPrebuild.txInfo, decodeWith: txPrebuild.decodeWith, pubs: keychainArray.map((k) => k.pub) as Triple, diff --git a/modules/abstract-utxo/test/unit/parseTransaction.ts b/modules/abstract-utxo/test/unit/parseTransaction.ts index 510efec400..c34b74acbd 100644 --- a/modules/abstract-utxo/test/unit/parseTransaction.ts +++ b/modules/abstract-utxo/test/unit/parseTransaction.ts @@ -58,7 +58,7 @@ describe('Parse Transaction', function () { amount: outputAmount, }, ], - } as TransactionExplanation); + } as unknown as TransactionExplanation); if (!txParams.changeAddress) { stubVerifyAddress = sinon.stub(coin, 'verifyAddress').throws(new UnexpectedAddressError('test error')); @@ -124,4 +124,40 @@ describe('Parse Transaction', function () { recipients: [{ address: externalAddress, amount: outputAmount }], }); }); + + describe('txHexPsbt (pending approval flow)', function () { + it('should pass txHexPsbt to explainTransaction when both txHex and txHexPsbt are present', async function () { + stubExplainTransaction = sinon.stub(coin, 'explainTransaction').resolves({ + outputs: [], + changeOutputs: [], + } as unknown as TransactionExplanation); + + await coin.parseTransaction({ + txParams: {}, + txPrebuild: { txHex: 'legacy-hex', txHexPsbt: 'psbt-hex' }, + wallet: wallet as unknown as UtxoWallet, + verification, + }); + + assert.strictEqual(stubExplainTransaction.callCount, 1); + assert.strictEqual(stubExplainTransaction.getCall(0).args[0].txHex, 'psbt-hex'); + }); + + it('should fall back to txHex when txHexPsbt is absent', async function () { + stubExplainTransaction = sinon.stub(coin, 'explainTransaction').resolves({ + outputs: [], + changeOutputs: [], + } as unknown as TransactionExplanation); + + await coin.parseTransaction({ + txParams: {}, + txPrebuild: { txHex: 'legacy-hex' }, + wallet: wallet as unknown as UtxoWallet, + verification, + }); + + assert.strictEqual(stubExplainTransaction.callCount, 1); + assert.strictEqual(stubExplainTransaction.getCall(0).args[0].txHex, 'legacy-hex'); + }); + }); }); diff --git a/modules/abstract-utxo/test/unit/pendingApprovalTxHexPsbt.ts b/modules/abstract-utxo/test/unit/pendingApprovalTxHexPsbt.ts new file mode 100644 index 0000000000..70a7d76866 --- /dev/null +++ b/modules/abstract-utxo/test/unit/pendingApprovalTxHexPsbt.ts @@ -0,0 +1,79 @@ +import assert from 'assert'; + +import nock = require('nock'); +import * as sinon from 'sinon'; +import { common, PendingApproval, PendingApprovalData, State, Type, Wallet } from '@bitgo/sdk-core'; + +import type { ParsedTransaction } from '../../src/transaction/types'; + +import { defaultBitGo, getUtxoCoin } from './util'; + +nock.disableNetConnect(); + +const coin = getUtxoCoin('tbtc'); +const bgUrl = common.Environments[defaultBitGo.getEnv()].uri; +const walletId = 'wallet0'; +const paId = 'pa0'; + +const paData: PendingApprovalData = { + id: paId, + wallet: walletId, + state: State.PENDING, + creator: 'test', + info: { + type: Type.TRANSACTION_REQUEST, + transactionRequest: { + coinSpecific: { + tbtc: { txHex: 'legacy-hex', txHexPsbt: 'psbt-hex' }, + }, + recipients: [], + // non-empty buildParams.recipients → canRecreateTransaction returns true for tbtc + buildParams: { recipients: [{ address: '2NAuziD75WnPPHJVwnd4ckgY4SuJaDVVbMD', amount: '100000' }] }, + sourceWallet: walletId, + }, + }, +}; + +describe('PendingApproval txHexPsbt flow', function () { + let sandbox: sinon.SinonSandbox; + let wallet: Wallet; + let pendingApproval: PendingApproval; + + beforeEach(function () { + sandbox = sinon.createSandbox(); + wallet = new Wallet(defaultBitGo, coin, { id: walletId, coin: 'tbtc', keys: [] }); + pendingApproval = new PendingApproval(defaultBitGo, coin, paData, wallet); + }); + + afterEach(function () { + sandbox.restore(); + nock.cleanAll(); + }); + + it('passes coinSpecific.txHexPsbt to parseTransaction as txPrebuild.txHexPsbt', async function () { + // Stub prebuildAndSignTransaction so we don't need /tx/build nocks or real signing + sandbox.stub(wallet, 'prebuildAndSignTransaction').resolves({ txHex: 'rebuilt' }); + + // Capture the args passed to parseTransaction; returning no implicitExternalSpendAmount + // causes recreateAndSignTransaction to return early after the first parseTransaction call + const parseStub = sandbox + .stub(coin, 'parseTransaction') + .resolves({ outputs: [], changeOutputs: [] } as unknown as ParsedTransaction); + + nock(bgUrl) + .put(`/api/v2/tbtc/pendingapprovals/${paId}`) + .reply(200, { ...paData, state: 'approved' }); + + await pendingApproval.approve({ xprv: 'dummy' }); + + // recreateAndSignTransaction calls parseTransaction twice: + // first with originalPrebuild (coinSpecific), then with the rebuilt tx. + // The first call must carry txHexPsbt from coinSpecific. + assert.ok(parseStub.callCount >= 1, 'parseTransaction should have been called'); + assert.strictEqual( + parseStub.firstCall.args[0].txPrebuild.txHexPsbt, + 'psbt-hex', + 'first parseTransaction call should carry txHexPsbt from coinSpecific' + ); + }); +}); diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index 9f127d619f..145972207d 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -434,6 +434,11 @@ export interface FullySignedTransaction { export interface HalfSignedUtxoTransaction { txHex: string; + /** + * PSBT-lite hex preserved from a pending approval's coinSpecific. + * Only present in the PA approval flow — not in regular sign/send flows. + */ + txHexPsbt?: string; } export interface HalfSignedAccountTransaction {