CLI: refactoring: remove Options from sc_service::Configuration's fields (#5271)

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* Rename IntoConfiguration to CliConfiguration

* Renamed into_configuration to create_configuration

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* Move keystore params to its own module

* Use in-memory keystore even for build-spec

* Enforce proper value for node name

* dev_key_seed

* Telemetry endpoints

* rustfmt

* Converted all RunCmd

* rustfmt

* Added export-blocks

* Missed something

* Removed config_path in NetworkConfiguration (not used)

* Fixed warnings

* public_addresses is used but never set, keeping it

* Merge Configuration.node and NetworkConfiguration.node_name

...because they are the same thing

* Added: import-blocks

* Adding a proc_macro to help impl SubstrateCli

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* Re-export spec_factory from sc_cli

* Re-added all the commands

* Refactored node_key_params

* Fixed previous refucktoring

* Clean-up and removed full_version()

* Renamed get_is_dev to not confuse with Configuration field

* Fixed sc-cli-derive example

* Fixing tests

* Fixing tests and removing some (will re-add later)

* Fixing more tests

* Removes the need of type parameter

* Converting bin/node and simplifying API

* Converting more

* Converting last command

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* Fixing tests and added default for WasmExecutionMethod

* Fixing stuff

* Fixed something I broke oops

* Update Cargo.lock

* Moving things around

* Convert everything to Result

* Added new macros to simplify the impl of CliConfiguration

* Added a macro to generate CliConfiguration automatically for subcommands

* Revert... too many macros (this one is not really useful)

This reverts commit 9c516dd38b40fbc420b02c1f8e61d5b2b1a4e434.

* Renamed is_dev to get_is_dev

Good enough for now

* Fixed name roles (this is plural, not singular)

* Clean-up

* Re-export NodeKeyConfig and TelemetryEndpoints from sc_service

* Improve styling/formatting

* Added copyrights

* Added doc and fixed warnings

* Added myself to code owners

* Yes it is needed according to the history

* Revert formatting

* Fixing conflict

* Updated build.rs

* Cargo.lock

* Clean-up

* Update client/cli-derive/Cargo.toml

Co-Authored-By: Seun Lanlege <seunlanlege@gmail.com>

* Fail if using proc_macro and build.rs is not set properly

* Dropped all get_ in front of methods

* Clean-up

* Fixing proc macro missing env var

* Get the configuration inside the Runtime (needed for polkadot)

* Clean-up

* Get is_dev from argument like the others

* Get chain ID instead of chain spec from shared params

* &self is passed to spec_factory/load_spec

* Wrong text

* Fix example

* Officialize macro and made a cool doc

* Renamed spec_factory to load_spec (substrate_cli_configuration)

* Removed not so useful ChainSpec

* Renamed SubstrateCLI to SubstrateCli

* Added changelog for impl_version being full now

* Renamed Runtime to Runner

* Update changelog to show example

* Removed option on database cache size

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* Fix on removal of option

* typo

* Clean-up imports

* Added info in Cargo.toml

* typo

* remarks

* Moved function for build.rs to substrate-build-script-utils

* Fixed example & test of cli-derive

* Moved function for build.rs to substrate-build-script-utils

* Renamed substrate_cli_configuration to substrate_cli oops

It implements SubstrateCli not CliConfiguration!

* Added documentation and wrapper macro

* Removed option on database cache size

* Removed option on database cache size

* Clean-up

* Reduce risk of errors due to typos

* Removed option on database cache size

* Added NOTE as suggested

* Added doc as suggested

* Fixed test

* typo

* renamed runtime to runner

* Fixed weird argument

* More commas

* Moved client/cli-derive to client/cli/derive

* Added 7 tests for the macros

* Improve error message

* Upgrade assert_cmd

* Fixing missing stuff

* Fixed unused import

* Improve SubstrateCli doc

* Applied suggestions

* Fix and clean-up imports

* Started replacing macros WIP

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* Started removing substrate_cli

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* WIP

Forked at: d6aa8e954c
Parent branch: origin/master

* fixed bug introduced while refactoring

* Renamed NetworkConfigurationParams to NetworkParams for consistency sake

* Fixed test

* Update client/cli/src/commands/runcmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/commands/runcmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/commands/export_blocks_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/commands/check_block_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update bin/node/cli/src/command.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update bin/node/cli/src/command.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/commands/export_blocks_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Revert "Update client/cli/src/commands/export_blocks_cmd.rs"

This reverts commit 5906776953392c02beac6bc0bf50f8cbe1a12a01.

* Revert "Update client/cli/src/commands/check_block_cmd.rs"

This reverts commit f705f42b7f3d732be001141afee210fe46a1ef47.

* Revert "Update client/cli/src/commands/export_blocks_cmd.rs"

This reverts commit 8d57c0550164449e6eb2d3bacb04c750c714fcea.

* Revert "Update client/cli/src/commands/runcmd.rs"

This reverts commit 93e74cf5d2e1c0dc49cdff8608d59fc40fc59338.

