another mega refactor

bin/node/runtime/src/lib.rs

@@ -1545,9 +1545,12 @@ impl_runtime_apis! {
 
 	#[cfg(feature = "try-runtime")]
 	impl frame_try_runtime::TryRuntime<Block> for Runtime {
-		fn on_runtime_upgrade() -> Result<(Weight, Weight), sp_runtime::RuntimeString> {
-			let weight = Executive::try_runtime_upgrade()?;
-			Ok((weight, RuntimeBlockWeights::get().max_block))
+		fn on_runtime_upgrade() -> (Weight, Weight) {
+			// NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to
+			//  have a backtrace here. If any of the pre/post migration checks fail, we shall stop
+			// right here and right now.
+			let weight = Executive::try_runtime_upgrade().unwrap();
+			(weight, RuntimeBlockWeights::get().max_block)
 		}
 
 		fn execute_block_no_state_root_check(block: Block) -> Weight {

frame/executive/src/lib.rs

@@ -237,6 +237,41 @@ where
 		frame_system::Pallet::<System>::block_weight().total()
 	}
 
+	// #[cfg(feature = "try-runtime")]
+	// fn execute_extrinsics_with_book_keeping_no_signature_check(
+	// 	extrinsics: Vec<Block::Extrinsic>,
+	// 	block_number: NumberFor<Block>,
+	// ) {
+	// 	extrinsics.into_iter().for_each(|uxt| {
+	// 		let encoded = uxt.encode();
+	// 		let encoded_len = encoded.len();
+
+	// 		// skip signature verification
+	// 		let xt = Self::blind_check(uxt)?;
+
+	// 		// We don't need to make sure to `note_extrinsic` only after we know it's going to be
+	// 		// executed to prevent it from leaking in storage since at this point, it will either
+	// 		// execute or panic (and revert storage changes).
+	// 		<frame_system::Pallet<System>>::note_extrinsic(encoded);
+
+	// 		// AUDIT: Under no circumstances may this function panic from here onwards.
+
+	// 		// Decode parameters and dispatch
+	// 		let dispatch_info = xt.get_dispatch_info();
+	// 		let r = Applyable::apply::<UnsignedValidator>(xt, &dispatch_info, encoded_len)
+	// 			.map(|_| ())
+	// 			.map_err(|e| e.error)
+	// 			.unwrap();
+
+	// 		<frame_system::Pallet<System>>::note_applied_extrinsic(&r, dispatch_info);
+	// 	});
+
+	// 	// post-extrinsics book-keeping
+	// 	<frame_system::Pallet<System>>::note_finished_extrinsics();
+
+	// 	Self::idle_and_finalize_hook(block_number);
+	// }
+
 	/// Execute all `OnRuntimeUpgrade` of this runtime, including the pre and post migration checks.
 	///
 	/// This should only be used for testing.

frame/try-runtime/src/lib.rs

@@ -32,7 +32,7 @@ sp_api::decl_runtime_apis! {
 		///
 		/// 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() -> Result<(Weight, Weight), sp_runtime::RuntimeString>;
+		fn on_runtime_upgrade() -> (Weight, Weight);
 
 		/// Execute the given block, but don't check that its state root matches that of yours.
 		///

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

@@ -112,8 +112,8 @@ pub struct OnlineConfig<B: BlockT> {
 	pub at: Option<B::Hash>,
 	/// An optional state snapshot file to WRITE to, not for reading. Not written if set to `None`.
 	pub state_snapshot: Option<SnapshotConfig>,
-	/// The modules to scrape. If empty, entire chain state will be scraped.
-	pub modules: Vec<String>,
+	/// The pallets to scrape. If empty, entire chain state will be scraped.
+	pub pallets: Vec<String>,
 	/// Transport config.
 	pub transport: Transport,
 }
@@ -134,7 +134,7 @@ impl<B: BlockT> Default for OnlineConfig<B> {
 			transport: Transport { uri: DEFAULT_TARGET.to_owned(), client: None },
 			at: None,
 			state_snapshot: None,
-			modules: vec![],
+			pallets: vec![],
 		}
 	}
 }
@@ -360,9 +360,9 @@ impl<B: BlockT> Builder<B> {
 			.clone();
 		info!(target: LOG_TARGET, "scraping key-pairs from remote @ {:?}", at);
 
-		let mut keys_and_values = if config.modules.len() > 0 {
+		let mut keys_and_values = if config.pallets.len() > 0 {
 			let mut filtered_kv = vec![];
-			for f in config.modules.iter() {
+			for f in config.pallets.iter() {
 				let hashed_prefix = StorageKey(twox_128(f.as_bytes()).to_vec());
 				let module_kv = self.rpc_get_pairs_paged(hashed_prefix.clone(), at).await?;
 				info!(
@@ -376,7 +376,7 @@ impl<B: BlockT> Builder<B> {
 			}
 			filtered_kv
 		} else {
-			info!(target: LOG_TARGET, "downloading data for all modules.");
+			info!(target: LOG_TARGET, "downloading data for all pallets.");
 			self.rpc_get_pairs_paged(StorageKey(vec![]), at).await?
 		};
 
@@ -552,7 +552,7 @@ mod remote_tests {
 		init_logger();
 		Builder::<Block>::new()
 			.mode(Mode::Online(OnlineConfig {
-				modules: vec!["System".to_owned()],
+				pallets: vec!["System".to_owned()],
 				..Default::default()
 			}))
 			.build()
@@ -566,7 +566,7 @@ mod remote_tests {
 		init_logger();
 		Builder::<Block>::new()
 			.mode(Mode::Online(OnlineConfig {
-				modules: vec![
+				pallets: vec![
 					"Proxy".to_owned(),
 					"Multisig".to_owned(),
 					"PhragmenElection".to_owned(),
@@ -594,7 +594,7 @@ mod remote_tests {
 		init_logger();
 		Builder::<Block>::new()
 			.mode(Mode::Online(OnlineConfig {
-				modules: vec!["PhragmenElection".to_owned()],
+				pallets: vec!["PhragmenElection".to_owned()],
 				..Default::default()
 			}))
 			.build()
@@ -620,7 +620,7 @@ mod remote_tests {
 		Builder::<Block>::new()
 			.mode(Mode::Online(OnlineConfig {
 				state_snapshot: Some(SnapshotConfig::new("test_snapshot_to_remove.bin")),
-				modules: vec!["Balances".to_owned()],
+				pallets: vec!["Balances".to_owned()],
 				..Default::default()
 			}))
 			.build()

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

@@ -28,6 +28,7 @@ sp-core = { version = "4.0.0-dev", path = "../../../../primitives/core" }
 sp-io = { version = "4.0.0-dev", path = "../../../../primitives/io" }
 sp-keystore = { version = "0.10.0-dev", path = "../../../../primitives/keystore" }
 sp-externalities = { version = "0.10.0-dev", path = "../../../../primitives/externalities" }
+sp-version = { version = "4.0.0-dev", path = "../../../../primitives/version" }
 
 remote-externalities = { version = "0.10.0-dev", path = "../../remote-externalities" }
 jsonrpsee-ws-client = { version = "0.3.0", default-features = false, features = [

utils/frame/try-runtime/cli/src/commands/execute_block.rs

@@ -1,33 +1,32 @@
-use crate::{check_spec_name, extract_code, hash_of, parse, SharedParams, State, LOG_TARGET};
+use crate::{
+	build_executor, ensure_matching_spec_name, extract_code, full_extensions, hash_of,
+	local_spec_name, state_machine_call, SharedParams, State, LOG_TARGET,
+};
 use remote_externalities::rpc_api;
-use sc_executor::NativeElseWasmExecutor;
 use sc_service::{Configuration, NativeExecutionDispatch};
-use sp_core::{
-	hashing::twox_128,
-	offchain::{
-		testing::{TestOffchainExt, TestTransactionPoolExt},
-		OffchainDbExt, OffchainWorkerExt, TransactionPoolExt,
-	},
-	storage::well_known_keys,
-};
-use sp_keystore::{testing::KeyStore, KeystoreExt};
+use sp_core::storage::well_known_keys;
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor};
-use sp_state_machine::StateMachine;
-use std::{fmt::Debug, str::FromStr, sync::Arc};
+use std::{fmt::Debug, str::FromStr};
 
 #[derive(Debug, Clone, structopt::StructOpt)]
 pub struct ExecuteBlockCmd {
+	/// Overwrite the wasm code in state or not.
 	#[structopt(long)]
 	overwrite_wasm_code: bool,
 
+	/// If set, then the state root check is disabled by the virtue of calling into
+	/// `TryRuntime_execute_block_no_state_root_check` instead of `Core_execute_block`.
+	#[structopt(long)]
+	no_state_root_check: bool,
+
 	/// The block hash at which to fetch the block.
 	///
 	/// If the `live` state type is being used, then this can be omitted, and is equal to whatever
 	/// the `state::at` is. Only use this (with care) when combined with a snapshot.
 	#[structopt(
 		long,
 		multiple = false,
-		parse(try_from_str = parse::hash)
+		parse(try_from_str = crate::parse::hash)
 	)]
 	block_at: Option<String>,
 
@@ -38,7 +37,7 @@ pub struct ExecuteBlockCmd {
 	#[structopt(
 		long,
 		multiple = false,
-		parse(try_from_str = parse::url)
+		parse(try_from_str = crate::parse::url)
 	)]
 	block_uri: Option<String>,
 
@@ -61,9 +60,9 @@ impl ExecuteBlockCmd {
 				log::warn!(target: LOG_TARGET, "--block-at is provided while state type is live, this will most likely lead to a nonsensical result.");
 				hash_of::<Block>(&block_at)
 			},
-			(None, State::Live { at, .. }) => hash_of::<Block>(&at),
-			(None, State::Snap { .. }) => {
-				panic!("either `--block-at` must be provided, or state must be `live`");
+			(None, State::Live { at: Some(at), .. }) => hash_of::<Block>(&at),
+			_ => {
+				panic!("either `--block-at` must be provided, or state must be `live with a proper `--at``");
 			},
 		}
 	}
@@ -100,17 +99,8 @@ where
 	<NumberFor<Block> as FromStr>::Err: Debug,
 	ExecDispatch: NativeExecutionDispatch + 'static,
 {
-	let wasm_method = shared.wasm_method;
+	let executor = build_executor::<ExecDispatch>(&shared, &config);
 	let execution = shared.execution;
-	let heap_pages = shared.heap_pages.or(config.default_heap_pages);
-
-	let mut changes = Default::default();
-	let max_runtime_instances = config.max_runtime_instances;
-	let executor = NativeElseWasmExecutor::<ExecDispatch>::new(
-		wasm_method.into(),
-		heap_pages,
-		max_runtime_instances,
-	);
 
 	let block_at = command.block_at::<Block>()?;
 	let block_uri = command.block_uri::<Block>();
@@ -123,35 +113,21 @@ where
 		parent_hash
 	);
 
-	check_spec_name::<Block>(block_uri.clone(), config.chain_spec.name().to_string()).await;
-
 	let ext = {
 		let builder = command
 			.state
 			.builder::<Block>()?
 			// make sure the state is being build with the parent hash, if it is online.
-			.overwrite_online_at(parent_hash.to_owned())
-			.inject_hashed_key(&[twox_128(b"System"), twox_128(b"LastRuntimeUpgrade")].concat());
+			.overwrite_online_at(parent_hash.to_owned());
 
 		let builder = if command.overwrite_wasm_code {
-			let (code_key, code) = extract_code(config.chain_spec)?;
+			let (code_key, code) = extract_code(&config.chain_spec)?;
 			builder.inject_key_value(&[(code_key, code)])
 		} else {
 			builder.inject_hashed_key(well_known_keys::CODE)
 		};
 
-		let mut ext = builder.build().await?;
-
-		// register externality extensions in order to provide host interface for OCW to the
-		// runtime.
-		let (offchain, _offchain_state) = TestOffchainExt::new();
-		let (pool, _pool_state) = TestTransactionPoolExt::new();
-		ext.register_extension(OffchainDbExt::new(offchain.clone()));
-		ext.register_extension(OffchainWorkerExt::new(offchain));
-		ext.register_extension(KeystoreExt(Arc::new(KeyStore::new())));
-		ext.register_extension(TransactionPoolExt::new(pool));
-
-		ext
+		builder.build().await?
 	};
 
 	// A digest item gets added when the runtime is processing the block, so we need to pop
@@ -160,19 +136,21 @@ where
 	header.digest_mut().pop();
 	let block = Block::new(header, extrinsics);
 
-	let _encoded_result = StateMachine::<_, _, NumberFor<Block>, _>::new(
-		&ext.backend,
-		None,
-		&mut changes,
+	let expected_spec_name = local_spec_name::<Block, ExecDispatch>(&ext, &executor);
+	ensure_matching_spec_name::<Block>(block_uri.clone(), expected_spec_name).await;
+
+	let _ = state_machine_call::<Block, ExecDispatch>(
+		&ext,
 		&executor,
-		"Core_execute_block",
+		execution,
+		if command.no_state_root_check {
+			"Core_execute_block"
+		} else {
+			"TryRuntime_execute_block_no_state_root_check"
+		},
 		block.encode().as_ref(),
-		ext.extensions,
-		&sp_state_machine::backend::BackendRuntimeCode::new(&ext.backend).runtime_code()?,
-		sp_core::testing::TaskExecutor::new(),
-	)
-	.execute(execution.into())
-	.map_err(|e| format!("failed to execute 'Core_execute_block': {:?}", e))?;
+		full_extensions(),
+	)?;
 
 	log::info!(target: LOG_TARGET, "Core_execute_block executed without errors.");