From 9d712bbf4c9651a78ab1b2660268db82ced892dc Mon Sep 17 00:00:00 2001 From: Federico Gimenez Date: Fri, 8 Aug 2025 00:13:20 +0000 Subject: [PATCH 1/2] feat(rpc): add Other(String) variant to RethRpcModule for custom RPC modules --- crates/node/core/src/args/rpc_server.rs | 15 +- crates/rpc/rpc-builder/src/lib.rs | 15 +- crates/rpc/rpc-server-types/src/module.rs | 170 +++++++++++++++++----- 3 files changed, 144 insertions(+), 56 deletions(-) diff --git a/crates/node/core/src/args/rpc_server.rs b/crates/node/core/src/args/rpc_server.rs index afcfd7f7262..b0d7ee1d878 100644 --- a/crates/node/core/src/args/rpc_server.rs +++ b/crates/node/core/src/args/rpc_server.rs @@ -391,22 +391,19 @@ 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 + '_>> { + // Only show standard modules in help text let values = RethRpcModule::all_variant_names().iter().map(PossibleValue::new); Some(Box::new(values)) } diff --git a/crates/rpc/rpc-builder/src/lib.rs b/crates/rpc/rpc-builder/src/lib.rs index 76e889eec63..ff79430276e 100644 --- a/crates/rpc/rpc-builder/src/lib.rs +++ b/crates/rpc/rpc-builder/src/lib.rs @@ -915,11 +915,10 @@ where let namespaces: Vec<_> = namespaces.collect(); namespaces .iter() - .copied() .map(|namespace| { self.modules - .entry(namespace) - .or_insert_with(|| match namespace { + .entry(namespace.clone()) + .or_insert_with(|| match namespace.clone() { RethRpcModule::Admin => { AdminApi::new(self.network.clone(), self.provider.chain_spec()) .into_rpc() @@ -981,7 +980,9 @@ where // only relevant for Ethereum and configured in `EthereumAddOns` // implementation // TODO: can we get rid of this here? - RethRpcModule::Flashbots => Default::default(), + // Custom modules are not handled here - they should be registered via + // extend_rpc_modules + RethRpcModule::Flashbots | RethRpcModule::Other(_) => Default::default(), RethRpcModule::Miner => MinerApi::default().into_rpc().into(), RethRpcModule::Mev => { EthSimBundle::new(eth_api.clone(), self.blocking_pool_guard.clone()) @@ -1570,9 +1571,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, diff --git a/crates/rpc/rpc-server-types/src/module.rs b/crates/rpc/rpc-server-types/src/module.rs index fdca41cc196..be814598e86 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,80 @@ 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 of standard modules pub const fn all_variant_names() -> &'static [&'static str] { ::VARIANTS } - /// Returns all variants of the enum + /// 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 + } + } +} + +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 +355,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 +379,7 @@ impl Serialize for RethRpcModule { where S: Serializer, { - s.serialize_str(self.as_ref()) + s.serialize_str(self.as_str()) } } @@ -559,10 +591,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 +604,65 @@ 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_err()); - assert_eq!(result.unwrap_err(), ParseError::VariantNotFound); + 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); } } From 6ac6d3addf955d64e7d3b6fd752da2e37cead843 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 29 Aug 2025 13:53:08 +0300 Subject: [PATCH 2/2] wip Co-authored-by: Federico Gimenez --- Cargo.lock | 2 + crates/ethereum/cli/Cargo.toml | 1 + crates/ethereum/cli/src/interface.rs | 101 +++++++++++- crates/node/core/src/args/rpc_server.rs | 6 +- crates/optimism/cli/Cargo.toml | 3 +- crates/optimism/cli/src/app.rs | 18 ++- crates/optimism/cli/src/lib.rs | 19 ++- crates/optimism/reth/src/lib.rs | 6 +- crates/rpc/rpc-builder/src/lib.rs | 2 +- crates/rpc/rpc-server-types/src/lib.rs | 5 +- crates/rpc/rpc-server-types/src/module.rs | 181 +++++++++++++++++++++- 11 files changed, 320 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3296270529c..3c90a976287 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8255,6 +8255,7 @@ dependencies = [ "reth-node-core", "reth-node-ethereum", "reth-node-metrics", + "reth-rpc-server-types", "reth-tracing", "tempfile", "tracing", @@ -9198,6 +9199,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/crates/ethereum/cli/Cargo.toml b/crates/ethereum/cli/Cargo.toml index 491d818eb92..01a7751e77b 100644 --- a/crates/ethereum/cli/Cargo.toml +++ b/crates/ethereum/cli/Cargo.toml @@ -21,6 +21,7 @@ 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 diff --git a/crates/ethereum/cli/src/interface.rs b/crates/ethereum/cli/src/interface.rs index 83b79abe811..41e4728e1f3 100644 --- a/crates/ethereum/cli/src/interface.rs +++ b/crates/ethereum/cli/src/interface.rs @@ -18,8 +18,9 @@ use reth_node_builder::{NodeBuilder, WithLaunchContext}; use reth_node_core::{args::LogArgs, version::version_metadata}; use reth_node_ethereum::{consensus::EthBeaconConsensus, EthEvmConfig, EthereumNode}; use reth_node_metrics::recorder::install_prometheus_recorder; +use reth_rpc_server_types::{DefaultRpcModuleValidator, RpcModuleValidator}; use reth_tracing::FileWorkerGuard; -use std::{ffi::OsString, fmt, future::Future, sync::Arc}; +use std::{ffi::OsString, fmt, future::Future, marker::PhantomData, sync::Arc}; use tracing::info; /// The main reth cli interface. @@ -27,8 +28,11 @@ 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< + C: ChainSpecParser = EthereumChainSpecParser, + Ext: clap::Args + fmt::Debug = NoArgs, + Rpc: RpcModuleValidator = DefaultRpcModuleValidator, +> { /// The command to run #[command(subcommand)] pub command: Commands, @@ -36,6 +40,10 @@ pub struct Cli, } impl Cli { @@ -54,7 +62,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 @@ -190,9 +198,20 @@ 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 { + Rpc::validate_selection(http_api, "http.api") + .map_err(|e| eyre::eyre!("{e}"))?; + } + if let Some(ws_api) = &command.rpc.ws_api { + Rpc::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::()) @@ -417,4 +436,72 @@ 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/node/core/src/args/rpc_server.rs b/crates/node/core/src/args/rpc_server.rs index b0d7ee1d878..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; @@ -403,8 +403,8 @@ impl TypedValueParser for RpcModuleSelectionValueParser { } fn possible_values(&self) -> Option + '_>> { - // Only show standard modules in help text - 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/optimism/cli/Cargo.toml b/crates/optimism/cli/Cargo.toml index 0da12c42b02..422da3b883e 100644 --- a/crates/optimism/cli/Cargo.toml +++ b/crates/optimism/cli/Cargo.toml @@ -12,8 +12,10 @@ workspace = true [dependencies] reth-static-file-types = { workspace = true, features = ["clap"] } +reth-cli.workspace = true reth-cli-commands.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 +41,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 diff --git a/crates/optimism/cli/src/app.rs b/crates/optimism/cli/src/app.rs index e0774068b7e..84c111ecbfa 100644 --- a/crates/optimism/cli/src/app.rs +++ b/crates/optimism/cli/src/app.rs @@ -7,25 +7,27 @@ use reth_node_metrics::recorder::install_prometheus_recorder; use reth_optimism_chainspec::OpChainSpec; use reth_optimism_consensus::OpBeaconConsensus; use reth_optimism_node::{OpExecutorProvider, OpNode}; +use reth_rpc_server_types::RpcModuleValidator; use reth_tracing::{FileWorkerGuard, Layers}; use std::{fmt, sync::Arc}; 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, Ext: clap::Args + fmt::Debug, + Rpc: RpcModuleValidator, { - pub(crate) fn new(cli: Cli) -> Self { + pub(crate) fn new(cli: Cli) -> Self { Self { cli, runner: None, layers: Some(Layers::new()), guard: None } } @@ -71,6 +73,14 @@ where match self.cli.command { Commands::Node(command) => { + // Validate RPC modules using the configured validator + if let Some(http_api) = &command.rpc.http_api { + Rpc::validate_selection(http_api, "http.api").map_err(|e| eyre!("{e}"))?; + } + if let Some(ws_api) = &command.rpc.ws_api { + Rpc::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/lib.rs b/crates/optimism/cli/src/lib.rs index 4d1d22aa4d0..529ad19bdb2 100644 --- a/crates/optimism/cli/src/lib.rs +++ b/crates/optimism/cli/src/lib.rs @@ -35,8 +35,9 @@ pub mod ovm_file_codec; pub use app::CliApp; pub use commands::{import::ImportOpCommand, import_receipts::ImportReceiptsOpCommand}; use reth_optimism_chainspec::OpChainSpec; +use reth_rpc_server_types::{DefaultRpcModuleValidator, RpcModuleValidator}; -use std::{ffi::OsString, fmt, sync::Arc}; +use std::{ffi::OsString, fmt, marker::PhantomData, sync::Arc}; use chainspec::OpChainSpecParser; use clap::{command, Parser}; @@ -59,8 +60,11 @@ 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< + Spec: ChainSpecParser = OpChainSpecParser, + Ext: clap::Args + fmt::Debug = RollupArgs, + Rpc: RpcModuleValidator = DefaultRpcModuleValidator, +> { /// The command to run #[command(subcommand)] pub command: Commands, @@ -68,6 +72,10 @@ pub struct Cli, } impl Cli { @@ -86,16 +94,17 @@ impl Cli { } } -impl Cli +impl Cli where C: ChainSpecParser, Ext: clap::Args + fmt::Debug, + Rpc: RpcModuleValidator, { /// 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) } diff --git a/crates/optimism/reth/src/lib.rs b/crates/optimism/reth/src/lib.rs index dd5fb5ba6c8..10cd2bd01f9 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, + }; #[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 ff79430276e..a9e8aaa3795 100644 --- a/crates/rpc/rpc-builder/src/lib.rs +++ b/crates/rpc/rpc-builder/src/lib.rs @@ -1709,7 +1709,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 be814598e86..9b90337e5d0 100644 --- a/crates/rpc/rpc-server-types/src/module.rs +++ b/crates/rpc/rpc-server-types/src/module.rs @@ -291,11 +291,16 @@ impl RethRpcModule { Self::STANDARD_VARIANTS.len() } - /// Returns all variant names of standard modules + /// Returns all variant names including Other (for parsing) pub const fn all_variant_names() -> &'static [&'static str] { ::VARIANTS } + /// 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] { Self::STANDARD_VARIANTS @@ -313,6 +318,11 @@ impl RethRpcModule { _ => 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 { @@ -383,6 +393,80 @@ impl Serialize for RethRpcModule { } } +/// 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)) + } +} + #[cfg(test)] mod test { use super::*; @@ -665,4 +749,99 @@ mod test { 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!(result.unwrap_err().contains("Unknown RPC module: 'mycustom'")); + } }