Merge pull request #5254 from BitGo/BTC-1450.guard-descriptor-wallets.2

feat(abstract-utxo): guard descriptor wallets

Author: OttoAllmendinger

modules/abstract-utxo/src/abstractUtxoCoin.ts

@@ -66,19 +66,20 @@ import { supportedCrossChainRecoveries } from './config';
 import {
   assertValidTransactionRecipient,
   explainTx,
+  parseTransaction,
+  verifyTransaction,
   fromExtendedAddressFormat,
   isScriptRecipient,
 } from './transaction';
-import { assertDescriptorWalletAddress } from './descriptor';
+import { assertDescriptorWalletAddress, getDescriptorMapFromWallet, isDescriptorWallet } from './descriptor';
 
 import { getChainFromNetwork, getFamilyFromNetwork, getFullNameFromNetwork } from './names';
-import { CustomChangeOptions, parseTransaction } from './transaction/fixedScript';
+import { CustomChangeOptions } from './transaction/fixedScript';
 import { NamedKeychains } from './keychains';
 
 const debug = debugLib('bitgo:v2:utxo');
 
 import ScriptType2Of3 = utxolib.bitgo.outputScripts.ScriptType2Of3;
-import { verifyTransaction } from './transaction/fixedScript/verifyTransaction';
 import { verifyKeySignature, verifyUserPublicKey } from './verifyKey';
 
 type UtxoCustomSigningFunction<TNumber extends number | bigint> = {
@@ -107,8 +108,13 @@ type UtxoCustomSigningFunction<TNumber extends number | bigint> = {
 };
 
 const { getExternalChainCode, isChainCode, scriptTypeForChain, outputScripts } = bitgo;
+
 type Unspent<TNumber extends number | bigint = number> = bitgo.Unspent<TNumber>;
 
+type DecodedTransaction<TNumber extends number | bigint> =
+  | utxolib.bitgo.UtxoTransaction<TNumber>
+  | utxolib.bitgo.UtxoPsbt;
+
 type RootWalletKeys = bitgo.RootWalletKeys;
 
 export type UtxoCoinSpecific = AddressCoinSpecific | DescriptorAddressCoinSpecific;
@@ -516,7 +522,7 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
     if (_.isUndefined(prebuild.txHex)) {
       throw new Error('missing required txPrebuild property txHex');
     }
-    const tx = this.decodeTransaction(prebuild.txHex);
+    const tx = this.decodeTransactionFromPrebuild(prebuild);
     if (_.isUndefined(prebuild.blockHeight)) {
       prebuild.blockHeight = (await this.getLatestBlockHeight()) as number;
     }
@@ -559,9 +565,7 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
     return utxolib.bitgo.createTransactionFromHex<TNumber>(hex, this.network, this.amountType);
   }
 
-  decodeTransaction<TNumber extends number | bigint>(
-    input: Buffer | string
-  ): utxolib.bitgo.UtxoTransaction<TNumber> | utxolib.bitgo.UtxoPsbt {
+  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);
@@ -586,6 +590,17 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
     }
   }
 
+  decodeTransactionFromPrebuild<TNumber extends number | bigint>(prebuild: {
+    txHex?: string;
+    txBase64?: string;
+  }): DecodedTransaction<TNumber> {
+    const string = prebuild.txHex ?? prebuild.txBase64;
+    if (!string) {
+      throw new Error('missing required txHex or txBase64 property');
+    }
+    return this.decodeTransaction(string);
+  }
+
   toCanonicalTransactionRecipient(output: { valueString: string; address?: string }): {
     amount: bigint;
     address?: string;
@@ -662,12 +677,9 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
       throw new InvalidAddressError(`invalid address: ${address}`);
     }
 
