Clean up CLI interface (#3334)

* Clean up CLI interface

- Removes `--validator` and `--grandpa-voter`
- Make `--alice` etc work without `--dev`

* Style fixes

substrate/core/cli/src/lib.rs

@@ -433,14 +433,11 @@ where
 		),
 	};
 
-	let role =
+	let role = if cli.light {
-		if cli.light {
+		service::Roles::LIGHT
-			service::Roles::LIGHT
+	} else {
-		} else if cli.validator || cli.shared_params.dev {
+		service::Roles::AUTHORITY
-			service::Roles::AUTHORITY
+	};
-		} else {
-			service::Roles::FULL
-		};
 
 	let exec = cli.execution_strategies;
 	let exec_all_or = |strat: params::ExecutionStrategy| exec.execution.unwrap_or(strat).into();
@@ -461,7 +458,6 @@ where
 
 	config.roles = role;
 	config.disable_grandpa = cli.no_grandpa;
-	config.grandpa_voter = cli.grandpa_voter;
 
 	let is_dev = cli.shared_params.dev;
 
@@ -475,26 +471,16 @@ where
 		is_dev,
 	)?;
 
-	fill_transaction_pool_configuration::<F>(
+	fill_transaction_pool_configuration::<F>(&mut config, cli.pool_config)?;
-		&mut config,
-		cli.pool_config,
-	)?;
 
-	if cli.shared_params.dev {
+	config.dev_key_seed = cli.keyring.account.map(|a| format!("//{}", a));
-		config.dev_key_seed = cli.keyring.account
-			.map(|a| format!("//{}", a))
-			.or_else(|| Some("//Alice".into()));
-	}
 
 	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(
+	config.rpc_http = Some(parse_address(&format!("{}:{}", rpc_interface, 9933), cli.rpc_port)?);
-		parse_address(&format!("{}:{}", rpc_interface, 9933), cli.rpc_port)?
+	config.rpc_ws = Some(parse_address(&format!("{}:{}", ws_interface, 9944), cli.ws_port)?);
-	);
+
-	config.rpc_ws = Some(
-		parse_address(&format!("{}:{}", ws_interface, 9944), cli.ws_port)?
-	);
 	config.rpc_ws_max_connections = cli.ws_max_connections;
 	config.rpc_cors = cli.rpc_cors.unwrap_or_else(|| if is_dev {
 		log::warn!("Running in --dev mode, RPC CORS has been disabled.");

substrate/core/cli/src/params.rs

@@ -311,19 +311,10 @@ pub struct ExecutionStrategies {
 /// The `run` command used to run a node.
 #[derive(Debug, StructOpt, Clone)]
 pub struct RunCmd {
-	/// Enable validator mode
-	#[structopt(long = "validator")]
-	pub validator: bool,
-
 	/// Disable GRANDPA when running in validator mode
 	#[structopt(long = "no-grandpa")]
 	pub no_grandpa: bool,
 
-	/// Run GRANDPA voter even when no additional key seed via `--key` is specified. This can for example be of interest
-	/// when running a sentry node in front of a validator, thus needing to forward GRANDPA gossip messages.
-	#[structopt(long = "grandpa-voter")]
-	pub grandpa_voter: bool,
-
 	/// Experimental: Run in light client mode
 	#[structopt(long = "light")]
 	pub light: bool,
@@ -505,7 +496,6 @@ impl AugmentClap for Keyring {
 					.long(&a.name)
 					.help(&a.help)
 					.conflicts_with_all(&conflicts_with_strs)
-					.requires("dev")
 					.takes_value(false)
 			)
 		})

substrate/core/service/src/config.rs

@@ -85,9 +85,6 @@ pub struct Configuration<C, G> {
 	pub force_authoring: bool,
 	/// Disable GRANDPA when running in validator mode
 	pub disable_grandpa: bool,
-	/// Run GRANDPA voter even when no additional key seed is specified. This can for example be of interest when
-	/// running a sentry node in front of a validator, thus needing to forward GRANDPA gossip messages.
-	pub grandpa_voter: bool,
 	/// Node keystore's password
 	pub keystore_password: Option<Protected<String>>,
 	/// Development key seed.
@@ -128,7 +125,6 @@ impl<C: Default, G: Serialize + DeserializeOwned + BuildStorage> Configuration<C
 			offchain_worker: Default::default(),
 			force_authoring: false,
 			disable_grandpa: false,
-			grandpa_voter: false,
 			keystore_password: None,
 			dev_key_seed: None,
 		};

substrate/core/service/test/src/lib.rs

@@ -190,7 +190,6 @@ fn node_config<F: ServiceFactory> (
 		offchain_worker: false,
 		force_authoring: false,
 		disable_grandpa: false,
-		grandpa_voter: false,
 		dev_key_seed: key_seed,
 	}
 }

substrate/node/cli/src/service.rs

@@ -146,26 +146,28 @@ construct_service_factory! {
 					keystore: Some(service.keystore()),
 				};
 
-				if service.config.roles.is_authority() {
+				if !service.config.disable_grandpa {
-					let telemetry_on_connect = TelemetryOnConnect {
+					if service.config.roles.is_authority() {
-						telemetry_connection_sinks: service.telemetry_on_connect_stream(),
+						let telemetry_on_connect = TelemetryOnConnect {
-					};
+							telemetry_connection_sinks: service.telemetry_on_connect_stream(),
-					let grandpa_config = grandpa::GrandpaParams {
+						};
-						config: config,
+						let grandpa_config = grandpa::GrandpaParams {
-						link: link_half,
+							config: config,
-						network: service.network(),
+							link: link_half,
-						inherent_data_providers: service.config.custom.inherent_data_providers.clone(),
+							network: service.network(),
-						on_exit: service.on_exit(),
+							inherent_data_providers: service.config.custom.inherent_data_providers.clone(),
-						telemetry_on_connect: Some(telemetry_on_connect),
+							on_exit: service.on_exit(),
-					};
+							telemetry_on_connect: Some(telemetry_on_connect),
-					service.spawn_task(Box::new(grandpa::run_grandpa_voter(grandpa_config)?));
+						};
-				} else if !service.config.grandpa_voter {
+						service.spawn_task(Box::new(grandpa::run_grandpa_voter(grandpa_config)?));
-					service.spawn_task(Box::new(grandpa::run_grandpa_observer(
+					} else {
-						config,
+						service.spawn_task(Box::new(grandpa::run_grandpa_observer(
-						link_half,
+							config,
-						service.network(),
+							link_half,
-						service.on_exit(),
+							service.network(),
-					)?));
+							service.on_exit(),
+						)?));
+					}
 				}
 
 				Ok(service)