Feat: evm module with custom EvmFactory trait (#554)

## 💡 Motivation and Context

This PR brings small code cleanup for `EvmFactory`. I have created a new
`EvmFactory` trait in rbuilder because the `Evm` trait from `alloy_revm`
has a lot of associated types and trait bounds.
These are now defined in the custom `EvmFactory` trait and resolved in
the implementation for `EthCachedEvmFactory`, so this removes the need
for extra trait bounds outside of the `evm` module (such as in the
`execute_evm` function in `order_commit`).

## 📝 Summary

- The `EthCachedEvmFactory` was moved to new module `evm` under
`building` (`rbuilder::building::evm`), IMO this makes the overall code
structure better as the separation of concerns is cleaner (more obvious)
- `EthCachedEvmFactory::Context` is defined to `EthEvmContext<DB>`,
which is an alias of `Context<BlockEnv, TxEnv, CfgEnv, DB>`
- `EvmFactory` custom trait, which is a more concrete version than
`alloy_revm::EvmFactory`
- Remove the dependency on `EthEvmFactory` in `EthCachedEvmFactory`
struct, but still use it's default implementation under the hood (we can
also consider completely remove it and use `Context` instead)

---

## ✅ I have completed the following steps:

* [x] Run `make lint`
* [x] Run `make test`
* [ ] Added tests (if applicable)

Author: julio4

crates/rbuilder/src/building/evm.rs

@@ -0,0 +1,115 @@
+use crate::building::precompile_cache::{PrecompileCache, WrappedPrecompile};
+use parking_lot::Mutex;
+use reth_evm::{
+    eth::EthEvmContext, EthEvm, EthEvmFactory, Evm, EvmEnv, EvmFactory as RethEvmFactory,
+};
+use revm::{
+    context::{
+        result::{EVMError, HaltReason},
+        TxEnv,
+    },
+    handler::EthPrecompiles,
+    inspector::NoOpInspector,
+    interpreter::interpreter::EthInterpreter,
+    primitives::hardfork::SpecId,
+    Database, Inspector,
+};
+use std::sync::Arc;
+
+/// Custom trait to abstract over EVM construction with a cleaner and more concrete
+/// interface than the `Evm` trait from `alloy-revm`.
+///
+/// # Motivation
+///
+/// The `alloy_revm::Evm` trait comes with a large number of associated types and trait
+/// bounds. This new `EvmFactory` trait is designed to encapsulate those complexities,
+/// providing an EVM interface less dependent on `alloy-revm` crate.
+///
+/// It is particularly useful in reducing trait bound noise in other parts of the codebase
+/// (i.e. `execute_evm` in `order_commit`), and improves modularity.
+///
+/// See [`EthCachedEvmFactory`] for an implementation that integrates precompile
+/// caching and uses `reth_evm::EthEvm` internally.
+pub trait EvmFactory {
+    type Evm<DB, I>: Evm<
+        DB = DB,
+        Tx = TxEnv,
+        HaltReason = HaltReason,
+        Error = EVMError<DB::Error>,
+        Spec = SpecId,
+    >
+    where
+        DB: Database<Error: Send + Sync + 'static>,
+        I: Inspector<EthEvmContext<DB>>;
+
+    /// Create an EVM instance with default (no-op) inspector.
+    fn create_evm<DB>(&self, db: DB, env: EvmEnv) -> Self::Evm<DB, NoOpInspector>
+    where
+        DB: Database<Error: Send + Sync + 'static>;
+
+    /// Create an EVM instance with a provided inspector.
+    fn create_evm_with_inspector<DB, I>(
+        &self,
+        db: DB,
+        env: EvmEnv,
+        inspector: I,
+    ) -> Self::Evm<DB, I>
+    where
+        DB: Database<Error: Send + Sync + 'static>,
+        I: Inspector<EthEvmContext<DB>, EthInterpreter>;
+}
+
+#[derive(Debug, Clone, Default)]
+pub struct EthCachedEvmFactory {
+    evm_factory: EthEvmFactory,
+    cache: Arc<Mutex<PrecompileCache>>,
+}
+
+/// Implementation of the `EvmFactory` trait for `EthCachedEvmFactory`.
+///
+/// This implementation uses `reth_evm::EthEvm` internally and provides a concrete
+/// type for the `Evm` trait.
+///
+/// It also integrates precompile caching using the [`PrecompileCache`] and
+/// [`WrappedPrecompile`] types.
+impl EvmFactory for EthCachedEvmFactory {
+    type Evm<DB, I>
+        = EthEvm<DB, I, WrappedPrecompile<EthPrecompiles>>
+    where
+        DB: Database<Error: Send + Sync + 'static>,
+        I: Inspector<EthEvmContext<DB>>;
+
+    fn create_evm<DB>(&self, db: DB, env: EvmEnv) -> Self::Evm<DB, NoOpInspector>
+    where
+        DB: Database<Error: Send + Sync + 'static>,
+    {
+        let evm = self
+            .evm_factory
+            .create_evm(db, env)
+            .into_inner()
+            .with_precompiles(WrappedPrecompile::new(
+                EthPrecompiles::default(),
+                self.cache.clone(),
+            ));
+
+        EthEvm::new(evm, false)
+    }
+
+    fn create_evm_with_inspector<DB, I>(
+        &self,
+        db: DB,
+        input: EvmEnv,
+        inspector: I,
+    ) -> Self::Evm<DB, I>
+    where
+        DB: Database<Error: Send + Sync + 'static>,
+        I: Inspector<EthEvmContext<DB>, EthInterpreter>,
+    {
+        EthEvm::new(
+            self.create_evm(db, input)
+                .into_inner()
+                .with_inspector(inspector),
+            true,
+        )
+    }
+}

