Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.
Merged
1 change: 1 addition & 0 deletions src/logic/exceptions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ enum ErrorCodes {
_613 = '613: Error fetching safe info',
_614 = '614: Error fetching transaction by id',
_615 = '615: Failed to retrieve last transaction from server',
_616 = '616: Failed to retrieve recommended nonce',
_700 = '700: Failed to read from local/session storage',
_701 = '701: Failed to write to local/session storage',
_702 = '702: Failed to remove from local/session storage',
Expand Down
25 changes: 20 additions & 5 deletions src/logic/safe/api/fetchSafeTxGasEstimation.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { postSafeGasEstimation, SafeTransactionEstimationRequest } from '@gnosis.pm/safe-react-gateway-sdk'
import {
postSafeGasEstimation,
SafeTransactionEstimationRequest,
SafeTransactionEstimation,
Operation,
} from '@gnosis.pm/safe-react-gateway-sdk'

import { _getChainId } from 'src/config'
import { checksumAddress } from 'src/utils/checksumAddress'
Expand All @@ -11,8 +16,18 @@ type FetchSafeTxGasEstimationProps = {
export const fetchSafeTxGasEstimation = async ({
safeAddress,
...body
}: FetchSafeTxGasEstimationProps): Promise<string> => {
return postSafeGasEstimation(GATEWAY_URL, _getChainId(), checksumAddress(safeAddress), body).then(
({ safeTxGas }) => safeTxGas,
)
}: FetchSafeTxGasEstimationProps): Promise<SafeTransactionEstimation> => {
return postSafeGasEstimation(GATEWAY_URL, _getChainId(), checksumAddress(safeAddress), body)
}

export const getRecommendedNonce = async (safeAddress: string): Promise<number> => {
const { recommendedNonce } = await fetchSafeTxGasEstimation({
safeAddress,
value: '0',
operation: Operation.CALL,
// Workaround: use a cancellation transaction to fetch only the recommendedNonce
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In future, we should make just one request – to fetch the actual safeTxGas and the nonce. They are displayed on the same Review screen.

to: safeAddress,
data: '0x',
})
return recommendedNonce
}
60 changes: 1 addition & 59 deletions src/logic/safe/store/actions/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'

import { ChainId } from 'src/config/chain.d'
import {
buildSafeOwners,
extractRemoteSafeInfo,
getNewTxNonce,
shouldExecuteTransaction,
} from 'src/logic/safe/store/actions/utils'
import { buildSafeOwners, extractRemoteSafeInfo, shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils'
import { SafeRecordProps } from 'src/logic/safe/store/models/safe'
import { getMockedSafeInstance, getMockedStoredTServiceModel } from 'src/test/utils/safeHelper'
import { LocalTransactionStatus } from '../../models/types/gateway.d'
Expand Down Expand Up @@ -73,59 +68,6 @@ describe('shouldExecuteTransaction', () => {
})
})

describe('getNewTxNonce', () => {
it('It should return 2 if given the last transaction with nonce 1', async () => {
// given
const safeInstance = getMockedSafeInstance({})
const lastTxFromStoreExecuted = {
...lastTxFromStore,
executionInfo: { type: 'MULTISIG', nonce: 1, confirmationsRequired: 1, confirmationsSubmitted: 1 },
}
const expectedResult = '2'

// when
const result = await getNewTxNonce(lastTxFromStoreExecuted.executionInfo.nonce, safeInstance)

// then
expect(result).toBe(expectedResult)
})
it('It should return 0 if given a safe with nonce 0 and no transactions should use safe contract instance for retrieving nonce', async () => {
// given
const safeNonce = '0'
const safeInstance = getMockedSafeInstance({ nonce: safeNonce })
const expectedResult = '0'
const mockFnCall = jest.fn().mockImplementation(() => safeNonce)
const mockFnNonce = jest.fn().mockImplementation(() => ({ call: mockFnCall }))

safeInstance.methods.nonce = mockFnNonce

// when
const result = await getNewTxNonce(undefined, safeInstance)

// then
expect(result).toBe(expectedResult)
expect(mockFnNonce).toHaveBeenCalled()
expect(mockFnCall).toHaveBeenCalled()
mockFnNonce.mockRestore()
mockFnCall.mockRestore()
})
it('Given a Safe and the last transaction, should return nonce of the last transaction + 1', async () => {
// given
const safeInstance = getMockedSafeInstance({})
const expectedResult = '11'
const lastTxFromStoreExecuted = {
...lastTxFromStore,
executionInfo: { type: 'MULTISIG', nonce: 10, confirmationsRequired: 1, confirmationsSubmitted: 1 },
}

// when
const result = await getNewTxNonce(lastTxFromStoreExecuted.executionInfo.nonce, safeInstance)

// then
expect(result).toBe(expectedResult)
})
})

