fix(bridge-ui): pending transactions custom store with better error h…

…andling (#13581)

Co-authored-by: jeff <113397187+cyberhorsey@users.noreply.github.com>

packages/bridge-ui/src/App.svelte

@@ -8,11 +8,7 @@
   import { MetaMaskConnector } from '@wagmi/core/connectors/metaMask';
 
   import { setupI18n } from './i18n';
-  import {
-    pendingTransactions,
-    transactioner,
-    transactions,
-  } from './store/transactions';
+  import { transactioner, transactions } from './store/transactions';
   import Navbar from './components/Navbar.svelte';
   import Toast, { successToast } from './components/Toast.svelte';
   import { signer } from './store/signer';
@@ -130,24 +126,6 @@
     }
   });
 
-  pendingTransactions.subscribe((store) => {
-    (async () => {
-      const confirmedPendingTxIndex = await Promise.race(
-        store.map((tx, index) => {
-          return new Promise<number>((resolve) => {
-            $signer.provider
-              .waitForTransaction(tx.hash, 1)
-              .then(() => resolve(index));
-          });
-        }),
-      );
-      successToast('Transaction completed!');
-      let s = store;
-      s.splice(confirmedPendingTxIndex, 1);
-      pendingTransactions.set(s);
-    })();
-  });
-
   const transactionToIntervalMap = new Map();
 
   transactions.subscribe((store) => {

packages/bridge-ui/src/components/Transactions/Transaction.svelte

@@ -108,15 +108,16 @@
         srcBridgeAddress: chains[bridgeTx.fromChainId].bridgeAddress,
       });
 
-      pendingTransactions.update((store) => {
-        store.push(tx);
-        return store;
-      });
-
       successToast($_('toast.transactionSent'));
+
+      await pendingTransactions.add(tx, $signer);
+
       // TODO: keep the MessageStatus as contract and use another way.
       transaction.status = MessageStatus.ClaimInProgress;
+
+      successToast('Transaction completed!');
     } catch (e) {
+      // TODO: handle potential transaction failure
       console.error(e);
       errorToast($_('toast.errorSendingTransaction'));
     } finally {
@@ -153,13 +154,13 @@
         srcTokenVaultAddress: tokenVaults[bridgeTx.fromChainId],
       });
 
-      pendingTransactions.update((store) => {
-        store.push(tx);
-        return store;
-      });
-
       successToast($_('toast.transactionSent'));
+
+      pendingTransactions.add(tx, $signer);
+
+      successToast('Transaction completed!');
     } catch (e) {
+      // TODO: handle potential transaction failure
       console.error(e);
       errorToast($_('toast.errorSendingTransaction'));
     } finally {

packages/bridge-ui/src/components/form/BridgeForm.svelte

@@ -171,17 +171,18 @@
         spenderAddress: tokenVaults[$fromChain.id],
       });
 
-      pendingTransactions.update((store) => {
-        store.push(tx);
-        return store;
-      });
-
       successToast($_('toast.transactionSent'));
-      await $signer.provider.waitForTransaction(tx.hash, 1);
+
+      await pendingTransactions.add(tx, $signer);
 
       requiresAllowance = false;
+
+      successToast('Transaction completed!');
     } catch (e) {
       console.error(e);
+      // TODO: if we have TransactionReceipt here means the tx failed
+      //       We might want to give the user a link to etherscan
+      //       to see the tx details
       errorToast($_('toast.errorSendingTransaction'));
     } finally {
       loading = false;
@@ -262,12 +263,17 @@
       );
 
       if (!doesUserHaveEnoughBalance) {
+        // TODO: about custom errors and catch it in the catch block?
         errorToast('Insufficient ETH balance');
         return;
       }
 
       const tx = await $activeBridge.Bridge(bridgeOpts);
 
+      successToast($_('toast.transactionSent'));
+
+      await pendingTransactions.add(tx, $signer);
+
       // tx.chainId is not set immediately but we need it later. set it
       // manually.
       tx.chainId = $fromChain.id;
