Make debug builds more usable (#4683)

* Make debug builds more usable

This pr makes debug builds more usable in terms of `cargo run -- --dev`.

1. `--dev` activates `--execution native`, iff `--execution` is not
given or no sub `--execution-*` is given.
2. It was probably a mistake to compile WASM in debug for a debug build.
So, we now build the WASM binary always as `release` (if not requested
differently by the user). So, we trade compilation time for a better
debug experience.

* Make sure we only overwrite default values

* Make it work

* Apply suggestion

substrate/client/cli/src/execution_strategy.rs

@@ -20,7 +20,7 @@ use structopt::clap::arg_enum;
 
 arg_enum! {
 	/// How to execute blocks
-	#[derive(Debug, Clone, Copy)]
+	#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 	pub enum ExecutionStrategy {
 		// Execute with native build (if available, WebAssembly otherwise).
 		Native,
@@ -33,3 +33,25 @@ arg_enum! {
 	}
 }
 
+impl ExecutionStrategy {
+	/// Returns the variant as `'&static str`.
+	pub fn as_str(&self) -> &'static str {
+		match self {
+			Self::Native => "Native",
+			Self::Wasm => "Wasm",
+			Self::Both => "Both",
+			Self::NativeElseWasm => "NativeElseWasm",
+		}
+	}
+}
+
+/// Default value for the `--execution-syncing` parameter.
+pub const DEFAULT_EXECUTION_SYNCING: ExecutionStrategy = ExecutionStrategy::NativeElseWasm;
+/// Default value for the `--execution-import-block` parameter.
+pub const DEFAULT_EXECUTION_IMPORT_BLOCK: ExecutionStrategy = ExecutionStrategy::NativeElseWasm;
+/// Default value for the `--execution-block-construction` parameter.
+pub const DEFAULT_EXECUTION_BLOCK_CONSTRUCTION: ExecutionStrategy = ExecutionStrategy::Wasm;
+/// Default value for the `--execution-offchain-worker` parameter.
+pub const DEFAULT_EXECUTION_OFFCHAIN_WORKER: ExecutionStrategy = ExecutionStrategy::Native;
+/// Default value for the `--execution-other` parameter.
+pub const DEFAULT_EXECUTION_OTHER: ExecutionStrategy = ExecutionStrategy::Native;

substrate/client/cli/src/lib.rs

@@ -40,6 +40,7 @@ use sc_network::{
 	},
 };
 use sp_core::H256;
+use execution_strategy::*;
 
 use std::{
 	io::{Write, Read, Seek, Cursor, stdin, stdout, ErrorKind}, iter, fmt::Debug, fs::{self, File},
@@ -556,7 +557,12 @@ impl<'a> ParseAndPrepareImport<'a> {
 			self.version,
 			None,
 		)?;
-		fill_import_params(&mut config, &self.params.import_params, sc_service::Roles::FULL)?;
+		fill_import_params(
+			&mut config,
+			&self.params.import_params,
+			sc_service::Roles::FULL,
+			self.params.shared_params.dev,
+		)?;
 
 		let file: Box<dyn ReadPlusSeek + Send> = match self.params.input {
 			Some(filename) => Box::new(File::open(filename)?),
@@ -620,7 +626,12 @@ impl<'a> CheckBlock<'a> {
 			self.version,
 			None,
 		)?;
-		fill_import_params(&mut config, &self.params.import_params, sc_service::Roles::FULL)?;
+		fill_import_params(
+			&mut config,
+			&self.params.import_params,
+			sc_service::Roles::FULL,
+			self.params.shared_params.dev,
+		)?;
 		fill_config_keystore_in_memory(&mut config)?;
 
 		let input = if self.params.input.starts_with("0x") { &self.params.input[2..] } else { &self.params.input[..] };
@@ -904,6 +915,7 @@ pub fn fill_import_params<C, G, E>(
 	config: &mut Configuration<C, G, E>,
 	cli: &ImportParams,
 	role: sc_service::Roles,
+	is_dev: bool,
 ) -> error::Result<()>
 	where
 		C: Default,
@@ -943,13 +955,22 @@ pub fn fill_import_params<C, G, E>(
 	config.wasm_method = cli.wasm_method.into();
 
 	let exec = &cli.execution_strategies;
-	let exec_all_or = |strat: ExecutionStrategy| exec.execution.unwrap_or(strat).into();
+	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),
-		importing: exec_all_or(exec.execution_import_block),
-		block_construction: exec_all_or(exec.execution_block_construction),
-		offchain_worker: exec_all_or(exec.execution_offchain_worker),
-		other: exec_all_or(exec.execution_other),
+		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(())
 }
@@ -987,7 +1008,7 @@ where
 			sc_service::Roles::FULL
 		};
 
-	fill_import_params(&mut config, &cli.import_params, role)?;
+	fill_import_params(&mut config, &cli.import_params, role, is_dev)?;
 
 	config.impl_name = impl_name;
 	config.impl_commit = version.commit;

substrate/client/cli/src/params.rs

@@ -18,6 +18,7 @@ use crate::traits::GetSharedParams;
 
 use std::{str::FromStr, path::PathBuf};
 use structopt::{StructOpt, StructOptInternal, clap::{arg_enum, App, AppSettings, SubCommand, Arg}};
+use crate::execution_strategy::*;
 
 pub use crate::execution_strategy::ExecutionStrategy;
 
@@ -321,7 +322,7 @@ pub struct ExecutionStrategies {
 		value_name = "STRATEGY",
 		possible_values = &ExecutionStrategy::variants(),
 		case_insensitive = true,
-		default_value = "NativeElseWasm"
+		default_value = DEFAULT_EXECUTION_SYNCING.as_str(),
 	)]
 	pub execution_syncing: ExecutionStrategy,
 
@@ -331,7 +332,7 @@ pub struct ExecutionStrategies {
 		value_name = "STRATEGY",
 		possible_values = &ExecutionStrategy::variants(),
 		case_insensitive = true,
-		default_value = "NativeElseWasm"
+		default_value = DEFAULT_EXECUTION_IMPORT_BLOCK.as_str(),
 	)]
 	pub execution_import_block: ExecutionStrategy,
 
@@ -341,7 +342,7 @@ pub struct ExecutionStrategies {
 		value_name = "STRATEGY",
 		possible_values = &ExecutionStrategy::variants(),
 		case_insensitive = true,
-		default_value = "Wasm"
+		default_value = DEFAULT_EXECUTION_BLOCK_CONSTRUCTION.as_str(),
 	)]
 	pub execution_block_construction: ExecutionStrategy,
 
@@ -351,7 +352,7 @@ pub struct ExecutionStrategies {
 		value_name = "STRATEGY",
 		possible_values = &ExecutionStrategy::variants(),
 		case_insensitive = true,
-		default_value = "Native"
+		default_value = DEFAULT_EXECUTION_OFFCHAIN_WORKER.as_str(),
 	)]
 	pub execution_offchain_worker: ExecutionStrategy,
 
@@ -361,7 +362,7 @@ pub struct ExecutionStrategies {
 		value_name = "STRATEGY",
 		possible_values = &ExecutionStrategy::variants(),
 		case_insensitive = true,
-		default_value = "Native"
+		default_value = DEFAULT_EXECUTION_OTHER.as_str(),
 	)]
 	pub execution_other: ExecutionStrategy,