diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index c1598ef1c6..5c139a3247 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -338,6 +338,11 @@ type UtxoBaseSignTransactionOptions = */ returnLegacyFormat?: boolean; wallet?: UtxoWallet; + /** + * When true (default), extract finalized PSBT to legacy transaction format. + * When false, return finalized PSBT. Useful for testing to keep transactions in PSBT format. + */ + extractTransaction?: boolean; }; export type SignTransactionOptions = UtxoBaseSignTransactionOptions & @@ -406,6 +411,11 @@ export abstract class AbstractUtxoCoin public readonly amountType: 'number' | 'bigint'; + protected readonly supportedTxFormats: { readonly psbt: boolean; readonly legacy: boolean } = { + psbt: true, + legacy: false, + }; + protected constructor(bitgo: BitGoBase, amountType: 'number' | 'bigint' = 'number') { super(bitgo); this.amountType = amountType; @@ -582,8 +592,15 @@ export abstract class AbstractUtxoCoin } if (utxolib.bitgo.isPsbt(input)) { + if (!this.supportedTxFormats.psbt) { + throw new ErrorDeprecatedTxFormat('psbt'); + } return decodePsbtWith(input, this.name, decodeWith); } else { + // Legacy format transactions are deprecated. This will be an unconditional error in the future. + if (!this.supportedTxFormats.legacy) { + throw new ErrorDeprecatedTxFormat('legacy'); + } if (decodeWith !== 'utxolib') { console.error('received decodeWith hint %s, ignoring for legacy transaction', decodeWith); } diff --git a/modules/abstract-utxo/src/transaction/fixedScript/signTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/signTransaction.ts index 4844733d88..493832be6f 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/signTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/signTransaction.ts @@ -67,6 +67,8 @@ export async function signTransaction< allowNonSegwitSigningWithoutPrevTx: boolean; pubs: string[] | undefined; cosignerPub: string | undefined; + /** When true (default), extract finalized PSBT to legacy transaction format. When false, return finalized PSBT. */ + extractTransaction?: boolean; } ): Promise< utxolib.bitgo.UtxoPsbt | utxolib.bitgo.UtxoTransaction | fixedScriptWallet.BitGoPsbt | Buffer @@ -77,6 +79,8 @@ export async function signTransaction< isLastSignature = params.isLastSignature; } + const { extractTransaction = true } = params; + if (tx instanceof bitgo.UtxoPsbt) { const signedPsbt = await signPsbtWithMusig2ParticipantUtxolib( coin as Musig2Participant, @@ -88,8 +92,12 @@ export async function signTransaction< } ); if (isLastSignature) { - signedPsbt.finalizeAllInputs(); - return signedPsbt.extractTransaction(); + if (extractTransaction) { + signedPsbt.finalizeAllInputs(); + return signedPsbt.extractTransaction(); + } + // Return signed PSBT without finalizing to preserve derivation info + return signedPsbt; } return signedPsbt; } else if (tx instanceof fixedScriptWallet.BitGoPsbt) { @@ -110,8 +118,12 @@ export async function signTransaction< } ); if (isLastSignature) { - signedPsbt.finalizeAllInputs(); - return Buffer.from(signedPsbt.extractTransaction().toBytes()); + if (extractTransaction) { + signedPsbt.finalizeAllInputs(); + return Buffer.from(signedPsbt.extractTransaction().toBytes()); + } + // Return finalized PSBT without extracting to legacy format + return signedPsbt; } return signedPsbt; } diff --git a/modules/abstract-utxo/src/transaction/signTransaction.ts b/modules/abstract-utxo/src/transaction/signTransaction.ts index 85f0f42313..ebc487aa2c 100644 --- a/modules/abstract-utxo/src/transaction/signTransaction.ts +++ b/modules/abstract-utxo/src/transaction/signTransaction.ts @@ -78,6 +78,7 @@ export async function signTransaction( allowNonSegwitSigningWithoutPrevTx: params.allowNonSegwitSigningWithoutPrevTx ?? false, pubs: params.pubs, cosignerPub: params.cosignerPub, + extractTransaction: params.extractTransaction, }); // Convert half-signed PSBT to legacy format when the caller explicitly requested txFormat: 'legacy' diff --git a/modules/abstract-utxo/test/unit/customSigner.ts b/modules/abstract-utxo/test/unit/customSigner.ts index 28da05b3b1..0e673009aa 100644 --- a/modules/abstract-utxo/test/unit/customSigner.ts +++ b/modules/abstract-utxo/test/unit/customSigner.ts @@ -83,20 +83,4 @@ describe('UTXO Custom Signer Function', function () { sinon.assert.calledOnce(customSigningFunction as sinon.SinonStub); scope.done(); }); - - it('should use a custom signing function if provided for Tx without taprootKeyPathSpend input', async function () { - const tx = utxoLib.testutil.constructTxnBuilder( - [{ scriptType: 'p2wsh', value: BigInt(1000) }], - [{ scriptType: 'p2sh', value: BigInt(900) }], - basecoin.network, - rootWalletKey, - 'unsigned' - ); - const scope = nocks({ txHex: tx.buildIncomplete().toHex() }); - const result = await wallet.sendMany({ recipients, customSigningFunction }); - - assertHasProperty(result, 'ok', true); - sinon.assert.calledOnce(customSigningFunction as sinon.SinonStub); - scope.done(); - }); }); diff --git a/modules/abstract-utxo/test/unit/signTransaction.ts b/modules/abstract-utxo/test/unit/signTransaction.ts index 7a5f67d5c1..cc79f732ba 100644 --- a/modules/abstract-utxo/test/unit/signTransaction.ts +++ b/modules/abstract-utxo/test/unit/signTransaction.ts @@ -6,7 +6,7 @@ import nock = require('nock'); import { testutil } from '@bitgo/utxo-lib'; import { common, Triple } from '@bitgo/sdk-core'; -import { getReplayProtectionPubkeys } from '../../src'; +import { getReplayProtectionPubkeys, ErrorDeprecatedTxFormat } from '../../src'; import type { Unspent } from '../../src/unspent'; import { getUtxoWallet, getDefaultWalletKeys, getUtxoCoin, keychainsBase58, defaultBitGo } from './util'; @@ -170,7 +170,7 @@ describe('signTransaction', function () { } }); - it('customSigningFunction flow - Network Tx', async function () { + it('customSigningFunction flow - Network Tx should reject legacy format', async function () { const inputs: testutil.TxnInput[] = testutil.txnInputScriptTypes .filter((v) => v !== 'p2shP2pk') .map((scriptType) => ({ @@ -182,9 +182,10 @@ describe('signTransaction', function () { const txBuilder = testutil.constructTxnBuilder(inputs, outputs, coin.network, rootWalletKeys, 'unsigned'); const unspents = inputs.map((v, i) => testutil.toTxnUnspent(v, i, coin.network, rootWalletKeys)); - for (const v of [false, true]) { - await signTransaction(txBuilder.buildIncomplete(), v, unspents); - } + // Legacy format transactions are now deprecated and should throw ErrorDeprecatedTxFormat + await assert.rejects(async () => { + await signTransaction(txBuilder.buildIncomplete(), false, unspents); + }, ErrorDeprecatedTxFormat); }); it('fails on PSBT cache miss', async function () { diff --git a/modules/abstract-utxo/test/unit/transaction.ts b/modules/abstract-utxo/test/unit/transaction.ts index 291cd6e6e5..c23794db27 100644 --- a/modules/abstract-utxo/test/unit/transaction.ts +++ b/modules/abstract-utxo/test/unit/transaction.ts @@ -5,7 +5,7 @@ import * as _ from 'lodash'; import * as utxolib from '@bitgo/utxo-lib'; import nock = require('nock'); import { BIP32Interface, bitgo, testutil } from '@bitgo/utxo-lib'; -import { address as wasmAddress } from '@bitgo/wasm-utxo'; +import { address as wasmAddress, fixedScriptWallet } from '@bitgo/wasm-utxo'; import { common, FullySignedTransaction, @@ -108,6 +108,7 @@ function run( prv: signer.toBase58(), pubs: walletKeys.triple.map((k) => k.neutered().toBase58()), cosignerPub: cosigner.neutered().toBase58(), + extractTransaction: false, } as WalletSignTransactionOptions; } @@ -223,7 +224,7 @@ function run( function toTransactionStagesObj(stages: TransactionStages): TransactionObjStages { return _.mapValues(stages, (v) => - v === undefined || v instanceof utxolib.bitgo.UtxoPsbt + v === undefined || v instanceof utxolib.bitgo.UtxoPsbt || v instanceof fixedScriptWallet.BitGoPsbt ? undefined : v instanceof utxolib.bitgo.UtxoTransaction ? transactionToObj(v) @@ -266,14 +267,11 @@ function run( signedBy: BIP32Interface[], sign: 'halfsigned' | 'fullsigned' ) { - if (txFormat === 'psbt' && sign === 'halfsigned') { + if (txFormat === 'psbt') { testPsbtValidSignatures(tx, signedBy); return; } - const unspents = - txFormat === 'psbt' - ? getUnspentsForPsbt().map((u) => ({ ...u, value: bitgo.toTNumber(u.value, amountType) as TNumber })) - : getUnspents(); + const unspents = getUnspents(); const prevOutputs = unspents.map( (u): utxolib.TxOutput => ({ script: Buffer.from(wasmAddress.toOutputScriptWithCoin(u.address, coin.name)), @@ -398,6 +396,8 @@ function run( const txHex = stageTx instanceof utxolib.bitgo.UtxoPsbt || stageTx instanceof utxolib.bitgo.UtxoTransaction ? stageTx.toBuffer().toString('hex') + : stageTx instanceof fixedScriptWallet.BitGoPsbt + ? Buffer.from(stageTx.serialize()).toString('hex') : stageTx.txHex; const pubs = walletKeys.triple.map((k) => k.neutered().toBase58()) as Triple; @@ -415,9 +415,7 @@ function run( } function runTestForCoin(coin: AbstractUtxoCoin) { - (['legacy', 'psbt'] as const).forEach((txFormat) => { - run(coin, getScriptTypes(coin, txFormat), txFormat, { decodeWith: 'wasm-utxo' }); - }); + run(coin, getScriptTypes(coin, 'psbt'), 'psbt', { decodeWith: 'wasm-utxo' }); } describe('Transaction Suite', function () {