refactor: TxIntentMismatchError across transaction verification methods

Ticket: WP-6189

Author: danielpeng1

modules/abstract-eth/src/abstractEthLikeNewCoins.ts

@@ -28,7 +28,7 @@ import {
   PresignTransactionOptions as BasePresignTransactionOptions,
   Recipient,
   SignTransactionOptions as BaseSignTransactionOptions,
-  SuspiciousTransactionError,
+  TxIntentMismatchError,
   TransactionParams,
   TransactionPrebuild as BaseTransactionPrebuild,
   TransactionRecipient,
@@ -2768,6 +2768,7 @@ 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
    */
   async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
     const { txParams, txPrebuild, wallet } = params;
@@ -2778,13 +2779,13 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
         (txParams.type && ['acceleration', 'fillNonce', 'transferToken', 'tokenApproval'].includes(txParams.type))
       )
     ) {
-      throw new SuspiciousTransactionError(`missing txParams`);
+      throw new TxIntentMismatchError(`missing txParams`, '', [], '');
     }
     if (!wallet || !txPrebuild) {
-      throw new SuspiciousTransactionError(`missing params`);
+      throw new TxIntentMismatchError(`missing params`, '', [], '');
     }
     if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) {
-      throw new SuspiciousTransactionError(`tx cannot be both a batch and hop transaction`);
+      throw new TxIntentMismatchError(`tx cannot be both a batch and hop transaction`, '', [], '');
     }
 
     if (txParams.type && ['transfer'].includes(txParams.type)) {
@@ -2799,25 +2800,41 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
         const txJson = tx.toJson();
         if (txJson.data === '0x') {
           if (expectedAmount !== txJson.value) {
-            throw new SuspiciousTransactionError(
-              'the transaction amount in txPrebuild does not match the value given by client'
+            throw new TxIntentMismatchError(
+              'the transaction amount in txPrebuild does not match the value given by client',
+              '',
+              [],
+              ''
             );
           }
           if (expectedDestination.toLowerCase() !== txJson.to.toLowerCase()) {
-            throw new SuspiciousTransactionError('destination address does not match with the recipient address');
+            throw new TxIntentMismatchError(
+              'destination address does not match with the recipient address',
+              '',
+              [],
+              ''
+            );
           }
         } else if (txJson.data.startsWith('0xa9059cbb')) {
           const [recipientAddress, amount] = getRawDecoded(
             ['address', 'uint256'],
             getBufferedByteCode('0xa9059cbb', txJson.data)
           );
           if (expectedAmount !== amount.toString()) {
-            throw new SuspiciousTransactionError(
-              'the transaction amount in txPrebuild does not match the value given by client'
+            throw new TxIntentMismatchError(
+              'the transaction amount in txPrebuild does not match the value given by client',
+              '',
+              [],
+              ''
             );
           }
           if (expectedDestination.toLowerCase() !== addHexPrefix(recipientAddress.toString()).toLowerCase()) {
-            throw new SuspiciousTransactionError('destination address does not match with the recipient address');
+            throw new TxIntentMismatchError(
+              'destination address does not match with the recipient address',
+              '',
+              [],
+              ''
+            );
           }
         }
       }
@@ -2834,6 +2851,7 @@ 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
    */
   async verifyTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
     const ethNetwork = this.getNetwork();
@@ -2844,21 +2862,27 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
     }
 
     if (!txParams?.recipients || !txPrebuild?.recipients || !wallet) {
-      throw new SuspiciousTransactionError(`missing params`);
+      throw new TxIntentMismatchError(`missing params`, '', [], '');
     }
     if (txParams.hop && txParams.recipients.length > 1) {
-      throw new SuspiciousTransactionError(`tx cannot be both a batch and hop transaction`);
+      throw new TxIntentMismatchError(`tx cannot be both a batch and hop transaction`, '', [], '');
     }
     if (txPrebuild.recipients.length > 1) {
-      throw new SuspiciousTransactionError(
-        `${this.getChain()} doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.`
+      throw new TxIntentMismatchError(
+        `${this.getChain()} doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.`,
+        '',
+        [],
+        ''
       );
     }
     if (txParams.hop && txPrebuild.hopTransaction) {
       // Check recipient amount for hop transaction
       if (txParams.recipients.length !== 1) {
-        throw new SuspiciousTransactionError(
-          `hop transaction only supports 1 recipient but ${txParams.recipients.length} found`
+        throw new TxIntentMismatchError(
+          `hop transaction only supports 1 recipient but ${txParams.recipients.length} found`,
+          '',
+          [],
+          ''
         );
       }
 
@@ -2869,7 +2893,7 @@ 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 SuspiciousTransactionError('recipient address of txPrebuild does not match hop address');
+        throw new TxIntentMismatchError('recipient address of txPrebuild does not match hop address', '', [], '');
       }
 
       // Convert TransactionRecipient array to Recipient array
@@ -2887,8 +2911,11 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
       if (txParams.tokenName) {
         const expectedTotalAmount = new BigNumber(0);
         if (!expectedTotalAmount.isEqualTo(txPrebuild.recipients[0].amount)) {
-          throw new SuspiciousTransactionError(
-            'batch token transaction amount in txPrebuild should be zero for token transfers'
+          throw new TxIntentMismatchError(
+            'batch token transaction amount in txPrebuild should be zero for token transfers',
+            '',
+            [],
+            ''
           );
         }
       } else {
@@ -2897,8 +2924,11 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
           expectedTotalAmount = expectedTotalAmount.plus(txParams.recipients[i].amount);
         }
         if (!expectedTotalAmount.isEqualTo(txPrebuild.recipients[0].amount)) {
-          throw new SuspiciousTransactionError(
-            'batch transaction amount in txPrebuild received from BitGo servers does not match txParams supplied by client'
+          throw new TxIntentMismatchError(
+            'batch transaction amount in txPrebuild received from BitGo servers does not match txParams supplied by client',
+            '',
+            [],
+            ''
           );
         }
       }
@@ -2909,33 +2939,47 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
         !batcherContractAddress ||
         batcherContractAddress.toLowerCase() !== txPrebuild.recipients[0].address.toLowerCase()
       ) {
-        throw new SuspiciousTransactionError('recipient address of txPrebuild does not match batcher address');
+        throw new TxIntentMismatchError('recipient address of txPrebuild does not match batcher address', '', [], '');
       }
     } else {
       // Check recipient address and amount for normal transaction
       if (txParams.recipients.length !== 1) {
-        throw new SuspiciousTransactionError(
-          `normal transaction only supports 1 recipient but ${txParams.recipients.length} found`
+        throw new TxIntentMismatchError(
+          `normal transaction only supports 1 recipient but ${txParams.recipients.length} found`,
+          '',
+          [],
+          ''
         );
       }
       const expectedAmount = new BigNumber(txParams.recipients[0].amount);
       if (!expectedAmount.isEqualTo(txPrebuild.recipients[0].amount)) {
-        throw new SuspiciousTransactionError(
-          'normal transaction amount in txPrebuild received from BitGo servers does not match txParams supplied by client'
+        throw new TxIntentMismatchError(
+          'normal transaction amount in txPrebuild received from BitGo servers does not match txParams supplied by client',
+          '',
+          [],
+          ''
         );
       }
       if (
         this.isETHAddress(txParams.recipients[0].address) &&
         txParams.recipients[0].address !== txPrebuild.recipients[0].address
       ) {
-        throw new SuspiciousTransactionError(
-          'destination address in normal txPrebuild does not match that in txParams supplied by client'
+        throw new TxIntentMismatchError(
+          'destination address in normal txPrebuild does not match that in txParams supplied by client',
+          '',
+          [],
+          ''
         );
       }
     }
     // Check coin is correct for all transaction types
     if (!this.verifyCoin(txPrebuild)) {
-      throw new SuspiciousTransactionError(`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`,
+        '',
+        [],
+        ''
+      );
     }
     return true;
   }

modules/abstract-utxo/src/abstractUtxoCoin.ts

@@ -632,6 +632,7 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
    * @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}
+   * @throws {TxIntentMismatchError} if transaction validation fails
    */
   async verifyTransaction<TNumber extends number | bigint = number>(
     params: VerifyTransactionOptions<TNumber>

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

@@ -1,5 +1,5 @@
 import * as utxolib from '@bitgo/utxo-lib';
-import { ITransactionRecipient, SuspiciousTransactionError, VerifyTransactionOptions } from '@bitgo/sdk-core';
+import { ITransactionRecipient, TxIntentMismatchError, VerifyTransactionOptions } from '@bitgo/sdk-core';
 import { DescriptorMap } from '@bitgo/utxo-core/descriptor';
 
 import { AbstractUtxoCoin, BaseOutput, BaseParsedTransactionOutputs } from '../../abstractUtxoCoin';
@@ -66,6 +66,8 @@ 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(
   coin: AbstractUtxoCoin,
@@ -74,7 +76,7 @@ export async function verifyTransaction(
 ): Promise<boolean> {
   const tx = coin.decodeTransactionFromPrebuild(params.txPrebuild);
   if (!(tx instanceof utxolib.bitgo.UtxoPsbt)) {
-    throw new SuspiciousTransactionError('unexpected transaction type');
+    throw new TxIntentMismatchError('unexpected transaction type', '', [], '');
   }
   assertValidTransaction(tx, descriptorMap, params.txParams.recipients ?? [], tx.network);
   return true;

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

@@ -1,7 +1,7 @@
 import buildDebug from 'debug';
 import _ from 'lodash';
 import BigNumber from 'bignumber.js';
-import { BitGoBase, SuspiciousTransactionError } from '@bitgo/sdk-core';
+import { BitGoBase, TxIntentMismatchError } from '@bitgo/sdk-core';
 import * as utxolib from '@bitgo/utxo-lib';
 
 import { AbstractUtxoCoin, Output, ParsedTransaction, VerifyTransactionOptions } from '../../abstractUtxoCoin';
@@ -25,6 +25,23 @@ function getPayGoLimit(allowPaygoOutput?: boolean): number {
   return 0.015;
 }
 
+/**
+ * Verify that a transaction prebuild complies with the original intention for fixed-script wallets
+ *
+ * This implementation handles transaction verification for traditional UTXO coins using fixed scripts
+ * (non-descriptor wallets). It validates keychains, signatures, outputs, and amounts.
+ *
+ * @param coin - The UTXO coin instance
+ * @param bitgo - BitGo API instance for network calls
+ * @param params - Verification parameters
+ * @param params.txParams - Transaction parameters passed to send
+ * @param params.txPrebuild - Prebuild object returned by server
+ * @param params.wallet - Wallet object to obtain keys to verify against
+ * @param params.verification - Verification options (disableNetworking, keychains, addresses)
+ * @param params.reqId - Optional request ID for logging
+ * @returns {boolean} True if verification passes
+ * @throws {TxIntentMismatchError} if transaction validation fails
+ */
 export async function verifyTransaction<TNumber extends bigint | number>(
   coin: AbstractUtxoCoin,
   bitgo: BitGoBase,
@@ -33,11 +50,11 @@ export async function verifyTransaction<TNumber extends bigint | number>(
   const { txParams, txPrebuild, wallet, verification = {}, reqId } = params;
 
   if (!_.isUndefined(verification.disableNetworking) && !_.isBoolean(verification.disableNetworking)) {
-    throw new SuspiciousTransactionError('verification.disableNetworking must be a boolean');
+    throw new TxIntentMismatchError('verification.disableNetworking must be a boolean', '', [], '');
   }
   const isPsbt = txPrebuild.txHex && utxolib.bitgo.isPsbt(txPrebuild.txHex);
   if (isPsbt && txPrebuild.txInfo?.unspents) {
-    throw new SuspiciousTransactionError('should not have unspents in txInfo for psbt');
+    throw new TxIntentMismatchError('should not have unspents in txInfo for psbt', '', [], '');
   }
   const disableNetworking = !!verification.disableNetworking;
   const parsedTransaction: ParsedTransaction<TNumber> = await coin.parseTransaction<TNumber>({
@@ -64,7 +81,7 @@ export async function verifyTransaction<TNumber extends bigint | number>(
   if (!_.isEmpty(keySignatures)) {
     const verify = (key, pub) => {
       if (!keychains.user || !keychains.user.pub) {
-        throw new SuspiciousTransactionError('missing user keychain');
+        throw new TxIntentMismatchError('missing user keychain', '', [], '');
       }
       return verifyKeySignature({
         userKeychain: keychains.user as { pub: string },
@@ -75,7 +92,7 @@ export async function verifyTransaction<TNumber extends bigint | number>(
     const isBackupKeySignatureValid = verify(keychains.backup, keySignatures.backupPub);
     const isBitgoKeySignatureValid = verify(keychains.bitgo, keySignatures.bitgoPub);
     if (!isBackupKeySignatureValid || !isBitgoKeySignatureValid) {
-      throw new SuspiciousTransactionError('secondary public key signatures invalid');
+      throw new TxIntentMismatchError('secondary public key signatures invalid', '', [], '');
     }
     debug('successfully verified backup and bitgo key signatures');
   } else if (!disableNetworking) {
@@ -86,14 +103,20 @@ export async function verifyTransaction<TNumber extends bigint | number>(
 
   if (parsedTransaction.needsCustomChangeKeySignatureVerification) {
     if (!keychains.user || !userPublicKeyVerified) {
-      throw new SuspiciousTransactionError(
-        'transaction requires verification of user public key, but it was unable to be verified'
+      throw new TxIntentMismatchError(
+        'transaction requires verification of user public key, but it was unable to be verified',
+        '',
+        [],
+        ''
       );
     }
     const customChangeKeySignaturesVerified = verifyCustomChangeKeySignatures(parsedTransaction, keychains.user);
     if (!customChangeKeySignaturesVerified) {
-      throw new SuspiciousTransactionError(
-        'transaction requires verification of custom change key signatures, but they were unable to be verified'
+      throw new TxIntentMismatchError(
+        'transaction requires verification of custom change key signatures, but they were unable to be verified',
+        '',
+        [],
+        ''
       );
     }
     debug('successfully verified user public key and custom change key signatures');
@@ -102,7 +125,7 @@ export async function verifyTransaction<TNumber extends bigint | number>(
   const missingOutputs = parsedTransaction.missingOutputs;
   if (missingOutputs.length !== 0) {
     // there are some outputs in the recipients list that have not made it into the actual transaction
-    throw new SuspiciousTransactionError('expected outputs missing in transaction prebuild');
+    throw new TxIntentMismatchError('expected outputs missing in transaction prebuild', '', [], '');
   }
 
   const intendedExternalSpend = parsedTransaction.explicitExternalSpendAmount;
@@ -142,13 +165,13 @@ export async function verifyTransaction<TNumber extends bigint | number>(
     } else {
       // the additional external outputs can only be BitGo's pay-as-you-go fee, but we cannot verify the wallet address
       // there are some addresses that are outside the scope of intended recipients that are not change addresses
-      throw new SuspiciousTransactionError('prebuild attempts to spend to unintended external recipients');
+      throw new TxIntentMismatchError('prebuild attempts to spend to unintended external recipients', '', [], '');
     }
   }
 
   const allOutputs = parsedTransaction.outputs;
   if (!txPrebuild.txHex) {
-    throw new SuspiciousTransactionError(`txPrebuild.txHex not set`);
+    throw new TxIntentMismatchError(`txPrebuild.txHex not set`, '', [], '');
   }
   const inputs = isPsbt
     ? getPsbtTxInputs(txPrebuild.txHex, coin.network).map((v) => ({
@@ -165,8 +188,11 @@ export async function verifyTransaction<TNumber extends bigint | number>(
   const fee = inputAmount - outputAmount;
 
   if (fee < 0) {
-    throw new SuspiciousTransactionError(
-      `attempting to spend ${outputAmount} satoshis, which exceeds the input amount (${inputAmount} satoshis) by ${-fee}`
+    throw new TxIntentMismatchError(
+      `attempting to spend ${outputAmount} satoshis, which exceeds the input amount (${inputAmount} satoshis) by ${-fee}`,
+      '',
+      [],
+      ''
     );
   }
 

modules/sdk-core/src/bitgo/errors.ts

@@ -160,12 +160,6 @@ export class InvalidTransactionError extends BitGoJsError {
   }
 }
 
-export class SuspiciousTransactionError extends BitGoJsError {
-  public constructor(message?: string) {
-    super(message || 'Suspicious transaction detected');
-  }
-}
-
 export class MissingEncryptedKeychainError extends Error {
   public constructor(message?: string) {
     super(message || 'No encrypted keychains on this wallet.');