From dd294ad0045561cca47aaaf604af3807b1961341 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Tue, 27 Jan 2026 16:47:32 +0100 Subject: [PATCH 1/6] fix(abstract-utxo): move outputDifference tests to correct directory Move outputDifference test file from transaction/descriptor/ to transaction/ directory to match the source file location structure. Issue: BTC-2962 Co-authored-by: llm-git --- .../test/unit/transaction/{descriptor => }/outputDifference.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename modules/abstract-utxo/test/unit/transaction/{descriptor => }/outputDifference.ts (98%) diff --git a/modules/abstract-utxo/test/unit/transaction/descriptor/outputDifference.ts b/modules/abstract-utxo/test/unit/transaction/outputDifference.ts similarity index 98% rename from modules/abstract-utxo/test/unit/transaction/descriptor/outputDifference.ts rename to modules/abstract-utxo/test/unit/transaction/outputDifference.ts index 6eb9ed87ae..a57bce6eea 100644 --- a/modules/abstract-utxo/test/unit/transaction/descriptor/outputDifference.ts +++ b/modules/abstract-utxo/test/unit/transaction/outputDifference.ts @@ -6,7 +6,7 @@ import { matchingOutput, outputDifference, outputDifferencesWithExpected, -} from '../../../../src/transaction/outputDifference'; +} from '../../../src/transaction/outputDifference'; describe('outputDifference', function () { function output(script: string, value: bigint | number): ActualOutput; From 836cbda23f977ada67dacc1671e77d0b12745a61 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Tue, 27 Jan 2026 15:10:11 +0100 Subject: [PATCH 2/6] fix(abstract-utxo): add override keyword to preprocessBuildParams method The `preprocessBuildParams` method was missing the `override` keyword, which is now added to correctly indicate that this method overrides a method from the parent class. Issue: BTC-2962 Co-authored-by: llm-git --- modules/abstract-utxo/src/abstractUtxoCoin.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 375590ba67..c3773e6f14 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -479,7 +479,11 @@ export abstract class AbstractUtxoCoin } } - preprocessBuildParams(params: Record): Record { + /** + * This is called before crafting the HTTP request to the BitGo API. + * It converts the recipient address `scriptPubKey:...` to { script: string } | { address: string }. + */ + override preprocessBuildParams(params: Record): Record { if (params.recipients !== undefined) { params.recipients = params.recipients instanceof Array From 3a2037f1be249e13f5b9baeedb409d15b1709e55 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Tue, 27 Jan 2026 15:10:31 +0100 Subject: [PATCH 3/6] fix(abstract-utxo): move toCanonicalTransactionRecipient to parseTransaction Move toCanonicalTransactionRecipient out of the AbstractUtxoCoin class into the parseTransaction module where it's used, removing it from the public API. Issue: BTC-2962 Co-authored-by: llm-git --- modules/abstract-utxo/src/abstractUtxoCoin.ts | 13 ---------- .../fixedScript/parseTransaction.ts | 25 +++++++++++++++++-- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index c3773e6f14..213bf0a123 100644 --- a/modules/abstract-utxo/src/abstractUtxoCoin.ts +++ b/modules/abstract-utxo/src/abstractUtxoCoin.ts @@ -592,19 +592,6 @@ export abstract class AbstractUtxoCoin return this.decodeTransaction(string, decodeWith); } - toCanonicalTransactionRecipient(output: { valueString: string; address?: string }): { - amount: bigint; - address: string; - } { - const amount = BigInt(output.valueString); - assertValidTransactionRecipient({ amount, address: output.address }); - assert(output.address, 'address is required'); - if (isScriptRecipient(output.address)) { - return { amount, address: output.address }; - } - return { amount, address: this.canonicalAddress(output.address) }; - } - /** * Extract and fill transaction details such as internal/change spend, external spend (explicit vs. implicit), etc. * @param params diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index caea6e5fe0..68a9cd8f92 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -8,7 +8,12 @@ import type { AbstractUtxoCoin, ParseTransactionOptions } from '../../abstractUt 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'; +import { + assertValidTransactionRecipient, + fromExtendedAddressFormatToScript, + isScriptRecipient, + toExtendedAddressFormat, +} from '../recipient'; import type { TransactionExplanation } from './explainTransaction'; import { CustomChangeOptions, parseOutput } from './parseOutput'; @@ -17,6 +22,22 @@ export type ComparableOutputWithExternal = ComparableOutput & { external: boolean | undefined; }; +function toCanonicalTransactionRecipient( + coin: AbstractUtxoCoin, + output: { valueString: string; address?: string } +): { + amount: bigint; + address: string; +} { + const amount = BigInt(output.valueString); + assertValidTransactionRecipient({ amount, address: output.address }); + assert(output.address, 'address is required'); + if (isScriptRecipient(output.address)) { + return { amount, address: output.address }; + } + return { amount, address: coin.canonicalAddress(output.address) }; +} + async function parseRbfTransaction( coin: AbstractUtxoCoin, params: ParseTransactionOptions @@ -33,7 +54,7 @@ async function parseRbfTransaction( if (output.wallet === wallet.id()) { return []; } - return [coin.toCanonicalTransactionRecipient(output)]; + return [toCanonicalTransactionRecipient(coin, output)]; } ); From 27c4e89a65f94a5cb75b1b595aabc53f063009d0 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Tue, 27 Jan 2026 14:48:12 +0100 Subject: [PATCH 4/6] fix(abstract-utxo): normalize address comparison in parseOutput When comparing addresses, normalize them to a standard script format to ensure accurate matching regardless of address representation. We can now pass `txParams.txRecipients` to `parseOutputs`. It previously passed `expectedOutputs`, which did multiple things at once: - Normalize the address format for comparison (which is what we wanted) - Append the changeAddress in certain cases (which is not desired in parseOutput) This allows us to change the `toExpectedOutputs` function. Issue: BTC-2962 Co-authored-by: llm-git --- .../abstract-utxo/src/transaction/fixedScript/parseOutput.ts | 5 ++++- .../src/transaction/fixedScript/parseTransaction.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts index aabcdf691d..a3b0186d88 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts @@ -14,6 +14,7 @@ import { import { AbstractUtxoCoin } from '../../abstractUtxoCoin'; import { Output, FixedScriptWalletOutput } from '../types'; +import { fromExtendedAddressFormatToScript } from '../recipient'; const debug = debugLib('bitgo:v2:parseoutput'); @@ -284,7 +285,9 @@ export async function parseOutput({ */ if (txParams.recipients !== undefined && txParams.recipients.length > RECIPIENT_THRESHOLD) { const isCurrentAddressInRecipients = txParams.recipients.some((recipient) => - recipient.address.includes(currentAddress) + fromExtendedAddressFormatToScript(recipient.address, coin.name).equals( + fromExtendedAddressFormatToScript(currentAddress, coin.name) + ) ); if (isCurrentAddressInRecipients) { diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index 68a9cd8f92..82aac4eeff 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -183,7 +183,7 @@ export async function parseTransaction( keychainArray: toKeychainTriple(keychains), wallet, txParams: { - recipients: expectedOutputs, + recipients: txParams.recipients ?? [], changeAddress: txParams.changeAddress, }, customChange, From 710e60bc5c075af39d885ba6c852970c6765020b Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Tue, 27 Jan 2026 16:46:17 +0100 Subject: [PATCH 5/6] feat(abstract-utxo): allow optional expected outputs Add optional flag to ExpectedOutput type for outputs that can be excluded. Update output difference logic to filter non-optional outputs when determining missing outputs. Issue: BTC-2962 Co-authored-by: llm-git --- .../src/transaction/outputDifference.ts | 14 ++++- .../test/unit/transaction/outputDifference.ts | 56 ++++++++++++++++++- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/modules/abstract-utxo/src/transaction/outputDifference.ts b/modules/abstract-utxo/src/transaction/outputDifference.ts index 0e6af1c156..8460a0c983 100644 --- a/modules/abstract-utxo/src/transaction/outputDifference.ts +++ b/modules/abstract-utxo/src/transaction/outputDifference.ts @@ -7,7 +7,10 @@ export type ComparableOutput = { export type ActualOutput = ComparableOutput; /** Expected outputs can have a fixed value or 'max'. */ -export type ExpectedOutput = ComparableOutput; +export type ExpectedOutput = ComparableOutput & { + /** When true, the output is not required to be present in the transaction. */ + optional?: boolean; +}; /** * @param a @@ -40,6 +43,13 @@ export function outputDifference( + actualOutputs: A[], + expectedOutputs: B[] +): B[] { + return outputDifference(expectedOutputs, actualOutputs).filter((o) => !o.optional); +} + export type OutputDifferenceWithExpected = { /** These are the external outputs that were expected and found in the transaction. */ explicitOutputs: TActual[]; @@ -65,7 +75,7 @@ export function outputDifferencesWithExpected { const implicitOutputs = outputDifference(actualOutputs, expectedOutputs); const explicitOutputs = outputDifference(actualOutputs, implicitOutputs); - const missingOutputs = outputDifference(expectedOutputs, actualOutputs); + const missingOutputs = getMissingOutputs(actualOutputs, expectedOutputs); return { explicitOutputs, implicitOutputs, diff --git a/modules/abstract-utxo/test/unit/transaction/outputDifference.ts b/modules/abstract-utxo/test/unit/transaction/outputDifference.ts index a57bce6eea..a26ef45fa4 100644 --- a/modules/abstract-utxo/test/unit/transaction/outputDifference.ts +++ b/modules/abstract-utxo/test/unit/transaction/outputDifference.ts @@ -3,6 +3,7 @@ import assert from 'assert'; import { ActualOutput, ExpectedOutput, + getMissingOutputs, matchingOutput, outputDifference, outputDifferencesWithExpected, @@ -11,7 +12,7 @@ import { describe('outputDifference', function () { function output(script: string, value: bigint | number): ActualOutput; function output(script: string, value: 'max'): ExpectedOutput; - function output(script: string, value: bigint | number | 'max'): ActualOutput | ExpectedOutput { + function output(script: string, value: bigint | number | 'max', optional?: boolean): ActualOutput | ExpectedOutput { const scriptBuffer = Buffer.from(script, 'hex'); if (scriptBuffer.toString('hex') !== script) { throw new Error('invalid script'); @@ -19,9 +20,14 @@ describe('outputDifference', function () { return { script: Buffer.from(script, 'hex'), value: value === 'max' ? 'max' : BigInt(value), + ...(optional !== undefined ? { optional } : {}), }; } + function expectedOutput(script: string, value: bigint | number | 'max', optional?: boolean): ExpectedOutput { + return output(script, value as 'max', optional) as ExpectedOutput; + } + const a = output('aa', 1); const a2 = output('aa', 2); const aMax = output('aa', 'max'); @@ -64,6 +70,32 @@ describe('outputDifference', function () { }); }); + describe('getMissingOutputs', function () { + it('returns missing non-optional outputs', function () { + const aOptional = expectedOutput('aa', 1, true); + const bOptional = expectedOutput('bb', 1, true); + + // No expected outputs means no missing outputs + assert.deepStrictEqual(getMissingOutputs([a], []), []); + + // Missing required output is returned + assert.deepStrictEqual(getMissingOutputs([], [a]), [a]); + assert.deepStrictEqual(getMissingOutputs([b], [a]), [a]); + + // Missing optional output is filtered out + assert.deepStrictEqual(getMissingOutputs([], [aOptional]), []); + assert.deepStrictEqual(getMissingOutputs([b], [aOptional]), []); + + // Mix of optional and required: only required missing outputs returned + assert.deepStrictEqual(getMissingOutputs([], [a, bOptional]), [a]); + assert.deepStrictEqual(getMissingOutputs([], [aOptional, b]), [b]); + + // Present outputs are not returned regardless of optional flag + assert.deepStrictEqual(getMissingOutputs([a], [a]), []); + assert.deepStrictEqual(getMissingOutputs([a], [aOptional]), []); + }); + }); + describe('outputDifferencesWithExpected', function () { function test( outputs: ActualOutput[], @@ -91,5 +123,27 @@ describe('outputDifference', function () { test([a, a], [a], { missing: [], explicit: [a], implicit: [a] }); test([a, b], [a], { missing: [], explicit: [a], implicit: [b] }); }); + + it('handles optional expected outputs', function () { + const aOptional = expectedOutput('aa', 1, true); + const bOptional = expectedOutput('bb', 1, true); + + // Missing optional output is not reported as missing + test([], [aOptional], { missing: [], explicit: [], implicit: [] }); + test([b], [aOptional], { missing: [], explicit: [], implicit: [b] }); + + // Present optional output is still explicit + test([a], [aOptional], { missing: [], explicit: [a], implicit: [] }); + + // Mix of required and optional outputs + test([], [a, bOptional], { missing: [a], explicit: [], implicit: [] }); + test([a], [a, bOptional], { missing: [], explicit: [a], implicit: [] }); + test([a, b], [a, bOptional], { missing: [], explicit: [a, b], implicit: [] }); + + // Multiple optional outputs + test([], [aOptional, bOptional], { missing: [], explicit: [], implicit: [] }); + test([a], [aOptional, bOptional], { missing: [], explicit: [a], implicit: [] }); + test([a, b], [aOptional, bOptional], { missing: [], explicit: [a, b], implicit: [] }); + }); }); }); From d79d95a2c74cb891bdfa1a4a916240cf0b2e3444 Mon Sep 17 00:00:00 2001 From: Otto Allmendinger Date: Tue, 27 Jan 2026 17:19:16 +0100 Subject: [PATCH 6/6] fix(abstract-utxo): check optional params for allowExternalChangeAddress Fix the way we handle `allowExternalChangeAddress` when verifying transactions with external change addresses. Use the new ExpectedOutput interface to mark the change address as optional, as it's not required to exist in the transaction. Issue: BTC-2962 Co-authored-by: llm-git --- .../fixedScript/parseTransaction.ts | 47 ++++++++++++------- .../src/transaction/recipient.ts | 13 +++++ .../test/unit/transaction/outputDifference.ts | 4 +- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts index 82aac4eeff..6a606cd10b 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -7,18 +7,19 @@ import * as utxolib from '@bitgo/utxo-lib'; 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 { assertValidTransactionRecipient, fromExtendedAddressFormatToScript, isScriptRecipient, toExtendedAddressFormat, + toOutputScript, } from '../recipient'; +import { ComparableOutput, ExpectedOutput, outputDifference } from '../outputDifference'; import type { TransactionExplanation } from './explainTransaction'; import { CustomChangeOptions, parseOutput } from './parseOutput'; -export type ComparableOutputWithExternal = ComparableOutput & { +export type ComparableOutputWithExternal = (ComparableOutput | ExpectedOutput) & { external: boolean | undefined; }; @@ -76,21 +77,37 @@ function toExpectedOutputs( allowExternalChangeAddress?: boolean; changeAddress?: string; } -): Output[] { +): ExpectedOutput[] { // verify that each recipient from txParams has their own output - const expectedOutputs = (txParams.recipients ?? []).flatMap((output) => { + const expectedOutputs: ExpectedOutput[] = (txParams.recipients ?? []).flatMap((output) => { if (output.address === undefined) { + assert('script' in output, 'script is required for non-encodeable scriptPubkeys'); if (output.amount.toString() !== '0') { throw new Error(`Only zero amounts allowed for non-encodeable scriptPubkeys: ${output}`); } - return [output]; + return [ + { + script: toOutputScript(output, coin.name), + value: output.amount === 'max' ? 'max' : BigInt(output.amount), + }, + ]; } - return [{ ...output, address: coin.canonicalAddress(output.address) }]; + return [ + { + script: fromExtendedAddressFormatToScript(output.address, coin.name), + value: output.amount === 'max' ? 'max' : BigInt(output.amount), + }, + ]; }); if (txParams.allowExternalChangeAddress && txParams.changeAddress) { - // when an external change address is explicitly specified, count all outputs going towards that - // address in the expected outputs (regardless of the output amount) - expectedOutputs.push({ address: coin.canonicalAddress(txParams.changeAddress), amount: 'max' }); + expectedOutputs.push({ + script: toOutputScript(txParams.changeAddress, coin.name), + // When an external change address is explicitly specified, count all outputs going towards that + // address in the expected outputs (regardless of the output amount) + value: 'max', + // Note that the change output is not required to exist, so we mark it as optional. + optional: true, + }); } return expectedOutputs; } @@ -206,15 +223,9 @@ export async function parseTransaction( })); } - const missingOutputs = outputDifference( - toComparableOutputsWithExternal(expectedOutputs), - toComparableOutputsWithExternal(allOutputs) - ); + const missingOutputs = outputDifference(expectedOutputs, toComparableOutputsWithExternal(allOutputs)); - const implicitOutputs = outputDifference( - toComparableOutputsWithExternal(allOutputDetails), - toComparableOutputsWithExternal(expectedOutputs) - ); + const implicitOutputs = outputDifference(toComparableOutputsWithExternal(allOutputDetails), expectedOutputs); const explicitOutputs = outputDifference(toComparableOutputsWithExternal(allOutputDetails), implicitOutputs); // these are all the non-wallet outputs that had been originally explicitly specified in recipients @@ -244,7 +255,7 @@ export async function parseTransaction( coin.amountType ); - function toOutputs(outputs: ComparableOutputWithExternal[]): Output[] { + function toOutputs(outputs: ExpectedOutput[] | ComparableOutputWithExternal[]): Output[] { return outputs.map((output) => ({ address: toExtendedAddressFormat(output.script, coin.name), amount: output.value.toString(), diff --git a/modules/abstract-utxo/src/transaction/recipient.ts b/modules/abstract-utxo/src/transaction/recipient.ts index a5dee328c2..3998d8c573 100644 --- a/modules/abstract-utxo/src/transaction/recipient.ts +++ b/modules/abstract-utxo/src/transaction/recipient.ts @@ -33,6 +33,19 @@ export function fromExtendedAddressFormatToScript(extendedAddress: string, coinN return utxolib.addressFormat.toOutputScriptTryFormats(result.address, network); } +export function toOutputScript(v: string | { address: string } | { script: string }, coinName: UtxoCoinName): Buffer { + if (typeof v === 'string') { + return fromExtendedAddressFormatToScript(v, coinName); + } + if ('script' in v) { + return Buffer.from(v.script, 'hex'); + } + if ('address' in v) { + return fromExtendedAddressFormatToScript(v.address, coinName); + } + throw new Error('invalid input'); +} + /** * Convert a script or address to the extended address format. * @param script diff --git a/modules/abstract-utxo/test/unit/transaction/outputDifference.ts b/modules/abstract-utxo/test/unit/transaction/outputDifference.ts index a26ef45fa4..a246855fcc 100644 --- a/modules/abstract-utxo/test/unit/transaction/outputDifference.ts +++ b/modules/abstract-utxo/test/unit/transaction/outputDifference.ts @@ -10,8 +10,8 @@ import { } from '../../../src/transaction/outputDifference'; describe('outputDifference', function () { - function output(script: string, value: bigint | number): ActualOutput; - function output(script: string, value: 'max'): ExpectedOutput; + function output(script: string, value: bigint | number, optional?: boolean): ActualOutput; + function output(script: string, value: 'max', optional?: boolean): ExpectedOutput; function output(script: string, value: bigint | number | 'max', optional?: boolean): ActualOutput | ExpectedOutput { const scriptBuffer = Buffer.from(script, 'hex'); if (scriptBuffer.toString('hex') !== script) {