From e15e33a07c0920189fc336391f538c3dad53da73 Mon Sep 17 00:00:00 2001
From: evalir <hi@enriqueortiz.dev>
Date: Mon, 1 May 2023 16:37:57 -0400
Subject: [PATCH] fix(anvil): properly estimate gas instead of bailing on
 `GasTooHigh` (#4861)

* fix: use appropiate GasTooHigh error instead of FatalExternalError

* feat: refactor gas estimation bsearch to treat gastoohigh properly

* chore: clippy happy
---
 anvil/src/eth/api.rs             | 71 +++++++++++++++++++++-----------
 anvil/src/eth/backend/mem/mod.rs |  4 +-
 forge/src/gas_report.rs          |  2 +-
 3 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/anvil/src/eth/api.rs b/anvil/src/eth/api.rs
index 744064a7e63f..fba7c95f4ccb 100644
--- a/anvil/src/eth/api.rs
+++ b/anvil/src/eth/api.rs
@@ -2076,40 +2076,61 @@ impl EthApi {
 
         let mut last_highest_gas_limit = highest_gas_limit;
 
+        // Binary search for the ideal gas limit
         while (highest_gas_limit - lowest_gas_limit) > U256::one() {
             request.gas = Some(mid_gas_limit);
-            let (exit, _, _gas, _) = self.backend.call_with_state(
+            match self.backend.call_with_state(
                 &state,
                 request.clone(),
                 fees.clone(),
                 block_env.clone(),
-            )?;
-            match exit {
-                return_ok!() => {
-                    highest_gas_limit = mid_gas_limit;
-                    // if last two successful estimations only vary by 10%, we consider this to
-                    // sufficiently accurate
-                    const ACCURACY: u64 = 10;
-                    if (last_highest_gas_limit - highest_gas_limit) * ACCURACY /
-                        last_highest_gas_limit <
-                        U256::one()
-                    {
-                        return Ok(highest_gas_limit)
+            ) {
+                Ok((exit, _, _gas, _)) => {
+                    match exit {
+                        return_ok!() => {
+                            highest_gas_limit = mid_gas_limit;
+                            // if last two successful estimations only vary by 10%, we consider this
+                            // to sufficiently accurate
+                            const ACCURACY: u64 = 10;
+                            if (last_highest_gas_limit - highest_gas_limit) * ACCURACY /
+                                last_highest_gas_limit <
+                                U256::one()
+                            {
+                                return Ok(highest_gas_limit)
+                            }
+                            last_highest_gas_limit = highest_gas_limit;
+                        }
+                        InstructionResult::Revert |
+                        InstructionResult::OutOfGas |
+                        InstructionResult::OutOfFund => {
+                            lowest_gas_limit = mid_gas_limit;
+                        }
+                        reason => {
+                            warn!(target: "node", "estimation failed due to {:?}", reason);
+                            return Err(BlockchainError::EvmError(reason))
+                        }
                     }
-                    last_highest_gas_limit = highest_gas_limit;
-                }
-                InstructionResult::Revert |
-                InstructionResult::OutOfGas |
-                InstructionResult::OutOfFund => {
-                    lowest_gas_limit = mid_gas_limit;
+                    // new midpoint
+                    mid_gas_limit = (highest_gas_limit + lowest_gas_limit) / 2;
                 }
-                reason => {
-                    warn!(target: "node", "estimation failed due to {:?}", reason);
-                    return Err(BlockchainError::EvmError(reason))
+                Err(reason) => {
+                    match reason {
+                        // We need to treat REVM reverting due to gas too high just like
+                        // revert/OOG/OOF (see above)
+                        BlockchainError::InvalidTransaction(
+                            InvalidTransactionError::GasTooHigh,
+                        ) => {
+                            lowest_gas_limit = mid_gas_limit;
+                        }
+                        _ => {
+                            warn!(target: "node", "estimation failed due to {:?}", reason);
+                            return Err(reason)
+                        }
+                    };
+                    // new midpoint
+                    mid_gas_limit = (highest_gas_limit + lowest_gas_limit) / 2;
                 }
-            }
-            // new midpoint
-            mid_gas_limit = (highest_gas_limit + lowest_gas_limit) / 2;
+            };
         }
 
         trace!(target : "node", "Estimated Gas for call {:?}", highest_gas_limit);
diff --git a/anvil/src/eth/backend/mem/mod.rs b/anvil/src/eth/backend/mem/mod.rs
index f10f13d93ebb..5bbe5419fd71 100644
--- a/anvil/src/eth/backend/mem/mod.rs
+++ b/anvil/src/eth/backend/mem/mod.rs
@@ -999,7 +999,9 @@ impl Backend {
         evm.database(state);
         let result_and_state = match evm.inspect_ref(&mut inspector) {
             Ok(result_and_state) => result_and_state,
-            Err(_) => return Err(BlockchainError::EvmError(InstructionResult::FatalExternalError)),
+            Err(_) => {
+                return Err(BlockchainError::InvalidTransaction(InvalidTransactionError::GasTooHigh))
+            }
         };
         let state = result_and_state.state;
         let state: hashbrown::HashMap<H160, Account> =
diff --git a/forge/src/gas_report.rs b/forge/src/gas_report.rs
index 817ce35fcd06..9ac5eae695e2 100644
--- a/forge/src/gas_report.rs
+++ b/forge/src/gas_report.rs
@@ -69,7 +69,7 @@ impl GasReport {
                 (!self.ignore.contains(&contract_name) && self.report_for.is_empty()) ||
                 (self.report_for.contains(&contract_name));
             if report_contract {
-                let mut contract_report =
+                let contract_report =
                     self.contracts.entry(name.to_string()).or_insert_with(Default::default);
 
                 match &trace.data {