From d0fc55366c353f5d66ebc86132b1d7200f5b6c80 Mon Sep 17 00:00:00 2001
From: Cecile Tonglet <cecile@parity.io>
Date: Thu, 6 Feb 2020 13:31:06 +0100
Subject: [PATCH] Moving out pub functions from sc_cli

Related to #4776
---
 client/cli/src/lib.rs        | 111 ++---------------------------------
 client/cli/src/params.rs     | 102 ++++++++++++++++++++++++++++++--
 client/network/src/config.rs |   6 ++
 client/service/src/config.rs |  13 +++-
 4 files changed, 119 insertions(+), 113 deletions(-)

diff --git a/client/cli/src/lib.rs b/client/cli/src/lib.rs
index 7f726893368a0..8a51a81695600 100644
--- a/client/cli/src/lib.rs
+++ b/client/cli/src/lib.rs
@@ -80,13 +80,6 @@ const DEFAULT_KEYSTORE_CONFIG_PATH : &'static str = "keystore";
 /// The maximum number of characters for a node name.
 const NODE_NAME_MAX_LENGTH: usize = 32;
 
-fn get_chain_key(cli: &SharedParams) -> String {
-	match cli.chain {
-		Some(ref chain) => chain.clone(),
-		None => if cli.dev { "dev".into() } else { "".into() }
-	}
-}
-
 fn generate_node_name() -> String {
 	let result = loop {
 		let node_name = Generator::with_naming(Name::Numbered).next().unwrap();
@@ -100,30 +93,6 @@ fn generate_node_name() -> String {
 	result
 }
 
-/// Load spec to `Configuration` from shared params and spec factory.
-pub fn load_spec<'a, G, E, F>(
-	mut config: &'a mut Configuration<G, E>,
-	cli: &SharedParams,
-	factory: F,
-) -> error::Result<&'a ChainSpec<G, E>> where
-	G: RuntimeGenesis,
-	E: ChainSpecExtension,
-	F: FnOnce(&str) -> Result<Option<ChainSpec<G, E>>, String>,
-{
-	let chain_key = get_chain_key(cli);
-	let spec = match factory(&chain_key)? {
-		Some(spec) => spec,
-		None => ChainSpec::from_json_file(PathBuf::from(chain_key))?
-	};
-
-	config.network.boot_nodes = spec.boot_nodes().to_vec();
-	config.telemetry_endpoints = spec.telemetry_endpoints().clone();
-
-	config.chain_spec = Some(spec);
-
-	Ok(config.chain_spec.as_ref().unwrap())
-}
-
 fn base_path(cli: &SharedParams, version: &VersionInfo) -> PathBuf {
 	cli.base_path.clone()
 		.unwrap_or_else(||
@@ -234,7 +203,7 @@ where
 	SF: AbstractService + Unpin,
 {
 	init(&run_cmd.shared_params, version)?;
-	load_spec(&mut config, &run_cmd.shared_params, spec_factory)?;
+	run_cmd.shared_params.update_config(&mut config, spec_factory)?;
 	run_cmd.run(config, new_light, new_full, version)
 }
 
@@ -259,7 +228,7 @@ where
 	let shared_params = subcommand.get_shared_params();
 
 	init(shared_params, version)?;
-	load_spec(&mut config, shared_params, spec_factory)?;
+	shared_params.update_config(&mut config, spec_factory)?;
 	subcommand.run(config, builder)
 }
 
@@ -306,7 +275,7 @@ where
 	info!("  by {}, {}-{}", version.author, version.copyright_start_year, Local::today().year());
 	info!("Chain specification: {}", config.expect_chain_spec().name());
 	info!("Node name: {}", config.name);
-	info!("Roles: {}", display_role(&config));
+	info!("Roles: {}", config.display_role());
 
 	match config.roles {
 		ServiceRoles::LIGHT => run_service_until_exit(
@@ -320,17 +289,6 @@ where
 	}
 }
 
-/// Returns a string displaying the node role, special casing the sentry mode
-/// (returning `SENTRY`), since the node technically has an `AUTHORITY` role but
-/// doesn't participate.
-pub fn display_role<G, E>(config: &Configuration<G, E>) -> String {
-	if config.sentry_mode {
-		"SENTRY".to_string()
-	} else {
-		format!("{:?}", config.roles)
-	}
-}
-
 /// Fill the given `PoolConfiguration` by looking at the cli parameters.
 fn fill_transaction_pool_configuration<G, E>(
 	options: &mut Configuration<G, E>,
@@ -450,67 +408,6 @@ fn fill_config_keystore_password_and_path<G, E>(
 	Ok(())
 }
 
-/// Put block import CLI params into `config` object.
-pub fn fill_import_params<G, E>(
-	config: &mut Configuration<G, E>,
-	cli: &ImportParams,
-	role: sc_service::Roles,
-	is_dev: bool,
-) -> error::Result<()>
-where
-	G: RuntimeGenesis,
-{
-	if let Some(DatabaseConfig::Path { ref mut cache_size, .. }) = config.database {
-		*cache_size = Some(cli.database_cache_size);
-	}
-
-	config.state_cache_size = cli.state_cache_size;
-
-	// by default we disable pruning if the node is an authority (i.e.
-	// `ArchiveAll`), otherwise we keep state for the last 256 blocks. if the
-	// node is an authority and pruning is enabled explicitly, then we error
-	// unless `unsafe_pruning` is set.
-	config.pruning = match &cli.pruning {
-		Some(ref s) if s == "archive" => PruningMode::ArchiveAll,
-		None if role == sc_service::Roles::AUTHORITY => PruningMode::ArchiveAll,
-		None => PruningMode::default(),
-		Some(s) => {
-			if role == sc_service::Roles::AUTHORITY && !cli.unsafe_pruning {
-				return Err(error::Error::Input(
-					"Validators should run with state pruning disabled (i.e. archive). \
-					You can ignore this check with `--unsafe-pruning`.".to_string()
-				));
-			}
-
-			PruningMode::keep_blocks(s.parse()
-				.map_err(|_| error::Error::Input("Invalid pruning mode specified".to_string()))?
-			)
-		},
-	};
-
-	config.wasm_method = cli.wasm_method.into();
-
-	let exec = &cli.execution_strategies;
-	let exec_all_or = |strat: ExecutionStrategy, default: ExecutionStrategy| {
-		exec.execution.unwrap_or(if strat == default && is_dev {
-			ExecutionStrategy::Native
-		} else {
-			strat
-		}).into()
-	};
-
-	config.execution_strategies = ExecutionStrategies {
-		syncing: exec_all_or(exec.execution_syncing, DEFAULT_EXECUTION_SYNCING),
-		importing: exec_all_or(exec.execution_import_block, DEFAULT_EXECUTION_IMPORT_BLOCK),
-		block_construction:
-			exec_all_or(exec.execution_block_construction, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION),
-		offchain_worker:
-			exec_all_or(exec.execution_offchain_worker, DEFAULT_EXECUTION_OFFCHAIN_WORKER),
-		other: exec_all_or(exec.execution_other, DEFAULT_EXECUTION_OTHER),
-	};
-	Ok(())
-}
-
 /// Update and prepare a `Configuration` with command line parameters of `RunCmd` and `VersionInfo`
 pub fn update_config_for_running_node<G, E>(
 	mut config: &mut Configuration<G, E>,
@@ -551,7 +448,7 @@ where
 			sc_service::Roles::FULL
 		};
 
-	fill_import_params(&mut config, &cli.import_params, role, is_dev)?;
+	cli.import_params.update_config(&mut config, role, is_dev)?;
 
 	config.name = match (cli.name.as_ref(), keyring) {
 		(Some(name), _) => name.to_string(),
diff --git a/client/cli/src/params.rs b/client/cli/src/params.rs
index 3a4aa319c6e8f..5950972fce4bd 100644
--- a/client/cli/src/params.rs
+++ b/client/cli/src/params.rs
@@ -18,7 +18,7 @@ use std::{str::FromStr, path::PathBuf};
 use structopt::{StructOpt, clap::arg_enum};
 use sc_service::{
 	AbstractService, Configuration, ChainSpecExtension, RuntimeGenesis, ServiceBuilderCommand,
-	config::DatabaseConfig,
+	config::DatabaseConfig, ChainSpec, PruningMode,
 };
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 use crate::VersionInfo;
@@ -115,6 +115,35 @@ pub struct SharedParams {
 	pub log: Option<String>,
 }
 
+impl SharedParams {
+	/// Load spec to `Configuration` from `SharedParams` and spec factory.
+	pub fn update_config<'a, G, E, F>(
+		&self,
+		mut config: &'a mut Configuration<G, E>,
+		spec_factory: F,
+	) -> error::Result<&'a ChainSpec<G, E>> where
+		G: RuntimeGenesis,
+		E: ChainSpecExtension,
+		F: FnOnce(&str) -> Result<Option<ChainSpec<G, E>>, String>,
+	{
+		let chain_key = match self.chain {
+			Some(ref chain) => chain.clone(),
+			None => if self.dev { "dev".into() } else { "".into() }
+		};
+		let spec = match spec_factory(&chain_key)? {
+			Some(spec) => spec,
+			None => ChainSpec::from_json_file(PathBuf::from(chain_key))?
+		};
+
+		config.network.boot_nodes = spec.boot_nodes().to_vec();
+		config.telemetry_endpoints = spec.telemetry_endpoints().clone();
+
+		config.chain_spec = Some(spec);
+
+		Ok(config.chain_spec.as_ref().unwrap())
+	}
+}
+
 /// Parameters for block import.
 #[derive(Debug, StructOpt, Clone)]
 pub struct ImportParams {
@@ -157,6 +186,71 @@ pub struct ImportParams {
 	pub state_cache_size: usize,
 }
 
+impl ImportParams {
+	/// Put block import CLI params into `config` object.
+	pub fn update_config<G, E>(
+		self,
+		config: &mut Configuration<G, E>,
+		role: sc_service::Roles,
+		is_dev: bool,
+	) -> error::Result<()>
+	where
+		G: RuntimeGenesis,
+	{
+		use sc_client_api::execution_extensions::ExecutionStrategies;
+
+		if let Some(DatabaseConfig::Path { ref mut cache_size, .. }) = config.database {
+			*cache_size = Some(self.database_cache_size);
+		}
+
+		config.state_cache_size = self.state_cache_size;
+
+		// by default we disable pruning if the node is an authority (i.e.
+		// `ArchiveAll`), otherwise we keep state for the last 256 blocks. if the
+		// node is an authority and pruning is enabled explicitly, then we error
+		// unless `unsafe_pruning` is set.
+		config.pruning = match &self.pruning {
+			Some(ref s) if s == "archive" => PruningMode::ArchiveAll,
+			None if role == sc_service::Roles::AUTHORITY => PruningMode::ArchiveAll,
+			None => PruningMode::default(),
+			Some(s) => {
+				if role == sc_service::Roles::AUTHORITY && !self.unsafe_pruning {
+					return Err(error::Error::Input(
+						"Validators should run with state pruning disabled (i.e. archive). \
+						You can ignore this check with `--unsafe-pruning`.".to_string()
+					));
+				}
+
+				PruningMode::keep_blocks(s.parse()
+					.map_err(|_| error::Error::Input("Invalid pruning mode specified".to_string()))?
+				)
+			},
+		};
+
+		config.wasm_method = self.wasm_method.into();
+
+		let exec = &self.execution_strategies;
+		let exec_all_or = |strat: ExecutionStrategy, default: ExecutionStrategy| {
+			exec.execution.unwrap_or(if strat == default && is_dev {
+				ExecutionStrategy::Native
+			} else {
+				strat
+			}).into()
+		};
+
+		config.execution_strategies = ExecutionStrategies {
+			syncing: exec_all_or(exec.execution_syncing, DEFAULT_EXECUTION_SYNCING),
+			importing: exec_all_or(exec.execution_import_block, DEFAULT_EXECUTION_IMPORT_BLOCK),
+			block_construction:
+				exec_all_or(exec.execution_block_construction, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION),
+			offchain_worker:
+				exec_all_or(exec.execution_offchain_worker, DEFAULT_EXECUTION_OFFCHAIN_WORKER),
+			other: exec_all_or(exec.execution_other, DEFAULT_EXECUTION_OTHER),
+		};
+		Ok(())
+	}
+}
+
 /// Parameters used to create the network configuration.
 #[derive(Debug, StructOpt, Clone)]
 pub struct NetworkConfigurationParams {
@@ -1044,9 +1138,8 @@ impl ImportBlocksCmd {
 		<<<BB as BlockT>::Header as HeaderT>::Number as std::str::FromStr>::Err: std::fmt::Debug,
 		<BB as BlockT>::Hash: std::str::FromStr,
 	{
-		crate::fill_import_params(
+		self.import_params.update_config(
 			&mut config,
-			&self.import_params,
 			sc_service::Roles::FULL,
 			self.shared_params.dev,
 		)?;
@@ -1084,9 +1177,8 @@ impl CheckBlockCmd {
 	{
 		assert!(config.chain_spec.is_some(), "chain_spec must be present before continuing");
 
-		crate::fill_import_params(
+		self.import_params.update_config(
 			&mut config,
-			&self.import_params,
 			sc_service::Roles::FULL,
 			self.shared_params.dev,
 		)?;
diff --git a/client/network/src/config.rs b/client/network/src/config.rs
index 6cf2587fe47cb..16798469ee832 100644
--- a/client/network/src/config.rs
+++ b/client/network/src/config.rs
@@ -120,6 +120,12 @@ impl Roles {
 	}
 }
 
+impl fmt::Display for Roles {
+	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+		write!(f, "{:?}", self)
+	}
+}
+
 impl codec::Encode for Roles {
 	fn encode_to<T: codec::Output>(&self, dest: &mut T) {
 		dest.push_byte(self.bits())
diff --git a/client/service/src/config.rs b/client/service/src/config.rs
index f4043d533e190..2131fbf1c36db 100644
--- a/client/service/src/config.rs
+++ b/client/service/src/config.rs
@@ -207,7 +207,7 @@ impl<G, E> Default for Configuration<G, E> {
 
 impl<G, E> Configuration<G, E> {
 	/// Create a default config using `VersionInfo`
-	pub fn new(version: &VersionInfo) -> Self {
+	pub fn from_version(version: &VersionInfo) -> Self {
 		let mut config = Configuration::default();
 		config.impl_name = version.name;
 		config.impl_version = version.version;
@@ -254,6 +254,17 @@ impl<G, E> Configuration<G, E> {
 	pub fn expect_database(&self) -> &DatabaseConfig {
 		self.database.as_ref().expect("database must be specified")
 	}
+
+	/// Returns a string displaying the node role, special casing the sentry mode
+	/// (returning `SENTRY`), since the node technically has an `AUTHORITY` role but
+	/// doesn't participate.
+	pub fn display_role(&self) -> String {
+		if self.sentry_mode {
+			"SENTRY".to_string()
+		} else {
+			self.roles.to_string()
+		}
+	}
 }
 
 /// Returns platform info