-    if (wallet) {
-      const walletCoinSpecific = wallet.coinSpecific();
-      if (walletCoinSpecific && 'descriptors' in walletCoinSpecific) {
-        assertDescriptorWalletAddress(this.network, params, walletCoinSpecific.descriptors);
-        return true;
-      }
+    if (wallet && isDescriptorWallet(wallet)) {
+      assertDescriptorWalletAddress(this.network, params, getDescriptorMapFromWallet(wallet));
+      return true;
     }
 
     if ((_.isUndefined(chain) && _.isUndefined(index)) || !(_.isFinite(chain) && _.isFinite(index))) {
@@ -857,7 +869,7 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
       throw new Error('missing txPrebuild parameter');
     }
 
-    let tx = this.decodeTransaction(params.txPrebuild.txHex);
+    let tx = this.decodeTransactionFromPrebuild(params.txPrebuild);
 
     const isTxWithKeyPathSpendInput = tx instanceof bitgo.UtxoPsbt && bitgo.isTransactionWithKeyPathSpendInput(tx);
 
@@ -1074,11 +1086,7 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
   async explainTransaction<TNumber extends number | bigint = number>(
     params: ExplainTransactionOptions<TNumber>
   ): Promise<TransactionExplanation> {
-    const { txHex } = params;
-    if (typeof txHex !== 'string' || !txHex.match(/^([a-f0-9]{2})+$/i)) {
-      throw new Error('invalid transaction hex, must be a valid hex string');
-    }
-    return explainTx(this.decodeTransaction(txHex), params, this.network);
+    return explainTx(this.decodeTransactionFromPrebuild(params), params, this.network);
   }
 
   /**

modules/abstract-utxo/src/descriptor/assertDescriptorWalletAddress.ts

@@ -1,10 +1,9 @@
 import assert from 'assert';
-import * as t from 'io-ts';
 import * as utxolib from '@bitgo/utxo-lib';
 import { Descriptor } from '@bitgo/wasm-miniscript';
 
 import { UtxoCoinSpecific, VerifyAddressOptions } from '../abstractUtxoCoin';
-import { NamedDescriptor } from './NamedDescriptor';
+import { DescriptorMap } from '../core/descriptor';
 
 class DescriptorAddressMismatchError extends Error {
   constructor(descriptor: Descriptor, index: number, derivedAddress: string, expectedAddress: string) {
@@ -17,19 +16,16 @@ class DescriptorAddressMismatchError extends Error {
 export function assertDescriptorWalletAddress(
   network: utxolib.Network,
   params: VerifyAddressOptions<UtxoCoinSpecific>,
-  descriptors: unknown
+  descriptors: DescriptorMap
 ): void {
   assert(params.coinSpecific);
-  assert(t.array(NamedDescriptor).is(descriptors));
   assert('descriptorName' in params.coinSpecific);
   assert('descriptorChecksum' in params.coinSpecific);
-  const descriptorName = params.coinSpecific.descriptorName;
-  const descriptorChecksum = params.coinSpecific.descriptorChecksum;
-  const namedDescriptor = descriptors.find((d) => d.name === descriptorName);
-  if (!namedDescriptor) {
+  const { descriptorName, descriptorChecksum } = params.coinSpecific;
+  const descriptor = descriptors.get(params.coinSpecific.descriptorName);
+  if (!descriptor) {
     throw new Error(`Descriptor ${descriptorName} not found`);
   }
-  const descriptor = Descriptor.fromString(namedDescriptor.value, 'derivable');
   const checksum = descriptor.toString().slice(-8);
   if (checksum !== descriptorChecksum) {
     throw new Error(

modules/abstract-utxo/src/descriptor/descriptorWallet.ts

@@ -2,20 +2,38 @@ import * as t from 'io-ts';
 import { NamedDescriptor } from './NamedDescriptor';
 import { AbstractUtxoCoinWalletData } from '../abstractUtxoCoin';
 import { DescriptorMap, toDescriptorMap } from '../core/descriptor';
+import { IWallet, WalletCoinSpecific } from '@bitgo/sdk-core';
+
+type DescriptorWalletCoinSpecific = {
+  descriptors: NamedDescriptor[];
+};
+
+function isDescriptorWalletCoinSpecific(obj: unknown): obj is DescriptorWalletCoinSpecific {
+  return (
+    obj !== null && typeof obj === 'object' && 'descriptors' in obj && t.array(NamedDescriptor).is(obj.descriptors)
+  );
+}
 
 type DescriptorWalletData = AbstractUtxoCoinWalletData & {
-  coinSpecific: {
-    descriptors: NamedDescriptor[];
-  };
+  coinSpecific: DescriptorWalletCoinSpecific;
 };
 
+interface IDescriptorWallet extends IWallet {
+  coinSpecific(): WalletCoinSpecific & DescriptorWalletCoinSpecific;
+}
+
 export function isDescriptorWalletData(obj: AbstractUtxoCoinWalletData): obj is DescriptorWalletData {
-  if ('coinSpecific' in obj && 'descriptors' in obj.coinSpecific) {
-    return t.array(NamedDescriptor).is(obj.coinSpecific.descriptors);
-  }
-  return false;
+  return isDescriptorWalletCoinSpecific(obj.coinSpecific);
+}
+
+export function isDescriptorWallet(obj: IWallet): obj is IDescriptorWallet {
+  return isDescriptorWalletCoinSpecific(obj.coinSpecific());
 }
 
 export function getDescriptorMapFromWalletData(wallet: DescriptorWalletData): DescriptorMap {
   return toDescriptorMap(wallet.coinSpecific.descriptors);
 }
+
+export function getDescriptorMapFromWallet(wallet: IDescriptorWallet): DescriptorMap {
+  return toDescriptorMap(wallet.coinSpecific().descriptors);
+}

modules/abstract-utxo/src/descriptor/index.ts

@@ -1,4 +1,9 @@
 export { Miniscript, Descriptor } from '@bitgo/wasm-miniscript';
 export { assertDescriptorWalletAddress } from './assertDescriptorWalletAddress';
 export { NamedDescriptor } from './NamedDescriptor';
-export { isDescriptorWalletData, getDescriptorMapFromWalletData } from './descriptorWallet';
+export {
+  isDescriptorWallet,
+  isDescriptorWalletData,
+  getDescriptorMapFromWallet,
+  getDescriptorMapFromWalletData,
+} from './descriptorWallet';

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

@@ -4,19 +4,26 @@ import { TransactionExplanation } from '../abstractUtxoCoin';
 
 import * as fixedScript from './fixedScript';
 
+import { IWallet } from '@bitgo/sdk-core';
+import { isDescriptorWallet } from '../descriptor';
+
 /**
  * Decompose a raw transaction into useful information, such as the total amounts,
  * change amounts, and transaction outputs.
  */
 export function explainTx<TNumber extends number | bigint>(
   tx: utxolib.bitgo.UtxoTransaction<TNumber> | utxolib.bitgo.UtxoPsbt,
   params: {
+    wallet?: IWallet;
     pubs?: string[];
     txInfo?: { unspents?: utxolib.bitgo.Unspent<TNumber>[] };
     changeInfo?: fixedScript.ChangeAddressInfo[];
   },
   network: utxolib.Network
 ): TransactionExplanation {
+  if (params.wallet && isDescriptorWallet(params.wallet)) {
+    throw new Error('Descriptor wallets are not supported');
+  }
   if (tx instanceof utxolib.bitgo.UtxoPsbt) {
     return fixedScript.explainPsbt(tx, params, network);
   } else {

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

@@ -1,3 +1,4 @@
 export { explainPsbt, explainLegacyTx, ChangeAddressInfo } from './explainTransaction';
 export { parseTransaction } from './parseTransaction';
+export { verifyTransaction } from './verifyTransaction';
 export { CustomChangeOptions } from './parseOutput';

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

@@ -1,3 +1,5 @@
 export * from './recipient';
 export { explainTx } from './explainTransaction';
+export { parseTransaction } from './parseTransaction';
+export { verifyTransaction } from './verifyTransaction';
 export * from './fetchInputs';

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

@@ -0,0 +1,16 @@
+import { AbstractUtxoCoin, ParsedTransaction, ParseTransactionOptions } from '../abstractUtxoCoin';
+
+import { isDescriptorWallet } from '../descriptor';
+
+import * as fixedScript from './fixedScript';
+
+export async function parseTransaction<TNumber extends bigint | number>(
+  coin: AbstractUtxoCoin,
+  params: ParseTransactionOptions<TNumber>
+): Promise<ParsedTransaction<TNumber>> {
+  if (isDescriptorWallet(params.wallet)) {
+    throw new Error('Descriptor wallets are not supported');
+  } else {
+    return fixedScript.parseTransaction(coin, params);
+  }
+}

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

@@ -0,0 +1,18 @@
+import { BitGoBase } from '@bitgo/sdk-core';
+
+import { AbstractUtxoCoin, VerifyTransactionOptions } from '../abstractUtxoCoin';
+import { isDescriptorWallet } from '../descriptor';
+
+import * as fixedScript from './fixedScript';
+
+export async function verifyTransaction<TNumber extends bigint | number>(
+  coin: AbstractUtxoCoin,
+  bitgo: BitGoBase,
+  params: VerifyTransactionOptions<TNumber>
+): Promise<boolean> {
+  if (isDescriptorWallet(params.wallet)) {
+    throw new Error('Descriptor wallets are not supported');
+  } else {
+    return fixedScript.verifyTransaction(coin, bitgo, params);
+  }
+}