Merge pull request #7236 from BitGo/WP-6189/introduce-suspicious-transaction-errors

feat: Replace Generic Errors with Suspicious Transaction Errors for UTXO/EthLike Coins

Author: danielpeng1

modules/abstract-eth/src/abstractEthLikeNewCoins.ts

@@ -28,6 +28,8 @@ import {
   PresignTransactionOptions as BasePresignTransactionOptions,
   Recipient,
   SignTransactionOptions as BaseSignTransactionOptions,
+  TxIntentMismatchError,
+  TxIntentMismatchRecipientError,
   TransactionParams,
   TransactionPrebuild as BaseTransactionPrebuild,
   TransactionRecipient,
@@ -2799,23 +2801,30 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
    * @param {TransactionPrebuild} params.txPrebuild - prebuild object returned by server
    * @param {Wallet} params.wallet - Wallet object to obtain keys to verify against
    * @returns {boolean}
+   * @throws {TxIntentMismatchRecipientError} if transaction recipients don't match user intent
    */
   async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
     const { txParams, txPrebuild, wallet } = params;
+
+    // Helper to throw TxIntentMismatchRecipientError with recipient details
+    const throwRecipientMismatch = (message: string, mismatchedRecipients: Recipient[]): never => {
+      throw new TxIntentMismatchRecipientError(message, undefined, [txParams], txPrebuild?.txHex, mismatchedRecipients);
+    };
+
     if (
       !txParams?.recipients &&
       !(
         txParams.prebuildTx?.consolidateId ||
         (txParams.type && ['acceleration', 'fillNonce', 'transferToken', 'tokenApproval'].includes(txParams.type))
       )
     ) {
-      throw new Error(`missing txParams`);
+      throw new Error('missing txParams');
     }
     if (!wallet || !txPrebuild) {
-      throw new Error(`missing params`);
+      throw new Error('missing params');
     }
     if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) {
-      throw new Error(`tx cannot be both a batch and hop transaction`);
+      throw new Error('tx cannot be both a batch and hop transaction');
     }
 
     if (txParams.type && ['transfer'].includes(txParams.type)) {
@@ -2830,21 +2839,29 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
         const txJson = tx.toJson();
         if (txJson.data === '0x') {
           if (expectedAmount !== txJson.value) {
-            throw new Error('the transaction amount in txPrebuild does not match the value given by client');
+            throwRecipientMismatch('the transaction amount in txPrebuild does not match the value given by client', [
+              { address: txJson.to, amount: txJson.value },
+            ]);
           }
           if (expectedDestination.toLowerCase() !== txJson.to.toLowerCase()) {
-            throw new Error('destination address does not match with the recipient address');
+            throwRecipientMismatch('destination address does not match with the recipient address', [
+              { address: txJson.to, amount: txJson.value },
+            ]);
           }
         } else if (txJson.data.startsWith('0xa9059cbb')) {
           const [recipientAddress, amount] = getRawDecoded(
             ['address', 'uint256'],
             getBufferedByteCode('0xa9059cbb', txJson.data)
           );
           if (expectedAmount !== amount.toString()) {
-            throw new Error('the transaction amount in txPrebuild does not match the value given by client');
+            throwRecipientMismatch('the transaction amount in txPrebuild does not match the value given by client', [
+              { address: addHexPrefix(recipientAddress.toString()), amount: amount.toString() },
+            ]);
           }
           if (expectedDestination.toLowerCase() !== addHexPrefix(recipientAddress.toString()).toLowerCase()) {
-            throw new Error('destination address does not match with the recipient address');
+            throwRecipientMismatch('destination address does not match with the recipient address', [
+              { address: addHexPrefix(recipientAddress.toString()), amount: amount.toString() },
+            ]);
           }
         }
       }
@@ -2861,6 +2878,8 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
    * @param {TransactionPrebuild} params.txPrebuild - prebuild object returned by server
    * @param {Wallet} params.wallet - Wallet object to obtain keys to verify against
    * @returns {boolean}
+   * @throws {TxIntentMismatchError} if transaction validation fails
+   * @throws {TxIntentMismatchRecipientError} if transaction recipients don't match user intent
    */
   async verifyTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
     const ethNetwork = this.getNetwork();
@@ -2870,11 +2889,19 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
       return this.verifyTssTransaction(params);
     }
 
