Refactor CLI handling (#1368)

* Rework cli handling

* Update readme

* Adds support for custom subcommands and extra run parameters

* Update readme

* Fixes compilation after master merge

* Make "Run" the default subcommand

Actually its hidden to the outside that is an subcommand.

* Rewrite CLI to work without breaking old CLI behavior

* Some cleanup

* Fix incorrect config setup

* Update README

* Fixes after merge

* Fixes incorrect README

substrate/Cargo.lock

@@ -1915,7 +1915,6 @@ dependencies = [
 name = "node-cli"
 version = "0.1.0"
 dependencies = [
- "clap 2.32.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "exit-future 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)",
  "hex-literal 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",

substrate/README.adoc

@@ -48,7 +48,7 @@ Other examples include the parachain-heads extrinsic in Polkadot and the "note-m
 
 === Runtime and API
 
-Substrate chains all have a runtime. The runtime is a WebAssembly "blob" that includes a number of entry-points. Some entry-points are required as part of the underlying Substrate specification. Others are merely convention and required for the default implementation of the Substrate client to be able to author blocks. 
+Substrate chains all have a runtime. The runtime is a WebAssembly "blob" that includes a number of entry-points. Some entry-points are required as part of the underlying Substrate specification. Others are merely convention and required for the default implementation of the Substrate client to be able to author blocks.
 
 If you want to develop a chain with Substrate, you will need to implement the `Core` trait. This `Core` trait generates an API with the minimum necessary functionality to interact with your runtime. A special macro is provided called `impl_runtime_apis!` that help you implement runtime API traits. All runtime API trait implementations need to be done in one call of the `impl_runtime_apis!` macro. All parameters and return values need to implement https://crates.io/crates/parity-codec[`parity-codec`] to be encodable and decodable.
 
@@ -148,7 +148,7 @@ Now with this new chainspec file, you can build a "raw" chain definition for you
 
 [source, shell]
 ----
-substrate --chain ~/chainspec.json build-spec --raw > ~/mychain.json
+substrate build-spec --chain ~/chainspec.json --raw > ~/mychain.json
 ----
 
 This can be fed into Substrate:
@@ -243,7 +243,7 @@ cargo test --all
 You can start a development chain with:
 
 [source, shell]
-cargo run -- --dev
+cargo run \-- --dev
 
 Detailed logs may be shown by running the node with the following environment variables set: `RUST_LOG=debug RUST_BACKTRACE=1 cargo run \-- --dev`.
 
@@ -252,7 +252,7 @@ If you want to see the multi-node consensus algorithm in action locally, then yo
 We'll start Alice's substrate node first on default TCP port 30333 with her chain database stored locally at `/tmp/alice`. The Bootnode ID of her node is `QmQZ8TjTqeDj3ciwr93EJ95hxfDsb9pEYDizUAbWpigtQN`, which is generated from the `--node-key` value that we specify below:
 
 [source, shell]
-cargo run -- \
+cargo run --release \-- \
   --base-path /tmp/alice \
   --chain=local \
   --key Alice \
@@ -264,7 +264,7 @@ cargo run -- \
 In the second terminal, we'll run the following to start Bob's substrate node on a different TCP port of 30334, and with his chain database stored locally at `/tmp/bob`. We'll specify a value for the `--bootnodes` option that will connect his node to Alice's Bootnode ID on TCP port 30333:
 
 [source, shell]
-cargo run -- \
+cargo run --release \-- \
   --base-path /tmp/bob \
   --bootnodes /ip4/127.0.0.1/tcp/30333/p2p/QmQZ8TjTqeDj3ciwr93EJ95hxfDsb9pEYDizUAbWpigtQN \
   --chain=local \
@@ -294,19 +294,19 @@ cargo test --all
 Start your node:
 
 [source, shell]
-cargo run --release
+cargo run --release \--
 
 To see a list of command line options, enter:
 
 [source, shell]
-cargo run --release -- --help
+cargo run --release \-- --help
 
 For example, you can choose a custom node name:
 
 [source, shell]
-cargo run --release -- --name my_custom_name
+cargo run --release \-- --name my_custom_name
 
-If you are successful, you will see your node syncing at https://telemetry.polkadot.io/#/Charred%20Cherry 
+If you are successful, you will see your node syncing at https://telemetry.polkadot.io/#/Charred%20Cherry
 
 
 == Documentation
@@ -373,18 +373,18 @@ Example (generic):
 /// ```rust
 /// // insert example 1 code here
 /// ```
-/// 
+///
 ```
 
 * Important notes:
 ** Documentation comments must use annotations with a triple slash `///`
-** Modules are documented using `//!` 
+** Modules are documented using `//!`
 ```
 //! Summary (of module)
 //!
 //! Description (of module)
 ```
-* Special section header is indicated with a hash `#`. 
+* Special section header is indicated with a hash `#`.
 ** `Panics` section requires an explanation if the function triggers a panic
 ** `Errors` section is for describing conditions under which a function of method returns `Err(E)` if it returns a `Result<T, E>`
 ** `Safety` section requires an explanation if the function is `unsafe`
@@ -392,7 +392,7 @@ Example (generic):
 * Code block annotations for examples are included between triple graves, as shown above.
 Instead of including the programming language to use for syntax highlighting as the annotation
 after the triple graves, alternative annotations include the `ignore`, `text`, `should_panic`, or `no_run`.
-* Summary sentence is a short high level sinngle sentence of its functionality
+* Summary sentence is a short high level single sentence of its functionality
 * Description paragraph is for details additional to the summary sentence
 * Missing documentation annotations may be used to identify where to generate warnings with `#![warn(missing_docs)]`
 or errors `#![deny(missing_docs)]`
@@ -404,7 +404,7 @@ The code block annotations in the `# Example` section may be used as https://doc
 
 * Important notes:
 ** Rustdoc will automatically add a `main()` wrapper around the code block to test it
-** https://doc.rust-lang.org/1.9.0/book/documentation.html#documenting-macros[Documentating macros].
+** https://doc.rust-lang.org/1.9.0/book/documentation.html#documenting-macros[Documenting macros].
 ** Documentation as tests examples are included when running `cargo test`
 
 == Contributing

substrate/core/cli/Cargo.toml

@@ -30,4 +30,4 @@ primitives = { package = "substrate-primitives", path = "../../core/primitives"
 service = { package = "substrate-service", path = "../../core/service" }
 substrate-telemetry = { path = "../../core/telemetry" }
 names = "0.11.0"
-structopt = "0.2.13"
+structopt = "0.2"

substrate/core/cli/src/lib.rs

@@ -19,6 +19,8 @@
 #![warn(missing_docs)]
 #![warn(unused_extern_crates)]
 
+#[macro_use]
+mod traits;
 mod params;
 pub mod error;
 pub mod informant;
@@ -30,22 +32,27 @@ use service::{
 	FactoryGenesis, PruningMode, ChainSpec,
 };
 use network::{
-	Protocol, config::{NetworkConfiguration, NonReservedPeerMode},
+	Protocol, config::{NetworkConfiguration, NonReservedPeerMode, Secret},
 	multiaddr,
 };
 use primitives::H256;
 
-use std::io::{Write, Read, stdin, stdout};
-use std::iter;
-use std::fs;
-use std::fs::File;
-use std::net::{Ipv4Addr, SocketAddr};
-use std::path::{Path, PathBuf};
-use std::str::FromStr;
+use std::{
+	io::{Write, Read, stdin, stdout}, iter, fs::{self, File}, net::{Ipv4Addr, SocketAddr},
+	path::{Path, PathBuf}, str::FromStr,
+};
+
 use names::{Generator, Name};
 use regex::Regex;
-use structopt::StructOpt;
-pub use params::{CoreParams, CoreCommands, ExecutionStrategy};
+use structopt::{StructOpt, clap::AppSettings};
+#[doc(hidden)]
+pub use structopt::clap::App;
+use params::{
+	RunCmd, PurgeChainCmd, RevertCmd, ImportBlocksCmd, ExportBlocksCmd, BuildSpecCmd,
+	NetworkConfigurationParams, SharedParams, MergeParameters
+};
+pub use params::{NoCustom, CoreParams};
+pub use traits::{GetLogFilter, AugmentClap};
 use app_dirs::{AppInfo, AppDataType};
 use error_chain::bail;
 use log::info;
@@ -69,14 +76,6 @@ pub struct VersionInfo {
 	pub author: &'static str,
 }
 
-/// CLI Action
-pub enum Action<E> {
-	/// Substrate handled the command. No need to do anything.
-	ExecutedInternally,
-	/// Service mode requested. Caller should start the service.
-	RunService(E),
-}
-
 /// Something that can be converted into an exit signal.
 pub trait IntoExit {
 	/// Exit signal type.
@@ -85,16 +84,17 @@ pub trait IntoExit {
 	fn into_exit(self) -> Self::Exit;
 }
 
-fn get_chain_key(matches: &clap::ArgMatches) -> String {
-	matches.value_of("chain").unwrap_or_else(
-		|| if matches.is_present("dev") { "dev" } else { "" }
-	).into()
+fn get_chain_key(cli: &SharedParams) -> String {
+	match cli.chain {
+		Some(ref chain) => chain.clone(),
+		None => if cli.dev { "dev".into() } else { "".into() }
+	}
 }
 
-fn load_spec<F, G>(matches: &clap::ArgMatches, factory: F) -> Result<ChainSpec<G>, String>
+fn load_spec<F, G>(cli: &SharedParams, factory: F) -> error::Result<ChainSpec<G>>
 	where G: RuntimeGenesis, F: FnOnce(&str) -> Result<Option<ChainSpec<G>>, String>,
 {
-	let chain_key = get_chain_key(matches);
+	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))?
@@ -102,9 +102,8 @@ fn load_spec<F, G>(matches: &clap::ArgMatches, factory: F) -> Result<ChainSpec<G
 	Ok(spec)
 }
 
-fn base_path(matches: &clap::ArgMatches, version: &VersionInfo) -> PathBuf {
-	matches.value_of("base_path")
-		.map(|x| Path::new(x).to_owned())
+fn base_path(cli: &SharedParams, version: &VersionInfo) -> PathBuf {
+	cli.base_path.clone()
 		.unwrap_or_else(||
 			app_dirs::get_app_root(
 				AppDataType::UserData,
@@ -143,50 +142,150 @@ fn is_node_name_valid(_name: &str) -> Result<(), &str> {
 	Ok(())
 }
 
-/// Parse command line arguments
-pub fn parse_args_default<'a, I, T>(args: I, version: VersionInfo) -> clap::ArgMatches<'a>
+/// Parse command line interface arguments and executes the desired command.
+///
+/// # Return value
+///
+/// A result that indicates if any error occurred.
+/// If no error occurred and a custom subcommand was found, the subcommand is returned.
+/// The user needs to handle this subcommand on its own.
+///
+/// # Remarks
+///
+/// `CC` is a custom subcommand. This needs to be an `enum`! If no custom subcommand is required,
+/// `NoCustom` can be used as type here.
+/// `RP` is are custom parameters for the run command. This needs to be a `struct`! The custom
+/// parameters are visible to the user as if they were normal run command parameters. If no custom
+/// parameters are required, `NoCustom` can be used as type here.
+pub fn parse_and_execute<'a, F, CC, RP, S, RS, E, I, T>(
+	spec_factory: S,
+	version: &VersionInfo,
+	impl_name: &'static str,
+	args: I,
+	exit: E,
+	run_service: RS,
+) -> error::Result<Option<CC>>
 where
+	F: ServiceFactory,
+	S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
+	CC: StructOpt + Clone + GetLogFilter,
+	RP: StructOpt + Clone + AugmentClap,
+	E: IntoExit,
+	RS: FnOnce(E, RP, FactoryFullConfiguration<F>) -> Result<(), String>,
 	I: IntoIterator<Item = T>,
 	T: Into<std::ffi::OsString> + Clone,
 {
+	panic_hook::set();
+
 	let full_version = service::config::full_version_from_strs(
 		version.version,
 		version.commit
 	);
 
-	match CoreParams::clap()
+	let matches = CoreParams::<CC, RP>::clap()
 		.name(version.executable_name)
 		.author(version.author)
 		.about(version.description)
 		.version(&(full_version + "\n")[..])
-		.get_matches_from_safe(args) {
-			Ok(m) => m,
-			Err(e) => e.exit(),
+		.setting(AppSettings::GlobalVersion)
+		.setting(AppSettings::ArgsNegateSubcommands)
+		.setting(AppSettings::SubcommandsNegateReqs)
+		.get_matches_from(args);
+	let cli_args = CoreParams::<CC, RP>::from_clap(&matches);
+
+	init_logger(cli_args.get_log_filter().as_ref().map(|v| v.as_ref()).unwrap_or(""));
+	fdlimit::raise_fd_limit();
+
+	match cli_args {
+		params::CoreParams::Run(params) => run_node::<F, _, _, _, _>(
+			params, spec_factory, exit, run_service, impl_name, version,
+		).map(|_| None),
+		params::CoreParams::BuildSpec(params) =>
+			build_spec::<F, _>(params, spec_factory, version).map(|_| None),
+		params::CoreParams::ExportBlocks(params) =>
+			export_blocks::<F, _, _>(params, spec_factory, exit, version).map(|_| None),
+		params::CoreParams::ImportBlocks(params) =>
+			import_blocks::<F, _, _>(params, spec_factory, exit, version).map(|_| None),
+		params::CoreParams::PurgeChain(params) =>
+			purge_chain::<F, _>(params, spec_factory, version).map(|_| None),
+		params::CoreParams::Revert(params) =>
+			revert_chain::<F, _>(params, spec_factory, version).map(|_| None),
+		params::CoreParams::Custom(params) => Ok(Some(params)),
 	}
 }
 
-/// Parse clap::Matches into config and chain specification
-pub fn parse_matches<'a, F, S>(
-	spec_factory: S,
-	version: &VersionInfo,
-	impl_name: &'static str,
-	matches: &clap::ArgMatches<'a>,
-) -> error::Result<(ChainSpec<<F as service::ServiceFactory>::Genesis>, FactoryFullConfiguration<F>)>
+fn parse_node_key(key: Option<String>) -> error::Result<Option<Secret>> {
+	match key.map(|k| H256::from_str(&k)) {
+		Some(Ok(secret)) => Ok(Some(secret.into())),
+		Some(Err(err)) => Err(create_input_err(format!("Error parsing node key: {}", err))),
+		None => Ok(None),
+	}
+}
+
+/// Fill the given `NetworkConfiguration` by looking at the cli parameters.
+fn fill_network_configuration(
+	cli: NetworkConfigurationParams,
+	base_path: &Path,
+	chain_spec_id: &str,
+	config: &mut NetworkConfiguration,
+	client_id: String,
+) -> error::Result<()> {
+	config.boot_nodes.extend(cli.bootnodes.into_iter());
+	config.config_path = Some(
+		network_path(&base_path, chain_spec_id).to_string_lossy().into()
+	);
+	config.net_config_path = config.config_path.clone();
+	config.reserved_nodes.extend(cli.reserved_nodes.into_iter());
+	if !config.reserved_nodes.is_empty() {
+		config.non_reserved_mode = NonReservedPeerMode::Deny;
+	}
+
+	for addr in cli.listen_addr.iter() {
+		let addr = addr.parse().map_err(|_| "Invalid listen multiaddress")?;
+		config.listen_addresses.push(addr);
+	}
+
+	if config.listen_addresses.is_empty() {
+		let port = match cli.port {
+			Some(port) => port,
+			None => 30333,
+		};
+
+		config.listen_addresses = vec![
+			iter::once(Protocol::Ip4(Ipv4Addr::new(0, 0, 0, 0)))
+				.chain(iter::once(Protocol::Tcp(port)))
+				.collect()
+		];
+	}
+
+	config.public_addresses = Vec::new();
+
+	config.client_version = client_id;
+	config.use_secret = parse_node_key(cli.node_key)?;
+
+	config.in_peers = cli.in_peers;
+	config.out_peers = cli.out_peers;
+
+	Ok(())
+}
+
+fn create_run_node_config<F, S>(
+	cli: RunCmd, spec_factory: S, impl_name: &'static str, version: &VersionInfo
+) -> error::Result<FactoryFullConfiguration<F>>
 where
 	F: ServiceFactory,
 	S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
-
 {
-	let spec = load_spec(&matches, spec_factory)?;
+	let spec = load_spec(&cli.shared_params, spec_factory)?;
 	let mut config = service::Configuration::default_with_spec(spec.clone());
 
 	config.impl_name = impl_name;
 	config.impl_commit = version.commit;
 	config.impl_version = version.version;
 
-	config.name = match matches.value_of("name") {
+	config.name = match cli.name {
 		None => Generator::with_naming(Name::Numbered).next().unwrap(),
-		Some(name) => name.into(),
+		Some(name) => name,
 	};
 	match is_node_name_valid(&config.name) {
 		Ok(_) => (),
@@ -200,31 +299,29 @@ where
 		)
 	}
 
-	let base_path = base_path(&matches, version);
+	let base_path = base_path(&cli.shared_params, version);
 
-	config.keystore_path = matches.value_of("keystore")
-		.map(|x| Path::new(x).to_owned())
+	config.keystore_path = cli.keystore_path
 		.unwrap_or_else(|| keystore_path(&base_path, config.chain_spec.id()))
 		.to_string_lossy()
 		.into();
 
-	config.database_path = db_path(&base_path, config.chain_spec.id()).to_string_lossy().into();
-	config.database_cache_size = match matches.value_of("database_cache_size") {
-		Some(s) => Some(s.parse().map_err(|_| "Invalid Database Cache size specified")?),
-		_=> None
-	};
-	config.pruning = match matches.value_of("pruning") {
-		Some("archive") => PruningMode::ArchiveAll,
+	config.database_path =
+		db_path(&base_path, config.chain_spec.id()).to_string_lossy().into();
+	config.database_cache_size = cli.database_cache_size;
+	config.pruning = match cli.pruning {
+		Some(ref s) if s == "archive" => PruningMode::ArchiveAll,
 		None => PruningMode::default(),
-		Some(s) => PruningMode::keep_blocks(s.parse()
-			.map_err(|_| create_input_err("Invalid pruning mode specified"))?),
+		Some(s) => PruningMode::keep_blocks(
+			s.parse().map_err(|_| create_input_err("Invalid pruning mode specified"))?
+		),
 	};
 
 	let role =
-		if matches.is_present("light") {
+		if cli.light {
 			config.block_execution_strategy = service::ExecutionStrategy::NativeWhenPossible;
 			service::Roles::LIGHT
-		} else if matches.is_present("validator") || matches.is_present("dev") {
+		} else if cli.validator || cli.shared_params.dev {
 			config.block_execution_strategy = service::ExecutionStrategy::Both;
 			service::Roles::AUTHORITY
 		} else {
@@ -232,88 +329,64 @@ where
 			service::Roles::FULL
 		};
 
-	if let Some(s) = matches.value_of("execution") {
-		config.block_execution_strategy = match s {
-			"both" => service::ExecutionStrategy::Both,
-			"native" => service::ExecutionStrategy::NativeWhenPossible,
-			"wasm" => service::ExecutionStrategy::AlwaysWasm,
-			_ => bail!(create_input_err("Invalid execution mode specified")),
-		};
-	}
+	config.block_execution_strategy = cli.execution.into();
 
 	config.roles = role;
-	{
-		config.network.boot_nodes.extend(matches
-			.values_of("bootnodes")
-			.map_or(Default::default(), |v| v.map(|n| n.to_owned()).collect::<Vec<_>>()));
-		config.network.config_path = Some(network_path(&base_path, config.chain_spec.id()).to_string_lossy().into());
-		config.network.net_config_path = config.network.config_path.clone();
-		config.network.reserved_nodes.extend(matches
-			 .values_of("reserved_nodes")
-			 .map_or(Default::default(), |v| v.map(|n| n.to_owned()).collect::<Vec<_>>()));
-		if !config.network.reserved_nodes.is_empty() {
-			config.network.non_reserved_mode = NonReservedPeerMode::Deny;
-		}
+	let client_id = config.client_id();
+	fill_network_configuration(
+		cli.network_config,
+		&base_path,
+		spec.id(),
+		&mut config.network,
+		client_id,
+	)?;
 
-		config.network.listen_addresses = Vec::new();
-		for addr in matches.values_of("listen_addr").unwrap_or_default() {
-			let addr = addr.parse().map_err(|_| "Invalid listen multiaddress")?;
-			config.network.listen_addresses.push(addr);
-		}
-		if config.network.listen_addresses.is_empty() {
-			let port = match matches.value_of("port") {
-				Some(port) => port.parse().map_err(|_| "Invalid p2p port value specified.")?,
-				None => 30333,
-			};
-			config.network.listen_addresses = vec![
-				iter::once(Protocol::Ip4(Ipv4Addr::new(0, 0, 0, 0)))
-					.chain(iter::once(Protocol::Tcp(port)))
-					.collect()
-			];
-		}
-
-		config.network.public_addresses = Vec::new();
-
-		config.network.client_version = config.client_id();
-		config.network.node_name = config.name.clone();
-		config.network.use_secret = match matches.value_of("node_key").map(H256::from_str) {
-			Some(Ok(secret)) => Some(secret.into()),
-			Some(Err(err)) => bail!(create_input_err(format!("Error parsing node key: {}", err))),
-			None => None,
-		};
-
-		let in_peers = match matches.value_of("in_peers") {
-			Some(in_peers) => in_peers.parse().map_err(|_| "Invalid in-peers value specified.")?,
-			None => 25,
-		};
-		let out_peers = match matches.value_of("out_peers") {
-			Some(out_peers) => out_peers.parse().map_err(|_| "Invalid out-peers value specified.")?,
-			None => 25,
-		};
-
-		config.network.in_peers = in_peers;
-		config.network.out_peers = out_peers;
+	if let Some(key) = cli.key {
+		config.keys.push(key);
 	}
 
-	config.keys = matches.values_of("key").unwrap_or_default().map(str::to_owned).collect();
-	if matches.is_present("dev") {
+	if cli.shared_params.dev {
 		config.keys.push("Alice".into());
 	}
 
-	let rpc_interface: &str = if matches.is_present("rpc_external") { "0.0.0.0" } else { "127.0.0.1" };
-	let ws_interface: &str = if matches.is_present("ws_external") { "0.0.0.0" } else { "127.0.0.1" };
+	let rpc_interface: &str = if cli.rpc_external { "0.0.0.0" } else { "127.0.0.1" };
+	let ws_interface: &str = if cli.ws_external { "0.0.0.0" } else { "127.0.0.1" };
 
-	config.rpc_http = Some(parse_address(&format!("{}:{}", rpc_interface, 9933), "rpc_port", &matches)?);
-	config.rpc_ws = Some(parse_address(&format!("{}:{}", ws_interface, 9944), "ws_port", &matches)?);
+	config.rpc_http = Some(
+		parse_address(&format!("{}:{}", rpc_interface, 9933), cli.rpc_port)?
+	);
+	config.rpc_ws = Some(
+		parse_address(&format!("{}:{}", ws_interface, 9944), cli.ws_port)?
+	);
 
 	// Override telemetry
-	if matches.is_present("no_telemetry") {
+	if cli.no_telemetry {
 		config.telemetry_url = None;
-	} else if let Some(url) = matches.value_of("telemetry_url") {
-		config.telemetry_url = Some(url.to_owned());
+	} else if let Some(url) = cli.telemetry_url {
+		config.telemetry_url = Some(url);
 	}
 
-	Ok((spec, config))
+	Ok(config)
+}
+
+fn run_node<F, S, RS, E, RP>(
+	cli: MergeParameters<RunCmd, RP>,
+	spec_factory: S,
+	exit: E,
+	run_service: RS,
+	impl_name: &'static str,
+	version: &VersionInfo,
+) -> error::Result<()>
+where
+	RP: StructOpt + Clone,
+	F: ServiceFactory,
+	E: IntoExit,
+	S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
+	RS: FnOnce(E, RP, FactoryFullConfiguration<F>) -> Result<(), String>,
+ {
+	let config = create_run_node_config::<F, _>(cli.left, spec_factory, impl_name, version)?;
+
+	run_service(exit, cli.right, config).map_err(Into::into)
 }
 
 //
@@ -324,69 +397,23 @@ where
 // 9803-9874		Unassigned
 // 9926-9949		Unassigned
 
-/// execute default commands or return service configuration
-pub fn execute_default<'a, F, E>(
-	spec: ChainSpec<FactoryGenesis<F>>,
-	exit: E,
-	matches: &clap::ArgMatches<'a>,
-	config: &FactoryFullConfiguration<F>
-) -> error::Result<Action<E>>
-where
-	E: IntoExit,
-	F: ServiceFactory,
-{
-	panic_hook::set();
-
-	let log_pattern = matches.value_of("log").unwrap_or("");
-	init_logger(log_pattern);
-	fdlimit::raise_fd_limit();
-
-	if let Some(sub_matches) = matches.subcommand_matches("build-spec") {
-		build_spec::<F>(sub_matches, spec, config)?;
-		return Ok(Action::ExecutedInternally);
-	} else if let Some(sub_matches) = matches.subcommand_matches("export-blocks") {
-		export_blocks::<F, _>(
-			&config.database_path,
-			sub_matches,
-			spec,
-			exit.into_exit()
-		)?;
-		return Ok(Action::ExecutedInternally);
-	} else if let Some(sub_matches) = matches.subcommand_matches("import-blocks") {
-		import_blocks::<F, _>(
-			&config.database_path,
-			sub_matches,
-			spec,
-			exit.into_exit()
-		)?;
-		return Ok(Action::ExecutedInternally);
-	} else if let Some(sub_matches) = matches.subcommand_matches("revert") {
-		revert_chain::<F>(
-			&config.database_path,
-			sub_matches,
-			spec
-		)?;
-		return Ok(Action::ExecutedInternally);
-	} else if let Some(_sub_matches) = matches.subcommand_matches("purge-chain") {
-		purge_chain::<F>(&config.database_path)?;
-		return Ok(Action::ExecutedInternally);
-	}
-
-	Ok(Action::RunService(exit))
-}
-
 fn with_default_boot_node<F>(
-	spec: &ChainSpec<FactoryGenesis<F>>,
-	config: &NetworkConfiguration
+	mut spec: ChainSpec<FactoryGenesis<F>>,
+	cli: &BuildSpecCmd,
+	version: &VersionInfo,
 ) -> error::Result<ChainSpec<FactoryGenesis<F>>>
 where
 	F: ServiceFactory
 {
-	let mut spec = spec.clone();
 	if spec.boot_nodes().is_empty() {
+		let network_path =
+			Some(network_path(&base_path(&cli.shared_params, version), spec.id()).to_string_lossy().into());
+		let network_key = parse_node_key(cli.node_key.clone())?;
+
 		let network_keys =
-			network::obtain_private_key(config)
+			network::obtain_private_key(&network_key, &network_path)
 				.map_err(|err| format!("Error obtaining network key: {}", err))?;
+
 		let peer_id = network_keys.to_peer_id();
 		let addr = multiaddr![
 			Ip4([127, 0, 0, 1]),
@@ -398,112 +425,119 @@ where
 	Ok(spec)
 }
 
-fn build_spec<F>(
-	matches: &clap::ArgMatches,
-	spec: ChainSpec<FactoryGenesis<F>>,
-	config: &FactoryFullConfiguration<F>
+fn build_spec<F, S>(
+	cli: BuildSpecCmd,
+	spec_factory: S,
+	version: &VersionInfo,
 ) -> error::Result<()>
 where
-	F: ServiceFactory
+	F: ServiceFactory,
+	S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
 {
 	info!("Building chain spec");
-	let raw = matches.is_present("raw");
-	let spec = with_default_boot_node::<F>(&spec, &config.network)?;
-	let json = service::chain_ops::build_spec::<FactoryGenesis<F>>(spec, raw)?;
+	let spec = load_spec(&cli.shared_params, spec_factory)?;
+	let spec = with_default_boot_node::<F>(spec, &cli, version)?;
+	let json = service::chain_ops::build_spec::<FactoryGenesis<F>>(spec, cli.raw)?;
+
 	print!("{}", json);
+
 	Ok(())
 }
 
-fn export_blocks<F, E>(
-	db_path: &str,
-	matches: &clap::ArgMatches,
-	spec: ChainSpec<FactoryGenesis<F>>,
-	exit: E
-) -> error::Result<()>
-	where F: ServiceFactory, E: Future<Item=(),Error=()> + Send + 'static,
+fn create_config_with_db_path<F, S>(
+	spec_factory: S, cli: &SharedParams, version: &VersionInfo,
+) -> error::Result<FactoryFullConfiguration<F>>
+where
+	F: ServiceFactory,
+	S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
 {
-	let mut config = service::Configuration::default_with_spec(spec);
-	config.database_path = db_path.to_string();
+	let spec = load_spec(cli, spec_factory)?;
+	let base_path = base_path(cli, version);
+
+	let mut config = service::Configuration::default_with_spec(spec.clone());
+	config.database_path = db_path(&base_path, spec.id()).to_string_lossy().into();
+
+	Ok(config)
+}
+
+fn export_blocks<F, E, S>(
+	cli: ExportBlocksCmd,
+	spec_factory: S,
+	exit: E,
+	version: &VersionInfo,
+) -> error::Result<()>
+where
+	F: ServiceFactory,
+	E: IntoExit,
+	S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
+{
+	let config = create_config_with_db_path::<F, _>(spec_factory, &cli.shared_params, version)?;
+
 	info!("DB path: {}", config.database_path);
-	let from: u64 = match matches.value_of("from") {
-		Some(v) => v.parse().map_err(|_| "Invalid --from argument")?,
-		None => 1,
-	};
+	let from = cli.from.unwrap_or(1);
+	let to = cli.to;
+	let json = cli.json;
 
-	let to: Option<u64> = match matches.value_of("to") {
-		Some(v) => Some(v.parse().map_err(|_| "Invalid --to argument")?),
-		None => None,
-	};
-	let json = matches.is_present("json");
-
-	let file: Box<Write> = match matches.value_of("output") {
+	let file: Box<Write> = match cli.output {
 		Some(filename) => Box::new(File::create(filename)?),
 		None => Box::new(stdout()),
 	};
 
-	Ok(service::chain_ops::export_blocks::<F, _, _>(config, exit, file, As::sa(from), to.map(As::sa), json)?)
+	service::chain_ops::export_blocks::<F, _, _>(
+		config, exit.into_exit(), file, As::sa(from), to.map(As::sa), json
+	).map_err(Into::into)
 }
 
-fn import_blocks<F, E>(
-	db_path: &str,
-	matches: &clap::ArgMatches,
-	spec: ChainSpec<FactoryGenesis<F>>,
-	exit: E
+fn import_blocks<F, E, S>(
+	cli: ImportBlocksCmd,
+	spec_factory: S,
+	exit: E,
+	version: &VersionInfo,
 ) -> error::Result<()>
-	where F: ServiceFactory, E: Future<Item=(),Error=()> + Send + 'static,
+where
+	F: ServiceFactory,
+	E: IntoExit,
+	S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
 {
-	let mut config = service::Configuration::default_with_spec(spec);
-	config.database_path = db_path.to_string();
+	let mut config = create_config_with_db_path::<F, _>(spec_factory, &cli.shared_params, version)?;
 
-	if let Some(s) = matches.value_of("execution") {
-		config.block_execution_strategy = match s {
-			"both" => service::ExecutionStrategy::Both,
-			"native" => service::ExecutionStrategy::NativeWhenPossible,
-			"wasm" => service::ExecutionStrategy::AlwaysWasm,
-			_ => return Err(error::ErrorKind::Input("Invalid block execution mode specified".to_owned()).into()),
-		};
-	}
+	config.block_execution_strategy = cli.execution.into();
+	config.api_execution_strategy = cli.api_execution.into();
 
-	if let Some(s) = matches.value_of("api-execution") {
-		config.api_execution_strategy = match s {
-			"both" => service::ExecutionStrategy::Both,
-			"native" => service::ExecutionStrategy::NativeWhenPossible,
-			"wasm" => service::ExecutionStrategy::AlwaysWasm,
-			_ => return Err(error::ErrorKind::Input("Invalid API execution mode specified".to_owned()).into()),
-		};
-	}
-
-	let file: Box<Read> = match matches.value_of("input") {
+	let file: Box<Read> = match cli.input {
 		Some(filename) => Box::new(File::open(filename)?),
 		None => Box::new(stdin()),
 	};
 
-	Ok(service::chain_ops::import_blocks::<F, _, _>(config, exit, file)?)
+	service::chain_ops::import_blocks::<F, _, _>(config, exit.into_exit(), file).map_err(Into::into)
 }
 
-fn revert_chain<F>(
-	db_path: &str,
-	matches: &clap::ArgMatches,
-	spec: ChainSpec<FactoryGenesis<F>>
+fn revert_chain<F, S>(
+	cli: RevertCmd,
+	spec_factory: S,
+	version: &VersionInfo,
 ) -> error::Result<()>
-	where F: ServiceFactory,
+where
+	F: ServiceFactory,
+	S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
 {
-	let mut config = service::Configuration::default_with_spec(spec);
-	config.database_path = db_path.to_string();
-
-	let blocks = match matches.value_of("num") {
-		Some(v) => v.parse().map_err(|_| "Invalid block count specified")?,
-		None => 256,
-	};
-
+	let config = create_config_with_db_path::<F, _>(spec_factory, &cli.shared_params, version)?;
+	let blocks = cli.num;
 	Ok(service::chain_ops::revert_chain::<F>(config, As::sa(blocks))?)
 }
 
-fn purge_chain<F>(
-	db_path: &str,
+fn purge_chain<F, S>(
+	cli: PurgeChainCmd,
+	spec_factory: S,
+	version: &VersionInfo,
 ) -> error::Result<()>
-	where F: ServiceFactory,
+where
+	F: ServiceFactory,
+	S: FnOnce(&str) -> Result<Option<ChainSpec<FactoryGenesis<F>>>, String>,
 {
+	let config = create_config_with_db_path::<F, _>(spec_factory, &cli.shared_params, version)?;
+
+	let db_path = config.database_path;
 	print!("Are you sure to remove {:?}? (y/n)", &db_path);
 	stdout().flush().expect("failed to flush stdout");
 
@@ -523,17 +557,13 @@ fn purge_chain<F>(
 }
 
 fn parse_address(
-	default: &str,
-	port_param: &str,
-	matches: &clap::ArgMatches
+	address: &str,
+	port: Option<u16>,
 ) -> Result<SocketAddr, String> {
-	let mut address: SocketAddr = default.parse().ok().ok_or_else(
-		|| format!("Invalid address specified for --{}.", port_param)
+	let mut address: SocketAddr = address.parse().map_err(
+		|_| format!("Invalid address: {}", address)
 	)?;
-	if let Some(port) = matches.value_of(port_param) {
-		let port: u16 = port.parse().ok().ok_or_else(
-			|| format!("Invalid port for --{} specified.", port_param)
-		)?;
+	if let Some(port) = port {
 		address.set_port(port);
 	}
 
@@ -583,13 +613,24 @@ fn init_logger(pattern: &str) {
 	let enable_color = isatty;
 
 	builder.format(move |buf, record| {
-		let timestamp = time::strftime("%Y-%m-%d %H:%M:%S", &time::now()).expect("Error formatting log timestamp");
+		let timestamp =
+			time::strftime("%Y-%m-%d %H:%M:%S", &time::now())
+				.expect("Error formatting log timestamp");
 
 		let mut output = if log::max_level() <= log::LevelFilter::Info {
 			format!("{} {}", Colour::Black.bold().paint(timestamp), record.args())
 		} else {
-			let name = ::std::thread::current().name().map_or_else(Default::default, |x| format!("{}", Colour::Blue.bold().paint(x)));
-			format!("{} {} {} {}  {}", Colour::Black.bold().paint(timestamp), name, record.level(), record.target(), record.args())
+			let name = ::std::thread::current()
+				.name()
+				.map_or_else(Default::default, |x| format!("{}", Colour::Blue.bold().paint(x)));
+			format!(
+				"{} {} {} {}  {}",
+				Colour::Black.bold().paint(timestamp),
+				name,
+				record.level(),
+				record.target(),
+				record.args()
+			)
 		};
 
 		if !enable_color {

substrate/core/cli/src/params.rs

@@ -14,230 +14,426 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
+use crate::traits::{AugmentClap, GetLogFilter};
+
 use std::path::PathBuf;
-use structopt::StructOpt;
-
-/// CLI Parameters provided by default
-#[derive(Debug, StructOpt)]
-#[structopt(name = "Substrate")]
-pub struct CoreParams {
-	///Sets a custom logging filter
-	#[structopt(short = "l", long = "log", value_name = "LOG_PATTERN")]
-	log: Option<String>,
-
-	/// Specify custom keystore path
-	#[structopt(long = "keystore-path", value_name = "PATH", parse(from_os_str))]
-	keystore_path: Option<PathBuf>,
-
-	/// Specify additional key seed
-	#[structopt(long = "key", value_name = "STRING")]
-	key: Option<String>,
-
-	/// Specify node secret key (64-character hex string)
-	#[structopt(long = "node-key", value_name = "KEY")]
-	node_key: Option<String>,
-
-	/// Enable validator mode
-	#[structopt(long = "validator")]
-	validator: bool,
-
-	/// Run in light client mode
-	#[structopt(long = "light")]
-	light: bool,
-
-	/// Limit the memory the database cache can use
-	#[structopt(long = "db-cache", value_name = "MiB")]
-	database_cache_size: Option<u32>,
-
-	/// Listen on this multiaddress
-	#[structopt(long = "listen-addr", value_name = "LISTEN_ADDR")]
-	listen_addr: Vec<String>,
-
-	/// Specify p2p protocol TCP port. Only used if --listen-addr is not specified.
-	#[structopt(long = "port", value_name = "PORT")]
-	port: Option<u32>,
-
-	/// Listen to all RPC interfaces (default is local)
-	#[structopt(long = "rpc-external")]
-	rpc_external: bool,
-
-	/// Listen to all Websocket interfaces (default is local)
-	#[structopt(long = "ws-external")]
-	ws_external: bool,
-
-	/// Specify HTTP RPC server TCP port
-	#[structopt(long = "rpc-port", value_name = "PORT")]
-	rpc_port: Option<u32>,
-
-	/// Specify WebSockets RPC server TCP port
-	#[structopt(long = "ws-port", value_name = "PORT")]
-	ws_port: Option<u32>,
-
-	/// Specify a list of bootnodes
-	#[structopt(long = "bootnodes", value_name = "URL")]
-	bootnodes: Vec<String>,
-
-	/// Specify a list of reserved node addresses
-	#[structopt(long = "reserved-nodes", value_name = "URL")]
-	reserved_nodes: Vec<String>,
-
-	/// Specify the number of outgoing connections we're trying to maintain
-	#[structopt(long = "out-peers", value_name = "OUT_PEERS")]
-	out_peers: Option<u8>,
-
-	/// Specify the maximum number of incoming connections we're accepting
-	#[structopt(long = "in-peers", value_name = "IN_PEERS")]
-	in_peers: Option<u8>,
-
-	/// Specify the pruning mode, a number of blocks to keep or 'archive'. Default is 256.
-	#[structopt(long = "pruning", value_name = "PRUNING_MODE")]
-	pruning: Option<String>,
-
-	/// The human-readable name for this node, as reported to the telemetry server, if enabled
-	#[structopt(long = "name", value_name = "NAME")]
-	name: Option<String>,
-
-	/// Should not connect to the Substrate telemetry server (telemetry is on by default on global chains)
-	#[structopt(long = "no-telemetry")]
-	no_telemetry: bool,
-
-	/// The URL of the telemetry server to connect to
-	#[structopt(long = "telemetry-url", value_name = "TELEMETRY_URL")]
-	telemetry_url: Option<String>,
-
-	/// The means of execution used when calling into the runtime. Can be either wasm, native or both.
-	#[structopt(long = "execution", value_name = "STRATEGY")]
-	execution: Option<ExecutionStrategy>,
-
-	#[allow(missing_docs)]
-	#[structopt(flatten)]
-	shared_flags: SharedFlags,
-
-	#[structopt(subcommand)]
-	cmds: Option<CoreCommands>,
-}
-
-/// How to execute blocks
-#[derive(Debug, StructOpt)]
-pub enum ExecutionStrategy {
-	/// Execute native only
-	Native,
-	/// Execute wasm only
-	Wasm,
-	/// Execute natively when possible, wasm otherwise
-	Both,
-}
-
-impl Default for ExecutionStrategy {
-	fn default() -> Self {
-		ExecutionStrategy::Both
-	}
-}
-
-impl std::str::FromStr for ExecutionStrategy {
-	type Err = String;
-	fn from_str(input: &str) -> Result<Self, Self::Err> {
-		match input {
-			"native" => Ok(ExecutionStrategy::Native),
-			"wasm" | "webassembly" => Ok(ExecutionStrategy::Wasm),
-			"both" => Ok(ExecutionStrategy::Both),
-			_ => Err("Please specify either 'native', 'wasm' or 'both".to_owned())
+use structopt::{StructOpt, clap::{arg_enum, _clap_count_exprs, App, AppSettings, SubCommand}};
+use client;
 
+/// Auxialary macro to implement `GetLogFilter` for all types that have the `shared_params` field.
+macro_rules! impl_get_log_filter {
+	( $type:ident ) => {
+		impl $crate::GetLogFilter for $type {
+			fn get_log_filter(&self) -> Option<String> {
+				self.shared_params.get_log_filter()
+			}
 		}
 	}
 }
 
-/// Flags used by `CoreParams` and almost all `CoreCommands`.
-#[derive(Debug, StructOpt)]
-pub struct SharedFlags {
+arg_enum! {
+	/// How to execute blocks
+	#[derive(Debug, Clone)]
+	pub enum ExecutionStrategy {
+		Native,
+		Wasm,
+		Both,
+	}
+}
+
+impl Into<client::ExecutionStrategy> for ExecutionStrategy {
+	fn into(self) -> client::ExecutionStrategy {
+		match self {
+			ExecutionStrategy::Native => client::ExecutionStrategy::NativeWhenPossible,
+			ExecutionStrategy::Wasm => client::ExecutionStrategy::AlwaysWasm,
+			ExecutionStrategy::Both => client::ExecutionStrategy::Both,
+		}
+	}
+}
+
+/// Shared parameters used by all `CoreParams`.
+#[derive(Debug, StructOpt, Clone)]
+pub struct SharedParams {
 	/// Specify the chain specification (one of dev, local or staging)
 	#[structopt(long = "chain", value_name = "CHAIN_SPEC")]
-	chain: Option<String>,
+	pub chain: Option<String>,
 
 	/// Specify the development chain
 	#[structopt(long = "dev")]
-	dev: bool,
+	pub dev: bool,
 
 	/// Specify custom base path.
-	#[structopt(long = "base-path", short = "d", value_name = "PATH")]
-	base_path: Option<String>,
+	#[structopt(long = "base-path", short = "d", value_name = "PATH", parse(from_os_str))]
+	pub base_path: Option<PathBuf>,
+
+	///Sets a custom logging filter
+	#[structopt(short = "l", long = "log", value_name = "LOG_PATTERN")]
+	pub log: Option<String>,
 }
 
-/// Subcommands provided by Default
-#[derive(Debug, StructOpt)]
-pub enum CoreCommands {
-	/// Build a spec.json file, outputing to stdout
-	#[structopt(name = "build-spec")]
-	BuildSpec {
-		/// Force raw genesis storage output.
-		#[structopt(long = "raw")]
-		raw: bool,
-	},
+impl GetLogFilter for SharedParams {
+	fn get_log_filter(&self) -> Option<String> {
+		self.log.clone()
+	}
+}
 
-	/// Export blocks to a file
-	#[structopt(name = "export-blocks")]
-	ExportBlocks {
-		/// Output file name or stdout if unspecified.
-		#[structopt(parse(from_os_str))]
-		output: Option<PathBuf>,
+/// Parameters used to create the network configuration.
+#[derive(Debug, StructOpt, Clone)]
+pub struct NetworkConfigurationParams {
+	/// Specify a list of bootnodes
+	#[structopt(long = "bootnodes", value_name = "URL")]
+	pub bootnodes: Vec<String>,
 
-		/// Specify starting block number. 1 by default.
-		#[structopt(long = "from", value_name = "BLOCK")]
-		from: Option<u128>,
+	/// Specify a list of reserved node addresses
+	#[structopt(long = "reserved-nodes", value_name = "URL")]
+	pub reserved_nodes: Vec<String>,
 
-		/// Specify last block number. Best block by default.
-		#[structopt(long = "to", value_name = "BLOCK")]
-		to: Option<u128>,
+	/// Listen on this multiaddress
+	#[structopt(long = "listen-addr", value_name = "LISTEN_ADDR")]
+	pub listen_addr: Vec<String>,
 
-		/// Use JSON output rather than binary.
-		#[structopt(long = "json")]
-		json: bool,
+	/// Specify p2p protocol TCP port. Only used if --listen-addr is not specified.
+	#[structopt(long = "port", value_name = "PORT")]
+	pub port: Option<u16>,
 
-		#[allow(missing_docs)]
-		#[structopt(flatten)]
-		shared_flags: SharedFlags,
-	},
+	/// Specify node secret key (64-character hex string)
+	#[structopt(long = "node-key", value_name = "KEY")]
+	pub node_key: Option<String>,
+
+	/// Specify the number of outgoing connections we're trying to maintain
+	#[structopt(long = "out-peers", value_name = "OUT_PEERS", default_value = "25")]
+	pub out_peers: u32,
+
+	/// Specify the maximum number of incoming connections we're accepting
+	#[structopt(long = "in-peers", value_name = "IN_PEERS", default_value = "25")]
+	pub in_peers: u32,
+}
+
+/// The `run` command used to run a node.
+#[derive(Debug, StructOpt, Clone)]
+pub struct RunCmd {
+	/// Specify custom keystore path
+	#[structopt(long = "keystore-path", value_name = "PATH", parse(from_os_str))]
+	pub keystore_path: Option<PathBuf>,
+
+	/// Specify additional key seed
+	#[structopt(long = "key", value_name = "STRING")]
+	pub key: Option<String>,
+
+	/// Enable validator mode
+	#[structopt(long = "validator")]
+	pub validator: bool,
+
+	/// Run in light client mode
+	#[structopt(long = "light")]
+	pub light: bool,
+
+	/// Limit the memory the database cache can use
+	#[structopt(long = "db-cache", value_name = "MiB")]
+	pub database_cache_size: Option<u32>,
+
+	/// Listen to all RPC interfaces (default is local)
+	#[structopt(long = "rpc-external")]
+	pub rpc_external: bool,
+
+	/// Listen to all Websocket interfaces (default is local)
+	#[structopt(long = "ws-external")]
+	pub ws_external: bool,
+
+	/// Specify HTTP RPC server TCP port
+	#[structopt(long = "rpc-port", value_name = "PORT")]
+	pub rpc_port: Option<u16>,
+
+	/// Specify WebSockets RPC server TCP port
+	#[structopt(long = "ws-port", value_name = "PORT")]
+	pub ws_port: Option<u16>,
+
+	/// Specify the pruning mode, a number of blocks to keep or 'archive'. Default is 256.
+	#[structopt(long = "pruning", value_name = "PRUNING_MODE")]
+	pub pruning: Option<String>,
+
+	/// The human-readable name for this node, as reported to the telemetry server, if enabled
+	#[structopt(long = "name", value_name = "NAME")]
+	pub name: Option<String>,
+
+	/// Should not connect to the Substrate telemetry server (telemetry is on by default on global chains)
+	#[structopt(long = "no-telemetry")]
+	pub no_telemetry: bool,
+
+	/// The URL of the telemetry server to connect to
+	#[structopt(long = "telemetry-url", value_name = "TELEMETRY_URL")]
+	pub telemetry_url: Option<String>,
+
+	/// The means of execution used when calling into the runtime. Can be either wasm, native or both.
+	#[structopt(
+		long = "execution",
+		value_name = "STRATEGY",
+		raw(
+			possible_values = "&ExecutionStrategy::variants()",
+			case_insensitive = "true",
+			default_value = r#""Both""#
+		)
+	)]
+	pub execution: ExecutionStrategy,
+
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub shared_params: SharedParams,
+
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub network_config: NetworkConfigurationParams,
+}
+
+impl_augment_clap!(RunCmd);
+impl_get_log_filter!(RunCmd);
+
+/// The `build-spec` command used to build a specification.
+#[derive(Debug, StructOpt, Clone)]
+pub struct BuildSpecCmd {
+	/// Force raw genesis storage output.
+	#[structopt(long = "raw")]
+	pub raw: bool,
+
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub shared_params: SharedParams,
+
+	/// Specify node secret key (64-character hex string)
+	#[structopt(long = "node-key", value_name = "KEY")]
+	pub node_key: Option<String>,
+}
+
+impl_get_log_filter!(BuildSpecCmd);
+
+/// The `export-blocks` command used to export blocks.
+#[derive(Debug, StructOpt, Clone)]
+pub struct ExportBlocksCmd {
+	/// Output file name or stdout if unspecified.
+	#[structopt(parse(from_os_str))]
+	pub output: Option<PathBuf>,
+
+	/// Specify starting block number. 1 by default.
+	#[structopt(long = "from", value_name = "BLOCK")]
+	pub from: Option<u64>,
+
+	/// Specify last block number. Best block by default.
+	#[structopt(long = "to", value_name = "BLOCK")]
+	pub to: Option<u64>,
+
+	/// Use JSON output rather than binary.
+	#[structopt(long = "json")]
+	pub json: bool,
+
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub shared_params: SharedParams,
+}
+
+impl_get_log_filter!(ExportBlocksCmd);
+
+/// The `import-blocks` command used to import blocks.
+#[derive(Debug, StructOpt, Clone)]
+pub struct ImportBlocksCmd {
+	/// Input file or stdin if unspecified.
+	#[structopt(parse(from_os_str))]
+	pub input: Option<PathBuf>,
+
+	/// The means of execution used when executing blocks. Can be either wasm, native or both.
+	#[structopt(
+		long = "execution",
+		value_name = "STRATEGY",
+		raw(
+			possible_values = "&ExecutionStrategy::variants()",
+			case_insensitive = "true",
+			default_value = r#""Both""#
+		)
+	)]
+	pub execution: ExecutionStrategy,
+
+	/// The means of execution used when calling into the runtime. Can be either wasm, native or both.
+	#[structopt(
+		long = "api-execution",
+		value_name = "STRATEGY",
+		raw(
+			possible_values = "&ExecutionStrategy::variants()",
+			case_insensitive = "true",
+			default_value = r#""Both""#
+		)
+	)]
+	pub api_execution: ExecutionStrategy,
+
+	/// The maximum number of 64KB pages to ever allocate for Wasm execution. Don't alter this unless you know what you're doing.
+	#[structopt(long = "max-heap-pages", value_name = "COUNT")]
+	pub max_heap_pages: Option<u32>,
+
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub shared_params: SharedParams,
+}
+
+impl_get_log_filter!(ImportBlocksCmd);
+
+/// The `revert` command used revert the chain to a previos state.
+#[derive(Debug, StructOpt, Clone)]
+pub struct RevertCmd {
+	/// Number of blocks to revert.
+	#[structopt(default_value = "256")]
+	pub num: u64,
+
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub shared_params: SharedParams,
+}
+
+impl_get_log_filter!(RevertCmd);
+
+/// The `purge-chain` command used to remove the whole chain.
+#[derive(Debug, StructOpt, Clone)]
+pub struct PurgeChainCmd {
+	#[allow(missing_docs)]
+	#[structopt(flatten)]
+	pub shared_params: SharedParams,
+}
+
+impl_get_log_filter!(PurgeChainCmd);
+
+/// All core commands that are provided by default.
+///
+/// The core commands are split into multiple subcommands and `Run` is the default subcommand. From
+/// the CLI user perspective, it is not visible that `Run` is a subcommand. So, all parameters of
+/// `Run` are exported as main executable parameters.
+#[derive(Debug, Clone)]
+pub enum CoreParams<CC, RP> {
+	/// Run a node.
+	Run(MergeParameters<RunCmd, RP>),
+
+	/// Build a spec.json file, outputing to stdout.
+	BuildSpec(BuildSpecCmd),
+
+	/// Export blocks to a file.
+	ExportBlocks(ExportBlocksCmd),
 
 	/// Import blocks from file.
-	#[structopt(name = "import-blocks")]
-	ImportBlocks {
-		/// Input file or stdin if unspecified.
-		#[structopt(parse(from_os_str))]
-		input: Option<PathBuf>,
+	ImportBlocks(ImportBlocksCmd),
 
-		/// The means of execution used when executing blocks. Can be either wasm, native or both.
-		#[structopt(long = "execution", value_name = "STRATEGY")]
-		execution: ExecutionStrategy,
-
-		/// The means of execution used when calling into the runtime. Can be either wasm, native or both.
-		#[structopt(long = "api-execution", value_name = "STRATEGY")]
-		api_execution: ExecutionStrategy,
-
-		/// The maximum number of 64KB pages to ever allocate for Wasm execution. Don't alter this unless you know what you're doing.
-		#[structopt(long = "max-heap-pages", value_name = "COUNT")]
-		max_heap_pages: Option<u32>,
-
-		#[allow(missing_docs)]
-		#[structopt(flatten)]
-		shared_flags: SharedFlags,
-	},
-
-	///Revert chain to the previous state
-	#[structopt(name = "revert")]
-	Revert {
-		/// Number of blocks to revert. Default is 256.
-		num: Option<u32>,
-
-		#[allow(missing_docs)]
-		#[structopt(flatten)]
-		shared_flags: SharedFlags,
-	},
+	/// Revert chain to the previous state.
+	Revert(RevertCmd),
 
 	/// Remove the whole chain data.
-	#[structopt(name = "purge-chain")]
-	PurgeChain {
-		#[allow(missing_docs)]
-		#[structopt(flatten)]
-		shared_flags: SharedFlags,
-	},
+	PurgeChain(PurgeChainCmd),
+
+	/// Further custom subcommands.
+	Custom(CC),
+}
+
+impl<CC, RP> StructOpt for CoreParams<CC, RP> where
+	CC: StructOpt + GetLogFilter,
+	RP: StructOpt + AugmentClap
+{
+	fn clap<'a, 'b>() -> App<'a, 'b> {
+		RP::augment_clap(
+			RunCmd::augment_clap(
+				CC::clap().unset_setting(AppSettings::SubcommandRequiredElseHelp)
+			)
+		).subcommand(
+			BuildSpecCmd::augment_clap(SubCommand::with_name("build-spec"))
+				.about("Build a spec.json file, outputing to stdout.")
+		)
+		.subcommand(
+			ExportBlocksCmd::augment_clap(SubCommand::with_name("export-blocks"))
+				.about("Export blocks to a file.")
+		)
+		.subcommand(
+			ImportBlocksCmd::augment_clap(SubCommand::with_name("import-blocks"))
+				.about("Import blocks from file.")
+		)
+		.subcommand(
+			RevertCmd::augment_clap(SubCommand::with_name("revert"))
+				.about("Revert chain to the previous state.")
+		)
+		.subcommand(
+			PurgeChainCmd::augment_clap(SubCommand::with_name("purge-chain"))
+				.about("Remove the whole chain data.")
+		)
+	}
+
+	fn from_clap(matches: &::structopt::clap::ArgMatches) -> Self {
+		match matches.subcommand() {
+			("build-spec", Some(matches)) =>
+				CoreParams::BuildSpec(BuildSpecCmd::from_clap(matches)),
+			("export-blocks", Some(matches)) =>
+				CoreParams::ExportBlocks(ExportBlocksCmd::from_clap(matches)),
+			("import-blocks", Some(matches)) =>
+				CoreParams::ImportBlocks(ImportBlocksCmd::from_clap(matches)),
+			("revert", Some(matches)) => CoreParams::Revert(RevertCmd::from_clap(matches)),
+			("purge-chain", Some(matches)) =>
+				CoreParams::PurgeChain(PurgeChainCmd::from_clap(matches)),
+			(_, None) => CoreParams::Run(MergeParameters::from_clap(matches)),
+			_ => CoreParams::Custom(CC::from_clap(matches)),
+		}
+	}
+}
+
+impl<CC, RP> GetLogFilter for CoreParams<CC, RP> where CC: GetLogFilter {
+	fn get_log_filter(&self) -> Option<String> {
+		match self {
+			CoreParams::Run(c) => c.left.get_log_filter(),
+			CoreParams::BuildSpec(c) => c.get_log_filter(),
+			CoreParams::ExportBlocks(c) => c.get_log_filter(),
+			CoreParams::ImportBlocks(c) => c.get_log_filter(),
+			CoreParams::PurgeChain(c) => c.get_log_filter(),
+			CoreParams::Revert(c) => c.get_log_filter(),
+			CoreParams::Custom(c) => c.get_log_filter(),
+		}
+	}
+}
+
+/// A special commandline parameter that expands to nothing.
+/// Should be used as custom subcommand/run arguments if no custom values are required.
+#[derive(Clone, Debug)]
+pub struct NoCustom {}
+
+impl StructOpt for NoCustom {
+	fn clap<'a, 'b>() -> App<'a, 'b> {
+		App::new("NoCustom")
+	}
+
+	fn from_clap(_: &::structopt::clap::ArgMatches) -> Self {
+		NoCustom {}
+	}
+}
+
+impl AugmentClap for NoCustom {
+	fn augment_clap<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
+		app
+	}
+}
+
+impl GetLogFilter for NoCustom {
+	fn get_log_filter(&self) -> Option<String> {
+		None
+	}
+}
+
+/// Merge all CLI parameters of `L` and `R` into the same level.
+#[derive(Clone, Debug)]
+pub struct MergeParameters<L, R> {
+	/// The left side parameters.
+	pub left: L,
+	/// The right side parameters.
+	pub right: R,
+}
+
+impl<L, R> StructOpt for MergeParameters<L, R> where L: StructOpt + AugmentClap, R: StructOpt {
+	fn clap<'a, 'b>() -> App<'a, 'b> {
+		L::augment_clap(R::clap())
+	}
+
+	fn from_clap(matches: &::structopt::clap::ArgMatches) -> Self {
+		MergeParameters {
+			left: L::from_clap(matches),
+			right: R::from_clap(matches),
+		}
+	}
 }

substrate/core/cli/src/traits.rs

@@ -0,0 +1,44 @@
+// Copyright 2017-2019 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate 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.
+
+// Substrate 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 Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+use structopt::{StructOpt, clap::App};
+
+/// Something that can augment a clapp app with further parameters.
+/// `derive(StructOpt)` is implementing this function by default, so a macro `impl_augment_clap!`
+/// is provided to simplify the implementation of this trait.
+pub trait AugmentClap: StructOpt {
+	/// Augment the given clap `App` with further parameters.
+	fn augment_clap<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b>;
+}
+
+/// Macro for implementing the `AugmentClap` trait.
+/// This requires that the given type uses `derive(StructOpt)`!
+#[macro_export]
+macro_rules! impl_augment_clap {
+	( $type:ident ) => {
+		impl $crate::AugmentClap for $type {
+			fn augment_clap<'a, 'b>(app: $crate::App<'a, 'b>) -> $crate::App<'a, 'b> {
+				$type::augment_clap(app)
+			}
+		}
+	}
+}
+
+/// Returns the log filter given by the user as commandline argument.
+pub trait GetLogFilter {
+	/// Returns the set log filter.
+	fn get_log_filter(&self) -> Option<String>;
+}

substrate/core/network-libp2p/src/secret.rs

@@ -25,16 +25,23 @@ use std::{fs, path::Path};
 const SECRET_FILE: &str = "secret";
 
 /// Obtains or generates the local private key using the configuration.
-pub fn obtain_private_key(
+pub fn obtain_private_key_from_config(
 	config: &NetworkConfiguration
 ) -> Result<secio::SecioKeyPair, IoError> {
-	if let Some(ref secret) = config.use_secret {
+	obtain_private_key(&config.use_secret, &config.net_config_path)
+}
+
+/// Obtains or generates the local private key using the configuration.
+pub fn obtain_private_key(
+	secret: &Option<[u8; 32]>,
+	net_config_path: &Option<String>,
+) -> Result<secio::SecioKeyPair, IoError> {
+	if let Some(ref secret) = secret {
 		// Key was specified in the configuration.
 		secio::SecioKeyPair::secp256k1_raw_key(&secret[..])
 			.map_err(|err| IoError::new(IoErrorKind::InvalidData, err))
-
 	} else {
-		if let Some(ref path) = config.net_config_path {
+		if let Some(ref path) = net_config_path {
 			// Try fetch the key from a the file containing the secret.
 			let secret_path = Path::new(path).join(SECRET_FILE);
 			match load_private_key_from_file(&secret_path) {

substrate/core/network-libp2p/src/service_task.rs

@@ -14,7 +14,10 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
-use crate::{behaviour::Behaviour, behaviour::BehaviourOut, secret::obtain_private_key, transport};
+use crate::{
+	behaviour::Behaviour, behaviour::BehaviourOut, secret::obtain_private_key_from_config,
+	transport
+};
 use crate::custom_proto::{RegisteredProtocol, RegisteredProtocols};
 use crate::topology::NetTopology;
 use crate::{Error, NetworkConfiguration, NodeIndex, ProtocolId, parse_str_addr};
@@ -51,7 +54,7 @@ where TProtos: IntoIterator<Item = RegisteredProtocol> {
 	}
 
 	// Private and public keys configuration.
-	let local_private_key = obtain_private_key(&config)?;
+	let local_private_key = obtain_private_key_from_config(&config)?;
 	let local_public_key = local_private_key.to_public_key();
 	let local_peer_id = local_public_key.clone().into_peer_id();
 

substrate/core/network/src/config.rs

@@ -16,7 +16,7 @@
 
 //! Configuration for the networking layer of Substrate.
 
-pub use network_libp2p::{NonReservedPeerMode, NetworkConfiguration};
+pub use network_libp2p::{NonReservedPeerMode, NetworkConfiguration, Secret};
 
 use chain::Client;
 use codec;

substrate/node/cli/Cargo.toml

@@ -28,7 +28,6 @@ substrate-consensus-aura = { path = "../../core/consensus/aura" }
 substrate-finality-grandpa = { path = "../../core/finality-grandpa" }
 sr-primitives = { path = "../../core/sr-primitives" }
 node-executor = { path = "../executor" }
-structopt = "0.2.13"
 substrate-keystore = { path = "../../core/keystore" }
 
 [dev-dependencies]
@@ -36,5 +35,4 @@ substrate-service-test = { path = "../../core/service/test" }
 
 [build-dependencies]
 substrate-cli = { path = "../../core/cli" }
-structopt = "0.2.13"
-clap = "~2.32"
+structopt = "0.2"

substrate/node/cli/build.rs

@@ -14,16 +14,14 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
-extern crate clap;
 extern crate substrate_cli as cli;
 extern crate structopt;
 
-use std::fs;
-use std::env;
-use clap::Shell;
-use std::path::Path;
+use cli::{NoCustom, CoreParams};
 
-include!("src/params.rs");
+use std::{fs, env, path::Path};
+
+use structopt::{StructOpt, clap::Shell};
 
 fn main() {
 	build_shell_completion();
@@ -32,30 +30,24 @@ fn main() {
 /// Build shell completion scripts for all known shells
 /// Full list in https://github.com/kbknapp/clap-rs/blob/e9d0562a1dc5dfe731ed7c767e6cee0af08f0cf9/src/app/parser.rs#L123
 fn build_shell_completion() {
-	let shells = [Shell::Bash, Shell::Fish, Shell::Zsh, Shell::Elvish, Shell::PowerShell];
-	for shell in shells.iter() {
-    	build_completion(shell);
+	for shell in &[Shell::Bash, Shell::Fish, Shell::Zsh, Shell::Elvish, Shell::PowerShell] {
+		build_completion(shell);
 	}
 }
 
 /// Build the shell auto-completion for a given Shell
 fn build_completion(shell: &Shell) {
-
 	let outdir = match env::var_os("OUT_DIR") {
-        None => return,
-        Some(dir) => dir,
-    };
-    let path = Path::new(&outdir)
-    	.parent().unwrap()
-    	.parent().unwrap()
-    	.parent().unwrap()
-    	.join("completion-scripts");
+		None => return,
+		Some(dir) => dir,
+	};
+	let path = Path::new(&outdir)
+		.parent().unwrap()
+		.parent().unwrap()
+		.parent().unwrap()
+		.join("completion-scripts");
 
-    fs::create_dir(&path).ok();
+	fs::create_dir(&path).ok();
 
-    let mut app = Params::clap();
-    app.gen_completions(
-    	"substrate-node",
-    	*shell,
-    	&path);
+	CoreParams::<NoCustom, NoCustom>::clap().gen_completions("substrate-node", *shell, &path);
 }

substrate/node/cli/src/lib.rs

@@ -44,19 +44,15 @@ extern crate substrate_inherents as inherents;
 
 #[macro_use]
 extern crate log;
-extern crate structopt;
 
 pub use cli::error;
 pub mod chain_spec;
 mod service;
-mod params;
 
 use tokio::prelude::Future;
 use tokio::runtime::Runtime;
-pub use cli::{VersionInfo, IntoExit};
+pub use cli::{VersionInfo, IntoExit, NoCustom};
 use substrate_service::{ServiceFactory, Roles as ServiceRoles};
-use params::{Params as NodeParams};
-use structopt::StructOpt;
 use std::ops::Deref;
 
 /// The chain specification option.
@@ -107,43 +103,31 @@ pub fn run<I, T, E>(args: I, exit: E, version: cli::VersionInfo) -> error::Resul
 	T: Into<std::ffi::OsString> + Clone,
 	E: IntoExit,
 {
-	let full_version = substrate_service::config::full_version_from_strs(
-		version.version,
-		version.commit
-	);
-
-	let matches = match NodeParams::clap()
-		.name(version.executable_name)
-		.author(version.author)
-		.about(version.description)
-		.version(&(full_version + "\n")[..])
-		.get_matches_from_safe(args) {
-			Ok(m) => m,
-			Err(e) => e.exit(),
-		};
-
-	let (spec, config) = cli::parse_matches::<service::Factory, _>(
-		load_spec, &version, "substrate-node", &matches
-	)?;
-
-	match cli::execute_default::<service::Factory, _>(spec, exit, &matches, &config)? {
-		cli::Action::ExecutedInternally => (),
-		cli::Action::RunService(exit) => {
+	cli::parse_and_execute::<service::Factory, NoCustom, NoCustom, _, _, _, _, _>(
+		load_spec, &version, "substrate-node", args, exit,
+		|exit, _custom_args, config| {
 			info!("{}", version.name);
 			info!("  version {}", config.full_version());
 			info!("  by {}, 2017, 2018", version.author);
 			info!("Chain specification: {}", config.chain_spec.name());
 			info!("Node name: {}", config.name);
 			info!("Roles: {:?}", config.roles);
-			let mut runtime = Runtime::new()?;
+			let runtime = Runtime::new().map_err(|e| format!("{:?}", e))?;
 			let executor = runtime.executor();
-			match config.roles == ServiceRoles::LIGHT {
-				true => run_until_exit(runtime, service::Factory::new_light(config, executor)?, exit)?,
-				false => run_until_exit(runtime, service::Factory::new_full(config, executor)?, exit)?,
-			}
+			match config.roles {
+				ServiceRoles::LIGHT => run_until_exit(
+					runtime,
+					service::Factory::new_light(config, executor).map_err(|e| format!("{:?}", e))?,
+					exit
+				),
+				_ => run_until_exit(
+					runtime,
+					service::Factory::new_full(config, executor).map_err(|e| format!("{:?}", e))?,
+					exit
+				),
+			}.map_err(|e| format!("{:?}", e))
 		}
-	}
-	Ok(())
+	).map_err(Into::into).map(|_| ())
 }
 
 fn run_until_exit<T, C, E>(

substrate/node/cli/src/params.rs

@@ -1,25 +0,0 @@
-// Copyright 2018 Parity Technologies (UK) Ltd.
-// This file is part of Substrate.
-
-// Substrate 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.
-
-// Substrate 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 Substrate.  If not, see <http://www.gnu.org/licenses/>.
-
-use structopt::StructOpt;
-use cli::CoreParams;
-
-/// Extend params for Node
-#[derive(Debug, StructOpt)]
-pub struct Params {
-	#[structopt(flatten)]
-	core: CoreParams
-}