Combine default values used at initialization in a trait (#6857)

This moves default values used in the Substrate code base when
initializing a service into a common trait. Currently this trait only
contains listen ports, but this could be extended in the future.
Essentially this will make overriding these values much easier for
Cumulus, where we have 2 nodes running in one binary.

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

@@ -64,7 +64,6 @@ pub enum Subcommand {
 	ExportState(ExportStateCmd),
 }
 
-// TODO: move to config.rs?
 /// Macro that helps implement CliConfiguration on an enum of subcommand automatically
 ///
 /// # Example
@@ -189,17 +188,24 @@ macro_rules! substrate_cli_subcommands {
 
 			fn network_config(
 				&self,
-				chain_spec: &::std::boxed::Box<dyn ::sc_service::ChainSpec>,
+				chain_spec: &std::boxed::Box<dyn sc_service::ChainSpec>,
 				is_dev: bool,
-				net_config_dir: ::std::path::PathBuf,
+				net_config_dir: std::path::PathBuf,
 				client_id: &str,
 				node_name: &str,
-				node_key: ::sc_service::config::NodeKeyConfig,
+				node_key: sc_service::config::NodeKeyConfig,
+				default_listen_port: u16,
 			) -> $crate::Result<::sc_service::config::NetworkConfiguration> {
 				match self {
 					$(
 						$enum::$variant(cmd) => cmd.network_config(
-							chain_spec, is_dev, net_config_dir, client_id, node_name, node_key
+							chain_spec,
+							is_dev,
+							net_config_dir,
+							client_id,
+							node_name,
+							node_key,
+							default_listen_port,
 						)
 					),*
 				}
@@ -291,15 +297,21 @@ macro_rules! substrate_cli_subcommands {
 				}
 			}
 
-			fn rpc_http(&self) -> $crate::Result<::std::option::Option<::std::net::SocketAddr>> {
+			fn rpc_http(
+				&self,
+				default_listen_port: u16,
+			) -> $crate::Result<std::option::Option<std::net::SocketAddr>> {
 				match self {
-					$($enum::$variant(cmd) => cmd.rpc_http()),*
+					$($enum::$variant(cmd) => cmd.rpc_http(default_listen_port)),*
 				}
 			}
 
-			fn rpc_ws(&self) -> $crate::Result<::std::option::Option<::std::net::SocketAddr>> {
+			fn rpc_ws(
+				&self,
+				default_listen_port: u16,
+			) -> $crate::Result<std::option::Option<std::net::SocketAddr>> {
 				match self {
-					$($enum::$variant(cmd) => cmd.rpc_ws()),*
+					$($enum::$variant(cmd) => cmd.rpc_ws(default_listen_port)),*
 				}
 			}
 
@@ -316,23 +328,23 @@ macro_rules! substrate_cli_subcommands {
 			}
 
 			fn rpc_cors(&self, is_dev: bool)
-			-> $crate::Result<::std::option::Option<::std::vec::Vec<String>>> {
+			-> $crate::Result<std::option::Option<std::vec::Vec<String>>> {
 				match self {
 					$($enum::$variant(cmd) => cmd.rpc_cors(is_dev)),*
 				}
 			}
 
-			fn prometheus_config(&self)
-			-> $crate::Result<::std::option::Option<::sc_service::config::PrometheusConfig>> {
+			fn prometheus_config(&self, default_listen_port: u16)
+			-> $crate::Result<std::option::Option<sc_service::config::PrometheusConfig>> {
 				match self {
-					$($enum::$variant(cmd) => cmd.prometheus_config()),*
+					$($enum::$variant(cmd) => cmd.prometheus_config(default_listen_port)),*
 				}
 			}
 
 			fn telemetry_endpoints(
 				&self,
-				chain_spec: &Box<dyn ::sc_service::ChainSpec>,
-			) -> $crate::Result<::std::option::Option<::sc_service::config::TelemetryEndpoints>> {
+				chain_spec: &Box<dyn sc_service::ChainSpec>,
+			) -> $crate::Result<std::option::Option<sc_service::config::TelemetryEndpoints>> {
 				match self {
 					$($enum::$variant(cmd) => cmd.telemetry_endpoints(chain_spec)),*
 				}

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

@@ -382,7 +382,7 @@ impl CliConfiguration for RunCmd {
 		Ok(self.shared_params.dev || self.force_authoring)
 	}
 
-	fn prometheus_config(&self) -> Result<Option<PrometheusConfig>> {
+	fn prometheus_config(&self, default_listen_port: u16) -> Result<Option<PrometheusConfig>> {
 		Ok(if self.no_prometheus {
 			None
 		} else {
@@ -393,7 +393,10 @@ impl CliConfiguration for RunCmd {
 			};
 
 			Some(PrometheusConfig::new_with_default_registry(
-				SocketAddr::new(interface.into(), self.prometheus_port.unwrap_or(9615))
+				SocketAddr::new(
+					interface.into(),
+					self.prometheus_port.unwrap_or(default_listen_port),
+				)
 			))
 		})
 	}
@@ -427,7 +430,7 @@ impl CliConfiguration for RunCmd {
 			.into())
 	}
 
-	fn rpc_http(&self) -> Result<Option<SocketAddr>> {
+	fn rpc_http(&self, default_listen_port: u16) -> Result<Option<SocketAddr>> {
 		let interface = rpc_interface(
 			self.rpc_external,
 			self.unsafe_rpc_external,
@@ -435,22 +438,22 @@ impl CliConfiguration for RunCmd {
 			self.validator
 		)?;
 
-		Ok(Some(SocketAddr::new(interface, self.rpc_port.unwrap_or(9933))))
+		Ok(Some(SocketAddr::new(interface, self.rpc_port.unwrap_or(default_listen_port))))
 	}
 
 	fn rpc_ipc(&self) -> Result<Option<String>> {
 		Ok(self.ipc_path.clone())
 	}
 
-	fn rpc_ws(&self) -> Result<Option<SocketAddr>> {
+	fn rpc_ws(&self, default_listen_port: u16) -> Result<Option<SocketAddr>> {
 		let interface = rpc_interface(
 			self.ws_external,
 			self.unsafe_ws_external,
 			self.rpc_methods,
-			self.validator
+			self.validator,
 		)?;
 
-		Ok(Some(SocketAddr::new(interface, self.ws_port.unwrap_or(9944))))
+		Ok(Some(SocketAddr::new(interface, self.ws_port.unwrap_or(default_listen_port))))
 	}
 
 	fn rpc_methods(&self) -> Result<sc_service::config::RpcMethods> {

substrate/client/cli/src/config.rs

@@ -41,8 +41,44 @@ pub(crate) const NODE_NAME_MAX_LENGTH: usize = 64;
 /// default sub directory to store network config
 pub(crate) const DEFAULT_NETWORK_CONFIG_PATH: &'static str = "network";
 
+/// Default configuration values used by Substrate
+///
+/// These values will be used by [`CliConfiguritation`] to set
+/// default values for e.g. the listen port or the RPC port.
+pub trait DefaultConfigurationValues {
+	/// The port Substrate should listen on for p2p connections.
+	///
+	/// By default this is `30333`.
+	fn p2p_listen_port() -> u16 {
+		30333
+	}
+
+	/// The port Substrate should listen on for websocket connections.
+	///
+	/// By default this is `9944`.
+	fn rpc_ws_listen_port() -> u16 {
+		9944
+	}
+
+	/// The port Substrate should listen on for http connections.
+	///
+	/// By default this is `9933`.
+	fn rpc_http_listen_port() -> u16 {
+		9933
+	}
+
+	/// The port Substrate should listen on for prometheus connections.
+	///
+	/// By default this is `9615`.
+	fn prometheus_listen_port() -> u16 {
+		9615
+	}
+}
+
+impl DefaultConfigurationValues for () {}
+
 /// A trait that allows converting an object to a Configuration
-pub trait CliConfiguration: Sized {
+pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 	/// Get the SharedParams for this object
 	fn shared_params(&self) -> &SharedParams;
 
@@ -122,6 +158,7 @@ pub trait CliConfiguration: Sized {
 		client_id: &str,
 		node_name: &str,
 		node_key: NodeKeyConfig,
+		default_listen_port: u16,
 	) -> Result<NetworkConfiguration> {
 		Ok(if let Some(network_params) = self.network_params() {
 			network_params.network_config(
@@ -131,6 +168,7 @@ pub trait CliConfiguration: Sized {
 				client_id,
 				node_name,
 				node_key,
+				default_listen_port,
 			)
 		} else {
 			NetworkConfiguration::new(
@@ -257,22 +295,22 @@ pub trait CliConfiguration: Sized {
 	/// Get the RPC HTTP address (`None` if disabled).
 	///
 	/// By default this is `None`.
-	fn rpc_http(&self) -> Result<Option<SocketAddr>> {
-		Ok(Default::default())
+	fn rpc_http(&self, _default_listen_port: u16) -> Result<Option<SocketAddr>> {
+		Ok(None)
 	}
 
 	/// Get the RPC IPC path (`None` if disabled).
 	///
 	/// By default this is `None`.
 	fn rpc_ipc(&self) -> Result<Option<String>> {
-		Ok(Default::default())
+		Ok(None)
 	}
 
 	/// Get the RPC websocket address (`None` if disabled).
 	///
 	/// By default this is `None`.
-	fn rpc_ws(&self) -> Result<Option<SocketAddr>> {
-		Ok(Default::default())
+	fn rpc_ws(&self, _default_listen_port: u16) -> Result<Option<SocketAddr>> {
+		Ok(None)
 	}
 
 	/// Returns the RPC method set to expose.
@@ -287,12 +325,12 @@ pub trait CliConfiguration: Sized {
 	///
 	/// By default this is `None`.
 	fn rpc_ws_max_connections(&self) -> Result<Option<usize>> {
-		Ok(Default::default())
+		Ok(None)
 	}
 
 	/// Get the RPC cors (`None` if disabled)
 	///
-	/// By default this is `None`.
+	/// By default this is `Some(Vec::new())`.
 	fn rpc_cors(&self, _is_dev: bool) -> Result<Option<Vec<String>>> {
 		Ok(Some(Vec::new()))
 	}
@@ -300,8 +338,8 @@ pub trait CliConfiguration: Sized {
 	/// Get the prometheus configuration (`None` if disabled)
 	///
 	/// By default this is `None`.
-	fn prometheus_config(&self) -> Result<Option<PrometheusConfig>> {
-		Ok(Default::default())
+	fn prometheus_config(&self, _default_listen_port: u16) -> Result<Option<PrometheusConfig>> {
+		Ok(None)
 	}
 
 	/// Get the telemetry endpoints (if any)
@@ -318,14 +356,14 @@ pub trait CliConfiguration: Sized {
 	///
 	/// By default this is `None`.
 	fn telemetry_external_transport(&self) -> Result<Option<ExtTransport>> {
-		Ok(Default::default())
+		Ok(None)
 	}
 
 	/// Get the default value for heap pages
 	///
 	/// By default this is `None`.
 	fn default_heap_pages(&self) -> Result<Option<u64>> {
-		Ok(Default::default())
+		Ok(None)
 	}
 
 	/// Returns an offchain worker config wrapped in `Ok(_)`
@@ -445,6 +483,7 @@ pub trait CliConfiguration: Sized {
 				client_id.as_str(),
 				self.node_name()?.as_str(),
 				node_key,
+				DCV::p2p_listen_port(),
 			)?,
 			keystore: self.keystore_config(&config_dir)?,
 			database: self.database_config(&config_dir, database_cache_size, database)?,
@@ -453,13 +492,13 @@ pub trait CliConfiguration: Sized {
 			pruning: self.pruning(unsafe_pruning, &role)?,
 			wasm_method: self.wasm_method()?,
 			execution_strategies: self.execution_strategies(is_dev, is_validator)?,
-			rpc_http: self.rpc_http()?,
-			rpc_ws: self.rpc_ws()?,
+			rpc_http: self.rpc_http(DCV::rpc_http_listen_port())?,
+			rpc_ws: self.rpc_ws(DCV::rpc_ws_listen_port())?,
 			rpc_ipc: self.rpc_ipc()?,
 			rpc_methods: self.rpc_methods()?,
 			rpc_ws_max_connections: self.rpc_ws_max_connections()?,
 			rpc_cors: self.rpc_cors(is_dev)?,
-			prometheus_config: self.prometheus_config()?,
+			prometheus_config: self.prometheus_config(DCV::prometheus_listen_port())?,
 			telemetry_endpoints: self.telemetry_endpoints(&chain_spec)?,
 			telemetry_external_transport: self.telemetry_external_transport()?,
 			default_heap_pages: self.default_heap_pages()?,

substrate/client/cli/src/lib.rs

@@ -209,7 +209,7 @@ pub trait SubstrateCli: Sized {
 	}
 
 	/// Only create a Configuration for the command provided in argument
-	fn create_configuration<T: CliConfiguration>(
+	fn create_configuration<T: CliConfiguration<DVC>, DVC: DefaultConfigurationValues>(
 		&self,
 		command: &T,
 		task_executor: TaskExecutor,

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

@@ -114,8 +114,9 @@ impl NetworkParams {
 		client_id: &str,
 		node_name: &str,
 		node_key: NodeKeyConfig,
+		default_listen_port: u16,
 	) -> NetworkConfiguration {
-		let port = self.port.unwrap_or(30333);
+		let port = self.port.unwrap_or(default_listen_port);
 
 		let listen_addresses = if self.listen_addr.is_empty() {
 			vec![