From 8b047e8c5721a30c3feddb72f4a85d0cf76fa0e5 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Date: Wed, 7 Apr 2021 11:50:02 +0200 Subject: [PATCH 1/4] [Feature] Use backend gas estimation (#2112) * Use safeTxGas backend estimation * Check transaction execution result and show warning if fails * Fix estimation for safes with just one required signature * Hide advanced parameters for sending funds with spending limit * Update variable names to be more clear --- src/logic/contracts/safeContracts.ts | 28 +- src/logic/hooks/useEstimateTransactionGas.tsx | 188 +++++------- .../safe/api/fetchSafeTxGasEstimation.ts | 25 ++ .../safe/store/actions/createTransaction.ts | 6 +- .../safe/store/actions/processTransaction.ts | 2 +- src/logic/safe/transactions/gas.ts | 279 +++++++++--------- src/logic/safe/utils/upgradeSafe.ts | 2 +- src/logic/wallets/ethTransactions.ts | 8 +- .../load/components/OwnerList/index.tsx | 2 +- .../ConfirmTxModal/ReviewConfirm.tsx | 6 + .../ContractInteraction/Review/index.tsx | 6 + .../screens/ReviewCollectible/index.tsx | 6 + .../screens/ReviewSendFundsTx/index.tsx | 26 +- .../Settings/Advanced/RemoveModuleModal.tsx | 6 + .../ManageOwners/AddOwnerModal/index.tsx | 2 +- .../AddOwnerModal/screens/Review/index.tsx | 10 +- .../ManageOwners/RemoveOwnerModal/index.tsx | 2 +- .../RemoveOwnerModal/screens/Review/index.tsx | 8 +- .../ManageOwners/ReplaceOwnerModal/index.tsx | 2 +- .../screens/Review/index.tsx | 8 +- .../SpendingLimit/NewLimitModal/Review.tsx | 6 + .../SpendingLimit/RemoveLimitModal.tsx | 6 + .../ChangeThreshold/index.tsx | 10 +- .../TxList/modals/ApproveTxModal.tsx | 8 +- .../hooks/useTransactionParameters.ts | 2 +- src/test/safe.dom.create.tsx | 2 +- 26 files changed, 356 insertions(+), 300 deletions(-) create mode 100644 src/logic/safe/api/fetchSafeTxGasEstimation.ts diff --git a/src/logic/contracts/safeContracts.ts b/src/logic/contracts/safeContracts.ts index 17345da0..5ba70c7c 100644 --- a/src/logic/contracts/safeContracts.ts +++ b/src/logic/contracts/safeContracts.ts @@ -52,17 +52,6 @@ const getProxyFactoryContract = (web3: Web3, networkId: ETHEREUM_NETWORK): Gnosi return (new web3.eth.Contract(ProxyFactorySol.abi as AbiItem[], contractAddress) as unknown) as GnosisSafeProxyFactory } -/** - * Creates a Contract instance of the GnosisSafeProxyFactory contract - */ -export const getSpendingLimitContract = () => { - const web3 = getWeb3() - return (new web3.eth.Contract( - SpendingLimitModule.abi as AbiItem[], - SPENDING_LIMIT_MODULE_ADDRESS, - ) as unknown) as AllowanceModule -} - export const getMasterCopyAddressFromProxyAddress = async (proxyAddress: string): Promise => { const res = await getSafeInfo(proxyAddress) const masterCopyAddress = (res as SafeInfo)?.masterCopy @@ -115,7 +104,7 @@ export const estimateGasForDeployingSafe = async ( userAccount: string, safeCreationSalt: number, ) => { - const gnosisSafeData = await safeMaster.methods + const gnosisSafeData = safeMaster.methods .setup( safeAccounts, numConfirmations, @@ -134,10 +123,23 @@ export const estimateGasForDeployingSafe = async ( data: proxyFactoryData, from: userAccount, to: proxyFactoryMaster.options.address, - }) + }).then(value => value * 2) } export const getGnosisSafeInstanceAt = (safeAddress: string): GnosisSafe => { const web3 = getWeb3() return (new web3.eth.Contract(GnosisSafeSol.abi as AbiItem[], safeAddress) as unknown) as GnosisSafe + } + +/** + * Creates a Contract instance of the SpendingLimitModule contract + */ + export const getSpendingLimitContract = () => { + const web3 = getWeb3() + + return (new web3.eth.Contract( + SpendingLimitModule.abi as AbiItem[], + SPENDING_LIMIT_MODULE_ADDRESS, + ) as unknown) as AllowanceModule +} \ No newline at end of file diff --git a/src/logic/hooks/useEstimateTransactionGas.tsx b/src/logic/hooks/useEstimateTransactionGas.tsx index 567d7659..6e9799ae 100644 --- a/src/logic/hooks/useEstimateTransactionGas.tsx +++ b/src/logic/hooks/useEstimateTransactionGas.tsx @@ -1,17 +1,16 @@ +import { List } from 'immutable' import { useEffect, useState } from 'react' +import { useSelector } from 'react-redux' +import { getNetworkInfo } from 'src/config' import { - estimateGasForTransactionApproval, - estimateGasForTransactionCreation, - estimateGasForTransactionExecution, - getFixedGasCosts, - SAFE_TX_GAS_DATA_COST, + checkTransactionExecution, + estimateSafeTxGas, + estimateTransactionGasLimit, } from 'src/logic/safe/transactions/gas' import { fromTokenUnit } from 'src/logic/tokens/utils/humanReadableValue' import { formatAmount } from 'src/logic/tokens/utils/formatAmount' import { calculateGasPrice } from 'src/logic/wallets/ethTransactions' -import { getNetworkInfo } from 'src/config' -import { useSelector } from 'react-redux' import { safeCurrentVersionSelector, safeParamAddressFromStateSelector, @@ -21,7 +20,6 @@ import { CALL } from 'src/logic/safe/transactions' import { web3ReadOnly as web3 } from 'src/logic/wallets/getWeb3' import { providerSelector } from 'src/logic/wallets/store/selectors' -import { List } from 'immutable' import { Confirmation } from 'src/logic/safe/store/models/types/confirmation' import { checkIfOffChainSignatureIsPossible } from 'src/logic/safe/safeTxSigner' import { ZERO_ADDRESS } from 'src/logic/wallets/ethAddresses' @@ -64,90 +62,16 @@ export const checkIfTxIsApproveAndExecution = ( return txConfirmations + 1 === threshold || sameString(txType, 'spendingLimit') } + if (threshold === 1) { + return true + } + return false } export const checkIfTxIsCreation = (txConfirmations: number, txType?: string): boolean => txConfirmations === 0 && !sameString(txType, 'spendingLimit') -type TransactionEstimationProps = { - txData: string - safeAddress: string - txRecipient: string - txConfirmations?: List - txAmount?: string - operation?: number - gasPrice?: string - gasToken?: string - refundReceiver?: string // Address of receiver of gas payment (or 0 if tx.origin). - safeTxGas?: number - from?: string - isExecution: boolean - isCreation: boolean - isOffChainSignature?: boolean - approvalAndExecution?: boolean -} - -const estimateTransactionGas = async ({ - txData, - safeAddress, - txRecipient, - txConfirmations, - txAmount, - operation, - gasPrice, - gasToken, - refundReceiver, - safeTxGas, - from, - isExecution, - isCreation, - isOffChainSignature = false, - approvalAndExecution, -}: TransactionEstimationProps): Promise => { - if (isCreation) { - return estimateGasForTransactionCreation( - safeAddress, - txData, - txRecipient, - txAmount || '0', - operation || CALL, - safeTxGas, - ) - } - - if (!from) { - throw new Error('No from provided for approving or execute transaction') - } - - if (isExecution) { - return estimateGasForTransactionExecution({ - safeAddress, - txRecipient, - txConfirmations, - txAmount: txAmount || '0', - txData, - operation: operation || CALL, - from, - gasPrice: gasPrice || '0', - gasToken: gasToken || ZERO_ADDRESS, - refundReceiver: refundReceiver || ZERO_ADDRESS, - safeTxGas: safeTxGas || 0, - approvalAndExecution, - }) - } - - return estimateGasForTransactionApproval({ - safeAddress, - operation: operation || CALL, - txData, - txAmount: txAmount || '0', - txRecipient, - from, - isOffChainSignature, - }) -} - type UseEstimateTransactionGasProps = { txData: string txRecipient: string @@ -158,6 +82,7 @@ type UseEstimateTransactionGasProps = { safeTxGas?: number txType?: string manualGasPrice?: string + manualGasLimit?: string } export type TransactionGasEstimationResult = { @@ -183,6 +108,7 @@ export const useEstimateTransactionGas = ({ safeTxGas, txType, manualGasPrice, + manualGasLimit, }: UseEstimateTransactionGasProps): TransactionGasEstimationResult => { const [gasEstimation, setGasEstimation] = useState({ txEstimationExecutionStatus: EstimationStatus.LOADING, @@ -208,51 +134,79 @@ export const useEstimateTransactionGas = ({ return } - const isExecution = checkIfTxIsExecution(Number(threshold), preApprovingOwner, txConfirmations?.size, txType) const isCreation = checkIfTxIsCreation(txConfirmations?.size || 0, txType) + const isExecution = checkIfTxIsExecution(Number(threshold), preApprovingOwner, txConfirmations?.size, txType) const approvalAndExecution = checkIfTxIsApproveAndExecution( Number(threshold), txConfirmations?.size || 0, txType, preApprovingOwner, ) - - const fixedGasCosts = getFixedGasCosts(Number(threshold)) const isOffChainSignature = checkIfOffChainSignatureIsPossible(isExecution, smartContractWallet, safeVersion) try { - const gasEstimation = await estimateTransactionGas({ - safeAddress, - txRecipient, - txData, - txAmount, - txConfirmations, - isExecution, - isCreation, - isOffChainSignature, - operation, - from, - safeTxGas, - approvalAndExecution, - }) + let safeTxGasEstimation = safeTxGas || 0 + let ethGasLimitEstimation = 0 + let transactionCallSuccess = true + let txEstimationExecutionStatus = EstimationStatus.LOADING + + if (isCreation) { + safeTxGasEstimation = await estimateSafeTxGas({ + safeAddress, + txData, + txRecipient, + txAmount: txAmount || '0', + operation: operation || CALL, + safeTxGas, + }) + } + if (isExecution || approvalAndExecution) { + ethGasLimitEstimation = await estimateTransactionGasLimit({ + safeAddress, + txRecipient, + txData, + txAmount: txAmount || '0', + txConfirmations, + isExecution, + isOffChainSignature, + operation: operation || CALL, + from, + safeTxGas: safeTxGasEstimation, + approvalAndExecution, + }) + } - const totalGasEstimation = (gasEstimation + fixedGasCosts) * 2 const gasPrice = manualGasPrice ? web3.utils.toWei(manualGasPrice, 'gwei') : await calculateGasPrice() const gasPriceFormatted = web3.utils.fromWei(gasPrice, 'gwei') - const estimatedGasCosts = totalGasEstimation * parseInt(gasPrice, 10) + const estimatedGasCosts = ethGasLimitEstimation * parseInt(gasPrice, 10) const gasCost = fromTokenUnit(estimatedGasCosts, nativeCoin.decimals) const gasCostFormatted = formatAmount(gasCost) - const gasLimit = totalGasEstimation.toString() + const gasLimit = manualGasLimit || ethGasLimitEstimation.toString() - let txEstimationExecutionStatus = EstimationStatus.SUCCESS - - if (gasEstimation <= 0) { - txEstimationExecutionStatus = isOffChainSignature ? EstimationStatus.SUCCESS : EstimationStatus.FAILURE + txEstimationExecutionStatus = EstimationStatus.SUCCESS + if (isExecution) { + transactionCallSuccess = await checkTransactionExecution({ + safeAddress, + txRecipient, + txData, + txAmount: txAmount || '0', + txConfirmations, + operation: operation || CALL, + from, + gasPrice: '0', + gasToken: ZERO_ADDRESS, + gasLimit, + refundReceiver: ZERO_ADDRESS, + safeTxGas: safeTxGasEstimation, + approvalAndExecution, + }) } + txEstimationExecutionStatus = transactionCallSuccess ? EstimationStatus.SUCCESS : EstimationStatus.FAILURE + setGasEstimation({ txEstimationExecutionStatus, - gasEstimation, + gasEstimation: safeTxGasEstimation, gasCost, gasCostFormatted, gasPrice, @@ -264,15 +218,12 @@ export const useEstimateTransactionGas = ({ }) } catch (error) { console.warn(error.message) - // We put a fixed the amount of gas to let the user try to execute the tx, but it's not accurate so it will probably fail - const gasEstimation = fixedGasCosts + SAFE_TX_GAS_DATA_COST - const gasCost = fromTokenUnit(gasEstimation, nativeCoin.decimals) - const gasCostFormatted = formatAmount(gasCost) + // If safeTxGas estimation fail we set this value to 0 (so up to all gasLimit can be used) setGasEstimation({ txEstimationExecutionStatus: EstimationStatus.FAILURE, - gasEstimation, - gasCost, - gasCostFormatted, + gasEstimation: 0, + gasCost: '0', + gasCostFormatted: '< 0.001', gasPrice: '1', gasPriceFormatted: '1', gasLimit: '0', @@ -301,6 +252,7 @@ export const useEstimateTransactionGas = ({ txType, providerName, manualGasPrice, + manualGasLimit, ]) return gasEstimation diff --git a/src/logic/safe/api/fetchSafeTxGasEstimation.ts b/src/logic/safe/api/fetchSafeTxGasEstimation.ts new file mode 100644 index 00000000..b0a4759e --- /dev/null +++ b/src/logic/safe/api/fetchSafeTxGasEstimation.ts @@ -0,0 +1,25 @@ +import axios from 'axios' + +import { getSafeServiceBaseUrl } from 'src/config' +import { checksumAddress } from 'src/utils/checksumAddress' + +export type GasEstimationResponse = { + safeTxGas: string +} + +type FetchSafeTxGasEstimationProps = { + safeAddress: string + to: string + value: string + data?: string + operation: number +} + +export const fetchSafeTxGasEstimation = async ({ + safeAddress, + ...body +}: FetchSafeTxGasEstimationProps): Promise => { + const url = `${getSafeServiceBaseUrl(checksumAddress(safeAddress))}/multisig-transactions/estimations/` + + return axios.post(url, body).then(({ data }) => data.safeTxGas) +} diff --git a/src/logic/safe/store/actions/createTransaction.ts b/src/logic/safe/store/actions/createTransaction.ts index b2e6b347..1b11c715 100644 --- a/src/logic/safe/store/actions/createTransaction.ts +++ b/src/logic/safe/store/actions/createTransaction.ts @@ -11,7 +11,7 @@ import { saveTxToHistory, tryOffchainSigning, } from 'src/logic/safe/transactions' -import { estimateGasForTransactionCreation } from 'src/logic/safe/transactions/gas' +import { estimateSafeTxGas } from 'src/logic/safe/transactions/gas' import * as aboutToExecuteTx from 'src/logic/safe/utils/aboutToExecuteTx' import { getCurrentSafeVersion } from 'src/logic/safe/utils/safeVersion' import { ZERO_ADDRESS } from 'src/logic/wallets/ethAddresses' @@ -78,7 +78,7 @@ export const createTransaction = ( if (!ready) return const { account: from, hardwareWallet, smartContractWallet } = providerSelector(state) - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) + const safeInstance = getGnosisSafeInstanceAt(safeAddress) const lastTx = await getLastTx(safeAddress) const nextNonce = await getNewTxNonce(lastTx, safeInstance) const nonce = txNonce !== undefined ? txNonce.toString() : nextNonce @@ -88,7 +88,7 @@ export const createTransaction = ( let safeTxGas = safeTxGasArg || 0 try { if (safeTxGasArg === undefined) { - safeTxGas = await estimateGasForTransactionCreation(safeAddress, txData, to, valueInWei, operation) + safeTxGas = await estimateSafeTxGas({ safeAddress, txData, txRecipient: to, txAmount: valueInWei, operation }) } } catch (error) { safeTxGas = safeTxGasArg || 0 diff --git a/src/logic/safe/store/actions/processTransaction.ts b/src/logic/safe/store/actions/processTransaction.ts index 28934b23..709805ef 100644 --- a/src/logic/safe/store/actions/processTransaction.ts +++ b/src/logic/safe/store/actions/processTransaction.ts @@ -73,7 +73,7 @@ export const processTransaction = ({ const state = getState() const { account: from, hardwareWallet, smartContractWallet } = providerSelector(state) - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) + const safeInstance = getGnosisSafeInstanceAt(safeAddress) const lastTx = await getLastTx(safeAddress) const nonce = await getNewTxNonce(lastTx, safeInstance) diff --git a/src/logic/safe/transactions/gas.ts b/src/logic/safe/transactions/gas.ts index 9c79dd1d..94773b2b 100644 --- a/src/logic/safe/transactions/gas.ts +++ b/src/logic/safe/transactions/gas.ts @@ -1,42 +1,18 @@ +import axios from 'axios' import { BigNumber } from 'bignumber.js' +import { List } from 'immutable' +import { getRpcServiceUrl, usesInfuraRPC } from 'src/config' import { getGnosisSafeInstanceAt } from 'src/logic/contracts/safeContracts' import { calculateGasOf, EMPTY_DATA } from 'src/logic/wallets/ethTransactions' import { getWeb3, web3ReadOnly } from 'src/logic/wallets/getWeb3' import { ZERO_ADDRESS } from 'src/logic/wallets/ethAddresses' import { generateSignaturesFromTxConfirmations } from 'src/logic/safe/safeTxSigner' -import { List } from 'immutable' +import { fetchSafeTxGasEstimation } from 'src/logic/safe/api/fetchSafeTxGasEstimation' import { Confirmation } from 'src/logic/safe/store/models/types/confirmation' -import axios from 'axios' -import { getRpcServiceUrl, usesInfuraRPC } from 'src/config' +import { checksumAddress } from 'src/utils/checksumAddress' import { sameString } from 'src/utils/strings' -// 21000 - additional gas costs (e.g. base tx costs, transfer costs) -export const MINIMUM_TRANSACTION_GAS = 21000 -// Estimation of gas required for each signature (aproximately 7800, roundup to 8000) -export const GAS_REQUIRED_PER_SIGNATURE = 8000 -// We require some gas to emit the events (at least 2500) after the execution and some to perform code until the execution (500) -// We also add 3k pay when processing safeTxGas value. We don't know this value when creating the transaction -// Hex values different than 0 has some gas cost -export const SAFE_TX_GAS_DATA_COST = 6000 - -// Receives the response data of the safe method requiredTxGas() and parses it to get the gas amount -const parseRequiredTxGasResponse = (data: string): number => { - const reducer = (accumulator, currentValue) => { - if (currentValue === EMPTY_DATA) { - return accumulator + 0 - } - - if (currentValue === '00') { - return accumulator + 4 - } - - return accumulator + 16 - } - - return data.match(/.{2}/g)?.reduce(reducer, 0) -} - interface ErrorDataJson extends JSON { originalError?: { data?: string @@ -178,94 +154,113 @@ export const getGasEstimationTxResponse = async (txConfig: { return estimateGasWithWeb3Provider(txConfig) } -const calculateMinimumGasForTransaction = async ( - additionalGasBatches: number[], - safeAddress: string, - estimateData: string, - safeTxGasEstimation: number, - fixedGasCosts: number, -): Promise => { - for (const additionalGas of additionalGasBatches) { - const batchedSafeTxGas = safeTxGasEstimation + additionalGas - // To simulate if safeTxGas is enough we need to send an estimated gasLimit that will be the sum - // of the safeTxGasEstimation and fixedGas costs for ethereum transaction - const gasLimit = batchedSafeTxGas + fixedGasCosts - console.info(`Estimating safeTxGas with gas amount: ${batchedSafeTxGas}`) - try { - const estimation = await getGasEstimationTxResponse({ - to: safeAddress, - from: safeAddress, - data: estimateData, - gasPrice: 0, - gas: gasLimit, - }) - if (estimation > 0) { - console.info(`Gas estimation successfully finished with gas amount: ${batchedSafeTxGas}`) - return batchedSafeTxGas - } - } catch (error) { - console.log(`Error trying to estimate gas with amount: ${batchedSafeTxGas}`) - } - } - - return 0 +type SafeTxGasEstimationProps = { + safeAddress: string + txData: string + txRecipient: string + txAmount: string + operation: number + safeTxGas?: number } -export const getFixedGasCosts = (threshold: number): number => { - // There are some minimum gas costs to execute an Ethereum transaction - // We add this fixed network minimum gas, the gas required to check each signature - return MINIMUM_TRANSACTION_GAS + (threshold || 1) * GAS_REQUIRED_PER_SIGNATURE -} - -export const estimateGasForTransactionCreation = async ( - safeAddress: string, - data: string, - to: string, - valueInWei: string, - operation: number, - safeTxGas?: number, -): Promise => { +export const estimateSafeTxGas = async ({ + safeAddress, + txData, + txRecipient, + txAmount, + operation, + safeTxGas, +}: SafeTxGasEstimationProps): Promise => { try { - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) - - const estimateData = safeInstance.methods.requiredTxGas(to, valueInWei, data, operation).encodeABI() - const threshold = await safeInstance.methods.getThreshold().call() - - const fixedGasCosts = getFixedGasCosts(Number(threshold)) - - const gasEstimationResponse = await getGasEstimationTxResponse({ - to: safeAddress, - from: safeAddress, - data: estimateData, - gas: safeTxGas ? safeTxGas + fixedGasCosts : undefined, + const safeTxGasEstimation = await fetchSafeTxGasEstimation({ + safeAddress, + to: checksumAddress(txRecipient), + value: txAmount, + data: txData, + operation, }) + console.log('Backend gas estimation', safeTxGasEstimation) + if (safeTxGas) { - // When we execute we get a more precise estimate value, we log for debug purposes - console.info('This is the smart contract minimum expected safeTxGas', gasEstimationResponse) + // If safeTxGas was already defined we leave it but log our estimation for debug purposes + console.info('This is the smart contract minimum expected safeTxGas', safeTxGasEstimation) // We return set safeTxGas return safeTxGas } - const dataGasEstimation = parseRequiredTxGasResponse(estimateData) - // Adding this values we should get the full safeTxGas value - const safeTxGasEstimation = gasEstimationResponse + dataGasEstimation + SAFE_TX_GAS_DATA_COST - // We will add gas batches in case is not enough - const additionalGasBatches = [0, 10000, 20000, 40000, 80000, 160000, 320000, 640000, 1280000, 2560000, 5120000] - - return await calculateMinimumGasForTransaction( - additionalGasBatches, - safeAddress, - estimateData, - safeTxGasEstimation, - fixedGasCosts, - ) + return parseInt(safeTxGasEstimation) } catch (error) { console.info('Error calculating tx gas estimation', error.message) throw error } } +type TransactionEstimationProps = { + txData: string + safeAddress: string + txRecipient: string + txConfirmations?: List + txAmount: string + operation: number + gasPrice?: string + gasToken?: string + refundReceiver?: string // Address of receiver of gas payment (or 0 if tx.origin). + safeTxGas?: number + from?: string + isExecution: boolean + isOffChainSignature?: boolean + approvalAndExecution?: boolean +} + +export const estimateTransactionGasLimit = async ({ + txData, + safeAddress, + txRecipient, + txConfirmations, + txAmount, + operation, + gasPrice, + gasToken, + refundReceiver, + safeTxGas, + from, + isExecution, + isOffChainSignature = false, + approvalAndExecution, +}: TransactionEstimationProps): Promise => { + if (!from) { + throw new Error('No from provided for approving or execute transaction') + } + + if (isExecution) { + return estimateGasForTransactionExecution({ + safeAddress, + txRecipient, + txConfirmations, + txAmount, + txData, + operation, + from, + gasPrice: gasPrice || '0', + gasToken: gasToken || ZERO_ADDRESS, + refundReceiver: refundReceiver || ZERO_ADDRESS, + safeTxGas: safeTxGas || 0, + approvalAndExecution, + }) + } + + return estimateGasForTransactionApproval({ + safeAddress, + operation, + txData, + txAmount, + txRecipient, + from, + isOffChainSignature, + }) +} + type TransactionExecutionEstimationProps = { txData: string safeAddress: string @@ -275,65 +270,75 @@ type TransactionExecutionEstimationProps = { operation: number gasPrice: string gasToken: string + gasLimit?: string refundReceiver: string // Address of receiver of gas payment (or 0 if tx.origin). safeTxGas: number from: string approvalAndExecution?: boolean } -export const estimateGasForTransactionExecution = async ({ +const estimateGasForTransactionExecution = async ({ safeAddress, txRecipient, txConfirmations, txAmount, txData, operation, + from, gasPrice, gasToken, refundReceiver, safeTxGas, approvalAndExecution, }: TransactionExecutionEstimationProps): Promise => { - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) - try { - let gasEstimation - // If safeTxGas === 0 we still have to estimate the gas limit to execute the transaction so we need to get an estimation - if (approvalAndExecution || safeTxGas === 0) { - console.info(`Estimating transaction necessary gas...`) - // @todo (agustin) once we solve the problem with the preApprovingOwner, we need to use the method bellow (execTransaction) with sigs = generateSignaturesFromTxConfirmations(txConfirmations,from) - gasEstimation = await estimateGasForTransactionCreation( - safeAddress, - txData, - txRecipient, - txAmount, - operation, - safeTxGas, - ) + const safeInstance = getGnosisSafeInstanceAt(safeAddress) + // If it's approvalAndExecution we have to add a preapproved signature else we have all signatures + const sigs = generateSignaturesFromTxConfirmations(txConfirmations, approvalAndExecution ? from : undefined) - if (approvalAndExecution) { - // If it's approve and execute we don't have all the signatures to do a complete simulation, we return the gas estimation - console.info(`Gas estimation successfully finished with gas amount: ${gasEstimation}`) - return gasEstimation - } - } - // If we have all signatures we can do a call to ensure the transaction will be successful or fail - const sigs = generateSignaturesFromTxConfirmations(txConfirmations) - console.info(`Check transaction success with gas amount: ${safeTxGas}...`) - await safeInstance.methods - .execTransaction(txRecipient, txAmount, txData, operation, safeTxGas, 0, gasPrice, gasToken, refundReceiver, sigs) - .call() - console.info(`Gas estimation successfully finished with gas amount: ${safeTxGas}`) - return safeTxGas || gasEstimation - } catch (error) { - throw new Error(`Gas estimation failed with gas amount: ${safeTxGas}`) - } + const estimationData = safeInstance.methods + .execTransaction(txRecipient, txAmount, txData, operation, safeTxGas, 0, gasPrice, gasToken, refundReceiver, sigs) + .encodeABI() + + return calculateGasOf({ + data: estimationData, + from, + to: safeAddress, + }) +} + +export const checkTransactionExecution = async ({ + safeAddress, + txRecipient, + txConfirmations, + txAmount, + txData, + operation, + from, + gasPrice, + gasToken, + gasLimit, + refundReceiver, + safeTxGas, + approvalAndExecution, +}: TransactionExecutionEstimationProps): Promise => { + const safeInstance = getGnosisSafeInstanceAt(safeAddress) + // If it's approvalAndExecution we have to add a preapproved signature else we have all signatures + const sigs = generateSignaturesFromTxConfirmations(txConfirmations, approvalAndExecution ? from : undefined) + + return safeInstance.methods + .execTransaction(txRecipient, txAmount, txData, operation, safeTxGas, 0, gasPrice, gasToken, refundReceiver, sigs) + .call({ + from, + gas: gasLimit, + }) + .catch(() => false) } type TransactionApprovalEstimationProps = { - txData: string safeAddress: string txRecipient: string txAmount: string + txData: string operation: number from: string isOffChainSignature: boolean @@ -352,7 +357,7 @@ export const estimateGasForTransactionApproval = async ({ return 0 } - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) + const safeInstance = getGnosisSafeInstanceAt(safeAddress) const nonce = await safeInstance.methods.nonce().call() const txHash = await safeInstance.methods @@ -360,7 +365,7 @@ export const estimateGasForTransactionApproval = async ({ .call({ from, }) - const approveTransactionTxData = await safeInstance.methods.approveHash(txHash).encodeABI() + const approveTransactionTxData = safeInstance.methods.approveHash(txHash).encodeABI() return calculateGasOf({ data: approveTransactionTxData, from, diff --git a/src/logic/safe/utils/upgradeSafe.ts b/src/logic/safe/utils/upgradeSafe.ts index 53caa747..3bbe6336 100644 --- a/src/logic/safe/utils/upgradeSafe.ts +++ b/src/logic/safe/utils/upgradeSafe.ts @@ -49,7 +49,7 @@ export const getEncodedMultiSendCallData = (txs: MultiSendTx[], web3: Web3): str } export const getUpgradeSafeTransactionHash = async (safeAddress: string): Promise => { - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) + const safeInstance = getGnosisSafeInstanceAt(safeAddress) const fallbackHandlerTxData = safeInstance.methods.setFallbackHandler(DEFAULT_FALLBACK_HANDLER_ADDRESS).encodeABI() const updateSafeTxData = safeInstance.methods.changeMasterCopy(SAFE_MASTER_COPY_ADDRESS).encodeABI() const txs = [ diff --git a/src/logic/wallets/ethTransactions.ts b/src/logic/wallets/ethTransactions.ts index bfedd1a0..bf1fd7b3 100644 --- a/src/logic/wallets/ethTransactions.ts +++ b/src/logic/wallets/ethTransactions.ts @@ -6,7 +6,7 @@ import { getGasPrice, getGasPriceOracle } from 'src/config' export const EMPTY_DATA = '0x' -export const checkReceiptStatus = async (hash) => { +export const checkReceiptStatus = async (hash: string): Promise => { if (!hash) { return Promise.reject(new Error('No valid Tx hash to get receipt from')) } @@ -27,10 +27,6 @@ export const checkReceiptStatus = async (hash) => { } export const calculateGasPrice = async (): Promise => { - if (process.env.NODE_ENV === 'test') { - return '20000000000' - } - const gasPrice = getGasPrice() const gasPriceOracle = getGasPriceOracle() @@ -61,7 +57,7 @@ export const calculateGasOf = async (txConfig: { try { const gas = await web3.eth.estimateGas(txConfig) - return gas * 2 + return gas } catch (err) { return Promise.reject(err) } diff --git a/src/routes/load/components/OwnerList/index.tsx b/src/routes/load/components/OwnerList/index.tsx index d6f6fc74..7888d8d5 100644 --- a/src/routes/load/components/OwnerList/index.tsx +++ b/src/routes/load/components/OwnerList/index.tsx @@ -55,7 +55,7 @@ const OwnerListComponent = (props) => { const fetchSafe = async () => { const safeAddress = values[FIELD_LOAD_ADDRESS] - const gnosisSafe = await getGnosisSafeInstanceAt(safeAddress) + const gnosisSafe = getGnosisSafeInstanceAt(safeAddress) const safeOwners = await gnosisSafe.methods.getOwners().call() const threshold = await gnosisSafe.methods.getThreshold().call() diff --git a/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx b/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx index 04e70266..016d165d 100644 --- a/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx +++ b/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx @@ -112,6 +112,7 @@ export const ReviewConfirm = ({ const operation = useMemo(() => (isMultiSend ? DELEGATE_CALL : CALL), [isMultiSend]) const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const { gasLimit, @@ -129,6 +130,7 @@ export const ReviewConfirm = ({ txAmount: txValue, safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) useEffect(() => { @@ -191,6 +193,10 @@ export const ReviewConfirm = ({ setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } 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 f0fb4546..f02e60f2 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 @@ -58,6 +58,7 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE const safeAddress = useSelector(safeParamAddressFromStateSelector) const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const [txInfo, setTxInfo] = useState<{ txRecipient: string @@ -80,6 +81,7 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE txData: txInfo?.txData, safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) useEffect(() => { @@ -120,6 +122,10 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } 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 ea68ea2a..16e3abc5 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx @@ -56,6 +56,7 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement = const nftTokens = useSelector(nftTokensSelector) const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const txToken = nftTokens.find( ({ assetAddress, tokenId }) => assetAddress === tx.assetAddress && tokenId === tx.nftTokenId, @@ -76,6 +77,7 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement = txRecipient: tx.assetAddress, safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) useEffect(() => { @@ -133,6 +135,10 @@ const ReviewCollectible = ({ onClose, onPrev, tx }: Props): React.ReactElement = setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } 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 1e2ab83c..aee98644 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/ReviewSendFundsTx/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/screens/ReviewSendFundsTx/index.tsx @@ -100,6 +100,7 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE const data = useTxData(isSendingNativeToken, tx.amount, tx.recipientAddress, txToken) const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const { gasCostFormatted, @@ -117,6 +118,7 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE txAmount: txValue, safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) const submitTx = async (txParameters: TxParameters) => { @@ -171,6 +173,10 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } @@ -257,17 +263,21 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE {/* Tx Parameters */} - + {/* FIXME TxParameters should be updated to be used with spending limits */} + {!sameString(tx.txType, 'spendingLimit') && ( + + )} {/* Disclaimer */} - {txEstimationExecutionStatus !== EstimationStatus.LOADING && ( + {/* FIXME Estimation should be fixed to be used with spending limits */} + {!sameString(tx.txType, 'spendingLimit') && txEstimationExecutionStatus !== EstimationStatus.LOADING && (
() + const [manualGasLimit, setManualGasLimit] = useState() const [, moduleAddress] = selectedModulePair const explorerInfo = getExplorerInfo(moduleAddress) @@ -77,6 +78,7 @@ export const RemoveModuleModal = ({ onClose, selectedModulePair }: RemoveModuleM txAmount: '0', safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) useEffect(() => { @@ -113,6 +115,10 @@ export const RemoveModuleModal = ({ onClose, selectedModulePair }: RemoveModuleM setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } diff --git a/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/index.tsx b/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/index.tsx index e376e73b..26d3e878 100644 --- a/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/index.tsx +++ b/src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/index.tsx @@ -29,7 +29,7 @@ export const sendAddOwner = async ( txParameters: TxParameters, dispatch: Dispatch, ): Promise => { - const gnosisSafe = await getGnosisSafeInstanceAt(safeAddress) + const gnosisSafe = getGnosisSafeInstanceAt(safeAddress) const txData = gnosisSafe.methods.addOwnerWithThreshold(values.ownerAddress, values.threshold).encodeABI() const txHash = await dispatch( 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 1acbd6ef..c606396f 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 @@ -45,6 +45,7 @@ export const ReviewAddOwner = ({ onClickBack, onClose, onSubmit, values }: Revie const owners = useSelector(safeOwnersSelector) const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const { gasLimit, @@ -60,14 +61,15 @@ export const ReviewAddOwner = ({ onClickBack, onClose, onSubmit, values }: Revie txRecipient: safeAddress, safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) useEffect(() => { let isCurrent = true - const calculateAddOwnerData = async () => { + const calculateAddOwnerData = () => { try { - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) + const safeInstance = getGnosisSafeInstanceAt(safeAddress) const txData = safeInstance.methods.addOwnerWithThreshold(values.ownerAddress, values.threshold).encodeABI() if (isCurrent) { @@ -94,6 +96,10 @@ export const ReviewAddOwner = ({ onClickBack, onClose, onSubmit, values }: Revie setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } diff --git a/src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/index.tsx b/src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/index.tsx index add0309d..4f89c537 100644 --- a/src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/index.tsx +++ b/src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/index.tsx @@ -29,7 +29,7 @@ export const sendRemoveOwner = async ( txParameters: TxParameters, threshold?: number, ): Promise => { - const gnosisSafe = await getGnosisSafeInstanceAt(safeAddress) + const gnosisSafe = getGnosisSafeInstanceAt(safeAddress) const safeOwners = await gnosisSafe.methods.getOwners().call() const index = safeOwners.findIndex( (ownerAddress) => ownerAddress.toLowerCase() === ownerAddressToRemove.toLowerCase(), 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 7d77084c..268f5729 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 @@ -59,6 +59,7 @@ export const ReviewRemoveOwnerModal = ({ const ownersWithAddressBookName = owners ? getOwnersWithNameFromAddressBook(addressBook, owners) : List([]) const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const { gasLimit, @@ -74,6 +75,7 @@ export const ReviewRemoveOwnerModal = ({ txRecipient: safeAddress, safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) useEffect(() => { @@ -86,7 +88,7 @@ export const ReviewRemoveOwnerModal = ({ const calculateRemoveOwnerData = async () => { try { - const gnosisSafe = await getGnosisSafeInstanceAt(safeAddress) + const gnosisSafe = getGnosisSafeInstanceAt(safeAddress) const safeOwners = await gnosisSafe.methods.getOwners().call() const index = safeOwners.findIndex((owner) => sameAddress(owner, ownerAddress)) const prevAddress = index === 0 ? SENTINEL_ADDRESS : safeOwners[index - 1] @@ -116,6 +118,10 @@ export const ReviewRemoveOwnerModal = ({ setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } diff --git a/src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/index.tsx b/src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/index.tsx index 0ef75508..0f4cc8ac 100644 --- a/src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/index.tsx +++ b/src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/index.tsx @@ -30,7 +30,7 @@ export const sendReplaceOwner = async ( txParameters: TxParameters, threshold?: number, ): Promise => { - const gnosisSafe = await getGnosisSafeInstanceAt(safeAddress) + const gnosisSafe = getGnosisSafeInstanceAt(safeAddress) const safeOwners = await gnosisSafe.methods.getOwners().call() const index = safeOwners.findIndex((ownerAddress) => sameAddress(ownerAddress, ownerAddressToRemove)) const prevAddress = index === 0 ? SENTINEL_ADDRESS : safeOwners[index - 1] 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 52327599..2c45a342 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 @@ -67,6 +67,7 @@ export const ReviewReplaceOwnerModal = ({ const ownersWithAddressBookName = owners ? getOwnersWithNameFromAddressBook(addressBook, owners) : List([]) const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const { gasLimit, @@ -82,12 +83,13 @@ export const ReviewReplaceOwnerModal = ({ txRecipient: safeAddress, safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) useEffect(() => { let isCurrent = true const calculateReplaceOwnerData = async () => { - const gnosisSafe = await getGnosisSafeInstanceAt(safeAddress) + const gnosisSafe = getGnosisSafeInstanceAt(safeAddress) const safeOwners = await gnosisSafe.methods.getOwners().call() const index = safeOwners.findIndex((owner) => owner.toLowerCase() === ownerAddress.toLowerCase()) const prevAddress = index === 0 ? SENTINEL_ADDRESS : safeOwners[index - 1] @@ -113,6 +115,10 @@ export const ReviewReplaceOwnerModal = ({ setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } diff --git a/src/routes/safe/components/Settings/SpendingLimit/NewLimitModal/Review.tsx b/src/routes/safe/components/Settings/SpendingLimit/NewLimitModal/Review.tsx index ff4feb57..8330c121 100644 --- a/src/routes/safe/components/Settings/SpendingLimit/NewLimitModal/Review.tsx +++ b/src/routes/safe/components/Settings/SpendingLimit/NewLimitModal/Review.tsx @@ -154,6 +154,7 @@ export const ReviewSpendingLimits = ({ onBack, onClose, txToken, values }: Revie }) const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const { gasCostFormatted, @@ -171,6 +172,7 @@ export const ReviewSpendingLimits = ({ onBack, onClose, txToken, values }: Revie operation: estimateGasArgs.operation, safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) useEffect(() => { @@ -225,6 +227,10 @@ export const ReviewSpendingLimits = ({ onBack, onClose, txToken, values }: Revie setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } diff --git a/src/routes/safe/components/Settings/SpendingLimit/RemoveLimitModal.tsx b/src/routes/safe/components/Settings/SpendingLimit/RemoveLimitModal.tsx index 7e9002c6..89b9ef5a 100644 --- a/src/routes/safe/components/Settings/SpendingLimit/RemoveLimitModal.tsx +++ b/src/routes/safe/components/Settings/SpendingLimit/RemoveLimitModal.tsx @@ -39,6 +39,7 @@ export const RemoveLimitModal = ({ onClose, spendingLimit, open }: RemoveSpendin const dispatch = useDispatch() const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() useEffect(() => { const { @@ -64,6 +65,7 @@ export const RemoveLimitModal = ({ onClose, spendingLimit, open }: RemoveSpendin txAmount: '0', safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) const removeSelectedSpendingLimit = async (txParameters: TxParameters): Promise => { @@ -101,6 +103,10 @@ export const RemoveLimitModal = ({ onClose, spendingLimit, open }: RemoveSpendin setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } diff --git a/src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx b/src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx index 0560b57a..ff2c2785 100644 --- a/src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx +++ b/src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx @@ -50,6 +50,7 @@ export const ChangeThresholdModal = ({ const [data, setData] = useState('') const [manualSafeTxGas, setManualSafeTxGas] = useState(0) const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const [editedThreshold, setEditedThreshold] = useState(threshold) const { @@ -66,12 +67,13 @@ export const ChangeThresholdModal = ({ txRecipient: safeAddress, safeTxGas: manualSafeTxGas, manualGasPrice, + manualGasLimit, }) useEffect(() => { let isCurrent = true - const calculateChangeThresholdData = async () => { - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) + const calculateChangeThresholdData = () => { + const safeInstance = getGnosisSafeInstanceAt(safeAddress) const txData = safeInstance.methods.changeThreshold(editedThreshold).encodeABI() if (isCurrent) { setData(txData) @@ -111,6 +113,10 @@ export const ChangeThresholdModal = ({ setManualGasPrice(txParameters.ethGasPrice) } + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit) + } + if (newSafeTxGas && oldSafeTxGas !== newSafeTxGas) { setManualSafeTxGas(newSafeTxGas) } diff --git a/src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx b/src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx index c8b24649..9816a7cd 100644 --- a/src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx +++ b/src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx @@ -229,6 +229,7 @@ export const ApproveTxModal = ({ const oneConfirmationLeft = !thresholdReached && _countingCurrentConfirmation === _threshold const isTheTxReadyToBeExecuted = oneConfirmationLeft ? true : thresholdReached const [manualGasPrice, setManualGasPrice] = useState() + const [manualGasLimit, setManualGasLimit] = useState() const { confirmations, data, @@ -262,7 +263,8 @@ export const ApproveTxModal = ({ preApprovingOwner: approveAndExecute ? userAddress : undefined, safeTxGas, operation, - manualGasPrice: manualGasPrice, + manualGasPrice, + manualGasLimit, }) const handleExecuteCheckbox = () => setApproveAndExecute((prevApproveAndExecute) => !prevApproveAndExecute) @@ -312,6 +314,10 @@ export const ApproveTxModal = ({ if (newGasPrice && oldGasPrice !== newGasPrice) { setManualGasPrice(newGasPrice.toString()) } + + if (txParameters.ethGasLimit && gasLimit !== txParameters.ethGasLimit) { + setManualGasLimit(txParameters.ethGasLimit.toString()) + } } return ( diff --git a/src/routes/safe/container/hooks/useTransactionParameters.ts b/src/routes/safe/container/hooks/useTransactionParameters.ts index 1185c765..fb7215d9 100644 --- a/src/routes/safe/container/hooks/useTransactionParameters.ts +++ b/src/routes/safe/container/hooks/useTransactionParameters.ts @@ -81,7 +81,7 @@ export const useTransactionParameters = (props?: Props): TxParameters => { useEffect(() => { const getSafeNonce = async () => { if (safeAddress) { - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) + const safeInstance = getGnosisSafeInstanceAt(safeAddress) const lastTx = await getLastTx(safeAddress) const nonce = await getNewTxNonce(lastTx, safeInstance) setSafeNonce(nonce) diff --git a/src/test/safe.dom.create.tsx b/src/test/safe.dom.create.tsx index d686009f..7c9d8475 100644 --- a/src/test/safe.dom.create.tsx +++ b/src/test/safe.dom.create.tsx @@ -130,7 +130,7 @@ describe('DOM > Feature > CREATE a Safe', () => { expect(address).not.toBe(null) expect(address).not.toBe(undefined) - const gnosisSafe = await getGnosisSafeInstanceAt(address) + const gnosisSafe = getGnosisSafeInstanceAt(address) const storedOwners = await gnosisSafe.methods.getOwners().call() expect(storedOwners.length).toEqual(4) const safeThreshold = await gnosisSafe.methods.getThreshold().call() From e48891ba2bb32a2ef7f83a550d5aceff93d43059 Mon Sep 17 00:00:00 2001 From: Mikhail Mikheev Date: Wed, 7 Apr 2021 14:21:30 +0400 Subject: [PATCH 2/4] Add Aave 2 safe app (#2127) --- src/routes/safe/components/Apps/utils.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/routes/safe/components/Apps/utils.ts b/src/routes/safe/components/Apps/utils.ts index fa66d807..46cbe08c 100644 --- a/src/routes/safe/components/Apps/utils.ts +++ b/src/routes/safe/components/Apps/utils.ts @@ -34,6 +34,12 @@ export const staticAppsList: Array = [ disabled: false, networks: [ETHEREUM_NETWORK.MAINNET], }, + // Aave v2 + { + url: `${process.env.REACT_APP_IPFS_GATEWAY}/QmVg7aXr5S8sT2iUdUwdkfTJNknmB7rcE3t92HiGoVsYDj`, + disabled: false, + networks: [ETHEREUM_NETWORK.MAINNET], + }, //Balancer Exchange { url: `${process.env.REACT_APP_IPFS_GATEWAY}/QmRb2VfPVYBrv6gi2zDywgVgTg3A19ZCRMqwL13Ez5f5AS`, From c96e3192ff4b36715298e7b6a2d66e3fe1f3d6c1 Mon Sep 17 00:00:00 2001 From: Fernando Date: Wed, 7 Apr 2021 07:40:33 -0300 Subject: [PATCH 3/4] (Feature) Proper rejections (#2096) * remove `isCancelTransaction` utility function in favor of `txInfo.isCancellation` flag provided by client-gateway * replace "cancel" concept in favor of "reject" * add circle-cross-red icon to "On-chain rejection" transaction info Adjust owner's list text color * identify queued on-chain rejection * apply styles to on-chain rejection type identifier * update awaiting messages wording * fix styles on styles to on-chain rejection * replace local svg with SRC `Icon` component wherever is possible --- package.json | 2 +- src/components/CustomIconText/index.tsx | 6 +- src/components/TransactionFailText/index.tsx | 2 +- .../safe/store/models/types/gateway.d.ts | 2 +- .../Transactions/TxList/TxCollapsed.tsx | 7 ++- .../TxList/TxCollapsedActions.tsx | 2 +- .../Transactions/TxList/TxDetails.tsx | 58 +++++++++++-------- .../Transactions/TxList/TxExpandedActions.tsx | 2 +- .../Transactions/TxList/TxOwners.tsx | 56 +++++++++++------- .../Transactions/TxList/TxSummary.tsx | 2 +- .../TxList/assets/circle-cross-red.svg | 11 ++++ .../TxList/hooks/useTransactionActions.ts | 10 +--- .../TxList/hooks/useTransactionStatus.ts | 4 +- .../TxList/hooks/useTransactionType.ts | 18 +----- .../components/Transactions/TxList/styled.tsx | 30 ++++++++++ .../components/Transactions/TxList/utils.ts | 24 +------- yarn.lock | 4 +- 17 files changed, 134 insertions(+), 106 deletions(-) create mode 100644 src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg diff --git a/package.json b/package.json index c8f82367..82ed3cfe 100644 --- a/package.json +++ b/package.json @@ -161,7 +161,7 @@ "@gnosis.pm/safe-apps-sdk": "1.0.3", "@gnosis.pm/safe-apps-sdk-v1": "npm:@gnosis.pm/safe-apps-sdk@0.4.2", "@gnosis.pm/safe-contracts": "1.1.1-dev.2", - "@gnosis.pm/safe-react-components": "https://github.com/gnosis/safe-react-components.git#a68a67e", + "@gnosis.pm/safe-react-components": "https://github.com/gnosis/safe-react-components.git#7ebc414", "@gnosis.pm/util-contracts": "2.0.6", "@ledgerhq/hw-transport-node-hid-singleton": "5.45.0", "@material-ui/core": "^4.11.0", diff --git a/src/components/CustomIconText/index.tsx b/src/components/CustomIconText/index.tsx index 31fc13ba..cc059de1 100644 --- a/src/components/CustomIconText/index.tsx +++ b/src/components/CustomIconText/index.tsx @@ -1,5 +1,5 @@ import { Text } from '@gnosis.pm/safe-react-components' -import React from 'react' +import React, { ReactElement } from 'react' import styled from 'styled-components' const Wrapper = styled.div` @@ -12,11 +12,9 @@ const Icon = styled.img` margin-right: 9px; ` -const CustomIconText = ({ iconUrl, text }: { iconUrl: string; text?: string }) => ( +export const CustomIconText = ({ iconUrl, text }: { iconUrl: string; text?: string }): ReactElement => ( {text && {text}} ) - -export default CustomIconText diff --git a/src/components/TransactionFailText/index.tsx b/src/components/TransactionFailText/index.tsx index d61664cc..25c6b56b 100644 --- a/src/components/TransactionFailText/index.tsx +++ b/src/components/TransactionFailText/index.tsx @@ -41,7 +41,7 @@ export const TransactionFailText = ({ if (isExecution) { errorMessage = threshold && threshold > 1 - ? `To save gas costs, cancel this transaction` + ? `To save gas costs, reject this transaction` : `To save gas costs, avoid executing the transaction.` } diff --git a/src/logic/safe/store/models/types/gateway.d.ts b/src/logic/safe/store/models/types/gateway.d.ts index c462971a..cf45553d 100644 --- a/src/logic/safe/store/models/types/gateway.d.ts +++ b/src/logic/safe/store/models/types/gateway.d.ts @@ -236,7 +236,7 @@ type MultiSigExecutionDetails = { type DetailedExecutionInfo = ModuleExecutionDetails | MultiSigExecutionDetails type ExpandedTxDetails = { - executedAt: number + executedAt: number | null txStatus: TransactionStatus txInfo: TransactionInfo txData: TransactionData | null diff --git a/src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx b/src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx index 4223be90..4e12f101 100644 --- a/src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx @@ -4,7 +4,7 @@ import CircularProgress from '@material-ui/core/CircularProgress' import React, { ReactElement, useContext, useRef } from 'react' import styled from 'styled-components' -import CustomIconText from 'src/components/CustomIconText' +import { CustomIconText } from 'src/components/CustomIconText' import { isCustomTxInfo, isMultiSendTxInfo, @@ -24,6 +24,7 @@ import { TokenTransferAmount } from './TokenTransferAmount' import { TxsInfiniteScrollContext } from './TxsInfiniteScroll' import { TxLocationContext } from './TxLocationProvider' import { CalculatedVotes } from './TxQueueCollapsed' +import { isCancelTxDetails } from './utils' const TxInfo = ({ info }: { info: AssetInfo }) => { if (isTokenTransferAsset(info)) { @@ -116,6 +117,8 @@ export const TxCollapsed = ({ const { ref, lastItemId } = useContext(TxsInfiniteScrollContext) const willBeReplaced = transaction?.txStatus === 'WILL_BE_REPLACED' ? ' will-be-replaced' : '' + const onChainRejection = + isCancelTxDetails(transaction.txInfo) && txLocation !== 'history' ? ' on-chain-rejection' : '' const txCollapsedNonce = (
@@ -124,7 +127,7 @@ export const TxCollapsed = ({ ) const txCollapsedType = ( -
+
) diff --git a/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx b/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx index 1e753ecd..4751cc37 100644 --- a/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxCollapsedActions.tsx @@ -46,7 +46,7 @@ export const TxCollapsedActions = ({ transaction }: TxCollapsedActionsProps): Re {canCancel && ( - + diff --git a/src/routes/safe/components/Transactions/TxList/TxDetails.tsx b/src/routes/safe/components/Transactions/TxList/TxDetails.tsx index 789afd82..675f59c1 100644 --- a/src/routes/safe/components/Transactions/TxList/TxDetails.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxDetails.tsx @@ -1,7 +1,6 @@ import { Icon, Link, Loader, Text } from '@gnosis.pm/safe-react-components' import cn from 'classnames' import React, { ReactElement, useContext } from 'react' -import { useSelector } from 'react-redux' import styled from 'styled-components' import { @@ -12,7 +11,6 @@ import { MultiSigExecutionDetails, Transaction, } from 'src/logic/safe/store/models/types/gateway.d' -import { safeParamAddressFromStateSelector } from 'src/logic/safe/store/selectors' import { TransactionActions } from './hooks/useTransactionActions' import { useTransactionDetails } from './hooks/useTransactionDetails' import { TxDetailsContainer, Centered, AlignItemsWithMargin } from './styled' @@ -30,34 +28,44 @@ const NormalBreakingText = styled(Text)` ` const TxDataGroup = ({ txDetails }: { txDetails: ExpandedTxDetails }): ReactElement | null => { - const safeAddress = useSelector(safeParamAddressFromStateSelector) - if (isTransferTxInfo(txDetails.txInfo) || isSettingsChangeTxInfo(txDetails.txInfo)) { return } - if (isCancelTxDetails({ executedAt: txDetails.executedAt, txInfo: txDetails.txInfo, safeAddress })) { + if (isCancelTxDetails(txDetails.txInfo)) { + const txNonce = `${(txDetails.detailedExecutionInfo as MultiSigExecutionDetails).nonce ?? NOT_AVAILABLE}` + const isTxExecuted = txDetails.executedAt + + // executed rejection transaction + let message = `This is an on-chain rejection that didn't send any funds. + This on-chain rejection replaced all transactions with nonce ${txNonce}.` + + if (!isTxExecuted) { + // queued rejection transaction + message = `This is an on-chain rejection that doesn't send any funds. + Executing this on-chain rejection will replace all currently awaiting transactions with nonce ${txNonce}.` + } return ( <> - - {`This is an empty cancelling transaction that doesn't send any funds. - Executing this transaction will replace all currently awaiting transactions with nonce ${ - (txDetails.detailedExecutionInfo as MultiSigExecutionDetails).nonce ?? NOT_AVAILABLE - }.`} - - - - - Why do I need to pay for cancelling a transaction? - - - - + {message} + {!isTxExecuted && ( + <> +
+ + + + Why do I need to pay for rejecting a transaction? + + + + + + )} ) } @@ -116,7 +124,7 @@ export const TxDetails = ({ transaction, actions }: TxDetailsProps): ReactElemen 'will-be-replaced': transaction.txStatus === 'WILL_BE_REPLACED', })} > - +
{!data.executedAt && txLocation !== 'history' && actions?.isUserAnOwner && (
diff --git a/src/routes/safe/components/Transactions/TxList/TxExpandedActions.tsx b/src/routes/safe/components/Transactions/TxList/TxExpandedActions.tsx index 71e0c28c..81f7d804 100644 --- a/src/routes/safe/components/Transactions/TxList/TxExpandedActions.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxExpandedActions.tsx @@ -41,7 +41,7 @@ export const TxExpandedActions = ({ transaction }: TxExpandedActionsProps): Reac {canCancel && ( )} diff --git a/src/routes/safe/components/Transactions/TxList/TxOwners.tsx b/src/routes/safe/components/Transactions/TxList/TxOwners.tsx index 6b97e081..22b45998 100644 --- a/src/routes/safe/components/Transactions/TxList/TxOwners.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxOwners.tsx @@ -1,4 +1,4 @@ -import { Text } from '@gnosis.pm/safe-react-components' +import { Text, Icon } from '@gnosis.pm/safe-react-components' import React, { ReactElement } from 'react' import styled from 'styled-components' @@ -6,43 +6,55 @@ import Img from 'src/components/layout/Img' import { ExpandedTxDetails, isModuleExecutionDetails } from 'src/logic/safe/store/models/types/gateway.d' import TransactionListActive from './assets/transactions-list-active.svg' import TransactionListInactive from './assets/transactions-list-inactive.svg' -import CheckCircleGreen from './assets/check-circle-green.svg' -import PlusCircleGreen from './assets/plus-circle-green.svg' import { OwnerRow } from './OwnerRow' import { OwnerList, OwnerListItem } from './styled' - -type TxOwnersProps = { - detailedExecutionInfo: ExpandedTxDetails['detailedExecutionInfo'] -} +import { isCancelTxDetails } from './utils' const StyledImg = styled(Img)` background-color: transparent; border-radius: 50%; ` -export const TxOwners = ({ detailedExecutionInfo }: TxOwnersProps): ReactElement | null => { +export const TxOwners = ({ txDetails }: { txDetails: ExpandedTxDetails }): ReactElement | null => { + const { txInfo, detailedExecutionInfo } = txDetails + if (!detailedExecutionInfo || isModuleExecutionDetails(detailedExecutionInfo)) { return null } const confirmationsNeeded = detailedExecutionInfo.confirmationsRequired - detailedExecutionInfo.confirmations.length + const CreationNode = isCancelTxDetails(txInfo) ? ( + + + + +
+ + On-chain rejection created + +
+
+ ) : ( + + + + +
+ + Created + +
+
+ ) + return ( - - - - -
- - Created - -
-
+ {CreationNode} {detailedExecutionInfo.confirmations.map(({ signer }) => ( - +
@@ -55,7 +67,11 @@ export const TxOwners = ({ detailedExecutionInfo }: TxOwnersProps): ReactElement {confirmationsNeeded <= 0 ? ( - + {detailedExecutionInfo.executor ? ( + + ) : ( + + )}
diff --git a/src/routes/safe/components/Transactions/TxList/TxSummary.tsx b/src/routes/safe/components/Transactions/TxList/TxSummary.tsx index 84fb6cc7..4bc50b77 100644 --- a/src/routes/safe/components/Transactions/TxList/TxSummary.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxSummary.tsx @@ -27,7 +27,7 @@ export const TxSummary = ({ txDetails }: { txDetails: ExpandedTxDetails }): Reac )}
- {nonce && ( + {nonce !== undefined && (
Nonce:{' '} diff --git a/src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg b/src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg new file mode 100644 index 00000000..ff8f6306 --- /dev/null +++ b/src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts index 5e3b7566..3609d6bc 100644 --- a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts +++ b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts @@ -7,7 +7,6 @@ import { getQueuedTransactionsByNonce } from 'src/logic/safe/store/selectors/gat import { sameAddress } from 'src/logic/wallets/ethAddresses' import { userAccountSelector } from 'src/logic/wallets/store/selectors' import { TxLocationContext } from 'src/routes/safe/components/Transactions/TxList/TxLocationProvider' -import { isCancelTransaction } from 'src/routes/safe/components/Transactions/TxList/utils' import { grantedSelector } from 'src/routes/safe/container/selector' import { AppReduxState } from 'src/store' @@ -60,14 +59,7 @@ export const useTransactionActions = (transaction: Transaction): TransactionActi canConfirm, canConfirmThenExecute: txLocation === 'queued.next' && canConfirm && oneToGo, canExecute: txLocation === 'queued.next' && thresholdReached, - canCancel: !transactionsByNonce.some( - ({ txInfo }) => - isCustomTxInfo(txInfo) && - isCancelTransaction({ - txInfo, - safeAddress, - }), - ), + canCancel: !transactionsByNonce.some(({ txInfo }) => isCustomTxInfo(txInfo) && txInfo.isCancellation), isUserAnOwner, oneToGo, }) diff --git a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionStatus.ts b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionStatus.ts index e64637b7..4df71af3 100644 --- a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionStatus.ts +++ b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionStatus.ts @@ -37,10 +37,10 @@ export const useTransactionStatus = (transaction: Transaction): TransactionStatu switch (transaction.txStatus) { case 'AWAITING_CONFIRMATIONS': - text = signaturePending(currentUser) ? 'Awaiting your confirmation' : 'Awaiting confirmations' + text = signaturePending(currentUser) ? 'Needs your confirmation' : 'Needs confirmations' break case 'AWAITING_EXECUTION': - text = 'Awaiting execution' + text = 'Needs execution' break case 'PENDING': case 'PENDING_FAILED': diff --git a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts index ca04efbc..e33b81b7 100644 --- a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts +++ b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionType.ts @@ -4,10 +4,10 @@ import { useSelector } from 'react-redux' import { Transaction } from 'src/logic/safe/store/models/types/gateway.d' import { safeParamAddressFromStateSelector } from 'src/logic/safe/store/selectors' import CustomTxIcon from 'src/routes/safe/components/Transactions/TxList/assets/custom.svg' +import CircleCrossRed from 'src/routes/safe/components/Transactions/TxList/assets/circle-cross-red.svg' import IncomingTxIcon from 'src/routes/safe/components/Transactions/TxList/assets/incoming.svg' import OutgoingTxIcon from 'src/routes/safe/components/Transactions/TxList/assets/outgoing.svg' import SettingsTxIcon from 'src/routes/safe/components/Transactions/TxList/assets/settings.svg' -import { isCancelTransaction } from 'src/routes/safe/components/Transactions/TxList/utils' export type TxTypeProps = { icon: string @@ -40,20 +40,8 @@ export const useTransactionType = (tx: Transaction): TxTypeProps => { break } - // TODO: isCancel - // there are two 'cancelling' tx identification - // this one is the candidate to remain when the client gateway implements - // https://github.com/gnosis/safe-client-gateway/issues/255 - if (typeof tx.txInfo.isCancellation === 'boolean' && tx.txInfo.isCancellation) { - setType({ icon: CustomTxIcon, text: 'Cancelling transaction' }) - break - } - - // TODO: isCancel - // remove the following condition when issue#255 is implemented - // also remove `isCancelTransaction` function - if (isCancelTransaction({ txInfo: tx.txInfo, safeAddress })) { - setType({ icon: CustomTxIcon, text: 'Cancelling transaction' }) + if (tx.txInfo.isCancellation) { + setType({ icon: CircleCrossRed, text: 'On-chain rejection' }) break } diff --git a/src/routes/safe/components/Transactions/TxList/styled.tsx b/src/routes/safe/components/Transactions/TxList/styled.tsx index de8d7f19..9058bc80 100644 --- a/src/routes/safe/components/Transactions/TxList/styled.tsx +++ b/src/routes/safe/components/Transactions/TxList/styled.tsx @@ -163,6 +163,32 @@ const failedTransaction = css` } ` +const onChainRejection = css` + &.on-chain-rejection { + background-color: ${({ theme }) => theme.colors.errorTooltip}; + border-left: 4px solid ${({ theme }) => theme.colors.error}; + border-radius: 4px; + padding-left: 7px; + height: 22px; + max-width: 165px; + + > div { + height: 17px; + align-items: center; + padding-top: 3px; + } + + p { + font-size: 11px; + line-height: 16px; + letter-spacing: 1px; + font-weight: bold; + text-transform: uppercase; + margin-left: -2px; + } + } +` + export const StyledTransaction = styled.div` ${willBeReplaced}; ${failedTransaction}; @@ -175,6 +201,10 @@ export const StyledTransaction = styled.div` align-self: center; } + .tx-type { + ${onChainRejection}; + } + .tx-votes { justify-self: center; } diff --git a/src/routes/safe/components/Transactions/TxList/utils.ts b/src/routes/safe/components/Transactions/TxList/utils.ts index d461e8cc..13b15057 100644 --- a/src/routes/safe/components/Transactions/TxList/utils.ts +++ b/src/routes/safe/components/Transactions/TxList/utils.ts @@ -2,7 +2,6 @@ import { BigNumber } from 'bignumber.js' import { getNetworkInfo } from 'src/config' import { - Custom, isCustomTxInfo, isTransferTxInfo, Transaction, @@ -12,7 +11,6 @@ import { import { formatAmount } from 'src/logic/tokens/utils/formatAmount' import { sameAddress } from 'src/logic/wallets/ethAddresses' -import { sameString } from 'src/utils/strings' export const NOT_AVAILABLE = 'n/a' @@ -90,27 +88,11 @@ export const getTxTokenData = (txInfo: Transfer): txTokenData => { } } -// TODO: isCancel -// how can we be sure that it's a cancel tx without asking for tx-details? -// can the client-gateway service provide info about the tx, Like: `isCancelTransaction: boolean`? -// it will be solved as part of: https://github.com/gnosis/safe-client-gateway/issues/255 -export const isCancelTransaction = ({ txInfo, safeAddress }: { txInfo: Custom; safeAddress: string }): boolean => - sameAddress(txInfo.to, safeAddress) && - sameString(txInfo.dataSize, '0') && - sameString(txInfo.value, '0') && - txInfo.methodName === null - -type IsCancelTxDetailsProps = { - executedAt: number | null - txInfo: Transaction['txInfo'] - safeAddress: string -} -export const isCancelTxDetails = ({ executedAt, txInfo, safeAddress }: IsCancelTxDetailsProps): boolean => - !executedAt && +export const isCancelTxDetails = (txInfo: Transaction['txInfo']): boolean => // custom transaction isCustomTxInfo(txInfo) && - // verify that it's a cancel tx based on it's info - isCancelTransaction({ safeAddress, txInfo }) + // flag-based identification + txInfo.isCancellation export const addressInList = (list: string[] = []) => (address: string): boolean => list.some((ownerAddress) => sameAddress(ownerAddress, address)) diff --git a/yarn.lock b/yarn.lock index 56873e07..b65e2fb0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1596,9 +1596,9 @@ solc "0.5.14" truffle "^5.1.21" -"@gnosis.pm/safe-react-components@https://github.com/gnosis/safe-react-components.git#a68a67e": +"@gnosis.pm/safe-react-components@https://github.com/gnosis/safe-react-components.git#7ebc414": version "0.5.0" - resolved "https://github.com/gnosis/safe-react-components.git#a68a67e634d0be091856ebba9f6874eebb767cd7" + resolved "https://github.com/gnosis/safe-react-components.git#7ebc414ae975d60846704c5a8db5e61c30348d12" dependencies: classnames "^2.2.6" react-media "^1.10.0" From 9f3ff69a75401cc87c883bab84b8894ced758b76 Mon Sep 17 00:00:00 2001 From: nicolas Date: Wed, 7 Apr 2021 08:10:48 -0300 Subject: [PATCH 4/4] Known Addresses V1 (#2113) * Tx Custom: Add toInfo (icon+name) in table row. * Custom tx: add to info if available * update EthHashInfo API usage Co-authored-by: Fernando --- package.json | 2 +- .../ProviderInfo/ProviderAccessible.tsx | 4 +- src/components/CustomIconText/index.tsx | 6 ++- src/components/DecodeTxs/index.tsx | 4 +- .../SafeList/AddressWrapper.tsx | 2 +- src/logic/contracts/safeContracts.ts | 15 +++--- .../ConfirmTxModal/ReviewConfirm.tsx | 2 +- .../Balances/SendModal/SafeInfo/index.tsx | 2 +- .../screens/AddressBookInput/index.tsx | 2 +- .../ContractInteraction/Review/index.tsx | 2 +- .../SendModal/screens/SendFunds/index.tsx | 2 +- .../SpendingLimit/FormFields/Beneficiary.tsx | 2 +- .../SpendingLimit/InfoDisplay/AddressInfo.tsx | 2 +- .../Transactions/TxList/AddressInfo.tsx | 13 +++-- .../Transactions/TxList/OwnerRow.tsx | 2 +- .../components/Transactions/TxList/TxData.tsx | 47 ++++++++++++++----- .../Transactions/TxList/TxDetails.tsx | 2 +- .../components/Transactions/TxList/TxInfo.tsx | 12 +---- .../Transactions/TxList/TxInfoDetails.tsx | 14 +++++- .../TxList/hooks/useTransactionType.ts | 8 +++- yarn.lock | 4 +- 21 files changed, 93 insertions(+), 56 deletions(-) diff --git a/package.json b/package.json index 82ed3cfe..4eaefeb1 100644 --- a/package.json +++ b/package.json @@ -161,7 +161,7 @@ "@gnosis.pm/safe-apps-sdk": "1.0.3", "@gnosis.pm/safe-apps-sdk-v1": "npm:@gnosis.pm/safe-apps-sdk@0.4.2", "@gnosis.pm/safe-contracts": "1.1.1-dev.2", - "@gnosis.pm/safe-react-components": "https://github.com/gnosis/safe-react-components.git#7ebc414", + "@gnosis.pm/safe-react-components": "https://github.com/gnosis/safe-react-components.git#2e427ee", "@gnosis.pm/util-contracts": "2.0.6", "@ledgerhq/hw-transport-node-hid-singleton": "5.45.0", "@material-ui/core": "^4.11.0", diff --git a/src/components/AppLayout/Header/components/ProviderInfo/ProviderAccessible.tsx b/src/components/AppLayout/Header/components/ProviderInfo/ProviderAccessible.tsx index 563c3abb..e4720113 100644 --- a/src/components/AppLayout/Header/components/ProviderInfo/ProviderAccessible.tsx +++ b/src/components/AppLayout/Header/components/ProviderInfo/ProviderAccessible.tsx @@ -89,8 +89,8 @@ const ProviderInfo = ({ connected, provider, userAddress }: ProviderInfoProps): diff --git a/src/components/CustomIconText/index.tsx b/src/components/CustomIconText/index.tsx index cc059de1..5cbb94ab 100644 --- a/src/components/CustomIconText/index.tsx +++ b/src/components/CustomIconText/index.tsx @@ -12,9 +12,11 @@ const Icon = styled.img` margin-right: 9px; ` -export const CustomIconText = ({ iconUrl, text }: { iconUrl: string; text?: string }): ReactElement => ( +type Props = { iconUrl: string | null | undefined; text?: string } + +export const CustomIconText = ({ iconUrl, text }: Props): ReactElement => ( - + {iconUrl && } {text && {text}} ) diff --git a/src/components/DecodeTxs/index.tsx b/src/components/DecodeTxs/index.tsx index 8b242e99..703ef001 100644 --- a/src/components/DecodeTxs/index.tsx +++ b/src/components/DecodeTxs/index.tsx @@ -69,7 +69,7 @@ export const BasicTxInfo = ({ { return (
- +
{`${formatAmount(safe.ethBalance)} ${nativeCoin.name}`} diff --git a/src/logic/contracts/safeContracts.ts b/src/logic/contracts/safeContracts.ts index 5ba70c7c..8001d894 100644 --- a/src/logic/contracts/safeContracts.ts +++ b/src/logic/contracts/safeContracts.ts @@ -123,23 +123,22 @@ export const estimateGasForDeployingSafe = async ( data: proxyFactoryData, from: userAccount, to: proxyFactoryMaster.options.address, - }).then(value => value * 2) + }).then((value) => value * 2) } export const getGnosisSafeInstanceAt = (safeAddress: string): GnosisSafe => { const web3 = getWeb3() return (new web3.eth.Contract(GnosisSafeSol.abi as AbiItem[], safeAddress) as unknown) as GnosisSafe - } /** * Creates a Contract instance of the SpendingLimitModule contract */ - export const getSpendingLimitContract = () => { +export const getSpendingLimitContract = () => { const web3 = getWeb3() - return (new web3.eth.Contract( - SpendingLimitModule.abi as AbiItem[], - SPENDING_LIMIT_MODULE_ADDRESS, - ) as unknown) as AllowanceModule -} \ No newline at end of file + return (new web3.eth.Contract( + SpendingLimitModule.abi as AbiItem[], + SPENDING_LIMIT_MODULE_ADDRESS, + ) as unknown) as AllowanceModule +} diff --git a/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx b/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx index 016d165d..06e4fcc4 100644 --- a/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx +++ b/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx @@ -219,7 +219,7 @@ export const ReviewConfirm = ({ {/* Safe */} - + Balance: {`${ethBalance} ${nativeCoin.symbol}`} diff --git a/src/routes/safe/components/Balances/SendModal/SafeInfo/index.tsx b/src/routes/safe/components/Balances/SendModal/SafeInfo/index.tsx index 36691422..27f5ad53 100644 --- a/src/routes/safe/components/Balances/SendModal/SafeInfo/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/SafeInfo/index.tsx @@ -32,7 +32,7 @@ const SafeInfo = (): React.ReactElement => { hash={safeAddress} name={safeName} explorerUrl={getExplorerInfo(safeAddress)} - showIdenticon + showAvatar showCopyBtn /> {ethBalance && ( diff --git a/src/routes/safe/components/Balances/SendModal/screens/AddressBookInput/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/AddressBookInput/index.tsx index 0bc42e7f..1e97da33 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/AddressBookInput/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/screens/AddressBookInput/index.tsx @@ -148,7 +148,7 @@ const BaseAddressBookInput = ({ /> )} getOptionLabel={({ address }) => address} - renderOption={({ address, name }) => } + renderOption={({ address, name }) => } role="listbox" style={{ display: 'flex', flexGrow: 1 }} /> 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 f02e60f2..d39700c2 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 @@ -151,7 +151,7 @@ const ContractInteractionReview = ({ onClose, onPrev, tx }: Props): React.ReactE - + diff --git a/src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx b/src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx index 8a8db233..8e5524d3 100644 --- a/src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx +++ b/src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx @@ -265,7 +265,7 @@ const SendFunds = ({ diff --git a/src/routes/safe/components/Settings/SpendingLimit/FormFields/Beneficiary.tsx b/src/routes/safe/components/Settings/SpendingLimit/FormFields/Beneficiary.tsx index 0e960d6e..4002bc5c 100644 --- a/src/routes/safe/components/Settings/SpendingLimit/FormFields/Beneficiary.tsx +++ b/src/routes/safe/components/Settings/SpendingLimit/FormFields/Beneficiary.tsx @@ -81,7 +81,7 @@ const Beneficiary = (): ReactElement => { hash={selectedEntry.address} name={selectedEntry.name} showCopyBtn - showIdenticon + showAvatar textSize="lg" shortenHash={4} explorerUrl={getExplorerInfo(selectedEntry.address)} diff --git a/src/routes/safe/components/Settings/SpendingLimit/InfoDisplay/AddressInfo.tsx b/src/routes/safe/components/Settings/SpendingLimit/InfoDisplay/AddressInfo.tsx index 9e98c1fc..c18fdd30 100644 --- a/src/routes/safe/components/Settings/SpendingLimit/InfoDisplay/AddressInfo.tsx +++ b/src/routes/safe/components/Settings/SpendingLimit/InfoDisplay/AddressInfo.tsx @@ -24,7 +24,7 @@ const AddressInfo = ({ address, cut = 4, title }: AddressInfoProps): ReactElemen hash={address} name={sameString(name, 'UNKNOWN') ? undefined : name} showCopyBtn - showIdenticon + showAvatar textSize="lg" explorerUrl={explorerUrl} shortenHash={cut} diff --git a/src/routes/safe/components/Transactions/TxList/AddressInfo.tsx b/src/routes/safe/components/Transactions/TxList/AddressInfo.tsx index 46199746..45f4b654 100644 --- a/src/routes/safe/components/Transactions/TxList/AddressInfo.tsx +++ b/src/routes/safe/components/Transactions/TxList/AddressInfo.tsx @@ -5,7 +5,13 @@ import { getExplorerInfo } from 'src/config' import { getNameFromAddressBookSelector } from 'src/logic/addressBook/store/selectors' -export const AddressInfo = ({ address }: { address: string }): ReactElement | null => { +type Props = { + address: string + name?: string | undefined + avatarUrl?: string | undefined +} + +export const AddressInfo = ({ address, name, avatarUrl }: Props): ReactElement | null => { const recipientName = useSelector((state) => getNameFromAddressBookSelector(state, address)) if (address === '') { @@ -15,8 +21,9 @@ export const AddressInfo = ({ address }: { address: string }): ReactElement | nu return ( diff --git a/src/routes/safe/components/Transactions/TxList/OwnerRow.tsx b/src/routes/safe/components/Transactions/TxList/OwnerRow.tsx index ebfca691..6cd2cab5 100644 --- a/src/routes/safe/components/Transactions/TxList/OwnerRow.tsx +++ b/src/routes/safe/components/Transactions/TxList/OwnerRow.tsx @@ -12,7 +12,7 @@ export const OwnerRow = ({ ownerAddress }: { ownerAddress: string }): ReactEleme ( - <> - - {children} - -) +const DetailsWithTxInfo = ({ children, txData, txInfo }: DetailsWithTxInfoProps): ReactElement => { + const amount = txData.value ? fromTokenUnit(txData.value, nativeCoin.decimals) : 'n/a' + let name + let avatarUrl + + if (isCustomTxInfo(txInfo)) { + name = txInfo.toInfo.name + avatarUrl = txInfo.toInfo.logoUri + } + + return ( + <> + + + {children} + + ) +} type TxDataProps = { txData: ExpandedTxDetails['txData'] + txInfo: TransactionInfo } -export const TxData = ({ txData }: TxDataProps): ReactElement | null => { +export const TxData = ({ txData, txInfo }: TxDataProps): ReactElement | null => { // nothing to render if (!txData) { return null @@ -51,7 +72,7 @@ export const TxData = ({ txData }: TxDataProps): ReactElement | null => { // we render the hex encoded data return ( - + ) @@ -74,7 +95,7 @@ export const TxData = ({ txData }: TxDataProps): ReactElement | null => { // we render the decoded data return ( - + ) diff --git a/src/routes/safe/components/Transactions/TxList/TxDetails.tsx b/src/routes/safe/components/Transactions/TxList/TxDetails.tsx index 675f59c1..fdc043ca 100644 --- a/src/routes/safe/components/Transactions/TxList/TxDetails.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxDetails.tsx @@ -74,7 +74,7 @@ const TxDataGroup = ({ txDetails }: { txDetails: ExpandedTxDetails }): ReactElem return null } - return + return } type TxDetailsProps = { diff --git a/src/routes/safe/components/Transactions/TxList/TxInfo.tsx b/src/routes/safe/components/Transactions/TxList/TxInfo.tsx index 9a485d9a..725d6780 100644 --- a/src/routes/safe/components/Transactions/TxList/TxInfo.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxInfo.tsx @@ -1,18 +1,10 @@ import React, { ReactElement } from 'react' -import { - ExpandedTxDetails, - isSettingsChangeTxInfo, - isTransferTxInfo, -} from 'src/logic/safe/store/models/types/gateway.d' +import { TransactionInfo, isSettingsChangeTxInfo, isTransferTxInfo } from 'src/logic/safe/store/models/types/gateway.d' import { TxInfoSettings } from './TxInfoSettings' import { TxInfoTransfer } from './TxInfoTransfer' -type TxInfoProps = { - txInfo: ExpandedTxDetails['txInfo'] -} - -export const TxInfo = ({ txInfo }: TxInfoProps): ReactElement | null => { +export const TxInfo = ({ txInfo }: { txInfo: TransactionInfo }): ReactElement | null => { if (isSettingsChangeTxInfo(txInfo)) { return } diff --git a/src/routes/safe/components/Transactions/TxList/TxInfoDetails.tsx b/src/routes/safe/components/Transactions/TxList/TxInfoDetails.tsx index 2ef7d24a..8c0a84a0 100644 --- a/src/routes/safe/components/Transactions/TxList/TxInfoDetails.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxInfoDetails.tsx @@ -21,11 +21,20 @@ const SingleRow = styled.div` type TxInfoDetailsProps = { title: string address: string + name?: string | undefined + avatarUrl?: string | undefined isTransferType?: boolean txInfo?: Transfer } -export const TxInfoDetails = ({ title, address, isTransferType, txInfo }: TxInfoDetailsProps): ReactElement => { +export const TxInfoDetails = ({ + title, + address, + isTransferType, + txInfo, + name, + avatarUrl, +}: TxInfoDetailsProps): ReactElement => { const recipientName = useSelector((state) => getNameFromAddressBookSelector(state, address)) const knownAddress = recipientName !== 'UNKNOWN' @@ -59,6 +68,7 @@ export const TxInfoDetails = ({ title, address, isTransferType, txInfo }: TxInfo selectedToken: ZERO_ADDRESS, tokenAmount: '0', }) + useEffect(() => { if (txInfo) { const isCollectible = txInfo.transferInfo.type === 'ERC721' @@ -76,7 +86,7 @@ export const TxInfoDetails = ({ title, address, isTransferType, txInfo }: TxInfo return ( - + { break } + const toInfo = tx.txInfo.toInfo + if (toInfo) { + setType({ icon: toInfo.logoUri, text: toInfo.name }) + break + } + setType({ icon: CustomTxIcon, text: 'Contract interaction' }) break } diff --git a/yarn.lock b/yarn.lock index b65e2fb0..79aa2e77 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1596,9 +1596,9 @@ solc "0.5.14" truffle "^5.1.21" -"@gnosis.pm/safe-react-components@https://github.com/gnosis/safe-react-components.git#7ebc414": +"@gnosis.pm/safe-react-components@https://github.com/gnosis/safe-react-components.git#2e427ee": version "0.5.0" - resolved "https://github.com/gnosis/safe-react-components.git#7ebc414ae975d60846704c5a8db5e61c30348d12" + resolved "https://github.com/gnosis/safe-react-components.git#2e427ee36694c7964301fc155b0c080101a34bed" dependencies: classnames "^2.2.6" react-media "^1.10.0"