From 9dfd1523dda161431950997675b61e962fd1ab0f Mon Sep 17 00:00:00 2001 From: Fredrik Simonsson Date: Wed, 1 Nov 2023 10:38:39 +0900 Subject: [PATCH 1/4] Stable build and Refactor to enumerated types Boolean options replaced with an enumerated types to improve readability of the code. --- core/src/commands/create_snapshot.rs | 10 ++++++-- core/src/commands/execute_block.rs | 10 ++++++-- core/src/commands/fast_forward.rs | 10 ++++++-- core/src/commands/follow_chain.rs | 10 ++++++-- core/src/commands/offchain_worker.rs | 10 ++++++-- core/src/commands/on_runtime_upgrade.rs | 13 ++++++++--- core/src/shared_parameters.rs | 11 +++++---- core/src/state.rs | 31 +++++++++++++++++++++---- 8 files changed, 83 insertions(+), 22 deletions(-) diff --git a/core/src/commands/create_snapshot.rs b/core/src/commands/create_snapshot.rs index 003d95df9fc..8594fb375d3 100644 --- a/core/src/commands/create_snapshot.rs +++ b/core/src/commands/create_snapshot.rs @@ -23,7 +23,7 @@ use substrate_rpc_client::{ws_client, StateApi}; use crate::{ build_executor, - state::{LiveState, State}, + state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck}, SharedParams, LOG_TARGET, }; @@ -76,7 +76,13 @@ where let executor = build_executor::(&shared); let _ = State::Live(command.from) - .to_ext::(&shared, &executor, Some(path.into()), false, false) + .to_ext::( + &shared, + &executor, + Some(path.into()), + TryRuntimeFeatureCheck::Skip, + SpecVersionCheck::Skip, + ) .await?; Ok(()) diff --git a/core/src/commands/execute_block.rs b/core/src/commands/execute_block.rs index b36afcbaa67..d9dc17184b4 100644 --- a/core/src/commands/execute_block.rs +++ b/core/src/commands/execute_block.rs @@ -28,7 +28,7 @@ use substrate_rpc_client::{ws_client, ChainApi}; use crate::{ build_executor, full_extensions, rpc_err_handler, - state::{LiveState, State}, + state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck}, state_machine_call_with_proof, SharedParams, LOG_TARGET, }; @@ -97,7 +97,13 @@ where let executor = build_executor::(&shared); let ext = command .state - .to_ext::(&shared, &executor, None, true, false) + .to_ext::( + &shared, + &executor, + None, + TryRuntimeFeatureCheck::Check, + SpecVersionCheck::Skip, + ) .await?; // get the block number associated with this block. diff --git a/core/src/commands/fast_forward.rs b/core/src/commands/fast_forward.rs index 10a2d964a83..3140e892f64 100644 --- a/core/src/commands/fast_forward.rs +++ b/core/src/commands/fast_forward.rs @@ -35,7 +35,7 @@ use crate::{ build_executor, full_extensions, inherent_provider::{Chain, InherentProvider}, rpc_err_handler, - state::{LiveState, State}, + state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck}, state_machine_call, state_machine_call_with_proof, BlockT, SharedParams, }; @@ -234,7 +234,13 @@ where let executor = build_executor::(&shared); let ext = command .state - .to_ext::(&shared, &executor, None, true, false) + .to_ext::( + &shared, + &executor, + None, + TryRuntimeFeatureCheck::Check, + SpecVersionCheck::Skip, + ) .await?; if command.run_migrations { diff --git a/core/src/commands/follow_chain.rs b/core/src/commands/follow_chain.rs index 82af36abd30..f6ac38e9677 100644 --- a/core/src/commands/follow_chain.rs +++ b/core/src/commands/follow_chain.rs @@ -29,7 +29,7 @@ use substrate_rpc_client::{ws_client, ChainApi, FinalizedHeaders, Subscription, use crate::{ build_executor, full_extensions, parse, rpc_err_handler, - state::{LiveState, State}, + state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck}, state_machine_call_with_proof, SharedParams, LOG_TARGET, }; @@ -143,7 +143,13 @@ where hashed_prefixes: vec![], }); let ext = state - .to_ext::(&shared, &executor, None, true, false) + .to_ext::( + &shared, + &executor, + None, + TryRuntimeFeatureCheck::Check, + SpecVersionCheck::Skip, + ) .await?; maybe_state_ext = Some(ext); } diff --git a/core/src/commands/offchain_worker.rs b/core/src/commands/offchain_worker.rs index 2b3daeca83c..fb9b54c2627 100644 --- a/core/src/commands/offchain_worker.rs +++ b/core/src/commands/offchain_worker.rs @@ -26,7 +26,7 @@ use crate::{ build_executor, commands::execute_block::next_hash_of, full_extensions, parse, rpc_err_handler, - state::{LiveState, State}, + state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck}, state_machine_call, SharedParams, LOG_TARGET, }; @@ -78,7 +78,13 @@ where // we first build the externalities with the remote code. let ext = command .state - .to_ext::(&shared, &executor, None, true, false) + .to_ext::( + &shared, + &executor, + None, + TryRuntimeFeatureCheck::Check, + SpecVersionCheck::Skip, + ) .await?; let header_ws_uri = command.header_ws_uri(); diff --git a/core/src/commands/on_runtime_upgrade.rs b/core/src/commands/on_runtime_upgrade.rs index 22a8c9ef421..5787745858c 100644 --- a/core/src/commands/on_runtime_upgrade.rs +++ b/core/src/commands/on_runtime_upgrade.rs @@ -27,8 +27,9 @@ use sp_runtime::traits::{Block as BlockT, NumberFor}; use sp_state_machine::{CompactProof, StorageProof}; use crate::{ - build_executor, state::State, state_machine_call_with_proof, RefTimeInfo, SharedParams, - LOG_TARGET, + build_executor, + state::{SpecVersionCheck, State, TryRuntimeFeatureCheck}, + state_machine_call_with_proof, RefTimeInfo, SharedParams, LOG_TARGET, }; /// Configuration for [`run`]. @@ -76,7 +77,13 @@ where let executor = build_executor(&shared); let ext = command .state - .to_ext::(&shared, &executor, None, true, true) + .to_ext::( + &shared, + &executor, + None, + TryRuntimeFeatureCheck::Check, + SpecVersionCheck::Check, + ) .await?; if let State::Live(_) = command.state { diff --git a/core/src/shared_parameters.rs b/core/src/shared_parameters.rs index 84a457a706e..2f766e4cc5e 100644 --- a/core/src/shared_parameters.rs +++ b/core/src/shared_parameters.rs @@ -17,12 +17,13 @@ use std::{path::PathBuf, str::FromStr}; -use sc_cli::{WasmExecutionMethod, WasmtimeInstantiationStrategy}; -use sp_runtime::StateVersion; -use { - crate::parse, - sc_cli::{DEFAULT_WASMTIME_INSTANTIATION_STRATEGY, DEFAULT_WASM_EXECUTION_METHOD}, +use sc_cli::{ + WasmExecutionMethod, WasmtimeInstantiationStrategy, DEFAULT_WASMTIME_INSTANTIATION_STRATEGY, + DEFAULT_WASM_EXECUTION_METHOD, }; +use sp_runtime::StateVersion; + +use crate::parse; /// Shared parameters of the `try-runtime` commands #[derive(Debug, Clone, clap::Parser)] diff --git a/core/src/state.rs b/core/src/state.rs index aab58bc4aaa..cb829f3367a 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -97,6 +97,25 @@ pub enum State { Live(LiveState), } +/// Options for [`to_ext`] +/// +/// Check if runtime is compiled with try-runtime option or not +#[derive(PartialEq, PartialOrd)] +pub enum TryRuntimeFeatureCheck { + Check, + Skip, +} +/// Options for [`to_ext`] +/// +/// Enable of disable check wether new runtime spec version is not greater +/// than the on-chain runtime spec version in [`to_ext`] + +#[derive(PartialEq, PartialOrd)] +pub enum SpecVersionCheck { + Check, + Skip, +} + impl State { /// Create the [`RemoteExternalities`]. /// @@ -107,8 +126,8 @@ impl State { shared: &SharedParams, executor: &WasmExecutor, state_snapshot: Option, - try_runtime_check: bool, - spec_version_check: bool, + try_runtime_check: TryRuntimeFeatureCheck, + spec_version_check: SpecVersionCheck, ) -> sc_cli::Result> where Block::Header: DeserializeOwned, @@ -256,7 +275,9 @@ impl State { return Err("Spec names must match.".into()); } - if spec_version_check && new_version.spec_version <= old_version.spec_version { + if spec_version_check == SpecVersionCheck::Check + && new_version.spec_version <= old_version.spec_version + { log::warn!( target: LOG_TARGET, "New runtime spec version is not greater than the on-chain runtime spec version. Don't forget to increment the spec version if you intend to use the new code in a runtime upgrade." @@ -265,7 +286,9 @@ impl State { } // whatever runtime we have in store now must have been compiled with try-runtime feature. - if try_runtime_check && !ensure_try_runtime::(executor, &mut ext) { + if try_runtime_check == TryRuntimeFeatureCheck::Check + && !ensure_try_runtime::(executor, &mut ext) + { return Err("given runtime is NOT compiled with try-runtime feature!".into()); } From 92d1ad23cafd7e1e73e9b6e85ee2a4732b9352b3 Mon Sep 17 00:00:00 2001 From: Fredrik Simonsson Date: Fri, 3 Nov 2023 09:27:07 +0900 Subject: [PATCH 2/4] Update core/src/state.rs Co-authored-by: Liam Aharon --- core/src/state.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/state.rs b/core/src/state.rs index cb829f3367a..a8197e583ac 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -107,12 +107,12 @@ pub enum TryRuntimeFeatureCheck { } /// Options for [`to_ext`] /// -/// Enable of disable check wether new runtime spec version is not greater -/// than the on-chain runtime spec version in [`to_ext`] - +/// Whether to check if the new runtime `spec_version` is greater than the previous runtime `spec_version` #[derive(PartialEq, PartialOrd)] pub enum SpecVersionCheck { + /// Check that the new runtime `spec_version` is greater than the previous runtime `spec_version` Check, + /// Don't check that the new runtime `spec_version` is greater than the previous runtime `spec_version` Skip, } From 6215f2435719600cb0e711f843c981da6fc9bece Mon Sep 17 00:00:00 2001 From: Fredrik Simonsson Date: Fri, 3 Nov 2023 09:27:20 +0900 Subject: [PATCH 3/4] Update core/src/state.rs Co-authored-by: Liam Aharon --- core/src/state.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/state.rs b/core/src/state.rs index a8197e583ac..933b9915daa 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -99,10 +99,12 @@ pub enum State { /// Options for [`to_ext`] /// -/// Check if runtime is compiled with try-runtime option or not +/// Whether to check that the runtime was compiled with try-runtime feature #[derive(PartialEq, PartialOrd)] pub enum TryRuntimeFeatureCheck { + /// Check the runtime was compiled with try-runtime feature Check, + /// Don't check if the runtime was compiled with try-runtime feature Skip, } /// Options for [`to_ext`] From 07280e7fbdf697b3b9dd477becd86bb115f5cdad Mon Sep 17 00:00:00 2001 From: Fredrik Simonsson Date: Fri, 3 Nov 2023 09:28:51 +0900 Subject: [PATCH 4/4] Auto reformat --- core/src/state.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/core/src/state.rs b/core/src/state.rs index 933b9915daa..03d986dc2ca 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -109,12 +109,15 @@ pub enum TryRuntimeFeatureCheck { } /// Options for [`to_ext`] /// -/// Whether to check if the new runtime `spec_version` is greater than the previous runtime `spec_version` +/// Whether to check if the new runtime `spec_version` is greater than the previous runtime +/// `spec_version` #[derive(PartialEq, PartialOrd)] pub enum SpecVersionCheck { - /// Check that the new runtime `spec_version` is greater than the previous runtime `spec_version` + /// Check that the new runtime `spec_version` is greater than the previous runtime + /// `spec_version` Check, - /// Don't check that the new runtime `spec_version` is greater than the previous runtime `spec_version` + /// Don't check that the new runtime `spec_version` is greater than the previous runtime + /// `spec_version` Skip, }