+    // Helper to throw TxIntentMismatchRecipientError with recipient details
+    const throwRecipientMismatch = (message: string, mismatchedRecipients: Recipient[]): never => {
+      throw new TxIntentMismatchRecipientError(message, undefined, [txParams], txPrebuild?.txHex, mismatchedRecipients);
+    };
+
     if (!txParams?.recipients || !txPrebuild?.recipients || !wallet) {
-      throw new Error(`missing params`);
+      throw new Error('missing params');
     }
-    if (txParams.hop && txParams.recipients.length > 1) {
-      throw new Error(`tx cannot be both a batch and hop transaction`);
+
+    const recipients = txParams.recipients!;
+
+    if (txParams.hop && recipients.length > 1) {
+      throw new Error('tx cannot be both a batch and hop transaction');
     }
     if (txPrebuild.recipients.length > 1) {
       throw new Error(
@@ -2883,8 +2910,8 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
     }
     if (txParams.hop && txPrebuild.hopTransaction) {
       // Check recipient amount for hop transaction
-      if (txParams.recipients.length !== 1) {
-        throw new Error(`hop transaction only supports 1 recipient but ${txParams.recipients.length} found`);
+      if (recipients.length !== 1) {
+        throw new Error(`hop transaction only supports 1 recipient but ${recipients.length} found`);
       }
 
       // Check tx sends to hop address
@@ -2894,34 +2921,39 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
       const expectedHopAddress = optionalDeps.ethUtil.stripHexPrefix(decodedHopTx.getSenderAddress().toString());
       const actualHopAddress = optionalDeps.ethUtil.stripHexPrefix(txPrebuild.recipients[0].address);
       if (expectedHopAddress.toLowerCase() !== actualHopAddress.toLowerCase()) {
-        throw new Error('recipient address of txPrebuild does not match hop address');
+        throwRecipientMismatch('recipient address of txPrebuild does not match hop address', [
+          { address: txPrebuild.recipients[0].address, amount: txPrebuild.recipients[0].amount.toString() },
+        ]);
       }
 
       // Convert TransactionRecipient array to Recipient array
-      const recipients: Recipient[] = txParams.recipients.map((r) => {
+      const hopRecipients: Recipient[] = recipients.map((r) => {
         return {
           address: r.address,
           amount: typeof r.amount === 'number' ? r.amount.toString() : r.amount,
         };
       });
 
       // Check destination address and amount
-      await this.validateHopPrebuild(wallet, txPrebuild.hopTransaction, { recipients });
-    } else if (txParams.recipients.length > 1) {
+      await this.validateHopPrebuild(wallet, txPrebuild.hopTransaction, { recipients: hopRecipients });
+    } else if (recipients.length > 1) {
       // Check total amount for batch transaction
       if (txParams.tokenName) {
         const expectedTotalAmount = new BigNumber(0);
         if (!expectedTotalAmount.isEqualTo(txPrebuild.recipients[0].amount)) {
-          throw new Error('batch token transaction amount in txPrebuild should be zero for token transfers');
+          throwRecipientMismatch('batch token transaction amount in txPrebuild should be zero for token transfers', [
+            { address: txPrebuild.recipients[0].address, amount: txPrebuild.recipients[0].amount.toString() },
+          ]);
         }
       } else {
         let expectedTotalAmount = new BigNumber(0);
-        for (let i = 0; i < txParams.recipients.length; i++) {
-          expectedTotalAmount = expectedTotalAmount.plus(txParams.recipients[i].amount);
+        for (let i = 0; i < recipients.length; i++) {
+          expectedTotalAmount = expectedTotalAmount.plus(recipients[i].amount);
         }
         if (!expectedTotalAmount.isEqualTo(txPrebuild.recipients[0].amount)) {
-          throw new Error(
-            'batch transaction amount in txPrebuild received from BitGo servers does not match txParams supplied by client'
+          throwRecipientMismatch(
+            'batch transaction amount in txPrebuild received from BitGo servers does not match txParams supplied by client',
+            [{ address: txPrebuild.recipients[0].address, amount: txPrebuild.recipients[0].amount.toString() }]
           );
         }
       }
@@ -2932,29 +2964,37 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
         !batcherContractAddress ||
         batcherContractAddress.toLowerCase() !== txPrebuild.recipients[0].address.toLowerCase()
       ) {
-        throw new Error('recipient address of txPrebuild does not match batcher address');
+        throwRecipientMismatch('recipient address of txPrebuild does not match batcher address', [
+          { address: txPrebuild.recipients[0].address, amount: txPrebuild.recipients[0].amount.toString() },
+        ]);
       }
     } else {
       // Check recipient address and amount for normal transaction
-      if (txParams.recipients.length !== 1) {
-        throw new Error(`normal transaction only supports 1 recipient but ${txParams.recipients.length} found`);
+      if (recipients.length !== 1) {
+        throw new Error(`normal transaction only supports 1 recipient but ${recipients.length} found`);
       }
-      const expectedAmount = new BigNumber(txParams.recipients[0].amount);
+      const expectedAmount = new BigNumber(recipients[0].amount);
       if (!expectedAmount.isEqualTo(txPrebuild.recipients[0].amount)) {
-        throw new Error(
-          'normal transaction amount in txPrebuild received from BitGo servers does not match txParams supplied by client'
+        throwRecipientMismatch(
+          'normal transaction amount in txPrebuild received from BitGo servers does not match txParams supplied by client',
+          [{ address: txPrebuild.recipients[0].address, amount: txPrebuild.recipients[0].amount.toString() }]
         );
       }
-      if (
-        this.isETHAddress(txParams.recipients[0].address) &&
-        txParams.recipients[0].address !== txPrebuild.recipients[0].address
-      ) {
-        throw new Error('destination address in normal txPrebuild does not match that in txParams supplied by client');
+      if (this.isETHAddress(recipients[0].address) && recipients[0].address !== txPrebuild.recipients[0].address) {
+        throwRecipientMismatch(
+          'destination address in normal txPrebuild does not match that in txParams supplied by client',
+          [{ address: txPrebuild.recipients[0].address, amount: txPrebuild.recipients[0].amount.toString() }]
+        );
       }
     }
     // Check coin is correct for all transaction types
     if (!this.verifyCoin(txPrebuild)) {
-      throw new Error(`coin in txPrebuild did not match that in txParams supplied by client`);
+      throw new TxIntentMismatchError(
+        'coin in txPrebuild did not match that in txParams supplied by client',
+        undefined,
+        [txParams],
+        txPrebuild?.txHex
+      );
     }
     return true;
   }

modules/abstract-utxo/src/abstractUtxoCoin.ts

@@ -22,6 +22,7 @@ import {
   IWallet,
   KeychainsTriplet,
   KeyIndices,
+  MismatchedRecipient,
   MultisigType,
   multisigTypes,
   P2shP2wshUnsupportedError,
@@ -39,6 +40,7 @@ import {
   TransactionParams as BaseTransactionParams,
   TransactionPrebuild as BaseTransactionPrebuild,
   Triple,
+  TxIntentMismatchRecipientError,
   UnexpectedAddressError,
   UnsupportedAddressTypeError,
   VerificationOptions,
@@ -72,6 +74,11 @@ import {
   parseTransaction,
   verifyTransaction,
 } from './transaction';
+import {
+  AggregateValidationError,
+  ErrorMissingOutputs,
+  ErrorImplicitExternalOutputs,
+} from './transaction/descriptor/verifyTransaction';
 import { assertDescriptorWalletAddress, getDescriptorMapFromWallet, isDescriptorWallet } from './descriptor';
 import { getChainFromNetwork, getFamilyFromNetwork, getFullNameFromNetwork } from './names';
 import { CustomChangeOptions } from './transaction/fixedScript';
@@ -113,6 +120,52 @@ const { getExternalChainCode, isChainCode, scriptTypeForChain, outputScripts } =
 
 type Unspent<TNumber extends number | bigint = number> = bitgo.Unspent<TNumber>;
 
+/**
+ * Convert ValidationError to TxIntentMismatchRecipientError with structured data
+ *
+ * This preserves the structured error information from the original ValidationError
+ * by extracting the mismatched outputs and converting them to the standardized format.
+ * The original error is preserved as the `cause` for debugging purposes.
+ */
+function convertValidationErrorToTxIntentMismatch(
+  error: AggregateValidationError,
+  reqId: string | IRequestTracer | undefined,
+  txParams: BaseTransactionParams,
+  txHex: string | undefined
+): TxIntentMismatchRecipientError {
+  const mismatchedRecipients: MismatchedRecipient[] = [];
+
+  for (const err of error.errors) {
+    if (err instanceof ErrorMissingOutputs) {
+      mismatchedRecipients.push(
+        ...err.missingOutputs.map((output) => ({
+          address: output.address,
+          amount: output.amount.toString(),
+        }))
+      );
+    } else if (err instanceof ErrorImplicitExternalOutputs) {
+      mismatchedRecipients.push(
+        ...err.implicitExternalOutputs.map((output) => ({
+          address: output.address,
+          amount: output.amount.toString(),
+        }))
+      );
+    }
+  }
+
+  const txIntentError = new TxIntentMismatchRecipientError(
+    error.message,
+    reqId,
+    [txParams],
+    txHex,
+    mismatchedRecipients
+  );
+  // Preserve the original structured error as the cause for debugging
+  // See: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause
+  (txIntentError as Error & { cause?: Error }).cause = error;
+  return txIntentError;
+}
+
 export type DecodedTransaction<TNumber extends number | bigint> =
   | utxolib.bitgo.UtxoTransaction<TNumber>
   | utxolib.bitgo.UtxoPsbt;
@@ -631,12 +684,21 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
    * @param params.verification.disableNetworking Disallow fetching any data from the internet for verification purposes
    * @param params.verification.keychains Pass keychains manually rather than fetching them by id
    * @param params.verification.addresses Address details to pass in for out-of-band verification
-   * @returns {boolean}
+   * @returns {boolean} True if verification passes
+   * @throws {TxIntentMismatchError} if transaction validation fails
+   * @throws {TxIntentMismatchRecipientError} if transaction recipients don't match user intent
    */
   async verifyTransaction<TNumber extends number | bigint = number>(
     params: VerifyTransactionOptions<TNumber>
   ): Promise<boolean> {
-    return verifyTransaction(this, this.bitgo, params);
+    try {
+      return await verifyTransaction(this, this.bitgo, params);
+    } catch (error) {
+      if (error instanceof AggregateValidationError) {
+        throw convertValidationErrorToTxIntentMismatch(error, params.reqId, params.txParams, params.txPrebuild.txHex);
+      }
+      throw error;
+    }
   }
 
   /**

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

@@ -1,8 +1,13 @@
 import * as utxolib from '@bitgo/utxo-lib';
-import { ITransactionRecipient, VerifyTransactionOptions } from '@bitgo/sdk-core';
+import { ITransactionRecipient, TxIntentMismatchError } from '@bitgo/sdk-core';
 import { DescriptorMap } from '@bitgo/utxo-core/descriptor';
 
-import { AbstractUtxoCoin, BaseOutput, BaseParsedTransactionOutputs } from '../../abstractUtxoCoin';
+import {
+  AbstractUtxoCoin,
+  BaseOutput,
+  BaseParsedTransactionOutputs,
+  VerifyTransactionOptions,
+} from '../../abstractUtxoCoin';
 
 import { toBaseParsedTransactionOutputsFromPsbt } from './parse';
 
@@ -66,16 +71,25 @@ export function assertValidTransaction(
  * @param coin
  * @param params
  * @param descriptorMap
+ * @returns {boolean} True if verification passes
+ * @throws {TxIntentMismatchError} if transaction validation fails
  */
-export async function verifyTransaction(
+export async function verifyTransaction<TNumber extends number | bigint>(
   coin: AbstractUtxoCoin,
-  params: VerifyTransactionOptions,
+  params: VerifyTransactionOptions<TNumber>,
   descriptorMap: DescriptorMap
 ): Promise<boolean> {
   const tx = coin.decodeTransactionFromPrebuild(params.txPrebuild);
   if (!(tx instanceof utxolib.bitgo.UtxoPsbt)) {
-    throw new Error('unexpected transaction type');
+    throw new TxIntentMismatchError(
+      'unexpected transaction type',
+      params.reqId,
+      [params.txParams],
+      params.txPrebuild.txHex
+    );
   }
+
   assertValidTransaction(tx, descriptorMap, params.txParams.recipients ?? [], tx.network);
+
   return true;
 }