Merge pull request #7539 from BitGo/BTC-2732.test-parsePsbt

OttoAllmendinger · web-flow · commit 192328003e2c · 2025-11-18T13:42:04.000+01:00
feat(abstract-utxo): improve transaction parsing and structure
diff --git 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<TCoinSpecific extends UtxoCoinSpecific> ex
   coinSpecific?: TCoinSpecific;
 }
 
-export interface BaseOutput<TAmount = string | number> {
-  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<TAmount = string | number> extends BaseOutput<TAmount> {
-  needsCustomChangeKeySignatureVerification?: boolean;
-  chain: number;
-  index: number;
-}
-
-export type Output<TAmount = string | number> = BaseOutput<TAmount> | FixedScriptWalletOutput<TAmount>;
-
 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<TNumber extends number | bigint = number> {
   /** Maps txid to txhex. Required for offline signing. */
   txHexes?: Record<string, string>;
@@ -255,42 +233,6 @@ export interface ParseTransactionOptions<TNumber extends number | bigint = numbe
   reqId?: IRequestTracer;
 }
 
-export type BaseParsedTransactionOutputs<TNumber extends number | bigint, TOutput> = {
-  /** 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<TNumber extends number | bigint, TOutput> = 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<TNumber extends number | bigint = number> = BaseParsedTransaction<TNumber, Output>;
-
 export interface GenerateAddressOptions {
   addressType?: ScriptType2Of3;
   threshold?: number;
diff --git 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
@@ -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}`);
-  }
 }
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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';
@@ -189,11 +184,13 @@ export async function parseTransaction<TNumber extends bigint | number>(
 
   /**
    * 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
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,55 @@
+import type { UtxoNamedKeychains } from '../keychains';
+
+import type { CustomChangeOptions } from './fixedScript';
+
+export interface BaseOutput<TAmount = string | number> {
+  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<TAmount = string | number> extends BaseOutput<TAmount> {
+  needsCustomChangeKeySignatureVerification?: boolean;
+  chain: number;
+  index: number;
+}
+
+export type Output<TAmount = string | number> = BaseOutput<TAmount> | FixedScriptWalletOutput<TAmount>;
+
+export type BaseParsedTransactionOutputs<TNumber extends number | bigint, TOutput> = {
+  /** 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<TNumber extends number | bigint, TOutput> = 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<TNumber extends number | bigint = number> = BaseParsedTransaction<TNumber, Output>;
diff --git 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
@@ -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
@@ -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
