From 75f142df377edfc29ad4cffe0d9cbcde0b944822 Mon Sep 17 00:00:00 2001 From: Usame Algan <5880855+usame-algan@users.noreply.github.com> Date: Thu, 29 Jun 2023 15:53:24 +0200 Subject: [PATCH] [TX flow] Adds nonce validation (#2175) * Adds nonce validation * Adjust nonce validation * Disable nonce for reject tx, use correct nonce when replacing a tx * Adjust wording * fix: migrate input to RHF * Limit nonce input width * Fix e2e tests --------- Co-authored-by: iamacook --- .../tx-flow/common/TxNonce/index.tsx | 193 +++++++++++------- .../tx-flow/common/TxNonce/styles.module.css | 1 - .../TokenTransfer/CreateTokenTransfer.tsx | 12 +- src/utils/transactions.ts | 5 + 4 files changed, 136 insertions(+), 75 deletions(-) diff --git a/src/components/tx-flow/common/TxNonce/index.tsx b/src/components/tx-flow/common/TxNonce/index.tsx index 54c1267b49..31945bc02e 100644 --- a/src/components/tx-flow/common/TxNonce/index.tsx +++ b/src/components/tx-flow/common/TxNonce/index.tsx @@ -1,5 +1,4 @@ -import { memo, type ReactElement, type SyntheticEvent, useCallback, useContext, useMemo, useRef, useState } from 'react' - +import { memo, type ReactElement, useContext, useMemo } from 'react' import { Autocomplete, Box, @@ -9,20 +8,23 @@ import { Tooltip, Popper, type PopperProps, - type AutocompleteValue, type MenuItemProps, MenuItem, } from '@mui/material' -import { SafeTxContext } from '../../SafeTxProvider' +import { Controller, useForm } from 'react-hook-form' + +import { SafeTxContext } from '@/components/tx-flow/SafeTxProvider' import RotateLeftIcon from '@mui/icons-material/RotateLeft' import NumberField from '@/components/common/NumberField' import { useQueuedTxByNonce } from '@/hooks/useTxQueue' import useSafeInfo from '@/hooks/useSafeInfo' -import css from './styles.module.css' import useAddressBook from '@/hooks/useAddressBook' import { getLatestTransactions } from '@/utils/tx-list' import { getTransactionType } from '@/hooks/useTransactionType' import usePreviousNonces from '@/hooks/usePreviousNonces' +import { isRejectionTx } from '@/utils/transactions' + +import css from './styles.module.css' const CustomPopper = function (props: PopperProps) { return @@ -50,84 +52,131 @@ const NonceFormOption = memo(function NonceFormOption({ ) }) -const TxNonce = () => { - const [error, setError] = useState(false) - const { safe } = useSafeInfo() +enum TxNonceFormFieldNames { + NONCE = 'nonce', +} + +const TxNonceForm = ({ nonce, recommendedNonce }: { nonce: number; recommendedNonce: number }) => { + const { safeTx, setNonce } = useContext(SafeTxContext) const previousNonces = usePreviousNonces() - const { nonce, setNonce, safeTx, recommendedNonce } = useContext(SafeTxContext) - const isEmpty = useRef(false) + const { safe } = useSafeInfo() + const isEditable = !safeTx || safeTx?.signatures.size === 0 - const readonly = !isEditable + const readOnly = !isEditable || isRejectionTx(safeTx) - const isValidInput = useCallback( - (value: string | AutocompleteValue) => { - return Number(value) >= safe.nonce + const formMethods = useForm({ + defaultValues: { + [TxNonceFormFieldNames.NONCE]: nonce.toString(), }, - [safe.nonce], - ) + mode: 'all', + }) - const handleChange = useCallback( - (_e: SyntheticEvent, value: string | AutocompleteValue) => { - isEmpty.current = value === '' - const nonce = Number(value) - if (isNaN(nonce)) return - setError(!isValidInput(value)) - setNonce(nonce) - }, - [isValidInput, setNonce], - ) + const resetNonce = () => { + formMethods.setValue(TxNonceFormFieldNames.NONCE, recommendedNonce.toString()) + } + + return ( + { + const newNonce = Number(value) + + if (isNaN(newNonce)) { + return 'Nonce must be a number' + } - const resetNonce = useCallback(() => { - setError(false) - isEmpty.current = false - setNonce(recommendedNonce) - }, [recommendedNonce, setNonce]) + if (newNonce < safe.nonce) { + return `Nonce can't be lower than ${safe.nonce}` + } - if (nonce === undefined) return + if (newNonce >= Number.MAX_SAFE_INTEGER) { + return 'Nonce is too high' + } + + // Update contect with valid nonce + setNonce(newNonce) + }, + }} + render={({ field, fieldState }) => { + return ( + field.onChange(value)} + onInputChange={(_, value) => field.onChange(value)} + onBlur={() => { + field.onBlur() + + if (fieldState.error) { + formMethods.setValue(field.name, recommendedNonce.toString()) + } + }} + options={previousNonces} + disabled={readOnly} + getOptionLabel={(option) => option.toString()} + renderOption={(props, option) => { + return + }} + disableClearable + componentsProps={{ + paper: { + elevation: 2, + }, + }} + renderInput={(params) => { + return ( + + + + + + + + + ), + readOnly, + }} + className={css.input} + sx={{ width: `${field.value.length}em`, minWidth: '5em', maxWidth: '200px' }} + /> + + ) + }} + PopperComponent={CustomPopper} + /> + ) + }} + /> + ) +} + +const TxNonce = () => { + const { nonce, recommendedNonce } = useContext(SafeTxContext) return ( Nonce # - option.toString()} - renderOption={(props, option: number) => } - disableClearable - componentsProps={{ - paper: { - elevation: 2, - }, - }} - renderInput={(params) => ( - - - - - - - - ), - readOnly: readonly, - }} - className={css.input} - sx={{ minWidth: `${nonce.toString().length + 0.5}em` }} - /> - )} - PopperComponent={CustomPopper} - /> + {nonce === undefined || recommendedNonce === undefined ? ( + + ) : ( + + )} ) } diff --git a/src/components/tx-flow/common/TxNonce/styles.module.css b/src/components/tx-flow/common/TxNonce/styles.module.css index edbe5b1664..6aa41cc01f 100644 --- a/src/components/tx-flow/common/TxNonce/styles.module.css +++ b/src/components/tx-flow/common/TxNonce/styles.module.css @@ -4,7 +4,6 @@ .input input { font-weight: bold; - text-align: center; padding-right: 6px !important; } diff --git a/src/components/tx-flow/flows/TokenTransfer/CreateTokenTransfer.tsx b/src/components/tx-flow/flows/TokenTransfer/CreateTokenTransfer.tsx index 6ee6c178af..44f2e7a844 100644 --- a/src/components/tx-flow/flows/TokenTransfer/CreateTokenTransfer.tsx +++ b/src/components/tx-flow/flows/TokenTransfer/CreateTokenTransfer.tsx @@ -1,4 +1,4 @@ -import { type ReactElement, useCallback, useMemo, useState } from 'react' +import { type ReactElement, useMemo, useState, useCallback, useContext, useEffect } from 'react' import { type TokenInfo } from '@safe-global/safe-gateway-typescript-sdk' import { useVisibleBalances } from '@/hooks/useVisibleBalances' import useAddressBook from '@/hooks/useAddressBook' @@ -24,6 +24,7 @@ import TxCard from '../../common/TxCard' import { formatVisualAmount, safeFormatUnits } from '@/utils/formatters' import commonCss from '@/components/tx-flow/common/styles.module.css' import TokenAmountInput, { TokenAmountFields } from '@/components/common/TokenAmountInput' +import { SafeTxContext } from '@/components/tx-flow/SafeTxProvider' export const AutocompleteItem = (item: { tokenInfo: TokenInfo; balance: string }): ReactElement => ( @@ -57,8 +58,15 @@ const CreateTokenTransfer = ({ const isOnlySpendingLimitBeneficiary = useIsOnlySpendingLimitBeneficiary() const spendingLimits = useAppSelector(selectSpendingLimits) const wallet = useWallet() + const { setNonce } = useContext(SafeTxContext) const [recipientFocus, setRecipientFocus] = useState(!params.recipient) + useEffect(() => { + if (txNonce) { + setNonce(txNonce) + } + }, [setNonce, txNonce]) + const formMethods = useForm({ defaultValues: { ...params, @@ -157,7 +165,7 @@ const CreateTokenTransfer = ({ /> {isDisabled && ( - + $SAFE is currently non-transferable. diff --git a/src/utils/transactions.ts b/src/utils/transactions.ts index 79253b2ccf..c6a8dbd856 100644 --- a/src/utils/transactions.ts +++ b/src/utils/transactions.ts @@ -30,6 +30,7 @@ import { Multi_send__factory } from '@/types/contracts' import { ethers } from 'ethers' import { type BaseTransaction } from '@safe-global/safe-apps-sdk' import { id } from 'ethers/lib/utils' +import { isEmptyHexData } from '@/utils/hex' export const makeTxFromDetails = (txDetails: TransactionDetails): Transaction => { const getMissingSigners = ({ @@ -266,3 +267,7 @@ export const decodeMultiSendTxs = (encodedMultiSendData: string): BaseTransactio return txs } + +export const isRejectionTx = (tx?: SafeTransaction) => { + return !!tx && !!tx.data.data && !!isEmptyHexData(tx.data.data) && tx.data.value === '0' +}