* Revert "Update client/cli/src/commands/runcmd.rs"

This reverts commit 11d527ba345c0d79f0d3b5b071933d95474d0614.

* Update client/cli/src/commands/export_blocks_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/commands/import_blocks_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/commands/purge_chain_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Changed ::sc_cli to $crate in the macro

* fixed tests

* fixed conflicts

* Fixing test

* Update client/cli/src/commands/purge_chain_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

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

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Remove comment as suggested

* Apply suggestion

* Update client/cli/src/commands/purge_chain_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/commands/purge_chain_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/commands/purge_chain_cmd.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update utils/frame/benchmarking-cli/src/command.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/runner.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/runner.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/runner.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

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

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

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

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

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

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/lib.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/cli/src/config.rs

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Added doc

* Fixed error introduced after applying suggestion

* Revert "Update client/cli/src/params/pruning_params.rs"

This reverts commit 0574d06a4f1efd86e94c1214420a12e7a4be0099.

* Print error

* Apply suggestions from code review

* Remove useless Results

* Fixed CI failing on polkadot approval

Co-authored-by: Seun Lanlege <seunlanlege@gmail.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

substrate/client/service/src/config.rs

@@ -18,60 +18,35 @@
 
 pub use sc_client::ExecutionStrategies;
 pub use sc_client_db::{kvdb::KeyValueDB, PruningMode};
-pub use sc_network::{Multiaddr, config::{MultiaddrWithPeerId, ExtTransport, NetworkConfiguration, Role}};
+pub use sc_network::Multiaddr;
+pub use sc_network::config::{ExtTransport, MultiaddrWithPeerId, NetworkConfiguration, Role, NodeKeyConfig};
 pub use sc_executor::WasmExecutionMethod;
 
 use std::{future::Future, path::{PathBuf, Path}, pin::Pin, net::SocketAddr, sync::Arc};
 pub use sc_transaction_pool::txpool::Options as TransactionPoolOptions;
 use sc_chain_spec::ChainSpec;
 use sp_core::crypto::Protected;
-use target_info::Target;
-use sc_telemetry::TelemetryEndpoints;
+pub use sc_telemetry::TelemetryEndpoints;
 use prometheus_endpoint::Registry;
 
