diff --git a/.github/assets/check_wasm.sh b/.github/assets/check_wasm.sh index e140d01e796..c9dc1a41bd8 100755 --- a/.github/assets/check_wasm.sh +++ b/.github/assets/check_wasm.sh @@ -14,6 +14,7 @@ exclude_crates=( reth-cli reth-cli-commands reth-cli-runner + reth-cli-util reth-consensus-debug-client reth-db-common reth-discv4 diff --git a/Cargo.lock b/Cargo.lock index 3296270529c..3210b480d90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7453,7 +7453,9 @@ dependencies = [ "libc", "rand 0.8.5", "rand 0.9.2", + "reth-cli", "reth-fs-util", + "reth-rpc-server-types", "secp256k1 0.30.0", "serde", "snmalloc-rs", @@ -8249,12 +8251,14 @@ dependencies = [ "reth-cli", "reth-cli-commands", "reth-cli-runner", + "reth-cli-util", "reth-db", "reth-node-api", "reth-node-builder", "reth-node-core", "reth-node-ethereum", "reth-node-metrics", + "reth-rpc-server-types", "reth-tracing", "tempfile", "tracing", @@ -9179,6 +9183,7 @@ dependencies = [ "reth-cli", "reth-cli-commands", "reth-cli-runner", + "reth-cli-util", "reth-consensus", "reth-db", "reth-db-api", @@ -9198,6 +9203,7 @@ dependencies = [ "reth-primitives-traits", "reth-provider", "reth-prune", + "reth-rpc-server-types", "reth-stages", "reth-static-file", "reth-static-file-types", diff --git a/bin/reth/src/lib.rs b/bin/reth/src/lib.rs index ae07f9f3567..d974fc7b6f2 100644 --- a/bin/reth/src/lib.rs +++ b/bin/reth/src/lib.rs @@ -92,6 +92,9 @@ pub mod chainspec { pub use reth_ethereum_cli::chainspec::*; } +/// Re-exported from `reth_ethereum_cli` +pub use reth_ethereum_cli::parsers::EthereumCliParsers; + /// Re-exported from `reth_provider`. pub mod providers { pub use reth_provider::*; diff --git a/bin/reth/src/main.rs b/bin/reth/src/main.rs index a81649d1112..654fc49bb77 100644 --- a/bin/reth/src/main.rs +++ b/bin/reth/src/main.rs @@ -4,8 +4,7 @@ static ALLOC: reth_cli_util::allocator::Allocator = reth_cli_util::allocator::new_allocator(); use clap::Parser; -use reth::{args::RessArgs, cli::Cli, ress::install_ress_subprotocol}; -use reth_ethereum_cli::chainspec::EthereumChainSpecParser; +use reth::{args::RessArgs, cli::Cli, ress::install_ress_subprotocol, EthereumCliParsers}; use reth_node_builder::NodeHandle; use reth_node_ethereum::EthereumNode; use tracing::info; @@ -19,7 +18,7 @@ fn main() { } if let Err(err) = - Cli::::parse().run(async move |builder, ress_args| { + Cli::::parse().run(async move |builder, ress_args| { info!(target: "reth::cli", "Launching node"); let NodeHandle { node, node_exit_future } = builder.node(EthereumNode::default()).launch_with_debug_capabilities().await?; diff --git a/crates/cli/util/Cargo.toml b/crates/cli/util/Cargo.toml index 2ae53e18065..b1bf7d93e3c 100644 --- a/crates/cli/util/Cargo.toml +++ b/crates/cli/util/Cargo.toml @@ -13,6 +13,8 @@ workspace = true [dependencies] # reth reth-fs-util.workspace = true +reth-cli.workspace = true +reth-rpc-server-types.workspace = true # eth alloy-primitives.workspace = true diff --git a/crates/cli/util/src/lib.rs b/crates/cli/util/src/lib.rs index a82c3ba57f7..e4d20863379 100644 --- a/crates/cli/util/src/lib.rs +++ b/crates/cli/util/src/lib.rs @@ -18,7 +18,7 @@ pub use load_secret_key::get_secret_key; pub mod parsers; pub use parsers::{ hash_or_num_value_parser, parse_duration_from_secs, parse_duration_from_secs_or_ms, - parse_ether_value, parse_socket_address, + parse_ether_value, parse_socket_address, RethCliParsers, }; #[cfg(all(unix, any(target_env = "gnu", target_os = "macos")))] diff --git a/crates/cli/util/src/parsers.rs b/crates/cli/util/src/parsers.rs index ddb120452e7..f40add86c31 100644 --- a/crates/cli/util/src/parsers.rs +++ b/crates/cli/util/src/parsers.rs @@ -8,6 +8,18 @@ use std::{ time::Duration, }; +/// Trait for configuring CLI parsers across reth. +/// +/// This trait provides configurable parsers for various CLI components, +/// allowing customization of validation behavior. +pub trait RethCliParsers: Clone + Send + Sync + 'static { + /// The chain specification parser type. + type ChainSpecParser: reth_cli::chainspec::ChainSpecParser + std::fmt::Debug; + + /// The RPC module validator type. + type RpcModuleValidator: reth_rpc_server_types::RpcModuleValidator; +} + /// Helper to parse a [Duration] from seconds pub fn parse_duration_from_secs(arg: &str) -> eyre::Result { let seconds = arg.parse()?; diff --git a/crates/ethereum/cli/Cargo.toml b/crates/ethereum/cli/Cargo.toml index 491d818eb92..dc991206a9d 100644 --- a/crates/ethereum/cli/Cargo.toml +++ b/crates/ethereum/cli/Cargo.toml @@ -15,12 +15,14 @@ workspace = true reth-cli.workspace = true reth-cli-commands.workspace = true reth-cli-runner.workspace = true +reth-cli-util.workspace = true reth-chainspec.workspace = true reth-db.workspace = true reth-node-builder.workspace = true reth-node-core.workspace = true reth-node-ethereum.workspace = true reth-node-metrics.workspace = true +reth-rpc-server-types.workspace = true reth-tracing.workspace = true reth-node-api.workspace = true @@ -44,13 +46,17 @@ asm-keccak = [ ] jemalloc = [ + "reth-cli-util/jemalloc", "reth-node-core/jemalloc", "reth-node-metrics/jemalloc", ] jemalloc-prof = [ + "reth-cli-util/jemalloc-prof", "reth-node-core/jemalloc", ] -tracy-allocator = [] +tracy-allocator = [ + "reth-cli-util/tracy-allocator", +] # Because jemalloc is default and preferred over snmalloc when both features are # enabled, `--no-default-features` should be used when enabling snmalloc or diff --git a/crates/ethereum/cli/src/interface.rs b/crates/ethereum/cli/src/interface.rs index 83b79abe811..efa11d66d96 100644 --- a/crates/ethereum/cli/src/interface.rs +++ b/crates/ethereum/cli/src/interface.rs @@ -1,9 +1,8 @@ //! CLI definition and entrypoint to executable -use crate::chainspec::EthereumChainSpecParser; +use crate::parsers::EthereumCliParsers; use clap::{Parser, Subcommand}; use reth_chainspec::{ChainSpec, EthChainSpec, Hardforks}; -use reth_cli::chainspec::ChainSpecParser; use reth_cli_commands::{ common::{CliComponentsBuilder, CliHeader, CliNodeTypes}, config_cmd, db, download, dump_genesis, export_era, import, import_era, init_cmd, init_state, @@ -12,6 +11,7 @@ use reth_cli_commands::{ p2p, prune, re_execute, recover, stage, }; use reth_cli_runner::CliRunner; +use reth_cli_util::RethCliParsers; use reth_db::DatabaseEnv; use reth_node_api::NodePrimitives; use reth_node_builder::{NodeBuilder, WithLaunchContext}; @@ -27,11 +27,10 @@ use tracing::info; /// This is the entrypoint to the executable. #[derive(Debug, Parser)] #[command(author, version =version_metadata().short_version.as_ref(), long_version = version_metadata().long_version.as_ref(), about = "Reth", long_about = None)] -pub struct Cli -{ +pub struct Cli { /// The command to run #[command(subcommand)] - pub command: Commands, + pub command: Commands, /// The logging configuration for the CLI. #[command(flatten)] @@ -54,7 +53,7 @@ impl Cli { } } -impl Cli { +impl Cli { /// Execute the configured cli command. /// /// This accepts a closure that is used to launch the node via the @@ -84,14 +83,14 @@ impl Cli { /// /// ```no_run /// use clap::Parser; - /// use reth_ethereum_cli::{chainspec::EthereumChainSpecParser, interface::Cli}; + /// use reth_ethereum_cli::{interface::Cli, parsers::EthereumCliParsers}; /// /// #[derive(Debug, Parser)] /// pub struct MyArgs { /// pub enable: bool, /// } /// - /// Cli::::parse() + /// Cli::::parse() /// .run(async move |builder, my_args: MyArgs| /// // launch the node /// Ok(())) @@ -99,9 +98,17 @@ impl Cli { /// ```` pub fn run(self, launcher: L) -> eyre::Result<()> where - L: FnOnce(WithLaunchContext, C::ChainSpec>>, Ext) -> Fut, + L: FnOnce( + WithLaunchContext< + NodeBuilder< + Arc, + ::ChainSpec, + >, + >, + Ext, + ) -> Fut, Fut: Future>, - C: ChainSpecParser, + P::ChainSpecParser: reth_cli::chainspec::ChainSpecParser, { self.with_runner(CliRunner::try_default_runtime()?, launcher) } @@ -116,13 +123,18 @@ impl Cli { self, components: impl CliComponentsBuilder, launcher: impl AsyncFnOnce( - WithLaunchContext, C::ChainSpec>>, + WithLaunchContext< + NodeBuilder< + Arc, + ::ChainSpec, + >, + >, Ext, ) -> eyre::Result<()>, ) -> eyre::Result<()> where N: CliNodeTypes, ChainSpec: Hardforks>, - C: ChainSpecParser, + P::ChainSpecParser: reth_cli::chainspec::ChainSpecParser, { self.with_runner_and_components(CliRunner::try_default_runtime()?, components, launcher) } @@ -148,13 +160,22 @@ impl Cli { /// ``` pub fn with_runner(self, runner: CliRunner, launcher: L) -> eyre::Result<()> where - L: FnOnce(WithLaunchContext, C::ChainSpec>>, Ext) -> Fut, + L: FnOnce( + WithLaunchContext< + NodeBuilder< + Arc, + ::ChainSpec, + >, + >, + Ext, + ) -> Fut, Fut: Future>, - C: ChainSpecParser, + P::ChainSpecParser: reth_cli::chainspec::ChainSpecParser, { - let components = |spec: Arc| { - (EthEvmConfig::ethereum(spec.clone()), EthBeaconConsensus::new(spec)) - }; + let components = + |spec: Arc<::ChainSpec>| { + (EthEvmConfig::ethereum(spec.clone()), EthBeaconConsensus::new(spec)) + }; self.with_runner_and_components::( runner, @@ -170,13 +191,18 @@ impl Cli { runner: CliRunner, components: impl CliComponentsBuilder, launcher: impl AsyncFnOnce( - WithLaunchContext, C::ChainSpec>>, + WithLaunchContext< + NodeBuilder< + Arc, + ::ChainSpec, + >, + >, Ext, ) -> eyre::Result<()>, ) -> eyre::Result<()> where N: CliNodeTypes, ChainSpec: Hardforks>, - C: ChainSpecParser, + P::ChainSpecParser: reth_cli::chainspec::ChainSpecParser, { // Add network name if available to the logs dir if let Some(chain_spec) = self.command.chain_spec() { @@ -190,9 +216,21 @@ impl Cli { let _ = install_prometheus_recorder(); match self.command { - Commands::Node(command) => runner.run_command_until_exit(|ctx| { - command.execute(ctx, FnLauncher::new::(launcher)) - }), + Commands::Node(command) => { + // Validate RPC modules using the configured validator + if let Some(http_api) = &command.rpc.http_api { + ::validate_selection(http_api, "http.api") + .map_err(|e| eyre::eyre!("{e}"))?; + } + if let Some(ws_api) = &command.rpc.ws_api { + ::validate_selection(ws_api, "ws.api") + .map_err(|e| eyre::eyre!("{e}"))?; + } + + runner.run_command_until_exit(|ctx| { + command.execute(ctx, FnLauncher::new::(launcher)) + }) + } Commands::Init(command) => runner.run_blocking_until_ctrl_c(command.execute::()), Commands::InitState(command) => { runner.run_blocking_until_ctrl_c(command.execute::()) @@ -238,39 +276,39 @@ impl Cli { /// Commands to be executed #[derive(Debug, Subcommand)] -pub enum Commands { +pub enum Commands { /// Start the node #[command(name = "node")] - Node(Box>), + Node(Box>), /// Initialize the database from a genesis file. #[command(name = "init")] - Init(init_cmd::InitCommand), + Init(init_cmd::InitCommand), /// Initialize the database from a state dump file. #[command(name = "init-state")] - InitState(init_state::InitStateCommand), + InitState(init_state::InitStateCommand), /// This syncs RLP encoded blocks from a file. #[command(name = "import")] - Import(import::ImportCommand), + Import(import::ImportCommand), /// This syncs ERA encoded blocks from a directory. #[command(name = "import-era")] - ImportEra(import_era::ImportEraCommand), + ImportEra(import_era::ImportEraCommand), /// Exports block to era1 files in a specified directory. #[command(name = "export-era")] - ExportEra(export_era::ExportEraCommand), + ExportEra(export_era::ExportEraCommand), /// Dumps genesis block JSON configuration to stdout. - DumpGenesis(dump_genesis::DumpGenesisCommand), + DumpGenesis(dump_genesis::DumpGenesisCommand), /// Database debugging utilities #[command(name = "db")] - Db(db::Command), + Db(db::Command), /// Download public node snapshots #[command(name = "download")] - Download(download::DownloadCommand), + Download(download::DownloadCommand), /// Manipulate individual stages. #[command(name = "stage")] - Stage(stage::Command), + Stage(stage::Command), /// P2P Debugging utilities #[command(name = "p2p")] - P2P(p2p::Command), + P2P(p2p::Command), /// Generate Test Vectors #[cfg(feature = "dev")] #[command(name = "test-vectors")] @@ -280,18 +318,20 @@ pub enum Commands { Config(config_cmd::Command), /// Scripts for node recovery #[command(name = "recover")] - Recover(recover::Command), + Recover(recover::Command), /// Prune according to the configuration without any limits #[command(name = "prune")] - Prune(prune::PruneCommand), + Prune(prune::PruneCommand), /// Re-execute blocks in parallel to verify historical sync correctness. #[command(name = "re-execute")] - ReExecute(re_execute::Command), + ReExecute(re_execute::Command), } -impl Commands { +impl Commands { /// Returns the underlying chain being used for commands - pub fn chain_spec(&self) -> Option<&Arc> { + pub fn chain_spec( + &self, + ) -> Option<&Arc<::ChainSpec>> { match self { Self::Node(cmd) => cmd.chain_spec(), Self::Init(cmd) => cmd.chain_spec(), @@ -317,8 +357,9 @@ impl Commands { #[cfg(test)] mod tests { use super::*; - use crate::chainspec::SUPPORTED_CHAINS; + use crate::{chainspec::SUPPORTED_CHAINS, parsers::EthereumCliParsers}; use clap::CommandFactory; + use reth_cli_commands::node::NoArgs; use reth_node_core::args::ColorMode; #[test] @@ -332,7 +373,7 @@ mod tests { /// runtime #[test] fn test_parse_help_all_subcommands() { - let reth = Cli::::command(); + let reth = Cli::::command(); for sub_command in reth.get_subcommands() { let err = Cli::try_parse_args_from(["reth", sub_command.get_name(), "--help"]) .err() @@ -417,4 +458,87 @@ mod tests { .unwrap(); assert!(reth.run(async move |_, _| Ok(())).is_ok()); } + + #[test] + fn test_rpc_module_validation() { + use reth_rpc_server_types::RethRpcModule; + + // Test that standard modules are accepted + let cli = Cli::::try_parse_args_from([ + "reth", + "node", + "--http.api", + "eth,admin,debug", + ]) + .unwrap(); + + if let Commands::Node(command) = &cli.command { + if let Some(http_api) = &command.rpc.http_api { + // Should contain the expected modules + let modules = http_api.to_selection(); + assert!(modules.contains(&RethRpcModule::Eth)); + assert!(modules.contains(&RethRpcModule::Admin)); + assert!(modules.contains(&RethRpcModule::Debug)); + } else { + panic!("Expected http.api to be set"); + } + } else { + panic!("Expected Node command"); + } + + // Test that unknown modules are parsed as Other variant + let cli = Cli::::try_parse_args_from([ + "reth", + "node", + "--http.api", + "eth,customrpc", + ]) + .unwrap(); + + if let Commands::Node(command) = &cli.command { + if let Some(http_api) = &command.rpc.http_api { + let modules = http_api.to_selection(); + assert!(modules.contains(&RethRpcModule::Eth)); + assert!(modules.contains(&RethRpcModule::Other("customrpc".to_string()))); + } else { + panic!("Expected http.api to be set"); + } + } else { + panic!("Expected Node command"); + } + } + + #[test] + fn test_rpc_module_unknown_rejected() { + use reth_cli_runner::CliRunner; + + // Test that unknown module names are rejected during validation + let cli = Cli::::try_parse_args_from([ + "reth", + "node", + "--http.api", + "unknownmodule", + ]) + .unwrap(); + + // When we try to run the CLI with validation, it should fail + let runner = CliRunner::try_default_runtime().unwrap(); + let result = cli.with_runner(runner, |_, _| async { Ok(()) }); + + assert!(result.is_err()); + let err = result.unwrap_err(); + let err_msg = err.to_string(); + + // The error should mention it's an unknown module + assert!( + err_msg.contains("Unknown RPC module"), + "Error should mention unknown module: {}", + err_msg + ); + assert!( + err_msg.contains("'unknownmodule'"), + "Error should mention the module name: {}", + err_msg + ); + } } diff --git a/crates/ethereum/cli/src/lib.rs b/crates/ethereum/cli/src/lib.rs index 067d49d1682..86cdb9b9d7b 100644 --- a/crates/ethereum/cli/src/lib.rs +++ b/crates/ethereum/cli/src/lib.rs @@ -11,7 +11,9 @@ /// Chain specification parser. pub mod chainspec; pub mod interface; +pub mod parsers; pub use interface::Cli; +pub use parsers::EthereumCliParsers; #[cfg(test)] mod test { diff --git a/crates/ethereum/cli/src/parsers.rs b/crates/ethereum/cli/src/parsers.rs new file mode 100644 index 00000000000..210870a7c4d --- /dev/null +++ b/crates/ethereum/cli/src/parsers.rs @@ -0,0 +1,14 @@ +//! Ethereum CLI parsers implementation. + +use crate::chainspec::EthereumChainSpecParser; +use reth_cli_util::RethCliParsers; +use reth_rpc_server_types::DefaultRpcModuleValidator; + +/// Default Ethereum CLI parsers with strict RPC module validation. +#[derive(Debug, Clone, Copy)] +pub struct EthereumCliParsers; + +impl RethCliParsers for EthereumCliParsers { + type ChainSpecParser = EthereumChainSpecParser; + type RpcModuleValidator = DefaultRpcModuleValidator; +} diff --git a/crates/ethereum/reth/src/lib.rs b/crates/ethereum/reth/src/lib.rs index 7c0141dc9a0..f9952610f27 100644 --- a/crates/ethereum/reth/src/lib.rs +++ b/crates/ethereum/reth/src/lib.rs @@ -23,7 +23,11 @@ pub mod primitives { #[cfg(feature = "cli")] pub mod cli { #[doc(inline)] - pub use reth_cli_util::*; + pub use reth_cli_util::{ + allocator, get_secret_key, hash_or_num_value_parser, load_secret_key, + parse_duration_from_secs, parse_duration_from_secs_or_ms, parse_ether_value, + parse_socket_address, sigsegv_handler, RethCliParsers, + }; #[doc(inline)] pub use reth_ethereum_cli::*; } diff --git a/crates/node/core/src/args/rpc_server.rs b/crates/node/core/src/args/rpc_server.rs index afcfd7f7262..8b91b6fe2bb 100644 --- a/crates/node/core/src/args/rpc_server.rs +++ b/crates/node/core/src/args/rpc_server.rs @@ -380,7 +380,7 @@ impl Default for RpcServerArgs { } } -/// clap value parser for [`RpcModuleSelection`]. +/// clap value parser for [`RpcModuleSelection`] with configurable validation. #[derive(Clone, Debug, Default)] #[non_exhaustive] struct RpcModuleSelectionValueParser; @@ -391,23 +391,20 @@ impl TypedValueParser for RpcModuleSelectionValueParser { fn parse_ref( &self, _cmd: &Command, - arg: Option<&Arg>, + _arg: Option<&Arg>, value: &OsStr, ) -> Result { let val = value.to_str().ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?; - val.parse::().map_err(|err| { - let arg = arg.map(|a| a.to_string()).unwrap_or_else(|| "...".to_owned()); - let possible_values = RethRpcModule::all_variant_names().to_vec().join(","); - let msg = format!( - "Invalid value '{val}' for {arg}: {err}.\n [possible values: {possible_values}]" - ); - clap::Error::raw(clap::error::ErrorKind::InvalidValue, msg) - }) + // This will now accept any module name, creating Other(name) for unknowns + Ok(val + .parse::() + .expect("RpcModuleSelection parsing cannot fail with Other variant")) } fn possible_values(&self) -> Option + '_>> { - let values = RethRpcModule::all_variant_names().iter().map(PossibleValue::new); + // Only show standard modules in help text (excludes "other") + let values = RethRpcModule::standard_variant_names().map(PossibleValue::new); Some(Box::new(values)) } } diff --git a/crates/node/core/src/version.rs b/crates/node/core/src/version.rs index 85a6077709f..ac7776f3319 100644 --- a/crates/node/core/src/version.rs +++ b/crates/node/core/src/version.rs @@ -113,7 +113,7 @@ pub fn default_reth_version_metadata() -> RethCliVersionConsts { vergen_git_sha: Cow::Owned(env!("VERGEN_GIT_SHA_SHORT").to_string()), vergen_build_timestamp: Cow::Owned(env!("VERGEN_BUILD_TIMESTAMP").to_string()), vergen_cargo_target_triple: Cow::Owned(env!("VERGEN_CARGO_TARGET_TRIPLE").to_string()), - vergen_cargo_features: Cow::Owned(env!("VERGEN_CARGO_FEATURES").to_string()), + vergen_cargo_features: Cow::Owned(String::new()), short_version: Cow::Owned(env!("RETH_SHORT_VERSION").to_string()), long_version: Cow::Owned(format!( "{}\n{}\n{}\n{}\n{}", diff --git a/crates/optimism/bin/src/main.rs b/crates/optimism/bin/src/main.rs index e1567ef1c9b..e8476753c95 100644 --- a/crates/optimism/bin/src/main.rs +++ b/crates/optimism/bin/src/main.rs @@ -1,7 +1,7 @@ #![allow(missing_docs, rustdoc::missing_crate_level_docs)] use clap::Parser; -use reth_optimism_cli::{chainspec::OpChainSpecParser, Cli}; +use reth_optimism_cli::{parsers::OpCliParsers, Cli}; use reth_optimism_node::{args::RollupArgs, OpNode}; use tracing::info; @@ -17,7 +17,7 @@ fn main() { } if let Err(err) = - Cli::::parse().run(async move |builder, rollup_args| { + Cli::::parse().run(async move |builder, rollup_args| { info!(target: "reth::cli", "Launching node"); let handle = builder.node(OpNode::new(rollup_args)).launch_with_debug_capabilities().await?; diff --git a/crates/optimism/cli/Cargo.toml b/crates/optimism/cli/Cargo.toml index 0da12c42b02..f2550ba1d6b 100644 --- a/crates/optimism/cli/Cargo.toml +++ b/crates/optimism/cli/Cargo.toml @@ -12,8 +12,11 @@ workspace = true [dependencies] reth-static-file-types = { workspace = true, features = ["clap"] } +reth-cli.workspace = true reth-cli-commands.workspace = true +reth-cli-util.workspace = true reth-consensus.workspace = true +reth-rpc-server-types.workspace = true reth-primitives-traits.workspace = true reth-db = { workspace = true, features = ["mdbx", "op"] } reth-db-api.workspace = true @@ -39,7 +42,6 @@ reth-optimism-consensus.workspace = true reth-chainspec.workspace = true reth-node-events.workspace = true reth-optimism-evm.workspace = true -reth-cli.workspace = true reth-cli-runner.workspace = true reth-node-builder = { workspace = true, features = ["op"] } reth-tracing.workspace = true @@ -81,6 +83,7 @@ asm-keccak = [ # Jemalloc feature for vergen to generate correct env vars jemalloc = [ + "reth-cli-util/jemalloc", "reth-node-core/jemalloc", "reth-node-metrics/jemalloc", ] diff --git a/crates/optimism/cli/src/app.rs b/crates/optimism/cli/src/app.rs index e0774068b7e..142bbb465a1 100644 --- a/crates/optimism/cli/src/app.rs +++ b/crates/optimism/cli/src/app.rs @@ -3,6 +3,7 @@ use eyre::{eyre, Result}; use reth_cli::chainspec::ChainSpecParser; use reth_cli_commands::launcher::Launcher; use reth_cli_runner::CliRunner; +use reth_cli_util::RethCliParsers; use reth_node_metrics::recorder::install_prometheus_recorder; use reth_optimism_chainspec::OpChainSpec; use reth_optimism_consensus::OpBeaconConsensus; @@ -13,19 +14,20 @@ use tracing::info; /// A wrapper around a parsed CLI that handles command execution. #[derive(Debug)] -pub struct CliApp { - cli: Cli, +pub struct CliApp { + cli: Cli, runner: Option, layers: Option, guard: Option, } -impl CliApp +impl CliApp where - C: ChainSpecParser, + P: RethCliParsers, + P::ChainSpecParser: ChainSpecParser, Ext: clap::Args + fmt::Debug, { - pub(crate) fn new(cli: Cli) -> Self { + pub(crate) fn new(cli: Cli) -> Self { Self { cli, runner: None, layers: Some(Layers::new()), guard: None } } @@ -48,7 +50,7 @@ where /// /// This accepts a closure that is used to launch the node via the /// [`NodeCommand`](reth_cli_commands::node::NodeCommand). - pub fn run(mut self, launcher: impl Launcher) -> Result<()> { + pub fn run(mut self, launcher: impl Launcher) -> Result<()> { let runner = match self.runner.take() { Some(runner) => runner, None => CliRunner::try_default_runtime()?, @@ -71,6 +73,16 @@ where match self.cli.command { Commands::Node(command) => { + // Validate RPC modules using the configured validator + if let Some(http_api) = &command.rpc.http_api { + ::validate_selection(http_api, "http.api") + .map_err(|e| eyre!("{e}"))?; + } + if let Some(ws_api) = &command.rpc.ws_api { + ::validate_selection(ws_api, "ws.api") + .map_err(|e| eyre!("{e}"))?; + } + runner.run_command_until_exit(|ctx| command.execute(ctx, launcher)) } Commands::Init(command) => { diff --git a/crates/optimism/cli/src/commands/mod.rs b/crates/optimism/cli/src/commands/mod.rs index 32e531a6710..09370ec8ce2 100644 --- a/crates/optimism/cli/src/commands/mod.rs +++ b/crates/optimism/cli/src/commands/mod.rs @@ -1,14 +1,14 @@ -use crate::chainspec::OpChainSpecParser; +use crate::parsers::OpCliParsers; use clap::Subcommand; use import::ImportOpCommand; use import_receipts::ImportReceiptsOpCommand; use reth_chainspec::{EthChainSpec, EthereumHardforks, Hardforks}; -use reth_cli::chainspec::ChainSpecParser; use reth_cli_commands::{ config_cmd, db, dump_genesis, init_cmd, node::{self, NoArgs}, p2p, prune, re_execute, recover, stage, }; +use reth_cli_util::RethCliParsers; use std::{fmt, sync::Arc}; pub mod import; @@ -20,59 +20,61 @@ pub mod test_vectors; /// Commands to be executed #[derive(Debug, Subcommand)] -pub enum Commands -{ +pub enum Commands { /// Start the node #[command(name = "node")] - Node(Box>), + Node(Box>), /// Initialize the database from a genesis file. #[command(name = "init")] - Init(init_cmd::InitCommand), + Init(init_cmd::InitCommand), /// Initialize the database from a state dump file. #[command(name = "init-state")] - InitState(init_state::InitStateCommandOp), + InitState(init_state::InitStateCommandOp), /// This syncs RLP encoded OP blocks below Bedrock from a file, without executing. #[command(name = "import-op")] - ImportOp(ImportOpCommand), + ImportOp(ImportOpCommand), /// This imports RLP encoded receipts from a file. #[command(name = "import-receipts-op")] - ImportReceiptsOp(ImportReceiptsOpCommand), + ImportReceiptsOp(ImportReceiptsOpCommand), /// Dumps genesis block JSON configuration to stdout. - DumpGenesis(dump_genesis::DumpGenesisCommand), + DumpGenesis(dump_genesis::DumpGenesisCommand), /// Database debugging utilities #[command(name = "db")] - Db(db::Command), + Db(db::Command), /// Manipulate individual stages. #[command(name = "stage")] - Stage(Box>), + Stage(Box>), /// P2P Debugging utilities #[command(name = "p2p")] - P2P(p2p::Command), + P2P(p2p::Command), /// Write config to stdout #[command(name = "config")] Config(config_cmd::Command), /// Scripts for node recovery #[command(name = "recover")] - Recover(recover::Command), + Recover(recover::Command), /// Prune according to the configuration without any limits #[command(name = "prune")] - Prune(prune::PruneCommand), + Prune(prune::PruneCommand), /// Generate Test Vectors #[cfg(feature = "dev")] #[command(name = "test-vectors")] TestVectors(test_vectors::Command), /// Re-execute blocks in parallel to verify historical sync correctness. #[command(name = "re-execute")] - ReExecute(re_execute::Command), + ReExecute(re_execute::Command), } -impl< - C: ChainSpecParser, - Ext: clap::Args + fmt::Debug, - > Commands +impl Commands +where + P::ChainSpecParser: reth_cli::chainspec::ChainSpecParser< + ChainSpec: EthChainSpec + Hardforks + EthereumHardforks, + >, { /// Returns the underlying chain being used for commands - pub fn chain_spec(&self) -> Option<&Arc> { + pub fn chain_spec( + &self, + ) -> Option<&Arc<::ChainSpec>> { match self { Self::Node(cmd) => cmd.chain_spec(), Self::Init(cmd) => cmd.chain_spec(), diff --git a/crates/optimism/cli/src/lib.rs b/crates/optimism/cli/src/lib.rs index 4d1d22aa4d0..ffbd2e0b18c 100644 --- a/crates/optimism/cli/src/lib.rs +++ b/crates/optimism/cli/src/lib.rs @@ -14,6 +14,8 @@ pub mod app; pub mod chainspec; /// Optimism CLI commands. pub mod commands; +/// Optimism CLI parsers. +pub mod parsers; /// Module with a codec for reading and encoding receipts in files. /// /// Enables decoding and encoding `OpGethReceipt` type. See . @@ -38,13 +40,13 @@ use reth_optimism_chainspec::OpChainSpec; use std::{ffi::OsString, fmt, sync::Arc}; -use chainspec::OpChainSpecParser; use clap::{command, Parser}; use commands::Commands; use futures_util::Future; -use reth_cli::chainspec::ChainSpecParser; +use parsers::OpCliParsers; use reth_cli_commands::launcher::FnLauncher; use reth_cli_runner::CliRunner; +use reth_cli_util::RethCliParsers; use reth_db::DatabaseEnv; use reth_node_builder::{NodeBuilder, WithLaunchContext}; use reth_node_core::{args::LogArgs, version::version_metadata}; @@ -59,11 +61,10 @@ use reth_node_metrics as _; /// This is the entrypoint to the executable. #[derive(Debug, Parser)] #[command(author, version = version_metadata().short_version.as_ref(), long_version = version_metadata().long_version.as_ref(), about = "Reth", long_about = None)] -pub struct Cli -{ +pub struct Cli { /// The command to run #[command(subcommand)] - pub command: Commands, + pub command: Commands, /// The logging configuration for the CLI. #[command(flatten)] @@ -86,16 +87,17 @@ impl Cli { } } -impl Cli +impl Cli where - C: ChainSpecParser, + P: RethCliParsers, + P::ChainSpecParser: reth_cli::chainspec::ChainSpecParser, Ext: clap::Args + fmt::Debug, { /// Configures the CLI and returns a [`CliApp`] instance. /// /// This method is used to prepare the CLI for execution by wrapping it in a /// [`CliApp`] that can be further configured before running. - pub fn configure(self) -> CliApp { + pub fn configure(self) -> CliApp { CliApp::new(self) } @@ -105,7 +107,15 @@ where /// [`NodeCommand`](reth_cli_commands::node::NodeCommand). pub fn run(self, launcher: L) -> eyre::Result<()> where - L: FnOnce(WithLaunchContext, C::ChainSpec>>, Ext) -> Fut, + L: FnOnce( + WithLaunchContext< + NodeBuilder< + Arc, + ::ChainSpec, + >, + >, + Ext, + ) -> Fut, Fut: Future>, { self.with_runner(CliRunner::try_default_runtime()?, launcher) @@ -114,12 +124,20 @@ where /// Execute the configured cli command with the provided [`CliRunner`]. pub fn with_runner(self, runner: CliRunner, launcher: L) -> eyre::Result<()> where - L: FnOnce(WithLaunchContext, C::ChainSpec>>, Ext) -> Fut, + L: FnOnce( + WithLaunchContext< + NodeBuilder< + Arc, + ::ChainSpec, + >, + >, + Ext, + ) -> Fut, Fut: Future>, { let mut this = self.configure(); this.set_runner(runner); - this.run(FnLauncher::new::(async move |builder, chain_spec| { + this.run(FnLauncher::new::(async move |builder, chain_spec| { launcher(builder, chain_spec).await })) } @@ -127,7 +145,7 @@ where #[cfg(test)] mod test { - use crate::{chainspec::OpChainSpecParser, commands::Commands, Cli}; + use crate::{chainspec::OpChainSpecParser, commands::Commands, parsers::OpCliParsers, Cli}; use clap::Parser; use reth_cli_commands::{node::NoArgs, NodeCommand}; use reth_optimism_chainspec::{BASE_MAINNET, OP_DEV}; @@ -153,7 +171,7 @@ mod test { #[test] fn parse_node() { - let cmd = Cli::::parse_from([ + let cmd = Cli::::parse_from([ "op-reth", "node", "--chain", diff --git a/crates/optimism/cli/src/parsers.rs b/crates/optimism/cli/src/parsers.rs new file mode 100644 index 00000000000..edb255d6849 --- /dev/null +++ b/crates/optimism/cli/src/parsers.rs @@ -0,0 +1,14 @@ +//! Optimism CLI parsers implementation. + +use crate::chainspec::OpChainSpecParser; +use reth_cli_util::RethCliParsers; +use reth_rpc_server_types::DefaultRpcModuleValidator; + +/// Default Optimism CLI parsers with strict RPC module validation. +#[derive(Debug, Clone, Copy)] +pub struct OpCliParsers; + +impl RethCliParsers for OpCliParsers { + type ChainSpecParser = OpChainSpecParser; + type RpcModuleValidator = DefaultRpcModuleValidator; +} diff --git a/crates/optimism/reth/src/lib.rs b/crates/optimism/reth/src/lib.rs index dd5fb5ba6c8..939dbc750da 100644 --- a/crates/optimism/reth/src/lib.rs +++ b/crates/optimism/reth/src/lib.rs @@ -24,7 +24,11 @@ pub mod primitives { #[cfg(feature = "cli")] pub mod cli { #[doc(inline)] - pub use reth_cli_util::*; + pub use reth_cli_util::{ + allocator, get_secret_key, hash_or_num_value_parser, load_secret_key, + parse_duration_from_secs, parse_duration_from_secs_or_ms, parse_ether_value, + parse_socket_address, sigsegv_handler, RethCliParsers, + }; #[doc(inline)] pub use reth_optimism_cli::*; } diff --git a/crates/rpc/rpc-builder/src/lib.rs b/crates/rpc/rpc-builder/src/lib.rs index 76e889eec63..ca890dbff06 100644 --- a/crates/rpc/rpc-builder/src/lib.rs +++ b/crates/rpc/rpc-builder/src/lib.rs @@ -915,81 +915,95 @@ where let namespaces: Vec<_> = namespaces.collect(); namespaces .iter() - .copied() - .map(|namespace| { - self.modules - .entry(namespace) - .or_insert_with(|| match namespace { - RethRpcModule::Admin => { - AdminApi::new(self.network.clone(), self.provider.chain_spec()) - .into_rpc() - .into() - } - RethRpcModule::Debug => { - DebugApi::new(eth_api.clone(), self.blocking_pool_guard.clone()) - .into_rpc() - .into() - } - RethRpcModule::Eth => { - // merge all eth handlers - let mut module = eth_api.clone().into_rpc(); - module.merge(eth_filter.clone().into_rpc()).expect("No conflicts"); - module.merge(eth_pubsub.clone().into_rpc()).expect("No conflicts"); - module - .merge( - EthBundle::new( - eth_api.clone(), - self.blocking_pool_guard.clone(), - ) - .into_rpc(), - ) - .expect("No conflicts"); + .filter_map(|namespace| { + // Skip Other variants - they should be registered via extend_rpc_modules + if namespace.is_other() { + return None; + } - module.into() - } - RethRpcModule::Net => { - NetApi::new(self.network.clone(), eth_api.clone()).into_rpc().into() - } - RethRpcModule::Trace => TraceApi::new( - eth_api.clone(), - self.blocking_pool_guard.clone(), - self.eth_config, - ) - .into_rpc() - .into(), - RethRpcModule::Web3 => Web3Api::new(self.network.clone()).into_rpc().into(), - RethRpcModule::Txpool => TxPoolApi::new( - self.eth.api.pool().clone(), - self.eth.api.tx_resp_builder().clone(), - ) - .into_rpc() - .into(), - RethRpcModule::Rpc => RPCApi::new( - namespaces - .iter() - .map(|module| (module.to_string(), "1.0".to_string())) - .collect(), - ) - .into_rpc() - .into(), - RethRpcModule::Ots => OtterscanApi::new(eth_api.clone()).into_rpc().into(), - RethRpcModule::Reth => { - RethApi::new(self.provider.clone(), self.executor.clone()) - .into_rpc() - .into() - } - // only relevant for Ethereum and configured in `EthereumAddOns` - // implementation - // TODO: can we get rid of this here? - RethRpcModule::Flashbots => Default::default(), - RethRpcModule::Miner => MinerApi::default().into_rpc().into(), - RethRpcModule::Mev => { - EthSimBundle::new(eth_api.clone(), self.blocking_pool_guard.clone()) - .into_rpc() - .into() - } - }) - .clone() + Some( + self.modules + .entry(namespace.clone()) + .or_insert_with(|| match namespace.clone() { + RethRpcModule::Admin => { + AdminApi::new(self.network.clone(), self.provider.chain_spec()) + .into_rpc() + .into() + } + RethRpcModule::Debug => { + DebugApi::new(eth_api.clone(), self.blocking_pool_guard.clone()) + .into_rpc() + .into() + } + RethRpcModule::Eth => { + // merge all eth handlers + let mut module = eth_api.clone().into_rpc(); + module.merge(eth_filter.clone().into_rpc()).expect("No conflicts"); + module.merge(eth_pubsub.clone().into_rpc()).expect("No conflicts"); + module + .merge( + EthBundle::new( + eth_api.clone(), + self.blocking_pool_guard.clone(), + ) + .into_rpc(), + ) + .expect("No conflicts"); + + module.into() + } + RethRpcModule::Net => { + NetApi::new(self.network.clone(), eth_api.clone()).into_rpc().into() + } + RethRpcModule::Trace => TraceApi::new( + eth_api.clone(), + self.blocking_pool_guard.clone(), + self.eth_config, + ) + .into_rpc() + .into(), + RethRpcModule::Web3 => { + Web3Api::new(self.network.clone()).into_rpc().into() + } + RethRpcModule::Txpool => TxPoolApi::new( + self.eth.api.pool().clone(), + self.eth.api.tx_resp_builder().clone(), + ) + .into_rpc() + .into(), + RethRpcModule::Rpc => RPCApi::new( + namespaces + .iter() + .map(|module| (module.to_string(), "1.0".to_string())) + .collect(), + ) + .into_rpc() + .into(), + RethRpcModule::Ots => { + OtterscanApi::new(eth_api.clone()).into_rpc().into() + } + RethRpcModule::Reth => { + RethApi::new(self.provider.clone(), self.executor.clone()) + .into_rpc() + .into() + } + // only relevant for Ethereum and configured in `EthereumAddOns` + // implementation + // TODO: can we get rid of this here? + RethRpcModule::Flashbots => Default::default(), + // Other variants are filtered out above + RethRpcModule::Other(_) => { + unreachable!("Other variants are filtered out") + } + RethRpcModule::Miner => MinerApi::default().into_rpc().into(), + RethRpcModule::Mev => { + EthSimBundle::new(eth_api.clone(), self.blocking_pool_guard.clone()) + .into_rpc() + .into() + } + }) + .clone(), + ) }) .collect::>() } @@ -1570,9 +1584,9 @@ impl TransportRpcModuleConfig { let ws_modules = self.ws.as_ref().map(RpcModuleSelection::to_selection).unwrap_or_default(); - let http_not_ws = http_modules.difference(&ws_modules).copied().collect(); - let ws_not_http = ws_modules.difference(&http_modules).copied().collect(); - let overlap = http_modules.intersection(&ws_modules).copied().collect(); + let http_not_ws = http_modules.difference(&ws_modules).cloned().collect(); + let ws_not_http = ws_modules.difference(&http_modules).cloned().collect(); + let overlap = http_modules.intersection(&ws_modules).cloned().collect(); Err(WsHttpSamePortError::ConflictingModules(Box::new(ConflictingModules { overlap, @@ -1708,7 +1722,7 @@ impl TransportRpcModules { /// Returns all unique endpoints installed for the given module. /// /// Note: In case of duplicate method names this only record the first occurrence. - pub fn methods_by_module(&self, module: RethRpcModule) -> Methods { + pub fn methods_by_module(&self, module: RethRpcModule) -> Methods { self.methods_by(|name| name.starts_with(module.as_str())) } diff --git a/crates/rpc/rpc-server-types/src/lib.rs b/crates/rpc/rpc-server-types/src/lib.rs index c20b578816b..2c7203241c0 100644 --- a/crates/rpc/rpc-server-types/src/lib.rs +++ b/crates/rpc/rpc-server-types/src/lib.rs @@ -13,6 +13,9 @@ pub mod constants; pub mod result; mod module; -pub use module::{RethRpcModule, RpcModuleSelection}; +pub use module::{ + DefaultRpcModuleValidator, LenientRpcModuleValidator, RethRpcModule, RpcModuleSelection, + RpcModuleValidator, +}; pub use result::ToRpcResult; diff --git a/crates/rpc/rpc-server-types/src/module.rs b/crates/rpc/rpc-server-types/src/module.rs index fdca41cc196..9b90337e5d0 100644 --- a/crates/rpc/rpc-server-types/src/module.rs +++ b/crates/rpc/rpc-server-types/src/module.rs @@ -1,7 +1,7 @@ use std::{collections::HashSet, fmt, str::FromStr}; use serde::{Deserialize, Serialize, Serializer}; -use strum::{AsRefStr, EnumIter, IntoStaticStr, ParseError, VariantArray, VariantNames}; +use strum::{ParseError, VariantNames}; /// Describes the modules that should be installed. /// @@ -102,8 +102,8 @@ impl RpcModuleSelection { pub fn iter_selection(&self) -> Box + '_> { match self { Self::All => Box::new(RethRpcModule::modules().into_iter()), - Self::Standard => Box::new(Self::STANDARD_MODULES.iter().copied()), - Self::Selection(s) => Box::new(s.iter().copied()), + Self::Standard => Box::new(Self::STANDARD_MODULES.iter().cloned()), + Self::Selection(s) => Box::new(s.iter().cloned()), } } @@ -165,7 +165,7 @@ impl From> for RpcModuleSelection { impl From<&[RethRpcModule]> for RpcModuleSelection { fn from(s: &[RethRpcModule]) -> Self { - Self::Selection(s.iter().copied().collect()) + Self::Selection(s.iter().cloned().collect()) } } @@ -177,7 +177,7 @@ impl From> for RpcModuleSelection { impl From<[RethRpcModule; N]> for RpcModuleSelection { fn from(s: [RethRpcModule; N]) -> Self { - Self::Selection(s.iter().copied().collect()) + Self::Selection(s.iter().cloned().collect()) } } @@ -186,7 +186,7 @@ impl<'a> FromIterator<&'a RethRpcModule> for RpcModuleSelection { where I: IntoIterator, { - iter.into_iter().copied().collect() + iter.into_iter().cloned().collect() } } @@ -230,20 +230,7 @@ impl fmt::Display for RpcModuleSelection { } /// Represents RPC modules that are supported by reth -#[derive( - Debug, - Clone, - Copy, - Eq, - PartialEq, - Hash, - AsRefStr, - IntoStaticStr, - VariantNames, - VariantArray, - EnumIter, - Deserialize, -)] +#[derive(Debug, Clone, Eq, PartialEq, Hash, VariantNames, Deserialize)] #[serde(rename_all = "snake_case")] #[strum(serialize_all = "kebab-case")] pub enum RethRpcModule { @@ -273,36 +260,90 @@ pub enum RethRpcModule { Miner, /// `mev_` module Mev, + /// Custom RPC module not part of the standard set + #[strum(default)] + #[serde(untagged)] + Other(String), } // === impl RethRpcModule === impl RethRpcModule { - /// Returns the number of variants in the enum + /// All standard variants (excludes Other) + const STANDARD_VARIANTS: &'static [Self] = &[ + Self::Admin, + Self::Debug, + Self::Eth, + Self::Net, + Self::Trace, + Self::Txpool, + Self::Web3, + Self::Rpc, + Self::Reth, + Self::Ots, + Self::Flashbots, + Self::Miner, + Self::Mev, + ]; + + /// Returns the number of standard variants (excludes Other) pub const fn variant_count() -> usize { - ::VARIANTS.len() + Self::STANDARD_VARIANTS.len() } - /// Returns all variant names of the enum + /// Returns all variant names including Other (for parsing) pub const fn all_variant_names() -> &'static [&'static str] { ::VARIANTS } - /// Returns all variants of the enum + /// Returns standard variant names (excludes "other") for CLI display + pub fn standard_variant_names() -> impl Iterator { + ::VARIANTS.iter().copied().filter(|&name| name != "other") + } + + /// Returns all standard variants (excludes Other) pub const fn all_variants() -> &'static [Self] { - ::VARIANTS + Self::STANDARD_VARIANTS } - /// Returns all variants of the enum - pub fn modules() -> impl IntoIterator { - use strum::IntoEnumIterator; - Self::iter() + /// Returns iterator over standard modules only + pub fn modules() -> impl IntoIterator + Clone { + Self::STANDARD_VARIANTS.iter().cloned() } /// Returns the string representation of the module. - #[inline] - pub fn as_str(&self) -> &'static str { - self.into() + pub fn as_str(&self) -> &str { + match self { + Self::Other(s) => s.as_str(), + _ => self.as_ref(), // Uses AsRefStr trait + } + } + + /// Returns true if this is an `Other` variant. + pub const fn is_other(&self) -> bool { + matches!(self, Self::Other(_)) + } +} + +impl AsRef for RethRpcModule { + fn as_ref(&self) -> &str { + match self { + Self::Other(s) => s.as_str(), + // For standard variants, use the derive-generated static strings + Self::Admin => "admin", + Self::Debug => "debug", + Self::Eth => "eth", + Self::Net => "net", + Self::Trace => "trace", + Self::Txpool => "txpool", + Self::Web3 => "web3", + Self::Rpc => "rpc", + Self::Reth => "reth", + Self::Ots => "ots", + Self::Flashbots => "flashbots", + Self::Miner => "miner", + Self::Mev => "mev", + } } } @@ -324,7 +365,8 @@ impl FromStr for RethRpcModule { "flashbots" => Self::Flashbots, "miner" => Self::Miner, "mev" => Self::Mev, - _ => return Err(ParseError::VariantNotFound), + // Any unknown module becomes Other + other => Self::Other(other.to_string()), }) } } @@ -347,7 +389,81 @@ impl Serialize for RethRpcModule { where S: Serializer, { - s.serialize_str(self.as_ref()) + s.serialize_str(self.as_str()) + } +} + +/// Trait for validating RPC module selections. +/// +/// This allows customizing how RPC module names are validated when parsing +/// CLI arguments or configuration. +pub trait RpcModuleValidator: Clone + Send + Sync + 'static { + /// Parse and validate an RPC module selection string. + fn parse_selection(s: &str) -> Result; + + /// Validates RPC module selection that was already parsed. + /// + /// This is used to validate modules that were parsed as `Other` variants + /// to ensure they meet the validation rules of the specific implementation. + fn validate_selection(modules: &RpcModuleSelection, arg_name: &str) -> Result<(), String> { + // Re-validate the modules using the parser's validator + // This is necessary because the clap value parser accepts any input + // and we need to validate according to the specific parser's rules + let RpcModuleSelection::Selection(module_set) = modules else { + // All or Standard variants are always valid + return Ok(()); + }; + + for module in module_set { + let RethRpcModule::Other(name) = module else { + // Standard modules are always valid + continue; + }; + + // Try to parse and validate using the configured validator + // This will check for typos and other validation rules + Self::parse_selection(name) + .map_err(|e| format!("Invalid RPC module '{name}' in {arg_name}: {e}"))?; + } + + Ok(()) + } +} + +/// Default validator that rejects unknown module names. +/// +/// This validator only accepts known RPC module names. +#[derive(Debug, Clone, Copy)] +pub struct DefaultRpcModuleValidator; + +impl RpcModuleValidator for DefaultRpcModuleValidator { + fn parse_selection(s: &str) -> Result { + // First try standard parsing + let selection = RpcModuleSelection::from_str(s) + .map_err(|e| format!("Failed to parse RPC modules: {}", e))?; + + // Validate each module in the selection + if let RpcModuleSelection::Selection(modules) = &selection { + for module in modules { + if let RethRpcModule::Other(name) = module { + return Err(format!("Unknown RPC module: '{}'", name)); + } + } + } + + Ok(selection) + } +} + +/// Lenient validator that accepts any module name without validation. +/// +/// This validator accepts any module name, including unknown ones. +#[derive(Debug, Clone, Copy)] +pub struct LenientRpcModuleValidator; + +impl RpcModuleValidator for LenientRpcModuleValidator { + fn parse_selection(s: &str) -> Result { + RpcModuleSelection::from_str(s).map_err(|e| format!("Failed to parse RPC modules: {}", e)) } } @@ -559,10 +675,12 @@ mod test { assert!(result.is_ok()); assert_eq!(result.unwrap(), expected_selection); - // Test invalid selection should return error + // Test custom module selections now work (no longer return errors) let result = RpcModuleSelection::from_str("invalid,unknown"); - assert!(result.is_err()); - assert_eq!(result.unwrap_err(), ParseError::VariantNotFound); + assert!(result.is_ok()); + let selection = result.unwrap(); + assert!(selection.contains(&RethRpcModule::Other("invalid".to_string()))); + assert!(selection.contains(&RethRpcModule::Other("unknown".to_string()))); // Test single valid selection: "eth" let result = RpcModuleSelection::from_str("eth"); @@ -570,9 +688,160 @@ mod test { let expected_selection = RpcModuleSelection::from([RethRpcModule::Eth]); assert_eq!(result.unwrap(), expected_selection); - // Test single invalid selection: "unknown" + // Test single custom module selection: "unknown" now becomes Other let result = RpcModuleSelection::from_str("unknown"); + assert!(result.is_ok()); + let expected_selection = + RpcModuleSelection::from([RethRpcModule::Other("unknown".to_string())]); + assert_eq!(result.unwrap(), expected_selection); + } + + #[test] + fn test_rpc_module_other_variant() { + // Test parsing custom module + let custom_module = RethRpcModule::from_str("myCustomModule").unwrap(); + assert_eq!(custom_module, RethRpcModule::Other("myCustomModule".to_string())); + + // Test as_str for Other variant + assert_eq!(custom_module.as_str(), "myCustomModule"); + + // Test as_ref for Other variant + assert_eq!(custom_module.as_ref(), "myCustomModule"); + + // Test Display impl + assert_eq!(custom_module.to_string(), "myCustomModule"); + } + + #[test] + fn test_rpc_module_selection_with_mixed_modules() { + // Test selection with both standard and custom modules + let result = RpcModuleSelection::from_str("eth,admin,myCustomModule,anotherCustom"); + assert!(result.is_ok()); + + let selection = result.unwrap(); + assert!(selection.contains(&RethRpcModule::Eth)); + assert!(selection.contains(&RethRpcModule::Admin)); + assert!(selection.contains(&RethRpcModule::Other("myCustomModule".to_string()))); + assert!(selection.contains(&RethRpcModule::Other("anotherCustom".to_string()))); + } + + #[test] + fn test_rpc_module_all_excludes_custom() { + // Test that All selection doesn't include custom modules + let all_selection = RpcModuleSelection::All; + + // All should contain standard modules + assert!(all_selection.contains(&RethRpcModule::Eth)); + assert!(all_selection.contains(&RethRpcModule::Admin)); + + // But All doesn't explicitly contain custom modules + // (though contains() returns true for all modules when selection is All) + assert_eq!(all_selection.len(), RethRpcModule::variant_count()); + } + + #[test] + fn test_rpc_module_equality_with_other() { + let other1 = RethRpcModule::Other("custom".to_string()); + let other2 = RethRpcModule::Other("custom".to_string()); + let other3 = RethRpcModule::Other("different".to_string()); + + assert_eq!(other1, other2); + assert_ne!(other1, other3); + assert_ne!(other1, RethRpcModule::Eth); + } + + #[test] + fn test_rpc_module_is_other() { + // Standard modules should return false + assert!(!RethRpcModule::Eth.is_other()); + assert!(!RethRpcModule::Admin.is_other()); + assert!(!RethRpcModule::Debug.is_other()); + + // Other variants should return true + assert!(RethRpcModule::Other("custom".to_string()).is_other()); + assert!(RethRpcModule::Other("mycustomrpc".to_string()).is_other()); + } + + #[test] + fn test_standard_variant_names_excludes_other() { + let standard_names: Vec<_> = RethRpcModule::standard_variant_names().collect(); + + // Verify "other" is not in the list + assert!(!standard_names.contains(&"other")); + + // Should have exactly as many names as STANDARD_VARIANTS + assert_eq!(standard_names.len(), RethRpcModule::STANDARD_VARIANTS.len()); + + // Verify all standard variants have their names in the list + for variant in RethRpcModule::STANDARD_VARIANTS { + assert!(standard_names.contains(&variant.as_ref())); + } + } + + #[test] + fn test_default_validator_accepts_standard_modules() { + // Should accept standard modules + let result = DefaultRpcModuleValidator::parse_selection("eth,admin,debug"); + assert!(result.is_ok()); + + let selection = result.unwrap(); + assert!(matches!(selection, RpcModuleSelection::Selection(_))); + } + + #[test] + fn test_default_validator_rejects_unknown_modules() { + // Should reject unknown module names + let result = DefaultRpcModuleValidator::parse_selection("eth,mycustom"); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Unknown RPC module: 'mycustom'")); + + let result = DefaultRpcModuleValidator::parse_selection("unknownmodule"); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Unknown RPC module: 'unknownmodule'")); + + let result = DefaultRpcModuleValidator::parse_selection("eth,admin,xyz123"); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Unknown RPC module: 'xyz123'")); + } + + #[test] + fn test_default_validator_all_selection() { + // Should accept "all" selection + let result = DefaultRpcModuleValidator::parse_selection("all"); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), RpcModuleSelection::All); + } + + #[test] + fn test_default_validator_none_selection() { + // Should accept "none" selection + let result = DefaultRpcModuleValidator::parse_selection("none"); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), RpcModuleSelection::Selection(Default::default())); + } + + #[test] + fn test_lenient_validator_accepts_unknown_modules() { + // Lenient validator should accept any module name without validation + let result = LenientRpcModuleValidator::parse_selection("eht,adimn,xyz123,customrpc"); + assert!(result.is_ok()); + + let selection = result.unwrap(); + if let RpcModuleSelection::Selection(modules) = selection { + assert!(modules.contains(&RethRpcModule::Other("eht".to_string()))); + assert!(modules.contains(&RethRpcModule::Other("adimn".to_string()))); + assert!(modules.contains(&RethRpcModule::Other("xyz123".to_string()))); + assert!(modules.contains(&RethRpcModule::Other("customrpc".to_string()))); + } else { + panic!("Expected Selection variant"); + } + } + + #[test] + fn test_default_validator_mixed_standard_and_custom() { + // Should reject mix of standard and custom modules + let result = DefaultRpcModuleValidator::parse_selection("eth,admin,mycustom,debug"); assert!(result.is_err()); - assert_eq!(result.unwrap_err(), ParseError::VariantNotFound); + assert!(result.unwrap_err().contains("Unknown RPC module: 'mycustom'")); } } diff --git a/examples/beacon-api-sidecar-fetcher/src/main.rs b/examples/beacon-api-sidecar-fetcher/src/main.rs index 382261a39d2..20d32cf98fc 100644 --- a/examples/beacon-api-sidecar-fetcher/src/main.rs +++ b/examples/beacon-api-sidecar-fetcher/src/main.rs @@ -23,7 +23,7 @@ use clap::Parser; use futures_util::{stream::FuturesUnordered, StreamExt}; use mined_sidecar::MinedSidecarStream; use reth_ethereum::{ - cli::{chainspec::EthereumChainSpecParser, interface::Cli}, + cli::{Cli, EthereumCliParsers}, node::{builder::NodeHandle, EthereumNode}, provider::CanonStateSubscriptions, }; @@ -31,7 +31,7 @@ use reth_ethereum::{ pub mod mined_sidecar; fn main() { - Cli::::parse() + Cli::::parse() .run(|builder, beacon_config| async move { // launch the node let NodeHandle { node, node_exit_future } = diff --git a/examples/beacon-api-sse/src/main.rs b/examples/beacon-api-sse/src/main.rs index fee20e09b1f..c8871195295 100644 --- a/examples/beacon-api-sse/src/main.rs +++ b/examples/beacon-api-sse/src/main.rs @@ -22,14 +22,14 @@ use clap::Parser; use futures_util::stream::StreamExt; use mev_share_sse::{client::EventStream, EventClient}; use reth_ethereum::{ - cli::{chainspec::EthereumChainSpecParser, interface::Cli}, + cli::{Cli, EthereumCliParsers}, node::EthereumNode, }; use std::net::{IpAddr, Ipv4Addr}; use tracing::{info, warn}; fn main() { - Cli::::parse() + Cli::::parse() .run(|builder, args| async move { let handle = builder.node(EthereumNode::default()).launch().await?; diff --git a/examples/custom-inspector/src/main.rs b/examples/custom-inspector/src/main.rs index 739038ae6de..f65b80ed3ba 100644 --- a/examples/custom-inspector/src/main.rs +++ b/examples/custom-inspector/src/main.rs @@ -17,7 +17,7 @@ use alloy_rpc_types_eth::{state::EvmOverrides, TransactionRequest}; use clap::Parser; use futures_util::StreamExt; use reth_ethereum::{ - cli::{chainspec::EthereumChainSpecParser, interface::Cli}, + cli::{Cli, EthereumCliParsers}, evm::{ primitives::ConfigureEvm, revm::revm::{ @@ -33,7 +33,7 @@ use reth_ethereum::{ }; fn main() { - Cli::::parse() + Cli::::parse() .run(|builder, args| async move { // launch the node let NodeHandle { node, node_exit_future } = diff --git a/examples/node-custom-rpc/src/main.rs b/examples/node-custom-rpc/src/main.rs index 8504949d9d9..04751f6fa1e 100644 --- a/examples/node-custom-rpc/src/main.rs +++ b/examples/node-custom-rpc/src/main.rs @@ -21,7 +21,7 @@ use jsonrpsee::{ PendingSubscriptionSink, SubscriptionMessage, }; use reth_ethereum::{ - cli::{chainspec::EthereumChainSpecParser, interface::Cli}, + cli::{Cli, EthereumCliParsers}, node::EthereumNode, pool::TransactionPool, }; @@ -29,7 +29,7 @@ use std::time::Duration; use tokio::time::sleep; fn main() { - Cli::::parse() + Cli::::parse() .run(|builder, args| async move { let handle = builder // configure default ethereum node diff --git a/examples/txpool-tracing/src/main.rs b/examples/txpool-tracing/src/main.rs index a1b61422cb9..86da5dd2be3 100644 --- a/examples/txpool-tracing/src/main.rs +++ b/examples/txpool-tracing/src/main.rs @@ -15,7 +15,7 @@ use alloy_rpc_types_trace::{parity::TraceType, tracerequest::TraceCallRequest}; use clap::Parser; use futures_util::StreamExt; use reth_ethereum::{ - cli::{chainspec::EthereumChainSpecParser, interface::Cli}, + cli::{Cli, EthereumCliParsers}, node::{builder::NodeHandle, EthereumNode}, pool::TransactionPool, rpc::eth::primitives::TransactionRequest, @@ -24,7 +24,7 @@ use reth_ethereum::{ mod submit; fn main() { - Cli::::parse() + Cli::::parse() .run(|builder, args| async move { // launch the node let NodeHandle { node, node_exit_future } =