(Feature) #1515 Gas estimation for NON-GETH nodes (#1523)

* Remove unnecesary await

* Implement gas calculation for NON-GETH nodes

* Add tests

* Refactor estimateSafeTxGas: now getGasEstimationTxResponse calculates gas or throws errors based on the current node

* Refactor getOpenEthereumErrorDataResult to make it works with Nethermind
Updates tests

Co-authored-by: Daniel Sanchez <daniel.sanchez@gnosis.pm>

.travis.yml

@@ -27,6 +27,7 @@ matrix:
         - REACT_APP_NETWORK='volta'
         - REACT_APP_GOOGLE_ANALYTICS=${REACT_APP_GOOGLE_ANALYTICS_ID_VOLTA}
         - STAGING_BUCKET_NAME=${STAGING_VOLTA_BUCKET_NAME}
+      if: (branch = master AND NOT type = pull_request) OR tag IS present
     - env:
         - REACT_APP_NETWORK='energy_web_chain'
         - REACT_APP_GOOGLE_ANALYTICS=${REACT_APP_GOOGLE_ANALYTICS_ID_EWC}

src/logic/safe/transactions/__tests__/gas.test.ts

@@ -0,0 +1,68 @@
+import { getNonGETHErrorDataResult } from 'src/logic/safe/transactions/gas'
+
+describe('getOpenEthereumErrorDataResult', () => {
+  it(`should return data hash from given OpenEthereum response`, () => {
+    // given
+    const resultExpected =
+      '0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000006457'
+    const openEthResponse =
+      'Internal JSON-RPC error.\n' +
+      '{\n' +
+      '  "code": -32015,\n' +
+      '  "message": "VM execution error.",\n' +
+      '  "data": "Reverted 0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000006457"\n' +
+      '}'
+
+    // when
+    const result = getNonGETHErrorDataResult(openEthResponse)
+
+    // then
+    expect(result).toBe(resultExpected)
+  })
+  it(`should return undefined from empty OpenEthereum response`, () => {
+    // given
+    const resultExpected = undefined
+    const openEthResponse = ''
+
+    // when
+    const result = getNonGETHErrorDataResult(openEthResponse)
+
+    // then
+    expect(result).toBe(resultExpected)
+  })
+  it(`should return undefined from wrong OpenEthereum response`, () => {
+    // given
+    const resultExpected = undefined
+    const openEthResponse =
+      'Internal JSON-RPC error.\n' +
+      '{\n' +
+      '  "code": -32015,\n' +
+      '  "message": "VM execution error.",\n' +
+      '  "data": "Reverted-test0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000006457"\n' +
+      '}'
+
+    // when
+    const result = getNonGETHErrorDataResult(openEthResponse)
+
+    // then
+    expect(result).toBe(resultExpected)
+  })
+  it(`should return data hash from given Nethermind response`, () => {
+    // given
+    const resultExpected =
+      '0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000006457'
+    const openEthResponse =
+      'Internal JSON-RPC error.\n' +
+      '{\n' +
+      '  "code": -32015,\n' +
+      '  "message": "VM execution error.",\n' +
+      '  "data": "revert 0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000006457"\n' +
+      '}'
+
+    // when
+    const result = getNonGETHErrorDataResult(openEthResponse)
+
+    // then
+    expect(result).toBe(resultExpected)
+  })
+})

src/logic/safe/transactions/gas.ts

@@ -11,6 +11,7 @@ import { ZERO_ADDRESS } from 'src/logic/wallets/ethAddresses'
 import { EMPTY_DATA, calculateGasOf, calculateGasPrice } from 'src/logic/wallets/ethTransactions'
 import { getAccountFrom, getWeb3 } from 'src/logic/wallets/getWeb3'
 import { GnosisSafe } from 'src/types/contracts/GnosisSafe.d'
