Skip to content

Commit 90aa99c

Browse files
klkvrfgimenez
andauthored
feat: support customizable RPC namespace parsers (#18160)
Co-authored-by: Federico Gimenez <[email protected]>
1 parent 394a53d commit 90aa99c

File tree

11 files changed

+461
-77
lines changed

11 files changed

+461
-77
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ethereum/cli/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ reth-node-builder.workspace = true
2121
reth-node-core.workspace = true
2222
reth-node-ethereum.workspace = true
2323
reth-node-metrics.workspace = true
24+
reth-rpc-server-types.workspace = true
2425
reth-tracing.workspace = true
2526
reth-node-api.workspace = true
2627

crates/ethereum/cli/src/interface.rs

Lines changed: 94 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,24 +18,32 @@ use reth_node_builder::{NodeBuilder, WithLaunchContext};
1818
use reth_node_core::{args::LogArgs, version::version_metadata};
1919
use reth_node_ethereum::{consensus::EthBeaconConsensus, EthEvmConfig, EthereumNode};
2020
use reth_node_metrics::recorder::install_prometheus_recorder;
21+
use reth_rpc_server_types::{DefaultRpcModuleValidator, RpcModuleValidator};
2122
use reth_tracing::FileWorkerGuard;
22-
use std::{ffi::OsString, fmt, future::Future, sync::Arc};
23+
use std::{ffi::OsString, fmt, future::Future, marker::PhantomData, sync::Arc};
2324
use tracing::info;
2425

2526
/// The main reth cli interface.
2627
///
2728
/// This is the entrypoint to the executable.
2829
#[derive(Debug, Parser)]
2930
#[command(author, version =version_metadata().short_version.as_ref(), long_version = version_metadata().long_version.as_ref(), about = "Reth", long_about = None)]
30-
pub struct Cli<C: ChainSpecParser = EthereumChainSpecParser, Ext: clap::Args + fmt::Debug = NoArgs>
31-
{
31+
pub struct Cli<
32+
C: ChainSpecParser = EthereumChainSpecParser,
33+
Ext: clap::Args + fmt::Debug = NoArgs,
34+
Rpc: RpcModuleValidator = DefaultRpcModuleValidator,
35+
> {
3236
/// The command to run
3337
#[command(subcommand)]
3438
pub command: Commands<C, Ext>,
3539

3640
/// The logging configuration for the CLI.
3741
#[command(flatten)]
3842
pub logs: LogArgs,
43+
44+
/// Type marker for the RPC module validator
45+
#[arg(skip)]
46+
pub _phantom: PhantomData<Rpc>,
3947
}
4048

4149
impl Cli {
@@ -54,7 +62,7 @@ impl Cli {
5462
}
5563
}
5664

57-
impl<C: ChainSpecParser, Ext: clap::Args + fmt::Debug> Cli<C, Ext> {
65+
impl<C: ChainSpecParser, Ext: clap::Args + fmt::Debug, Rpc: RpcModuleValidator> Cli<C, Ext, Rpc> {
5866
/// Execute the configured cli command.
5967
///
6068
/// This accepts a closure that is used to launch the node via the
@@ -190,9 +198,20 @@ impl<C: ChainSpecParser, Ext: clap::Args + fmt::Debug> Cli<C, Ext> {
190198
let _ = install_prometheus_recorder();
191199

192200
match self.command {
193-
Commands::Node(command) => runner.run_command_until_exit(|ctx| {
194-
command.execute(ctx, FnLauncher::new::<C, Ext>(launcher))
195-
}),
201+
Commands::Node(command) => {
202+
// Validate RPC modules using the configured validator
203+
if let Some(http_api) = &command.rpc.http_api {
204+
Rpc::validate_selection(http_api, "http.api")
205+
.map_err(|e| eyre::eyre!("{e}"))?;
206+
}
207+
if let Some(ws_api) = &command.rpc.ws_api {
208+
Rpc::validate_selection(ws_api, "ws.api").map_err(|e| eyre::eyre!("{e}"))?;
209+
}
210+
211+
runner.run_command_until_exit(|ctx| {
212+
command.execute(ctx, FnLauncher::new::<C, Ext>(launcher))
213+
})
214+
}
196215
Commands::Init(command) => runner.run_blocking_until_ctrl_c(command.execute::<N>()),
197216
Commands::InitState(command) => {
198217
runner.run_blocking_until_ctrl_c(command.execute::<N>())
@@ -417,4 +436,72 @@ mod tests {
417436
.unwrap();
418437
assert!(reth.run(async move |_, _| Ok(())).is_ok());
419438
}
439+
440+
#[test]
441+
fn test_rpc_module_validation() {
442+
use reth_rpc_server_types::RethRpcModule;
443+
444+
// Test that standard modules are accepted
445+
let cli =
446+
Cli::try_parse_args_from(["reth", "node", "--http.api", "eth,admin,debug"]).unwrap();
447+
448+
if let Commands::Node(command) = &cli.command {
449+
if let Some(http_api) = &command.rpc.http_api {
450+
// Should contain the expected modules
451+
let modules = http_api.to_selection();
452+
assert!(modules.contains(&RethRpcModule::Eth));
453+
assert!(modules.contains(&RethRpcModule::Admin));
454+
assert!(modules.contains(&RethRpcModule::Debug));
455+
} else {
456+
panic!("Expected http.api to be set");
457+
}
458+
} else {
459+
panic!("Expected Node command");
460+
}
461+
462+
// Test that unknown modules are parsed as Other variant
463+
let cli =
464+
Cli::try_parse_args_from(["reth", "node", "--http.api", "eth,customrpc"]).unwrap();
465+
466+
if let Commands::Node(command) = &cli.command {
467+
if let Some(http_api) = &command.rpc.http_api {
468+
let modules = http_api.to_selection();
469+
assert!(modules.contains(&RethRpcModule::Eth));
470+
assert!(modules.contains(&RethRpcModule::Other("customrpc".to_string())));
471+
} else {
472+
panic!("Expected http.api to be set");
473+
}
474+
} else {
475+
panic!("Expected Node command");
476+
}
477+
}
478+
479+
#[test]
480+
fn test_rpc_module_unknown_rejected() {
481+
use reth_cli_runner::CliRunner;
482+
483+
// Test that unknown module names are rejected during validation
484+
let cli =
485+
Cli::try_parse_args_from(["reth", "node", "--http.api", "unknownmodule"]).unwrap();
486+
487+
// When we try to run the CLI with validation, it should fail
488+
let runner = CliRunner::try_default_runtime().unwrap();
489+
let result = cli.with_runner(runner, |_, _| async { Ok(()) });
490+
491+
assert!(result.is_err());
492+
let err = result.unwrap_err();
493+
let err_msg = err.to_string();
494+
495+
// The error should mention it's an unknown module
496+
assert!(
497+
err_msg.contains("Unknown RPC module"),
498+
"Error should mention unknown module: {}",
499+
err_msg
500+
);
501+
assert!(
502+
err_msg.contains("'unknownmodule'"),
503+
"Error should mention the module name: {}",
504+
err_msg
505+
);
506+
}
420507
}

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ impl Default for RpcServerArgs {
407407
}
408408
}
409409

410-
/// clap value parser for [`RpcModuleSelection`].
410+
/// clap value parser for [`RpcModuleSelection`] with configurable validation.
411411
#[derive(Clone, Debug, Default)]
412412
#[non_exhaustive]
413413
struct RpcModuleSelectionValueParser;
@@ -418,23 +418,20 @@ impl TypedValueParser for RpcModuleSelectionValueParser {
418418
fn parse_ref(
419419
&self,
420420
_cmd: &Command,
421-
arg: Option<&Arg>,
421+
_arg: Option<&Arg>,
422422
value: &OsStr,
423423
) -> Result<Self::Value, clap::Error> {
424424
let val =
425425
value.to_str().ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?;
426-
val.parse::<RpcModuleSelection>().map_err(|err| {
427-
let arg = arg.map(|a| a.to_string()).unwrap_or_else(|| "...".to_owned());
428-
let possible_values = RethRpcModule::all_variant_names().to_vec().join(",");
429-
let msg = format!(
430-
"Invalid value '{val}' for {arg}: {err}.\n [possible values: {possible_values}]"
431-
);
432-
clap::Error::raw(clap::error::ErrorKind::InvalidValue, msg)
433-
})
426+
// This will now accept any module name, creating Other(name) for unknowns
427+
Ok(val
428+
.parse::<RpcModuleSelection>()
429+
.expect("RpcModuleSelection parsing cannot fail with Other variant"))
434430
}
435431

436432
fn possible_values(&self) -> Option<Box<dyn Iterator<Item = PossibleValue> + '_>> {
437-
let values = RethRpcModule::all_variant_names().iter().map(PossibleValue::new);
433+
// Only show standard modules in help text (excludes "other")
434+
let values = RethRpcModule::standard_variant_names().map(PossibleValue::new);
438435
Some(Box::new(values))
439436
}
440437
}

crates/optimism/cli/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@ workspace = true
1212

1313
[dependencies]
1414
reth-static-file-types = { workspace = true, features = ["clap"] }
15+
reth-cli.workspace = true
1516
reth-cli-commands.workspace = true
1617
reth-consensus.workspace = true
18+
reth-rpc-server-types.workspace = true
1719
reth-primitives-traits.workspace = true
1820
reth-db = { workspace = true, features = ["mdbx", "op"] }
1921
reth-db-api.workspace = true
@@ -39,7 +41,6 @@ reth-optimism-consensus.workspace = true
3941
reth-chainspec.workspace = true
4042
reth-node-events.workspace = true
4143
reth-optimism-evm.workspace = true
42-
reth-cli.workspace = true
4344
reth-cli-runner.workspace = true
4445
reth-node-builder = { workspace = true, features = ["op"] }
4546
reth-tracing.workspace = true

crates/optimism/cli/src/app.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,25 +7,27 @@ use reth_node_metrics::recorder::install_prometheus_recorder;
77
use reth_optimism_chainspec::OpChainSpec;
88
use reth_optimism_consensus::OpBeaconConsensus;
99
use reth_optimism_node::{OpExecutorProvider, OpNode};
10+
use reth_rpc_server_types::RpcModuleValidator;
1011
use reth_tracing::{FileWorkerGuard, Layers};
1112
use std::{fmt, sync::Arc};
1213
use tracing::info;
1314

1415
/// A wrapper around a parsed CLI that handles command execution.
1516
#[derive(Debug)]
16-
pub struct CliApp<Spec: ChainSpecParser, Ext: clap::Args + fmt::Debug> {
17-
cli: Cli<Spec, Ext>,
17+
pub struct CliApp<Spec: ChainSpecParser, Ext: clap::Args + fmt::Debug, Rpc: RpcModuleValidator> {
18+
cli: Cli<Spec, Ext, Rpc>,
1819
runner: Option<CliRunner>,
1920
layers: Option<Layers>,
2021
guard: Option<FileWorkerGuard>,
2122
}
2223

23-
impl<C, Ext> CliApp<C, Ext>
24+
impl<C, Ext, Rpc> CliApp<C, Ext, Rpc>
2425
where
2526
C: ChainSpecParser<ChainSpec = OpChainSpec>,
2627
Ext: clap::Args + fmt::Debug,
28+
Rpc: RpcModuleValidator,
2729
{
28-
pub(crate) fn new(cli: Cli<C, Ext>) -> Self {
30+
pub(crate) fn new(cli: Cli<C, Ext, Rpc>) -> Self {
2931
Self { cli, runner: None, layers: Some(Layers::new()), guard: None }
3032
}
3133

@@ -71,6 +73,14 @@ where
7173

7274
match self.cli.command {
7375
Commands::Node(command) => {
76+
// Validate RPC modules using the configured validator
77+
if let Some(http_api) = &command.rpc.http_api {
78+
Rpc::validate_selection(http_api, "http.api").map_err(|e| eyre!("{e}"))?;
79+
}
80+
if let Some(ws_api) = &command.rpc.ws_api {
81+
Rpc::validate_selection(ws_api, "ws.api").map_err(|e| eyre!("{e}"))?;
82+
}
83+
7484
runner.run_command_until_exit(|ctx| command.execute(ctx, launcher))
7585
}
7686
Commands::Init(command) => {

crates/optimism/cli/src/lib.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,9 @@ pub mod ovm_file_codec;
3535
pub use app::CliApp;
3636
pub use commands::{import::ImportOpCommand, import_receipts::ImportReceiptsOpCommand};
3737
use reth_optimism_chainspec::OpChainSpec;
38+
use reth_rpc_server_types::{DefaultRpcModuleValidator, RpcModuleValidator};
3839

39-
use std::{ffi::OsString, fmt, sync::Arc};
40+
use std::{ffi::OsString, fmt, marker::PhantomData, sync::Arc};
4041

4142
use chainspec::OpChainSpecParser;
4243
use clap::{command, Parser};
@@ -59,15 +60,22 @@ use reth_node_metrics as _;
5960
/// This is the entrypoint to the executable.
6061
#[derive(Debug, Parser)]
6162
#[command(author, version = version_metadata().short_version.as_ref(), long_version = version_metadata().long_version.as_ref(), about = "Reth", long_about = None)]
62-
pub struct Cli<Spec: ChainSpecParser = OpChainSpecParser, Ext: clap::Args + fmt::Debug = RollupArgs>
63-
{
63+
pub struct Cli<
64+
Spec: ChainSpecParser = OpChainSpecParser,
65+
Ext: clap::Args + fmt::Debug = RollupArgs,
66+
Rpc: RpcModuleValidator = DefaultRpcModuleValidator,
67+
> {
6468
/// The command to run
6569
#[command(subcommand)]
6670
pub command: Commands<Spec, Ext>,
6771

6872
/// The logging configuration for the CLI.
6973
#[command(flatten)]
7074
pub logs: LogArgs,
75+
76+
/// Type marker for the RPC module validator
77+
#[arg(skip)]
78+
_phantom: PhantomData<Rpc>,
7179
}
7280

7381
impl Cli {
@@ -86,16 +94,17 @@ impl Cli {
8694
}
8795
}
8896

89-
impl<C, Ext> Cli<C, Ext>
97+
impl<C, Ext, Rpc> Cli<C, Ext, Rpc>
9098
where
9199
C: ChainSpecParser<ChainSpec = OpChainSpec>,
92100
Ext: clap::Args + fmt::Debug,
101+
Rpc: RpcModuleValidator,
93102
{
94103
/// Configures the CLI and returns a [`CliApp`] instance.
95104
///
96105
/// This method is used to prepare the CLI for execution by wrapping it in a
97106
/// [`CliApp`] that can be further configured before running.
98-
pub fn configure(self) -> CliApp<C, Ext> {
107+
pub fn configure(self) -> CliApp<C, Ext, Rpc> {
99108
CliApp::new(self)
100109
}
101110

crates/optimism/reth/src/lib.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ pub mod primitives {
2424
#[cfg(feature = "cli")]
2525
pub mod cli {
2626
#[doc(inline)]
27-
pub use reth_cli_util::*;
27+
pub use reth_cli_util::{
28+
allocator, get_secret_key, hash_or_num_value_parser, load_secret_key,
29+
parse_duration_from_secs, parse_duration_from_secs_or_ms, parse_ether_value,
30+
parse_socket_address, sigsegv_handler,
31+
};
2832
#[doc(inline)]
2933
pub use reth_optimism_cli::*;
3034
}

crates/rpc/rpc-builder/src/lib.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -919,11 +919,10 @@ where
919919
let namespaces: Vec<_> = namespaces.collect();
920920
namespaces
921921
.iter()
922-
.copied()
923922
.map(|namespace| {
924923
self.modules
925-
.entry(namespace)
926-
.or_insert_with(|| match namespace {
924+
.entry(namespace.clone())
925+
.or_insert_with(|| match namespace.clone() {
927926
RethRpcModule::Admin => {
928927
AdminApi::new(self.network.clone(), self.provider.chain_spec())
929928
.into_rpc()
@@ -985,7 +984,9 @@ where
985984
// only relevant for Ethereum and configured in `EthereumAddOns`
986985
// implementation
987986
// TODO: can we get rid of this here?
988-
RethRpcModule::Flashbots => Default::default(),
987+
// Custom modules are not handled here - they should be registered via
988+
// extend_rpc_modules
989+
RethRpcModule::Flashbots | RethRpcModule::Other(_) => Default::default(),
989990
RethRpcModule::Miner => MinerApi::default().into_rpc().into(),
990991
RethRpcModule::Mev => {
991992
EthSimBundle::new(eth_api.clone(), self.blocking_pool_guard.clone())
@@ -1574,9 +1575,9 @@ impl TransportRpcModuleConfig {
15741575
let ws_modules =
15751576
self.ws.as_ref().map(RpcModuleSelection::to_selection).unwrap_or_default();
15761577

1577-
let http_not_ws = http_modules.difference(&ws_modules).copied().collect();
1578-
let ws_not_http = ws_modules.difference(&http_modules).copied().collect();
1579-
let overlap = http_modules.intersection(&ws_modules).copied().collect();
1578+
let http_not_ws = http_modules.difference(&ws_modules).cloned().collect();
1579+
let ws_not_http = ws_modules.difference(&http_modules).cloned().collect();
1580+
let overlap = http_modules.intersection(&ws_modules).cloned().collect();
15801581

15811582
Err(WsHttpSamePortError::ConflictingModules(Box::new(ConflictingModules {
15821583
overlap,
@@ -1712,7 +1713,7 @@ impl TransportRpcModules {
17121713
/// Returns all unique endpoints installed for the given module.
17131714
///
17141715
/// Note: In case of duplicate method names this only record the first occurrence.
1715-
pub fn methods_by_module<F>(&self, module: RethRpcModule) -> Methods {
1716+
pub fn methods_by_module(&self, module: RethRpcModule) -> Methods {
17161717
self.methods_by(|name| name.starts_with(module.as_str()))
17171718
}
17181719

crates/rpc/rpc-server-types/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ pub mod constants;
1313
pub mod result;
1414

1515
mod module;
16-
pub use module::{RethRpcModule, RpcModuleSelection};
16+
pub use module::{
17+
DefaultRpcModuleValidator, LenientRpcModuleValidator, RethRpcModule, RpcModuleSelection,
18+
RpcModuleValidator,
19+
};
1720

1821
pub use result::ToRpcResult;

0 commit comments

Comments
 (0)