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

bin/node-template/runtime/build.rs

@@ -19,7 +19,7 @@ use wasm_builder_runner::{build_current_project_with_rustflags, WasmBuilderSourc
 fn main() {
 	build_current_project_with_rustflags(
 		"wasm_binary.rs",
-		WasmBuilderSource::Crates("1.0.8"),
+		WasmBuilderSource::Crates("1.0.9"),
 		// This instructs LLD to export __heap_base as a global variable, which is used by the
 		// external memory allocator.
 		"-Clink-arg=--export=__heap_base",

bin/node/cli/src/cli.rs

@@ -141,7 +141,12 @@ pub fn run<I, T, E>(args: I, exit: E, version: sc_cli::VersionInfo) -> error::Re
 				&version,
 				None,
 			)?;
-			sc_cli::fill_import_params(&mut config, &cli_args.import_params, ServiceRoles::FULL)?;
+			sc_cli::fill_import_params(
+				&mut config,
+				&cli_args.import_params,
+				ServiceRoles::FULL,
+				cli_args.shared_params.dev,
+			)?;
 
 			match ChainSpec::from(config.chain_spec.id()) {
 				Some(ref c) if c == &ChainSpec::Development || c == &ChainSpec::LocalTestnet => {},

bin/node/runtime/build.rs

@@ -21,7 +21,7 @@ fn main() {
 		"wasm_binary.rs",
 		WasmBuilderSource::CratesOrPath {
 			path: "../../../utils/wasm-builder",
-			version: "1.0.8",
+			version: "1.0.9",
 		},
 		// This instructs LLD to export __heap_base as a global variable, which is used by the
 		// external memory allocator.

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;

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;

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,
 

client/executor/runtime-test/build.rs

@@ -21,7 +21,7 @@ fn main() {
 		"wasm_binary.rs",
 		WasmBuilderSource::CratesOrPath {
 			path: "../../../utils/wasm-builder",
-			version: "1.0.8",
+			version: "1.0.9",
 		},
 		// This instructs LLD to export __heap_base as a global variable, which is used by the
 		// external memory allocator.

primitives/runtime-interface/test-wasm/build.rs

@@ -21,7 +21,7 @@ fn main() {
 		"wasm_binary.rs",
 		WasmBuilderSource::CratesOrPath {
 			path: "../../../utils/wasm-builder",
-			version: "1.0.6",
+			version: "1.0.9",
 		},
 		// This instructs LLD to export __heap_base as a global variable, which is used by the
 		// external memory allocator.

test-utils/runtime/build.rs

@@ -21,7 +21,7 @@ fn main() {
 		"wasm_binary.rs",
 		WasmBuilderSource::CratesOrPath {
 			path: "../../utils/wasm-builder",
-			version: "1.0.8",
+			version: "1.0.9",
 		},
 		// Note that we set the stack-size to 1MB explicitly even though it is set
 		// to this value by default. This is because some of our tests (`restoration_of_globals`)

utils/wasm-builder/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "substrate-wasm-builder"
-version = "1.0.8"
+version = "1.0.9"
 authors = ["Parity Technologies <admin@parity.io>"]
 description = "Utility for building WASM binaries"
 edition = "2018"

utils/wasm-builder/src/wasm_project.rs

@@ -357,7 +357,7 @@ fn is_release_build() -> bool {
 			),
 		}
 	} else {
-		!build_helper::debug()
+		true
 	}
 }