-/// Executable version. Used to pass version information from the root crate.
-#[derive(Clone)]
-pub struct VersionInfo {
-	/// Implementation name.
-	pub name: &'static str,
-	/// Implementation version.
-	pub version: &'static str,
-	/// SCM Commit hash.
-	pub commit: &'static str,
-	/// Executable file name.
-	pub executable_name: &'static str,
-	/// Executable file description.
-	pub description: &'static str,
-	/// Executable file author.
-	pub author: &'static str,
-	/// Support URL.
-	pub support_url: &'static str,
-	/// Copyright starting year (x-current year)
-	pub copyright_start_year: i32,
-}
-
 /// Service configuration.
 pub struct Configuration {
 	/// Implementation name
 	pub impl_name: &'static str,
-	/// Implementation version
+	/// Implementation version (see sc-cli to see an example of format)
 	pub impl_version: &'static str,
-	/// Git commit if any.
-	pub impl_commit: &'static str,
 	/// Node role.
 	pub role: Role,
 	/// How to spawn background tasks. Mandatory, otherwise creating a `Service` will error.
-	pub task_executor: Option<Arc<dyn Fn(Pin<Box<dyn Future<Output = ()> + Send>>) + Send + Sync>>,
+	pub task_executor: Arc<dyn Fn(Pin<Box<dyn Future<Output = ()> + Send>>) + Send + Sync>,
 	/// Extrinsic pool configuration.
 	pub transaction_pool: TransactionPoolOptions,
 	/// Network configuration.
 	pub network: NetworkConfiguration,
-	/// Path to the base configuration directory.
-	pub config_dir: Option<PathBuf>,
 	/// Configuration for the keystore.
 	pub keystore: KeystoreConfig,
 	/// Configuration for the database.
-	pub database: Option<DatabaseConfig>,
+	pub database: DatabaseConfig,
 	/// Size of internal state cache in Bytes
 	pub state_cache_size: usize,
 	/// Size in percent of cache size dedicated to child tries
@@ -79,9 +54,7 @@ pub struct Configuration {
 	/// Pruning settings.
 	pub pruning: PruningMode,
 	/// Chain configuration.
-	pub chain_spec: Option<Box<dyn ChainSpec>>,
-	/// Node name.
-	pub name: String,
+	pub chain_spec: Box<dyn ChainSpec>,
 	/// Wasm execution method.
 	pub wasm_method: WasmExecutionMethod,
 	/// Execution strategies.
@@ -130,8 +103,6 @@ pub struct Configuration {
 /// Configuration of the client keystore.
 #[derive(Clone)]
 pub enum KeystoreConfig {
-	/// No config supplied.
-	None,
 	/// Keystore at a path on-disk. Recommended for native nodes.
 	Path {
 		/// The path of the keystore.
@@ -147,8 +118,8 @@ impl KeystoreConfig {
 	/// Returns the path for the keystore.
 	pub fn path(&self) -> Option<&Path> {
 		match self {
-			Self::Path { path, .. } => Some(&path),
-			Self::None | Self::InMemory => None,
+			Self::Path { path, .. } => Some(path),
+			Self::InMemory => None,
 		}
 	}
 }
@@ -161,7 +132,7 @@ pub enum DatabaseConfig {
 		/// Path to the database.
 		path: PathBuf,
 		/// Cache Size for internal database in MiB
-		cache_size: Option<u32>,
+		cache_size: usize,
 	},
 
 	/// A custom implementation of an already-open database.
@@ -190,123 +161,9 @@ impl PrometheusConfig {
 	}
 }
 
-impl Default for Configuration {
-	/// Create a default config
-	fn default() -> Self {
-		Configuration {
-			impl_name: "parity-substrate",
-			impl_version: "0.0.0",
-			impl_commit: "",
-			chain_spec: None,
-			config_dir: None,
-			name: Default::default(),
-			role: Role::Full,
-			task_executor: None,
-			transaction_pool: Default::default(),
-			network: Default::default(),
-			keystore: KeystoreConfig::None,
-			database: None,
-			state_cache_size: Default::default(),
-			state_cache_child_ratio: Default::default(),
-			pruning: PruningMode::default(),
-			wasm_method: WasmExecutionMethod::Interpreted,
-			execution_strategies: Default::default(),
-			rpc_http: None,
-			rpc_ws: None,
-			rpc_ws_max_connections: None,
-			rpc_cors: Some(vec![]),
-			prometheus_config: None,
-			telemetry_endpoints: None,
-			telemetry_external_transport: None,
-			default_heap_pages: None,
-			offchain_worker: Default::default(),
-			force_authoring: false,
-			disable_grandpa: false,
-			dev_key_seed: None,
-			tracing_targets: Default::default(),
-			tracing_receiver: Default::default(),
-			max_runtime_instances: 8,
-			announce_block: true,
-		}
-	}
-}
-
 impl Configuration {
-	/// Create a default config using `VersionInfo`
-	pub fn from_version(version: &VersionInfo) -> Self {
-		let mut config = Configuration::default();
-		config.impl_name = version.name;
-		config.impl_version = version.version;
-		config.impl_commit = version.commit;
-
-		config
-	}
-
-	/// Returns full version string of this configuration.
-	pub fn full_version(&self) -> String {
-		full_version_from_strs(self.impl_version, self.impl_commit)
-	}
-
-	/// Implementation id and version.
-	pub fn client_id(&self) -> String {
-		format!("{}/v{}", self.impl_name, self.full_version())
-	}
-
-	/// Generate a PathBuf to sub in the chain configuration directory
-	/// if given
-	pub fn in_chain_config_dir(&self, sub: &str) -> Option<PathBuf> {
-		self.config_dir.clone().map(|mut path| {
-			path.push("chains");
-			path.push(self.expect_chain_spec().id());
-			path.push(sub);
-			path
-		})
-	}
-
-	/// Return a reference to the `ChainSpec` of this `Configuration`.
-	///
-	/// ### Panics
-	///
-	/// This method panic if the `chain_spec` is `None`
-	pub fn expect_chain_spec(&self) -> &dyn ChainSpec {
-		&**self.chain_spec.as_ref().expect("chain_spec must be specified")
-	}
-
-	/// Return a reference to the `DatabaseConfig` of this `Configuration`.
-	///
-	/// ### Panics
-	///
-	/// This method panic if the `database` is `None`
-	pub fn expect_database(&self) -> &DatabaseConfig {
-		self.database.as_ref().expect("database must be specified")
-	}
-
 	/// Returns a string displaying the node role.
 	pub fn display_role(&self) -> String {
 		self.role.to_string()
 	}
-
-	/// Use in memory keystore config when it is not required at all.
-	///
-	/// This function returns an error if the keystore is already set to something different than
-	/// `KeystoreConfig::None`.
-	pub fn use_in_memory_keystore(&mut self) -> Result<(), String> {
-		match &mut self.keystore {
-			cfg @ KeystoreConfig::None => { *cfg = KeystoreConfig::InMemory; Ok(()) },
-			_ => Err("Keystore config specified when it should not be!".into()),
-		}
-	}
-}
-
-/// Returns platform info
-pub fn platform() -> String {
-	let env = Target::env();
-	let env_dash = if env.is_empty() { "" } else { "-" };
-	format!("{}-{}{}{}", Target::arch(), Target::os(), env_dash, env)
-}
-
-/// Returns full version string, using supplied version and commit.
-pub fn full_version_from_strs(impl_version: &str, impl_commit: &str) -> String {
-	let commit_dash = if impl_commit.is_empty() { "" } else { "-" };
-	format!("{}{}{}-{}", impl_version, commit_dash, impl_commit, platform())
 }