From 6ed1e2ea18da5bc8976930171e5818068dcbe38b Mon Sep 17 00:00:00 2001
From: Agustin Pane <agustin.pane@gmail.com>
Date: Fri, 20 Nov 2020 13:56:29 -0300
Subject: [PATCH] (Fix) Geth nodes gas estimation errors (#1635)

* Replaces getNonGETHErrorDataResult with getDataFromNodeErrorMessage
Updates the extraction of data from the error message to include new GETH nodes version

* Add tests for new GETH nodes
---
 .../safe/transactions/__tests__/gas.test.ts   | Bin 2797 -> 6395 bytes
 src/logic/safe/transactions/gas.ts            |  39 +++++++++++++-----
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/logic/safe/transactions/__tests__/gas.test.ts b/src/logic/safe/transactions/__tests__/gas.test.ts
index 589ae4d3cade1add21aa0528080cdd1b84f966d6..4c6982407a5c9bc130212ea01333fbc1e8e43ae7 100644
GIT binary patch
delta 514
zcmaDW`rA+?Gq)hWs6?S!Aw9LkC9x#Yttda&FFz&KwWuh+$TzjPI59nSqf!L31TM+V
z6PP&}amr4fz?{EXj#Y>WpNfRd%h(^`S21}RyXeN17jY}1pEFq8T|+!33$j>G{BR|l
zO92Ry^Ye;JKmnK$lv-Q><fJOtf+Z9b)Ke=`lS@l7^YavnQp-|{N>Wp-6d3S=wD<e>
zPTt5WCy-y1nVy-Km;;UqB`bx=`&cCyjVIq>l@Z2b>SS&<smV1=H<%R^YA63?GnU2`
QVH)YFlYQ9Tm}@n;04W`?UjP6A

delta 169
zcmexu_*PUYGq)hWs6?S!Aw9LkFF()SHN?ZUs3^b4C9xziD7CmWr(~l_1hW`+smTWf
j6DGIw^G`12n}tWo=5~HXCLHpc4+uKpG2Vx{R+9?={EI#%

diff --git a/src/logic/safe/transactions/gas.ts b/src/logic/safe/transactions/gas.ts
index d4e9353b..92982f12 100644
--- a/src/logic/safe/transactions/gas.ts
+++ b/src/logic/safe/transactions/gas.ts
@@ -93,19 +93,40 @@ 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 => {
+// Parses the result from the error message (GETH, OpenEthereum/Parity and Nethermind) and returns the data value
+export const getDataFromNodeErrorMessage = (errorMessage: string): string | undefined => {
   // Extracts JSON object from the error message
   const [, ...error] = errorMessage.split('\n')
   try {
     const errorAsJSON = JSON.parse(error.join(''))
 
+    // 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 openEthereum/Nethermind error message: ${errorMessage}`)
+    console.error(`Error trying to extract data from node error message: ${errorMessage}`)
   }
 }
 
@@ -120,18 +141,16 @@ const getGasEstimationTxResponse = async (txConfig: {
   try {
     const result = await web3.eth.call(txConfig)
 
-    // GETH Nodes
+    // 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
+    // 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) {
-    // 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)
+    const estimationData = getDataFromNodeErrorMessage(error.message)
 
     if (!estimationData || sameString(estimationData, EMPTY_DATA)) {
       throw error
@@ -140,8 +159,8 @@ const getGasEstimationTxResponse = async (txConfig: {
     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
+  // This will fail in case that we receive an EMPTY_DATA on the GETH node gas estimation (for version < v1.9.24 of geth nodes)
+  // We cannot throw this error above because it will be captured again on the catch block bellow
   throw new Error('Error while estimating the gas required for tx')
 }