refactor(cli): Make some run params reusable (#13870)

* refactor: extract TelemetryParams and PrometheusParams

* improve run_cmd docs

* extract `RuntimeParams`

* use `normalize`

* keep params types same style

* improve `max_runtime_instances`

* fmt

* add license and improve code

* Update client/cli/src/params/runtime_params.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

---------

Co-authored-by: Bastian Köcher <git@kchr.de>

substrate/client/cli/src/commands/run_cmd.rs

@@ -23,7 +23,7 @@ use crate::{
 		ImportParams, KeystoreParams, NetworkParams, OffchainWorkerParams, SharedParams,
 		TransactionPoolParams,
 	},
-	CliConfiguration,
+	CliConfiguration, PrometheusParams, RuntimeParams, TelemetryParams,
 };
 use clap::Parser;
 use regex::Regex;
@@ -116,12 +116,6 @@ pub struct RunCmd {
 	#[arg(long)]
 	pub rpc_max_subscriptions_per_connection: Option<usize>,
 
-	/// Expose Prometheus exporter on all interfaces.
-	///
-	/// Default is local.
-	#[arg(long)]
-	pub prometheus_external: bool,
-
 	/// DEPRECATED, IPC support has been removed.
 	#[arg(long, value_name = "PATH")]
 	pub ipc_path: Option<String>,
@@ -151,36 +145,23 @@ pub struct RunCmd {
 	#[arg(long, value_name = "ORIGINS", value_parser = parse_cors)]
 	pub rpc_cors: Option<Cors>,
 
-	/// Specify Prometheus exporter TCP Port.
-	#[arg(long, value_name = "PORT")]
-	pub prometheus_port: Option<u16>,
-
-	/// Do not expose a Prometheus exporter endpoint.
-	///
-	/// Prometheus metric endpoint is enabled by default.
-	#[arg(long)]
-	pub no_prometheus: bool,
-
 	/// The human-readable name for this node.
 	///
-	/// The node name will be reported to the telemetry server, if enabled.
+	/// It's used as network node name.
 	#[arg(long, value_name = "NAME")]
 	pub name: Option<String>,
 
-	/// Disable connecting to the Substrate telemetry server.
-	///
-	/// Telemetry is on by default on global chains.
-	#[arg(long)]
-	pub no_telemetry: bool,
+	#[allow(missing_docs)]
+	#[clap(flatten)]
+	pub telemetry_params: TelemetryParams,
 
-	/// The URL of the telemetry server to connect to.
-	///
-	/// This flag can be passed multiple times as a means to specify multiple
-	/// telemetry endpoints. Verbosity levels range from 0-9, with 0 denoting
-	/// the least verbosity.
-	/// Expected format is 'URL VERBOSITY', e.g. `--telemetry-url 'wss://foo/bar 0'`.
-	#[arg(long = "telemetry-url", value_name = "URL VERBOSITY", value_parser = parse_telemetry_endpoints)]
-	pub telemetry_endpoints: Vec<(String, u8)>,
+	#[allow(missing_docs)]
+	#[clap(flatten)]
+	pub prometheus_params: PrometheusParams,
+
+	#[allow(missing_docs)]
+	#[clap(flatten)]
+	pub runtime_params: RuntimeParams,
 
 	#[allow(missing_docs)]
 	#[clap(flatten)]
@@ -202,6 +183,10 @@ pub struct RunCmd {
 	#[clap(flatten)]
 	pub pool_config: TransactionPoolParams,
 
+	#[allow(missing_docs)]
+	#[clap(flatten)]
+	pub keystore_params: KeystoreParams,
+
 	/// Shortcut for `--name Alice --validator` with session keys for `Alice` added to keystore.
 	#[arg(long, conflicts_with_all = &["bob", "charlie", "dave", "eve", "ferdie", "one", "two"])]
 	pub alice: bool,
@@ -239,20 +224,6 @@ pub struct RunCmd {
 	#[arg(long)]
 	pub force_authoring: bool,
 
-	#[allow(missing_docs)]
-	#[clap(flatten)]
-	pub keystore_params: KeystoreParams,
-
-	/// The size of the instances cache for each runtime.
-	///
-	/// The default value is 8 and the values higher than 256 are ignored.
-	#[arg(long)]
-	pub max_runtime_instances: Option<usize>,
-
-	/// Maximum number of different runtimes that can be cached.
-	#[arg(long, default_value_t = 2)]
-	pub runtime_cache_size: u8,
-
 	/// Run a temporary node.
 	///
 	/// A temporary directory will be created to store the configuration and will be deleted
@@ -345,11 +316,12 @@ impl CliConfiguration for RunCmd {
 		&self,
 		chain_spec: &Box<dyn ChainSpec>,
 	) -> Result<Option<TelemetryEndpoints>> {
-		Ok(if self.no_telemetry {
+		let params = &self.telemetry_params;
+		Ok(if params.no_telemetry {
 			None
-		} else if !self.telemetry_endpoints.is_empty() {
+		} else if !params.telemetry_endpoints.is_empty() {
 			Some(
-				TelemetryEndpoints::new(self.telemetry_endpoints.clone())
+				TelemetryEndpoints::new(params.telemetry_endpoints.clone())
 					.map_err(|e| e.to_string())?,
 			)
 		} else {
@@ -361,7 +333,7 @@ impl CliConfiguration for RunCmd {
 		let keyring = self.get_keyring();
 		let is_authority = self.validator || is_dev || keyring.is_some();
 
-		Ok(if is_authority { sc_service::Role::Authority } else { sc_service::Role::Full })
+		Ok(if is_authority { Role::Authority } else { Role::Full })
 	}
 
 	fn force_authoring(&self) -> Result<bool> {
@@ -374,20 +346,9 @@ impl CliConfiguration for RunCmd {
 		default_listen_port: u16,
 		chain_spec: &Box<dyn ChainSpec>,
 	) -> Result<Option<PrometheusConfig>> {
-		Ok(if self.no_prometheus {
-			None
-		} else {
-			let interface =
-				if self.prometheus_external { Ipv4Addr::UNSPECIFIED } else { Ipv4Addr::LOCALHOST };
-
-			Some(PrometheusConfig::new_with_default_registry(
-				SocketAddr::new(
-					interface.into(),
-					self.prometheus_port.unwrap_or(default_listen_port),
-				),
-				chain_spec.id().into(),
-			))
-		})
+		Ok(self
+			.prometheus_params
+			.prometheus_config(default_listen_port, chain_spec.id().to_string()))
 	}
 
 	fn disable_grandpa(&self) -> Result<bool> {
@@ -474,11 +435,11 @@ impl CliConfiguration for RunCmd {
 	}
 
 	fn max_runtime_instances(&self) -> Result<Option<usize>> {
-		Ok(self.max_runtime_instances.map(|x| x.min(256)))
+		Ok(Some(self.runtime_params.max_runtime_instances))
 	}
 
 	fn runtime_cache_size(&self) -> Result<u8> {
-		Ok(self.runtime_cache_size)
+		Ok(self.runtime_params.runtime_cache_size)
 	}
 
 	fn base_path(&self) -> Result<Option<BasePath>> {
@@ -546,36 +507,6 @@ fn rpc_interface(
 	}
 }
 
-#[derive(Debug)]
-enum TelemetryParsingError {
-	MissingVerbosity,
-	VerbosityParsingError(std::num::ParseIntError),
-}
-
-impl std::error::Error for TelemetryParsingError {}
-
-impl std::fmt::Display for TelemetryParsingError {
-	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-		match self {
-			TelemetryParsingError::MissingVerbosity => write!(f, "Verbosity level missing"),
-			TelemetryParsingError::VerbosityParsingError(e) => write!(f, "{}", e),
-		}
-	}
-}
-
-fn parse_telemetry_endpoints(s: &str) -> std::result::Result<(String, u8), TelemetryParsingError> {
-	let pos = s.find(' ');
-	match pos {
-		None => Err(TelemetryParsingError::MissingVerbosity),
-		Some(pos_) => {
-			let url = s[..pos_].to_string();
-			let verbosity =
-				s[pos_ + 1..].parse().map_err(TelemetryParsingError::VerbosityParsingError)?;
-			Ok((url, verbosity))
-		},
-	}
-}
-
 /// CORS setting
 ///
 /// The type is introduced to overcome `Option<Option<T>>` handling of `clap`.

substrate/client/cli/src/params/keystore_params.rs

@@ -61,7 +61,7 @@ pub struct KeystoreParams {
 	pub password_filename: Option<PathBuf>,
 }
 
-/// Parse a sercret string, returning a displayable error.
+/// Parse a secret string, returning a displayable error.
 pub fn secret_string_from_str(s: &str) -> std::result::Result<SecretString, String> {
 	std::str::FromStr::from_str(s).map_err(|_| "Could not get SecretString".to_string())
 }

substrate/client/cli/src/params/message_params.rs

@@ -20,14 +20,13 @@
 
 use crate::error::Error;
 use array_bytes::{hex2bytes, hex_bytes2hex_str};
-use clap::Parser;
+use clap::Args;
 use std::io::BufRead;
 
 /// Params to configure how a message should be passed into a command.
-#[derive(Parser, Debug, Clone)]
+#[derive(Debug, Clone, Args)]
 pub struct MessageParams {
 	/// Message to process. Will be read from STDIN otherwise.
-	///
 	/// The message is assumed to be raw bytes per default. Use `--hex` for hex input. Can
 	/// optionally be prefixed with `0x` in the hex case.
 	#[arg(long)]

substrate/client/cli/src/params/mod.rs

@@ -22,8 +22,11 @@ mod message_params;
 mod network_params;
 mod node_key_params;
 mod offchain_worker_params;
+mod prometheus_params;
 mod pruning_params;
+mod runtime_params;
 mod shared_params;
+mod telemetry_params;
 mod transaction_pool_params;
 
 use crate::arg_enums::{CryptoScheme, OutputType};
@@ -37,8 +40,8 @@ use std::{fmt::Debug, str::FromStr};
 
 pub use crate::params::{
 	database_params::*, import_params::*, keystore_params::*, message_params::*, network_params::*,
-	node_key_params::*, offchain_worker_params::*, pruning_params::*, shared_params::*,
-	transaction_pool_params::*,
+	node_key_params::*, offchain_worker_params::*, prometheus_params::*, pruning_params::*,
+	runtime_params::*, shared_params::*, telemetry_params::*, transaction_pool_params::*,
 };
 
 /// Parse Ss58AddressFormat

substrate/client/cli/src/params/prometheus_params.rs

+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
+
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+use clap::Args;
+use sc_service::config::PrometheusConfig;
+use std::net::{Ipv4Addr, SocketAddr};
+
+/// Parameters used to config prometheus.
+#[derive(Debug, Clone, Args)]
+pub struct PrometheusParams {
+	/// Specify Prometheus exporter TCP Port.
+	#[arg(long, value_name = "PORT")]
+	pub prometheus_port: Option<u16>,
+	/// Expose Prometheus exporter on all interfaces.
+	///
+	/// Default is local.
+	#[arg(long)]
+	pub prometheus_external: bool,
+	/// Do not expose a Prometheus exporter endpoint.
+	///
+	/// Prometheus metric endpoint is enabled by default.
+	#[arg(long)]
+	pub no_prometheus: bool,
+}
+
+impl PrometheusParams {
+	/// Creates [`PrometheusConfig`].
+	pub fn prometheus_config(
+		&self,
+		default_listen_port: u16,
+		chain_id: String,
+	) -> Option<PrometheusConfig> {
+		if self.no_prometheus {
+			None
+		} else {
+			let interface =
+				if self.prometheus_external { Ipv4Addr::UNSPECIFIED } else { Ipv4Addr::LOCALHOST };
+
+			Some(PrometheusConfig::new_with_default_registry(
+				SocketAddr::new(
+					interface.into(),
+					self.prometheus_port.unwrap_or(default_listen_port),
+				),
+				chain_id,
+			))
+		}
+	}
+}

substrate/client/cli/src/params/pruning_params.rs

@@ -21,7 +21,7 @@ use clap::Args;
 use sc_service::{BlocksPruning, PruningMode};
 
 /// Parameters to define the pruning mode
-#[derive(Debug, Clone, PartialEq, Args)]
+#[derive(Debug, Clone, Args)]
 pub struct PruningParams {
 	/// Specify the state pruning mode.
 	///

substrate/client/cli/src/params/runtime_params.rs

+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
+
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+use clap::Args;
+use std::str::FromStr;
+
+/// Parameters used to config runtime.
+#[derive(Debug, Clone, Args)]
+pub struct RuntimeParams {
+	/// The size of the instances cache for each runtime. The values higher than 256 are illegal.
+	#[arg(long, default_value_t = 8, value_parser = parse_max_runtime_instances)]
+	pub max_runtime_instances: usize,
+
+	/// Maximum number of different runtimes that can be cached.
+	#[arg(long, default_value_t = 2)]
+	pub runtime_cache_size: u8,
+}
+
+fn parse_max_runtime_instances(s: &str) -> Result<usize, String> {
+	let max_runtime_instances = usize::from_str(s)
+		.map_err(|_err| format!("Illegal `--max-runtime-instances` value: {s}"))?;
+
+	if max_runtime_instances > 256 {
+		Err(format!("Illegal `--max-runtime-instances` value: {max_runtime_instances} is more than the allowed maximum of `256` "))
+	} else {
+		Ok(max_runtime_instances)
+	}
+}

substrate/client/cli/src/params/shared_params.rs

@@ -22,7 +22,7 @@ use sc_service::config::BasePath;
 use std::path::PathBuf;
 
 /// Shared parameters used by all `CoreParams`.
-#[derive(Debug, Clone, PartialEq, Args)]
+#[derive(Debug, Clone, Args)]
 pub struct SharedParams {
 	/// Specify the chain specification.
 	///

substrate/client/cli/src/params/telemetry_params.rs

+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
+
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+use clap::Args;
+
+/// Parameters used to config telemetry.
+#[derive(Debug, Clone, Args)]
+pub struct TelemetryParams {
+	/// Disable connecting to the Substrate telemetry server.
+	///
+	/// Telemetry is on by default on global chains.
+	#[arg(long)]
+	pub no_telemetry: bool,
+
+	/// The URL of the telemetry server to connect to.
+	///
+	/// This flag can be passed multiple times as a means to specify multiple
+	/// telemetry endpoints. Verbosity levels range from 0-9, with 0 denoting
+	/// the least verbosity.
+	/// Expected format is 'URL VERBOSITY', e.g. `--telemetry-url 'wss://foo/bar 0'`.
+	#[arg(long = "telemetry-url", value_name = "URL VERBOSITY", value_parser = parse_telemetry_endpoints)]
+	pub telemetry_endpoints: Vec<(String, u8)>,
+}
+
+#[derive(Debug)]
+enum TelemetryParsingError {
+	MissingVerbosity,
+	VerbosityParsingError(std::num::ParseIntError),
+}
+
+impl std::error::Error for TelemetryParsingError {}
+
+impl std::fmt::Display for TelemetryParsingError {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		match self {
+			TelemetryParsingError::MissingVerbosity => write!(f, "Verbosity level missing"),
+			TelemetryParsingError::VerbosityParsingError(e) => write!(f, "{}", e),
+		}
+	}
+}
+
+fn parse_telemetry_endpoints(s: &str) -> Result<(String, u8), TelemetryParsingError> {
+	let pos = s.find(' ');
+	match pos {
+		None => Err(TelemetryParsingError::MissingVerbosity),
+		Some(pos_) => {
+			let url = s[..pos_].to_string();
+			let verbosity =
+				s[pos_ + 1..].parse().map_err(TelemetryParsingError::VerbosityParsingError)?;
+			Ok((url, verbosity))
+		},
+	}
+}