Fix gas estimation (#1959)

* Update gas estimation to handle extra data for execution

* Tweak safe gas calculation

* Optimize send ETH transaction estimation

src/logic/hooks/useEstimateTransactionGas.tsx

@@ -4,8 +4,8 @@ import {
   estimateGasForTransactionApproval,
   estimateGasForTransactionCreation,
   estimateGasForTransactionExecution,
-  MINIMUM_TRANSACTION_GAS,
+  getFixedGasCosts,
-  GAS_REQUIRED_PER_SIGNATURE,
+  SAFE_TX_GAS_DATA_COST,
 } from 'src/logic/safe/transactions/gas'
 import { fromTokenUnit } from 'src/logic/tokens/utils/humanReadableValue'
 import { formatAmount } from 'src/logic/tokens/utils/formatAmount'
@@ -213,6 +213,8 @@ export const useEstimateTransactionGas = ({
         preApprovingOwner,
       )
 
+      const fixedGasCosts = getFixedGasCosts(Number(threshold))
+
       try {
         const isOffChainSignature = checkIfOffChainSignatureIsPossible(isExecution, smartContractWallet, safeVersion)
 
@@ -233,10 +235,10 @@ export const useEstimateTransactionGas = ({
 
         const gasPrice = manualGasPrice ? web3.utils.toWei(manualGasPrice, 'gwei') : await calculateGasPrice()
         const gasPriceFormatted = web3.utils.fromWei(gasPrice, 'gwei')
-        const estimatedGasCosts = gasEstimation * parseInt(gasPrice, 10)
+        const estimatedGasCosts = (gasEstimation + fixedGasCosts) * parseInt(gasPrice, 10)
         const gasCost = fromTokenUnit(estimatedGasCosts, nativeCoin.decimals)
         const gasCostFormatted = formatAmount(gasCost)
-        const gasLimit = (gasEstimation * 2).toString()
+        const gasLimit = ((gasEstimation + fixedGasCosts) * 2).toString()
 
         let txEstimationExecutionStatus = EstimationStatus.SUCCESS
 
@@ -259,7 +261,7 @@ 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 = MINIMUM_TRANSACTION_GAS + (threshold || 1) * GAS_REQUIRED_PER_SIGNATURE
+        const gasEstimation = fixedGasCosts + SAFE_TX_GAS_DATA_COST
         const gasCost = fromTokenUnit(gasEstimation, nativeCoin.decimals)
         const gasCostFormatted = formatAmount(gasCost)
         setGasEstimation({

src/logic/safe/store/actions/createTransaction.ts

@@ -84,10 +84,11 @@ export const createTransaction = (
 
   const isExecution = await shouldExecuteTransaction(safeInstance, nonce, lastTx)
   const safeVersion = await getCurrentSafeVersion(safeInstance)
-  let safeTxGas
+  let safeTxGas = safeTxGasArg || 0
   try {
-    safeTxGas =
+    if (safeTxGasArg === undefined) {
-      safeTxGasArg || (await estimateGasForTransactionCreation(safeAddress, txData, to, valueInWei, operation))
+      safeTxGas = await estimateGasForTransactionCreation(safeAddress, txData, to, valueInWei, operation)
+    }
   } catch (error) {
     safeTxGas = safeTxGasArg || 0
   }

src/logic/safe/transactions/gas.ts

@@ -15,6 +15,10 @@ import { sameString } from 'src/utils/strings'
 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 => {
@@ -151,7 +155,7 @@ const estimateGasWithRPCCall = async (txConfig: {
 
     const { error } = data
     if (error?.data) {
-      return new BigNumber(data.error.data.substring(138), 16).toNumber()
+      return new BigNumber(error.data.substring(138), 16).toNumber()
     }
   } catch (error) {
     console.log('Gas estimation endpoint errored: ', error.message)
@@ -178,33 +182,41 @@ const calculateMinimumGasForTransaction = async (
   additionalGasBatches: number[],
   safeAddress: string,
   estimateData: string,
-  txGasEstimation: number,
+  safeTxGasEstimation: number,
-  dataGasEstimation: number,
   fixedGasCosts: number,
 ): Promise<number> => {
   for (const additionalGas of additionalGasBatches) {
-    const amountOfGasToTryTx = txGasEstimation + dataGasEstimation + fixedGasCosts + additionalGas
+    const batchedSafeTxGas = safeTxGasEstimation + additionalGas
-    console.info(`Estimating transaction creation with gas amount: ${amountOfGasToTryTx}`)
+    // 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: amountOfGasToTryTx,
+        gas: gasLimit,
       })
       if (estimation > 0) {
-        console.info(`Gas estimation successfully finished with gas amount: ${amountOfGasToTryTx}`)
+        console.info(`Gas estimation successfully finished with gas amount: ${batchedSafeTxGas}`)
-        return amountOfGasToTryTx
+        return batchedSafeTxGas
       }
     } catch (error) {
-      console.log(`Error trying to estimate gas with amount: ${amountOfGasToTryTx}`)
+      console.log(`Error trying to estimate gas with amount: ${batchedSafeTxGas}`)
     }
   }
 
   return 0
 }
 
+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,
@@ -217,32 +229,35 @@ export const estimateGasForTransactionCreation = async (
     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 : undefined,
+      gas: safeTxGas ? safeTxGas + fixedGasCosts : undefined,
     })
 
     if (safeTxGas) {
-      return gasEstimationResponse
+      // 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)
+      // We return set safeTxGas
+      return safeTxGas
     }
 
-    const threshold = await safeInstance.methods.getThreshold().call()
-
     const dataGasEstimation = parseRequiredTxGasResponse(estimateData)
-    // We add the minimum required gas for a transaction
+    // Adding this values we should get the full safeTxGas value
-    // TODO: This fix will be more accurate when we have a service for estimation.
+    const safeTxGasEstimation = gasEstimationResponse + dataGasEstimation + SAFE_TX_GAS_DATA_COST
-    // This fix takes the safe threshold and multiplies it by GAS_REQUIRED_PER_SIGNATURE.
+    // We will add gas batches in case is not enough
-    const fixedGasCosts = MINIMUM_TRANSACTION_GAS + (Number(threshold) || 1) * GAS_REQUIRED_PER_SIGNATURE
     const additionalGasBatches = [0, 10000, 20000, 40000, 80000, 160000, 320000, 640000, 1280000, 2560000, 5120000]
 
     return await calculateMinimumGasForTransaction(
       additionalGasBatches,
       safeAddress,
       estimateData,
-      gasEstimationResponse,
+      safeTxGasEstimation,
-      dataGasEstimation,
       fixedGasCosts,
     )
   } catch (error) {
@@ -290,6 +305,7 @@ export const estimateGasForTransactionExecution = async ({
         txRecipient,
         txAmount,
         operation,
+        safeTxGas,
       )
       console.info(`Gas estimation successfully finished with gas amount: ${gasEstimation}`)
       return gasEstimation

src/routes/safe/components/Balances/SendModal/screens/ReviewSendFundsTx/index.tsx

@@ -114,6 +114,7 @@ const ReviewSendFundsTx = ({ onClose, onPrev, tx }: ReviewTxProps): React.ReactE
     txData: data,
     txRecipient,
     txType: tx.txType,
+    txAmount: txValue,
     safeTxGas: manualSafeTxGas,
     manualGasPrice,
   })