Merge pull request #51 from BitGo/WP-5185

feat: raise error on no recipients eth musig rec and some refactors

Author: pranavjain97

src/api/enclaved/handlers/recoveryMultisigTransaction.ts

@@ -1,6 +1,8 @@
 import { SignFinalOptions } from '@bitgo/abstract-eth';
-import { HalfSignedUtxoTransaction, MethodNotImplementedError } from 'bitgo';
+import { AbstractUtxoCoin } from '@bitgo/abstract-utxo';
+import { HalfSignedUtxoTransaction, MethodNotImplementedError, TransactionRecipient } from 'bitgo';
 import { EnclavedApiSpecRouteRequest } from '../../../enclavedBitgoExpress/routers/enclavedApiSpec';
+import { EnvironmentName } from '../../../initConfig';
 import logger from '../../../logger';
 import {
   isEthLikeCoin,
@@ -9,12 +11,11 @@ import {
 } from '../../../shared/coinUtils';
 import {
   addEthLikeRecoveryExtras,
-  getDefaultMusigEthGasParams,
+  DEFAULT_MUSIG_ETH_GAS_PARAMS,
   getReplayProtectionOptions,
 } from '../../../shared/recoveryUtils';
 import { SignedEthLikeRecoveryTx } from '../../../types/transaction';
 import { retrieveKmsPrvKey } from '../utils';
-import { AbstractUtxoCoin } from '@bitgo/abstract-utxo';
 
 export async function recoveryMultisigTransaction(
   req: EnclavedApiSpecRouteRequest<'v1.multisig.recovery', 'post'>,
@@ -43,9 +44,13 @@ export async function recoveryMultisigTransaction(
       const walletKeys = unsignedSweepPrebuildTx.xpubxWithDerivationPath;
       const pubs = [walletKeys?.user?.xpub, walletKeys?.backup?.xpub, walletKeys?.bitgo?.xpub];
       const { gasPrice, gasLimit, maxFeePerGas, maxPriorityFeePerGas } =
-        getDefaultMusigEthGasParams();
+        DEFAULT_MUSIG_ETH_GAS_PARAMS;
 
       try {
+        checkIfNoRecipients({
+          recipients: unsignedSweepPrebuildTx.recipients,
+          coin: req.decoded.coin,
+        });
         const halfSignedTxBase = await baseCoin.signTransaction({
           isLastSignature: false,
           prv: userPrv,
@@ -61,6 +66,7 @@ export async function recoveryMultisigTransaction(
             maxPriorityFeePerGas,
           },
           replayProtectionOptions: getReplayProtectionOptions(
+            bitgo.env as EnvironmentName,
             unsignedSweepPrebuildTx.replayProtectionOptions,
           ),
           txPrebuild: {
@@ -72,13 +78,15 @@ export async function recoveryMultisigTransaction(
               maxPriorityFeePerGas,
             },
             replayProtectionOptions: getReplayProtectionOptions(
+              bitgo.env as EnvironmentName,
               unsignedSweepPrebuildTx.replayProtectionOptions,
             ),
           },
           walletContractAddress,
         });
 
         const halfSignedTx = addEthLikeRecoveryExtras({
+          env: bitgo.env as EnvironmentName,
           signedTx: halfSignedTxBase as SignedEthLikeRecoveryTx,
           transaction: unsignedSweepPrebuildTx,
           isLastSignature: false,
@@ -108,6 +116,7 @@ export async function recoveryMultisigTransaction(
               maxPriorityFeePerGas,
             },
             replayProtectionOptions: getReplayProtectionOptions(
+              bitgo.env as EnvironmentName,
               halfSignedTx?.replayProtectionOptions,
             ),
           } as unknown as SignFinalOptions,
@@ -159,3 +168,17 @@ export async function recoveryMultisigTransaction(
     throw new MethodNotImplementedError('Unsupported coin type for recovery: ' + baseCoin);
   }
 }
+
+function checkIfNoRecipients({
+  recipients,
+  coin,
+}: {
+  recipients?: TransactionRecipient[];
+  coin: string;
+}) {
+  if (!recipients || recipients.length === 0) {
+    const errorMsg = `Recovery tx for coin ${coin} must have at least one recipient.`;
+    logger.error(errorMsg);
+    throw new Error(errorMsg);
+  }
+}

src/api/master/handlers/recoveryWallet.ts

@@ -1,19 +1,20 @@
 import { BaseCoin, MethodNotImplementedError } from 'bitgo';
 
-import { AbstractUtxoCoin } from '@bitgo/abstract-utxo';
 import { AbstractEthLikeNewCoins } from '@bitgo/abstract-eth';
+import { AbstractUtxoCoin } from '@bitgo/abstract-utxo';
 
 import {
   isEthLikeCoin,
   isFormattedOfflineVaultTxInfo,
   isUtxoCoin,
 } from '../../../shared/coinUtils';
 import {
-  getDefaultMusigEthGasParams,
+  DEFAULT_MUSIG_ETH_GAS_PARAMS,
   getReplayProtectionOptions,
 } from '../../../shared/recoveryUtils';
-import { MasterApiSpecRouteRequest } from '../routers/masterApiSpec';
+import { EnvironmentName } from '../../../shared/types/index';
 import { EnclavedExpressClient } from '../clients/enclavedExpressClient';
+import { MasterApiSpecRouteRequest } from '../routers/masterApiSpec';
 
 interface RecoveryParams {
   userKey: string;
@@ -37,10 +38,10 @@ async function handleEthLikeRecovery(
   commonRecoveryParams: RecoveryParams,
   enclavedExpressClient: any,
   params: EnclavedRecoveryParams,
+  env: EnvironmentName,
 ) {
   try {
-    const { gasLimit, gasPrice, maxFeePerGas, maxPriorityFeePerGas } =
-      getDefaultMusigEthGasParams();
+    const { gasLimit, gasPrice, maxFeePerGas, maxPriorityFeePerGas } = DEFAULT_MUSIG_ETH_GAS_PARAMS;
     const unsignedSweepPrebuildTx = await (sdkCoin as AbstractEthLikeNewCoins).recover({
       ...commonRecoveryParams,
       gasPrice,
@@ -49,7 +50,7 @@ async function handleEthLikeRecovery(
         maxFeePerGas,
         maxPriorityFeePerGas,
       },
-      replayProtectionOptions: getReplayProtectionOptions(),
+      replayProtectionOptions: getReplayProtectionOptions(env),
     });
 
     const fullSignedRecoveryTx = await enclavedExpressClient.recoveryMultisig({
@@ -133,14 +134,20 @@ export async function handleRecoveryWalletOnPrem(
   }
 
   if (isEthLikeCoin(sdkCoin)) {
-    return handleEthLikeRecovery(sdkCoin, commonRecoveryParams, enclavedExpressClient, {
-      userPub,
-      backupPub,
-      apiKey,
-      unsignedSweepPrebuildTx: undefined,
-      coinSpecificParams: undefined,
-      walletContractAddress,
-    });
+    return handleEthLikeRecovery(
+      sdkCoin,
+      commonRecoveryParams,
+      enclavedExpressClient,
+      {
+        userPub,
+        backupPub,
+        apiKey,
+        unsignedSweepPrebuildTx: undefined,
+        coinSpecificParams: undefined,
+        walletContractAddress,
+      },
+      bitgo.env as EnvironmentName,
+    );
   }
   if (!bitgoPub) {
     throw new Error('BitGo public key is required for recovery');

src/shared/recoveryUtils.ts

@@ -1,11 +1,14 @@
+import { type EnvironmentName } from '../shared/types/index';
 import type { ReplayProtectionOptions, SignedEthLikeRecoveryTx } from '../types/transaction';
 
 export function addEthLikeRecoveryExtras({
+  env,
   signedTx,
   transaction,
   isLastSignature,
   replayProtectionOptions,
 }: {
+  env: EnvironmentName;
   signedTx: SignedEthLikeRecoveryTx;
   transaction: any; // Same type as UnsignedSweepPrebuildTx
   isLastSignature: boolean;
@@ -50,28 +53,30 @@ export function addEthLikeRecoveryExtras({
         ? transaction.backupKeyNonce
         : transaction.nextContractSequenceId;
     decoratedSignedTx.walletContractAddress = transaction.walletContractAddress;
-    decoratedSignedTx.replayProtectionOptions = getReplayProtectionOptions(replayProtectionOptions);
+    decoratedSignedTx.replayProtectionOptions = getReplayProtectionOptions(
+      env,
+      replayProtectionOptions,
+    );
   }
 
   return decoratedSignedTx;
 }
 
 export function getReplayProtectionOptions(
+  env: EnvironmentName,
   replayProtectionOptions: ReplayProtectionOptions | undefined = undefined,
 ): ReplayProtectionOptions {
   return (
     replayProtectionOptions ?? {
-      chain: 17000, // 1 if mainnet, 17000 if testnet
+      chain: env === 'prod' ? 1 : 17000,
       hardfork: 'london',
     }
   );
 }
 
-export function getDefaultMusigEthGasParams() {
-  return {
-    gasPrice: 20000000000,
-    gasLimit: 200000,
-    maxFeePerGas: 20000000000,
-    maxPriorityFeePerGas: 10000000000,
-  };
-}
+export const DEFAULT_MUSIG_ETH_GAS_PARAMS = {
+  gasPrice: 20000000000,
+  gasLimit: 200000,
+  maxFeePerGas: 20000000000,
+  maxPriorityFeePerGas: 10000000000,
+};