From e3797d9e786d2554ae683e501aa3f56a7d05ebca Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Date: Thu, 29 Apr 2021 10:33:52 +0200 Subject: [PATCH] Fix ignored safeTxGas from app (#2193) --- src/logic/hooks/useEstimateTransactionGas.tsx | 1 - .../safe/transactions/__tests__/gas.test.ts | Bin 8122 -> 0 bytes src/logic/safe/transactions/gas.ts | 159 +----------------- .../components/Apps/components/AppFrame.tsx | 4 +- .../ConfirmTxModal/ReviewConfirm.tsx | 36 +--- .../Apps/components/GasEstimationInfo.tsx | 47 ------ 6 files changed, 7 insertions(+), 240 deletions(-) delete mode 100644 src/logic/safe/transactions/__tests__/gas.test.ts delete mode 100644 src/routes/safe/components/Apps/components/GasEstimationInfo.tsx diff --git a/src/logic/hooks/useEstimateTransactionGas.tsx b/src/logic/hooks/useEstimateTransactionGas.tsx index 6e9799ae..af12936f 100644 --- a/src/logic/hooks/useEstimateTransactionGas.tsx +++ b/src/logic/hooks/useEstimateTransactionGas.tsx @@ -157,7 +157,6 @@ export const useEstimateTransactionGas = ({ txRecipient, txAmount: txAmount || '0', operation: operation || CALL, - safeTxGas, }) } if (isExecution || approvalAndExecution) { diff --git a/src/logic/safe/transactions/__tests__/gas.test.ts b/src/logic/safe/transactions/__tests__/gas.test.ts deleted file mode 100644 index 028796ad206923d510f12579a643e819f097a517..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 8122 zcmeHM+iuf95al^vG0KY*(bS=7X^9jbfN+zVqEPX~Ll=8uujH(w-OUA}d>Z1D_yuO| zTj-Lf z=sbsc1%T_?jq<~B;DSUwmI7Qd1fMBCUfAOk6p(aBhH6C1;!V;KfNeu$OkM#TGSJ47 z7VOSO=xB7I6Ws!^nzQDG(>i~~n#GQjVvjeAJ^%Fd?A%JcZ5YR#q8x>1^SbYZMk!bT z?>}Dk8vPG10ZodJKMM;UuVOM!odHxGDurqr8m*J&le44kyTiCX$e{Y^5^#pj*c8Lk zb2Vdc(rS$RI6-NWaX2}sred=oD3}VQa-8}o68VX%5jwl@R@+KqeeI0E6g$JHoNKYS z`mqjQ;PPZu=yh6jVScjc8VeT(Tu}TYSmSV{zoyV-UrYQJ$^syR^%{wH-+C0ZArz=f-i_~Go%mpW0b=u)=)en#Nj$d7S7quYi@ zJCDK7AKz25T}J*4CHFXqrjdG0%4No2sgJfYQfERM&G}=$mqrgKWoddNF)|hEf(mJ} z1pc)=n#*-5L;`Km1@Hlo;Lgg#s=nEZ{CUK9e*aL8;0qg_%+$n$u+dHZzJG zr2Hv_3DUdf86}{zJ { - try { - return JSON.parse(stringInput) - } catch (error) { - return null - } -} - -// Parses the result from the error message (GETH, OpenEthereum/Parity and Nethermind) and returns the data value -export const getDataFromNodeErrorMessage = (errorMessage: string): string | undefined => { - // Replace illegal characters that often comes within the error string (like � for example) - // https://stackoverflow.com/questions/12754256/removing-invalid-characters-in-javascript - const normalizedErrorString = errorMessage.replace(/\uFFFD/g, '') - - // Extracts JSON object from the error message - const [, ...error] = normalizedErrorString.split('\n') - - try { - const errorAsString = error.join('') - const errorAsJSON = getJSONOrNullFromString(errorAsString) - - // Trezor wallet returns the error not as an JSON object but directly as string - if (!errorAsJSON) { - return errorAsString.length ? errorAsString : undefined - } - - // For new GETH nodes they will return the data as error in the format: - // { - // "originalError": { - // "code": number, - // "data": string, - // "message": "execution reverted: ..." - // } - // } - if (errorAsJSON.originalError && errorAsJSON.originalError.data) { - return errorAsJSON.originalError.data - } - - // OpenEthereum/Parity nodes will return the data as error in the format: - // { - // "error": { - // "code": number, - // "message": string, - // "data": "revert: 0x..." -> this is the result data that should be extracted from the message - // }, - // "id": number - // } - if (errorAsJSON?.data) { - const [, dataResult] = errorAsJSON.data.split(' ') - return dataResult - } - } catch (error) { - console.error(`Error trying to extract data from node error message: ${errorMessage}`) - } -} - -const estimateGasWithWeb3Provider = async (txConfig: { - to: string - from: string - data: string - gasPrice?: number - gas?: number -}): Promise => { - const web3 = getWeb3() - try { - const result = await web3.eth.call(txConfig) - - // GETH Nodes (geth version < v1.9.24) - // In case that the gas is not enough we will receive an EMPTY data - // Otherwise we will receive the gas amount as hash data -> this is valid for old versions of GETH nodes ( < v1.9.24) - - if (!sameString(result, EMPTY_DATA)) { - return new BigNumber(result.substring(138), 16).toNumber() - } - } catch (error) { - // So we try to extract the estimation result within the error in case is possible - const estimationData = getDataFromNodeErrorMessage(error.message) - - if (!estimationData || sameString(estimationData, EMPTY_DATA)) { - throw error - } - - return new BigNumber(estimationData.substring(138), 16).toNumber() - } - throw new Error('Error while estimating the gas required for tx') -} - -const estimateGasWithRPCCall = async (txConfig: { - to: string - from: string - data: string - gasPrice?: number - gas?: number -}): Promise => { - try { - const { data } = await axios.post(getRpcServiceUrl(), { - jsonrpc: '2.0', - method: 'eth_call', - id: 1, - params: [ - { - ...txConfig, - gasPrice: web3ReadOnly.utils.toHex(txConfig.gasPrice || 0), - gas: txConfig.gas ? web3ReadOnly.utils.toHex(txConfig.gas) : undefined, - }, - 'latest', - ], - }) - - const { error } = data - if (error?.data) { - return new BigNumber(error.data.substring(138), 16).toNumber() - } - } catch (error) { - console.log('Gas estimation endpoint errored: ', error.message) - } - throw new Error('Error while estimating the gas required for tx') -} - -export const getGasEstimationTxResponse = async (txConfig: { - to: string - from: string - data: string - gasPrice?: number - gas?: number -}): Promise => { - // If we are in a infura supported network we estimate using infura - if (usesInfuraRPC) { - return estimateGasWithRPCCall(txConfig) - } - // Otherwise we estimate using the current connected provider - return estimateGasWithWeb3Provider(txConfig) -} type SafeTxGasEstimationProps = { safeAddress: string @@ -160,7 +14,6 @@ type SafeTxGasEstimationProps = { txRecipient: string txAmount: string operation: number - safeTxGas?: number } export const estimateSafeTxGas = async ({ @@ -169,7 +22,6 @@ export const estimateSafeTxGas = async ({ txRecipient, txAmount, operation, - safeTxGas, }: SafeTxGasEstimationProps): Promise => { try { const safeTxGasEstimation = await fetchSafeTxGasEstimation({ @@ -180,15 +32,6 @@ export const estimateSafeTxGas = async ({ operation, }) - console.log('Backend gas estimation', safeTxGasEstimation) - - if (safeTxGas) { - // 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 - } - return parseInt(safeTxGasEstimation) } catch (error) { console.info('Error calculating tx gas estimation', error.message) diff --git a/src/routes/safe/components/Apps/components/AppFrame.tsx b/src/routes/safe/components/Apps/components/AppFrame.tsx index 43774350..e332d6c9 100644 --- a/src/routes/safe/components/Apps/components/AppFrame.tsx +++ b/src/routes/safe/components/Apps/components/AppFrame.tsx @@ -1,4 +1,4 @@ -import React, { useState, useRef, useCallback, useEffect } from 'react' +import React, { ReactElement, useState, useRef, useCallback, useEffect } from 'react' import styled from 'styled-components' import { FixedIcon, Loader, Title, Card } from '@gnosis.pm/safe-react-components' import { MethodToResponse, RPCPayload } from '@gnosis.pm/safe-apps-sdk' @@ -82,7 +82,7 @@ const INITIAL_CONFIRM_TX_MODAL_STATE: ConfirmTransactionModalState = { params: undefined, } -const AppFrame = ({ appUrl }: Props): React.ReactElement => { +const AppFrame = ({ appUrl }: Props): ReactElement => { const granted = useSelector(grantedSelector) const safeAddress = useSelector(safeParamAddressFromStateSelector) const ethBalance = useSelector(safeEthBalanceSelector) diff --git a/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx b/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx index 56712d83..bdc91cc7 100644 --- a/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx +++ b/src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx @@ -1,11 +1,9 @@ -import React, { useEffect, useMemo, useState } from 'react' +import React, { ReactElement, useEffect, useMemo, useState } from 'react' import { Text, EthHashInfo, ModalFooterConfirmation } from '@gnosis.pm/safe-react-components' import styled from 'styled-components' import { useDispatch } from 'react-redux' -import TextBox from 'src/components/TextBox' import ModalTitle from 'src/components/ModalTitle' -import Heading from 'src/components/layout/Heading' import { createTransaction } from 'src/logic/safe/store/actions/createTransaction' import { MULTI_SEND_ADDRESS } from 'src/logic/contracts/safeContracts' import { DELEGATE_CALL, TX_NOTIFICATION_TYPES, CALL } from 'src/logic/safe/transactions' @@ -26,16 +24,11 @@ import { getExplorerInfo } from 'src/config' import Block from 'src/components/layout/Block' import Divider from 'src/components/Divider' -import GasEstimationInfo from '../GasEstimationInfo' import { ConfirmTxModalProps, DecodedTxDetail } from '.' import Hairline from 'src/components/layout/Hairline' const { nativeCoin } = getNetworkInfo() -const StyledTextBox = styled(TextBox)` - max-width: 444px; -` - const Container = styled.div` max-width: 480px; padding: ${md} ${lg} 0; @@ -90,8 +83,7 @@ export const ReviewConfirm = ({ onTxReject, areTxsMalformed, showDecodedTxData, -}: Props): React.ReactElement => { - const [estimatedSafeTxGas, setEstimatedSafeTxGas] = useState(0) +}: Props): ReactElement => { const isMultiSend = txs.length > 1 const [decodedData, setDecodedData] = useState(null) const dispatch = useDispatch() @@ -133,12 +125,6 @@ export const ReviewConfirm = ({ manualGasLimit, }) - useEffect(() => { - if (params?.safeTxGas) { - setEstimatedSafeTxGas(gasEstimation) - } - }, [params, gasEstimation]) - // Decode tx data. useEffect(() => { const decodeTxData = async () => { @@ -171,9 +157,7 @@ export const ReviewConfirm = ({ origin: app.id, navigateToTransactionsTab: false, txNonce: txParameters.safeNonce, - safeTxGas: txParameters.safeTxGas - ? Number(txParameters.safeTxGas) - : Math.max(params?.safeTxGas || 0, estimatedSafeTxGas), + safeTxGas: txParameters.safeTxGas ? Number(txParameters.safeTxGas) : undefined, ethParameters: txParameters, notifiedTransaction: TX_NOTIFICATION_TYPES.STANDARD_TX, }, @@ -206,7 +190,7 @@ export const ReviewConfirm = ({ {!isMultiSend && } - {/* Warning gas estimation */} - {params?.safeTxGas && ( -
- SafeTxGas - {params?.safeTxGas} - -
- )} {/* Tx Parameters */} { - if (loading) { - return

Checking transaction parameters...

- } - - let content: React.ReactElement | null = null - if (appEstimation >= internalEstimation) { - content = ( - <> - Success Gas estimation is OK - - ) - } - - if (internalEstimation === 0) { - content = ( - <> - Warning Error while estimating gas. The transaction may fail. - - ) - } - - return {content} -} - -export default GasEstimationInfo