Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/ethereum/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
101 changes: 94 additions & 7 deletions crates/ethereum/cli/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,32 @@ 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.
///
/// 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<C: ChainSpecParser = EthereumChainSpecParser, Ext: clap::Args + fmt::Debug = NoArgs>
{
pub struct Cli<
C: ChainSpecParser = EthereumChainSpecParser,
Ext: clap::Args + fmt::Debug = NoArgs,
Rpc: RpcModuleValidator = DefaultRpcModuleValidator,
> {
/// The command to run
#[command(subcommand)]
pub command: Commands<C, Ext>,

/// The logging configuration for the CLI.
#[command(flatten)]
pub logs: LogArgs,

/// Type marker for the RPC module validator
#[arg(skip)]
pub _phantom: PhantomData<Rpc>,
}

impl Cli {
Expand All @@ -54,7 +62,7 @@ impl Cli {
}
}

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

match self.command {
Commands::Node(command) => runner.run_command_until_exit(|ctx| {
command.execute(ctx, FnLauncher::new::<C, Ext>(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::<C, Ext>(launcher))
})
}
Commands::Init(command) => runner.run_blocking_until_ctrl_c(command.execute::<N>()),
Commands::InitState(command) => {
runner.run_blocking_until_ctrl_c(command.execute::<N>())
Expand Down Expand Up @@ -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
);
}
}
19 changes: 8 additions & 11 deletions crates/node/core/src/args/rpc_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -391,23 +391,20 @@ impl TypedValueParser for RpcModuleSelectionValueParser {
fn parse_ref(
&self,
_cmd: &Command,
arg: Option<&Arg>,
_arg: Option<&Arg>,
value: &OsStr,
) -> Result<Self::Value, clap::Error> {
let val =
value.to_str().ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?;
val.parse::<RpcModuleSelection>().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::<RpcModuleSelection>()
.expect("RpcModuleSelection parsing cannot fail with Other variant"))
}

fn possible_values(&self) -> Option<Box<dyn Iterator<Item = PossibleValue> + '_>> {
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))
}
}
Expand Down
3 changes: 2 additions & 1 deletion crates/optimism/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 14 additions & 4 deletions crates/optimism/cli/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Spec: ChainSpecParser, Ext: clap::Args + fmt::Debug> {
cli: Cli<Spec, Ext>,
pub struct CliApp<Spec: ChainSpecParser, Ext: clap::Args + fmt::Debug, Rpc: RpcModuleValidator> {
cli: Cli<Spec, Ext, Rpc>,
runner: Option<CliRunner>,
layers: Option<Layers>,
guard: Option<FileWorkerGuard>,
}

impl<C, Ext> CliApp<C, Ext>
impl<C, Ext, Rpc> CliApp<C, Ext, Rpc>
where
C: ChainSpecParser<ChainSpec = OpChainSpec>,
Ext: clap::Args + fmt::Debug,
Rpc: RpcModuleValidator,
{
pub(crate) fn new(cli: Cli<C, Ext>) -> Self {
pub(crate) fn new(cli: Cli<C, Ext, Rpc>) -> Self {
Self { cli, runner: None, layers: Some(Layers::new()), guard: None }
}

Expand Down Expand Up @@ -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) => {
Expand Down
19 changes: 14 additions & 5 deletions crates/optimism/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -59,15 +60,22 @@ 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<Spec: ChainSpecParser = OpChainSpecParser, Ext: clap::Args + fmt::Debug = RollupArgs>
{
pub struct Cli<
Spec: ChainSpecParser = OpChainSpecParser,
Ext: clap::Args + fmt::Debug = RollupArgs,
Rpc: RpcModuleValidator = DefaultRpcModuleValidator,
> {
/// The command to run
#[command(subcommand)]
pub command: Commands<Spec, Ext>,

/// The logging configuration for the CLI.
#[command(flatten)]
pub logs: LogArgs,

/// Type marker for the RPC module validator
#[arg(skip)]
_phantom: PhantomData<Rpc>,
}

impl Cli {
Expand All @@ -86,16 +94,17 @@ impl Cli {
}
}

impl<C, Ext> Cli<C, Ext>
impl<C, Ext, Rpc> Cli<C, Ext, Rpc>
where
C: ChainSpecParser<ChainSpec = OpChainSpec>,
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<C, Ext> {
pub fn configure(self) -> CliApp<C, Ext, Rpc> {
CliApp::new(self)
}

Expand Down
6 changes: 5 additions & 1 deletion crates/optimism/reth/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
}
Expand Down
17 changes: 9 additions & 8 deletions crates/rpc/rpc-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1708,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<F>(&self, module: RethRpcModule) -> Methods {
pub fn methods_by_module(&self, module: RethRpcModule) -> Methods {
self.methods_by(|name| name.starts_with(module.as_str()))
}

Expand Down
5 changes: 4 additions & 1 deletion crates/rpc/rpc-server-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Loading
Loading