From 8a538efdcbe1291b0f003f401023e0a2fe69754a Mon Sep 17 00:00:00 2001 From: James Date: Wed, 18 Jun 2025 11:43:44 -0400 Subject: [PATCH 1/3] fix: estimation min set to gas_used - 1 rather than to estimate - 1 --- src/evm.rs | 14 +++++++++----- src/macros.rs | 7 ++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/evm.rs b/src/evm.rs index 99611a9..5dc7e9a 100644 --- a/src/evm.rs +++ b/src/evm.rs @@ -1799,7 +1799,7 @@ where /// [`MIN_TRANSACTION_GAS`]: crate::MIN_TRANSACTION_GAS #[cfg(feature = "estimate_gas")] pub fn estimate_gas(mut self) -> Result<(crate::EstimationResult, Self), EvmErrored> { - use tracing::{debug, enabled}; + use tracing::{debug, enabled, trace}; if let Some(est) = trevm_try!(self.estimate_gas_simple_transfer(), self) { return Ok((crate::EstimationResult::basic_transfer_success(est), self)); @@ -1814,13 +1814,15 @@ where crate::est::SearchRange::new(crate::MIN_TRANSACTION_GAS, initial_limit); let span = tracing::debug_span!( - "estimate_gas", + "Trevm::estimate_gas", start_min = search_range.min(), start_max = search_range.max(), - tx = "omitted", ); if enabled!(tracing::Level::TRACE) { span.record("tx", format!("{:?}", &self.tx())); + span.record("block", format!("{:?}", &self.block())); + } else { + span.record("tx", "omitted. Use TRACE for details"); } let _e = span.enter(); @@ -1834,7 +1836,7 @@ where // Run an estimate with the max gas limit. // NB: we declare these mut as we re-use the binding throughout the // function. - debug!(gas_limit = search_range.max(), "running optimistic estimate"); + debug!(gas_limit = self.gas_limit(), "running optimistic estimate"); let (mut estimate, mut trevm) = self.run_estimate(&search_range.max().into())?; // If it failed, no amount of gas is likely to work, so we shortcut @@ -1843,10 +1845,11 @@ where debug!(%estimate, "optimistic estimate failed"); return Ok((estimate, trevm)); } + trace!(%estimate, "optimistic estimate succeeded"); // Now we know that it succeeds at _some_ gas limit. We can now binary // search. - let mut gas_used = estimate.gas_estimation().expect("checked is_failure"); + let mut gas_used = estimate.gas_used(); let gas_refunded = estimate.gas_refunded().expect("checked is_failure"); // NB: if we've made it this far it's very unlikely that `gas_used` is @@ -1862,6 +1865,7 @@ where // frame can forward only 63/64 of the gas it has when it makes a new // frame. let mut needle = gas_used + gas_refunded + revm::interpreter::gas::CALL_STIPEND * 64 / 63; + // 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); diff --git a/src/macros.rs b/src/macros.rs index 27506fc..f4a15e1 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -46,12 +46,17 @@ macro_rules! estimate_and_adjust { ::tracing::trace!( estimate = %$est, gas_limit = $gas_limit, - range = %$range, + max = %$range.max(), + min = %$range.min(), "running gas estimate call" ); ($est, $trevm) = $trevm.run_estimate(&$gas_limit.into())?; if let Err(e) = $est.adjust_binary_search_range($gas_limit, &mut $range) { + ::tracing::trace!( + %e, + "error adjusting binary search range" + ); return Ok((e, $trevm)); } }; From 25ef0d2d5ecd61d98f962509e2b62c509283eba5 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 18 Jun 2025 11:51:01 -0400 Subject: [PATCH 2/3] chore: version bump --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d7be830..ce209e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "trevm" -version = "0.23.6" +version = "0.23.7" rust-version = "1.83.0" edition = "2021" authors = ["init4"] From ea6b2df62df06558e3af9912c7a7acd3dd895479 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 18 Jun 2025 12:01:25 -0400 Subject: [PATCH 3/3] fix: rename to limit --- src/est.rs | 50 +++++++++++++++++++++++++++----------------------- src/macros.rs | 3 +-- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/est.rs b/src/est.rs index 61e81e5..1217f66 100644 --- a/src/est.rs +++ b/src/est.rs @@ -95,8 +95,8 @@ impl SearchRange { pub enum EstimationResult { /// The estimation was successful, the result is the gas estimation. Success { - /// The gas estimation. - estimation: u64, + /// The input estimation limit. + limit: u64, /// The amount of gas that was refunded to the caller as unused. refund: u64, /// The amount of gas used in the execution. @@ -106,6 +106,8 @@ pub enum EstimationResult { }, /// Estimation failed due to contract revert. Revert { + /// The input estimation limit. + limit: u64, /// The revert reason. reason: Bytes, /// The amount of gas used in the execution. @@ -113,6 +115,8 @@ pub enum EstimationResult { }, /// The estimation failed due to EVM halt. Halt { + /// The input estimation limit. + limit: u64, /// The halt reason. reason: HaltReason, /// The amount of gas used in the execution @@ -123,18 +127,17 @@ pub enum EstimationResult { impl core::fmt::Display for EstimationResult { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { - Self::Success { estimation, refund, gas_used, .. } => { + Self::Success { limit, refund, gas_used, .. } => { write!( f, - "Success {{ estimation: {}, refund: {}, gas_used: {}, .. }}", - estimation, refund, gas_used + "Success {{ gas_limit: {limit}, refund: {refund}, gas_used: {gas_used}, .. }}", ) } - Self::Revert { gas_used, .. } => { - write!(f, "Revert {{ gas_used: {}, .. }}", gas_used) + Self::Revert { limit, gas_used, .. } => { + write!(f, "Revert {{ gas_limit: {limit}, gas_used: {gas_used}, .. }}") } - Self::Halt { reason, gas_used } => { - write!(f, "Halt {{ reason: {:?}, gas_used: {} }}", reason, gas_used) + Self::Halt { limit, reason, gas_used } => { + write!(f, "Halt {{ gas_limit: {limit}, reason: {reason:?}, gas_used: {gas_used} }}") } } } @@ -146,16 +149,16 @@ impl EstimationResult { pub fn from_limit_and_execution_result(limit: u64, value: &ExecutionResult) -> Self { match value { ExecutionResult::Success { gas_used, output, gas_refunded, .. } => Self::Success { - estimation: limit, + limit, refund: *gas_refunded, gas_used: *gas_used, output: output.clone(), }, ExecutionResult::Revert { output, gas_used } => { - Self::Revert { reason: output.clone(), gas_used: *gas_used } + Self::Revert { limit, reason: output.clone(), gas_used: *gas_used } } ExecutionResult::Halt { reason, gas_used } => { - Self::Halt { reason: *reason, gas_used: *gas_used } + Self::Halt { limit, reason: *reason, gas_used: *gas_used } } } } @@ -163,7 +166,7 @@ impl EstimationResult { /// Create a successful estimation result with a gas estimation of 21000. pub const fn basic_transfer_success(estimation: u64) -> Self { Self::Success { - estimation, + limit: estimation, refund: 0, gas_used: estimation, output: Output::Call(Bytes::new()), @@ -180,11 +183,13 @@ impl EstimationResult { !self.is_success() } - /// Get the gas estimation, if the execution was successful. - pub const fn gas_estimation(&self) -> Option { + /// Get the gas limit that was set in the EVM when the estimation was + /// produced. + pub const fn limit(&self) -> u64 { match self { - Self::Success { estimation, .. } => Some(*estimation), - _ => None, + Self::Success { limit, .. } => *limit, + Self::Revert { limit, .. } => *limit, + Self::Halt { limit, .. } => *limit, } } @@ -242,13 +247,12 @@ impl EstimationResult { /// Adjust the binary search range based on the estimation outcome. pub(crate) const fn adjust_binary_search_range( &self, - limit: u64, range: &mut SearchRange, ) -> Result<(), Self> { match self { - Self::Success { .. } => range.set_max(limit), - Self::Revert { .. } => range.set_min(limit), - Self::Halt { reason, gas_used } => { + Self::Success { limit, .. } => range.set_max(*limit), + Self::Revert { limit, .. } => range.set_min(*limit), + Self::Halt { limit, reason, gas_used } => { // Both `OutOfGas` and `InvalidEFOpcode` can occur dynamically // if the gas left is too low. Treat this as an out of gas // condition, knowing that the call succeeds with a @@ -257,9 +261,9 @@ impl EstimationResult { // Common usage of invalid opcode in OpenZeppelin: // if matches!(reason, HaltReason::OutOfGas(_) | HaltReason::InvalidFEOpcode) { - range.set_min(limit); + range.set_min(*limit); } else { - return Err(Self::Halt { reason: *reason, gas_used: *gas_used }); + return Err(Self::Halt { limit: *limit, reason: *reason, gas_used: *gas_used }); } } } diff --git a/src/macros.rs b/src/macros.rs index f4a15e1..9bbc1c0 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -45,14 +45,13 @@ macro_rules! estimate_and_adjust { ($est:ident, $trevm:ident, $gas_limit:ident, $range:ident) => { ::tracing::trace!( estimate = %$est, - gas_limit = $gas_limit, max = %$range.max(), min = %$range.min(), "running gas estimate call" ); ($est, $trevm) = $trevm.run_estimate(&$gas_limit.into())?; - if let Err(e) = $est.adjust_binary_search_range($gas_limit, &mut $range) { + if let Err(e) = $est.adjust_binary_search_range(&mut $range) { ::tracing::trace!( %e, "error adjusting binary search range"