+import { sameString } from 'src/utils/strings'
 
 const estimateDataGasCosts = (data: string): number => {
   const reducer = (accumulator, currentValue) => {
@@ -54,7 +55,7 @@ export const estimateTxGasCosts = async (
       const signatures = tx?.confirmations
         ? generateSignaturesFromTxConfirmations(tx.confirmations, preApprovingOwner)
         : `0x000000000000000000000000${from.replace(
-            '0x',
+            EMPTY_DATA,
             '',
           )}000000000000000000000000000000000000000000000000000000000000000001`
       txData = await safeInstance.methods
@@ -92,6 +93,84 @@ export const estimateTxGasCosts = async (
   }
 }
 
+// Parses the result of OpenEthereum/Parity and Nethermind error messages and returns the value
+export const getNonGETHErrorDataResult = (errorMessage: string): string | undefined => {
+  // Extracts JSON object from the error message
+  const [, ...error] = errorMessage.split('\n')
+  try {
+    const errorAsJSON = JSON.parse(error.join(''))
+
+    if (errorAsJSON?.data) {
+      const [, dataResult] = errorAsJSON.data.split(' ')
+      return dataResult
+    }
+  } catch (error) {
+    console.error(`Error trying to extract data from openEthereum/Nethermind error message: ${errorMessage}`)
+  }
+}
+
+const getGasEstimationTxResponse = async (txConfig: {
+  to: string
+  from: string
+  data: string
+  gasPrice?: number
+  gas?: number
+}): Promise<number> => {
+  const web3 = getWeb3()
+  try {
+    const result = await web3.eth.call(txConfig)
+
+    // GETH Nodes
+    // In case that the gas is not enough we will receive an EMPTY data
+    // Otherwise we will receive the gas amount as hash data
+
+    if (!sameString(result, EMPTY_DATA)) {
+      return new BigNumber(result.substring(138), 16).toNumber()
+    }
+  } catch (error) {
+    // OpenEthereum/Parity nodes
+    // Parity/OpenEthereum nodes always returns the response as an error
+    // So we try to extract the estimation result within the error in case is possible
+    const estimationData = getNonGETHErrorDataResult(error.message)
+
+    if (!estimationData || sameString(estimationData, EMPTY_DATA)) {
+      throw error
+    }
+
+    return new BigNumber(estimationData.substring(138), 16).toNumber()
+  }
+
+  // This will fail in case that we receive an EMPTY_DATA on the GETH node gas estimation
+  // We cannot throw this error above because it will be captured again on the OpenEthereum code bellow
+  throw new Error('Error while estimating the gas required for tx')
+}
+
+const calculateMinimumGasForTransaction = async (
+  additionalGasBatches: number[],
+  safeAddress: string,
+  estimateData: string,
+  txGasEstimation: number,
+  dataGasEstimation: number,
+): Promise<number> => {
+  for (const additionalGas of additionalGasBatches) {
+    const amountOfGasToTryTx = txGasEstimation + dataGasEstimation + additionalGas
+    try {
+      await getGasEstimationTxResponse({
+        to: safeAddress,
+        from: safeAddress,
+        data: estimateData,
+        gasPrice: 0,
+        gas: amountOfGasToTryTx,
+      })
+      return txGasEstimation + additionalGas
+    } catch (error) {
+      console.log(`Error trying to estimate gas with amount: ${amountOfGasToTryTx}`)
+    }
+  }
+
+  return 0
+}
+
 export const estimateSafeTxGas = async (
   safe: GnosisSafe | undefined,
   safeAddress: string,
@@ -106,63 +185,26 @@ export const estimateSafeTxGas = async (
       safeInstance = await getGnosisSafeInstanceAt(safeAddress)
     }
 
-    const web3 = await getWeb3()
     const estimateData = safeInstance.methods.requiredTxGas(to, valueInWei, data, operation).encodeABI()
-    const estimateResponse = await web3.eth.call({
+    const gasEstimationResponse = await getGasEstimationTxResponse({
       to: safeAddress,
       from: safeAddress,
       data: estimateData,
     })
-    const txGasEstimation = new BigNumber(estimateResponse.substring(138), 16).toNumber() + 10000
+
+    const txGasEstimation = gasEstimationResponse + 10000
 
     // 21000 - additional gas costs (e.g. base tx costs, transfer costs)
     const dataGasEstimation = estimateDataGasCosts(estimateData) + 21000
-    const additionalGasBatches = [10000, 20000, 40000, 80000, 160000, 320000, 640000, 1280000, 2560000, 5120000]
+    const additionalGasBatches = [0, 10000, 20000, 40000, 80000, 160000, 320000, 640000, 1280000, 2560000, 5120000]
 
-    const batch = new web3.BatchRequest()
+    return await calculateMinimumGasForTransaction(
-    const estimationRequests = additionalGasBatches.map(
+      additionalGasBatches,
-      (additionalGas) =>
+      safeAddress,
-        new Promise((resolve) => {
+      estimateData,
-          // there are no type definitions for .request, so for now ts-ignore is there
+      txGasEstimation,
-          // Issue link: https://github.com/ethereum/web3.js/issues/3144
+      dataGasEstimation,
-          // eslint-disable-next-line
-          // @ts-ignore
-          const request = web3.eth.call.request(
-            {
-              to: safeAddress,
-              from: safeAddress,
-              data: estimateData,
-              gasPrice: 0,
-              gasLimit: txGasEstimation + dataGasEstimation + additionalGas,
-            },
-            (error, res) => {
-              // res.data check is for OpenEthereum/Parity revert messages format
-              const isOpenEthereumRevertMsg = res && typeof res.data === 'string'
-
-              const isEstimationSuccessful =
-                !error &&
-                ((typeof res === 'string' && res !== '0x') || (isOpenEthereumRevertMsg && res.data.slice(9) !== '0x'))
-
-              resolve({
-                success: isEstimationSuccessful,
-                estimation: txGasEstimation + additionalGas,
-              })
-            },
-          )
-
-          batch.add(request)
-        }),
     )
-    batch.execute()
-
-    const estimationResponses = await Promise.all(estimationRequests)
-    const firstSuccessfulRequest: any = estimationResponses.find((res: any) => res.success)
-
-    if (firstSuccessfulRequest) {
-      return firstSuccessfulRequest.estimation
-    }
-
-    return 0
   } catch (error) {
     console.error('Error calculating tx gas estimation', error)
     return 0

src/logic/safe/transactions/offchainSigner/ethSigner.ts

@@ -10,7 +10,7 @@ type EthSignerArgs = {
 }
 
 export const ethSigner = async ({ safeTxHash, sender }: EthSignerArgs): Promise<string> => {
-  const web3 = await getWeb3()
+  const web3 = getWeb3()
 
   return new Promise(function (resolve, reject) {
     const provider = web3.currentProvider as AbstractProvider