From 93a3af8f0bdcd8043a1d855387bc931a71857cf7 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Fri, 14 Nov 2025 13:52:02 +0100 Subject: [PATCH 1/6] feat(abstract-utxo): update transaction parsing comments Update the comments about the pay-as-you-go fee logic in the transaction parsing code to indicate that it has become obsolete with the introduction of `utxocore.paygo.verifyPayGoAddressProof()`. Issue: BTC-2732 Co-authored-by: llm-git --- .../src/transaction/fixedScript/parseTransaction.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index 28879a88f2..9b1504f3a7 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -189,11 +189,13 @@ export async function parseTransaction( /** * The calculation of the implicit external spend amount pertains to verifying the pay-as-you-go-fee BitGo - * automatically applies to transactions sending money out of the wallet. The logic is fairly straightforward + * automatically applied to transactions sending money out of the wallet. The logic is fairly straightforward * in that we compare the external spend amount that was specified explicitly by the user to the portion * that was specified implicitly. To protect customers from people tampering with the transaction outputs, we * define a threshold for the maximum percentage of the implicit external spend in relation to the explicit * external spend. + * + * This has become obsolete with the intoduction of `utxocore.paygo.verifyPayGoAddressProof()`. */ // make sure that all the extra addresses are change addresses From 709a56552cc3c3704c49ec581a8cdeae883e3e88 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Fri, 14 Nov 2025 13:52:11 +0100 Subject: [PATCH 2/6] test(abstract-utxo): add PSBT parsing tests Add unit tests for parsePsbt functionality, covering transaction parsing with various recipient configurations. Tests verify proper output classification and spend amount calculations. Issue: BTC-2732 Co-authored-by: llm-git --- .../unit/transaction/fixedScript/parsePsbt.ts | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts diff --git a/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts b/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts new file mode 100644 index 0000000000..c51ce41e1a --- /dev/null +++ b/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts @@ -0,0 +1,160 @@ +import assert from 'node:assert/strict'; + +import * as sinon from 'sinon'; +import * as utxolib from '@bitgo/utxo-lib'; +import { Wallet, VerificationOptions, ITransactionRecipient } from '@bitgo/sdk-core'; + +import { parseTransaction } from '../../../../src/transaction/fixedScript/parseTransaction'; +import { ParsedTransaction } from '../../../../src/abstractUtxoCoin'; +import { UtxoWallet } from '../../../../src/wallet'; +import { getUtxoCoin } from '../../util'; +import { explainPsbt } from '../../../../src/transaction/fixedScript'; +import type { TransactionExplanation } from '../../../../src/transaction/fixedScript/explainTransaction'; +import { getChainFromNetwork } from '../../../../src/names'; + +function getTxParamsFromExplanation(explanation: TransactionExplanation): { + recipients: ITransactionRecipient[]; + changeAddress?: string; +} { + // The external outputs are the ones that are in outputs but not in changeOutputs + const changeAddresses = new Set(explanation.changeOutputs.map((o) => o.address)); + const externalOutputs = explanation.outputs.filter((o) => o.address && !changeAddresses.has(o.address)); + + return { + recipients: externalOutputs.map((output) => ({ + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + address: output.address!, + amount: output.amount, + })), + changeAddress: undefined, + }; +} + +function describeParseTransactionWith( + acidTest: utxolib.testutil.AcidTest, + explanation: TransactionExplanation, + { + label = 'default', + txParams, + expectedExplicitExternalSpendAmount, + expectedImplicitExternalSpendAmount, + }: { + label?: string; + txParams: { + recipients: ITransactionRecipient[]; + changeAddress?: string; + }; + expectedExplicitExternalSpendAmount: bigint; + expectedImplicitExternalSpendAmount: bigint; + } +) { + describe(`${acidTest.name}/${label}`, function () { + let refParsedTransaction: ParsedTransaction; + let coin: ReturnType; + let mockWallet: sinon.SinonStubbedInstance; + let stubExplainTransaction: sinon.SinonStub; + + before('prepare', async function () { + const coinName = getChainFromNetwork(acidTest.network); + coin = getUtxoCoin(coinName); + + // Create mock wallet + mockWallet = sinon.createStubInstance(Wallet); + mockWallet.id.returns('test-wallet-id'); + mockWallet.coin.returns(coin.getChain()); + mockWallet.coinSpecific.returns(undefined); + + // Mock verification options with keychains to disable networking + // Use the same keychains that were used to create the PSBT + const pubs = acidTest.rootWalletKeys.triple.map((k) => k.neutered().toBase58()); + const verification: VerificationOptions = { + disableNetworking: true, + keychains: { + user: { id: '0', pub: pubs[0], type: 'independent' }, + backup: { id: '1', pub: pubs[1], type: 'independent' }, + bitgo: { id: '2', pub: pubs[2], type: 'independent' }, + }, + }; + + // Stub explainTransaction to return the explanation without making network calls + stubExplainTransaction = sinon.stub(coin, 'explainTransaction').resolves(explanation); + + refParsedTransaction = await parseTransaction(coin, { + wallet: mockWallet as unknown as UtxoWallet, + txParams, + txPrebuild: { + txHex: acidTest.createPsbt().toHex(), + }, + verification, + }); + }); + + after('cleanup', function () { + if (stubExplainTransaction) { + stubExplainTransaction.restore(); + } + }); + + it('should parse transaction without network calls', function () { + assert.ok(refParsedTransaction); + assert.ok(refParsedTransaction.keychains); + assert.ok(refParsedTransaction.outputs); + }); + + it('should have valid keychains', function () { + assert.ok(refParsedTransaction.keychains.user); + assert.ok(refParsedTransaction.keychains.backup); + assert.ok(refParsedTransaction.keychains.bitgo); + const pubs = acidTest.rootWalletKeys.triple.map((k) => k.neutered().toBase58()); + assert.strictEqual(refParsedTransaction.keychains.user.pub, pubs[0]); + assert.strictEqual(refParsedTransaction.keychains.backup.pub, pubs[1]); + assert.strictEqual(refParsedTransaction.keychains.bitgo.pub, pubs[2]); + }); + + it('should have outputs classified as internal or external', function () { + // Since we didn't specify any recipients, outputs will be classified based on whether they can be + // verified as wallet addresses. Some may be external if address verification fails without a proper wallet setup. + const totalOutputs = refParsedTransaction.outputs.length; + const changeOutputs = refParsedTransaction.changeOutputs.length; + const externalOutputs = refParsedTransaction.outputs.filter((o) => o.external === true).length; + assert.strictEqual(externalOutputs, 3); + + assert.ok(totalOutputs > 0, 'should have at least one output'); + assert.strictEqual(changeOutputs + externalOutputs, totalOutputs, 'all outputs should be classified'); + }); + + it('should have expected explicit and implicit external spend amounts', function () { + assert.strictEqual(BigInt(refParsedTransaction.explicitExternalSpendAmount), expectedExplicitExternalSpendAmount); + assert.strictEqual(BigInt(refParsedTransaction.implicitExternalSpendAmount), expectedImplicitExternalSpendAmount); + }); + }); +} + +describe('parsePsbt', function () { + utxolib.testutil.AcidTest.suite().forEach((test) => { + const psbt = test.createPsbt(); + const explanation: TransactionExplanation = explainPsbt(psbt, { pubs: test.rootWalletKeys }, test.network, { + strict: true, + }); + + // Default case: infer recipients from explanation + describeParseTransactionWith(test, explanation, { + txParams: getTxParamsFromExplanation(explanation), + expectedExplicitExternalSpendAmount: 2700n, + expectedImplicitExternalSpendAmount: 0n, + }); + + if (test.network === utxolib.networks.bitcoin) { + // extended test suite for bitcoin + describeParseTransactionWith(test, explanation, { + label: 'empty recipients', + txParams: { + recipients: [], + changeAddress: undefined, + }, + expectedExplicitExternalSpendAmount: 0n, + expectedImplicitExternalSpendAmount: 2700n, + }); + } + }); +}); From ba2343d1ab7c2e37656f8f5462105aec970f0a84 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Fri, 14 Nov 2025 13:47:14 +0100 Subject: [PATCH 3/6] feat(abstract-utxo): handle script recipients in BCH canonicalAddress Add isScriptRecipient check to BCH's canonicalAddress function to properly handle script recipients. Remove redundant implementation from BCHA as it now inherits the corrected functionality from BCH. Issue: BTC-2732 Co-authored-by: llm-git --- modules/abstract-utxo/src/impl/bch/bch.ts | 5 +++++ modules/abstract-utxo/src/impl/bcha/bcha.ts | 13 ------------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/modules/abstract-utxo/src/impl/bch/bch.ts b/modules/abstract-utxo/src/impl/bch/bch.ts index dda7c96f3f..f46faf875c 100644 --- a/modules/abstract-utxo/src/impl/bch/bch.ts +++ b/modules/abstract-utxo/src/impl/bch/bch.ts @@ -2,6 +2,7 @@ import { BitGoBase } from '@bitgo/sdk-core'; import * as utxolib from '@bitgo/utxo-lib'; import { AbstractUtxoCoin, UtxoNetwork } from '../../abstractUtxoCoin'; +import { isScriptRecipient } from '../../transaction'; export class Bch extends AbstractUtxoCoin { protected constructor(bitgo: BitGoBase, network?: UtxoNetwork) { @@ -25,6 +26,10 @@ export class Bch extends AbstractUtxoCoin { * @returns {*} address string */ canonicalAddress(address: string, version: unknown = 'base58'): string { + if (isScriptRecipient(address)) { + return address; + } + if (version === 'base58') { return utxolib.addressFormat.toCanonicalFormat(address, this.network); } diff --git a/modules/abstract-utxo/src/impl/bcha/bcha.ts b/modules/abstract-utxo/src/impl/bcha/bcha.ts index 7b8203f800..98defc0798 100644 --- a/modules/abstract-utxo/src/impl/bcha/bcha.ts +++ b/modules/abstract-utxo/src/impl/bcha/bcha.ts @@ -12,17 +12,4 @@ export class Bcha extends Bch { static createInstance(bitgo: BitGoBase): Bcha { return new Bcha(bitgo); } - - canonicalAddress(address: string, version: unknown = 'base58'): string { - if (version === 'base58') { - return utxolib.addressFormat.toCanonicalFormat(address, this.network); - } - - if (version === 'cashaddr') { - const script = utxolib.addressFormat.toOutputScriptTryFormats(address, this.network); - return utxolib.addressFormat.fromOutputScriptWithFormat(script, version, this.network); - } - - throw new Error(`invalid version ${version}`); - } } From bb66fb9b1b4288bfa87cc3369c39341ac9ed9047 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Fri, 14 Nov 2025 14:26:38 +0100 Subject: [PATCH 4/6] feat(abstract-utxo): move transaction types to separate file Refactor transaction-related types out of abstractUtxoCoin.ts into a new transaction/types.ts file. This helps organize code better and reduces coupling between modules. Move BaseOutput, FixedScriptWalletOutput, Output, BaseParsedTransactionOutputs, BaseParsedTransaction, and ParsedTransaction to the new file. Also moved isWalletOutput helper to its relevant module. Issue: BTC-2732 Co-authored-by: llm-git --- modules/abstract-utxo/src/abstractUtxoCoin.ts | 62 +------------------ modules/abstract-utxo/src/impl/doge/doge.ts | 2 +- .../src/transaction/descriptor/parse.ts | 9 +-- .../descriptor/parseToAmountType.ts | 3 +- .../descriptor/verifyTransaction.ts | 8 +-- .../fixedScript/explainPsbtWasm.ts | 2 +- .../fixedScript/explainTransaction.ts | 3 +- .../transaction/fixedScript/parseOutput.ts | 9 ++- .../fixedScript/parseTransaction.ts | 9 +-- .../fixedScript/verifyTransaction.ts | 3 +- .../abstract-utxo/src/transaction/index.ts | 1 + .../src/transaction/parseTransaction.ts | 3 +- .../abstract-utxo/src/transaction/types.ts | 55 ++++++++++++++++ modules/abstract-utxo/src/verifyKey.ts | 3 +- .../test/unit/parseTransaction.ts | 3 +- .../test/unit/transaction/descriptor/parse.ts | 2 +- .../unit/transaction/fixedScript/parsePsbt.ts | 2 +- 17 files changed, 88 insertions(+), 91 deletions(-) create mode 100644 modules/abstract-utxo/src/transaction/types.ts diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 058079f62d..404d53c502 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -74,8 +74,8 @@ import { import { assertDescriptorWalletAddress, getDescriptorMapFromWallet, isDescriptorWallet } from './descriptor'; import { getChainFromNetwork, getFamilyFromNetwork, getFullNameFromNetwork } from './names'; import { assertFixedScriptWalletAddress } from './address/fixedScript'; -import { CustomChangeOptions } from './transaction/fixedScript'; -import { toBip32Triple, UtxoKeychain, UtxoNamedKeychains } from './keychains'; +import { ParsedTransaction } from './transaction/types'; +import { toBip32Triple, UtxoKeychain } from './keychains'; import { verifyKeySignature, verifyUserPublicKey } from './verifyKey'; import { getPolicyForEnv } from './descriptor/validatePolicy'; import { signTransaction } from './transaction/signTransaction'; @@ -186,33 +186,11 @@ export interface VerifyAddressOptions ex coinSpecific?: TCoinSpecific; } -export interface BaseOutput { - address: string; - amount: TAmount; - // Even though this external flag is redundant with the chain property, it is necessary for backwards compatibility - // with legacy transaction format. - external?: boolean; -} - -export interface FixedScriptWalletOutput extends BaseOutput { - needsCustomChangeKeySignatureVerification?: boolean; - chain: number; - index: number; -} - -export type Output = BaseOutput | FixedScriptWalletOutput; - export type Bip322Message = { address: string; message: string; }; -export function isWalletOutput(output: Output): output is FixedScriptWalletOutput { - return ( - (output as FixedScriptWalletOutput).chain !== undefined && (output as FixedScriptWalletOutput).index !== undefined - ); -} - export interface TransactionInfo { /** Maps txid to txhex. Required for offline signing. */ txHexes?: Record; @@ -255,42 +233,6 @@ export interface ParseTransactionOptions = { - /** all transaction outputs */ - outputs: TOutput[]; - /** transaction outputs that were specified as recipients but are missing from the transaction */ - missingOutputs: TOutput[]; - /** transaction outputs that were specified as recipients and are present in the transaction */ - explicitExternalOutputs: TOutput[]; - /** transaction outputs that were not specified as recipients but are present in the transaction */ - implicitExternalOutputs: TOutput[]; - /** transaction outputs that are change outputs */ - changeOutputs: TOutput[]; - /** sum of all explicit external outputs */ - explicitExternalSpendAmount: TNumber; - /** sum of all implicit external outputs */ - implicitExternalSpendAmount: TNumber; -}; - -export type BaseParsedTransaction = BaseParsedTransactionOutputs< - TNumber, - TOutput -> /** Some extra properties that have nothing to do with an individual transaction */ & { - keychains: UtxoNamedKeychains; - keySignatures: { - backupPub?: string; - bitgoPub?: string; - }; - needsCustomChangeKeySignatureVerification: boolean; - customChange?: CustomChangeOptions; -}; - -/** - * This type is a bit silly because it allows the type for the aggregate amounts to be different from the type of - * individual amounts. - */ -export type ParsedTransaction = BaseParsedTransaction; - export interface GenerateAddressOptions { addressType?: ScriptType2Of3; threshold?: number; diff --git a/modules/abstract-utxo/src/impl/doge/doge.ts b/modules/abstract-utxo/src/impl/doge/doge.ts index 937e08cbf3..26eaae5f4f 100644 --- a/modules/abstract-utxo/src/impl/doge/doge.ts +++ b/modules/abstract-utxo/src/impl/doge/doge.ts @@ -7,12 +7,12 @@ import { SignTransactionOptions, ExplainTransactionOptions, ParseTransactionOptions, - ParsedTransaction, VerifyTransactionOptions, RecoverFromWrongChainOptions, TransactionInfo, TransactionPrebuild, } from '../../abstractUtxoCoin'; +import { ParsedTransaction } from '../../transaction/types'; import type { TransactionExplanation } from '../../transaction/fixedScript/explainTransaction'; import type { CrossChainRecoverySigned, CrossChainRecoveryUnsigned } from '../../recovery/crossChainRecovery'; diff --git a/modules/abstract-utxo/src/transaction/descriptor/parse.ts b/modules/abstract-utxo/src/transaction/descriptor/parse.ts index 09af79b31f..3d3afdfbac 100644 --- a/modules/abstract-utxo/src/transaction/descriptor/parse.ts +++ b/modules/abstract-utxo/src/transaction/descriptor/parse.ts @@ -2,13 +2,8 @@ import * as utxolib from '@bitgo/utxo-lib'; import { ITransactionRecipient } from '@bitgo/sdk-core'; import * as coreDescriptors from '@bitgo/utxo-core/descriptor'; -import { - AbstractUtxoCoin, - BaseOutput, - BaseParsedTransaction, - BaseParsedTransactionOutputs, - ParseTransactionOptions, -} from '../../abstractUtxoCoin'; +import { AbstractUtxoCoin, ParseTransactionOptions } from '../../abstractUtxoCoin'; +import { BaseOutput, BaseParsedTransaction, BaseParsedTransactionOutputs } from '../types'; import { getKeySignatures, toBip32Triple, UtxoNamedKeychains } from '../../keychains'; import { getDescriptorMapFromWallet, getPolicyForEnv } from '../../descriptor'; import { IDescriptorWallet } from '../../descriptor/descriptorWallet'; diff --git a/modules/abstract-utxo/src/transaction/descriptor/parseToAmountType.ts b/modules/abstract-utxo/src/transaction/descriptor/parseToAmountType.ts index d8dc77ffd7..9ddac22ffb 100644 --- a/modules/abstract-utxo/src/transaction/descriptor/parseToAmountType.ts +++ b/modules/abstract-utxo/src/transaction/descriptor/parseToAmountType.ts @@ -1,4 +1,5 @@ -import { AbstractUtxoCoin, BaseOutput, BaseParsedTransaction, ParseTransactionOptions } from '../../abstractUtxoCoin'; +import { AbstractUtxoCoin, ParseTransactionOptions } from '../../abstractUtxoCoin'; +import { BaseOutput, BaseParsedTransaction } from '../types'; import { IDescriptorWallet } from '../../descriptor/descriptorWallet'; import { parse, ParsedDescriptorTransaction } from './parse'; diff --git a/modules/abstract-utxo/src/transaction/descriptor/verifyTransaction.ts b/modules/abstract-utxo/src/transaction/descriptor/verifyTransaction.ts index 939accb62d..97d4fcb136 100644 --- a/modules/abstract-utxo/src/transaction/descriptor/verifyTransaction.ts +++ b/modules/abstract-utxo/src/transaction/descriptor/verifyTransaction.ts @@ -2,12 +2,8 @@ import * as utxolib from '@bitgo/utxo-lib'; import { ITransactionRecipient, TxIntentMismatchError } from '@bitgo/sdk-core'; import { DescriptorMap } from '@bitgo/utxo-core/descriptor'; -import { - AbstractUtxoCoin, - BaseOutput, - BaseParsedTransactionOutputs, - VerifyTransactionOptions, -} from '../../abstractUtxoCoin'; +import { AbstractUtxoCoin, VerifyTransactionOptions } from '../../abstractUtxoCoin'; +import { BaseOutput, BaseParsedTransactionOutputs } from '../types'; import { toBaseParsedTransactionOutputsFromPsbt } from './parse'; diff --git a/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts b/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts index e941b78366..7789c5b519 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/explainPsbtWasm.ts @@ -1,7 +1,7 @@ import { fixedScriptWallet } from '@bitgo/wasm-utxo'; import { Triple } from '@bitgo/sdk-core'; -import type { Output, FixedScriptWalletOutput } from '../../abstractUtxoCoin'; +import type { FixedScriptWalletOutput, Output } from '../types'; import type { TransactionExplanationWasm } from './explainTransaction'; diff --git a/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts index 03ac278ae9..9d11fd14e8 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts @@ -5,7 +5,8 @@ import { bitgo } from '@bitgo/utxo-lib'; import { ITransactionExplanation as BaseTransactionExplanation, Triple } from '@bitgo/sdk-core'; import * as utxocore from '@bitgo/utxo-core'; -import type { Output, Bip322Message, FixedScriptWalletOutput } from '../../abstractUtxoCoin'; +import type { Bip322Message } from '../../abstractUtxoCoin'; +import type { Output, FixedScriptWalletOutput } from '../types'; import { toExtendedAddressFormat } from '../recipient'; import { getPayGoVerificationPubkey } from '../getPayGoVerificationPubkey'; diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts index b9527b1888..232b24a411 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts @@ -12,10 +12,17 @@ import { Triple, } from '@bitgo/sdk-core'; -import { AbstractUtxoCoin, Output, isWalletOutput } from '../../abstractUtxoCoin'; +import { AbstractUtxoCoin } from '../../abstractUtxoCoin'; +import { Output, FixedScriptWalletOutput } from '../types'; const debug = debugLib('bitgo:v2:parseoutput'); +export function isWalletOutput(output: Output): output is FixedScriptWalletOutput { + return ( + (output as FixedScriptWalletOutput).chain !== undefined && (output as FixedScriptWalletOutput).index !== undefined + ); +} + interface HandleVerifyAddressErrorResponse { external: boolean; needsCustomChangeKeySignatureVerification?: boolean; diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index 9b1504f3a7..7123fb12be 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -4,13 +4,8 @@ import _ from 'lodash'; import { Triple, VerificationOptions, Wallet } from '@bitgo/sdk-core'; import * as utxolib from '@bitgo/utxo-lib'; -import type { - AbstractUtxoCoin, - FixedScriptWalletOutput, - Output, - ParsedTransaction, - ParseTransactionOptions, -} from '../../abstractUtxoCoin'; +import type { AbstractUtxoCoin, ParseTransactionOptions } from '../../abstractUtxoCoin'; +import type { FixedScriptWalletOutput, Output, ParsedTransaction } from '../types'; import { fetchKeychains, getKeySignatures, toKeychainTriple, UtxoKeychain, UtxoNamedKeychains } from '../../keychains'; import { ComparableOutput, outputDifference } from '../outputDifference'; import { fromExtendedAddressFormatToScript, toExtendedAddressFormat } from '../recipient'; diff --git a/modules/abstract-utxo/src/transaction/fixedScript/verifyTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/verifyTransaction.ts index 4e54d51cd9..fdbb294805 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/verifyTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/verifyTransaction.ts @@ -4,7 +4,8 @@ import BigNumber from 'bignumber.js'; import { BitGoBase, TxIntentMismatchError } from '@bitgo/sdk-core'; import * as utxolib from '@bitgo/utxo-lib'; -import { AbstractUtxoCoin, Output, ParsedTransaction, VerifyTransactionOptions } from '../../abstractUtxoCoin'; +import { AbstractUtxoCoin, VerifyTransactionOptions } from '../../abstractUtxoCoin'; +import { Output, ParsedTransaction } from '../types'; import { verifyCustomChangeKeySignatures, verifyKeySignature, verifyUserPublicKey } from '../../verifyKey'; import { getPsbtTxInputs, getTxInputs } from '../fetchInputs'; diff --git a/modules/abstract-utxo/src/transaction/index.ts b/modules/abstract-utxo/src/transaction/index.ts index dd0ae915a9..4bacd7522e 100644 --- a/modules/abstract-utxo/src/transaction/index.ts +++ b/modules/abstract-utxo/src/transaction/index.ts @@ -1,3 +1,4 @@ +export * from './types'; export * from './recipient'; export { explainTx } from './explainTransaction'; export { parseTransaction } from './parseTransaction'; diff --git a/modules/abstract-utxo/src/transaction/parseTransaction.ts b/modules/abstract-utxo/src/transaction/parseTransaction.ts index 4f5274bf1e..d597b42a33 100644 --- a/modules/abstract-utxo/src/transaction/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/parseTransaction.ts @@ -1,6 +1,7 @@ -import { AbstractUtxoCoin, ParsedTransaction, ParseTransactionOptions } from '../abstractUtxoCoin'; +import { AbstractUtxoCoin, ParseTransactionOptions } from '../abstractUtxoCoin'; import { isDescriptorWallet } from '../descriptor'; +import { ParsedTransaction } from './types'; import * as descriptor from './descriptor'; import * as fixedScript from './fixedScript'; diff --git a/modules/abstract-utxo/src/transaction/types.ts b/modules/abstract-utxo/src/transaction/types.ts new file mode 100644 index 0000000000..f3a514594e --- /dev/null +++ b/modules/abstract-utxo/src/transaction/types.ts @@ -0,0 +1,55 @@ +import type { UtxoNamedKeychains } from '../keychains'; + +import type { CustomChangeOptions } from './fixedScript'; + +export interface BaseOutput { + address: string; + amount: TAmount; + // Even though this external flag is redundant with the chain property, it is necessary for backwards compatibility + // with legacy transaction format. + external?: boolean; +} + +export interface FixedScriptWalletOutput extends BaseOutput { + needsCustomChangeKeySignatureVerification?: boolean; + chain: number; + index: number; +} + +export type Output = BaseOutput | FixedScriptWalletOutput; + +export type BaseParsedTransactionOutputs = { + /** all transaction outputs */ + outputs: TOutput[]; + /** transaction outputs that were specified as recipients but are missing from the transaction */ + missingOutputs: TOutput[]; + /** transaction outputs that were specified as recipients and are present in the transaction */ + explicitExternalOutputs: TOutput[]; + /** transaction outputs that were not specified as recipients but are present in the transaction */ + implicitExternalOutputs: TOutput[]; + /** transaction outputs that are change outputs */ + changeOutputs: TOutput[]; + /** sum of all explicit external outputs */ + explicitExternalSpendAmount: TNumber; + /** sum of all implicit external outputs */ + implicitExternalSpendAmount: TNumber; +}; + +export type BaseParsedTransaction = BaseParsedTransactionOutputs< + TNumber, + TOutput +> /** Some extra properties that have nothing to do with an individual transaction */ & { + keychains: UtxoNamedKeychains; + keySignatures: { + backupPub?: string; + bitgoPub?: string; + }; + needsCustomChangeKeySignatureVerification: boolean; + customChange?: CustomChangeOptions; +}; + +/** + * This type is a bit silly because it allows the type for the aggregate amounts to be different from the type of + * individual amounts. + */ +export type ParsedTransaction = BaseParsedTransaction; diff --git a/modules/abstract-utxo/src/verifyKey.ts b/modules/abstract-utxo/src/verifyKey.ts index a7813f4200..204bf4bf56 100644 --- a/modules/abstract-utxo/src/verifyKey.ts +++ b/modules/abstract-utxo/src/verifyKey.ts @@ -11,7 +11,8 @@ import { bip32 } from '@bitgo/secp256k1'; import * as bitcoinMessage from 'bitcoinjs-message'; import { BitGoBase, decryptKeychainPrivateKey, KeyIndices } from '@bitgo/sdk-core'; -import { ParsedTransaction, VerifyKeySignaturesOptions, VerifyUserPublicKeyOptions } from './abstractUtxoCoin'; +import { VerifyKeySignaturesOptions, VerifyUserPublicKeyOptions } from './abstractUtxoCoin'; +import { ParsedTransaction } from './transaction/types'; import { UtxoKeychain } from './keychains'; const debug = buildDebug('bitgo:abstract-utxo:verifyKey'); diff --git a/modules/abstract-utxo/test/unit/parseTransaction.ts b/modules/abstract-utxo/test/unit/parseTransaction.ts index f746c9794b..510efec400 100644 --- a/modules/abstract-utxo/test/unit/parseTransaction.ts +++ b/modules/abstract-utxo/test/unit/parseTransaction.ts @@ -3,7 +3,8 @@ import assert from 'assert'; import * as sinon from 'sinon'; import { Wallet, UnexpectedAddressError, VerificationOptions } from '@bitgo/sdk-core'; -import { UtxoWallet, Output, TransactionParams } from '../../src'; +import { UtxoWallet, TransactionParams } from '../../src'; +import { Output } from '../../src/transaction/types'; import type { TransactionExplanation } from '../../src/transaction/fixedScript/explainTransaction'; import { getUtxoCoin } from './util'; diff --git a/modules/abstract-utxo/test/unit/transaction/descriptor/parse.ts b/modules/abstract-utxo/test/unit/transaction/descriptor/parse.ts index 8a99c40803..ae095a5fee 100644 --- a/modules/abstract-utxo/test/unit/transaction/descriptor/parse.ts +++ b/modules/abstract-utxo/test/unit/transaction/descriptor/parse.ts @@ -22,7 +22,7 @@ import { ErrorMissingOutputs, } from '../../../../src/transaction/descriptor/verifyTransaction'; import { toAmountType } from '../../../../src/transaction/descriptor/parseToAmountType'; -import { BaseOutput } from '../../../../src'; +import { BaseOutput } from '../../../../src/transaction/types'; import { getFixtureRoot } from './fixtures.utils'; diff --git a/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts b/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts index c51ce41e1a..cb25a44d81 100644 --- a/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts +++ b/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts @@ -5,7 +5,7 @@ import * as utxolib from '@bitgo/utxo-lib'; import { Wallet, VerificationOptions, ITransactionRecipient } from '@bitgo/sdk-core'; import { parseTransaction } from '../../../../src/transaction/fixedScript/parseTransaction'; -import { ParsedTransaction } from '../../../../src/abstractUtxoCoin'; +import { ParsedTransaction } from '../../../../src/transaction/types'; import { UtxoWallet } from '../../../../src/wallet'; import { getUtxoCoin } from '../../util'; import { explainPsbt } from '../../../../src/transaction/fixedScript'; From 7c982bf0d9a389c55d884578c302d4109271b075 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Fri, 14 Nov 2025 14:40:19 +0100 Subject: [PATCH 5/6] feat(abstract-utxo): refactor parsePsbt test to improve reusability Simplify test structure by moving explanation creation inside the test helper function, which allows inferring txParams directly from explanation. This avoids passing explanation as a separate parameter and makes tests more self-contained. Issue: BTC-2732 Co-authored-by: llm-git --- .../unit/transaction/fixedScript/parsePsbt.ts | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts b/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts index cb25a44d81..bfc7957f2d 100644 --- a/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts +++ b/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts @@ -32,7 +32,6 @@ function getTxParamsFromExplanation(explanation: TransactionExplanation): { function describeParseTransactionWith( acidTest: utxolib.testutil.AcidTest, - explanation: TransactionExplanation, { label = 'default', txParams, @@ -40,10 +39,12 @@ function describeParseTransactionWith( expectedImplicitExternalSpendAmount, }: { label?: string; - txParams: { - recipients: ITransactionRecipient[]; - changeAddress?: string; - }; + txParams: + | { + recipients: ITransactionRecipient[]; + changeAddress?: string; + } + | 'inferFromExplanation'; expectedExplicitExternalSpendAmount: bigint; expectedImplicitExternalSpendAmount: bigint; } @@ -58,6 +59,18 @@ function describeParseTransactionWith( const coinName = getChainFromNetwork(acidTest.network); coin = getUtxoCoin(coinName); + // Create PSBT and explanation + const psbt = acidTest.createPsbt(); + const explanation = explainPsbt(psbt, { pubs: acidTest.rootWalletKeys }, acidTest.network, { + strict: true, + }); + + // Determine txParams + const resolvedTxParams = + txParams === 'inferFromExplanation' || txParams === undefined + ? getTxParamsFromExplanation(explanation) + : txParams; + // Create mock wallet mockWallet = sinon.createStubInstance(Wallet); mockWallet.id.returns('test-wallet-id'); @@ -81,9 +94,9 @@ function describeParseTransactionWith( refParsedTransaction = await parseTransaction(coin, { wallet: mockWallet as unknown as UtxoWallet, - txParams, + txParams: resolvedTxParams, txPrebuild: { - txHex: acidTest.createPsbt().toHex(), + txHex: psbt.toHex(), }, verification, }); @@ -132,21 +145,16 @@ function describeParseTransactionWith( describe('parsePsbt', function () { utxolib.testutil.AcidTest.suite().forEach((test) => { - const psbt = test.createPsbt(); - const explanation: TransactionExplanation = explainPsbt(psbt, { pubs: test.rootWalletKeys }, test.network, { - strict: true, - }); - // Default case: infer recipients from explanation - describeParseTransactionWith(test, explanation, { - txParams: getTxParamsFromExplanation(explanation), + describeParseTransactionWith(test, { + txParams: 'inferFromExplanation', expectedExplicitExternalSpendAmount: 2700n, expectedImplicitExternalSpendAmount: 0n, }); if (test.network === utxolib.networks.bitcoin) { // extended test suite for bitcoin - describeParseTransactionWith(test, explanation, { + describeParseTransactionWith(test, { label: 'empty recipients', txParams: { recipients: [], From 2d36d4e566691fcc32a5c5797608ef9064c4b6c3 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Fri, 14 Nov 2025 14:53:35 +0100 Subject: [PATCH 6/6] feat(abstract-utxo): add support for parsing legacy transactions Implement parsing support for legacy transactions in addition to PSBT format. Add helper function to extract change information from PSBTs for use with legacy transaction parsing. Issue: BTC-2732 Co-authored-by: llm-git --- .../unit/transaction/fixedScript/parsePsbt.ts | 89 +++++++++++++++---- 1 file changed, 74 insertions(+), 15 deletions(-) diff --git a/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts b/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts index bfc7957f2d..4d736c92c5 100644 --- a/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts +++ b/modules/abstract-utxo/test/unit/transaction/fixedScript/parsePsbt.ts @@ -8,9 +8,13 @@ import { parseTransaction } from '../../../../src/transaction/fixedScript/parseT import { ParsedTransaction } from '../../../../src/transaction/types'; import { UtxoWallet } from '../../../../src/wallet'; import { getUtxoCoin } from '../../util'; -import { explainPsbt } from '../../../../src/transaction/fixedScript'; -import type { TransactionExplanation } from '../../../../src/transaction/fixedScript/explainTransaction'; +import { explainLegacyTx, explainPsbt } from '../../../../src/transaction/fixedScript'; +import type { + TransactionExplanation, + ChangeAddressInfo, +} from '../../../../src/transaction/fixedScript/explainTransaction'; import { getChainFromNetwork } from '../../../../src/names'; +import { TransactionPrebuild } from '../../../../src/abstractUtxoCoin'; function getTxParamsFromExplanation(explanation: TransactionExplanation): { recipients: ITransactionRecipient[]; @@ -30,15 +34,39 @@ function getTxParamsFromExplanation(explanation: TransactionExplanation): { }; } +function getChangeInfoFromPsbt(psbt: utxolib.bitgo.UtxoPsbt): ChangeAddressInfo[] | undefined { + try { + return utxolib.bitgo.findInternalOutputIndices(psbt).map((i) => { + const output = psbt.data.outputs[i]; + const derivations = output.bip32Derivation ?? output.tapBip32Derivation ?? undefined; + if (!derivations || derivations.length !== 3) { + throw new Error('expected 3 derivation paths'); + } + const path = derivations[0].path; + const { chain, index } = utxolib.bitgo.getChainAndIndexFromPath(path); + return { + address: utxolib.address.fromOutputScript(psbt.txOutputs[i].script, psbt.network), + chain, + index, + }; + }); + } catch (e) { + if (e instanceof utxolib.bitgo.ErrorNoMultiSigInputFound) { + return undefined; + } + throw e; + } +} + function describeParseTransactionWith( acidTest: utxolib.testutil.AcidTest, + label: string, { - label = 'default', txParams, expectedExplicitExternalSpendAmount, expectedImplicitExternalSpendAmount, + txFormat = 'psbt', }: { - label?: string; txParams: | { recipients: ITransactionRecipient[]; @@ -47,6 +75,7 @@ function describeParseTransactionWith( | 'inferFromExplanation'; expectedExplicitExternalSpendAmount: bigint; expectedImplicitExternalSpendAmount: bigint; + txFormat?: 'psbt' | 'legacy'; } ) { describe(`${acidTest.name}/${label}`, function () { @@ -61,9 +90,21 @@ function describeParseTransactionWith( // Create PSBT and explanation const psbt = acidTest.createPsbt(); - const explanation = explainPsbt(psbt, { pubs: acidTest.rootWalletKeys }, acidTest.network, { - strict: true, - }); + + let explanation: TransactionExplanation; + if (txFormat === 'psbt') { + explanation = explainPsbt(psbt, { pubs: acidTest.rootWalletKeys }, acidTest.network, { + strict: true, + }); + } else if (txFormat === 'legacy') { + const tx = psbt.getUnsignedTx(); + const pubs = acidTest.rootWalletKeys.triple.map((k) => k.neutered().toBase58()); + // Extract change info from PSBT to pass to explainLegacyTx + const changeInfo = getChangeInfoFromPsbt(psbt); + explanation = explainLegacyTx(tx, { pubs, changeInfo }, acidTest.network); + } else { + throw new Error(`Invalid txFormat: ${txFormat}`); + } // Determine txParams const resolvedTxParams = @@ -92,12 +133,23 @@ function describeParseTransactionWith( // Stub explainTransaction to return the explanation without making network calls stubExplainTransaction = sinon.stub(coin, 'explainTransaction').resolves(explanation); + let txPrebuild: TransactionPrebuild; + if (txFormat === 'psbt') { + txPrebuild = { + txHex: psbt.toHex(), + }; + } else if (txFormat === 'legacy') { + txPrebuild = { + txHex: psbt.getUnsignedTx().toHex(), + }; + } else { + throw new Error(`Invalid txFormat: ${txFormat}`); + } + refParsedTransaction = await parseTransaction(coin, { wallet: mockWallet as unknown as UtxoWallet, txParams: resolvedTxParams, - txPrebuild: { - txHex: psbt.toHex(), - }, + txPrebuild, verification, }); }); @@ -143,10 +195,10 @@ function describeParseTransactionWith( }); } -describe('parsePsbt', function () { +describe('parseTransaction', function () { utxolib.testutil.AcidTest.suite().forEach((test) => { - // Default case: infer recipients from explanation - describeParseTransactionWith(test, { + // Default case: psbt format, infer recipients from explanation + describeParseTransactionWith(test, 'default', { txParams: 'inferFromExplanation', expectedExplicitExternalSpendAmount: 2700n, expectedImplicitExternalSpendAmount: 0n, @@ -154,8 +206,15 @@ describe('parsePsbt', function () { if (test.network === utxolib.networks.bitcoin) { // extended test suite for bitcoin - describeParseTransactionWith(test, { - label: 'empty recipients', + + describeParseTransactionWith(test, 'legacy', { + txFormat: 'legacy', + txParams: 'inferFromExplanation', + expectedExplicitExternalSpendAmount: 2700n, + expectedImplicitExternalSpendAmount: 0n, + }); + + describeParseTransactionWith(test, 'empty recipients', { txParams: { recipients: [], changeAddress: undefined,