@@ -284,6 +290,7 @@
         hash: tx.hash,
         status: MessageStatus.New,
       };
+
       if (!transactions) {
         transactions = [bridgeTransaction];
       } else {
@@ -292,11 +299,6 @@
 
       $transactioner.updateStorageByAddress(userAddress, transactions);
 
-      pendingTransactions.update((store) => {
-        store.push(tx);
-        return store;
-      });
-
       const allTransactions = $transactionsStore;
 
       // get full BridgeTransaction object
@@ -307,11 +309,12 @@
 
       transactionsStore.set([bridgeTransaction, ...allTransactions]);
 
-      successToast($_('toast.transactionSent'));
-      await $signer.provider.waitForTransaction(tx.hash, 1);
       memo = '';
+
+      successToast('Transaction completed!');
     } catch (e) {
       console.error(e);
+      // TODO: Same as in approve()
       errorToast($_('toast.errorSendingTransaction'));
     } finally {
       loading = false;

packages/bridge-ui/src/components/modals/FaucetModal.svelte

@@ -73,17 +73,17 @@
       const address = await $signer.getAddress();
       const tx = await contract.mint(address);
 
-      pendingTransactions.update((store) => {
-        store.push(tx);
-        return store;
-      });
-
       successToast($_('toast.transactionSent'));
+
+      await pendingTransactions.add(tx, $signer);
+
       isOpen = false;
-      await $signer.provider.waitForTransaction(tx.hash, 1);
+
+      successToast('Transaction completed!');
 
       await onMint();
     } catch (e) {
+      // TODO: handle potential transaction failure
       console.error(e);
       errorToast($_('toast.errorSendingTransaction'));
     }

packages/bridge-ui/src/store/transactions.spec.ts

@@ -0,0 +1,67 @@
+import { get } from 'svelte/store';
+import type { Signer, Transaction, ethers } from 'ethers';
+import { pendingTransactions } from './transactions';
+
+jest.mock('../constants/envVars');
+
+// Transaction we're going to add to the store
+const tx = { hash: '0x789' } as Transaction;
+
+// These are the pending transactions we'll have initially in the store
+const initialTxs = [{ hash: '0x123' }, { hash: '0x456' }] as Transaction[];
+
+const mockSigner = (receipt: ethers.providers.TransactionReceipt) => {
+  const waitForTransaction = jest
+    .fn()
+    .mockImplementation(() => Promise.resolve(receipt));
+
+  return {
+    provider: { waitForTransaction },
+  } as unknown as Signer;
+};
+
+describe('transaction stores', () => {
+  beforeEach(() => {
+    pendingTransactions.set(initialTxs);
+  });
+
+  it('tests a successful pendingTransactions', () => {
+    const txTeceipt = { status: 1 } as ethers.providers.TransactionReceipt;
+    const signer = mockSigner(txTeceipt);
+
+    pendingTransactions
+      .add(tx, signer)
+      .then((receipt) => {
+        // The transaction should have been removed from the store
+        expect(get(pendingTransactions)).toStrictEqual(initialTxs);
+
+        expect(receipt).toEqual(txTeceipt);
+      })
+      .catch(() => {
+        throw new Error('Should not have thrown');
+      });
+
+    // The transaction should have added to the store
+    expect(get(pendingTransactions)).toStrictEqual([...initialTxs, tx]);
+  });
+
+  it('tests a failed pendingTransactions custom store', () => {
+    const txTeceipt = { status: 0 } as ethers.providers.TransactionReceipt;
+    const signer = mockSigner(txTeceipt);
+
+    pendingTransactions
+      .add(tx, signer)
+      .then(() => {
+        throw new Error('Should have thrown');
+      })
+      .catch((receipt) => {
+        // The transaction should have been removed from the store
+        expect(get(pendingTransactions)).toStrictEqual(initialTxs);
+
+        expect(receipt).toEqual(txTeceipt);
+      });
+
+    // The transaction should have added to the store
+    expect(get(pendingTransactions)).toStrictEqual([...initialTxs, tx]);
+  });
+});