crates/rbuilder/src/building/mod.rs

@@ -18,8 +18,8 @@ use alloy_evm::{block::system_calls::SystemCaller, env::EvmEnv, eth::eip6110};
 use alloy_primitives::{Address, Bytes, B256, U256};
 use alloy_rpc_types_beacon::events::PayloadAttributesEvent;
 use cached_reads::{LocalCachedReads, SharedCachedReads};
+use evm::EthCachedEvmFactory;
 use jsonrpsee::core::Serialize;
-use precompile_cache::EthCachedEvmFactory;
 use reth::{
     payload::PayloadId,
     primitives::{Block, Receipt, SealedBlock},
@@ -56,6 +56,7 @@ pub mod built_block_trace;
 pub mod cached_reads;
 #[cfg(test)]
 pub mod conflict;
+pub mod evm;
 pub mod evm_inspector;
 pub mod fmt;
 pub mod order_commit;

crates/rbuilder/src/building/order_commit.rs

@@ -7,6 +7,7 @@ use super::{
 use crate::{
     building::{
         estimate_payout_gas_limit,
+        evm::EvmFactory,
         evm_inspector::{RBuilderEVMInspector, UsedStateTrace},
     },
     primitives::{
@@ -21,14 +22,11 @@ use alloy_eips::eip4844::{DATA_GAS_PER_BLOB, MAX_DATA_GAS_PER_BLOCK};
 use alloy_primitives::{Address, B256, U256};
 use reth::revm::database::StateProviderDatabase;
 use reth_errors::ProviderError;
-use reth_evm::{Evm, EvmEnv, EvmFactory};
+use reth_evm::{Evm, EvmEnv};
 use reth_primitives::Receipt;
 use reth_provider::{StateProvider, StateProviderBox};
 use revm::{
-    context::{
-        result::{HaltReason, ResultAndState},
-        TxEnv,
-    },
+    context::result::ResultAndState,
     context_interface::result::{EVMError, ExecutionResult, InvalidTransaction},
     database::{states::bundle_state::BundleRetention, BundleState, State},
     Database, DatabaseCommit,
@@ -1247,18 +1245,14 @@ fn update_nonce_list_with_updates(
 /// thats why it can't return `TransactionErr::GasLeft` and  `TransactionErr::BlobGasLeft`
 fn execute_evm<Factory>(
     evm_factory: &Factory,
-    evm_env: EvmEnv<Factory::Spec>,
+    evm_env: EvmEnv,
     tx_with_blobs: &TransactionSignedEcRecoveredWithBlobs,
     used_state_tracer: Option<&mut UsedStateTrace>,
     db: impl Database<Error = ProviderError>,
     blocklist: &HashSet<Address>,
 ) -> Result<Result<ResultAndState, TransactionErr>, CriticalCommitOrderError>
 where
-    Factory: EvmFactory<
-        Tx = TxEnv,
-        HaltReason = HaltReason,
-        Error<ProviderError> = EVMError<ProviderError>,
-    >,
+    Factory: EvmFactory,
 {
     let tx = tx_with_blobs.internal_tx_unsecure();
     let mut rbuilder_inspector = RBuilderEVMInspector::new(tx, used_state_tracer);

crates/rbuilder/src/building/payout_tx.rs

@@ -1,10 +1,10 @@
-use super::{BlockBuildingContext, BlockState, ThreadBlockBuildingContext};
+use super::{evm::EvmFactory, BlockBuildingContext, BlockState, ThreadBlockBuildingContext};
 use crate::utils::Signer;
 use alloy_consensus::{constants::KECCAK_EMPTY, TxEip1559};
 use alloy_primitives::{Address, TxKind as TransactionKind, U256};
 use reth_chainspec::ChainSpec;
 use reth_errors::ProviderError;
-use reth_evm::{Evm, EvmFactory};
+use reth_evm::Evm;
 use reth_primitives::{Recovered, Transaction, TransactionSigned};
 use revm::context::result::{EVMError, ExecutionResult};
 

crates/rbuilder/src/building/precompile_cache.rs

@@ -4,17 +4,11 @@ use alloy_primitives::{Address, Bytes};
 use derive_more::{Deref, DerefMut};
 use lru::LruCache;
 use parking_lot::Mutex;
-use reth_evm::{eth::EthEvmContext, EthEvm, EthEvmFactory, EvmEnv, EvmFactory};
 use revm::{
-    context::{
-        result::{EVMError, HaltReason},
-        BlockEnv, Cfg, CfgEnv, ContextTr, TxEnv,
-    },
-    handler::{EthPrecompiles, PrecompileProvider},
-    inspector::NoOpInspector,
-    interpreter::{interpreter::EthInterpreter, InputsImpl, InterpreterResult},
+    context::{Cfg, ContextTr},
+    handler::PrecompileProvider,
+    interpreter::{InputsImpl, InterpreterResult},
     primitives::hardfork::SpecId,
-    Context, Database, Inspector,
 };
 use std::{num::NonZeroUsize, sync::Arc};
 
@@ -104,65 +98,3 @@ impl<CTX: ContextTr, P: PrecompileProvider<CTX, Output = InterpreterResult>> Pre
         self.precompile.contains(address)
     }
 }
-
-#[derive(Debug, Clone, Default)]
-pub struct EthCachedEvmFactory {
-    evm_factory: EthEvmFactory,
-    cache: Arc<Mutex<PrecompileCache>>,
-}
-
-impl EvmFactory for EthCachedEvmFactory {
-    type Evm<DB, I>
-        = EthEvm<DB, I, WrappedPrecompile<EthPrecompiles>>
-    where
-        DB: Database<Error: Send + Sync + 'static>,
-        I: Inspector<EthEvmContext<DB>>;
-
-    type Context<DB>
-        = Context<BlockEnv, TxEnv, CfgEnv, DB>
-    where
-        DB: Database<Error: Send + Sync + 'static>;
-
-    type Error<DBError>
-        = EVMError<DBError>
-    where
-        DBError: core::error::Error + Send + Sync + 'static;
-
-    type Tx = TxEnv;
-    type HaltReason = HaltReason;
-    type Spec = SpecId;
-
-    fn create_evm<DB>(&self, db: DB, input: EvmEnv) -> Self::Evm<DB, NoOpInspector>
-    where
-        DB: Database<Error: Send + Sync + 'static>,
-    {
-        let evm = self
-            .evm_factory
-            .create_evm(db, input)
-            .into_inner()
-            .with_precompiles(WrappedPrecompile::new(
-                EthPrecompiles::default(),
-                self.cache.clone(),
-            ));
-
-        EthEvm::new(evm, false)
-    }
-
-    fn create_evm_with_inspector<DB, I>(
-        &self,
-        db: DB,
-        input: EvmEnv,
-        inspector: I,
-    ) -> Self::Evm<DB, I>
-    where
-        DB: Database<Error: Send + Sync + 'static>,
-        I: Inspector<Self::Context<DB>, EthInterpreter>,
-    {
-        EthEvm::new(
-            self.create_evm(db, input)
-                .into_inner()
-                .with_inspector(inspector),
-            true,
-        )
-    }
-}

crates/rbuilder/src/building/testing/evm_inspector_tests/setup.rs

@@ -1,11 +1,12 @@
 use crate::building::{
     cached_reads::LocalCachedReads,
+    evm::EvmFactory,
     evm_inspector::{RBuilderEVMInspector, UsedStateTrace},
     testing::test_chain_state::{BlockArgs, NamedAddr, TestChainState, TestContracts, TxArgs},
     BlockState,
 };
 use alloy_primitives::Address;
-use reth_evm::{Evm, EvmFactory};
+use reth_evm::Evm;
 use reth_primitives::{Recovered, TransactionSigned};
 
 #[derive(Debug)]