fix: Change default gas estimation state from 0 to undefined (#3964)

* fix: Change default gas estimation state from 0 to undefined * fix: Return 0 for maxPrioFee if pre EIP-1559 and gasPrice estimation throws * chore: Add more unit tests
5afe · Jun 13, 2022 · e19ba47 · e19ba47
1 parent 097d070
commit e19ba47
Show file tree

Hide file tree

Showing 8 changed files with 121 additions and 36 deletions.
diff --git a/src/logic/hooks/__tests__/useEstimateTransactionGas.test.ts b/src/logic/hooks/__tests__/useEstimateTransactionGas.test.ts
@@ -25,10 +25,10 @@ describe('useEstimateTransactionGas', () => {
       isExecution: true,
     }
     initialState = {
-      gasPrice: '0',
-      gasPriceFormatted: '0',
-      gasMaxPrioFee: '0',
-      gasMaxPrioFeeFormatted: '0',
+      gasPrice: undefined,
+      gasPriceFormatted: undefined,
+      gasMaxPrioFee: undefined,
+      gasMaxPrioFeeFormatted: undefined,
     }
     failureState = {
       gasPrice: DEFAULT_MAX_GAS_FEE.toString(),
@@ -110,6 +110,21 @@ describe('useEstimateTransactionGas', () => {
     })
   })
 
+  it('returns 0 for maxPrioFee pre EIP-1559 if calculateGasPrice throws', async () => {
+    jest.spyOn(gas, 'isMaxFeeParam').mockImplementation(() => false)
+    jest.spyOn(ethTransactions, 'calculateGasPrice').mockImplementation(() => {
+      throw new Error()
+    })
+
+    const { result } = renderHook(() => useEstimateTransactionGas(mockParams))
+
+    await waitFor(() => {
+      expect(result.current.gasMaxPrioFee).toBe('0')
+      expect(result.current.gasMaxPrioFeeFormatted).toBe('0')
+      expect(prioFeeEstimationSpy).toHaveBeenCalledTimes(0)
+    })
+  })
+
   it('returns failure state if getFeesPerGas throws', async () => {
     jest.spyOn(gas, 'isMaxFeeParam').mockImplementation(() => true)
     jest.spyOn(ethTransactions, 'getFeesPerGas').mockImplementation(() => {

diff --git a/src/logic/hooks/__tests__/useEstimationStatus.test.ts b/src/logic/hooks/__tests__/useEstimationStatus.test.ts
@@ -0,0 +1,31 @@
+import { renderHook } from '@testing-library/react-hooks'
+import { waitFor } from 'src/utils/test-utils'
+import { EstimationStatus } from 'src/logic/hooks/useEstimateTransactionGas'
+import { useEstimationStatus } from 'src/logic/hooks/useEstimationStatus'
+import { ButtonStatus } from 'src/components/Modal'
+
+describe('useEstimationStatus', () => {
+  it('returns LOADING if estimation is LOADING', async () => {
+    const { result } = renderHook(() => useEstimationStatus(EstimationStatus.LOADING))
+
+    await waitFor(() => {
+      expect(result.current[0]).toBe(ButtonStatus.LOADING)
+    })
+  })
+
+  it('returns READY if estimation is FAILURE', async () => {
+    const { result } = renderHook(() => useEstimationStatus(EstimationStatus.FAILURE))
+
+    await waitFor(() => {
+      expect(result.current[0]).toBe(ButtonStatus.READY)
+    })
+  })
+
+  it('returns READY if estimation is SUCCESS', async () => {
+    const { result } = renderHook(() => useEstimationStatus(EstimationStatus.SUCCESS))
+
+    await waitFor(() => {
+      expect(result.current[0]).toBe(ButtonStatus.READY)
+    })
+  })
+})
diff --git a/src/logic/hooks/__tests__/useExecutionStatus.test.ts b/src/logic/hooks/__tests__/useExecutionStatus.test.ts
@@ -63,7 +63,7 @@ describe('useExecutionStatus', () => {
     })
   })
 
-  it('returns LOADING if gasPrice is 0', async () => {
+  it('returns LOADING if gasPrice is undefined', async () => {
     const mockGasLimit = '21000'
     const mockFn = jest.fn(() => Promise.resolve(true))
 
@@ -73,7 +73,7 @@ describe('useExecutionStatus', () => {
         isExecution: true,
         txData: EMPTY_DATA,
         gasLimit: mockGasLimit,
-        gasPrice: '0',
+        gasPrice: undefined,
         gasMaxPrioFee: '2',
       }),
     )
@@ -84,7 +84,7 @@ describe('useExecutionStatus', () => {
     })
   })
 
-  it('returns LOADING if gasMaxPrioFee is 0', async () => {
+  it('returns LOADING if gasMaxPrioFee is undefined', async () => {
     const mockGasLimit = '21000'
     const mockFn = jest.fn(() => Promise.resolve(true))
 
@@ -95,7 +95,7 @@ describe('useExecutionStatus', () => {
         txData: EMPTY_DATA,
         gasLimit: mockGasLimit,
         gasPrice: '10',
-        gasMaxPrioFee: '0',
+        gasMaxPrioFee: undefined,
       }),
     )
 
@@ -105,6 +105,27 @@ describe('useExecutionStatus', () => {
     })
   })
 
+  it('returns SUCCESS if gasMaxPrioFee is 0', async () => {
+    const mockGasLimit = '21000'
+    const mockFn = jest.fn(() => Promise.resolve(true))
+
+    const { result } = renderHook(() =>
+      useExecutionStatus({
+        checkTxExecution: mockFn,
+        isExecution: true,
+        txData: EMPTY_DATA,
+        gasLimit: mockGasLimit,
+        gasPrice: '10',
+        gasMaxPrioFee: '0',
+      }),
+    )
+
+    await waitFor(() => {
+      expect(result.current).toBe(EstimationStatus.SUCCESS)
+      expect(mockFn).toHaveBeenCalledTimes(1)
+    })
+  })
+
   it('returns SUCCESS if callback fn returns true', async () => {
     const mockGasLimit = '21000'
     const mockFn = jest.fn(() => Promise.resolve(true))
@@ -188,4 +209,27 @@ describe('useExecutionStatus', () => {
       expect(result.current).toBe(EstimationStatus.FAILURE)
     })
   })
+
+  it('returns FAILURE if callback fn throws', async () => {
+    const mockGasLimit = '21000'
+    const mockFn = jest.fn(() => {
+      throw new Error()
+    })
+
+    const { result } = renderHook(() =>
+      useExecutionStatus({
+        checkTxExecution: mockFn,
+        isExecution: true,
+        txData: EMPTY_DATA,
+        gasLimit: mockGasLimit,
+        gasPrice: '10',
+        gasMaxPrioFee: '2',
+      }),
+    )
+
+    await waitFor(() => {
+      expect(result.current).toBe(EstimationStatus.FAILURE)
+      expect(mockFn).toHaveBeenCalledTimes(1)
+    })
+  })
 })
diff --git a/src/logic/hooks/useEstimateTransactionGas.tsx b/src/logic/hooks/useEstimateTransactionGas.tsx
@@ -28,10 +28,10 @@ type UseEstimateTransactionGasProps = {
 }
 
 type TransactionGasEstimationResult = {
-  gasPrice: string
-  gasPriceFormatted: string
-  gasMaxPrioFee: string
-  gasMaxPrioFeeFormatted: string
+  gasPrice?: string
+  gasPriceFormatted?: string
+  gasMaxPrioFee?: string
+  gasMaxPrioFeeFormatted?: string
 }
 
 export const calculateTotalGasCost = (
@@ -40,7 +40,7 @@ export const calculateTotalGasCost = (
   gasMaxPrioFee: string,
   decimals: number,
 ): { gasCost: string; gasCostFormatted: string } => {
-  const totalPricePerGas = parseInt(gasPrice, 10) + parseInt(gasMaxPrioFee || '0', 10)
+  const totalPricePerGas = parseInt(gasPrice, 10) + parseInt(gasMaxPrioFee, 10)
   const estimatedGasCosts = parseInt(gasLimit, 10) * totalPricePerGas
   const gasCost = fromTokenUnit(estimatedGasCosts, decimals)
   const gasCostFormatted = formatAmount(gasCost)
@@ -57,10 +57,10 @@ export const useEstimateTransactionGas = ({
   txData,
 }: UseEstimateTransactionGasProps): TransactionGasEstimationResult => {
   const [gasEstimation, setGasEstimation] = useState<TransactionGasEstimationResult>({
-    gasPrice: DEFAULT_GAS,
-    gasPriceFormatted: DEFAULT_GAS,
-    gasMaxPrioFee: DEFAULT_GAS,
-    gasMaxPrioFeeFormatted: DEFAULT_GAS,
+    gasPrice: undefined,
+    gasPriceFormatted: undefined,
+    gasMaxPrioFee: undefined,
+    gasMaxPrioFeeFormatted: undefined,
   })
 
   useEffect(() => {
@@ -88,8 +88,8 @@ export const useEstimateTransactionGas = ({
         setGasEstimation({
           gasPrice: DEFAULT_MAX_GAS_FEE.toString(),
           gasPriceFormatted: fromWei(DEFAULT_MAX_GAS_FEE.toString(), 'gwei'),
-          gasMaxPrioFee: DEFAULT_MAX_PRIO_FEE.toString(),
-          gasMaxPrioFeeFormatted: fromWei(DEFAULT_MAX_PRIO_FEE.toString(), 'gwei'),
+          gasMaxPrioFee: isMaxFeeParam() ? DEFAULT_MAX_PRIO_FEE.toString() : '0',
+          gasMaxPrioFeeFormatted: isMaxFeeParam() ? fromWei(DEFAULT_MAX_PRIO_FEE.toString(), 'gwei') : '0',
         })
       }
     }

diff --git a/src/logic/hooks/useEstimationStatus.tsx b/src/logic/hooks/useEstimationStatus.tsx
@@ -4,22 +4,17 @@ import { EstimationStatus } from './useEstimateTransactionGas'
 import { ButtonStatus } from 'src/components/Modal'
 
 export const useEstimationStatus = (
-  txEstimationStatus?: EstimationStatus,
+  txEstimationStatus: EstimationStatus,
 ): [buttonStatus: ButtonStatus, setButtonStatus: Dispatch<SetStateAction<ButtonStatus>>] => {
   const [buttonStatus, setButtonStatus] = useState<ButtonStatus>(ButtonStatus.DISABLED)
 
   useEffect(() => {
     let mounted = true
 
-    if (mounted) {
-      switch (txEstimationStatus) {
-        case EstimationStatus.LOADING:
-          setButtonStatus(ButtonStatus.LOADING)
-          break
-        default:
-          setButtonStatus(ButtonStatus.READY)
-          break
-      }
+    if (txEstimationStatus === EstimationStatus.LOADING) {
+      mounted && setButtonStatus(ButtonStatus.LOADING)
+    } else {
+      mounted && setButtonStatus(ButtonStatus.READY)
     }
 
     return () => {

diff --git a/src/logic/hooks/useExecutionStatus.ts b/src/logic/hooks/useExecutionStatus.ts
@@ -1,14 +1,14 @@
-import { DEFAULT_GAS, EstimationStatus } from 'src/logic/hooks/useEstimateTransactionGas'
+import { EstimationStatus } from 'src/logic/hooks/useEstimateTransactionGas'
 import { useEffect, useState } from 'react'
 import useAsync from 'src/logic/hooks/useAsync'
 
 type Props = {
   checkTxExecution: () => Promise<boolean>
   isExecution: boolean
   txData: string
-  gasLimit: string | undefined
-  gasPrice: string
-  gasMaxPrioFee: string
+  gasLimit?: string
+  gasPrice?: string
+  gasMaxPrioFee?: string
 }
 
 export const useExecutionStatus = ({
@@ -23,7 +23,7 @@ export const useExecutionStatus = ({
 
   const [status, error, loading] = useAsync(async () => {
     if (!isExecution || !txData) return EstimationStatus.SUCCESS
-    const isEstimationPending = !gasLimit || gasPrice === DEFAULT_GAS || gasMaxPrioFee === DEFAULT_GAS
+    const isEstimationPending = !gasLimit || !gasPrice || !gasMaxPrioFee
     if (isEstimationPending) return EstimationStatus.LOADING
 
     const success = await checkTxExecution()

diff --git a/src/routes/safe/components/Transactions/helpers/SpendingLimitModalWrapper/index.tsx b/src/routes/safe/components/Transactions/helpers/SpendingLimitModalWrapper/index.tsx
@@ -113,7 +113,7 @@ export const SpendingLimitModalWrapper = ({
   })
 
   const getGasCostFormatted = useCallback(() => {
-    if (!gasLimit || gasLimit === DEFAULT_GAS_LIMIT) return '> 0.001'
+    if (!gasLimit || gasLimit === DEFAULT_GAS_LIMIT || !gasPrice || !gasMaxPrioFee) return '> 0.001'
     return calculateTotalGasCost(gasLimit, gasPrice, gasMaxPrioFee, nativeCurrency.decimals).gasCostFormatted
   }, [gasLimit, gasMaxPrioFee, gasPrice, nativeCurrency.decimals])
 

diff --git a/src/routes/safe/components/Transactions/helpers/TxModalWrapper/index.tsx b/src/routes/safe/components/Transactions/helpers/TxModalWrapper/index.tsx
@@ -178,7 +178,7 @@ export const TxModalWrapper = ({
   })
 
   const getGasCostFormatted = useCallback(() => {
-    if (!gasLimit || gasLimit === DEFAULT_GAS_LIMIT) return '> 0.001'
+    if (!gasLimit || gasLimit === DEFAULT_GAS_LIMIT || !gasPrice || !gasMaxPrioFee) return '> 0.001'
     return calculateTotalGasCost(gasLimit, gasPrice, gasMaxPrioFee, nativeCurrency.decimals).gasCostFormatted
   }, [gasLimit, gasMaxPrioFee, gasPrice, nativeCurrency.decimals])