fix(anvil): Port Reth's gas estimation and fix wrong executor error

anvil/src/eth/api.rs

@@ -2022,12 +2022,29 @@ impl EthApi {
         call_to_estimate.gas = Some(gas_limit);
 
         // execute the call without writing to db
-        let (exit, out, gas, _) = self.backend.call_with_state(
-            &state,
-            call_to_estimate,
-            fees.clone(),
-            block_env.clone(),
-        )?;
+        let ethres =
+            self.backend.call_with_state(&state, call_to_estimate, fees.clone(), block_env.clone());
+
+        // Exceptional case: init used too much gas, we need to increase the gas limit and try
+        // again
+        if let Err(BlockchainError::InvalidTransaction(InvalidTransactionError::GasTooHigh)) =
+            ethres
+        {
+            // if price or limit was included in the request then we can execute the request
+            // again with the block's gas limit to check if revert is gas related or not
+            if request.gas.is_some() || request.gas_price.is_some() {
+                return Err(map_out_of_gas_err(
+                    request,
+                    state,
+                    self.backend.clone(),
+                    block_env,
+                    fees,
+                    gas_limit,
+                ))
+            }
+        }
+
+        let (exit, out, gas, _) = ethres?;
         match exit {
             return_ok!() => {
                 // succeeded
@@ -2040,23 +2057,14 @@ impl EthApi {
                 // if price or limit was included in the request then we can execute the request
                 // again with the max gas limit to check if revert is gas related or not
                 return if request.gas.is_some() || request.gas_price.is_some() {
-                    request.gas = Some(self.backend.gas_limit());
-                    let (exit, out, _, _) =
-                        self.backend.call_with_state(&state, request, fees, block_env)?;
-                    match exit {
-                        return_ok!() => {
-                            // transaction succeeded by manually increasing the gas limit to highest
-                            Err(InvalidTransactionError::BasicOutOfGas(gas_limit).into())
-                        }
-                        return_revert!() => {
-                            Err(InvalidTransactionError::Revert(Some(convert_transact_out(&out)))
-                                .into())
-                        }
-                        reason => {
-                            warn!(target: "node", "estimation failed due to {:?}", reason);
-                            Err(BlockchainError::EvmError(reason))
-                        }
-                    }
+                    Err(map_out_of_gas_err(
+                        request,
+                        state,
+                        self.backend.clone(),
+                        block_env,
+                        fees,
+                        gas_limit,
+                    ))
                 } else {
                     // the transaction did fail due to lack of gas from the user
                     Err(InvalidTransactionError::Revert(Some(convert_transact_out(&out))).into())
@@ -2068,72 +2076,71 @@ impl EthApi {
             }
         }
 
-        // at this point we know the call succeeded but want to find the best gas, so we do a binary
-        // search over the possible range
-
+        // at this point we know the call succeeded but want to find the _best_ (lowest) gas the
+        // transaction succeeds with. we find this by doing a binary search over the
+        // possible range NOTE: this is the gas the transaction used, which is less than the
+        // transaction requires to succeed
         let gas: U256 = gas.into();
         let mut lowest_gas_limit = MIN_TRANSACTION_GAS;
 
         // pick a point that's close to the estimated gas
         let mut mid_gas_limit = std::cmp::min(gas * 3, (highest_gas_limit + lowest_gas_limit) / 2);
 
-        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);
-            match self.backend.call_with_state(
+            let ethres = self.backend.call_with_state(
                 &state,
                 request.clone(),
                 fees.clone(),
                 block_env.clone(),
-            ) {
-                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))
-                        }
+            );
+
+            // Exceptional case: init used too much gas, we need to increase the gas limit and try
+            // again
+            if let Err(BlockchainError::InvalidTransaction(InvalidTransactionError::GasTooHigh)) =
+                ethres
+            {
+                // increase the lowest gas limit
+                lowest_gas_limit = mid_gas_limit;
+
+                // new midpoint
+                mid_gas_limit = (highest_gas_limit + lowest_gas_limit) / 2;
+                continue
+            }
+
+            match ethres {
+                Ok((exit, _, _gas, _)) => match exit {
+                    // If the transaction succeeded, we can set a ceiling for the highest gas limit
+                    // at the current midpoint, as spending any more gas would
+                    // make no sense (as the TX would still succeed).
+                    return_ok!() => {
+                        highest_gas_limit = mid_gas_limit;
                     }
-                    // new midpoint
-                    mid_gas_limit = (highest_gas_limit + lowest_gas_limit) / 2;
-                }
+                    // If the transaction failed due to lack of gas, we can set a floor for the
+                    // lowest gas limit at the current midpoint, as spending any
+                    // less gas would make no sense (as the TX would still revert due to lack of
+                    // gas).
+                    InstructionResult::Revert |
+                    InstructionResult::OutOfGas |
+                    InstructionResult::OutOfFund => {
+                        lowest_gas_limit = mid_gas_limit;
+                    }
+                    // The tx failed for some other reason.
+                    reason => {
+                        warn!(target: "node", "estimation failed due to {:?}", reason);
+                        return Err(BlockchainError::EvmError(reason))
+                    }
+                },
+                // We've already checked for the exceptional GasTooHigh case above, so this is a
+                // real error.
                 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;
+                    warn!(target: "node", "estimation failed due to {:?}", reason);
+                    return Err(reason)
                 }
