fix: Mark multiple transactions as failed if a batch smart transaction fails (#551)

* Fail multiple transactions

* fix: Mark multiple transactions as failed if a batch smart transaction fails

Signed-off-by: dan437 <80175477+dan437@users.noreply.github.com>

* Renaming

Signed-off-by: dan437 <80175477+dan437@users.noreply.github.com>

---------

Signed-off-by: dan437 <80175477+dan437@users.noreply.github.com>
Co-authored-by: Matthew Walsh <matthew.walsh@consensys.net>

Author: dan437

src/SmartTransactionsController.ts

@@ -59,8 +59,8 @@ import {
   getTxHash,
   getSmartTransactionMetricsProperties,
   getSmartTransactionMetricsSensitiveProperties,
-  shouldMarkRegularTransactionAsFailed,
-  markRegularTransactionAsFailed,
+  shouldMarkRegularTransactionsAsFailed,
+  markRegularTransactionsAsFailed,
 } from './utils';
 
 const SECOND = 1000;
@@ -561,13 +561,13 @@ export class SmartTransactionsController extends StaticIntervalPollingController
     );
 
     if (
-      shouldMarkRegularTransactionAsFailed({
+      shouldMarkRegularTransactionsAsFailed({
         smartTransaction: nextSmartTransaction,
         clientId: this.#clientId,
         getFeatureFlags: this.#getFeatureFlags,
       })
     ) {
-      markRegularTransactionAsFailed({
+      markRegularTransactionsAsFailed({
         smartTransaction: nextSmartTransaction,
         getRegularTransactions: () =>
           this.messenger.call('TransactionController:getTransactions'),

src/utils.test.ts

@@ -403,7 +403,7 @@ describe('src/utils.js', () => {
     });
   });
 
-  describe('shouldMarkRegularTransactionAsFailed', () => {
+  describe('shouldMarkRegularTransactionsAsFailed', () => {
     const createSmartTransaction = (status: SmartTransactionStatuses) => ({
       uuid: 'test-uuid',
       status,
@@ -428,7 +428,7 @@ describe('src/utils.js', () => {
       });
 
     it('returns true for "cancelled" status when feature flag is enabled', () => {
-      const result = utils.shouldMarkRegularTransactionAsFailed({
+      const result = utils.shouldMarkRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.CANCELLED,
         ),
@@ -439,7 +439,7 @@ describe('src/utils.js', () => {
     });
 
     it('returns true for "cancelled_user_cancelled" status when feature flag is enabled', () => {
-      const result = utils.shouldMarkRegularTransactionAsFailed({
+      const result = utils.shouldMarkRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.CANCELLED_USER_CANCELLED,
         ),
@@ -450,7 +450,7 @@ describe('src/utils.js', () => {
     });
 
     it('returns true for "unknown" status when feature flag is enabled', () => {
-      const result = utils.shouldMarkRegularTransactionAsFailed({
+      const result = utils.shouldMarkRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.UNKNOWN,
         ),
@@ -461,7 +461,7 @@ describe('src/utils.js', () => {
     });
 
     it('returns true for "resolved" status when feature flag is enabled', () => {
-      const result = utils.shouldMarkRegularTransactionAsFailed({
+      const result = utils.shouldMarkRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.RESOLVED,
         ),
@@ -472,7 +472,7 @@ describe('src/utils.js', () => {
     });
 
     it('returns false for "pending" status when feature flag is enabled', () => {
-      const result = utils.shouldMarkRegularTransactionAsFailed({
+      const result = utils.shouldMarkRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.PENDING,
         ),
@@ -483,7 +483,7 @@ describe('src/utils.js', () => {
     });
 
     it('returns false for "success" status when feature flag is enabled', () => {
-      const result = utils.shouldMarkRegularTransactionAsFailed({
+      const result = utils.shouldMarkRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.SUCCESS,
         ),
@@ -494,7 +494,7 @@ describe('src/utils.js', () => {
     });
 
     it('returns false when feature flag is disabled regardless of status', () => {
-      const result = utils.shouldMarkRegularTransactionAsFailed({
+      const result = utils.shouldMarkRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.CANCELLED,
         ),
@@ -509,7 +509,7 @@ describe('src/utils.js', () => {
         ...createSmartTransaction(SmartTransactionStatuses.CANCELLED),
         transactionId: undefined,
       };
-      const result = utils.shouldMarkRegularTransactionAsFailed({
+      const result = utils.shouldMarkRegularTransactionsAsFailed({
         smartTransaction,
         clientId: ClientId.Extension,
         getFeatureFlags: mockGetFeatureFlags(true),
@@ -518,7 +518,7 @@ describe('src/utils.js', () => {
     });
 
     it('returns true for mobile client when mobile feature flag is enabled', () => {
-      const result = utils.shouldMarkRegularTransactionAsFailed({
+      const result = utils.shouldMarkRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.CANCELLED,
         ),
@@ -529,7 +529,7 @@ describe('src/utils.js', () => {
     });
   });
 
-  describe('markRegularTransactionAsFailed', () => {
+  describe('markRegularTransactionsAsFailed', () => {
     const createSmartTransaction = (status: SmartTransactionStatuses) => ({
       uuid: 'test-uuid',
       status,
@@ -561,7 +561,7 @@ describe('src/utils.js', () => {
     it('updates transaction with failed status and error message', () => {
       const updateTransactionMock = jest.fn();
 
-      utils.markRegularTransactionAsFailed({
+      utils.markRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.CANCELLED,
         ),
@@ -587,7 +587,7 @@ describe('src/utils.js', () => {
       const getRegularTransactionsMock = jest.fn(() => []);
 
       expect(() =>
-        utils.markRegularTransactionAsFailed({
+        utils.markRegularTransactionsAsFailed({
           smartTransaction: createSmartTransaction(
             SmartTransactionStatuses.CANCELLED,
           ),
@@ -610,7 +610,7 @@ describe('src/utils.js', () => {
         },
       };
 
-      utils.markRegularTransactionAsFailed({
+      utils.markRegularTransactionsAsFailed({
         smartTransaction: createSmartTransaction(
           SmartTransactionStatuses.CANCELLED,
         ),
@@ -620,5 +620,53 @@ describe('src/utils.js', () => {
 
       expect(updateTransactionMock).not.toHaveBeenCalled();
     });
+
+    it('marks multiple transactions as failed when txHashes match', () => {
+      const updateTransactionMock = jest.fn();
+      const transaction1: TransactionMeta = {
+        ...mockTransaction,
+        id: '456',
+        hash: '0xhash1',
+      };
+      const transaction2: TransactionMeta = {
+        ...mockTransaction,
+        id: '789',
+        hash: '0xhash2',
+      };
+      const smartTransaction = {
+        ...createSmartTransaction(SmartTransactionStatuses.CANCELLED),
+        txHashes: ['0xhash1', '0xhash2'],
+      };
+
+      utils.markRegularTransactionsAsFailed({
+        smartTransaction,
+        getRegularTransactions: () => [transaction1, transaction2],
+        updateTransaction: updateTransactionMock,
+      });
+
+      expect(updateTransactionMock).toHaveBeenCalledTimes(2);
+      expect(updateTransactionMock).toHaveBeenCalledWith(
+        {
+          ...transaction1,
+          status: TransactionStatus.failed,
+          error: {
+            name: 'SmartTransactionFailed',
+            message: 'Smart transaction failed with status: cancelled',
+          },
+        },
+        'Smart transaction status: cancelled',
+      );
+      expect(updateTransactionMock).toHaveBeenCalledWith(
+        {
+          ...transaction2,
+          status: TransactionStatus.failed,
+          error: {
+            name: 'SmartTransactionFailed',
+            message: 'Smart transaction failed with status: cancelled',
+          },
+        },
+        'Smart transaction status: cancelled',
+      );
+    });
   });
 });

src/utils.ts

@@ -240,7 +240,7 @@ export const getReturnTxHashAsap = (
     : smartTransactionsFeatureFlags?.mobileReturnTxHashAsap;
 };
 
-export const shouldMarkRegularTransactionAsFailed = ({
+export const shouldMarkRegularTransactionsAsFailed = ({
   smartTransaction,
   clientId,
   getFeatureFlags,
@@ -271,7 +271,7 @@ export const shouldMarkRegularTransactionAsFailed = ({
   return Boolean(returnTxHashAsapEnabled && transactionId);
 };
 
-export const markRegularTransactionAsFailed = ({
+export const markRegularTransactionsAsFailed = ({
   smartTransaction,
   getRegularTransactions,
   updateTransaction,
@@ -280,23 +280,32 @@ export const markRegularTransactionAsFailed = ({
   getRegularTransactions: TransactionControllerGetTransactionsAction['handler'];
   updateTransaction: TransactionControllerUpdateTransactionAction['handler'];
 }) => {
-  const { transactionId, status } = smartTransaction;
-  const originalTransaction = getRegularTransactions().find(
-    (transaction) => transaction.id === transactionId,
+  const { transactionId, status, txHashes } = smartTransaction;
+
+  const transactionsToFail = getRegularTransactions().filter(
+    (tx) => (tx.hash && txHashes?.includes(tx.hash)) || tx.id === transactionId,
   );
-  if (!originalTransaction) {
+
+  if (!transactionsToFail.length) {
     throw new Error('Cannot find regular transaction to mark it as failed');
   }
-  if (originalTransaction.status === TransactionStatus.failed) {
-    return; // Already marked as failed.
+
+  for (const tx of transactionsToFail) {
+    if (tx.status === TransactionStatus.failed) {
+      continue; // Already marked as failed.
+    }
+    const updatedTransaction: TransactionMeta = {
+      ...tx,
+      status: TransactionStatus.failed,
+      error: {
+        name: 'SmartTransactionFailed',
+        message: `Smart transaction failed with status: ${status}`,
+      },
+    };
+
+    updateTransaction(
+      updatedTransaction,
+      `Smart transaction status: ${status}`,
+    );
   }
-  const updatedTransaction: TransactionMeta = {
-    ...originalTransaction,
-    status: TransactionStatus.failed,
-    error: {
-      name: 'SmartTransactionFailed',
-      message: `Smart transaction failed with status: ${status}`,
-    },
-  };
-  updateTransaction(updatedTransaction, `Smart transaction status: ${status}`);
 };