Address most review grumbles.

bin/node/cli/src/cli.rs

@@ -49,6 +49,7 @@ pub enum Subcommand {
 
 	/// Try some experimental command on the runtime. This includes migration and runtime-upgrade
 	/// testing.
+	#[cfg(feature = "try-runtime")]
 	TryRuntime(try_runtime_cli::TryRuntimeCmd),
 
 	/// Verify a signature for a message, provided on STDIN, with a given (public or secret) key.

bin/node/cli/src/command.rs

@@ -149,6 +149,7 @@ pub fn run() -> Result<()> {
 				Ok((cmd.run(client, backend), task_manager))
 			})
 		},
+		#[cfg(feature = "try-runtime")]
 		Some(Subcommand::TryRuntime(cmd)) => {
 			let runner = cli.create_runner(cmd)?;
 			runner.async_run(|config| {

bin/node/runtime/Cargo.toml

@@ -152,7 +152,6 @@ std = [
 	"pallet-society/std",
 	"pallet-recovery/std",
 	"pallet-vesting/std",
-	"frame-try-runtime/std",
 ]
 runtime-benchmarks = [
 	"frame-benchmarking",

bin/node/runtime/src/lib.rs

@@ -1334,16 +1334,16 @@ impl_runtime_apis! {
 
 	#[cfg(feature = "try-runtime")]
 	impl frame_try_runtime::TryRuntime<Block> for Runtime {
-		fn on_runtime_upgrade(target: frame_try_runtime::Target) -> Weight {
+		fn on_runtime_upgrade(target: frame_try_runtime::Target) -> Result<(Weight, Weight), sp_runtime::RuntimeString> {
 			frame_support::debug::RuntimeLogger::init();
 
 			let weight = match target {
 				frame_try_runtime::Target::All => {
 					frame_support::debug::info!("Dry-running all on-runtime-upgrades.");
-					Executive::try_runtime_upgrade()
+					Executive::try_runtime_upgrade()?
 				},
 				frame_try_runtime::Target::Pallet(name) => {
-					let name = sp_std::str::from_utf8(&name).unwrap();
+					let name = sp_std::str::from_utf8(&name).expect("pallet name is utf8-decodable; qed");
 					frame_support::debug::info!("Dry-running on-runtime-upgrade of {}.", name);
 
 					frame_try_runtime::match_pallet_on_runtime_upgrade!(name,
@@ -1354,12 +1354,10 @@ impl_runtime_apis! {
 						RandomnessCollectiveFlip, Identity,  Society, Recovery, Vesting, Scheduler,
 						Proxy, Multisig, Bounties, Tips, Assets, Mmr, Lottery,
 					)
-					// TODO: ^^ this can be done better, or generated by construct runtime at the
-					// least.
 				}
 			};
 
-			weight
+			Ok((weight, RuntimeBlockWeights::get().max_block))
 		}
 	}
 

frame/executive/src/lib.rs

@@ -220,22 +220,22 @@ where
 	///
 	/// This should only be used for testing.
 	#[cfg(feature = "try-runtime")]
-	pub fn try_runtime_upgrade() -> frame_support::weights::Weight {
+	pub fn try_runtime_upgrade() -> Result<frame_support::weights::Weight, &'static str> {
 		<
 			(frame_system::Module::<System>, COnRuntimeUpgrade, AllModules)
 			as
 			OnRuntimeUpgrade
-		>::pre_upgrade().expect("pre_upgrade hook failed.");
+		>::pre_upgrade()?;
 
 		let weight = Self::execute_on_runtime_upgrade();
 
 		<
 			(frame_system::Module::<System>, COnRuntimeUpgrade, AllModules)
 			as
 			OnRuntimeUpgrade
-		>::post_upgrade().expect("post_upgrade hook failed.");
+		>::post_upgrade()?;
 
-		weight
+		Ok(weight)
 	}
 
 	/// Start the execution of a particular block.

frame/try-runtime/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "frame-try-runtime"
-version = "3.0.0"
+version = "0.9.0"
 authors = ["Parity Technologies <admin@parity.io>"]
 edition = "2018"
 license = "Apache-2.0"
@@ -17,6 +17,7 @@ codec = { package = "parity-scale-codec", version = "2.0.0", default-features =
 
 sp-api = { version = "3.0.0", path = "../../primitives/api", default-features = false }
 sp-std = { version = "3.0.0", path = "../../primitives/std" , default-features = false }
+sp-runtime = { version = "3.0.0", path = "../../primitives/runtime" , default-features = false }
 
 frame-support = { version = "3.0.0", path = "../support", default-features = false }
 
@@ -25,5 +26,6 @@ default = [ "std" ]
 std = [
 	"sp-api/std",
 	"sp-std/std",
+	"sp-runtime/std",
 	"frame-support/std",
 ]

frame/try-runtime/src/lib.rs

@@ -21,6 +21,7 @@
 
 use codec::{Decode, Encode};
 use sp_std::prelude::*;
+use frame_support::weights::Weight;
 
 #[doc(hidden)]
 pub use frame_support as _support;
@@ -33,9 +34,9 @@ macro_rules! match_pallet_on_runtime_upgrade {
 		match $name {
 			$(
 				stringify!($pallet) => {
-					<$pallet as $crate::_support::traits::OnRuntimeUpgrade>::pre_upgrade().unwrap();
+					<$pallet as $crate::_support::traits::OnRuntimeUpgrade>::pre_upgrade()?;
 					let weight = <$pallet as $crate::_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade();
-					<$pallet as $crate::_support::traits::OnRuntimeUpgrade>::post_upgrade().unwrap();
+					<$pallet as $crate::_support::traits::OnRuntimeUpgrade>::post_upgrade()?;
 					weight
 				},
 			)*
@@ -70,8 +71,8 @@ sp_api::decl_runtime_apis! {
 	pub trait TryRuntime {
 		/// dry-run runtime upgrades, returning the total weight consumed.
 		///
-		/// Returns the consumed weight of the migration in case of a successful one, and panics
-		/// otherwise.
-		fn on_runtime_upgrade(target: Target) -> frame_support::weights::Weight;
+		/// Returns the consumed weight of the migration in case of a successful one, combined with
+		/// the total allowed block weight of the runtime.
+		fn on_runtime_upgrade(target: Target) -> Result<(Weight, Weight), sp_runtime::RuntimeString>;
 	}
 }

primitives/core/src/hexdisplay.rs

@@ -53,6 +53,18 @@ impl<'a> sp_std::fmt::Debug for HexDisplay<'a> {
 	}
 }
 
+/// Extension trait for wrapping a [`HexDisplay`].
+pub trait HexDisplayExt {
+	/// Display self as hex string.
+	fn hex_display(&self) -> HexDisplay<'_>;
+}
+
+impl<T: AsBytesRef> HexDisplayExt for T {
+	fn hex_display(&self) -> HexDisplay<'_> {
+		HexDisplay::from(self)
+	}
+}
+
 /// Simple trait to transform various types to `&[u8]`
 pub trait AsBytesRef {
 	/// Transform `self` into `&[u8]`.
@@ -71,6 +83,12 @@ impl AsBytesRef for sp_std::vec::Vec<u8> {
 	fn as_bytes_ref(&self) -> &[u8] { &self }
 }
 
+impl AsBytesRef for sp_storage::StorageKey {
+	fn as_bytes_ref(&self) -> &[u8] {
+		self.as_ref()
+	}
+}
+
 macro_rules! impl_non_endians {
 	( $( $t:ty ),* ) => { $(
 		impl AsBytesRef for $t {

utils/frame/try-runtime/cli/Cargo.toml

@@ -29,4 +29,4 @@ sp-externalities = { version = "0.9.0", path = "../../../../primitives/externali
 sp-core = { version = "3.0.0", path = "../../../../primitives/core" }
 frame-try-runtime = { version = "3.0.0", path = "../../../../frame/try-runtime" }
 
-remote-externalities = { verson = "3.0.0", path = "../remote-externalities" }
+remote-externalities = { path = "../remote-externalities" }

utils/frame/try-runtime/cli/src/lib.rs

@@ -20,10 +20,10 @@
 use parity_scale_codec::{Decode, Encode};
 use std::{fmt::Debug, str::FromStr};
 use sc_service::Configuration;
-use sc_cli::{CliConfiguration, ExecutionStrategy};
-use sc_executor::{WasmExecutionMethod, NativeExecutor};
-use sp_state_machine::StateMachine;
+use sc_cli::{CliConfiguration, ExecutionStrategy, WasmExecutionMethod};
+use sc_executor::NativeExecutor;
 use sc_service::NativeExecutionDispatch;
+use sp_state_machine::StateMachine;
 use sp_runtime::traits::{Block as BlockT, NumberFor};
 use sp_core::storage::{StorageData, StorageKey, well_known_keys};
 use frame_try_runtime::Target;
@@ -45,6 +45,26 @@ pub struct TryRuntimeCmd {
 	/// The state to use to run the migration. Should be a valid FILE or HTTP URI.
 	#[structopt(short, long, default_value = "http://localhost:9933")]
 	pub state: State,
+
+	/// The execution strategy that should be used for benchmarks
+	#[structopt(
+		long = "execution",
+		value_name = "STRATEGY",
+		possible_values = &ExecutionStrategy::variants(),
+		case_insensitive = true,
+		default_value = "Native",
+	)]
+	pub execution: ExecutionStrategy,
+
+	/// Method for executing Wasm runtime code.
+	#[structopt(
+		long = "wasm-execution",
+		value_name = "METHOD",
+		possible_values = &WasmExecutionMethod::enabled_variants(),
+		case_insensitive = true,
+		default_value = "Interpreted"
+	)]
+	pub wasm_method: WasmExecutionMethod,
 }
 
 /// The state to use for a migration dry-run.
@@ -80,19 +100,30 @@ impl TryRuntimeCmd {
 		B: BlockT,
 		ExecDispatch: NativeExecutionDispatch + 'static,
 	{
-
 		let spec = config.chain_spec;
 		let genesis_storage = spec.build_storage()?;
 
-		let code = StorageData(genesis_storage.top.get(well_known_keys::CODE).unwrap().to_vec());
+		let code = StorageData(
+			genesis_storage
+				.top
+				.get(well_known_keys::CODE)
+				.expect("code key must exist in genesis storage; qed")
+				.to_vec(),
+		);
 		let code_key = StorageKey(well_known_keys::CODE.to_vec());
 
-		let wasm_method = WasmExecutionMethod::Interpreted;
-		let strategy = ExecutionStrategy::Native;
-		let mut changes = Default::default();
+		let wasm_method = self.wasm_method;
+		let execution = self.execution;
 
-		let heap_pages = Some(1024);
-		let executor = NativeExecutor::<ExecDispatch>::new(wasm_method, heap_pages, 2);
+		let mut changes = Default::default();
+		// don't really care about these -- use the default values.
+		let max_runtime_instances = config.max_runtime_instances;
+		let heap_pages = config.default_heap_pages;
+		let executor = NativeExecutor::<ExecDispatch>::new(
+			wasm_method.into(),
+			heap_pages,
+			max_runtime_instances,
+		);
 
 		let ext = {
 			use remote_externalities::{Builder, Mode, CacheConfig, OfflineConfig, OnlineConfig};
@@ -106,10 +137,11 @@ impl TryRuntimeCmd {
 				})),
 			};
 
+			// inject the code into this ext.
 			builder.inject(&[(code_key, code)]).build().await
 		};
 
-		let consumed_weight = StateMachine::<_, _, NumberFor<B>, _>::new(
+		let encoded_result = StateMachine::<_, _, NumberFor<B>, _>::new(
 			&ext.backend,
 			None,
 			&mut changes,
@@ -118,17 +150,19 @@ impl TryRuntimeCmd {
 			&self.target.encode(),
 			ext.extensions,
 			&sp_state_machine::backend::BackendRuntimeCode::new(&ext.backend)
-				.runtime_code()
-				.unwrap(),
+				.runtime_code()?,
 			sp_core::testing::TaskExecutor::new(),
 		)
-		.execute(strategy.into())
-		.unwrap();
+		.execute(execution.into())
+		.map_err(|e| format!("failed to execute 'TryRuntime_on_runtime_upgrade' due to {:?}", e))?;
 
-		let weight = <u64 as Decode>::decode(&mut &*consumed_weight).unwrap();
+		let (weight, total_weight) = <(u64, u64) as Decode>::decode(&mut &*encoded_result)
+			.map_err(|e| format!("failed to decode output due to {:?}", e))?;
 		log::info!(
-			"try-runtime executed without errors. Consumed weight = {}",
+			"try-runtime executed without errors. Consumed weight = {}, total weight = {} ({})",
 			weight,
+			total_weight,
+			weight as f64 / total_weight as f64
 		);
 
 		Ok(())

utils/frame/try-runtime/remote-externalities/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "remote-externalities"
-version = "3.0.0"
+version = "0.9.0"
 authors = ["Parity Technologies <admin@parity.io>"]
 edition = "2018"
 license = "Apache-2.0"

utils/frame/try-runtime/remote-externalities/src/lib.rs

@@ -105,11 +105,13 @@ use std::{
 	fs,
 	path::{Path, PathBuf},
 };
-use std::fmt::{Debug, Formatter, Result as FmtResult};
 use log::*;
 use sp_core::{hashing::twox_128};
 pub use sp_io::TestExternalities;
-use sp_core::storage::{StorageKey, StorageData};
+use sp_core::{
+	hexdisplay::HexDisplayExt,
+	storage::{StorageKey, StorageData},
+};
 use futures::future::Future;
 
 type KeyPair = (StorageKey, StorageData);
@@ -119,39 +121,6 @@ type Hash = sp_core::H256;
 
 const LOG_TARGET: &'static str = "remote-ext";
 
-/// Struct for better hex printing of slice types.
-pub struct HexSlice<'a>(&'a [u8]);
-
-impl<'a> HexSlice<'a> {
-	pub fn new<T>(data: &'a T) -> HexSlice<'a>
-	where
-		T: ?Sized + AsRef<[u8]> + 'a,
-	{
-		HexSlice(data.as_ref())
-	}
-}
-
-// You can choose to implement multiple traits, like Lower and UpperHex
-impl Debug for HexSlice<'_> {
-	fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
-		write!(f, "0x")?;
-		for byte in self.0 {
-			write!(f, "{:x}", byte)?;
-		}
-		Ok(())
-	}
-}
-/// Extension trait for hex display.
-pub trait HexDisplayExt {
-	fn hex_display(&self) -> HexSlice<'_>;
-}
-
-impl<T: ?Sized + AsRef<[u8]>> HexDisplayExt for T {
-	fn hex_display(&self) -> HexSlice<'_> {
-		HexSlice::new(self)
-	}
-}
-
 /// The execution mode.
 #[derive(Clone)]
 pub enum Mode {