feat(abstract-utxo): enhance output handling and fix address comparison

modules/abstract-utxo/src/abstractUtxoCoin.ts

@@ -479,7 +479,11 @@ export abstract class AbstractUtxoCoin
     }
   }
 
-  preprocessBuildParams(params: Record<string, any>): Record<string, any> {
+  /**
+   * 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<string, any>): Record<string, any> {
     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

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) {

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<TValue> = ComparableOutput<TValue> & {
+export type ComparableOutputWithExternal<TValue> = (ComparableOutput<TValue> | 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<TNumber extends bigint | number>(
   coin: AbstractUtxoCoin,
   params: ParseTransactionOptions<TNumber>
@@ -33,7 +55,7 @@ async function parseRbfTransaction<TNumber extends bigint | number>(
       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<TNumber extends bigint | number>(
         keychainArray: toKeychainTriple(keychains),
         wallet,
         txParams: {
-          recipients: expectedOutputs,
+          recipients: txParams.recipients ?? [],
           changeAddress: txParams.changeAddress,
         },
         customChange,
@@ -185,15 +223,9 @@ export async function parseTransaction<TNumber extends bigint | number>(
     }));
   }
 
-  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<TNumber extends bigint | number>(
     coin.amountType
   );
 
-  function toOutputs(outputs: ComparableOutputWithExternal<bigint | 'max'>[]): Output[] {
+  function toOutputs(outputs: ExpectedOutput[] | ComparableOutputWithExternal<bigint | 'max'>[]): Output[] {
     return outputs.map((output) => ({
       address: toExtendedAddressFormat(output.script, coin.name),
       amount: output.value.toString(),

modules/abstract-utxo/src/transaction/outputDifference.ts

@@ -7,7 +7,10 @@ export type ComparableOutput<TValue> = {
 export type ActualOutput = ComparableOutput<bigint>;
 
 /** Expected outputs can have a fixed value or 'max'. */
-export type ExpectedOutput = ComparableOutput<bigint | 'max'>;
+export type ExpectedOutput = ComparableOutput<bigint | 'max'> & {
+  /** When true, the output is not required to be present in the transaction. */
+  optional?: boolean;
+};
 
 /**
  * @param a
@@ -40,6 +43,13 @@ export function outputDifference<A extends ActualOutput | ExpectedOutput, B exte
   return first;
 }
 
+export function getMissingOutputs<A extends ActualOutput, B extends ExpectedOutput>(
+  actualOutputs: A[],
+  expectedOutputs: B[]
+): B[] {
+  return outputDifference(expectedOutputs, actualOutputs).filter((o) => !o.optional);
+}
+
 export type OutputDifferenceWithExpected<TActual extends ActualOutput, TExpected extends ExpectedOutput> = {
   /** These are the external outputs that were expected and found in the transaction. */
   explicitOutputs: TActual[];
@@ -65,7 +75,7 @@ export function outputDifferencesWithExpected<TActual extends ActualOutput, TExp
 ): OutputDifferenceWithExpected<TActual, TExpected> {
   const implicitOutputs = outputDifference(actualOutputs, expectedOutputs);
   const explicitOutputs = outputDifference(actualOutputs, implicitOutputs);
-  const missingOutputs = outputDifference(expectedOutputs, actualOutputs);
+  const missingOutputs = getMissingOutputs(actualOutputs, expectedOutputs);
   return {
     explicitOutputs,
     implicitOutputs,

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

modules/abstract-utxo/test/unit/transaction/descriptor/outputDifference.ts → modules/abstract-utxo/test/unit/transaction/outputDifference.ts

@@ -3,25 +3,31 @@ 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');
     }
     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: [] });
+    });
   });
 });