fix: gas estimation does not return revert when a previous iteration succeeded (#113)

* fix: gas estimation does not return revert when a previous iteration succeeded

* chore: bump version

Author: prestwich

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "trevm"
-version = "0.23.7"
+version = "0.23.8"
 rust-version = "1.83.0"
 edition = "2021"
 authors = ["init4"]

src/est.rs

@@ -263,6 +263,7 @@ impl EstimationResult {
                 if matches!(reason, HaltReason::OutOfGas(_) | HaltReason::InvalidFEOpcode) {
                     range.set_min(*limit);
                 } else {
+                    // NB: can't clone here as this is a const fn.
                     return Err(Self::Halt { limit: *limit, reason: *reason, gas_used: *gas_used });
                 }
             }

src/evm.rs

@@ -1729,6 +1729,8 @@ where
         self,
         filler: &crate::fillers::GasEstimationFiller,
     ) -> Result<(crate::EstimationResult, Self), EvmErrored<Db, Insp>> {
+        use tracing::trace;
+
         let mut estimation = std::mem::MaybeUninit::uninit();
 
         let this = self.try_with_estimate_gas_filler(filler, |this| match this.run() {
@@ -1744,6 +1746,7 @@ where
         // SAFETY: if we did not shortcut return, then estimation was
         // definitely written
         Ok((unsafe { estimation.assume_init() }, this))
+            .inspect(|(est, _)| trace!(?est, "gas estimation result",))
     }
 
     /// Implements gas estimation. This will output an estimate of the minimum
@@ -1848,7 +1851,10 @@ where
         trace!(%estimate, "optimistic estimate succeeded");
 
         // Now we know that it succeeds at _some_ gas limit. We can now binary
-        // search.
+        // search. We start by recording the initial best estimate. We'll update
+        // this best-estimate as we go inside the `estimate_and_adjust` macro
+        // invocations.
+        let mut best = estimate.clone();
         let mut gas_used = estimate.gas_used();
         let gas_refunded = estimate.gas_refunded().expect("checked is_failure");
 
@@ -1868,7 +1874,7 @@ where
 
         // If the first search is outside the range, we don't need to try it.
         if search_range.contains(needle) {
-            estimate_and_adjust!(estimate, trevm, needle, search_range);
+            estimate_and_adjust!(best, estimate, trevm, needle, search_range);
             // NB: `estimate` is rebound in the macro, so do not move this line
             // up.
             gas_used = estimate.gas_used();
@@ -1885,11 +1891,11 @@ where
         // highest gas limit)
         // <https://github.com/ethereum/go-ethereum/blob/a5a4fa7032bb248f5a7c40f4e8df2b131c4186
         while search_range.size() > 1 && search_range.ratio() > 0.015 {
-            estimate_and_adjust!(estimate, trevm, needle, search_range);
+            estimate_and_adjust!(best, estimate, trevm, needle, search_range);
             needle = search_range.midpoint();
         }
 
-        Ok((estimate, trevm))
+        Ok((best, trevm))
     }
 }
 

src/macros.rs

@@ -42,15 +42,18 @@ macro_rules! trevm_bail {
 /// Macro for gas estimation binary search loop.
 #[cfg(feature = "estimate_gas")]
 macro_rules! estimate_and_adjust {
-    ($est:ident, $trevm:ident, $gas_limit:ident, $range:ident) => {
+    ($best:ident, $est:ident, $trevm:ident, $gas_limit:ident, $range:ident) => {
         ::tracing::trace!(
-            estimate = %$est,
+            best = $best.gas_used(),
             max = %$range.max(),
             min = %$range.min(),
-            "running gas estimate call"
+            needle = $gas_limit,
+            "running gas estimation"
         );
-
         ($est, $trevm) = $trevm.run_estimate(&$gas_limit.into())?;
+        if $est.is_success() {
+            $best = $est.clone();
+        }
         if let Err(e) = $est.adjust_binary_search_range(&mut $range) {
             ::tracing::trace!(
                 %e,