Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: reactive recommended nonce #2050

Merged
merged 3 commits into from
May 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/components/tx/AdvancedParams/useAdvancedParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import useGasPrice from '@/hooks/useGasPrice'
import { type AdvancedParameters } from './types'
import useUserNonce from './useUserNonce'

export const useAdvancedParams = ({
nonce,
gasLimit,
safeTxGas,
}: AdvancedParameters): [AdvancedParameters, (params: AdvancedParameters) => void] => {
export const useAdvancedParams = (
gasLimit?: AdvancedParameters['gasLimit'],
nonce?: AdvancedParameters['nonce'],
safeTxGas?: AdvancedParameters['safeTxGas'],
): [AdvancedParameters, (params: AdvancedParameters) => void] => {
const [manualParams, setManualParams] = useState<AdvancedParameters>()
const { maxFeePerGas, maxPriorityFeePerGas } = useGasPrice()
const userNonce = useUserNonce()
Expand Down
126 changes: 126 additions & 0 deletions src/components/tx/SignOrExecuteForm/ExecuteForm.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { type ReactElement, type SyntheticEvent, useState } from 'react'
import { Box, Button } from '@mui/material'

import ErrorMessage from '@/components/tx/ErrorMessage'
import { logError, Errors } from '@/services/exceptions'
import { useCurrentChain } from '@/hooks/useChains'
import { getTxOptions } from '@/utils/transactions'
import useIsValidExecution from '@/hooks/useIsValidExecution'
import CheckWallet from '@/components/common/CheckWallet'
import { useImmediatelyExecutable, useIsExecutionLoop, useTxActions } from './hooks'
import UnknownContractError from './UnknownContractError'
import { useRelaysBySafe } from '@/hooks/useRemainingRelays'
import useWalletCanRelay from '@/hooks/useWalletCanRelay'
import { ExecutionMethod, ExecutionMethodSelector } from '../ExecutionMethodSelector'
import { hasRemainingRelays } from '@/utils/relaying'
import type { SignOrExecuteProps } from '.'
import type { AdvancedParameters } from '../AdvancedParams'

const ExecuteForm = ({
safeTx,
txId,
onSubmit,
children,
isExecutable = false,
isRejection = false,
disableSubmit = false,
origin,
advancedParams,
...props
}: SignOrExecuteProps & { advancedParams: AdvancedParameters }): ReactElement => {
//
// Hooks & variables
//
const [isSubmittable, setIsSubmittable] = useState<boolean>(true)
const [submitError, setSubmitError] = useState<Error | undefined>()

// Hooks
const currentChain = useCurrentChain()
const { executeTx } = useTxActions()
const [relays] = useRelaysBySafe()

// Check that the transaction is executable
const isCreation = !txId
const isNewExecutableTx = useImmediatelyExecutable() && isCreation
const isExecutionLoop = useIsExecutionLoop()

// We default to relay, but the option is only shown if we canRelay
const [executionMethod, setExecutionMethod] = useState(ExecutionMethod.RELAY)

// SC wallets can relay fully signed transactions
const [walletCanRelay] = useWalletCanRelay(safeTx)

// The transaction can/will be relayed
const canRelay = walletCanRelay && hasRemainingRelays(relays)
const willRelay = canRelay && executionMethod === ExecutionMethod.RELAY

// Check if transaction will fail
const { executionValidationError, isValidExecutionLoading } = useIsValidExecution(safeTx, advancedParams.gasLimit)

// On modal submit
const handleSubmit = async (e: SyntheticEvent) => {
e.preventDefault()
setIsSubmittable(false)
setSubmitError(undefined)

const txOptions = getTxOptions(advancedParams, currentChain)

try {
await executeTx(txOptions, safeTx, txId, origin, willRelay)
} catch (err) {
logError(Errors._804, (err as Error).message)
setIsSubmittable(true)
setSubmitError(err as Error)
return
}

onSubmit()
}

const submitDisabled = !safeTx || !isSubmittable || disableSubmit || isValidExecutionLoading || isExecutionLoop

const error = props.error || executionValidationError

return (
<form onSubmit={handleSubmit}>
{canRelay && (
<Box mb={2}>
<ExecutionMethodSelector
executionMethod={executionMethod}
setExecutionMethod={setExecutionMethod}
relays={relays}
/>
</Box>
)}

{/* Error messages */}
{isExecutionLoop ? (
<ErrorMessage>
Cannot execute a transaction from the Safe Account itself, please connect a different account.
</ErrorMessage>
) : error ? (
<ErrorMessage error={error}>
This transaction will most likely fail.{' '}
{isNewExecutableTx
? 'To save gas costs, avoid creating the transaction.'
: 'To save gas costs, reject this transaction.'}
</ErrorMessage>
) : submitError ? (
<ErrorMessage error={submitError}>Error submitting the transaction. Please try again.</ErrorMessage>
) : (
<UnknownContractError />
)}

{/* Submit button */}
<CheckWallet allowNonOwner={true}>
{(isOk) => (
<Button variant="contained" type="submit" disabled={!isOk || submitDisabled}>
Submit
</Button>
)}
</CheckWallet>
</form>
)
}

export default ExecuteForm
92 changes: 92 additions & 0 deletions src/components/tx/SignOrExecuteForm/SignForm.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { type ReactElement, type SyntheticEvent, useState } from 'react'
import { Button, Typography } from '@mui/material'

import ErrorMessage from '@/components/tx/ErrorMessage'
import { logError, Errors } from '@/services/exceptions'
import useIsSafeOwner from '@/hooks/useIsSafeOwner'
import CheckWallet from '@/components/common/CheckWallet'
import { useTxActions } from './hooks'
import type { SignOrExecuteProps } from '.'

const SignForm = ({
safeTx,
txId,
onSubmit,
children,
isRejection = false,
disableSubmit = false,
origin,
...props
}: SignOrExecuteProps): ReactElement => {
//
// Hooks & variables
//
const [isSubmittable, setIsSubmittable] = useState<boolean>(true)
const [submitError, setSubmitError] = useState<Error | undefined>()

// Hooks
const isOwner = useIsSafeOwner()
const { signTx } = useTxActions()

// Check that the transaction is executable
const isCreation = !txId

// On modal submit
const handleSubmit = async (e: SyntheticEvent) => {
e.preventDefault()
setIsSubmittable(false)
setSubmitError(undefined)

try {
await signTx(safeTx, txId, origin)
} catch (err) {
logError(Errors._804, (err as Error).message)
setIsSubmittable(true)
setSubmitError(err as Error)
return
}

onSubmit()
}

const cannotPropose = !isOwner
const submitDisabled = !safeTx || !isSubmittable || disableSubmit || cannotPropose

return (
<form onSubmit={handleSubmit}>
{/* Error messages */}
{isSubmittable && cannotPropose ? (
<ErrorMessage>
You are currently not an owner of this Safe Account and won&apos;t be able to submit this transaction.
</ErrorMessage>
) : props.error ? (
<ErrorMessage error={props.error}>
This transaction will most likely fail.{' '}
{isCreation
? 'To save gas costs, avoid creating the transaction.'
: 'To save gas costs, reject this transaction.'}
</ErrorMessage>
) : (
submitError && (
<ErrorMessage error={submitError}>Error submitting the transaction. Please try again.</ErrorMessage>
)
)}

<Typography variant="body2" color="border.main" textAlign="center" mt={3}>
You&apos;re about to {isCreation ? 'create and ' : ''}
sign a transaction and will need to confirm it with your currently connected wallet.
</Typography>

{/* Submit button */}
<CheckWallet>
{(isOk) => (
<Button variant="contained" type="submit" disabled={!isOk || submitDisabled}>
Submit
</Button>
)}
</CheckWallet>
</form>
)
}

export default SignForm
38 changes: 36 additions & 2 deletions src/components/tx/SignOrExecuteForm/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import { useHasPendingTxs } from '@/hooks/usePendingTxs'
import { sameString } from '@safe-global/safe-core-sdk/dist/src/utils'
import type { ConnectedWallet } from '@/services/onboard'
import type { OnboardAPI } from '@web3-onboard/core'
import { hasEnoughSignatures } from '@/utils/transactions'
import { getRecommendedTxParams } from '@/services/tx/tx-sender/recommendedNonce'
import useAsync from '@/hooks/useAsync'

type TxActions = {
signTx: (safeTx?: SafeTransaction, txId?: string, origin?: string) => Promise<string>
Expand Down Expand Up @@ -97,7 +98,7 @@ export const useTxActions = (): TxActions => {
assertOnboard(onboard)

// Relayed transactions must be fully signed, so request a final signature if needed
if (isRelayed && !hasEnoughSignatures(safeTx, safe)) {
if (isRelayed && safeTx.signatures.size < safe.threshold) {
safeTx = await signRelayedTx(safeTx)
txId = await proposeTx(wallet.address, safeTx, txId, origin)
}
Expand Down Expand Up @@ -138,3 +139,36 @@ export const useIsExecutionLoop = (): boolean => {
const { safeAddress } = useSafeInfo()
return wallet ? sameString(wallet.address, safeAddress) : false
}

export const useRecommendedNonce = (
safeTx?: SafeTransaction,
):
| {
nonce: number
safeTxGas: number
}
Comment on lines +146 to +149
Copy link
Member

Choose a reason for hiding this comment

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

nit: We could extract this into a type that can be reused by getRecommendedTxParams as well.

| undefined => {
const { safe } = useSafeInfo()
const safeTxData = safeTx?.data

// Memoize only the necessary params, so that it doesn't change every time safeTx is changed
const safeTxParams = useMemo(
() =>
safeTxData?.to
usame-algan marked this conversation as resolved.
Show resolved Hide resolved
? {
to: safeTxData.to,
value: safeTxData.value,
data: safeTxData.data,
operation: safeTxData.operation,
}
: undefined,
[safeTxData?.to, safeTxData?.value, safeTxData?.data, safeTxData?.operation],
)

const [recommendedParams] = useAsync(() => {
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest also exporting the loading flag here since there is a small window when creating transactions now where it will show the execute option in any case until the nonce is estimated.

if (!safeTxParams) return
return getRecommendedTxParams(safeTxParams)
usame-algan marked this conversation as resolved.
Show resolved Hide resolved
}, [safeTxParams, safe?.txQueuedTag])

return recommendedParams
}
Loading