packages/bridge-ui/src/store/transactions.ts

@@ -1,10 +1,69 @@
 import { writable } from 'svelte/store';
-
-import type { Transaction } from 'ethers';
+import type { Signer, Transaction, ethers } from 'ethers';
 import type { BridgeTransaction, Transactioner } from '../domain/transactions';
+import { Deferred } from '../utils/Deferred';
+
+export const transactions = writable<BridgeTransaction[]>([]);
+export const transactioner = writable<Transactioner>();
+
+// Custom store: pendingTransactions
+const { subscribe, set, update } = writable<Transaction[]>([]);
+export const pendingTransactions = {
+  /**
+   * We're creating here a custom store, which is a writable store.
+   * We must stick to the store contract, which is:
+   */
+  set,
+  subscribe,
+  // update, // this method is optional.
+
+  /**
+   * Custom method, which will help us add a new transaction to the store
+   * and get it removed onces the transaction is mined.
+   */
+  add: (tx: Transaction, signer: Signer) => {
+    const deferred = new Deferred<ethers.providers.TransactionReceipt>();
+
+    update((txs: Transaction[]) => {
+      // New array with the new transaction appended
+      const newPendingTransactions = [...txs, tx];
+
+      // Save the index of the new transaction to later on remove it
+      // from the list of pending transactions.
+      const idxAppendedTransaction = newPendingTransactions.length - 1;
+
+      // Next step is to wait for the transaction to be mined
+      // before removing it from the store.
+
+      /**
+       * Returns a Promise which will not resolve until transactionHash is mined.
+       * If confirms is 0, this method is non-blocking and if the transaction
+       * has not been mined returns null. Otherwise, this method will block until
+       * the transaction has confirms blocks mined on top of the block in which
+       * is was mined.
+       * See https://docs.ethers.org/v5/api/providers/provider/#Provider-waitForTransaction
+       */
+      signer.provider.waitForTransaction(tx.hash, 1).then((receipt) => {
+        // The transaction has been mined.
+
+        // Removes the transaction from the store
+        update((txs: Transaction[]) => {
+          const copyPendingTransactions = [...txs];
+          copyPendingTransactions.splice(idxAppendedTransaction, 1);
+          return copyPendingTransactions;
+        });
+
+        // Resolves or rejects the promise depending on the transaction status.
+        if (receipt.status === 1) {
+          deferred.resolve(receipt);
+        } else {
+          deferred.reject(receipt);
+        }
+      });
 
-const pendingTransactions = writable<Transaction[]>([]);
-const transactions = writable<BridgeTransaction[]>([]);
-const transactioner = writable<Transactioner>();
+      return newPendingTransactions;
+    });
 
-export { pendingTransactions, transactions, transactioner };
+    return deferred.promise;
+  },
+};

packages/bridge-ui/src/utils/Deferred.spec.ts

@@ -0,0 +1,25 @@
+import { Deferred } from './Deferred';
+
+describe('Deferred', () => {
+  it('should resolve', async () => {
+    const deferred = new Deferred();
+    deferred.resolve('test');
+
+    try {
+      await expect(deferred.promise).resolves.toBe('test');
+    } catch (e) {
+      throw Error('This should never happen');
+    }
+  });
+
+  it('should reject', async () => {
+    const deferred = new Deferred();
+    deferred.reject('test');
+
+    try {
+      await deferred.promise;
+    } catch (err) {
+      expect(err).toBe('test');
+    }
+  });
+});

packages/bridge-ui/src/utils/Deferred.ts

@@ -0,0 +1,12 @@
+export class Deferred<T> {
+  public promise: Promise<T>;
+  public resolve: (value?: T) => void;
+  public reject: (reason?: T) => void;
+
+  constructor() {
+    this.promise = new Promise((resolve, reject) => {
+      this.resolve = resolve;
+      this.reject = reject;
+    });
+  }
+}