fix: cache block of pending transaction

src/logic/safe/store/actions/__tests__/TxSender.test.ts

@@ -112,7 +112,7 @@ const mockPromiEvent = {
 }
 
 describe('TxSender', () => {
-  let tryOffChainSigningSpy, saveTxToHistorySpy, addPendingTransactionSpy, navigateToTxSpy, fetchTransactionsSpy
+  let tryOffChainSigningSpy, saveTxToHistorySpy, setPendingTransactionSpy, navigateToTxSpy, fetchTransactionsSpy
 
   beforeEach(() => {
     jest.restoreAllMocks()
@@ -135,7 +135,7 @@ describe('TxSender', () => {
     saveTxToHistorySpy = jest
       .spyOn(txHistory, 'saveTxToHistory')
       .mockImplementation(() => Promise.resolve(mockTransactionDetails as any))
-    addPendingTransactionSpy = jest.spyOn(pendingTransactions, 'addPendingTransaction')
+    setPendingTransactionSpy = jest.spyOn(pendingTransactions, 'setPendingTransaction')
     navigateToTxSpy = jest.spyOn(utils, 'navigateToTx')
     fetchTransactionsSpy = jest.spyOn(fetchTransactions, 'default')
 
@@ -173,7 +173,7 @@ describe('TxSender', () => {
     await waitFor(() => {
       expect(tryOffChainSigningSpy).toHaveBeenCalledTimes(1)
       expect(saveTxToHistorySpy).toHaveBeenCalledTimes(1)
-      expect(addPendingTransactionSpy).toHaveBeenCalledTimes(0)
+      expect(setPendingTransactionSpy).toHaveBeenCalledTimes(0)
       expect(navigateToTxSpy).toHaveBeenCalledTimes(0)
       expect(fetchTransactionsSpy).toHaveBeenCalledTimes(1)
     })
@@ -210,7 +210,7 @@ describe('TxSender', () => {
     await waitFor(() => {
       expect(tryOffChainSigningSpy).toHaveBeenCalledTimes(1)
       expect(saveTxToHistorySpy).toHaveBeenCalledTimes(1)
-      expect(addPendingTransactionSpy).toHaveBeenCalledTimes(0)
+      expect(setPendingTransactionSpy).toHaveBeenCalledTimes(0)
       expect(navigateToTxSpy).toHaveBeenCalledTimes(1)
       expect(fetchTransactionsSpy).toHaveBeenCalledTimes(1)
     })
@@ -266,7 +266,7 @@ describe('TxSender', () => {
       expect(getExecutionTransactionSpy).toHaveBeenCalledTimes(1)
       expect(setNonceSpy).toHaveBeenCalledTimes(1)
       expect(saveTxToHistorySpy).toHaveBeenCalledTimes(1)
-      expect(addPendingTransactionSpy).toHaveBeenCalledTimes(1)
+      expect(setPendingTransactionSpy).toHaveBeenCalledTimes(1)
       expect(navigateToTxSpy).toHaveBeenCalledTimes(1)
       expect(fetchTransactionsSpy).toHaveBeenCalledTimes(1)
     })
@@ -317,7 +317,7 @@ describe('TxSender', () => {
     await waitFor(() => {
       expect(getExecutionTransactionSpy).toHaveBeenCalledTimes(1)
       expect(setNonceSpy).toHaveBeenCalledTimes(1)
-      expect(addPendingTransactionSpy).toHaveBeenCalledTimes(1)
+      expect(setPendingTransactionSpy).toHaveBeenCalledTimes(1)
       expect(saveTxToHistorySpy).toHaveBeenCalledTimes(0)
       expect(navigateToTxSpy).toHaveBeenCalledTimes(0)
       expect(fetchTransactionsSpy).toHaveBeenCalledTimes(1)

src/logic/safe/store/actions/createTransaction.ts

@@ -25,7 +25,7 @@ import { Dispatch, DispatchReturn } from './types'
 import { checkIfOffChainSignatureIsPossible, getPreValidatedSignatures } from 'src/logic/safe/safeTxSigner'
 import { TxParameters } from 'src/routes/safe/container/hooks/useTransactionParameters'
 import { Errors, logError } from 'src/logic/exceptions/CodedException'
-import { removePendingTransaction, addPendingTransaction } from 'src/logic/safe/store/actions/pendingTransactions'
+import { removePendingTransaction, setPendingTransaction } from 'src/logic/safe/store/actions/pendingTransactions'
 import { _getChainId } from 'src/config'
 import { GnosisSafe } from 'src/types/contracts/gnosis_safe.d'
 import * as aboutToExecuteTx from 'src/logic/safe/utils/aboutToExecuteTx'
@@ -109,7 +109,7 @@ export class TxSender {
     }
 
     if (isFinalization && this.txId && this.txHash) {
-      dispatch(addPendingTransaction({ id: this.txId, txHash: this.txHash }))
+      dispatch(setPendingTransaction({ id: this.txId, txHash: this.txHash }))
     }
 
     notifications.closePending()

src/logic/safe/store/actions/pendingTransactions.ts

@@ -3,6 +3,8 @@ import {
   AddPendingTransactionPayload,
   RemovePendingTransactionPayload,
 } from 'src/logic/safe/store/reducer/pendingTransactions'
+import { getWeb3 } from 'src/logic/wallets/getWeb3'
+import { Dispatch } from './types'
 
 export enum PENDING_TRANSACTIONS_ACTIONS {
   ADD = 'pendingTransactions/add',
@@ -13,3 +15,19 @@ export const addPendingTransaction = createAction<AddPendingTransactionPayload>(
 export const removePendingTransaction = createAction<RemovePendingTransactionPayload>(
   PENDING_TRANSACTIONS_ACTIONS.REMOVE,
 )
+
+export const setPendingTransaction = (details: { id: string; txHash: string }) => {
+  return async (dispatch: Dispatch): Promise<void> => {
+    let block: number | undefined
+    try {
+      block = await getWeb3().eth.getBlockNumber()
+    } catch {}
+
+    const pendingTransaction = {
+      ...details,
+      block,
+    }
+
+    dispatch(addPendingTransaction(pendingTransaction))
+  }
+}

src/logic/safe/store/reducer/pendingTransactions.ts

@@ -7,7 +7,14 @@ import { _getChainId } from 'src/config'
 
 export const PENDING_TRANSACTIONS_ID = 'pendingTransactions'
 
-export type PendingTransactionsState = Record<ChainId, Record<string, string>>
+export type PendingTransactionsState = {
+  [chainId: ChainId]: {
+    [id: string]: {
+      txHash: string
+      block?: number
+    }
+  }
+}
 
 const initialPendingTxsState = session.getItem<PendingTransactionsState>(PENDING_TRANSACTIONS_ID) || {}
 
@@ -18,6 +25,7 @@ export type RemovePendingTransactionPayload = {
 
 export type AddPendingTransactionPayload = RemovePendingTransactionPayload & {
   txHash: string
+  block?: number
 }
 
 export type PendingTransactionPayloads = AddPendingTransactionPayload | RemovePendingTransactionPayload
@@ -29,11 +37,17 @@ export const pendingTransactionsReducer = handleActions<PendingTransactionsState
       action: Action<AddPendingTransactionPayload>,
     ) => {
       const chainId = _getChainId()
-      const { id, txHash } = action.payload
+      const { id, txHash, block } = action.payload
 
       return {
         ...state,
-        [chainId]: { ...state[chainId], [id]: txHash },
+        [chainId]: {
+          ...state[chainId],
+          [id]: {
+            txHash,
+            block,
+          },
+        },
       }
     },
     [PENDING_TRANSACTIONS_ACTIONS.REMOVE]: (

src/logic/safe/transactions/__tests__/pendingTxMonitor.test.ts

@@ -65,23 +65,7 @@ describe('PendingTxMonitor', () => {
 
   describe('monitorTx', () => {
     it("doesn't clear the pending transaction if it was mined", async () => {
-      PendingTxMonitor._isTxMined = jest.fn(() =>
-        Promise.resolve({
-          blockHash: '0x123',
-          blockNumber: 1,
-          transactionHash: 'fakeTxHash',
-          transactionIndex: 0,
-          from: '0x123',
-          to: '0x123',
-          cumulativeGasUsed: 1,
-          gasUsed: 1,
-          contractAddress: '0x123',
-          logs: [],
-          status: true, // Mined successfully
-          logsBloom: '0x123',
-          effectiveGasPrice: 0,
-        }),
-      )
+      PendingTxMonitor._isTxMined = jest.fn(() => Promise.resolve(true))
 
       const dispatchSpy = jest.spyOn(store.store, 'dispatch').mockImplementation(() => jest.fn())
 
@@ -94,23 +78,7 @@ describe('PendingTxMonitor', () => {
       expect(dispatchSpy).not.toBeCalled()
     })
     it('clears the pending transaction if it failed', async () => {
-      PendingTxMonitor._isTxMined = jest.fn(() =>
-        Promise.resolve({
-          blockHash: '0x123',
-          blockNumber: 1,
-          transactionHash: 'fakeTxHash',
-          transactionIndex: 0,
-          from: '0x123',
-          to: '0x123',
-          cumulativeGasUsed: 1,
-          gasUsed: 1,
-          contractAddress: '0x123',
-          logs: [],
-          status: false, // Mining failed
-          logsBloom: '0x123',
-          effectiveGasPrice: 0,
-        }),
-      )
+      PendingTxMonitor._isTxMined = jest.fn(() => Promise.resolve(false))
 
       const dispatchSpy = jest.spyOn(store.store, 'dispatch').mockImplementation(() => jest.fn())
 
@@ -127,8 +95,39 @@ describe('PendingTxMonitor', () => {
     })
 
     it('clears the pending transaction it the tx was not mined within 50 blocks', async () => {
-      // Can return null if transaction is pending: https://web3js.readthedocs.io/en/v1.2.11/web3-eth.html#gettransactionreceipt
-      PendingTxMonitor._isTxMined = jest.fn(() => Promise.reject(null as unknown as TransactionReceipt))
+      PendingTxMonitor._isTxMined = jest.fn(() => Promise.resolve(false))
+
+      const dispatchSpy = jest.spyOn(store.store, 'dispatch').mockImplementation(() => jest.fn())
+
+      await PendingTxMonitor.monitorTx(0, 'fakeTxId', 'fakeTxHash', {
+        numOfAttempts: 1,
+        startingDelay: 0,
+        timeMultiple: 0,
+      })
+
+      expect(dispatchSpy).toHaveBeenCalledTimes(1)
+    })
+
+    it('clears the pending transaction it retrieving the block number throws', async () => {
+      jest
+        .spyOn(web3.getWeb3().eth, 'getTransactionReceipt')
+        // Returns `null` if transaction is pending: https://web3js.readthedocs.io/en/v1.2.11/web3-eth.html#gettransactionreceipt
+        .mockImplementation(() => Promise.resolve(null as any))
+      jest.spyOn(web3.getWeb3().eth, 'getBlockNumber').mockImplementation(() => Promise.reject())
+
+      const dispatchSpy = jest.spyOn(store.store, 'dispatch').mockImplementation(() => jest.fn())
+
+      await PendingTxMonitor.monitorTx(0, 'fakeTxId', 'fakeTxHash', {
+        numOfAttempts: 1,
+        startingDelay: 0,
+        timeMultiple: 0,
+      })
+
+      expect(dispatchSpy).toHaveBeenCalledTimes(2)
+    })
+
+    it('clears the pending transaction it throws in the final backOff', async () => {
+      PendingTxMonitor._isTxMined = jest.fn(() => Promise.reject())
 
       const dispatchSpy = jest.spyOn(store.store, 'dispatch').mockImplementation(() => jest.fn())
 
@@ -185,9 +184,18 @@ describe('PendingTxMonitor', () => {
       jest.spyOn(store.store, 'getState').mockImplementation(() => ({
         pendingTransactions: {
           '4': {
-            fakeTxId: 'fakeTxHash',
-            fakeTxId2: 'fakeTxHash2',
-            fakeTxId3: 'fakeTxHash3',
+            fakeTxId: {
+              txHash: 'fakeTxHash',
+              block: 0,
+            },
+            fakeTxId2: {
+              txHash: 'fakeTxHash2',
+              block: 1,
+            },
+            fakeTxId3: {
+              txHash: 'fakeTxHash3',
+              block: 2,
+            },
           },
         },
         config: {
@@ -197,31 +205,37 @@ describe('PendingTxMonitor', () => {
 
       jest.spyOn(web3.getWeb3().eth, 'getBlockNumber').mockImplementation(() => Promise.resolve(0))
 
-      PendingTxMonitor._isTxMined = jest.fn(() =>
-        Promise.resolve({
-          blockHash: '0x123',
-          blockNumber: 1,
-          transactionHash: 'fakeTxHash',
-          transactionIndex: 0,
-          from: '0x123',
-          to: '0x123',
-          cumulativeGasUsed: 1,
-          gasUsed: 1,
-          contractAddress: '0x123',
-          logs: [],
-          status: true, // Mined successfully
-          logsBloom: '0x123',
-          effectiveGasPrice: 0,
-        }),
-      )
+      PendingTxMonitor._isTxMined = jest.fn(() => Promise.resolve(true))
 
       await PendingTxMonitor.monitorAllTxs()
 
       expect((PendingTxMonitor._isTxMined as jest.Mock).mock.calls).toEqual([
         [0, 'fakeTxHash'],
-        [0, 'fakeTxHash2'],
-        [0, 'fakeTxHash3'],
+        [1, 'fakeTxHash2'],
+        [2, 'fakeTxHash3'],
       ])
     })
+    it('falls back to the current block number if none was set', async () => {
+      jest.spyOn(store.store, 'getState').mockImplementation(() => ({
+        pendingTransactions: {
+          '4': {
+            fakeTxId: {
+              txHash: 'fakeTxHash',
+            },
+          },
+        },
+        config: {
+          chainId: '4',
+        },
+      }))
+
+      jest.spyOn(web3.getWeb3().eth, 'getBlockNumber').mockImplementation(() => Promise.resolve(0))
+
+      PendingTxMonitor._isTxMined = jest.fn(() => Promise.resolve(true))
+
+      await PendingTxMonitor.monitorAllTxs()
+
+      expect((PendingTxMonitor._isTxMined as jest.Mock).mock.calls).toEqual([[0, 'fakeTxHash']])
+    })
   })
 })

src/logic/safe/transactions/pendingTxMonitor.ts

@@ -1,5 +1,4 @@
 import { backOff, IBackOffOptions } from 'exponential-backoff'
-import { TransactionReceipt } from 'web3-core'
 
 import { NOTIFICATIONS } from 'src/logic/notifications'
 import enqueueSnackbar from 'src/logic/notifications/store/actions/enqueueSnackbar'
@@ -9,23 +8,23 @@ import { removePendingTransaction } from 'src/logic/safe/store/actions/pendingTr
 import { pendingTxIdsByChain } from 'src/logic/safe/store/selectors/pendingTransactions'
 import { didTxRevert } from 'src/logic/safe/store/actions/transactions/utils/transactionHelpers'
 
-const _isTxMined = async (sessionBlockNumber: number, txHash: string): Promise<TransactionReceipt> => {
-  const MAX_WAITING_BLOCK = sessionBlockNumber + 50
+const _isTxMined = async (blockNumber: number, txHash: string): Promise<boolean> => {
+  const MAX_WAITING_BLOCK = blockNumber + 50
 
   const web3 = getWeb3()
 
   const receipt = await web3.eth.getTransactionReceipt(txHash)
 
-  if (
-    // Transaction hasn't yet been mined
-    !receipt &&
-    // The current block is within waiting window
-    (await web3.eth.getBlockNumber()) <= MAX_WAITING_BLOCK
-  ) {
+  if (receipt) {
+    return !didTxRevert(receipt)
+  }
+
+  if ((await web3.eth.getBlockNumber()) <= MAX_WAITING_BLOCK) {
+    // backOff retries
     throw new Error('Pending transaction not found')
   }
 
-  return receipt
+  return false
 }
 
 // Progressively after 10s, 20s, 40s, 80s, 160s, 320s - total of 6.5 minutes
@@ -44,12 +43,11 @@ const monitorTx = async (
   },
 ): Promise<void> => {
   return backOff(() => PendingTxMonitor._isTxMined(sessionBlockNumber, txHash), options)
-    .then((receipt) => {
-      if (didTxRevert(receipt)) {
+    .then((isMined) => {
+      if (!isMined) {
         store.dispatch(removePendingTransaction({ id: txId }))
       }
-      // If successfully mined, pending status is removed in the transaction
-      // middleware when a transaction is added to historical transactions list
+      // If successfully mined the transaction will be removed by the automatic polling
     })
     .catch(() => {
       // Unsuccessfully mined (threw in last backOff attempt)
@@ -72,8 +70,8 @@ const monitorAllTxs = async (): Promise<void> => {
   try {
     const sessionBlockNumber = await web3.eth.getBlockNumber()
     await Promise.all(
-      pendingTxs.map(([txId, txHash]) => {
-        return PendingTxMonitor.monitorTx(sessionBlockNumber, txId, txHash)
+      pendingTxs.map(([txId, { txHash, block = sessionBlockNumber }]) => {
+        return PendingTxMonitor.monitorTx(block, txId, txHash)
       }),
     )
   } catch {