Merge pull request #7676 from BitGo/BTC-2806.refactor-signtx

feat(abstract-utxo): refactor transaction signing functionality

Author: OttoAllmendinger

modules/abstract-utxo/src/abstractUtxoCoin.ts

@@ -77,6 +77,7 @@ import { assertDescriptorWalletAddress, getDescriptorMapFromWallet, isDescriptor
 import { getChainFromNetwork, getFamilyFromNetwork, getFullNameFromNetwork } from './names';
 import { assertFixedScriptWalletAddress } from './address/fixedScript';
 import { ParsedTransaction } from './transaction/types';
+import { decodePsbtWith, stringToBufferTryFormats } from './transaction/decode';
 import { toBip32Triple, UtxoKeychain } from './keychains';
 import { verifyKeySignature, verifyUserPublicKey } from './verifyKey';
 import { getPolicyForEnv } from './descriptor/validatePolicy';
@@ -527,22 +528,12 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
 
   decodeTransaction<TNumber extends number | bigint>(input: Buffer | string): DecodedTransaction<TNumber> {
     if (typeof input === 'string') {
-      for (const format of ['hex', 'base64'] as const) {
-        const buffer = Buffer.from(input, format);
-        const bufferToString = buffer.toString(format);
-        if (
-          (format === 'base64' && bufferToString === input) ||
-          (format === 'hex' && bufferToString === input.toLowerCase())
-        ) {
-          return this.decodeTransaction(buffer);
-        }
-      }
-
-      throw new Error('input must be a valid hex or base64 string');
+      const buffer = stringToBufferTryFormats(input, ['hex', 'base64']);
+      return this.decodeTransaction(buffer);
     }
 
     if (utxolib.bitgo.isPsbt(input)) {
-      return utxolib.bitgo.createPsbtFromBuffer(input, this.network);
+      return decodePsbtWith(input, this.network, 'utxolib');
     } else {
       return utxolib.bitgo.createTransactionFromBuffer(input, this.network, {
         amountType: this.amountType,

modules/abstract-utxo/src/index.ts

@@ -3,7 +3,7 @@ export * from './address';
 export * from './config';
 export * from './recovery';
 export * from './transaction/fixedScript/replayProtection';
-export * from './transaction/fixedScript/sign';
+export * from './transaction/fixedScript/signLegacyTransaction';
 
 export { UtxoWallet } from './wallet';
 export * as descriptor from './descriptor';

modules/abstract-utxo/src/recovery/backupKeyRecovery.ts

@@ -16,7 +16,7 @@ import {
 import { getMainnet, networks } from '@bitgo/utxo-lib';
 
 import { AbstractUtxoCoin } from '../abstractUtxoCoin';
-import { signAndVerifyPsbt } from '../transaction/fixedScript/sign';
+import { signAndVerifyPsbt } from '../transaction/fixedScript/signPsbt';
 import { generateAddressWithChainAndIndex } from '../address';
 
 import { forCoin, RecoveryProvider } from './RecoveryProvider';

modules/abstract-utxo/src/recovery/crossChainRecovery.ts

@@ -5,7 +5,7 @@ import { BitGoBase, IWallet, Keychain, Triple, Wallet } from '@bitgo/sdk-core';
 import { decrypt } from '@bitgo/sdk-api';
 
 import { AbstractUtxoCoin, TransactionInfo } from '../abstractUtxoCoin';
-import { signAndVerifyWalletTransaction } from '../transaction/fixedScript/sign';
+import { signAndVerifyWalletTransaction } from '../transaction/fixedScript/signLegacyTransaction';
 
 const { unspentSum, scriptTypeForChain, outputScripts } = utxolib.bitgo;
 type RootWalletKeys = utxolib.bitgo.RootWalletKeys;

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

@@ -0,0 +1,59 @@
+import * as utxolib from '@bitgo/utxo-lib';
+import { fixedScriptWallet, utxolibCompat } from '@bitgo/wasm-utxo';
+
+import { SdkBackend } from './types';
+
+type BufferEncoding = 'hex' | 'base64';
+
+export function stringToBufferTryFormats(input: string, formats: BufferEncoding[] = ['hex', 'base64']): Buffer {
+  for (const format of formats) {
+    const buffer = Buffer.from(input, format);
+    const bufferToString = buffer.toString(format);
+    if (
+      (format === 'base64' && bufferToString === input) ||
+      (format === 'hex' && bufferToString === input.toLowerCase())
+    ) {
+      return buffer;
+    }
+  }
+
+  throw new Error('input must be a valid hex or base64 string');
+}
+
+function toNetworkName(network: utxolib.Network): utxolibCompat.UtxolibName {
+  const networkName = utxolib.getNetworkName(network);
+  if (!networkName) {
+    throw new Error(`Invalid network: ${network}`);
+  }
+  return networkName;
+}
+
+export function decodePsbtWith(
+  psbt: string | Buffer,
+  network: utxolib.Network,
+  backend: 'utxolib'
+): utxolib.bitgo.UtxoPsbt;
+export function decodePsbtWith(
+  psbt: string | Buffer,
+  network: utxolib.Network,
+  backend: 'wasm-utxo'
+): fixedScriptWallet.BitGoPsbt;
+export function decodePsbtWith(
+  psbt: string | Buffer,
+  network: utxolib.Network,
+  backend: SdkBackend
+): utxolib.bitgo.UtxoPsbt | fixedScriptWallet.BitGoPsbt;
+export function decodePsbtWith(
+  psbt: string | Buffer,
+  network: utxolib.Network,
+  backend: SdkBackend
+): utxolib.bitgo.UtxoPsbt | fixedScriptWallet.BitGoPsbt {
+  if (typeof psbt === 'string') {
+    psbt = Buffer.from(psbt, 'hex');
+  }
+  if (backend === 'utxolib') {
+    return utxolib.bitgo.createPsbtFromBuffer(psbt, network);
+  } else {
+    return fixedScriptWallet.BitGoPsbt.fromBytes(psbt, toNetworkName(network));
+  }
+}

modules/abstract-utxo/src/transaction/fixedScript/SigningError.ts

@@ -0,0 +1,38 @@
+import * as utxolib from '@bitgo/utxo-lib';
+
+import type { PsbtParsedScriptType } from './signPsbt';
+
+type Unspent<TNumber extends number | bigint = number> = utxolib.bitgo.Unspent<TNumber>;
+
+export class InputSigningError<TNumber extends number | bigint = number> extends Error {
+  static expectedWalletUnspent<TNumber extends number | bigint>(
+    inputIndex: number,
+    inputType: PsbtParsedScriptType | null, // null for legacy transaction format
+    unspent: Unspent<TNumber> | { id: string }
+  ): InputSigningError<TNumber> {
+    return new InputSigningError(
+      inputIndex,
+      inputType,
+      unspent,
+      `not a wallet unspent, not a replay protection unspent`
+    );
+  }
+
+  constructor(
+    public inputIndex: number,
+    public inputType: PsbtParsedScriptType | null, // null for legacy transaction format
+    public unspent: Unspent<TNumber> | { id: string },
+    public reason: Error | string
+  ) {
+    super(`signing error at input ${inputIndex}: type=${inputType} unspentId=${unspent.id}: ${reason}`);
+  }
+}
+
+export class TransactionSigningError<TNumber extends number | bigint = number> extends Error {
+  constructor(signErrors: InputSigningError<TNumber>[], verifyError: InputSigningError<TNumber>[]) {
+    super(
+      `sign errors at inputs: [${signErrors.join(',')}], ` +
+        `verify errors at inputs: [${verifyError.join(',')}], see log for details`
+    );
+  }
+}

modules/abstract-utxo/src/transaction/fixedScript/index.ts

@@ -3,6 +3,8 @@ export { explainPsbtWasm } from './explainPsbtWasm';
 export { parseTransaction } from './parseTransaction';
 export { CustomChangeOptions } from './parseOutput';
 export { verifyTransaction } from './verifyTransaction';
-export { signTransaction, Musig2Participant } from './signTransaction';
-export * from './sign';
+export { signTransaction } from './signTransaction';
+export { Musig2Participant } from './signPsbt';
+export * from './signLegacyTransaction';
+export * from './SigningError';
 export * from './replayProtection';

…utxo/src/transaction/fixedScript/sign.ts …ion/fixedScript/signLegacyTransaction.tsmodules/abstract-utxo/src/transaction/fixedScript/sign.ts renamed to modules/abstract-utxo/src/transaction/fixedScript/signLegacyTransaction.ts modules/abstract-utxo/src/transaction/fixedScript/sign.ts renamed to modules/abstract-utxo/src/transaction/fixedScript/signLegacyTransaction.ts

@@ -1,7 +1,13 @@
+import assert from 'assert';
+
 import * as utxolib from '@bitgo/utxo-lib';
+import { BIP32Interface, bip32 } from '@bitgo/secp256k1';
+import { bitgo } from '@bitgo/utxo-lib';
+import { isTriple, Triple } from '@bitgo/sdk-core';
 import debugLib from 'debug';
 
 import { getReplayProtectionAddresses } from './replayProtection';
+import { InputSigningError, TransactionSigningError } from './SigningError';
 
 const debug = debugLib('bitgo:v2:utxo');
 
@@ -11,132 +17,6 @@ type Unspent<TNumber extends number | bigint = number> = utxolib.bitgo.Unspent<T
 
 type RootWalletKeys = utxolib.bitgo.RootWalletKeys;
 
-type PsbtParsedScriptType =
-  | 'p2sh'
-  | 'p2wsh'
-  | 'p2shP2wsh'
-  | 'p2shP2pk'
-  | 'taprootKeyPathSpend'
-  | 'taprootScriptPathSpend';
-
-export class InputSigningError<TNumber extends number | bigint = number> extends Error {
-  static expectedWalletUnspent<TNumber extends number | bigint>(
-    inputIndex: number,
-    inputType: PsbtParsedScriptType | null, // null for legacy transaction format
-    unspent: Unspent<TNumber> | { id: string }
-  ): InputSigningError<TNumber> {
-    return new InputSigningError(
-      inputIndex,
-      inputType,
-      unspent,
-      `not a wallet unspent, not a replay protection unspent`
-    );
-  }
-
-  constructor(
-    public inputIndex: number,
-    public inputType: PsbtParsedScriptType | null, // null for legacy transaction format
-    public unspent: Unspent<TNumber> | { id: string },
-    public reason: Error | string
-  ) {
-    super(`signing error at input ${inputIndex}: type=${inputType} unspentId=${unspent.id}: ${reason}`);
-  }
-}
-
-export class TransactionSigningError<TNumber extends number | bigint = number> extends Error {
-  constructor(signErrors: InputSigningError<TNumber>[], verifyError: InputSigningError<TNumber>[]) {
-    super(
-      `sign errors at inputs: [${signErrors.join(',')}], ` +
-        `verify errors at inputs: [${verifyError.join(',')}], see log for details`
-    );
-  }
-}
-
-/**
- * Sign all inputs of a psbt and verify signatures after signing.
- * Collects and logs signing errors and verification errors, throws error in the end if any of them
- * failed.
- *
- * If it is the last signature, finalize and extract the transaction from the psbt.
- *
- * This function mirrors signAndVerifyWalletTransaction, but is used for signing PSBTs instead of
- * using TransactionBuilder
- *
- * @param psbt
- * @param signerKeychain
- * @param isLastSignature
- */
-export function signAndVerifyPsbt(
-  psbt: utxolib.bitgo.UtxoPsbt,
-  signerKeychain: utxolib.BIP32Interface,
-  {
-    isLastSignature,
-    /** deprecated */
-    allowNonSegwitSigningWithoutPrevTx,
-  }: { isLastSignature: boolean; allowNonSegwitSigningWithoutPrevTx?: boolean }
-): utxolib.bitgo.UtxoPsbt | utxolib.bitgo.UtxoTransaction<bigint> {
-  const txInputs = psbt.txInputs;
-  const outputIds: string[] = [];
-  const scriptTypes: PsbtParsedScriptType[] = [];
-
-  const signErrors: InputSigningError<bigint>[] = psbt.data.inputs
-    .map((input, inputIndex: number) => {
-      const outputId = utxolib.bitgo.formatOutputId(utxolib.bitgo.getOutputIdForInput(txInputs[inputIndex]));
-      outputIds.push(outputId);
-
-      const { scriptType } = utxolib.bitgo.parsePsbtInput(input);
-      scriptTypes.push(scriptType);
-
-      if (scriptType === 'p2shP2pk') {
-        debug('Skipping signature for input %d of %d (RP input?)', inputIndex + 1, psbt.data.inputs.length);
-        return;
-      }
-
-      try {
-        psbt.signInputHD(inputIndex, signerKeychain);
-        debug('Successfully signed input %d of %d', inputIndex + 1, psbt.data.inputs.length);
-      } catch (e) {
-        return new InputSigningError<bigint>(inputIndex, scriptType, { id: outputId }, e);
-      }
-    })
-    .filter((e): e is InputSigningError<bigint> => e !== undefined);
-
-  const verifyErrors: InputSigningError<bigint>[] = psbt.data.inputs
-    .map((input, inputIndex) => {
-      const scriptType = scriptTypes[inputIndex];
-      if (scriptType === 'p2shP2pk') {
-        debug(
-          'Skipping input signature %d of %d (unspent from replay protection address which is platform signed only)',
-          inputIndex + 1,
-          psbt.data.inputs.length
-        );
-        return;
-      }
-
-      const outputId = outputIds[inputIndex];
-      try {
-        if (!psbt.validateSignaturesOfInputHD(inputIndex, signerKeychain)) {
-          return new InputSigningError(inputIndex, scriptType, { id: outputId }, new Error(`invalid signature`));
-        }
-      } catch (e) {
-        debug('Invalid signature');
-        return new InputSigningError<bigint>(inputIndex, scriptType, { id: outputId }, e);
-      }
-    })
-    .filter((e): e is InputSigningError<bigint> => e !== undefined);
-
-  if (signErrors.length || verifyErrors.length) {
-    throw new TransactionSigningError(signErrors, verifyErrors);
-  }
-
-  if (isLastSignature) {
-    psbt.finalizeAllInputs();
-    return psbt.extractTransaction();
-  }
-
-  return psbt;
-}
-
 /**
  * Sign all inputs of a wallet transaction and verify signatures after signing.
  * Collects and logs signing errors and verification errors, throws error in the end if any of them
@@ -232,3 +112,43 @@ export function signAndVerifyWalletTransaction<TNumber extends number | bigint>(
 
   return signedTransaction;
 }
+
+export function signLegacyTransaction<TNumber extends number | bigint>(
+  tx: utxolib.bitgo.UtxoTransaction<TNumber>,
+  signerKeychain: BIP32Interface | undefined,
+  params: {
+    isLastSignature: boolean;
+    signingStep: 'signerNonce' | 'cosignerNonce' | 'signerSignature' | undefined;
+    txInfo: { unspents?: utxolib.bitgo.Unspent<TNumber>[] } | undefined;
+    pubs: string[] | undefined;
+    cosignerPub: string | undefined;
+  }
+): utxolib.bitgo.UtxoTransaction<TNumber> {
+  switch (params.signingStep) {
+    case 'signerNonce':
+    case 'cosignerNonce':
+      /**
+       * In certain cases, the caller of this method may not know whether the txHex contains a psbt with taproot key path spend input(s).
+       * Instead of throwing error, no-op and return the txHex. So that the caller can call this method in the same sequence.
+       */
+      return tx;
+  }
+
+  if (tx.ins.length !== params.txInfo?.unspents?.length) {
+    throw new Error('length of unspents array should equal to the number of transaction inputs');
+  }
+
+  if (!params.pubs || !isTriple(params.pubs)) {
+    throw new Error(`must provide xpub array`);
+  }
+
+  const keychains = params.pubs.map((pub) => bip32.fromBase58(pub)) as Triple<BIP32Interface>;
+  const cosignerPub = params.cosignerPub ?? params.pubs[2];
+  const cosignerKeychain = bip32.fromBase58(cosignerPub);
+
+  assert(signerKeychain);
+  const walletSigner = new bitgo.WalletUnspentSigner<RootWalletKeys>(keychains, signerKeychain, cosignerKeychain);
+  return signAndVerifyWalletTransaction(tx, params.txInfo.unspents, walletSigner, {
+    isLastSignature: params.isLastSignature,
+  }) as utxolib.bitgo.UtxoTransaction<TNumber>;
+}