From 90f3161256f2dbfafedbc4f71266887ecfd41116 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Fri, 3 May 2024 14:15:04 +0200 Subject: [PATCH] chore: remove InspectorStack (#8073) --- Cargo.lock | 1 - crates/ethereum/evm/src/execute.rs | 39 +---- crates/node-core/src/args/debug.rs | 33 +--- crates/node-ethereum/src/node.rs | 3 +- crates/node/builder/Cargo.toml | 1 - crates/node/builder/src/builder/mod.rs | 23 --- crates/optimism/evm/src/execute.rs | 39 +---- crates/optimism/node/src/node.rs | 3 +- crates/revm/src/lib.rs | 5 - crates/revm/src/stack.rs | 202 ------------------------- 10 files changed, 15 insertions(+), 334 deletions(-) delete mode 100644 crates/revm/src/stack.rs diff --git a/Cargo.lock b/Cargo.lock index b2e179a95..a3f0450ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7278,7 +7278,6 @@ dependencies = [ "reth-primitives", "reth-provider", "reth-prune", - "reth-revm", "reth-rpc", "reth-rpc-engine-api", "reth-stages", diff --git a/crates/ethereum/evm/src/execute.rs b/crates/ethereum/evm/src/execute.rs index c3dd315f7..b65e7be17 100644 --- a/crates/ethereum/evm/src/execute.rs +++ b/crates/ethereum/evm/src/execute.rs @@ -20,7 +20,6 @@ use reth_revm::{ batch::{BlockBatchRecord, BlockExecutorStats}, db::states::bundle_state::BundleRetention, eth_dao_fork::{DAO_HARDFORK_BENEFICIARY, DAO_HARDKFORK_ACCOUNTS}, - stack::InspectorStack, state_change::{apply_beacon_root_contract_call, post_block_balance_increments}, Evm, State, }; @@ -36,7 +35,6 @@ use tracing::debug; pub struct EthExecutorProvider { chain_spec: Arc, evm_config: EvmConfig, - inspector: Option, } impl EthExecutorProvider { @@ -54,13 +52,7 @@ impl EthExecutorProvider { impl EthExecutorProvider { /// Creates a new executor provider. pub fn new(chain_spec: Arc, evm_config: EvmConfig) -> Self { - Self { chain_spec, evm_config, inspector: None } - } - - /// Configures an optional inspector stack for debugging. - pub fn with_inspector(mut self, inspector: Option) -> Self { - self.inspector = inspector; - self + Self { chain_spec, evm_config } } } @@ -78,7 +70,6 @@ where self.evm_config.clone(), State::builder().with_database(db).with_bundle_update().without_state_clear().build(), ) - .with_inspector(self.inspector.clone()) } } @@ -221,20 +212,12 @@ pub struct EthBlockExecutor { executor: EthEvmExecutor, /// The state to use for execution state: State, - /// Optional inspector stack for debugging - inspector: Option, } impl EthBlockExecutor { /// Creates a new Ethereum block executor. pub fn new(chain_spec: Arc, evm_config: EvmConfig, state: State) -> Self { - Self { executor: EthEvmExecutor { chain_spec, evm_config }, state, inspector: None } - } - - /// Sets the inspector stack for debugging. - pub fn with_inspector(mut self, inspector: Option) -> Self { - self.inspector = inspector; - self + Self { executor: EthEvmExecutor { chain_spec, evm_config }, state } } #[inline] @@ -292,19 +275,9 @@ where let env = self.evm_env_for_block(&block.header, total_difficulty); let (receipts, gas_used) = { - if let Some(inspector) = self.inspector.as_mut() { - let evm = self.executor.evm_config.evm_with_env_and_inspector( - &mut self.state, - env, - inspector, - ); - self.executor.execute_pre_and_transactions(block, evm)? - } else { - let evm = self.executor.evm_config.evm_with_env(&mut self.state, env); - - self.executor.execute_pre_and_transactions(block, evm)? - } - }; + let evm = self.executor.evm_config.evm_with_env(&mut self.state, env); + self.executor.execute_pre_and_transactions(block, evm) + }?; // 3. apply post execution changes self.post_execution(block, total_difficulty)?; @@ -507,7 +480,7 @@ mod tests { } fn executor_provider(chain_spec: Arc) -> EthExecutorProvider { - EthExecutorProvider { chain_spec, evm_config: Default::default(), inspector: None } + EthExecutorProvider { chain_spec, evm_config: Default::default() } } #[test] diff --git a/crates/node-core/src/args/debug.rs b/crates/node-core/src/args/debug.rs index 3eda71ad0..d1c4e9b73 100644 --- a/crates/node-core/src/args/debug.rs +++ b/crates/node-core/src/args/debug.rs @@ -1,7 +1,7 @@ //! clap [Args](clap::Args) for debugging purposes use clap::Args; -use reth_primitives::{TxHash, B256}; +use reth_primitives::B256; use std::path::PathBuf; /// Parameters for debugging purposes @@ -28,37 +28,6 @@ pub struct DebugArgs { #[arg(long = "debug.max-block", help_heading = "Debug")] pub max_block: Option, - /// Print opcode level traces directly to console during execution. - #[arg(long = "debug.print-inspector", help_heading = "Debug")] - pub print_inspector: bool, - - /// Hook on a specific block during execution. - #[arg( - long = "debug.hook-block", - help_heading = "Debug", - conflicts_with = "hook_transaction", - conflicts_with = "hook_all" - )] - pub hook_block: Option, - - /// Hook on a specific transaction during execution. - #[arg( - long = "debug.hook-transaction", - help_heading = "Debug", - conflicts_with = "hook_block", - conflicts_with = "hook_all" - )] - pub hook_transaction: Option, - - /// Hook on every transaction in a block. - #[arg( - long = "debug.hook-all", - help_heading = "Debug", - conflicts_with = "hook_block", - conflicts_with = "hook_transaction" - )] - pub hook_all: bool, - /// If provided, the engine will skip `n` consecutive FCUs. #[arg(long = "debug.skip-fcu", help_heading = "Debug")] pub skip_fcu: Option, diff --git a/crates/node-ethereum/src/node.rs b/crates/node-ethereum/src/node.rs index 235130b42..87bc54d15 100644 --- a/crates/node-ethereum/src/node.rs +++ b/crates/node-ethereum/src/node.rs @@ -85,8 +85,7 @@ where ) -> eyre::Result<(Self::EVM, Self::Executor)> { let chain_spec = ctx.chain_spec(); let evm_config = EthEvmConfig::default(); - let executor = - EthExecutorProvider::new(chain_spec, evm_config).with_inspector(ctx.inspector_stack()); + let executor = EthExecutorProvider::new(chain_spec, evm_config); Ok((evm_config, executor)) } diff --git a/crates/node/builder/Cargo.toml b/crates/node/builder/Cargo.toml index 68c1d5f0c..26635e536 100644 --- a/crates/node/builder/Cargo.toml +++ b/crates/node/builder/Cargo.toml @@ -19,7 +19,6 @@ reth-blockchain-tree.workspace = true reth-exex.workspace = true reth-evm.workspace = true reth-provider.workspace = true -reth-revm.workspace = true reth-db.workspace = true reth-rpc-engine-api.workspace = true reth-rpc.workspace = true diff --git a/crates/node/builder/src/builder/mod.rs b/crates/node/builder/src/builder/mod.rs index 0457bbe3e..b6f0a191e 100644 --- a/crates/node/builder/src/builder/mod.rs +++ b/crates/node/builder/src/builder/mod.rs @@ -27,7 +27,6 @@ use reth_node_core::{ }; use reth_primitives::{constants::eip4844::MAINNET_KZG_TRUSTED_SETUP, ChainSpec}; use reth_provider::{providers::BlockchainProvider, ChainSpecProvider}; -use reth_revm::stack::{InspectorStack, InspectorStackConfig}; use reth_tasks::TaskExecutor; use reth_transaction_pool::{PoolConfig, TransactionPool}; pub use states::*; @@ -461,28 +460,6 @@ impl BuilderContext { &self.config } - /// Returns an inspector stack if configured. - /// - /// This can be used to debug block execution. - pub fn inspector_stack(&self) -> Option { - use reth_revm::stack::Hook; - let stack_config = InspectorStackConfig { - use_printer_tracer: self.config.debug.print_inspector, - hook: if let Some(hook_block) = self.config.debug.hook_block { - Hook::Block(hook_block) - } else if let Some(tx) = self.config.debug.hook_transaction { - Hook::Transaction(tx) - } else if self.config.debug.hook_all { - Hook::All - } else { - // no inspector - return None - }, - }; - - Some(InspectorStack::new(stack_config)) - } - /// Returns the data dir of the node. /// /// This gives access to all relevant files and directories of the node's datadir. diff --git a/crates/optimism/evm/src/execute.rs b/crates/optimism/evm/src/execute.rs index 2ea32782c..d19d441a8 100644 --- a/crates/optimism/evm/src/execute.rs +++ b/crates/optimism/evm/src/execute.rs @@ -19,7 +19,6 @@ use reth_primitives::{ use reth_revm::{ batch::{BlockBatchRecord, BlockExecutorStats}, db::states::bundle_state::BundleRetention, - stack::InspectorStack, state_change::{apply_beacon_root_contract_call, post_block_balance_increments}, Evm, State, }; @@ -35,7 +34,6 @@ use tracing::{debug, trace}; pub struct OpExecutorProvider { chain_spec: Arc, evm_config: EvmConfig, - inspector: Option, } impl OpExecutorProvider { @@ -48,13 +46,7 @@ impl OpExecutorProvider { impl OpExecutorProvider { /// Creates a new executor provider. pub fn new(chain_spec: Arc, evm_config: EvmConfig) -> Self { - Self { chain_spec, evm_config, inspector: None } - } - - /// Configures an optional inspector stack for debugging. - pub fn with_inspector(mut self, inspector: Option) -> Self { - self.inspector = inspector; - self + Self { chain_spec, evm_config } } } @@ -72,7 +64,6 @@ where self.evm_config.clone(), State::builder().with_database(db).with_bundle_update().without_state_clear().build(), ) - .with_inspector(self.inspector.clone()) } } @@ -268,20 +259,12 @@ pub struct OpBlockExecutor { executor: OpEvmExecutor, /// The state to use for execution state: State, - /// Optional inspector stack for debugging - inspector: Option, } impl OpBlockExecutor { /// Creates a new Ethereum block executor. pub fn new(chain_spec: Arc, evm_config: EvmConfig, state: State) -> Self { - Self { executor: OpEvmExecutor { chain_spec, evm_config }, state, inspector: None } - } - - /// Sets the inspector stack for debugging. - pub fn with_inspector(mut self, inspector: Option) -> Self { - self.inspector = inspector; - self + Self { executor: OpEvmExecutor { chain_spec, evm_config }, state } } #[inline] @@ -337,19 +320,9 @@ where let env = self.evm_env_for_block(&block.header, total_difficulty); let (receipts, gas_used) = { - if let Some(inspector) = self.inspector.as_mut() { - let evm = self.executor.evm_config.evm_with_env_and_inspector( - &mut self.state, - env, - inspector, - ); - self.executor.execute_pre_and_transactions(block, evm)? - } else { - let evm = self.executor.evm_config.evm_with_env(&mut self.state, env); - - self.executor.execute_pre_and_transactions(block, evm)? - } - }; + let evm = self.executor.evm_config.evm_with_env(&mut self.state, env); + self.executor.execute_pre_and_transactions(block, evm) + }?; // 3. apply post execution changes self.post_execution(block, total_difficulty)?; @@ -548,7 +521,7 @@ mod tests { } fn executor_provider(chain_spec: Arc) -> OpExecutorProvider { - OpExecutorProvider { chain_spec, evm_config: Default::default(), inspector: None } + OpExecutorProvider { chain_spec, evm_config: Default::default() } } #[test] diff --git a/crates/optimism/node/src/node.rs b/crates/optimism/node/src/node.rs index a2cbc287c..7d715fece 100644 --- a/crates/optimism/node/src/node.rs +++ b/crates/optimism/node/src/node.rs @@ -105,8 +105,7 @@ where ) -> eyre::Result<(Self::EVM, Self::Executor)> { let chain_spec = ctx.chain_spec(); let evm_config = OptimismEvmConfig::default(); - let executor = - OpExecutorProvider::new(chain_spec, evm_config).with_inspector(ctx.inspector_stack()); + let executor = OpExecutorProvider::new(chain_spec, evm_config); Ok((evm_config, executor)) } diff --git a/crates/revm/src/lib.rs b/crates/revm/src/lib.rs index d8c5761d0..375b230ab 100644 --- a/crates/revm/src/lib.rs +++ b/crates/revm/src/lib.rs @@ -19,11 +19,6 @@ pub mod state_change; /// Ethereum DAO hardfork state change data. pub mod eth_dao_fork; -/// An inspector stack abstracting the implementation details of -/// each inspector and allowing to hook on block/transaction execution, -/// used in the main Reth executor. -pub mod stack; - /// Common test helpers #[cfg(any(test, feature = "test-utils"))] pub mod test_utils; diff --git a/crates/revm/src/stack.rs b/crates/revm/src/stack.rs deleted file mode 100644 index 8f8bfa5ce..000000000 --- a/crates/revm/src/stack.rs +++ /dev/null @@ -1,202 +0,0 @@ -use revm::{ - inspectors::CustomPrintTracer, - interpreter::{CallInputs, CallOutcome, CreateInputs, CreateOutcome, Interpreter}, - primitives::{Address, Env, Log, B256, U256}, - Database, EvmContext, Inspector, -}; -use std::fmt::Debug; - -/// A hook to inspect the execution of the EVM. -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] -pub enum Hook { - /// No hook. - #[default] - None, - /// Hook on a specific block. - Block(u64), - /// Hook on a specific transaction hash. - Transaction(B256), - /// Hooks on every transaction in a block. - All, -} - -impl Hook { - /// Returns `true` if this hook should be used. - #[inline] - pub fn is_enabled(&self, block_number: u64, tx_hash: &B256) -> bool { - match self { - Hook::None => false, - Hook::Block(block) => block_number == *block, - Hook::Transaction(hash) => hash == tx_hash, - Hook::All => true, - } - } -} - -/// An inspector that calls multiple inspectors in sequence. -#[derive(Clone, Default)] -pub struct InspectorStack { - /// An inspector that prints the opcode traces to the console. - pub custom_print_tracer: Option, - /// The provided hook - pub hook: Hook, -} - -impl Debug for InspectorStack { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("InspectorStack") - .field("custom_print_tracer", &self.custom_print_tracer.is_some()) - .field("hook", &self.hook) - .finish() - } -} - -impl InspectorStack { - /// Creates a new inspector stack with the given configuration. - #[inline] - pub fn new(config: InspectorStackConfig) -> Self { - Self { - hook: config.hook, - custom_print_tracer: config.use_printer_tracer.then(Default::default), - } - } - - /// Returns `true` if this inspector should be used. - #[inline] - pub fn should_inspect(&self, env: &Env, tx_hash: &B256) -> bool { - self.custom_print_tracer.is_some() && - self.hook.is_enabled(env.block.number.saturating_to(), tx_hash) - } -} - -/// Configuration for the inspectors. -#[derive(Clone, Copy, Debug, Default)] -pub struct InspectorStackConfig { - /// Enable revm inspector printer. - /// In execution this will print opcode level traces directly to console. - pub use_printer_tracer: bool, - - /// Hook on a specific block or transaction. - pub hook: Hook, -} - -/// Helper macro to call the same method on multiple inspectors without resorting to dynamic -/// dispatch. -#[macro_export] -macro_rules! call_inspectors { - ([$($inspector:expr),+ $(,)?], |$id:ident $(,)?| $call:expr $(,)?) => {{$( - if let Some($id) = $inspector { - $call - } - )+}} -} - -impl Inspector for InspectorStack -where - DB: Database, -{ - #[inline] - fn initialize_interp(&mut self, interp: &mut Interpreter, context: &mut EvmContext) { - call_inspectors!([&mut self.custom_print_tracer], |inspector| { - inspector.initialize_interp(interp, context); - }); - } - - #[inline] - fn step(&mut self, interp: &mut Interpreter, context: &mut EvmContext) { - call_inspectors!([&mut self.custom_print_tracer], |inspector| { - inspector.step(interp, context); - }); - } - - #[inline] - fn step_end(&mut self, interp: &mut Interpreter, context: &mut EvmContext) { - call_inspectors!([&mut self.custom_print_tracer], |inspector| { - inspector.step_end(interp, context); - }); - } - - #[inline] - fn log(&mut self, context: &mut EvmContext, log: &Log) { - call_inspectors!([&mut self.custom_print_tracer], |inspector| { - inspector.log(context, log); - }); - } - - #[inline] - fn call( - &mut self, - context: &mut EvmContext, - inputs: &mut CallInputs, - ) -> Option { - call_inspectors!([&mut self.custom_print_tracer], |inspector| { - if let Some(outcome) = inspector.call(context, inputs) { - return Some(outcome) - } - }); - - None - } - - #[inline] - fn call_end( - &mut self, - context: &mut EvmContext, - inputs: &CallInputs, - outcome: CallOutcome, - ) -> CallOutcome { - call_inspectors!([&mut self.custom_print_tracer], |inspector| { - let new_ret = inspector.call_end(context, inputs, outcome.clone()); - - // If the inspector returns a different ret or a revert with a non-empty message, - // we assume it wants to tell us something - if new_ret != outcome { - return new_ret - } - }); - - outcome - } - - #[inline] - fn create( - &mut self, - context: &mut EvmContext, - inputs: &mut CreateInputs, - ) -> Option { - call_inspectors!([&mut self.custom_print_tracer], |inspector| { - if let Some(out) = inspector.create(context, inputs) { - return Some(out) - } - }); - - None - } - - #[inline] - fn create_end( - &mut self, - context: &mut EvmContext, - inputs: &CreateInputs, - outcome: CreateOutcome, - ) -> CreateOutcome { - call_inspectors!([&mut self.custom_print_tracer], |inspector| { - let new_ret = inspector.create_end(context, inputs, outcome.clone()); - - // If the inspector returns a different ret or a revert with a non-empty message, - // we assume it wants to tell us something - if new_ret != outcome { - return new_ret - } - }); - - outcome - } - - #[inline] - fn selfdestruct(&mut self, contract: Address, target: Address, value: U256) { - call_inspectors!([&mut self.custom_print_tracer], |inspector| { - Inspector::::selfdestruct(inspector, contract, target, value); - }); - } -}