improvement(transactions): some clean up and unit tests

- fetch transaction state less often for old transactions
- fix a bug calling non payable methods with value 0

src/connectors/index.ts

@@ -6,7 +6,6 @@ import { PortisConnector } from '@web3-react/portis-connector'
 import { FortmaticConnector } from './Fortmatic'
 import { NetworkConnector } from './NetworkConnector'
 
-const POLLING_INTERVAL = 12500
 const NETWORK_URL = process.env.REACT_APP_NETWORK_URL
 const FORMATIC_KEY = process.env.REACT_APP_FORTMATIC_KEY
 const PORTIS_ID = process.env.REACT_APP_PORTIS_ID
@@ -28,7 +27,7 @@ export const walletconnect = new WalletConnectConnector({
   rpc: { 1: NETWORK_URL },
   bridge: 'https://bridge.walletconnect.org',
   qrcode: true,
-  pollingInterval: POLLING_INTERVAL
+  pollingInterval: 15000
 })
 
 // mainnet only

src/hooks/useSwapCallback.ts

@@ -12,6 +12,10 @@ import { useV1ExchangeContract } from './useContract'
 import useENS from './useENS'
 import { Version } from './useToggledVersion'
 
+function isZero(hexNumber: string) {
+  return /^0x0*$/.test(hexNumber)
+}
+
 // returns a function that will execute a swap, if the parameters are all valid
 // and the user has approved the slippage adjusted input amount for the trade
 export function useSwapCallback(
@@ -76,7 +80,7 @@ export function useSwapCallback(
 
       const safeGasEstimates: (BigNumber | undefined)[] = await Promise.all(
         swapMethods.map(({ args, methodName, value }) =>
-          contract.estimateGas[methodName](...args, value ? { value } : {})
+          contract.estimateGas[methodName](...args, value && !isZero(value) ? { value } : {})
             .then(calculateGasMargin)
             .catch(error => {
               console.error(`estimateGas failed for ${methodName}`, error)
@@ -127,7 +131,7 @@ export function useSwapCallback(
 
         return contract[methodName](...args, {
           gasLimit: safeGasEstimate,
-          ...(value ? { value } : {})
+          ...(value && !isZero(value) ? { value } : {})
         })
           .then((response: any) => {
             const inputSymbol = trade.inputAmount.currency.symbol

src/state/transactions/actions.ts

@@ -1,4 +1,5 @@
 import { createAction } from '@reduxjs/toolkit'
+import { ChainId } from '@uniswap/sdk'
 
 export interface SerializableTransactionReceipt {
   to: string
@@ -12,15 +13,20 @@ export interface SerializableTransactionReceipt {
 }
 
 export const addTransaction = createAction<{
-  chainId: number
+  chainId: ChainId
   hash: string
   from: string
   approval?: { tokenAddress: string; spender: string }
   summary?: string
-}>('addTransaction')
-export const clearAllTransactions = createAction<{ chainId: number }>('clearAllTransactions')
+}>('transactions/addTransaction')
+export const clearAllTransactions = createAction<{ chainId: ChainId }>('transactions/clearAllTransactions')
 export const finalizeTransaction = createAction<{
-  chainId: number
+  chainId: ChainId
   hash: string
   receipt: SerializableTransactionReceipt
-}>('finalizeTransaction')
+}>('transactions/finalizeTransaction')
+export const checkedTransaction = createAction<{
+  chainId: ChainId
+  hash: string
+  blockNumber: number
+}>('transactions/checkedTransaction')

src/state/transactions/hooks.tsx

@@ -5,7 +5,7 @@ import { useDispatch, useSelector } from 'react-redux'
 import { useActiveWeb3React } from '../../hooks'
 import { AppDispatch, AppState } from '../index'
 import { addTransaction } from './actions'
-import { TransactionDetails, TransactionState } from './reducer'
+import { TransactionDetails } from './reducer'
 
 // helper that can take a ethers library transaction response and add it to the list of transactions
 export function useTransactionAdder(): (
@@ -37,7 +37,7 @@ export function useTransactionAdder(): (
 export function useAllTransactions(): { [txHash: string]: TransactionDetails } {
   const { chainId } = useActiveWeb3React()
 
-  const state = useSelector<AppState, TransactionState>(state => state.transactions)
+  const state = useSelector<AppState, AppState['transactions']>(state => state.transactions)
 
   return chainId ? state[chainId] ?? {} : {}
 }

src/state/transactions/reducer.test.ts

@@ -0,0 +1,192 @@
+import { ChainId } from '@uniswap/sdk'
+import { createStore, Store } from 'redux'
+import { addTransaction, checkedTransaction, clearAllTransactions, finalizeTransaction } from './actions'
+import reducer, { initialState, TransactionState } from './reducer'
+
+describe('transaction reducer', () => {
+  let store: Store<TransactionState>
+
+  beforeEach(() => {
+    store = createStore(reducer, initialState)
+  })
+
+  describe('addTransaction', () => {
+    it('adds the transaction', () => {
+      const beforeTime = new Date().getTime()
+      store.dispatch(
+        addTransaction({
+          chainId: ChainId.MAINNET,
+          summary: 'hello world',
+          hash: '0x0',
+          approval: { tokenAddress: 'abc', spender: 'def' },
+          from: 'abc'
+        })
+      )
+      const txs = store.getState()
+      expect(txs[ChainId.MAINNET]).toBeTruthy()
+      expect(txs[ChainId.MAINNET]?.['0x0']).toBeTruthy()
+      const tx = txs[ChainId.MAINNET]?.['0x0']
+      expect(tx).toBeTruthy()
+      expect(tx?.hash).toEqual('0x0')
+      expect(tx?.summary).toEqual('hello world')
+      expect(tx?.approval).toEqual({ tokenAddress: 'abc', spender: 'def' })
+      expect(tx?.from).toEqual('abc')
+      expect(tx?.addedTime).toBeGreaterThanOrEqual(beforeTime)
+    })
+  })
+
+  describe('finalizeTransaction', () => {
+    it('no op if not valid transaction', () => {
+      store.dispatch(
+        finalizeTransaction({
+          chainId: ChainId.RINKEBY,
+          hash: '0x0',
+          receipt: {
+            status: 1,
+            transactionIndex: 1,
+            transactionHash: '0x0',
+            to: '0x0',
+            from: '0x0',
+            contractAddress: '0x0',
+            blockHash: '0x0',
+            blockNumber: 1
+          }
+        })
+      )
+      expect(store.getState()).toEqual({})
+    })
+    it('sets receipt', () => {
+      store.dispatch(
+        addTransaction({
+          hash: '0x0',
+          chainId: ChainId.RINKEBY,
+          approval: { spender: '0x0', tokenAddress: '0x0' },
+          summary: 'hello world',
+          from: '0x0'
+        })
+      )
+      const beforeTime = new Date().getTime()
+      store.dispatch(
+        finalizeTransaction({
+          chainId: ChainId.RINKEBY,
+          hash: '0x0',
+          receipt: {
+            status: 1,
+            transactionIndex: 1,
+            transactionHash: '0x0',
+            to: '0x0',
+            from: '0x0',
+            contractAddress: '0x0',
+            blockHash: '0x0',
+            blockNumber: 1
+          }
+        })
+      )
+      const tx = store.getState()[ChainId.RINKEBY]?.['0x0']
+      expect(tx?.summary).toEqual('hello world')
+      expect(tx?.confirmedTime).toBeGreaterThanOrEqual(beforeTime)
+      expect(tx?.receipt).toEqual({
+        status: 1,
+        transactionIndex: 1,
+        transactionHash: '0x0',
+        to: '0x0',
+        from: '0x0',
+        contractAddress: '0x0',
+        blockHash: '0x0',
+        blockNumber: 1
+      })
+    })
+  })
+
+  describe('checkedTransaction', () => {
+    it('no op if not valid transaction', () => {
+      store.dispatch(
+        checkedTransaction({
+          chainId: ChainId.RINKEBY,
+          hash: '0x0',
+          blockNumber: 1
+        })
+      )
+      expect(store.getState()).toEqual({})
+    })
+    it('sets lastCheckedBlockNumber', () => {
+      store.dispatch(
+        addTransaction({
+          hash: '0x0',
+          chainId: ChainId.RINKEBY,
+          approval: { spender: '0x0', tokenAddress: '0x0' },
+          summary: 'hello world',
+          from: '0x0'
+        })
+      )
+      store.dispatch(
+        checkedTransaction({
+          chainId: ChainId.RINKEBY,
+          hash: '0x0',
+          blockNumber: 1
+        })
+      )
+      const tx = store.getState()[ChainId.RINKEBY]?.['0x0']
+      expect(tx?.lastCheckedBlockNumber).toEqual(1)
+    })
+    it('never decreases', () => {
+      store.dispatch(
+        addTransaction({
+          hash: '0x0',
+          chainId: ChainId.RINKEBY,
+          approval: { spender: '0x0', tokenAddress: '0x0' },
+          summary: 'hello world',
+          from: '0x0'
+        })
+      )
+      store.dispatch(
+        checkedTransaction({
+          chainId: ChainId.RINKEBY,
+          hash: '0x0',
+          blockNumber: 3
+        })
+      )
+      store.dispatch(
+        checkedTransaction({
+          chainId: ChainId.RINKEBY,
+          hash: '0x0',
+          blockNumber: 1
+        })
+      )
+      const tx = store.getState()[ChainId.RINKEBY]?.['0x0']
+      expect(tx?.lastCheckedBlockNumber).toEqual(3)
+    })
+  })
+
+  describe('clearAllTransactions', () => {
+    it('removes all transactions for the chain', () => {
+      store.dispatch(
+        addTransaction({
+          chainId: ChainId.MAINNET,
+          summary: 'hello world',
+          hash: '0x0',
+          approval: { tokenAddress: 'abc', spender: 'def' },
+          from: 'abc'
+        })
+      )
+      store.dispatch(
+        addTransaction({
+          chainId: ChainId.RINKEBY,
+          summary: 'hello world',
+          hash: '0x1',
+          approval: { tokenAddress: 'abc', spender: 'def' },
+          from: 'abc'
+        })
+      )
+      expect(Object.keys(store.getState())).toHaveLength(2)
+      expect(Object.keys(store.getState())).toEqual([String(ChainId.MAINNET), String(ChainId.RINKEBY)])
+      expect(Object.keys(store.getState()[ChainId.MAINNET] ?? {})).toEqual(['0x0'])
+      expect(Object.keys(store.getState()[ChainId.RINKEBY] ?? {})).toEqual(['0x1'])
+      store.dispatch(clearAllTransactions({ chainId: ChainId.MAINNET }))
+      expect(Object.keys(store.getState())).toHaveLength(2)
+      expect(Object.keys(store.getState())).toEqual([String(ChainId.MAINNET), String(ChainId.RINKEBY)])
+      expect(Object.keys(store.getState()[ChainId.MAINNET] ?? {})).toEqual([])
+      expect(Object.keys(store.getState()[ChainId.RINKEBY] ?? {})).toEqual(['0x1'])
+    })
+  })
+})

src/state/transactions/reducer.ts

@@ -1,5 +1,11 @@
 import { createReducer } from '@reduxjs/toolkit'
-import { addTransaction, clearAllTransactions, finalizeTransaction, SerializableTransactionReceipt } from './actions'
+import {
+  addTransaction,
+  checkedTransaction,
+  clearAllTransactions,
+  finalizeTransaction,
+  SerializableTransactionReceipt
+} from './actions'
 
 const now = () => new Date().getTime()
 
@@ -8,12 +14,10 @@ export interface TransactionDetails {
   approval?: { tokenAddress: string; spender: string }
   summary?: string
   receipt?: SerializableTransactionReceipt
+  lastCheckedBlockNumber?: number
   addedTime: number
   confirmedTime?: number
   from: string
-
-  // set to true when we receive a transaction count that exceeds the nonce of this transaction
-  unknownStatus?: boolean
 }
 
 export interface TransactionState {
@@ -22,28 +26,39 @@ export interface TransactionState {
   }
 }
 
-const initialState: TransactionState = {}
+export const initialState: TransactionState = {}
 
 export default createReducer(initialState, builder =>
   builder
-    .addCase(addTransaction, (state, { payload: { chainId, from, hash, approval, summary } }) => {
-      if (state[chainId]?.[hash]) {
+    .addCase(addTransaction, (transactions, { payload: { chainId, from, hash, approval, summary } }) => {
+      if (transactions[chainId]?.[hash]) {
         throw Error('Attempted to add existing transaction.')
       }
-      state[chainId] = state[chainId] ?? {}
-      state[chainId][hash] = { hash, approval, summary, from, addedTime: now() }
+      const txs = transactions[chainId] ?? {}
+      txs[hash] = { hash, approval, summary, from, addedTime: now() }
+      transactions[chainId] = txs
     })
-    .addCase(clearAllTransactions, (state, { payload: { chainId } }) => {
-      if (!state[chainId]) return
-      state[chainId] = {}
+    .addCase(clearAllTransactions, (transactions, { payload: { chainId } }) => {
+      if (!transactions[chainId]) return
+      transactions[chainId] = {}
     })
-    .addCase(finalizeTransaction, (state, { payload: { hash, chainId, receipt } }) => {
-      if (!state[chainId]?.[hash]) {
-        throw Error('Attempted to finalize non-existent transaction.')
+    .addCase(checkedTransaction, (transactions, { payload: { chainId, hash, blockNumber } }) => {
+      const tx = transactions[chainId]?.[hash]
+      if (!tx) {
+        return
       }
-      state[chainId] = state[chainId] ?? {}
-      state[chainId][hash].receipt = receipt
-      state[chainId][hash].unknownStatus = false
-      state[chainId][hash].confirmedTime = now()
+      if (!tx.lastCheckedBlockNumber) {
+        tx.lastCheckedBlockNumber = blockNumber
+      } else {
+        tx.lastCheckedBlockNumber = Math.max(blockNumber, tx.lastCheckedBlockNumber)
+      }
+    })
+    .addCase(finalizeTransaction, (transactions, { payload: { hash, chainId, receipt } }) => {
+      const tx = transactions[chainId]?.[hash]
+      if (!tx) {
+        return
+      }
+      tx.receipt = receipt
+      tx.confirmedTime = now()
     })
 )

src/state/transactions/updater.test.ts

@@ -0,0 +1,35 @@
+import { shouldCheck } from './updater'
+
+describe('transactions updater', () => {
+  describe('shouldCheck', () => {
+    it('returns true if no receipt and never checked', () => {
+      expect(shouldCheck(10, { addedTime: 100 })).toEqual(true)
+    })
+    it('returns false if has receipt and never checked', () => {
+      expect(shouldCheck(10, { addedTime: 100, receipt: {} })).toEqual(false)
+    })
+    it('returns true if has not been checked in 1 blocks', () => {
+      expect(shouldCheck(10, { addedTime: new Date().getTime(), lastCheckedBlockNumber: 9 })).toEqual(true)
+    })
+    it('returns false if checked in last 3 blocks and greater than 20 minutes old', () => {
+      expect(shouldCheck(10, { addedTime: new Date().getTime() - 21 * 60 * 1000, lastCheckedBlockNumber: 8 })).toEqual(
+        false
+      )
+    })
+    it('returns true if not checked in last 5 blocks and greater than 20 minutes old', () => {
+      expect(shouldCheck(10, { addedTime: new Date().getTime() - 21 * 60 * 1000, lastCheckedBlockNumber: 5 })).toEqual(
+        true
+      )
+    })
+    it('returns false if checked in last 10 blocks and greater than 60 minutes old', () => {
+      expect(shouldCheck(20, { addedTime: new Date().getTime() - 61 * 60 * 1000, lastCheckedBlockNumber: 11 })).toEqual(
+        false
+      )
+    })
+    it('returns true if checked in last 3 blocks and greater than 20 minutes old', () => {
+      expect(shouldCheck(20, { addedTime: new Date().getTime() - 61 * 60 * 1000, lastCheckedBlockNumber: 10 })).toEqual(
+        true
+      )
+    })
+  })
+})

src/state/transactions/updater.tsx

@@ -3,7 +3,28 @@ import { useDispatch, useSelector } from 'react-redux'
 import { useActiveWeb3React } from '../../hooks'
 import { useAddPopup, useBlockNumber } from '../application/hooks'
 import { AppDispatch, AppState } from '../index'
-import { finalizeTransaction } from './actions'
+import { checkedTransaction, finalizeTransaction } from './actions'
+
+export function shouldCheck(
+  lastBlockNumber: number,
+  tx: { addedTime: number; receipt?: {}; lastCheckedBlockNumber?: number }
+): boolean {
+  if (tx.receipt) return false
+  if (!tx.lastCheckedBlockNumber) return true
+  const blocksSinceCheck = lastBlockNumber - tx.lastCheckedBlockNumber
+  if (blocksSinceCheck < 1) return false
+  const minutesPending = (new Date().getTime() - tx.addedTime) / 1000 / 60
+  if (minutesPending > 60) {
+    // every 10 blocks if pending for longer than an hour
+    return blocksSinceCheck > 9
+  } else if (minutesPending > 5) {
+    // every 3 blocks if pending more than 5 minutes
+    return blocksSinceCheck > 2
+  } else {
+    // otherwise every block
+    return true
+  }
+}
 
 export default function Updater() {
   const { chainId, library } = useActiveWeb3React()
@@ -11,9 +32,9 @@ export default function Updater() {
   const lastBlockNumber = useBlockNumber()
 
   const dispatch = useDispatch<AppDispatch>()
-  const transactions = useSelector<AppState, AppState['transactions']>(state => state.transactions)
+  const state = useSelector<AppState, AppState['transactions']>(state => state.transactions)
 
-  const allTransactions = chainId ? transactions[chainId] ?? {} : {}
+  const transactions = chainId ? state[chainId] ?? {} : {}
 
   // show popup on confirm
   const addPopup = useAddPopup()
@@ -21,8 +42,8 @@ export default function Updater() {
   useEffect(() => {
     if (!chainId || !library || !lastBlockNumber) return
 
-    Object.keys(allTransactions)
-      .filter(hash => !allTransactions[hash].receipt)
+    Object.keys(transactions)
+      .filter(hash => shouldCheck(lastBlockNumber, transactions[hash]))
       .forEach(hash => {
         library
           .getTransactionReceipt(hash)
@@ -50,18 +71,20 @@ export default function Updater() {
                   txn: {
                     hash,
                     success: receipt.status === 1,
-                    summary: allTransactions[hash]?.summary
+                    summary: transactions[hash]?.summary
                   }
                 },
                 hash
               )
+            } else {
+              dispatch(checkedTransaction({ chainId, hash, blockNumber: lastBlockNumber }))
             }
           })
           .catch(error => {
             console.error(`failed to check transaction hash: ${hash}`, error)
           })
       })
-  }, [chainId, library, allTransactions, lastBlockNumber, dispatch, addPopup])
+  }, [chainId, library, transactions, lastBlockNumber, dispatch, addPopup])
 
   return null
 }

src/theme/index.tsx

@@ -175,6 +175,7 @@ html, input, textarea, button {
 @supports (font-variation-settings: normal) {
   html, input, textarea, button {
     font-family: 'Inter var', sans-serif;
+    font-display: fallback;
   }
 }