Merge pull request #7523 from BitGo/BTC-2732.refactor-explaintx

OttoAllmendinger · web-flow · commit 73a2fb5c214e · 2025-11-14T21:45:57.000+01:00
refactor(abstract-utxo): refactor explainTransaction return type
diff --git a/modules/abstract-utxo/src/abstractUtxoCoin.ts b/modules/abstract-utxo/src/abstractUtxoCoin.ts
@@ -17,7 +17,6 @@ import {
   InvalidAddressError,
   IRequestTracer,
   isTriple,
-  ITransactionExplanation as BaseTransactionExplanation,
   IWallet,
   KeychainsTriplet,
   KeyIndices,
@@ -66,6 +65,7 @@ import {
   parseTransaction,
   verifyTransaction,
 } from './transaction';
+import type { TransactionExplanation } from './transaction/fixedScript/explainTransaction';
 import {
   AggregateValidationError,
   ErrorMissingOutputs,
@@ -200,29 +200,6 @@ export function isWalletOutput(output: Output): output is FixedScriptWalletOutpu
   );
 }
 
-export interface TransactionExplanation<TFee = string> extends BaseTransactionExplanation<TFee, string> {
-  locktime?: number;
-  /** NOTE: this actually only captures external outputs */
-  outputs: Output[];
-  changeOutputs: Output[];
-
-  /**
-   * Number of input signatures per input.
-   */
-  inputSignatures: number[];
-
-  /**
-   * Highest input signature count for the transaction
-   */
-  signatures: number;
-
-  /**
-   * BIP322 messages extracted from the transaction inputs.
-   * These messages are used for verifying the transaction against the BIP322 standard.
-   */
-  messages?: Bip322Message[];
-}
-
 export interface TransactionInfo<TNumber extends number | bigint = number> {
   /** Maps txid to txhex. Required for offline signing. */
   txHexes?: Record<string, string>;
@@ -870,9 +847,9 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
    * change amounts, and transaction outputs.
    * @param params
    */
-  async explainTransaction<TNumber extends number | bigint = number>(
+  override async explainTransaction<TNumber extends number | bigint = number>(
     params: ExplainTransactionOptions<TNumber>
-  ): Promise<TransactionExplanation<string | undefined>> {
+  ): Promise<TransactionExplanation> {
     return explainTx(this.decodeTransactionFromPrebuild(params), params, this.network);
   }
 
diff --git a/modules/abstract-utxo/src/impl/doge/doge.ts b/modules/abstract-utxo/src/impl/doge/doge.ts
@@ -6,14 +6,14 @@ import {
   UtxoNetwork,
   SignTransactionOptions,
   ExplainTransactionOptions,
-  TransactionExplanation,
   ParseTransactionOptions,
   ParsedTransaction,
   VerifyTransactionOptions,
   RecoverFromWrongChainOptions,
   TransactionInfo,
   TransactionPrebuild,
 } from '../../abstractUtxoCoin';
+import type { TransactionExplanation } from '../../transaction/fixedScript/explainTransaction';
 import type { CrossChainRecoverySigned, CrossChainRecoveryUnsigned } from '../../recovery/crossChainRecovery';
 
 type UnspentJSON = bitgo.Unspent<number> & { valueString: string };
@@ -114,7 +114,7 @@ export class Doge extends AbstractUtxoCoin {
 
   async explainTransaction<TNumber extends number | bigint = bigint>(
     params: ExplainTransactionOptions<TNumber> | (ExplainTransactionOptions<TNumber> & { txInfo: TransactionInfoJSON })
-  ): Promise<TransactionExplanation<string | undefined>> {
+  ): Promise<TransactionExplanation> {
     return super.explainTransaction({
       ...params,
       txInfo: params.txInfo ? parseTransactionInfo(params.txInfo as TransactionInfoJSON) : undefined,
diff --git a/modules/abstract-utxo/src/transaction/descriptor/explainPsbt.ts b/modules/abstract-utxo/src/transaction/descriptor/explainPsbt.ts
@@ -3,7 +3,7 @@ import { ITransactionRecipient } from '@bitgo/sdk-core';
 import * as coreDescriptors from '@bitgo/utxo-core/descriptor';
 
 import { toExtendedAddressFormat } from '../recipient';
-import { TransactionExplanation } from '../../abstractUtxoCoin';
+import type { TransactionExplanationUtxolibPsbt } from '../fixedScript/explainTransaction';
 
 function toRecipient(output: coreDescriptors.ParsedOutput, network: utxolib.Network): ITransactionRecipient {
   return {
@@ -34,7 +34,7 @@ function getInputSignatures(psbt: utxolib.bitgo.UtxoPsbt): number[] {
 export function explainPsbt(
   psbt: utxolib.bitgo.UtxoPsbt,
   descriptors: coreDescriptors.DescriptorMap
-): TransactionExplanation<string> {
+): TransactionExplanationUtxolibPsbt {
   const parsedTransaction = coreDescriptors.parse(psbt, descriptors, psbt.network);
   const { inputs, outputs } = parsedTransaction;
   const externalOutputs = outputs.filter((o) => o.scriptId === undefined);
diff --git a/modules/abstract-utxo/src/transaction/explainTransaction.ts b/modules/abstract-utxo/src/transaction/explainTransaction.ts
@@ -1,11 +1,14 @@
 import * as utxolib from '@bitgo/utxo-lib';
 import { isTriple, IWallet } from '@bitgo/sdk-core';
 
-import { TransactionExplanation } from '../abstractUtxoCoin';
 import { getDescriptorMapFromWallet, isDescriptorWallet } from '../descriptor';
 import { toBip32Triple } from '../keychains';
 import { getPolicyForEnv } from '../descriptor/validatePolicy';
 
+import type {
+  TransactionExplanationUtxolibLegacy,
+  TransactionExplanationUtxolibPsbt,
+} from './fixedScript/explainTransaction';
 import * as fixedScript from './fixedScript';
 import * as descriptor from './descriptor';
 
@@ -22,7 +25,7 @@ export function explainTx<TNumber extends number | bigint>(
     changeInfo?: fixedScript.ChangeAddressInfo[];
   },
   network: utxolib.Network
-): TransactionExplanation<string | undefined> {
+): TransactionExplanationUtxolibLegacy | TransactionExplanationUtxolibPsbt {
   if (params.wallet && isDescriptorWallet(params.wallet)) {
     if (tx instanceof utxolib.bitgo.UtxoPsbt) {
       if (!params.pubs || !isTriple(params.pubs)) {
diff --git a/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/explainTransaction.ts
@@ -2,13 +2,53 @@ import * as utxolib from '@bitgo/utxo-lib';
 import { bip322 } from '@bitgo/utxo-core';
 import { BIP32Interface, bip32 } from '@bitgo/secp256k1';
 import { bitgo } from '@bitgo/utxo-lib';
-import { Triple } from '@bitgo/sdk-core';
+import { ITransactionExplanation as BaseTransactionExplanation, Triple } from '@bitgo/sdk-core';
 import * as utxocore from '@bitgo/utxo-core';
 
-import { Output, TransactionExplanation, Bip322Message, FixedScriptWalletOutput } from '../../abstractUtxoCoin';
+import type { Output, Bip322Message, FixedScriptWalletOutput } from '../../abstractUtxoCoin';
 import { toExtendedAddressFormat } from '../recipient';
 import { getPayGoVerificationPubkey } from '../getPayGoVerificationPubkey';
 
+// ===== Transaction Explanation Type Definitions =====
+
+export interface AbstractUtxoTransactionExplanation<TFee = string> extends BaseTransactionExplanation<TFee, string> {
+  /** NOTE: this actually only captures external outputs */
+  outputs: Output[];
+  changeOutputs: Output[];
+
+  /**
+   * BIP322 messages extracted from the transaction inputs.
+   * These messages are used for verifying the transaction against the BIP322 standard.
+   */
+  messages?: Bip322Message[];
+}
+
+/** @deprecated - the signature fields are not very useful */
+interface TransactionExplanationWithSignatures<TFee = string> extends AbstractUtxoTransactionExplanation<TFee> {
+  /** @deprecated - unused outside of tests */
+  locktime?: number;
+
+  /**
+   * Number of input signatures per input.
+   * @deprecated - this is not very useful without knowing who signed each input.
+   */
+  inputSignatures: number[];
+
+  /**
+   * Highest input signature count for the transaction
+   * @deprecated - this is not very useful without knowing who signed each input.
+   */
+  signatures: number;
+}
+
+/** When parsing the legacy transaction format, we cannot always infer the fee so we set it to string | undefined */
+export type TransactionExplanationUtxolibLegacy = TransactionExplanationWithSignatures<string | undefined>;
+
+/** When parsing a PSBT, we can infer the fee so we set TFee to string. */
+export type TransactionExplanationUtxolibPsbt = TransactionExplanationWithSignatures<string>;
+
+export type TransactionExplanation = TransactionExplanationUtxolibLegacy | TransactionExplanationUtxolibPsbt;
+
 export type ChangeAddressInfo = {
   address: string;
   chain: number;
@@ -302,7 +342,7 @@ export function explainPsbt(
   },
   network: utxolib.Network,
   { strict = false }: { strict?: boolean } = {}
-): TransactionExplanation {
+): TransactionExplanationUtxolibPsbt {
   const payGoVerificationInfo = getPayGoVerificationInfo(psbt, network);
   if (payGoVerificationInfo) {
     try {
@@ -356,7 +396,7 @@ export function explainLegacyTx<TNumber extends number | bigint>(
     changeInfo?: { address: string; chain: number; index: number }[];
   },
   network: utxolib.Network
-): TransactionExplanation<string | undefined> {
+): TransactionExplanationUtxolibLegacy {
   const common = explainCommon(tx, params, network);
   const inputSignaturesCount = getTxInputSignaturesCount(tx, params, network);
   return {
diff --git a/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts b/modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts
@@ -4,18 +4,18 @@ import _ from 'lodash';
 import { Triple, VerificationOptions, Wallet } from '@bitgo/sdk-core';
 import * as utxolib from '@bitgo/utxo-lib';
 
-import {
+import type {
   AbstractUtxoCoin,
   FixedScriptWalletOutput,
   Output,
-  TransactionExplanation,
   ParsedTransaction,
   ParseTransactionOptions,
 } from '../../abstractUtxoCoin';
 import { fetchKeychains, getKeySignatures, toKeychainTriple, UtxoKeychain, UtxoNamedKeychains } from '../../keychains';
 import { ComparableOutput, outputDifference } from '../outputDifference';
 import { fromExtendedAddressFormatToScript, toExtendedAddressFormat } from '../recipient';
 
+import type { TransactionExplanation } from './explainTransaction';
 import { CustomChangeOptions, parseOutput } from './parseOutput';
 
 export type ComparableOutputWithExternal<TValue> = ComparableOutput<TValue> & {
@@ -53,7 +53,7 @@ export async function parseTransaction<TNumber extends bigint | number>(
   }
 
   // obtain all outputs
-  const explanation: TransactionExplanation<string | undefined> = await coin.explainTransaction<TNumber>({
+  const explanation: TransactionExplanation = await coin.explainTransaction<TNumber>({
     txHex: txPrebuild.txHex,
     txInfo: txPrebuild.txInfo,
     pubs: keychainArray.map((k) => k.pub) as Triple<string>,
diff --git a/modules/abstract-utxo/test/unit/explainTransaction.ts b/modules/abstract-utxo/test/unit/explainTransaction.ts
@@ -28,6 +28,7 @@ describe('Explain Transaction', function () {
       assert.strictEqual(result.outputs.length, 1);
       assert.strictEqual(result.outputs[0].address, 'scriptPubKey:6a');
       assert.strictEqual(result.fee, '0');
+      assert.ok('signatures' in result);
       assert.strictEqual(result.signatures, 0);
       assert.ok(result.messages);
       result.messages?.forEach((obj) => {
@@ -45,6 +46,7 @@ describe('Explain Transaction', function () {
       assert.strictEqual(result.outputs.length, 1);
       assert.strictEqual(result.outputs[0].address, 'scriptPubKey:6a');
       assert.strictEqual(result.fee, '0');
+      assert.ok('signatures' in result);
       assert.strictEqual(result.signatures, 1);
       assert.ok(result.messages);
       result.messages?.forEach((obj) => {
@@ -62,6 +64,7 @@ describe('Explain Transaction', function () {
       assert.strictEqual(result.outputs.length, 1);
       assert.strictEqual(result.outputs[0].address, 'scriptPubKey:6a');
       assert.strictEqual(result.fee, '0');
+      assert.ok('signatures' in result);
       assert.strictEqual(result.signatures, 2);
       assert.ok(result.messages);
       result.messages?.forEach((obj) => {
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, TransactionExplanation, TransactionParams } from '../../src';
+import { UtxoWallet, Output, TransactionParams } from '../../src';
+import type { TransactionExplanation } from '../../src/transaction/fixedScript/explainTransaction';
 
 import { getUtxoCoin } from './util';
 
diff --git a/modules/abstract-utxo/test/unit/transaction.ts b/modules/abstract-utxo/test/unit/transaction.ts
@@ -564,12 +564,16 @@ function run<TNumber extends number | bigint = number>(
           ? 2
           : undefined;
 
-      explanation.inputSignatures.should.eql(
-        // FIXME(BG-35154): implement signature verification for replay protection inputs
-        inputScripts.map((type) => (type === 'p2shP2pk' ? 0 : expectedSignatureCount))
-      );
-      explanation.signatures.should.eql(expectedSignatureCount);
-      explanation.changeAmount.should.eql('0'); // no change addresses given
+      if ('inputSignatures' in explanation) {
+        explanation.inputSignatures.should.eql(
+          // FIXME(BG-35154): implement signature verification for replay protection inputs
+          inputScripts.map((type) => (type === 'p2shP2pk' ? 0 : expectedSignatureCount))
+        );
+      }
+      if ('signatures' in explanation) {
+        explanation.signatures.should.eql(expectedSignatureCount);
+        explanation.changeAmount.should.eql('0'); // no change addresses given
+      }
       let expectedOutputAmount =
         BigInt((txFormat === 'psbt' ? getUnspentsForPsbt() : getUnspents()).length) * BigInt(value);
       inputScripts.forEach((type) => {
diff --git a/modules/abstract-utxo/test/unit/transaction/descriptor/explainPsbt.ts b/modules/abstract-utxo/test/unit/transaction/descriptor/explainPsbt.ts
@@ -3,14 +3,16 @@ import assert from 'assert';
 import { getKeyTriple } from '@bitgo/utxo-core/testutil';
 import { getDescriptorMap, mockPsbtDefaultWithDescriptorTemplate } from '@bitgo/utxo-core/testutil/descriptor';
 
-import { TransactionExplanation } from '../../../../src';
+import type { TransactionExplanation } from '../../../../src/transaction/fixedScript/explainTransaction';
 import { explainPsbt } from '../../../../src/transaction/descriptor';
 
 import { getFixtureRoot } from './fixtures.utils';
 
 const { assertEqualFixture } = getFixtureRoot(__dirname + '/fixtures');
 
 function assertSignatureCount(expl: TransactionExplanation, signatures: number, inputSignatures: number[]) {
+  assert.ok('signatures' in expl);
+  assert.ok('inputSignatures' in expl);
   assert.deepStrictEqual(expl.signatures, signatures);
   assert.deepStrictEqual(expl.inputSignatures, inputSignatures);
 }
diff --git a/modules/abstract-utxo/test/unit/transaction/fixedScript/explainPsbt.ts b/modules/abstract-utxo/test/unit/transaction/fixedScript/explainPsbt.ts
@@ -2,35 +2,33 @@ import assert from 'node:assert/strict';
 
 import { testutil } from '@bitgo/utxo-lib';
 
+import type { TransactionExplanation } from '../../../../src/transaction/fixedScript/explainTransaction';
 import { explainPsbt } from '../../../../src/transaction/fixedScript';
 
 function describeTransactionWith(acidTest: testutil.AcidTest) {
-  describe(`explainPsbt ${acidTest.name}`, function () {
-    it('should explain the transaction', function () {
+  describe(`${acidTest.name}`, function () {
+    let refExplanation: TransactionExplanation;
+    before('prepare', function () {
       const psbt = acidTest.createPsbt();
-      const explanation = explainPsbt(psbt, { pubs: acidTest.rootWalletKeys }, acidTest.network, { strict: true });
-      assert.strictEqual(explanation.outputs.length, 3);
-      assert.strictEqual(explanation.outputAmount, '2700');
-      assert.strictEqual(explanation.changeOutputs.length, acidTest.outputs.length - 3);
-      explanation.changeOutputs.forEach((change) => {
+      refExplanation = explainPsbt(psbt, { pubs: acidTest.rootWalletKeys }, acidTest.network, {
+        strict: true,
+      });
+    });
+
+    it('should match the expected values for explainPsbt', function () {
+      // note: `outputs` means external outputs here
+      assert.strictEqual(refExplanation.outputs.length, 3);
+      assert.strictEqual(refExplanation.changeOutputs.length, acidTest.outputs.length - 3);
+      assert.strictEqual(refExplanation.outputAmount, '2700');
+      assert.strictEqual(refExplanation.changeOutputs.length, acidTest.outputs.length - 3);
+      refExplanation.changeOutputs.forEach((change) => {
         assert.strictEqual(change.amount, '900');
         assert.strictEqual(typeof change.address, 'string');
       });
-      assert.strictEqual(explanation.inputSignatures.length, acidTest.inputs.length);
-      explanation.inputSignatures.forEach((signature, i) => {
-        if (acidTest.inputs[i].scriptType === 'p2shP2pk') {
-          return;
-        }
-        if (acidTest.signStage === 'unsigned') {
-          assert.strictEqual(signature, 0);
-        } else if (acidTest.signStage === 'halfsigned') {
-          assert.strictEqual(signature, 1);
-        } else if (acidTest.signStage === 'fullsigned') {
-          assert.strictEqual(signature, 2);
-        }
-      });
     });
   });
 }
 
-testutil.AcidTest.suite().forEach((test) => describeTransactionWith(test));
+describe('explainPsbt', function () {
+  testutil.AcidTest.suite().forEach((test) => describeTransactionWith(test));
+});
