diff --git a/src/logic/hooks/__tests__/useAsync.test.ts b/src/logic/hooks/__tests__/useAsync.test.ts new file mode 100644 index 0000000000..202392ad73 --- /dev/null +++ b/src/logic/hooks/__tests__/useAsync.test.ts @@ -0,0 +1,63 @@ +import { renderHook } from '@testing-library/react-hooks' +import useAsync from '../useAsync' + +describe('useAsync tests', () => { + it('returns a successful result', async () => { + const fakeCallback = jest.fn(() => Promise.resolve('success')) + + const { result, waitForNextUpdate } = renderHook(() => useAsync(fakeCallback)) + + await waitForNextUpdate() + + expect(result.current).toEqual({ result: 'success', error: undefined }) + + expect(fakeCallback).toHaveBeenCalledTimes(1) + }) + + it('returns an error', async () => { + const fakeCallback = jest.fn(() => Promise.reject(new Error('failure'))) + + const { result, waitForNextUpdate } = renderHook(() => useAsync(fakeCallback)) + + await waitForNextUpdate() + + expect(result.current.result).toBe(undefined) + expect(result.current.error).toBeDefined() + expect(result.current.error!.message).toBe('failure') + + expect(fakeCallback).toHaveBeenCalledTimes(1) + }) + + it('ignores a stale result if a new request has been launched already', async () => { + // This will resolve AFTER the second callback but the result should be ignored + const fakeCallback1 = jest.fn(() => { + return new Promise((resolve) => { + setTimeout(() => { + resolve('success 1') + }, 10) + }) + }) + + const fakeCallback2 = jest.fn(() => Promise.resolve('success 2')) + + let counter = 0 + const { result, waitForNextUpdate, rerender } = renderHook(() => { + const callback = counter > 0 ? fakeCallback2 : fakeCallback1 + counter += 1 + return useAsync(callback) + }) + + expect(result.current.result).toBe(undefined) + expect(result.current.error).toBe(undefined) + + rerender() // re-render immediately + + await waitForNextUpdate() + + expect(result.current.result).toBe('success 2') + expect(result.current.error).toBe(undefined) + + expect(fakeCallback1).toHaveBeenCalledTimes(1) + expect(fakeCallback2).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/logic/hooks/__tests__/useEstimateSafeTxGas.test.ts b/src/logic/hooks/__tests__/useEstimateSafeTxGas.test.ts index b1055bf06d..bbe95bb932 100644 --- a/src/logic/hooks/__tests__/useEstimateSafeTxGas.test.ts +++ b/src/logic/hooks/__tests__/useEstimateSafeTxGas.test.ts @@ -1,5 +1,5 @@ +import { renderHook, act } from '@testing-library/react-hooks' import { useEstimateSafeTxGas } from 'src/logic/hooks/useEstimateSafeTxGas' -import { renderHook } from '@testing-library/react-hooks' import * as gas from 'src/logic/safe/transactions/gas' jest.mock('react-redux', () => { @@ -10,69 +10,111 @@ jest.mock('react-redux', () => { } }) +const actResolve = async (callback: () => unknown): Promise => { + await act(() => { + callback() + return new Promise((reslolve) => { + setTimeout(reslolve, 0) + }) + }) +} + describe('useEstimateSafeTxGas', () => { - it(`should return 0 if it is not a tx creation`, () => { + it(`should return 0 if it is not a tx creation`, async () => { + const spy = jest.spyOn(gas, 'estimateSafeTxGas') + + await actResolve(() => { + const { result } = renderHook(() => + useEstimateSafeTxGas({ + txAmount: '', + txData: '0x', + txRecipient: '', + isCreation: false, + isRejectTx: false, + }), + ) + + expect(result.current).toBe('0') + }) + + expect(spy).toHaveBeenCalledTimes(0) + }) + + it(`should return 0 if it is a reject tx`, async () => { const spy = jest.spyOn(gas, 'estimateSafeTxGas') - const { result } = renderHook(() => - useEstimateSafeTxGas({ - txAmount: '', - txData: '', - txRecipient: '', - isCreation: false, - isRejectTx: false, - }), - ) - expect(result.current).toBe('0') + await actResolve(() => { + const { result } = renderHook(() => + useEstimateSafeTxGas({ + txAmount: '', + txData: '0x', + txRecipient: '', + isCreation: false, + isRejectTx: true, + }), + ) + expect(result.current).toBe('0') + }) + expect(spy).toHaveBeenCalledTimes(0) }) - it(`should return 0 if it is a reject tx`, () => { + it(`should return 0 if tx data is not passed`, async () => { const spy = jest.spyOn(gas, 'estimateSafeTxGas') - const { result } = renderHook(() => - useEstimateSafeTxGas({ - txAmount: '', - txData: '', - txRecipient: '', - isCreation: false, - isRejectTx: true, - }), - ) - expect(result.current).toBe('0') + await actResolve(() => { + const { result } = renderHook(() => + useEstimateSafeTxGas({ + txAmount: '', + txData: '', + txRecipient: '', + isCreation: true, + isRejectTx: false, + }), + ) + + expect(result.current).toBe('0') + }) + expect(spy).toHaveBeenCalledTimes(0) }) - it(`calls estimateSafeTxGas if it is a tx creation`, () => { + it(`calls estimateSafeTxGas if it is a tx creation`, async () => { const spy = jest.spyOn(gas, 'estimateSafeTxGas') - renderHook(() => - useEstimateSafeTxGas({ - txAmount: '', - txData: '', - txRecipient: '', - isCreation: true, - isRejectTx: false, - }), - ) + await actResolve(() => { + renderHook(() => + useEstimateSafeTxGas({ + txAmount: '', + txData: '0x', + txRecipient: '', + isCreation: true, + isRejectTx: false, + }), + ) + }) + expect(spy).toHaveBeenCalledTimes(1) }) - it(`returns 0 if estimateSafeTxGas throws`, () => { + it(`returns 0 if estimateSafeTxGas throws`, async () => { const spy = jest.spyOn(gas, 'estimateSafeTxGas').mockImplementation(() => { throw new Error() }) - const { result } = renderHook(() => - useEstimateSafeTxGas({ - txAmount: '', - txData: '', - txRecipient: '', - isCreation: true, - isRejectTx: false, - }), - ) + await actResolve(() => { + const { result } = renderHook(() => + useEstimateSafeTxGas({ + txAmount: '', + txData: '0x', + txRecipient: '', + isCreation: true, + isRejectTx: false, + }), + ) + expect(result.current).toBe('0') + }) + expect(spy).toHaveBeenCalledTimes(1) - expect(result.current).toBe('0') }) }) diff --git a/src/logic/hooks/useAsync.ts b/src/logic/hooks/useAsync.ts new file mode 100644 index 0000000000..c135ba510f --- /dev/null +++ b/src/logic/hooks/useAsync.ts @@ -0,0 +1,34 @@ +import { useEffect, useState } from 'react' + +type AsyncResult = { + error: Error | undefined + result: T | undefined +} + +const useAsync = (asyncCall: () => Promise): AsyncResult => { + const [asyncVal, setAsyncVal] = useState() + const [err, setErr] = useState() + + useEffect(() => { + let isCurrent = true + + setAsyncVal(undefined) + setErr(undefined) + + asyncCall() + .then((val: T) => { + if (isCurrent) setAsyncVal(val) + }) + .catch((error) => { + if (isCurrent) setErr(error) + }) + + return () => { + isCurrent = false + } + }, [asyncCall]) + + return { error: err, result: asyncVal } +} + +export default useAsync diff --git a/src/logic/hooks/useEstimateSafeTxGas.tsx b/src/logic/hooks/useEstimateSafeTxGas.tsx index 636f227e00..c27a0469ee 100644 --- a/src/logic/hooks/useEstimateSafeTxGas.tsx +++ b/src/logic/hooks/useEstimateSafeTxGas.tsx @@ -1,9 +1,10 @@ import { Operation } from '@gnosis.pm/safe-react-gateway-sdk' -import { useEffect, useState } from 'react' +import { useCallback } from 'react' import { useSelector } from 'react-redux' import { estimateSafeTxGas } from 'src/logic/safe/transactions/gas' import { currentSafe } from 'src/logic/safe/store/selectors' +import useAsync from 'src/logic/hooks/useAsync' type UseEstimateSafeTxGasProps = { isCreation: boolean @@ -22,30 +23,25 @@ export const useEstimateSafeTxGas = ({ txAmount, operation, }: UseEstimateSafeTxGasProps): string => { - const [safeTxGasEstimation, setSafeTxGasEstimation] = useState('0') + const defaultEstimation = '0' const { address: safeAddress, currentVersion: safeVersion } = useSelector(currentSafe) ?? {} - useEffect(() => { - if (!isCreation || isRejectTx) return - const estimateSafeTxGasCall = async () => { - try { - const safeTxGasEstimation = await estimateSafeTxGas( - { - safeAddress, - txData, - txRecipient, - txAmount: txAmount || '0', - operation: operation || Operation.CALL, - }, - safeVersion, - ) - setSafeTxGasEstimation(safeTxGasEstimation) - } catch (error) { - console.warn(error.message) - } - } - estimateSafeTxGasCall() + const requestSafeTxGas = useCallback((): Promise => { + if (!isCreation || isRejectTx || !txData) return Promise.resolve(defaultEstimation) + + return estimateSafeTxGas( + { + safeAddress, + txData, + txRecipient, + txAmount: txAmount || '0', + operation: operation || Operation.CALL, + }, + safeVersion, + ) }, [isCreation, isRejectTx, operation, safeAddress, safeVersion, txAmount, txData, txRecipient]) - return safeTxGasEstimation + const { result } = useAsync(requestSafeTxGas) + + return result || defaultEstimation } diff --git a/src/logic/hooks/useRecommendedNonce.tsx b/src/logic/hooks/useRecommendedNonce.tsx index a820c60a15..a4656fe848 100644 --- a/src/logic/hooks/useRecommendedNonce.tsx +++ b/src/logic/hooks/useRecommendedNonce.tsx @@ -1,36 +1,19 @@ -import { SafeTransactionEstimation } from '@gnosis.pm/safe-react-gateway-sdk' -import { useEffect, useState } from 'react' +import { useCallback } from 'react' import { useSelector } from 'react-redux' import { getRecommendedNonce } from 'src/logic/safe/api/fetchSafeTxGasEstimation' import { getLastTxNonce } from 'src/logic/safe/store/selectors/gatewayTransactions' +import useAsync from 'src/logic/hooks/useAsync' const useRecommendedNonce = (safeAddress: string): number => { - const lastTxNonce = useSelector(getLastTxNonce) - const [recommendedNonce, setRecommendedNonce] = useState(lastTxNonce ? lastTxNonce + 1 : 0) + const lastTxNonce = useSelector(getLastTxNonce) || -1 - useEffect(() => { - let isCurrent = true + const getNonce = useCallback(() => { + return getRecommendedNonce(safeAddress) + }, [safeAddress]) - const fetchRecommendedNonce = async () => { - let recommendedNonce: SafeTransactionEstimation['recommendedNonce'] - try { - recommendedNonce = await getRecommendedNonce(safeAddress) - } catch (e) { - return - } + const { result } = useAsync(getNonce) - if (isCurrent) { - setRecommendedNonce(recommendedNonce) - } - } - fetchRecommendedNonce() - - return () => { - isCurrent = false - } - }, [lastTxNonce, safeAddress]) - - return recommendedNonce + return result == null ? lastTxNonce + 1 : result } export default useRecommendedNonce diff --git a/src/logic/safe/transactions/gas.ts b/src/logic/safe/transactions/gas.ts index d24456f24a..cc92804af3 100644 --- a/src/logic/safe/transactions/gas.ts +++ b/src/logic/safe/transactions/gas.ts @@ -116,7 +116,11 @@ export const checkTransactionExecution = async ({ from, gas: gasLimit, }) - .catch(() => false) + .then(() => true) + .catch((e) => { + console.warn('Transaction will fail\n\n', e) + return false + }) } export const isMaxFeeParam = (): boolean => { diff --git a/src/routes/safe/components/Transactions/helpers/TxModalWrapper/index.tsx b/src/routes/safe/components/Transactions/helpers/TxModalWrapper/index.tsx index 7cc9e7c938..d3c092340d 100644 --- a/src/routes/safe/components/Transactions/helpers/TxModalWrapper/index.tsx +++ b/src/routes/safe/components/Transactions/helpers/TxModalWrapper/index.tsx @@ -125,6 +125,7 @@ export const TxModalWrapper = ({ txAmount: txValue, operation, }) + if (safeTxGas == null) safeTxGas = safeTxGasEstimation const { gasCostFormatted, gasPriceFormatted, gasMaxPrioFeeFormatted, gasLimit, txEstimationExecutionStatus } = useEstimateTransactionGas({ @@ -150,7 +151,7 @@ export const TxModalWrapper = ({ const newGasLimit = txParameters.ethGasLimit const oldMaxPrioFee = gasMaxPrioFeeFormatted const newMaxPrioFee = txParameters.ethMaxPrioFee - const oldSafeTxGas = safeTxGasEstimation + const oldSafeTxGas = safeTxGas const newSafeTxGas = txParameters.safeTxGas if (oldGasPrice !== newGasPrice) { @@ -192,7 +193,7 @@ export const TxModalWrapper = ({ ethGasLimit={gasLimit} ethGasPrice={gasPriceFormatted} ethMaxPrioFee={gasMaxPrioFeeFormatted} - safeTxGas={safeTxGasEstimation} + safeTxGas={safeTxGas} safeNonce={txNonce} parametersStatus={parametersStatus} closeEditModalCallback={onEditClose} diff --git a/src/routes/safe/components/Transactions/helpers/TxParametersDetail/index.tsx b/src/routes/safe/components/Transactions/helpers/TxParametersDetail/index.tsx index 255e11fa3d..be814b4350 100644 --- a/src/routes/safe/components/Transactions/helpers/TxParametersDetail/index.tsx +++ b/src/routes/safe/components/Transactions/helpers/TxParametersDetail/index.tsx @@ -118,15 +118,19 @@ export const TxParametersDetail = ({ useEffect(() => { if (Number.isNaN(safeNonceNumber) || safeNonceNumber === nonce) { + setIsAccordionExpanded(false) + setIsTxNonceOutOfOrder(false) return } if (lastQueuedTxNonce === undefined && safeNonceNumber !== nonce) { setIsAccordionExpanded(true) setIsTxNonceOutOfOrder(true) + return } if (lastQueuedTxNonce && safeNonceNumber !== lastQueuedTxNonce + 1) { setIsAccordionExpanded(true) setIsTxNonceOutOfOrder(true) + return } }, [lastQueuedTxNonce, nonce, safeNonceNumber])