diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts index 375590ba67..213bf0a123 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 @@ -588,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/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 caea6e5fe0..6a606cd10b 100644 --- a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts +++ b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts @@ -7,16 +7,38 @@ 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 { fromExtendedAddressFormatToScript, toExtendedAddressFormat } from '../recipient'; +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; }; +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 +55,7 @@ async function parseRbfTransaction( if (output.wallet === wallet.id()) { return []; } - return [coin.toCanonicalTransactionRecipient(output)]; + return [toCanonicalTransactionRecipient(coin, output)]; } ); @@ -55,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; } @@ -162,7 +200,7 @@ export async function parseTransaction( keychainArray: toKeychainTriple(keychains), wallet, txParams: { - recipients: expectedOutputs, + recipients: txParams.recipients ?? [], changeAddress: txParams.changeAddress, }, customChange, @@ -185,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 @@ -223,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/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/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/descriptor/outputDifference.ts b/modules/abstract-utxo/test/unit/transaction/outputDifference.ts similarity index 55% rename from modules/abstract-utxo/test/unit/transaction/descriptor/outputDifference.ts rename to modules/abstract-utxo/test/unit/transaction/outputDifference.ts index 6eb9ed87ae..a246855fcc 100644 --- a/modules/abstract-utxo/test/unit/transaction/descriptor/outputDifference.ts +++ b/modules/abstract-utxo/test/unit/transaction/outputDifference.ts @@ -3,15 +3,16 @@ import assert from 'assert'; import { ActualOutput, ExpectedOutput, + getMissingOutputs, matchingOutput, outputDifference, outputDifferencesWithExpected, -} from '../../../../src/transaction/outputDifference'; +} 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 | 'max'): ActualOutput | 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) { 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: [] }); + }); }); });