fix(bitgo): added data validation on tx

Ticket: SC-1436

Author: henryyang183

modules/bitgo/test/v2/unit/staking/stakingWalletNonTSS.ts

@@ -400,7 +400,7 @@ describe('non-TSS Staking Wallet', function () {
       await topethWctStakingWallet.buildAndSign({ walletPassphrase: 'passphrase' }, stakingTransaction);
     });
 
-    it('should log an error if a transaction is failed to be validated transaction if unsigned transaction does not match the staking transaction', async function () {
+    it('should throw an error if unsigned transaction does not match the staking transaction', async function () {
       const unsignedTransaction: PrebuildTransactionResult = {
         walletId: topethWctStakingWallet.walletId,
         ...opethFixtures.unsignedStakingTransaction,
@@ -409,8 +409,6 @@ describe('non-TSS Staking Wallet', function () {
       } as PrebuildTransactionResult;
       const stakingTransaction: StakingTransaction = opethFixtures.updatedStakingRequest;
 
-      const consoleStub = sinon.stub(console, 'error');
-
       nock(microservicesUri)
         .get(
           `/api/staking/v1/${topethWctStakingWallet.coin}/wallets/${topethWctStakingWallet.walletId}/requests/${stakingTransaction.stakingRequestId}/transactions/${stakingTransaction.id}`
@@ -426,11 +424,14 @@ describe('non-TSS Staking Wallet', function () {
         .post(`/api/v2/topeth/wallet/${topethWctStakingWallet.walletId}/tx/build`)
         .reply(200, unsignedTransaction);
 
-      await topethWctStakingWallet.buildAndSign({ walletPassphrase: 'passphrase' }, stakingTransaction);
+      const expectedErrorMessage =
+        'Staking transaction validation failed before signing: ' +
+        'Unexpected recipient address found in built transaction: 0x86bb6dca2cd6f9a0189c478bbb8f7ee2fef07c89; ' +
+        'Expected recipient address not found in built transaction: 0x75bb6dca2cd6f9a0189c478bbb8f7ee2fef07c78';
 
-      consoleStub.calledWith(
-        'Invalid recipient address: 0x86bb6dca2cd6f9a0189c478bbb8f7ee2fef07c89, Missing recipient address(es): 0x75bb6dca2cd6f9a0189c478bbb8f7ee2fef07c78'
-      );
+      await topethWctStakingWallet
+        .buildAndSign({ walletPassphrase: 'passphrase' }, stakingTransaction)
+        .should.be.rejectedWith(expectedErrorMessage);
     });
   });
 });

modules/sdk-core/src/bitgo/staking/stakingWallet.ts

@@ -271,7 +271,7 @@ export class StakingWallet implements IStakingWallet {
     // default to verifying a transaction unless explicitly skipped
     const skipVerification = signOptions.transactionVerificationOptions?.skipTransactionVerification ?? false;
     if (!isStakingTxRequestPrebuildResult(builtTx.result) && !skipVerification) {
-      await this.validateBuiltStakingTransaction(transaction, builtTx);
+      await this.validateBuiltStakingTransaction(builtTx.transaction, builtTx);
     }
     return await this.sign(signOptions, builtTx);
   }
@@ -371,55 +371,100 @@ export class StakingWallet implements IStakingWallet {
     }
 
     const explainedTransaction = await coin.explainTransaction(result);
+    const mismatchErrors: string[] = [];
 
-    if (buildParams?.recipients) {
+    if (buildParams?.recipients && buildParams.recipients.length > 0) {
       const userRecipientMap = new Map(
         buildParams.recipients.map((recipient) => [recipient.address.toLowerCase(), recipient])
       );
       const platformRecipientMap = new Map(
         (explainedTransaction?.outputs ?? []).map((recipient) => [recipient.address.toLowerCase(), recipient])
       );
 
-      const mismatchErrors: string[] = [];
-
-      for (const [recipientAddress, recipientInfo] of platformRecipientMap) {
-        if (userRecipientMap.has(recipientAddress)) {
-          const userRecipient = userRecipientMap.get(recipientAddress);
-          if (!userRecipient) {
-            console.error('Unable to determine recipient address');
-            return;
-          }
-          const matchResult = transactionRecipientsMatch(userRecipient, recipientInfo);
-          if (!matchResult.exactMatch) {
-            if (!matchResult.tokenMatch) {
-              mismatchErrors.push(
-                `Invalid token ${recipientInfo.tokenName} transfer with amount ${recipientInfo.amount} to ${recipientInfo.address} found in built transaction, specified ${userRecipient.tokenName}`
-              );
-            }
-            if (!matchResult.amountMatch) {
-              mismatchErrors.push(
-                `Invalid recipient amount for ${recipientInfo.address}, specified ${userRecipient.amount} got ${recipientInfo.amount}`
-              );
-            }
-          }
-        } else {
-          mismatchErrors.push(`Invalid recipient address: ${recipientAddress}`);
+      for (const [address] of platformRecipientMap) {
+        if (!userRecipientMap.has(address)) {
+          mismatchErrors.push(`Unexpected recipient address found in built transaction: ${address}`);
         }
       }
 
-      const missingRecipientAddresses = Array.from(userRecipientMap.keys()).filter(
-        (address) => !platformRecipientMap.has(address)
+      for (const [address, userRecipient] of userRecipientMap) {
+        const platformRecipient = platformRecipientMap.get(address);
+        if (!platformRecipient) {
+          mismatchErrors.push(`Expected recipient address not found in built transaction: ${address}`);
+          continue;
+        }
+
+        const matchResult = transactionRecipientsMatch(userRecipient, platformRecipient);
+
+        if (!matchResult.amountMatch) {
+          mismatchErrors.push(
+            `Recipient ${address} amount mismatch. Expected: ${userRecipient.amount}, Got: ${platformRecipient.amount}`
+          );
+        }
+        if (!matchResult.tokenMatch) {
+          mismatchErrors.push(
+            `Recipient ${address} token mismatch. Expected: ${userRecipient.tokenName ?? 'native coin'}, Got: ${
+              platformRecipient.tokenName ?? 'native coin'
+            }`
+          );
+        }
+      }
+    }
+    if (buildParams?.memo && (explainedTransaction as any).memo !== buildParams.memo) {
+      mismatchErrors.push(
+        `Memo mismatch. Expected: '${JSON.stringify(buildParams.memo)}', Got: '${JSON.stringify(
+          (explainedTransaction as any).memo
+        )}'`
+      );
+    }
+
+    if (buildParams?.gasLimit && String((explainedTransaction as any).gasLimit) !== String(buildParams.gasLimit)) {
+      mismatchErrors.push(
+        `Gas Limit mismatch. Expected: ${buildParams.gasLimit}, Got: ${(explainedTransaction as any).gasLimit}`
+      );
+    }
+
+    if (buildParams?.type && (explainedTransaction as any).type !== buildParams.type) {
+      mismatchErrors.push(
+        `Transaction type mismatch. Expected: '${buildParams.type}', Got: '${(explainedTransaction as any).type}'`
       );
+    }
 
-      if (missingRecipientAddresses.length > 0) {
-        mismatchErrors.push(`Missing recipient address(es): ${missingRecipientAddresses.join(', ')}`);
+    if (buildParams?.solInstructions) {
+      if (
+        JSON.stringify((explainedTransaction as any).solInstructions) !== JSON.stringify(buildParams.solInstructions)
+      ) {
+        mismatchErrors.push(
+          `Solana instructions mismatch. Expected: ${JSON.stringify(
+            buildParams.solInstructions
+          )}, Got: ${JSON.stringify((explainedTransaction as any).solInstructions)}`
+        );
       }
-      if (mismatchErrors.length > 0) {
-        console.error(mismatchErrors.join(', '));
-        return;
+    }
+
+    if (buildParams?.aptosCustomTransactionParams) {
+      if (
+        JSON.stringify((explainedTransaction as any).aptosCustomTransactionParams) !==
+        JSON.stringify(buildParams.aptosCustomTransactionParams)
+      ) {
+        mismatchErrors.push(
+          `Aptos custom transaction parameters mismatch. Expected: ${JSON.stringify(
+            buildParams.aptosCustomTransactionParams
+          )}, Got: ${JSON.stringify((explainedTransaction as any).aptosCustomTransactionParams)}`
+        );
       }
-    } else {
-      debug(`Cannot validate staking transaction ${transaction.stakingRequestId} without specified build params`);
+    }
+
+    if (mismatchErrors.length > 0) {
+      const errorMessage = `Staking transaction validation failed before signing: ${mismatchErrors.join('; ')}`;
+      debug(errorMessage);
+      throw new Error(errorMessage);
+    }
+
+    if (!buildParams) {
+      debug(
+        `Cannot perform deep validation for staking transaction ${transaction.stakingRequestId} without specified build params`
+      );
     }
   }
 }