From da9031568f12d014c2d8fcb0af9c2eea18687300 Mon Sep 17 00:00:00 2001 From: nicolas Date: Tue, 23 Mar 2021 05:01:49 -0300 Subject: [PATCH] Advanced Options refactor (#2029) * useEstimateTxGas: set the correct value of isOffChainSignature if gas estimation fails --- src/logic/hooks/useEstimateTransactionGas.tsx | 5 +- .../components/ConfirmTransactionModal.tsx | 12 +-- .../ContractInteraction/Review/index.tsx | 3 + .../ReviewCustomTx/index.tsx | 9 +- .../screens/ReviewCollectible/index.tsx | 3 + .../screens/ReviewSendFundsTx/index.tsx | 7 +- .../Settings/Advanced/RemoveModuleModal.tsx | 3 + .../AddOwnerModal/screens/Review/index.tsx | 3 + .../RemoveOwnerModal/screens/Review/index.tsx | 3 + .../screens/Review/index.tsx | 3 + .../SpendingLimit/NewLimitModal/Review.tsx | 3 + .../SpendingLimit/RemoveLimitModal.tsx | 3 + .../ChangeThreshold/index.tsx | 6 +- .../Settings/UpdateSafeModal/index.tsx | 9 +- .../TxList/modals/ApproveTxModal.tsx | 5 +- .../TxList/modals/RejectTxModal.tsx | 3 + .../helpers/EditTxParametersForm/index.tsx | 96 ++++++++++--------- .../helpers/EditableTxParameters.tsx | 7 +- .../helpers/TxParametersDetail/index.tsx | 81 ++++++---------- .../components/Transactions/helpers/utils.ts | 10 +- 20 files changed, 153 insertions(+), 121 deletions(-) diff --git a/src/logic/hooks/useEstimateTransactionGas.tsx b/src/logic/hooks/useEstimateTransactionGas.tsx index 8cd5e268..567d7659 100644 --- a/src/logic/hooks/useEstimateTransactionGas.tsx +++ b/src/logic/hooks/useEstimateTransactionGas.tsx @@ -218,10 +218,9 @@ export const useEstimateTransactionGas = ({ ) const fixedGasCosts = getFixedGasCosts(Number(threshold)) + const isOffChainSignature = checkIfOffChainSignatureIsPossible(isExecution, smartContractWallet, safeVersion) try { - const isOffChainSignature = checkIfOffChainSignatureIsPossible(isExecution, smartContractWallet, safeVersion) - const gasEstimation = await estimateTransactionGas({ safeAddress, txRecipient, @@ -279,7 +278,7 @@ export const useEstimateTransactionGas = ({ gasLimit: '0', isExecution, isCreation, - isOffChainSignature: false, + isOffChainSignature, }) } } diff --git a/src/routes/safe/components/Apps/components/ConfirmTransactionModal.tsx b/src/routes/safe/components/Apps/components/ConfirmTransactionModal.tsx index 784c9e33..4815717f 100644 --- a/src/routes/safe/components/Apps/components/ConfirmTransactionModal.tsx +++ b/src/routes/safe/components/Apps/components/ConfirmTransactionModal.tsx @@ -2,7 +2,7 @@ import React, { useEffect, useMemo, useState } from 'react' import { Icon, ModalFooterConfirmation, Text, Title } from '@gnosis.pm/safe-react-components' import { Transaction } from '@gnosis.pm/safe-apps-sdk-v1' import styled from 'styled-components' -import { useDispatch, useSelector } from 'react-redux' +import { useDispatch } from 'react-redux' import AddressInfo from 'src/components/AddressInfo' import DividerLine from 'src/components/DividerLine' @@ -26,7 +26,6 @@ import GasEstimationInfo from './GasEstimationInfo' import { getNetworkInfo } from 'src/config' import { TransactionParams } from './AppFrame' import { EstimationStatus, useEstimateTransactionGas } from 'src/logic/hooks/useEstimateTransactionGas' -import { safeThresholdSelector } from 'src/logic/safe/store/selectors' import Modal from 'src/components/Modal' import Row from 'src/components/layout/Row' import Hairline from 'src/components/layout/Hairline' @@ -123,7 +122,6 @@ export const ConfirmTransactionModal = ({ onTxReject, }: OwnProps): React.ReactElement | null => { const [estimatedSafeTxGas, setEstimatedSafeTxGas] = useState(0) - const threshold = useSelector(safeThresholdSelector) || 1 const txRecipient: string | undefined = useMemo(() => (txs.length > 1 ? MULTI_SEND_ADDRESS : txs[0]?.to), [txs]) const txData: string | undefined = useMemo(() => (txs.length > 1 ? encodeMultiSendCall(txs) : txs[0]?.data), [txs]) @@ -174,8 +172,6 @@ export const ConfirmTransactionModal = ({ onClose() } - const getParametersStatus = () => (threshold > 1 ? 'ETH_DISABLED' : 'ENABLED') - const confirmTransactions = async (txParameters: TxParameters) => { await dispatch( createTransaction( @@ -274,9 +270,9 @@ export const ConfirmTransactionModal = ({ {txEstimationExecutionStatus === EstimationStatus.LOADING ? null : ( @@ -297,16 +293,16 @@ export const ConfirmTransactionModal = ({ return ( {(txParameters, toggleEditMode) => ( <> - {body(txParameters, toggleEditMode)} diff --git a/src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/Review/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/Review/index.tsx index 5311f2e4..9352bcdb 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/Review/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/Review/index.tsx @@ -126,6 +126,8 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE return (
diff --git a/src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/ReviewCustomTx/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/ReviewCustomTx/index.tsx index d7e4e736..0a39b8dc 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/ReviewCustomTx/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/ReviewCustomTx/index.tsx @@ -94,7 +94,13 @@ const ReviewCustomTx = ({ onClose, onPrev, tx }: Props): React.ReactElement => { } return ( - + {(txParameters, toggleEditMode) => ( <> @@ -168,6 +174,7 @@ const ReviewCustomTx = ({ onClose, onPrev, tx }: Props): React.ReactElement => { onEdit={toggleEditMode} isTransactionCreation={isCreation} isTransactionExecution={isExecution} + isOffChainSignature={isOffChainSignature} /> {txEstimationExecutionStatus === EstimationStatus.LOADING ? null : ( diff --git a/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx index f46d897c..ea68ea2a 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx @@ -140,6 +140,8 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement = return (
diff --git a/src/routes/safe/components/Balances/SendModal/screens/ReviewSendFundsTx/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/ReviewSendFundsTx/index.tsx index 21e0075c..1e2ab83c 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/ReviewSendFundsTx/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/screens/ReviewSendFundsTx/index.tsx @@ -178,6 +178,8 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE return ( - - {/* Disclaimer */} + + {/* Disclaimer */} {txEstimationExecutionStatus !== EstimationStatus.LOADING && (
diff --git a/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/Review/index.tsx b/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/Review/index.tsx index dd53427a..1acbd6ef 100644 --- a/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/Review/index.tsx +++ b/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/Review/index.tsx @@ -101,6 +101,8 @@ export const ReviewAddOwner = ({ onClickBack, onClose, onSubmit, values }: Revie return ( diff --git a/src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/Review/index.tsx b/src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/Review/index.tsx index c69aba52..7d77084c 100644 --- a/src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/Review/index.tsx +++ b/src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/Review/index.tsx @@ -123,6 +123,8 @@ export const ReviewRemoveOwnerModal = ({ return ( {txEstimationExecutionStatus === EstimationStatus.LOADING ? null : ( diff --git a/src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/Review/index.tsx b/src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/Review/index.tsx index fa47290b..52327599 100644 --- a/src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/Review/index.tsx +++ b/src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/Review/index.tsx @@ -120,6 +120,8 @@ export const ReviewReplaceOwnerModal = ({ return ( diff --git a/src/routes/safe/components/Settings/SpendingLimit/NewLimitModal/Review.tsx b/src/routes/safe/components/Settings/SpendingLimit/NewLimitModal/Review.tsx index 9ef63bd4..9d803ded 100644 --- a/src/routes/safe/components/Settings/SpendingLimit/NewLimitModal/Review.tsx +++ b/src/routes/safe/components/Settings/SpendingLimit/NewLimitModal/Review.tsx @@ -233,6 +233,8 @@ export const ReviewSpendingLimits = ({ onBack, onClose, txToken, values }: Revie return (
diff --git a/src/routes/safe/components/Settings/SpendingLimit/RemoveLimitModal.tsx b/src/routes/safe/components/Settings/SpendingLimit/RemoveLimitModal.tsx index 0c5a5090..6ad0ec6e 100644 --- a/src/routes/safe/components/Settings/SpendingLimit/RemoveLimitModal.tsx +++ b/src/routes/safe/components/Settings/SpendingLimit/RemoveLimitModal.tsx @@ -116,6 +116,8 @@ export const RemoveLimitModal = ({ onClose, spendingLimit, open }: RemoveSpendin description="Remove the selected Spending Limit" > diff --git a/src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx b/src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx index 8f6a7455..0560b57a 100644 --- a/src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx +++ b/src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx @@ -84,8 +84,6 @@ export const ChangeThresholdModal = ({ } }, [safeAddress, editedThreshold]) - const getParametersStatus = () => (threshold > 1 ? 'ETH_DISABLED' : 'ENABLED') - const handleSubmit = async ({ txParameters }) => { await dispatch( createTransaction({ @@ -120,6 +118,8 @@ export const ChangeThresholdModal = ({ return ( {txEstimationExecutionStatus !== EstimationStatus.LOADING && ( diff --git a/src/routes/safe/components/Settings/UpdateSafeModal/index.tsx b/src/routes/safe/components/Settings/UpdateSafeModal/index.tsx index aea5f0ce..b56ae3d0 100644 --- a/src/routes/safe/components/Settings/UpdateSafeModal/index.tsx +++ b/src/routes/safe/components/Settings/UpdateSafeModal/index.tsx @@ -76,7 +76,13 @@ export const UpdateSafeModal = ({ onClose, safeAddress }: Props): React.ReactEle }) return ( - + {(txParameters, toggleEditMode) => ( <> @@ -116,6 +122,7 @@ export const UpdateSafeModal = ({ onClose, safeAddress }: Props): React.ReactEle compact={false} isTransactionCreation={isCreation} isTransactionExecution={isExecution} + isOffChainSignature={isOffChainSignature} /> {txEstimationExecutionStatus === EstimationStatus.LOADING ? null : ( diff --git a/src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx b/src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx index 687dea96..c8b24649 100644 --- a/src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx +++ b/src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx @@ -317,6 +317,8 @@ export const ApproveTxModal = ({ return ( )} diff --git a/src/routes/safe/components/Transactions/TxList/modals/RejectTxModal.tsx b/src/routes/safe/components/Transactions/TxList/modals/RejectTxModal.tsx index 48280b8e..8ebf6186 100644 --- a/src/routes/safe/components/Transactions/TxList/modals/RejectTxModal.tsx +++ b/src/routes/safe/components/Transactions/TxList/modals/RejectTxModal.tsx @@ -82,6 +82,8 @@ export const RejectTxModal = ({ isOpen, onClose, gwTransaction }: Props): React. return ( diff --git a/src/routes/safe/components/Transactions/helpers/EditTxParametersForm/index.tsx b/src/routes/safe/components/Transactions/helpers/EditTxParametersForm/index.tsx index 436df795..a497eb2b 100644 --- a/src/routes/safe/components/Transactions/helpers/EditTxParametersForm/index.tsx +++ b/src/routes/safe/components/Transactions/helpers/EditTxParametersForm/index.tsx @@ -15,11 +15,11 @@ import GnoForm from 'src/components/forms/GnoForm' import { TxParameters } from 'src/routes/safe/container/hooks/useTransactionParameters' import { composeValidators, minValue } from 'src/components/forms/validator' -import { ParametersStatus, areSafeParamsEnabled, areEthereumParamsEnabled } from '../utils' +import { ParametersStatus, areSafeParamsEnabled, areEthereumParamsVisible, ethereumTxParametersTitle } from '../utils' import { getNetworkInfo } from 'src/config' const StyledDivider = styled(Divider)` - margin: 0px; + margin: 16px 0; ` const SafeOptions = styled.div` @@ -39,7 +39,7 @@ const EthereumOptions = styled.div` } ` const StyledLink = styled(Link)` - margin: 16px 0; + margin: 16px 0 0 0; display: inline-flex; align-items: center; @@ -65,6 +65,7 @@ interface Props { txParameters: TxParameters onClose: (txParameters?: TxParameters) => void parametersStatus: ParametersStatus + isExecution: boolean } const formValidation = (values) => { @@ -101,6 +102,7 @@ export const EditTxParametersForm = ({ onClose, txParameters, parametersStatus = 'ENABLED', + isExecution, }: Props): React.ReactElement => { const classes = useStyles() const { safeNonce, safeTxGas, ethNonce, ethGasLimit, ethGasPrice } = txParameters @@ -142,7 +144,7 @@ export const EditTxParametersForm = ({ {() => ( <> - Safe transactions parameters + Safe transaction @@ -168,49 +170,53 @@ export const EditTxParametersForm = ({ /> - - Ethereum transactions parameters - + {areEthereumParamsVisible(parametersStatus) && ( + <> + + {ethereumTxParametersTitle(isExecution)} + - - - - - + + + + + - - - How can I configure the gas price manually? - - - + + + How can I configure these parameters manually? + + + + + )} diff --git a/src/routes/safe/components/Transactions/helpers/EditableTxParameters.tsx b/src/routes/safe/components/Transactions/helpers/EditableTxParameters.tsx index a4a29ada..fb9eac57 100644 --- a/src/routes/safe/components/Transactions/helpers/EditableTxParameters.tsx +++ b/src/routes/safe/components/Transactions/helpers/EditableTxParameters.tsx @@ -7,6 +7,8 @@ import { safeThresholdSelector } from 'src/logic/safe/store/selectors' type Props = { children: (txParameters: TxParameters, toggleStatus: (txParameters?: TxParameters) => void) => any + isOffChainSignature: boolean + isExecution: boolean parametersStatus?: ParametersStatus ethGasLimit?: TxParameters['ethGasLimit'] ethGasPrice?: TxParameters['ethGasPrice'] @@ -17,6 +19,8 @@ type Props = { export const EditableTxParameters = ({ children, + isOffChainSignature, + isExecution, parametersStatus, ethGasLimit, ethGasPrice, @@ -27,7 +31,7 @@ export const EditableTxParameters = ({ const [isEditMode, toggleEditMode] = useState(false) const [useManualValues, setUseManualValues] = useState(false) const threshold = useSelector(safeThresholdSelector) || 1 - const defaultParameterStatus = threshold > 1 ? 'ETH_DISABLED' : 'ENABLED' + const defaultParameterStatus = isOffChainSignature && threshold > 1 ? 'ETH_HIDDEN' : 'ENABLED' const txParameters = useTransactionParameters({ parameterStatus: parametersStatus || defaultParameterStatus, initialEthGasLimit: ethGasLimit, @@ -65,6 +69,7 @@ export const EditableTxParameters = ({ return isEditMode ? ( void compact?: boolean parametersStatus?: ParametersStatus - isTransactionExecution: boolean isTransactionCreation: boolean + isTransactionExecution: boolean + isOffChainSignature: boolean } export const TxParametersDetail = ({ @@ -46,11 +47,12 @@ export const TxParametersDetail = ({ parametersStatus, isTransactionCreation, isTransactionExecution, + isOffChainSignature, }: Props): ReactElement | null => { const threshold = useSelector(safeThresholdSelector) || 1 - const defaultParameterStatus = threshold > 1 ? 'ETH_DISABLED' : 'ENABLED' + const defaultParameterStatus = isOffChainSignature && threshold > 1 ? 'ETH_HIDDEN' : 'ENABLED' - if (!isTransactionExecution && !isTransactionCreation) { + if (!isTransactionExecution && !isTransactionCreation && isOffChainSignature) { return null } @@ -62,7 +64,7 @@ export const TxParametersDetail = ({ - Safe transactions parameters + Safe transaction @@ -95,57 +97,30 @@ export const TxParametersDetail = ({ - - - Ethereum transaction parameters - - + {areEthereumParamsVisible(parametersStatus || defaultParameterStatus) && ( + <> + + + {ethereumTxParametersTitle(isTransactionExecution)} + + - - - Ethereum nonce - - - {txParameters.ethNonce} - - + + Nonce + {txParameters.ethNonce} + - - - Ethereum gas limit - - - {txParameters.ethGasLimit} - - - - - - Ethereum gas price - - - {txParameters.ethGasPrice} - - + + Gas limit + {txParameters.ethGasLimit} + + + Gas price + {txParameters.ethGasPrice} + + + )} Edit diff --git a/src/routes/safe/components/Transactions/helpers/utils.ts b/src/routes/safe/components/Transactions/helpers/utils.ts index ce1564e6..ccb3742f 100644 --- a/src/routes/safe/components/Transactions/helpers/utils.ts +++ b/src/routes/safe/components/Transactions/helpers/utils.ts @@ -1,8 +1,8 @@ -export type ParametersStatus = 'ENABLED' | 'DISABLED' | 'SAFE_DISABLED' | 'ETH_DISABLED' | 'CANCEL_TRANSACTION' +export type ParametersStatus = 'ENABLED' | 'DISABLED' | 'SAFE_DISABLED' | 'ETH_HIDDEN' | 'CANCEL_TRANSACTION' -export const areEthereumParamsEnabled = (parametersStatus: ParametersStatus): boolean => { +export const areEthereumParamsVisible = (parametersStatus: ParametersStatus): boolean => { return ( - parametersStatus === 'ENABLED' || (parametersStatus !== 'ETH_DISABLED' && parametersStatus !== 'CANCEL_TRANSACTION') + parametersStatus === 'ENABLED' || (parametersStatus !== 'ETH_HIDDEN' && parametersStatus !== 'CANCEL_TRANSACTION') ) } @@ -12,3 +12,7 @@ export const areSafeParamsEnabled = (parametersStatus: ParametersStatus): boolea (parametersStatus !== 'SAFE_DISABLED' && parametersStatus !== 'CANCEL_TRANSACTION') ) } + +export const ethereumTxParametersTitle = (isExecution: boolean): string => { + return `Owner transaction ${isExecution ? '(Execution)' : '(On-chain approval)'}` +}