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

anvil/src/eth/api.rs

@@ -2,7 +2,9 @@ use crate::{
     eth::{
         backend,
         backend::{
-            db::SerializableState, mem::MIN_TRANSACTION_GAS, notifications::NewBlockNotifications,
+            db::SerializableState,
+            mem::{MIN_CREATE_GAS, MIN_TRANSACTION_GAS},
+            notifications::NewBlockNotifications,
             validate::TransactionValidator,
         },
         error::{
@@ -32,8 +34,8 @@ use anvil_core::{
         proof::AccountProof,
         state::StateOverride,
         transaction::{
-            EthTransactionRequest, LegacyTransaction, PendingTransaction, TypedTransaction,
-            TypedTransactionRequest,
+            EthTransactionRequest, LegacyTransaction, PendingTransaction, TransactionKind,
+            TypedTransaction, TypedTransactionRequest,
         },
         EthRequest,
     },
@@ -2022,12 +2024,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 +2059,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 +2078,72 @@ 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;
+        // Get the starting lowest gas needed depending on the transaction kind.
+        let mut lowest_gas_limit = determine_base_gas_by_kind(request.clone());
 
         // 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 +2389,63 @@ 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)
+        }
+    }
+}
+
+/// Determines the minimum gas needed for a transaction depending on the transaction kind.
+#[inline]
+fn determine_base_gas_by_kind(request: EthTransactionRequest) -> U256 {
+    match request.into_typed_request() {
+        Some(request) => match request {
+            TypedTransactionRequest::Legacy(req) => match req.kind {
+                TransactionKind::Call(_) => MIN_TRANSACTION_GAS,
+                TransactionKind::Create => MIN_CREATE_GAS,
+            },
+            TypedTransactionRequest::EIP1559(req) => match req.kind {
+                TransactionKind::Call(_) => MIN_TRANSACTION_GAS,
+                TransactionKind::Create => MIN_CREATE_GAS,
+            },
+            TypedTransactionRequest::EIP2930(req) => match req.kind {
+                TransactionKind::Call(_) => MIN_TRANSACTION_GAS,
+                TransactionKind::Create => MIN_CREATE_GAS,
+            },
+        },
+        // Tighten the gas limit upwards if we don't know the transaction type to avoid deployments
+        // failing.
+        _ => MIN_CREATE_GAS,
+    }
+}

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();

anvil/src/eth/backend/mem/mod.rs

@@ -94,6 +94,8 @@ pub mod storage;
 
 // Gas per transaction not creating a contract.
 pub const MIN_TRANSACTION_GAS: U256 = U256([21_000, 0, 0, 0]);
+// Gas per transaction creating a contract.
+pub const MIN_CREATE_GAS: U256 = U256([53_000, 0, 0, 0]);
 
 pub type State = foundry_evm::HashMap<Address, Account>;
 

cli/tests/fixtures/can_create_template_contract-2nd.stdout

@@ -1,4 +1,4 @@
 No files changed, compilation skipped
 Deployer: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
 Deployed to: 0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512
-Transaction hash: 0xe4fffb8145d57d12e70bcb9586e2c714592dfd66811a87af92e15638fa027322
+Transaction hash: 0x3d78b08c411f05d5e79adc92a4c814e0f818d1a09c111b0ab688270f35a07ae7

cli/tests/fixtures/can_create_template_contract.stdout

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

cli/tests/fixtures/can_create_using_unlocked-2nd.stdout

@@ -1,4 +1,4 @@
 No files changed, compilation skipped
 Deployer: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
 Deployed to: 0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512
-Transaction hash: 0xe4fffb8145d57d12e70bcb9586e2c714592dfd66811a87af92e15638fa027322
+Transaction hash: 0x3d78b08c411f05d5e79adc92a4c814e0f818d1a09c111b0ab688270f35a07ae7

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