Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Fix: conditions to display "Execute transaction" #3257

Merged
merged 28 commits into from
Jan 14, 2022
Merged

Fix: conditions to display "Execute transaction" #3257

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a216e28
Build: create a hook for getting the recommended nonce
Jan 6, 2022
0266a1e
Build: extract "can execute transaction" to a hook
Jan 6, 2022
b084b55
Build: remove unused return type
Jan 6, 2022
f0fb4cf
Build: use "can execute" hook to retrieve condition to show the checkbox
Jan 6, 2022
ab24bd3
Refactor: use ComponentProps to get component types
Jan 6, 2022
240478a
Fix: remove from tests logic extracted to hook
Jan 6, 2022
ffc1bfc
refactor: refactor the useCanExecuteType to test the logic
Jan 7, 2022
d56093b
refactor: exclude the spending limit logic from useCanTxExecute
Jan 8, 2022
57612cf
refactor: use useCanTxExecute in other review modals
Jan 8, 2022
a989041
fix: improve useCanTxExecute types and logic
Jan 8, 2022
1fa2d51
refactor: do not return isExecution from useEstimateTransactionGas
Jan 8, 2022
e1c7de6
fix: approve modal logic
Jan 9, 2022
93ef7c2
build: include PENDING_FAILED transactions in awaiting for execution
Jan 9, 2022
674c8a8
build: flag execution to calculate tx gas
Jan 9, 2022
5cca7a9
build: include PENDING_FAILED tx status in showing execution tooltip …
Jan 9, 2022
31f0efb
fix: adjust tests to reflect latest logic
Jan 9, 2022
01ac45e
fix: add missing hook dependency
Jan 10, 2022
144a35c
fix: avoid race conditions when fetching from the backend
Jan 10, 2022
2befeb9
fix: implement minor PR comments
Jan 10, 2022
738e7a2
build: display the Execute checkbox if safe nonce is edited to be next
Jan 10, 2022
9b622dd
fix: update tests to take manualSafeNonce
Jan 10, 2022
be54a48
fix: remove wrong check
Jan 10, 2022
f501dde
refactor: rename execution related variables
Jan 11, 2022
669f8e4
fix: rename variable
Jan 11, 2022
3f26613
Obtain isOffChainSignature by calling a function instead of prop dril…
Jan 13, 2022
fa26e8d
Calculate tx gas even for off chain transactions
Jan 13, 2022
572c8f8
Merge branch 'dev' into hide-execute-checkbox-conditions
DiogoSoaress Jan 13, 2022
7e8d179
fix TransactionFees rendering
Jan 13, 2022
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
12 changes: 6 additions & 6 deletions src/logic/hooks/useEstimateTransactionGas.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,14 @@ export const useEstimateTransactionGas = ({
const { address: safeAddress = '', threshold = 1, currentVersion: safeVersion = '' } = useSelector(currentSafe) ?? {}
const { account: from, smartContractWallet, name: providerName } = useSelector(providerSelector)

const canExecute = useCanTxExecute(isExecution, manualSafeNonce, preApprovingOwner, txConfirmations?.size)
const canTxExecute = useCanTxExecute(isExecution, manualSafeNonce, preApprovingOwner, txConfirmations?.size)

useEffect(() => {
const estimateGas = async () => {
if (!txData.length) {
return
}
const isOffChainSignature = checkIfOffChainSignatureIsPossible(canExecute, smartContractWallet, safeVersion)
const isOffChainSignature = checkIfOffChainSignatureIsPossible(canTxExecute, smartContractWallet, safeVersion)
const isCreation = checkIfTxIsCreation(txConfirmations?.size || 0, txType)

if (isOffChainSignature && !isCreation) {
Expand Down Expand Up @@ -155,15 +155,15 @@ export const useEstimateTransactionGas = ({
)
}

if (canExecute || approvalAndExecution) {
if (canTxExecute || approvalAndExecution) {
ethGasLimitEstimation = await estimateTransactionGasLimit({
safeAddress,
safeVersion,
txRecipient,
txData,
txAmount: txAmount || '0',
txConfirmations,
isExecution: canExecute,
isExecution: canTxExecute,
isOffChainSignature,
operation: operation || Operation.CALL,
from,
Expand All @@ -179,7 +179,7 @@ export const useEstimateTransactionGas = ({
const gasCost = fromTokenUnit(estimatedGasCosts, nativeCurrency.decimals)
const gasCostFormatted = formatAmount(gasCost)

if (canExecute) {
if (canTxExecute) {
transactionCallSuccess = await checkTransactionExecution({
safeAddress,
safeVersion,
Expand Down Expand Up @@ -238,7 +238,7 @@ export const useEstimateTransactionGas = ({
manualGasPrice,
manualGasLimit,
manualSafeNonce,
canExecute,
canTxExecute,
])

return gasEstimation
Expand Down
8 changes: 4 additions & 4 deletions src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ export const ReviewConfirm = ({
})

const [buttonStatus, setButtonStatus] = useEstimationStatus(txEstimationExecutionStatus)
const [executionApproved, setExecutionApproved] = useState<boolean>(true)
const [shouldExecute, setShouldExecute] = useState<boolean>(true)
const canTxExecute = useCanTxExecute(false, manualSafeNonce)
const isExecution = canTxExecute && executionApproved
const isExecution = canTxExecute && shouldExecute
usame-algan marked this conversation as resolved.
Show resolved Hide resolved

// Decode tx data.
useEffect(() => {
Expand Down Expand Up @@ -161,7 +161,7 @@ export const ReviewConfirm = ({
safeTxGas: txParameters.safeTxGas,
ethParameters: txParameters,
notifiedTransaction: TX_NOTIFICATION_TYPES.STANDARD_TX,
delayExecution: !executionApproved,
delayExecution: !shouldExecute,
},
handleUserConfirmation,
onReject,
Expand Down Expand Up @@ -234,7 +234,7 @@ export const ReviewConfirm = ({

{!isMultiSend && <Divider />}

{canTxExecute && <ExecuteCheckbox onChange={setExecutionApproved} />}
{canTxExecute && <ExecuteCheckbox onChange={setShouldExecute} />}

{/* Tx Parameters */}
<TxParametersDetail
Expand Down
8 changes: 4 additions & 4 deletions src/routes/safe/components/Apps/components/SignMessageModal/ReviewMessage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export const ReviewMessage = ({
manualGasLimit,
manualSafeNonce,
})
const isExecution = useCanTxExecute(false, manualSafeNonce)
const canTxExecute = useCanTxExecute(false, manualSafeNonce)

const [buttonStatus, setButtonStatus] = useEstimationStatus(txEstimationExecutionStatus)

Expand Down Expand Up @@ -180,7 +180,7 @@ export const ReviewMessage = ({
safeTxGas={gasEstimation}
closeEditModalCallback={closeEditModalCallback}
isOffChainSignature={isOffChainSignature}
isExecution={isExecution}
isExecution={canTxExecute}
>
{(txParameters, toggleEditMode) => (
<div>
Expand Down Expand Up @@ -229,7 +229,7 @@ export const ReviewMessage = ({
txParameters={txParameters}
onEdit={toggleEditMode}
isTransactionCreation={isCreation}
isTransactionExecution={isExecution}
isTransactionExecution={canTxExecute}
isOffChainSignature={isOffChainSignature}
/>
</Container>
Expand All @@ -239,7 +239,7 @@ export const ReviewMessage = ({
<ReviewInfoText
gasCostFormatted={isOwner ? gasCostFormatted : undefined}
isCreation={isCreation}
isExecution={isExecution}
isExecution={canTxExecute}
isOffChainSignature={isOffChainSignature}
safeNonce={txParameters.safeNonce}
txEstimationExecutionStatus={txEstimationExecutionStatus}
Expand Down
14 changes: 7 additions & 7 deletions src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/Review/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE
const [manualGasPrice, setManualGasPrice] = useState<string | undefined>()
const [manualGasLimit, setManualGasLimit] = useState<string | undefined>()
const [manualSafeNonce, setManualSafeNonce] = useState<number | undefined>()
const [executionApproved, setExecutionApproved] = useState<boolean>(true)
const [shouldExecute, setShouldExecute] = useState<boolean>(true)
const addressName = useSelector((state) => addressBookEntryName(state, { address: tx.contractAddress as string }))

const [txInfo, setTxInfo] = useState<{
Expand Down Expand Up @@ -90,7 +90,7 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE
})

const canTxExecute = useCanTxExecute(false, manualSafeNonce)
const doExecute = canTxExecute && executionApproved
const willExecute = canTxExecute && shouldExecute
const [buttonStatus] = useEstimationStatus(txEstimationExecutionStatus)

useEffect(() => {
Expand All @@ -113,7 +113,7 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE
safeTxGas: txParameters.safeTxGas,
ethParameters: txParameters,
notifiedTransaction: TX_NOTIFICATION_TYPES.STANDARD_TX,
delayExecution: !executionApproved,
delayExecution: !shouldExecute,
}),
)
} else {
Expand Down Expand Up @@ -150,7 +150,7 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE
return (
<EditableTxParameters
isOffChainSignature={isOffChainSignature}
isExecution={doExecute}
isExecution={willExecute}
ethGasLimit={gasLimit}
ethGasPrice={gasPriceFormatted}
safeTxGas={gasEstimation}
Expand Down Expand Up @@ -235,21 +235,21 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE
</Col>
</Row>

{canTxExecute && <ExecuteCheckbox onChange={setExecutionApproved} />}
{canTxExecute && <ExecuteCheckbox onChange={setShouldExecute} />}

{/* Tx Parameters */}
<TxParametersDetail
txParameters={txParameters}
onEdit={toggleEditMode}
isTransactionCreation={isCreation}
isTransactionExecution={doExecute}
isTransactionExecution={willExecute}
isOffChainSignature={isOffChainSignature}
/>
</Block>
<ReviewInfoText
gasCostFormatted={gasCostFormatted}
isCreation={isCreation}
isExecution={doExecute}
isExecution={willExecute}
isOffChainSignature={isOffChainSignature}
safeNonce={txParameters.safeNonce}
txEstimationExecutionStatus={txEstimationExecutionStatus}
Expand Down
14 changes: 7 additions & 7 deletions ...s/safe/components/Balances/SendModal/screens/ContractInteraction/ReviewCustomTx/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const ReviewCustomTx = ({ onClose, onPrev, tx }: Props): ReactElement => {
const dispatch = useDispatch()
const safeAddress = extractSafeAddress()
const nativeCurrency = getNativeCurrency()
const [executionApproved, setExecutionApproved] = useState<boolean>(true)
const [shouldExecute, setShouldExecute] = useState<boolean>(true)

const {
gasLimit,
Expand All @@ -67,7 +67,7 @@ const ReviewCustomTx = ({ onClose, onPrev, tx }: Props): ReactElement => {
})

const canTxExecute = useCanTxExecute()
iamacook marked this conversation as resolved.
Show resolved Hide resolved
const doExecute = canTxExecute && executionApproved
const willExecute = canTxExecute && shouldExecute
const [buttonStatus] = useEstimationStatus(txEstimationExecutionStatus)

const submitTx = (txParameters: TxParameters) => {
Expand All @@ -86,7 +86,7 @@ const ReviewCustomTx = ({ onClose, onPrev, tx }: Props): ReactElement => {
safeTxGas: txParameters.safeTxGas,
ethParameters: txParameters,
notifiedTransaction: TX_NOTIFICATION_TYPES.STANDARD_TX,
delayExecution: !executionApproved,
delayExecution: !shouldExecute,
}),
)
} else {
Expand All @@ -98,7 +98,7 @@ const ReviewCustomTx = ({ onClose, onPrev, tx }: Props): ReactElement => {
return (
<EditableTxParameters
isOffChainSignature={isOffChainSignature}
isExecution={doExecute}
isExecution={willExecute}
ethGasLimit={gasLimit}
ethGasPrice={gasPriceFormatted}
safeTxGas={gasEstimation.toString()}
Expand Down Expand Up @@ -152,22 +152,22 @@ const ReviewCustomTx = ({ onClose, onPrev, tx }: Props): ReactElement => {
</Col>
</Row>

{canTxExecute && <ExecuteCheckbox onChange={setExecutionApproved} />}
{canTxExecute && <ExecuteCheckbox onChange={setShouldExecute} />}

{/* Tx Parameters */}
<TxParametersDetail
txParameters={txParameters}
onEdit={toggleEditMode}
isTransactionCreation={isCreation}
isTransactionExecution={doExecute}
isTransactionExecution={willExecute}
isOffChainSignature={isOffChainSignature}
/>
</Block>
{txEstimationExecutionStatus === EstimationStatus.LOADING ? null : (
<ReviewInfoText
gasCostFormatted={gasCostFormatted}
isCreation={isCreation}
isExecution={doExecute}
isExecution={willExecute}
isOffChainSignature={isOffChainSignature}
safeNonce={txParameters.safeNonce}
txEstimationExecutionStatus={txEstimationExecutionStatus}
Expand Down
14 changes: 7 additions & 7 deletions src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement =
const [manualGasPrice, setManualGasPrice] = useState<string | undefined>()
const [manualGasLimit, setManualGasLimit] = useState<string | undefined>()
const [manualSafeNonce, setManualSafeNonce] = useState<number | undefined>()
const [executionApproved, setExecutionApproved] = useState<boolean>(true)
const [shouldExecute, setShouldExecute] = useState<boolean>(true)

const txToken = nftTokens.find(
({ assetAddress, tokenId }) => assetAddress === tx.assetAddress && tokenId === tx.nftTokenId,
Expand All @@ -82,7 +82,7 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement =
manualSafeNonce,
})
const canTxExecute = useCanTxExecute(false, manualSafeNonce)
const doExecute = canTxExecute && executionApproved
const willExecute = canTxExecute && shouldExecute
const [buttonStatus] = useEstimationStatus(txEstimationExecutionStatus)

useEffect(() => {
Expand Down Expand Up @@ -118,7 +118,7 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement =
safeTxGas: txParameters.safeTxGas,
ethParameters: txParameters,
notifiedTransaction: TX_NOTIFICATION_TYPES.STANDARD_TX,
delayExecution: !executionApproved,
delayExecution: !shouldExecute,
}),
)
} else {
Expand Down Expand Up @@ -159,7 +159,7 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement =
return (
<EditableTxParameters
isOffChainSignature={isOffChainSignature}
isExecution={doExecute}
isExecution={willExecute}
ethGasLimit={gasLimit}
ethGasPrice={gasPriceFormatted}
safeTxGas={gasEstimation}
Expand Down Expand Up @@ -202,21 +202,21 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement =
</Row>
)}

{canTxExecute && <ExecuteCheckbox onChange={setExecutionApproved} />}
{canTxExecute && <ExecuteCheckbox onChange={setShouldExecute} />}

{/* Tx Parameters */}
<TxParametersDetail
txParameters={txParameters}
onEdit={toggleEditMode}
isTransactionCreation={isCreation}
isTransactionExecution={doExecute}
isTransactionExecution={willExecute}
isOffChainSignature={isOffChainSignature}
/>
</Block>
<ReviewInfoText
gasCostFormatted={gasCostFormatted}
isCreation={isCreation}
isExecution={doExecute}
isExecution={willExecute}
isOffChainSignature={isOffChainSignature}
safeNonce={txParameters.safeNonce}
txEstimationExecutionStatus={txEstimationExecutionStatus}
Expand Down
14 changes: 7 additions & 7 deletions src/routes/safe/components/Balances/SendModal/screens/ReviewSendFundsTx/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE

const [buttonStatus, setButtonStatus] = useEstimationStatus(txEstimationExecutionStatus)
const isSpendingLimit = sameString(tx.txType, 'spendingLimit')
const [executionApproved, setExecutionApproved] = useState<boolean>(true)
const [shouldExecute, setShouldExecute] = useState<boolean>(true)

const canTxExecute = useCanTxExecute(false, manualSafeNonce)
const doExecute = canTxExecute && executionApproved
const willExecute = canTxExecute && shouldExecute

const submitTx = async (txParameters: TxParameters) => {
setButtonStatus(ButtonStatus.LOADING)
Expand Down Expand Up @@ -174,7 +174,7 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE
safeTxGas: txParameters.safeTxGas,
ethParameters: txParameters,
notifiedTransaction: TX_NOTIFICATION_TYPES.STANDARD_TX,
delayExecution: !executionApproved,
delayExecution: !shouldExecute,
}),
)
onClose()
Expand Down Expand Up @@ -208,7 +208,7 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE
return (
<EditableTxParameters
isOffChainSignature={isOffChainSignature}
isExecution={doExecute}
isExecution={willExecute}
ethGasLimit={gasLimit}
ethGasPrice={gasPriceFormatted}
safeTxGas={gasEstimation}
Expand Down Expand Up @@ -262,7 +262,7 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE
</Paragraph>
</Row>

{canTxExecute && !isSpendingLimit && <ExecuteCheckbox onChange={setExecutionApproved} />}
{canTxExecute && !isSpendingLimit && <ExecuteCheckbox onChange={setShouldExecute} />}

{/* Tx Parameters */}
{/* FIXME TxParameters should be updated to be used with spending limits */}
Expand All @@ -271,7 +271,7 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE
txParameters={txParameters}
onEdit={toggleEditMode}
isTransactionCreation={isCreation}
isTransactionExecution={doExecute}
isTransactionExecution={willExecute}
isOffChainSignature={isOffChainSignature}
/>
)}
Expand All @@ -283,7 +283,7 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE
<ReviewInfoText
gasCostFormatted={gasCostFormatted}
isCreation={isCreation}
isExecution={doExecute}
isExecution={willExecute}
isOffChainSignature={isOffChainSignature}
safeNonce={txParameters.safeNonce}
txEstimationExecutionStatus={txEstimationExecutionStatus}
Expand Down
8 changes: 4 additions & 4 deletions src/routes/safe/components/Settings/Advanced/RemoveGuardModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const RemoveGuardModal = ({ onClose, guardAddress }: RemoveGuardModalProp
manualGasLimit,
manualSafeNonce,
})
const isExecution = useCanTxExecute(false, manualSafeNonce)
const canTxExecute = useCanTxExecute(false, manualSafeNonce)

const [buttonStatus] = useEstimationStatus(txEstimationExecutionStatus)

Expand Down Expand Up @@ -123,7 +123,7 @@ export const RemoveGuardModal = ({ onClose, guardAddress }: RemoveGuardModalProp
>
<EditableTxParameters
isOffChainSignature={isOffChainSignature}
isExecution={isExecution}
isExecution={canTxExecute}
ethGasLimit={gasLimit}
ethGasPrice={gasPriceFormatted}
safeTxGas={gasEstimation}
Expand Down Expand Up @@ -158,15 +158,15 @@ export const RemoveGuardModal = ({ onClose, guardAddress }: RemoveGuardModalProp
txParameters={txParameters}
onEdit={toggleEditMode}
isTransactionCreation={isCreation}
isTransactionExecution={isExecution}
isTransactionExecution={canTxExecute}
isOffChainSignature={isOffChainSignature}
/>
</Block>
<Row className={classes.modalDescription}>
<ReviewInfoText
gasCostFormatted={gasCostFormatted}
isCreation={isCreation}
isExecution={isExecution}
isExecution={canTxExecute}
isOffChainSignature={isOffChainSignature}
safeNonce={txParameters.safeNonce}
txEstimationExecutionStatus={txEstimationExecutionStatus}
Expand Down
Loading