-            };
+            }
+            // new midpoint
+            mid_gas_limit = (highest_gas_limit + lowest_gas_limit) / 2;
         }
 
         trace!(target : "node", "Estimated Gas for call {:?}", highest_gas_limit);
@@ -2379,3 +2386,39 @@ fn ensure_return_ok(exit: InstructionResult, out: &Option<Output>) -> Result<Byt
         reason => Err(BlockchainError::EvmError(reason)),
     }
 }
+
+/// Executes the requests again after an out of gas error to check if the error is gas related or
+/// not
+#[inline]
+fn map_out_of_gas_err<D>(
+    mut request: EthTransactionRequest,
+    state: D,
+    backend: Arc<backend::mem::Backend>,
+    block_env: BlockEnv,
+    fees: FeeDetails,
+    gas_limit: U256,
+) -> BlockchainError
+where
+    D: DatabaseRef<Error = DatabaseError>,
+{
+    request.gas = Some(backend.gas_limit());
+    let (exit, out, _, _) = match backend.call_with_state(&state, request, fees, block_env) {
+        Ok(res) => res,
+        Err(err) => return err,
+    };
+    match exit {
+        return_ok!() => {
+            // transaction succeeded by manually increasing the gas limit to
+            // highest, which means the caller lacks funds to pay for the tx
+            InvalidTransactionError::BasicOutOfGas(gas_limit).into()
+        }
+        return_revert!() => {
+            // reverted again after bumping the limit
+            InvalidTransactionError::Revert(Some(convert_transact_out(&out))).into()
+        }
+        reason => {
+            warn!(target: "node", "estimation failed due to {:?}", reason);
+            BlockchainError::EvmError(reason)
+        }
+    }
+}

anvil/src/eth/backend/executor.rs

@@ -18,7 +18,7 @@ use ethers::{
     utils::rlp,
 };
 use forge::{
-    revm::primitives::ExecutionResult,
+    revm::primitives::{EVMError, ExecutionResult},
     utils::{
         b160_to_h160, eval_to_instruction_result, h160_to_b160, halt_to_instruction_result,
         ru256_to_u256,
@@ -33,7 +33,7 @@ use foundry_evm::{
     },
     trace::{node::CallTraceNode, CallTraceArena},
 };
-use std::{str::FromStr, sync::Arc};
+use std::sync::Arc;
 use tracing::{trace, warn};
 
 /// Represents an executed transaction (transacted on the DB)
@@ -271,10 +271,19 @@ impl<'a, 'b, DB: Db + ?Sized, Validator: TransactionValidator> Iterator
             Ok(exec_result) => exec_result,
             Err(err) => {
                 warn!(target: "backend", "[{:?}] failed to execute: {:?}", transaction.hash(), err);
-                return Some(TransactionExecutionOutcome::DatabaseError(
-                    transaction,
-                    DatabaseError::TransactionNotFound(H256::from_str("0x").unwrap()),
-                ))
+                match err {
+                    EVMError::Database(err) => {
+                        return Some(TransactionExecutionOutcome::DatabaseError(transaction, err))
+                    }
+                    EVMError::Transaction(err) => {
+                        return Some(TransactionExecutionOutcome::Invalid(transaction, err.into()))
+                    }
+                    // This will correspond to prevrandao not set, and it should never happen.
+                    // If it does, it's a bug.
+                    e => {
+                        panic!("Failed to execute transaction. This is a bug.\n {:?}", e)
+                    }
+                }
             }
         };
         inspector.print_logs();

cli/tests/fixtures/can_create_using_unlocked.stdout

@@ -3,4 +3,4 @@ Solc 0.8.15 finished in 1.95s
 Compiler run successful
 Deployer: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
 Deployed to: 0x5FbDB2315678afecb367f032d93F642f64180aa3
-Transaction hash: 0xd6e14926926a3bf1f51ccd13ccb190887a82d0ba7d5f3f0af7928faa22c4d3ec
+Transaction hash: 0x4c3d9f7c4cc26876b43a11ba7ff218374471786a8ae8bf5574deb1d97fc1e851