feat: integrate EngineArgs into NodeCommand (#13748)

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>

bin/reth/src/main.rs

@@ -3,66 +3,12 @@
 #[global_allocator]
 static ALLOC: reth_cli_util::allocator::Allocator = reth_cli_util::allocator::new_allocator();
 
-use clap::{Args, Parser};
+use clap::Parser;
 use reth::cli::Cli;
 use reth_ethereum_cli::chainspec::EthereumChainSpecParser;
-use reth_node_builder::{
-    engine_tree_config::{
-        TreeConfig, DEFAULT_MEMORY_BLOCK_BUFFER_TARGET, DEFAULT_PERSISTENCE_THRESHOLD,
-    },
-    EngineNodeLauncher,
-};
-use reth_node_ethereum::{node::EthereumAddOns, EthereumNode};
-use reth_provider::providers::BlockchainProvider;
-use reth_tracing::tracing::warn;
+use reth_node_ethereum::EthereumNode;
 use tracing::info;
 
-/// Parameters for configuring the engine
-#[derive(Debug, Clone, Args, PartialEq, Eq)]
-#[command(next_help_heading = "Engine")]
-pub struct EngineArgs {
-    /// Enable the experimental engine features on reth binary
-    ///
-    /// DEPRECATED: experimental engine is default now, use --engine.legacy to enable the legacy
-    /// functionality
-    #[arg(long = "engine.experimental", default_value = "false")]
-    pub experimental: bool,
-
-    /// Enable the legacy engine on reth binary
-    #[arg(long = "engine.legacy", default_value = "false")]
-    pub legacy: bool,
-
-    /// Configure persistence threshold for engine experimental.
-    #[arg(long = "engine.persistence-threshold", conflicts_with = "legacy", default_value_t = DEFAULT_PERSISTENCE_THRESHOLD)]
-    pub persistence_threshold: u64,
-
-    /// Configure the target number of blocks to keep in memory.
-    #[arg(long = "engine.memory-block-buffer-target", conflicts_with = "legacy", default_value_t = DEFAULT_MEMORY_BLOCK_BUFFER_TARGET)]
-    pub memory_block_buffer_target: u64,
-
-    /// Enable state root task
-    #[arg(long = "engine.state-root-task", conflicts_with = "legacy")]
-    pub state_root_task_enabled: bool,
-
-    /// Enable comparing trie updates from the state root task to the trie updates from the regular
-    /// state root calculation.
-    #[arg(long = "engine.state-root-task-compare-updates", conflicts_with = "legacy")]
-    pub state_root_task_compare_updates: bool,
-}
-
-impl Default for EngineArgs {
-    fn default() -> Self {
-        Self {
-            experimental: false,
-            legacy: false,
-            persistence_threshold: DEFAULT_PERSISTENCE_THRESHOLD,
-            memory_block_buffer_target: DEFAULT_MEMORY_BLOCK_BUFFER_TARGET,
-            state_root_task_enabled: false,
-            state_root_task_compare_updates: false,
-        }
-    }
-}
-
 fn main() {
     reth_cli_util::sigsegv_handler::install();
 
@@ -71,64 +17,12 @@ fn main() {
         std::env::set_var("RUST_BACKTRACE", "1");
     }
 
-    if let Err(err) =
-        Cli::<EthereumChainSpecParser, EngineArgs>::parse().run(|builder, engine_args| async move {
-            if engine_args.experimental {
-                warn!(target: "reth::cli", "Experimental engine is default now, and the --engine.experimental flag is deprecated. To enable the legacy functionality, use --engine.legacy.");
-            }
-
-            let use_legacy_engine = engine_args.legacy;
-            match use_legacy_engine {
-                false => {
-                    let engine_tree_config = TreeConfig::default()
-                        .with_persistence_threshold(engine_args.persistence_threshold)
-                        .with_memory_block_buffer_target(engine_args.memory_block_buffer_target)
-                        .with_state_root_task(engine_args.state_root_task_enabled)
-                        .with_always_compare_trie_updates(engine_args.state_root_task_compare_updates);
-                    let handle = builder
-                        .with_types_and_provider::<EthereumNode, BlockchainProvider<_>>()
-                        .with_components(EthereumNode::components())
-                        .with_add_ons(EthereumAddOns::default())
-                        .launch_with_fn(|builder| {
-                            let launcher = EngineNodeLauncher::new(
-                                builder.task_executor().clone(),
-                                builder.config().datadir(),
-                                engine_tree_config,
-                            );
-                            builder.launch_with(launcher)
-                        })
-                        .await?;
-                    handle.node_exit_future.await
-                }
-                true => {
-                    info!(target: "reth::cli", "Running with legacy engine");
-                    let handle = builder.launch_node(EthereumNode::default()).await?;
-                    handle.node_exit_future.await
-                }
-            }
-        })
-    {
+    if let Err(err) = Cli::<EthereumChainSpecParser>::parse().run(|builder, _| async move {
+        info!(target: "reth::cli", "Launching node");
+        let handle = builder.launch_node(EthereumNode::default()).await?;
+        handle.node_exit_future.await
+    }) {
         eprintln!("Error: {err:?}");
         std::process::exit(1);
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use clap::Parser;
-
-    /// A helper type to parse Args more easily
-    #[derive(Parser)]
-    struct CommandParser<T: Args> {
-        #[command(flatten)]
-        args: T,
-    }
-
-    #[test]
-    fn test_parse_engine_args() {
-        let default_args = EngineArgs::default();
-        let args = CommandParser::<EngineArgs>::parse_from(["reth"]).args;
-        assert_eq!(args, default_args);
-    }
-}

book/cli/reth/node.md

@@ -682,14 +682,6 @@ Pruning:
           Configure receipts log filter. Format: <`address`>:<`prune_mode`>[,<`address`>:<`prune_mode`>...] Where <`prune_mode`> can be 'full', 'distance:<`blocks`>', or 'before:<`block_number`>'
 
 Engine:
-      --engine.experimental
-          Enable the experimental engine features on reth binary
-
-          DEPRECATED: experimental engine is default now, use --engine.legacy to enable the legacy functionality
-
-      --engine.legacy
-          Enable the legacy engine on reth binary
-
       --engine.persistence-threshold <PERSISTENCE_THRESHOLD>
           Configure persistence threshold for engine experimental
 

crates/cli/commands/src/node.rs

@@ -10,7 +10,7 @@ use reth_ethereum_cli::chainspec::EthereumChainSpecParser;
 use reth_node_builder::{NodeBuilder, WithLaunchContext};
 use reth_node_core::{
     args::{
-        DatabaseArgs, DatadirArgs, DebugArgs, DevArgs, NetworkArgs, PayloadBuilderArgs,
+        DatabaseArgs, DatadirArgs, DebugArgs, DevArgs, EngineArgs, NetworkArgs, PayloadBuilderArgs,
         PruningArgs, RpcServerArgs, TxPoolArgs,
     },
     node_config::NodeConfig,
@@ -107,6 +107,10 @@ pub struct NodeCommand<
     #[command(flatten)]
     pub pruning: PruningArgs,
 
+    /// Engine cli arguments
+    #[command(flatten, next_help_heading = "Engine")]
+    pub engine: EngineArgs,
+
     /// Additional cli arguments
     #[command(flatten, next_help_heading = "Extension")]
     pub ext: Ext,
@@ -160,6 +164,7 @@ impl<
             dev,
             pruning,
             ext,
+            engine,
         } = self;
 
         // set up node config
@@ -177,6 +182,7 @@ impl<
             db,
             dev,
             pruning,
+            engine,
         };
 
         let data_dir = node_config.datadir();

crates/node/builder/src/builder/mod.rs

@@ -17,6 +17,7 @@ use reth_db_api::{
     database::Database,
     database_metrics::{DatabaseMetadata, DatabaseMetrics},
 };
+use reth_engine_tree::tree::TreeConfig;
 use reth_exex::ExExContext;
 use reth_network::{
     transactions::TransactionsManagerConfig, NetworkBuilder, NetworkConfig, NetworkConfigBuilder,
@@ -563,8 +564,16 @@ where
     > {
         let Self { builder, task_executor } = self;
 
+        let engine_tree_config = TreeConfig::default()
+            .with_persistence_threshold(builder.config.engine.persistence_threshold)
+            .with_memory_block_buffer_target(builder.config.engine.memory_block_buffer_target)
+            .with_state_root_task(builder.config.engine.state_root_task_enabled)
+            .with_always_compare_trie_updates(
+                builder.config.engine.state_root_task_compare_updates,
+            );
+
         let launcher =
-            EngineNodeLauncher::new(task_executor, builder.config.datadir(), Default::default());
+            EngineNodeLauncher::new(task_executor, builder.config.datadir(), engine_tree_config);
         builder.launch_with(launcher).await
     }
 }

crates/node/core/src/args/engine.rs

@@ -0,0 +1,58 @@
+//! clap [Args](clap::Args) for engine purposes
+
+use clap::Args;
+
+use crate::node_config::{DEFAULT_MEMORY_BLOCK_BUFFER_TARGET, DEFAULT_PERSISTENCE_THRESHOLD};
+
+/// Parameters for configuring the engine driver.
+#[derive(Debug, Clone, Args, PartialEq, Eq)]
+#[command(next_help_heading = "Engine")]
+pub struct EngineArgs {
+    /// Configure persistence threshold for engine experimental.
+    #[arg(long = "engine.persistence-threshold", default_value_t = DEFAULT_PERSISTENCE_THRESHOLD)]
+    pub persistence_threshold: u64,
+
+    /// Configure the target number of blocks to keep in memory.
+    #[arg(long = "engine.memory-block-buffer-target", default_value_t = DEFAULT_MEMORY_BLOCK_BUFFER_TARGET)]
+    pub memory_block_buffer_target: u64,
+
+    /// Enable state root task
+    #[arg(long = "engine.state-root-task")]
+    pub state_root_task_enabled: bool,
+
+    /// Enable comparing trie updates from the state root task to the trie updates from the regular
+    /// state root calculation.
+    #[arg(long = "engine.state-root-task-compare-updates")]
+    pub state_root_task_compare_updates: bool,
+}
+
+impl Default for EngineArgs {
+    fn default() -> Self {
+        Self {
+            persistence_threshold: DEFAULT_PERSISTENCE_THRESHOLD,
+            memory_block_buffer_target: DEFAULT_MEMORY_BLOCK_BUFFER_TARGET,
+            state_root_task_enabled: false,
+            state_root_task_compare_updates: false,
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use clap::Parser;
+
+    /// A helper type to parse Args more easily
+    #[derive(Parser)]
+    struct CommandParser<T: Args> {
+        #[command(flatten)]
+        args: T,
+    }
+
+    #[test]
+    fn test_parse_engine_args() {
+        let default_args = EngineArgs::default();
+        let args = CommandParser::<EngineArgs>::parse_from(["reth"]).args;
+        assert_eq!(args, default_args);
+    }
+}

crates/node/core/src/args/mod.rs

@@ -56,5 +56,9 @@ pub use datadir_args::DatadirArgs;
 mod benchmark_args;
 pub use benchmark_args::BenchmarkArgs;
 
+/// EngineArgs for configuring the engine
+mod engine;
+pub use engine::EngineArgs;
+
 mod error;
 pub mod types;

crates/node/core/src/node_config.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     args::{
-        DatabaseArgs, DatadirArgs, DebugArgs, DevArgs, NetworkArgs, PayloadBuilderArgs,
+        DatabaseArgs, DatadirArgs, DebugArgs, DevArgs, EngineArgs, NetworkArgs, PayloadBuilderArgs,
         PruningArgs, RpcServerArgs, TxPoolArgs,
     },
     dirs::{ChainPath, DataDirPath},
@@ -31,6 +31,12 @@ use std::{
 };
 use tracing::*;
 
+/// Triggers persistence when the number of canonical blocks in memory exceeds this threshold.
+pub const DEFAULT_PERSISTENCE_THRESHOLD: u64 = 2;
+
+/// How close to the canonical head we persist blocks.
+pub const DEFAULT_MEMORY_BLOCK_BUFFER_TARGET: u64 = 2;
+
 /// This includes all necessary configuration to launch the node.
 /// The individual configuration options can be overwritten before launching the node.
 ///
@@ -133,6 +139,9 @@ pub struct NodeConfig<ChainSpec> {
 
     /// All pruning related arguments
     pub pruning: PruningArgs,
+
+    /// All engine related arguments
+    pub engine: EngineArgs,
 }
 
 impl NodeConfig<ChainSpec> {
@@ -161,6 +170,7 @@ impl<ChainSpec> NodeConfig<ChainSpec> {
             dev: DevArgs::default(),
             pruning: PruningArgs::default(),
             datadir: DatadirArgs::default(),
+            engine: EngineArgs::default(),
         }
     }
 
@@ -449,6 +459,7 @@ impl<ChainSpec> NodeConfig<ChainSpec> {
             db: self.db,
             dev: self.dev,
             pruning: self.pruning,
+            engine: self.engine,
         }
     }
 }
@@ -475,6 +486,7 @@ impl<ChainSpec> Clone for NodeConfig<ChainSpec> {
             dev: self.dev,
             pruning: self.pruning.clone(),
             datadir: self.datadir.clone(),
+            engine: self.engine.clone(),
         }
     }
 }