From c5e53d03fadc5757de586afa0b712b32d0545055 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 20 Feb 2025 12:28:00 -0500 Subject: [PATCH 1/5] fix: gas estimation for simple transfers --- Cargo.toml | 4 +-- src/db/mod.rs | 35 +++++++++++++++++++++++- src/est.rs | 7 +++-- src/evm.rs | 75 ++++++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 104 insertions(+), 17 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 42fb8e9..75a7ea2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "trevm" -version = "0.19.3" +version = "0.19.5" rust-version = "1.83.0" edition = "2021" authors = ["init4"] @@ -32,7 +32,7 @@ alloy-sol-types = { version = "0.8.11", default-features = false, features = ["s alloy = { version = "=0.9.2", default-features = false, features = ["consensus", "rpc-types-mev", "eips", "k256", "std"] } -revm = { version = "19.2.0", default-features = false, features = ["std"] } +revm = { version = "19.5.0", default-features = false, features = ["std"] } zenith-types = { version = "0.14" } diff --git a/src/db/mod.rs b/src/db/mod.rs index c0aa2ad..5779317 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -1,5 +1,6 @@ mod builder; +use alloy::rpc::types::BlockOverrides; pub use builder::ConcurrentStateBuilder; mod cache_state; @@ -8,7 +9,7 @@ pub use cache_state::ConcurrentCacheState; mod sync_state; pub use sync_state::{ConcurrentState, ConcurrentStateInfo}; -use crate::{EvmNeedsBlock, Trevm}; +use crate::{Block, EvmNeedsBlock, EvmNeedsTx, Trevm}; use revm::{ db::{states::bundle_state::BundleRetention, BundleState}, DatabaseRef, @@ -41,3 +42,35 @@ impl EvmNeedsBlock<'_, Ext, ConcurrentState> { bundle } } + +impl EvmNeedsTx<'_, Ext, ConcurrentState> { + /// Apply block overrides to the current block. + /// + /// Note that this is NOT reversible. The overrides are applied directly to + /// the underlying state and these changes cannot be removed. If it is + /// important that you have access to the pre-change state, you should wrap + /// the existing DB in a new [`State`] and apply the overrides to that. + pub fn apply_block_overrides(mut self, overrides: &BlockOverrides) -> Self { + overrides.fill_block(&mut self.inner); + + if let Some(hashes) = &overrides.block_hash { + self.inner.db_mut().info.block_hashes.write().unwrap().extend(hashes) + } + + self + } + + /// Apply block overrides to the current block, if they are provided. + /// + /// Note that this is NOT reversible. The overrides are applied directly to + /// the underlying state and these changes cannot be removed. If it is + /// important that you have access to the pre-change state, you should wrap + /// the existing DB in a new [`State`] and apply the overrides to that. + pub fn maybe_apply_block_overrides(self, overrides: Option<&BlockOverrides>) -> Self { + if let Some(overrides) = overrides { + self.apply_block_overrides(overrides) + } else { + self + } + } +} diff --git a/src/est.rs b/src/est.rs index 041604b..86db0f9 100644 --- a/src/est.rs +++ b/src/est.rs @@ -1,4 +1,3 @@ -use crate::MIN_TRANSACTION_GAS; use revm::primitives::{Bytes, ExecutionResult, HaltReason, Output}; use std::ops::Range; @@ -132,11 +131,11 @@ impl From<&ExecutionResult> for EstimationResult { impl EstimationResult { /// Create a successful estimation result with a gas estimation of 21000. - pub const fn basic_transfer_success() -> Self { + pub const fn basic_transfer_success(estimation: u64) -> Self { Self::Success { - estimation: MIN_TRANSACTION_GAS, + estimation, refund: 0, - gas_used: MIN_TRANSACTION_GAS, + gas_used: estimation, output: Output::Call(Bytes::new()), } } diff --git a/src/evm.rs b/src/evm.rs index d9e5505..39c987b 100644 --- a/src/evm.rs +++ b/src/evm.rs @@ -14,10 +14,10 @@ use alloy::{ use core::convert::Infallible; use revm::{ db::{states::bundle_state::BundleRetention, BundleState, State}, - interpreter::gas::CALL_STIPEND, + interpreter::gas::{calculate_initial_tx_gas, CALL_STIPEND}, primitives::{ - AccountInfo, BlockEnv, Bytecode, EVMError, EvmState, ExecutionResult, InvalidTransaction, - ResultAndState, SpecId, TxEnv, TxKind, KECCAK_EMPTY, + AccountInfo, AuthorizationList, BlockEnv, Bytecode, EVMError, Env, EvmState, + ExecutionResult, InvalidTransaction, ResultAndState, SpecId, TxEnv, TxKind, KECCAK_EMPTY, }, Database, DatabaseCommit, DatabaseRef, Evm, }; @@ -71,6 +71,30 @@ impl<'a, Ext, Db: Database + DatabaseCommit, TrevmState> Trevm<'a, Ext, Db, Trev self.inner } + /// Get a reference to the inner env. This contains the current + /// [`BlockEnv`], [`TxEnv`], and [`CfgEnv`]. + /// + /// These values may be meaningless, stale, or otherwise incorrect. Reading + /// them should be done with caution, as it may lead to logic bugs. + /// + /// [`CfgEnv`]: revm::primitives::CfgEnv + pub fn env_unchecked(&self) -> &Env { + &self.inner().context.evm.inner.env + } + + /// Get a mutable reference to the inner env. This contains the current + /// [`BlockEnv`], [`TxEnv`], and [`CfgEnv`]. + /// + /// These values may be meaningless, stale, or otherwise incorrect. Reading + /// them should be done with caution, as it may lead to logic bugs. + /// Modifying these values may lead to inconsistent state or invalid + /// execution. + /// + /// [`CfgEnv`]: revm::primitives::CfgEnv + pub fn env_mut_unchecked(&mut self) -> &mut Env { + &mut self.inner_mut_unchecked().context.evm.inner.env + } + /// Get the id of the currently running hardfork spec. Convenience function /// calling [`Evm::spec_id`]. pub fn spec_id(&self) -> SpecId { @@ -1176,7 +1200,7 @@ impl<'a, Ext, Db: Database + DatabaseCommit, TrevmState: HasTx> Trevm<'a, Ext, D } } -// -- HAS TX with State +// -- NEEDS TX with State impl EvmNeedsTx<'_, Ext, State> { /// Apply block overrides to the current block. @@ -1236,12 +1260,36 @@ impl<'a, Ext, Db: Database + DatabaseCommit> EvmReady<'a, Ext, Db> { } } + /// Calculate the minimum gas required to start EVM execution. + /// + /// This uses [`calculate_initial_tx_gas`] to calculate the initial gas. + /// Its output is dependent on + /// - the EVM spec + /// - the input data + /// - whether the transaction is a contract creation or a call + /// - the EIP-2930 access list + /// - the number of [EIP-7702] authorizations + /// + /// [EIP-2930]: https://eips.ethereum.org/EIPS/eip-2930 + /// [EIP-7702]: https://eips.ethereum.org/EIPS/eip-7702 + fn caluculate_initial_gas(&self) -> u64 { + calculate_initial_tx_gas( + self.spec_id(), + &[], + false, + &self.tx().access_list, + self.tx().authorization_list.as_ref().map(AuthorizationList::len).unwrap_or_default() + as u64, + ) + .initial_gas + } + /// Estimate gas for a simple transfer. This will /// - Check that the transaction has no input data. /// - Check that the target is not a `create`. /// - Check that the target is not a contract. /// - Return the minimum gas required for the transfer. - fn estimate_gas_simple_transfer(&mut self) -> Result, EVMError> { + fn estimate_gas_simple_transfer(&mut self) -> Result, EVMError> { if !self.is_transfer() { return Ok(None); } @@ -1254,8 +1302,9 @@ impl<'a, Ext, Db: Database + DatabaseCommit> EvmReady<'a, Ext, Db> { return Ok(None); } - // If the target is not a contract, then the gas is the minimum gas. - Ok(Some(())) + // delegate calculation to revm. This ensures that things like bogus + // 2930 access lists don't mess up our estimates + Ok(Some(self.caluculate_initial_gas())) } /// Convenience function to simplify nesting of [`Self::estimate_gas`]. @@ -1330,15 +1379,18 @@ impl<'a, Ext, Db: Database + DatabaseCommit> EvmReady<'a, Ext, Db> { /// /// [here]: https://github.com/paradigmxyz/reth/blob/ad503a08fa242b28ad3c1fea9caa83df2dfcf72d/crates/rpc/rpc-eth-api/src/helpers/estimate.rs#L35-L42 pub fn estimate_gas(mut self) -> Result<(EstimationResult, Self), EvmErrored<'a, Ext, Db>> { - if unwrap_or_trevm_err!(self.estimate_gas_simple_transfer(), self).is_some() { - return Ok((EstimationResult::basic_transfer_success(), self)); + if let Some(est) = unwrap_or_trevm_err!(self.estimate_gas_simple_transfer(), self) { + return Ok((EstimationResult::basic_transfer_success(est), self)); } // We shrink the gas limit to 64 bits, as using more than 18 quintillion - // gas in a block is not likely. + // gas in a block is unlikely. let initial_limit = self.gas_limit(); + // Start the search range at 21_000 gas. let mut search_range = SearchRange::new(MIN_TRANSACTION_GAS, initial_limit); + + // Block it to the gas cap. search_range.maybe_lower_max(self.block_gas_limit().saturating_to::()); // Check that the account has enough ETH to cover the gas, and lower if @@ -1346,6 +1398,9 @@ impl<'a, Ext, Db: Database + DatabaseCommit> EvmReady<'a, Ext, Db> { let allowance = unwrap_or_trevm_err!(self.gas_allowance(), self); search_range.maybe_lower_max(allowance); + // Raise the floor to the amount of gas required to initialize the EVM. + search_range.maybe_raise_min(self.caluculate_initial_gas()); + // Run an estimate with the max gas limit. // NB: we declare these mut as we re-use the binding throughout the // function. From 5932673d5ab6f1887bc075651ebe4e78ba10a9e7 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 20 Feb 2025 12:41:11 -0500 Subject: [PATCH 2/5] fix: concurrent state link --- src/db/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/db/mod.rs b/src/db/mod.rs index 5779317..e0bbd3e 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -49,7 +49,8 @@ impl EvmNeedsTx<'_, Ext, ConcurrentState> { /// Note that this is NOT reversible. The overrides are applied directly to /// the underlying state and these changes cannot be removed. If it is /// important that you have access to the pre-change state, you should wrap - /// the existing DB in a new [`State`] and apply the overrides to that. + /// the existing DB in a new [`ConcurrentState`] and apply the overrides to + /// that. pub fn apply_block_overrides(mut self, overrides: &BlockOverrides) -> Self { overrides.fill_block(&mut self.inner); @@ -65,7 +66,8 @@ impl EvmNeedsTx<'_, Ext, ConcurrentState> { /// Note that this is NOT reversible. The overrides are applied directly to /// the underlying state and these changes cannot be removed. If it is /// important that you have access to the pre-change state, you should wrap - /// the existing DB in a new [`State`] and apply the overrides to that. + /// the existing DB in a new [`ConcurrentState`] and apply the overrides to + /// that. pub fn maybe_apply_block_overrides(self, overrides: Option<&BlockOverrides>) -> Self { if let Some(overrides) = overrides { self.apply_block_overrides(overrides) From 7f0c79ba7145654968986e4edf29cff537bfbfba Mon Sep 17 00:00:00 2001 From: James Prestwich Date: Thu, 20 Feb 2025 12:51:38 -0500 Subject: [PATCH 3/5] Update evm.rs Co-authored-by: evalir --- src/evm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evm.rs b/src/evm.rs index 39c987b..2490427 100644 --- a/src/evm.rs +++ b/src/evm.rs @@ -1272,7 +1272,7 @@ impl<'a, Ext, Db: Database + DatabaseCommit> EvmReady<'a, Ext, Db> { /// /// [EIP-2930]: https://eips.ethereum.org/EIPS/eip-2930 /// [EIP-7702]: https://eips.ethereum.org/EIPS/eip-7702 - fn caluculate_initial_gas(&self) -> u64 { + fn calculate_initial_gas(&self) -> u64 { calculate_initial_tx_gas( self.spec_id(), &[], From 1657ffd68f728b062cd3466d838c64eab52bc55a Mon Sep 17 00:00:00 2001 From: James Prestwich Date: Thu, 20 Feb 2025 12:51:45 -0500 Subject: [PATCH 4/5] Update evm.rs Co-authored-by: evalir --- src/evm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evm.rs b/src/evm.rs index 2490427..1ba85b4 100644 --- a/src/evm.rs +++ b/src/evm.rs @@ -1304,7 +1304,7 @@ impl<'a, Ext, Db: Database + DatabaseCommit> EvmReady<'a, Ext, Db> { // delegate calculation to revm. This ensures that things like bogus // 2930 access lists don't mess up our estimates - Ok(Some(self.caluculate_initial_gas())) + Ok(Some(self.calculate_initial_gas())) } /// Convenience function to simplify nesting of [`Self::estimate_gas`]. From 0fb8e4d745c39091e7a273cb2b25ab006924a733 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 20 Feb 2025 13:49:20 -0500 Subject: [PATCH 5/5] fix: calu --- src/evm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evm.rs b/src/evm.rs index 1ba85b4..cb57d36 100644 --- a/src/evm.rs +++ b/src/evm.rs @@ -1399,7 +1399,7 @@ impl<'a, Ext, Db: Database + DatabaseCommit> EvmReady<'a, Ext, Db> { search_range.maybe_lower_max(allowance); // Raise the floor to the amount of gas required to initialize the EVM. - search_range.maybe_raise_min(self.caluculate_initial_gas()); + search_range.maybe_raise_min(self.calculate_initial_gas()); // Run an estimate with the max gas limit. // NB: we declare these mut as we re-use the binding throughout the