jest.mock('axios')
jest.mock('console')

Expand Down
15 changes: 10 additions & 5 deletions src/logic/safe/store/actions/createTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { providerSelector } from 'src/logic/wallets/store/selectors'
import enqueueSnackbar from 'src/logic/notifications/store/actions/enqueueSnackbar'
import closeSnackbarAction from 'src/logic/notifications/store/actions/closeSnackbar'
import { generateSafeTxHash } from 'src/logic/safe/store/actions/transactions/utils/transactionHelpers'
import { getNewTxNonce, shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils'
import { shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils'
import fetchTransactions from './transactions/fetchTransactions'
import { TxArgs } from 'src/logic/safe/store/models/types/transaction'
import { PayableTx } from 'src/types/contracts/types.d'
Expand All @@ -35,7 +35,8 @@ import { extractShortChainName, history, SAFE_ROUTES } from 'src/routes/routes'
import { getPrefixedSafeAddressSlug, SAFE_ADDRESS_SLUG, TRANSACTION_ID_SLUG } from 'src/routes/routes'
import { generatePath } from 'react-router-dom'
import { getContractErrorMessage } from 'src/logic/contracts/safeContractErrors'
import { getLastTransaction, getLastTxNonce } from '../selectors/gatewayTransactions'
import { getLastTransaction } from '../selectors/gatewayTransactions'
import { getRecommendedNonce } from '../../api/fetchSafeTxGasEstimation'
import { isMultiSigExecutionDetails, LocalTransactionStatus } from '../models/types/gateway.d'
import { updateTransactionStatus } from './updateTransactionStatus'

Expand Down Expand Up @@ -108,9 +109,13 @@ export const createTransaction =
const chainId = currentChainId(state)

const lastTx = getLastTransaction(state)
const lastTxNonce = getLastTxNonce(state)

const nextNonce = await getNewTxNonce(lastTxNonce, safeInstance)
let nextNonce: string
try {
nextNonce = (await getRecommendedNonce(safeAddress)).toString()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already a toString() on line 128.

} catch (e) {
logError(Errors._616, e.message)
nextNonce = await safeInstance.methods.nonce().call()
}
const nonce = txNonce !== undefined ? txNonce.toString() : nextNonce

const isExecution = !delayExecution && (await shouldExecuteTransaction(safeInstance, nonce, lastTx))
Expand Down
15 changes: 10 additions & 5 deletions src/logic/safe/store/actions/processTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import enqueueSnackbar from 'src/logic/notifications/store/actions/enqueueSnackb
import closeSnackbarAction from 'src/logic/notifications/store/actions/closeSnackbar'
import { fetchSafe } from 'src/logic/safe/store/actions/fetchSafe'
import fetchTransactions from 'src/logic/safe/store/actions/transactions/fetchTransactions'
import { getNewTxNonce, shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils'
import { shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils'
import { AppReduxState } from 'src/store'
import { TxParameters } from 'src/routes/safe/container/hooks/useTransactionParameters'
import { Dispatch, DispatchReturn } from './types'
Expand All @@ -33,7 +33,8 @@ import { Errors, logError } from 'src/logic/exceptions/CodedException'
import { getContractErrorMessage } from 'src/logic/contracts/safeContractErrors'
import { onboardUser } from 'src/components/ConnectButton'
import { getGasParam } from '../../transactions/gas'
import { getLastTransaction, getLastTxNonce } from '../selectors/gatewayTransactions'
import { getLastTransaction } from '../selectors/gatewayTransactions'
import { getRecommendedNonce } from '../../api/fetchSafeTxGasEstimation'
import { LocalTransactionStatus } from '../models/types/gateway.d'

interface ProcessTransactionArgs {
Expand Down Expand Up @@ -85,9 +86,13 @@ export const processTransaction =
const safeInstance = getGnosisSafeInstanceAt(safeAddress, safeVersion)

const lastTx = getLastTransaction(state)
const lastTxNonce = getLastTxNonce(state)

const nonce = await getNewTxNonce(lastTxNonce, safeInstance)
let nonce: string
try {
nonce = (await getRecommendedNonce(safeAddress)).toString()
} catch (e) {
logError(Errors._616, e.message)
nonce = await safeInstance.methods.nonce().call()
}
const isExecution = approveAndExecute || (await shouldExecuteTransaction(safeInstance, nonce, lastTx))

const preApprovingOwner = approveAndExecute && !thresholdReached ? userAddress : undefined
Expand Down
6 changes: 0 additions & 6 deletions src/logic/safe/store/actions/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ import {
LocalTransactionStatus,
} from 'src/logic/safe/store/models/types/gateway.d'

export const getNewTxNonce = async (lastTxNonce = -1, safeInstance: GnosisSafe): Promise<string> => {
const safeInstanceNonce = await safeInstance.methods.nonce().call()
const getNewTxNonce = Math.max(lastTxNonce + 1, +safeInstanceNonce)
return getNewTxNonce.toString()
}

export const shouldExecuteTransaction = async (
safeInstance: GnosisSafe,
nonce: string,
Expand Down
4 changes: 2 additions & 2 deletions src/logic/safe/transactions/gas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ export const estimateSafeTxGas = async (
}

try {
const safeTxGasEstimation = await fetchSafeTxGasEstimation({
const { safeTxGas } = await fetchSafeTxGasEstimation({
safeAddress,
to: checksumAddress(txRecipient),
value: txAmount,
data: txData,
operation,
})

return safeTxGasEstimation
return safeTxGas
} catch (error) {
console.info('Error calculating tx gas estimation', error.message)
throw error
Expand Down
15 changes: 8 additions & 7 deletions src/routes/safe/container/hooks/useTransactionParameters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ import { toWei } from 'web3-utils'

import { getUserNonce } from 'src/logic/wallets/ethTransactions'
import { userAccountSelector } from 'src/logic/wallets/store/selectors'
import { getNewTxNonce } from 'src/logic/safe/store/actions/utils'
import { getGnosisSafeInstanceAt } from 'src/logic/contracts/safeContracts'
import { currentSafeCurrentVersion } from 'src/logic/safe/store/selectors'
import { ParametersStatus } from 'src/routes/safe/components/Transactions/helpers/utils'
import { sameString } from 'src/utils/strings'
import { extractSafeAddress } from 'src/routes/routes'
import { getLastTxNonce } from 'src/logic/safe/store/selectors/gatewayTransactions'
import { AppReduxState } from 'src/store'
import { getRecommendedNonce } from 'src/logic/safe/api/fetchSafeTxGasEstimation'
import { Errors, logError } from 'src/logic/exceptions/CodedException'

export type TxParameters = {
safeNonce: string | undefined
Expand Down Expand Up @@ -86,10 +85,12 @@ export const useTransactionParameters = (props?: Props): TxParameters => {
useEffect(() => {
const getSafeNonce = async () => {
if (safeAddress) {
const safeInstance = getGnosisSafeInstanceAt(safeAddress, safeVersion)
const lastTxNonce = getLastTxNonce(state)
const nonce = await getNewTxNonce(lastTxNonce, safeInstance)
setSafeNonce(nonce)
try {
const recommendedNonce = (await getRecommendedNonce(safeAddress)).toString()
setSafeNonce(recommendedNonce)
} catch (e) {
logError(Errors._616, e.message)
}
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,4 @@ const isProdGateway = () => {

export const GATEWAY_URL =
process.env.REACT_APP_GATEWAY_URL ||
(IS_PRODUCTION || isProdGateway()
? 'https://safe-client.gnosis.io/v1'
: 'https://safe-client.staging.gnosisdev.com/v1')
(IS_PRODUCTION || isProdGateway() ? 'https://safe-client.gnosis.io' : 'https://safe-client.staging.gnosisdev.com')