From 8e7768db2aa4416d5dd209040342f4151cba6f9f Mon Sep 17 00:00:00 2001 From: Tien Nguyen <116023870+htiennv@users.noreply.github.com> Date: Sun, 12 Jan 2025 00:27:11 +0700 Subject: [PATCH] feat: integrate EngineArgs into NodeCommand (#13748) Co-authored-by: Matthias Seitz --- bin/reth/src/main.rs | 120 ++----------------------- book/cli/reth/node.md | 8 -- crates/cli/commands/src/node.rs | 8 +- crates/node/builder/src/builder/mod.rs | 11 ++- crates/node/core/src/args/engine.rs | 58 ++++++++++++ crates/node/core/src/args/mod.rs | 4 + crates/node/core/src/node_config.rs | 14 ++- 7 files changed, 99 insertions(+), 124 deletions(-) create mode 100644 crates/node/core/src/args/engine.rs diff --git a/bin/reth/src/main.rs b/bin/reth/src/main.rs index b263f03a3..645a8e5d5 100644 --- a/bin/reth/src/main.rs +++ b/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::::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::>() - .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::::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 { - #[command(flatten)] - args: T, - } - - #[test] - fn test_parse_engine_args() { - let default_args = EngineArgs::default(); - let args = CommandParser::::parse_from(["reth"]).args; - assert_eq!(args, default_args); - } -} diff --git a/book/cli/reth/node.md b/book/cli/reth/node.md index 66a04a1db..210a3506e 100644 --- a/book/cli/reth/node.md +++ b/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 Configure persistence threshold for engine experimental diff --git a/crates/cli/commands/src/node.rs b/crates/cli/commands/src/node.rs index b099a2c05..189ca6b79 100644 --- a/crates/cli/commands/src/node.rs +++ b/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(); diff --git a/crates/node/builder/src/builder/mod.rs b/crates/node/builder/src/builder/mod.rs index 82e81209a..8134f8d56 100644 --- a/crates/node/builder/src/builder/mod.rs +++ b/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 } } diff --git a/crates/node/core/src/args/engine.rs b/crates/node/core/src/args/engine.rs new file mode 100644 index 000000000..411074d28 --- /dev/null +++ b/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 { + #[command(flatten)] + args: T, + } + + #[test] + fn test_parse_engine_args() { + let default_args = EngineArgs::default(); + let args = CommandParser::::parse_from(["reth"]).args; + assert_eq!(args, default_args); + } +} diff --git a/crates/node/core/src/args/mod.rs b/crates/node/core/src/args/mod.rs index 7f1b64361..1649b8b56 100644 --- a/crates/node/core/src/args/mod.rs +++ b/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; diff --git a/crates/node/core/src/node_config.rs b/crates/node/core/src/node_config.rs index 861e47fc3..eb8aa2378 100644 --- a/crates/node/core/src/node_config.rs +++ b/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 { /// All pruning related arguments pub pruning: PruningArgs, + + /// All engine related arguments + pub engine: EngineArgs, } impl NodeConfig { @@ -161,6 +170,7 @@ impl NodeConfig { dev: DevArgs::default(), pruning: PruningArgs::default(), datadir: DatadirArgs::default(), + engine: EngineArgs::default(), } } @@ -449,6 +459,7 @@ impl NodeConfig { db: self.db, dev: self.dev, pruning: self.pruning, + engine: self.engine, } } } @@ -475,6 +486,7 @@ impl Clone for NodeConfig { dev: self.dev, pruning: self.pruning.clone(), datadir: self.datadir.clone(), + engine: self.engine.clone(), } } }