Refactor tx monitor

package.json

@@ -248,7 +248,7 @@
     "@testing-library/react": "^11.2.7",
     "@typechain/web3-v1": "^3.0.0",
     "@types/history": "4.6.2",
-    "@types/jest": "^26.0.22",
+    "@types/jest": "^26.0.23",
     "@types/js-cookie": "^2.2.6",
     "@types/lodash.get": "^4.4.6",
     "@types/lodash.memoize": "^4.1.6",

src/logic/exceptions/registry.ts

@@ -30,6 +30,7 @@ enum ErrorCodes {
   _802 = '802: Error submitting a transaction, safeAddress not found',
   _803 = '803: Error creating a transaction',
   _804 = '804: Error processing a transaction',
+  _805 = '805: TX monitor error',
   _900 = '900: Error loading Safe App',
   _901 = '901: Error processing Safe Apps SDK request',
   _902 = '902: Error loading Safe Apps list',

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

@@ -0,0 +1,122 @@
+import { txMonitor } from 'src/logic/safe/transactions/txMonitor'
+import { web3ReadOnly } from 'src/logic/wallets/getWeb3'
+
+jest.mock('src/logic/wallets/getWeb3', () => ({
+  web3ReadOnly: {
+    eth: {},
+  },
+}))
+
+const params = {
+  sender: '0x474e5Ded6b5D078163BFB8F6dBa355C3aA5478C8',
+  hash: '0x510bec3129a8dcc57075b67de6292ada338fa05518d18ec81b2cda3cea593a64',
+  nonce: 1,
+  data: '0',
+  gasPrice: '1',
+}
+
+const options = {
+  delay: 0,
+  maxRetries: 10,
+}
+
+describe('txMonitor', () => {
+  beforeEach(() => {
+    web3ReadOnly.eth.getTransaction = jest.fn(() => Promise.reject('getTransaction')) as any
+    web3ReadOnly.eth.getTransactionReceipt = jest.fn(() => Promise.reject('getTransactionReceipt')) as any
+    web3ReadOnly.eth.getBlock = jest.fn(() => Promise.reject('getBlock')) as any
+  })
+
+  it('should reject when max retries are reached', async () => {
+    try {
+      await txMonitor(params, options)
+      expect(false).toBe('Should not go here')
+    } catch (e) {
+      expect(e.message).toBe('Code 805: TX monitor error (max retries reached)')
+    }
+    expect(web3ReadOnly.eth.getTransaction).toHaveBeenCalledTimes(0)
+    expect(web3ReadOnly.eth.getTransactionReceipt).toHaveBeenCalledTimes(11)
+  })
+
+  it('should load original tx if nonce is undefined', async () => {
+    web3ReadOnly.eth.getTransaction = jest.fn(() => Promise.resolve({ nonce: 1, gasPrice: 1 })) as any
+    web3ReadOnly.eth.getTransactionReceipt = jest.fn((hash) => Promise.resolve({ hash, status: 'success' })) as any
+
+    await expect(txMonitor({ ...params, nonce: undefined }, options)).resolves.toEqual({
+      status: 'success',
+      hash: '0x510bec3129a8dcc57075b67de6292ada338fa05518d18ec81b2cda3cea593a64',
+    })
+    expect(web3ReadOnly.eth.getTransaction).toHaveBeenCalledTimes(1)
+    expect(web3ReadOnly.eth.getTransactionReceipt).toHaveBeenCalledTimes(1)
+  })
+
+  it('should fail if it cannot load the original tx receipt', async () => {
+    web3ReadOnly.eth.getTransaction = jest.fn(() => Promise.resolve({ nonce: 1, gasPrice: 1 })) as any
+    web3ReadOnly.eth.getTransactionReceipt = jest.fn(() => Promise.reject('No receipt'))
+
+    try {
+      await txMonitor(params, options)
+      expect(false).toBe('Should not go here')
+    } catch (e) {
+      expect(e.message).toBe('Code 805: TX monitor error (max retries reached)')
+    }
+
+    expect(web3ReadOnly.eth.getTransaction).toHaveBeenCalledTimes(0)
+    expect(web3ReadOnly.eth.getTransactionReceipt).toHaveBeenCalledTimes(11)
+  })
+
+  it('should return speed-up tx receipt', async () => {
+    web3ReadOnly.eth.getBlock = jest.fn(() =>
+      Promise.resolve({
+        transactions: [
+          {
+            hash: '0xSPEEDY',
+            from: params.sender,
+            nonce: params.nonce,
+            input: params.data,
+          },
+        ],
+      }),
+    ) as any
+
+    web3ReadOnly.eth.getTransactionReceipt = jest.fn((hash) => {
+      return hash === '0xSPEEDY'
+        ? Promise.resolve({ hash, status: 'success' } as any)
+        : Promise.reject('No original receipt')
+    })
+
+    await expect(txMonitor(params, options)).resolves.toEqual({
+      status: 'success',
+      hash: '0xSPEEDY',
+    })
+    expect(web3ReadOnly.eth.getBlock).toHaveBeenCalledTimes(1)
+    expect(web3ReadOnly.eth.getTransactionReceipt).toHaveBeenCalledTimes(2)
+  })
+
+  it('should fail if it cannot find a speed-up tx', async () => {
+    web3ReadOnly.eth.getBlock = jest.fn(() =>
+      Promise.resolve({
+        transactions: [
+          {
+            hash: '0x123',
+            from: 'Someone',
+            nonce: 12,
+            input: '123',
+          },
+        ],
+      }),
+    ) as any
+
+    web3ReadOnly.eth.getTransactionReceipt = jest.fn(() => Promise.reject('No original receipt'))
+
+    try {
+      await txMonitor(params, options)
+      expect(false).toBe('Should not go here')
+    } catch (e) {
+      expect(e.message).toBe('Code 805: TX monitor error (max retries reached)')
+    }
+
+    expect(web3ReadOnly.eth.getBlock).toHaveBeenCalledTimes(11)
+    expect(web3ReadOnly.eth.getTransactionReceipt).toHaveBeenCalledTimes(11)
+  })
+})

src/logic/safe/transactions/txMonitor.ts

@@ -1,8 +1,9 @@
-import { TransactionReceipt } from 'web3-core'
+import { Transaction, TransactionReceipt } from 'web3-core'
 
 import { web3ReadOnly } from 'src/logic/wallets/getWeb3'
 import { sameAddress } from 'src/logic/wallets/ethAddresses'
 import { sameString } from 'src/utils/strings'
+import { CodedException, Errors } from 'src/logic/exceptions/CodedException'
 
 type TxMonitorProps = {
   sender: string
@@ -14,6 +15,27 @@ type TxMonitorProps = {
 
 type TxMonitorOptions = {
   delay?: number
+  maxRetries?: number
+}
+
+const MAX_RETRIES = 720
+const DEFAULT_DELAY = 5000
+
+async function findSpeedupTx({ sender, hash, nonce, data }: TxMonitorProps): Promise<Transaction | undefined> {
+  const latestBlock = await web3ReadOnly.eth.getBlock('latest', true)
+
+  const replacementTransaction = latestBlock.transactions.find((transaction) => {
+    // TODO: use gasPrice, timestamp or another better way to differentiate
+    return (
+      sameAddress(transaction.from, sender) &&
+      transaction.nonce === nonce &&
+      !sameString(transaction.hash, hash) &&
+      // if `data` differs, then it's a replacement tx, not a speedup
+      sameString(transaction.input, data)
+    )
+  })
+
+  return replacementTransaction
 }
 
 /**
@@ -25,74 +47,88 @@ type TxMonitorOptions = {
  * @param {string} txParams.data
  * @param {number | undefined} txParams.nonce
  * @param {string | undefined} txParams.gasPrice
- * @param {function(txReceipt: TransactionReceipt): void} cb - called with the tx receipt as argument when tx is mined
  * @param {object} options
  * @param {number} options.delay
+ * @returns {Promise<TransactionReceipt>}
  */
-export const txMonitor = async (
+export const txMonitor = (
   { sender, hash, data, nonce, gasPrice }: TxMonitorProps,
-  cb: (txReceipt: TransactionReceipt) => void,
-  tries = 0,
   options?: TxMonitorOptions,
-): Promise<void> => {
-  if (tries > 720) {
+  tries = 0,
+): Promise<TransactionReceipt> => {
+  return new Promise<TransactionReceipt>((resolve, reject) => {
+    const { maxRetries = MAX_RETRIES } = options || {}
+    if (tries > maxRetries) {
+      reject(new CodedException(Errors._805, 'max retries reached'))
       return
     }
-  setTimeout(async () => {
-    if (nonce === undefined || gasPrice === undefined) {
-      // this block is accessed only the first time, to lookup the tx nonce and gasPrice
-      // find the nonce for the current tx
-      const transaction = await web3ReadOnly.eth.getTransaction(hash)
 
-      if (transaction !== null) {
-        // transaction found
-        return txMonitor(
-          { sender, hash, data, nonce: transaction.nonce, gasPrice: transaction.gasPrice },
-          cb,
-          tries + 1,
-          options,
-        )
-      } else {
-        return txMonitor({ sender, hash, data }, cb, tries + 1, options)
+    const monitorFn = async (): Promise<unknown> => {
+      // Case 1: this block is accessed for the first time, no nonce
+      if (nonce == null || gasPrice == null) {
+        let params: TxMonitorProps = { sender, hash, data }
+        try {
+          // Find the nonce for the current tx
+          const transaction = await web3ReadOnly.eth.getTransaction(hash)
+          if (transaction) {
+            params = { ...params, nonce: transaction.nonce, gasPrice: transaction.gasPrice }
+          }
+        } catch (e) {
+          // ignore error
         }
+
+        return txMonitor(params, options, tries + 1)
+          .then(resolve)
+          .catch(reject)
       }
+
+      // Case 2: the nonce exists, try to get the receipt for the original tx
+      try {
         const firstTxReceipt = await web3ReadOnly.eth.getTransactionReceipt(hash)
         if (firstTxReceipt) {
-      return
+          return resolve(firstTxReceipt)
+        }
+      } catch (e) {
+        // proceed to case 3
       }
-    const latestBlock = await web3ReadOnly.eth.getBlock('latest', true)
 
-    const replacementTransaction = latestBlock.transactions.find((transaction) => {
-      // TODO: use gasPrice, timestamp or another better way to differentiate
-      return (
-        sameAddress(transaction.from, sender) &&
-        transaction.nonce === nonce &&
-        !sameString(transaction.hash, hash) &&
-        // if `data` differs, then it's a replacement tx, not a speedup
-        sameString(transaction.input, data)
-      )
-    })
-    if (replacementTransaction) {
-      const transactionReceipt = await web3ReadOnly.eth.getTransactionReceipt(replacementTransaction.hash)
-      if (transactionReceipt === null) {
-        // pending transaction
+      // Case 3: original tx not found, try to find a sped-up tx
+      try {
+        const replacementTx = await findSpeedupTx({ sender, hash, nonce, data })
+
+        if (replacementTx) {
+          const replacementReceipt = await web3ReadOnly.eth.getTransactionReceipt(replacementTx.hash)
+
+          // goal achieved
+          if (replacementReceipt) {
+            return resolve(replacementReceipt)
+          }
+
+          // tx exists but no receipt yet, it's pending
           return txMonitor(
             {
               sender,
-            hash: replacementTransaction.hash,
-            data: replacementTransaction.input,
               nonce,
-            gasPrice: replacementTransaction.gasPrice,
+              hash: replacementTx.hash,
+              data: replacementTx.input,
+              gasPrice: replacementTx.gasPrice,
             },
-          cb,
-          tries + 1,
             options,
+            tries + 1,
           )
+            .then(resolve)
+            .catch(reject)
         }
-      cb(transactionReceipt)
-      return
+      } catch (e) {
+        // ignore error
       }
 
-    return txMonitor({ sender, hash, data, nonce, gasPrice }, cb, tries + 1, options)
-  }, options?.delay ?? 5000)
+      // Neither the original nor a replacement transactions were found, try again
+      txMonitor({ sender, hash, data, nonce, gasPrice }, options, tries + 1)
+        .then(resolve)
+        .catch(reject)
+    }
+
+    setTimeout(monitorFn, options?.delay ?? DEFAULT_DELAY)
+  })
 }

src/routes/open/container/Open.tsx

@@ -84,17 +84,24 @@ export const createSafe = async (values: CreateSafeValues, userAccount: string):
     const ownerAddresses = getAccountsFrom(values)
     const safeCreationSalt = getSafeCreationSaltFrom(values)
     const deploymentTx = getSafeDeploymentTransaction(ownerAddresses, confirmations, safeCreationSalt)
+
     deploymentTx
       .send({
         from: userAccount,
         gas: values?.gasLimit,
       })
       .once('transactionHash', (txHash) => {
         saveToStorage(SAFE_PENDING_CREATION_STORAGE_KEY, { txHash, ...values })
-        txMonitor({ sender: userAccount, hash: txHash, data: deploymentTx.encodeABI() }, (txReceipt) => {
+
+        // Monitor the latest block to find a potential speed-up tx
+        txMonitor({ sender: userAccount, hash: txHash, data: deploymentTx.encodeABI() })
+          .then((txReceipt) => {
             console.log('Speed up tx mined:', txReceipt)
             resolve(txReceipt)
           })
+          .catch((error) => {
+            reject(error)
+          })
       })
       .then((txReceipt) => {
         console.log('First tx mined:', txReceipt)

yarn.lock

@@ -3748,7 +3748,7 @@
   dependencies:
     "@types/istanbul-lib-report" "*"
 
-"@types/jest@*", "@types/jest@^26.0.22":
+"@types/jest@*", "@types/jest@^26.0.23":
   version "26.0.23"
   resolved "https://registry.yarnpkg.com/@types/jest/-/jest-26.0.23.tgz#a1b7eab3c503b80451d019efb588ec63522ee4e7"
   integrity sha512-ZHLmWMJ9jJ9PTiT58juykZpL7KjwJywFN3Rr2pTSkyQfydf/rk22yS7W8p5DaVUMQ2BQC7oYiU3FjbTM/mYrOA==