diff --git a/Cargo.lock b/Cargo.lock index 07c95ad5fba..fd8acbffcf9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11652,7 +11652,7 @@ checksum = "3528ecfd12c466c6f163363caf2d02a71161dd5e1cc6ae7b34207ea2d42d81ed" [[package]] name = "try-runtime-cli" -version = "0.4.0" +version = "0.5.0" dependencies = [ "clap", "env_logger", @@ -11707,7 +11707,7 @@ dependencies = [ [[package]] name = "try-runtime-core" -version = "0.4.0" +version = "0.5.0" dependencies = [ "assert_cmd", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index a1cecf0efc3..d4772a9a6cf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ resolver = "2" members = ["cli", "core"] [workspace.package] -version = "0.4.0" +version = "0.5.0" authors = ["Parity Technologies "] description = "Substrate's programmatic testing framework." edition = "2021" diff --git a/core/src/commands/create_snapshot.rs b/core/src/commands/create_snapshot.rs index 8594fb375d3..689256b9783 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, SpecVersionCheck, State, TryRuntimeFeatureCheck}, + state::{LiveState, RuntimeChecks, State}, SharedParams, LOG_TARGET, }; @@ -75,14 +75,13 @@ where }; let executor = build_executor::(&shared); + let runtime_checks = RuntimeChecks { + name_matches: false, + version_increases: false, + try_runtime_feature_enabled: false, + }; let _ = State::Live(command.from) - .to_ext::( - &shared, - &executor, - Some(path.into()), - TryRuntimeFeatureCheck::Skip, - SpecVersionCheck::Skip, - ) + .to_ext::(&shared, &executor, Some(path.into()), runtime_checks) .await?; Ok(()) diff --git a/core/src/commands/execute_block.rs b/core/src/commands/execute_block.rs index 7dd5a109d49..8f1883f2d40 100644 --- a/core/src/commands/execute_block.rs +++ b/core/src/commands/execute_block.rs @@ -27,7 +27,7 @@ use substrate_rpc_client::{ws_client, ChainApi}; use crate::{ build_executor, full_extensions, rpc_err_handler, - state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck}, + state::{LiveState, RuntimeChecks, State}, state_machine_call_with_proof, SharedParams, LOG_TARGET, }; @@ -110,14 +110,13 @@ where let prev_block_live_state = live_state.to_prev_block_live_state::().await?; // Get state for the prev block + let runtime_checks = RuntimeChecks { + name_matches: !shared.disable_spec_name_check, + version_increases: false, + try_runtime_feature_enabled: true, + }; let ext = State::Live(prev_block_live_state) - .to_ext::( - &shared, - &executor, - None, - TryRuntimeFeatureCheck::Check, - SpecVersionCheck::Skip, - ) + .to_ext::(&shared, &executor, None, runtime_checks) .await?; // Execute the desired block on top of it diff --git a/core/src/commands/fast_forward.rs b/core/src/commands/fast_forward.rs index a5e1e9ad390..6304a8acec2 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, SpecVersionCheck, State, TryRuntimeFeatureCheck}, + state::{LiveState, RuntimeChecks, State}, state_machine_call, state_machine_call_with_proof, BlockT, SharedParams, }; @@ -232,15 +232,14 @@ where HostFns: HostFunctions, { let executor = build_executor::(&shared); + let runtime_checks = RuntimeChecks { + name_matches: !shared.disable_spec_name_check, + version_increases: false, + try_runtime_feature_enabled: true, + }; let ext = command .state - .to_ext::( - &shared, - &executor, - None, - TryRuntimeFeatureCheck::Check, - SpecVersionCheck::Skip, - ) + .to_ext::(&shared, &executor, None, runtime_checks) .await?; if command.run_migrations { diff --git a/core/src/commands/follow_chain.rs b/core/src/commands/follow_chain.rs index 09375ad0156..1381d67c9c9 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, SpecVersionCheck, State, TryRuntimeFeatureCheck}, + state::{LiveState, RuntimeChecks, State}, state_machine_call_with_proof, SharedParams, LOG_TARGET, }; @@ -142,14 +142,13 @@ where child_tree: true, hashed_prefixes: vec![], }); + let runtime_checks = RuntimeChecks { + name_matches: !shared.disable_spec_name_check, + version_increases: false, + try_runtime_feature_enabled: true, + }; let ext = state - .to_ext::( - &shared, - &executor, - None, - TryRuntimeFeatureCheck::Check, - SpecVersionCheck::Skip, - ) + .to_ext::(&shared, &executor, None, runtime_checks) .await?; maybe_state_ext = Some(ext); } diff --git a/core/src/commands/offchain_worker.rs b/core/src/commands/offchain_worker.rs index 499af1dbe1f..5b0ab78a579 100644 --- a/core/src/commands/offchain_worker.rs +++ b/core/src/commands/offchain_worker.rs @@ -24,7 +24,7 @@ use substrate_rpc_client::{ws_client, ChainApi}; use crate::{ build_executor, full_extensions, parse, rpc_err_handler, - state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck}, + state::{LiveState, RuntimeChecks, State}, state_machine_call, SharedParams, LOG_TARGET, }; @@ -72,7 +72,7 @@ where as FromStr>::Err: Debug, HostFns: HostFunctions, { - let executor = build_executor::(&shared); + let executor = build_executor(&shared); let block_ws_uri = command.header_ws_uri(); let rpc = ws_client(&block_ws_uri).await?; @@ -86,17 +86,15 @@ where // The block we want to *execute* at is the block passed by the user let execute_at = live_state.at::()?; - let prev_block_live_state = live_state.to_prev_block_live_state::().await?; - // Get state for the prev block + let prev_block_live_state = live_state.to_prev_block_live_state::().await?; + let runtime_checks = RuntimeChecks { + name_matches: !shared.disable_spec_name_check, + version_increases: false, + try_runtime_feature_enabled: true, + }; let ext = State::Live(prev_block_live_state) - .to_ext::( - &shared, - &executor, - None, - TryRuntimeFeatureCheck::Check, - SpecVersionCheck::Skip, - ) + .to_ext::(&shared, &executor, None, runtime_checks) .await?; let header = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, execute_at) diff --git a/core/src/commands/on_runtime_upgrade.rs b/core/src/commands/on_runtime_upgrade.rs index d88ed677eba..9a6936e2484 100644 --- a/core/src/commands/on_runtime_upgrade.rs +++ b/core/src/commands/on_runtime_upgrade.rs @@ -28,7 +28,7 @@ use sp_state_machine::{CompactProof, StorageProof}; use crate::{ build_executor, - state::{SpecVersionCheck, State, TryRuntimeFeatureCheck}, + state::{RuntimeChecks, State}, state_machine_call_with_proof, RefTimeInfo, SharedParams, LOG_TARGET, }; @@ -61,9 +61,14 @@ pub struct Command { #[clap(long, default_value = "false", default_missing_value = "true")] pub no_weight_warnings: bool, + /// Whether to skip enforcing that the new runtime `spec_version` is greater or equal to the + /// existing `spec_version`. + #[clap(long, default_value = "false", default_missing_value = "true")] + pub disable_spec_version_check: bool, + /// Whether to disable migration idempotency checks #[clap(long, default_value = "false", default_missing_value = "true")] - pub no_idempotency_checks: bool, + pub disable_idempotency_checks: bool, } // Runs the `on-runtime-upgrade` command. @@ -77,15 +82,14 @@ where HostFns: HostFunctions, { let executor = build_executor(&shared); + let runtime_checks = RuntimeChecks { + name_matches: !shared.disable_spec_name_check, + version_increases: !command.disable_spec_version_check, + try_runtime_feature_enabled: true, + }; let ext = command .state - .to_ext::( - &shared, - &executor, - None, - TryRuntimeFeatureCheck::Check, - SpecVersionCheck::Check, - ) + .to_ext::(&shared, &executor, None, runtime_checks) .await?; if let State::Live(_) = command.state { @@ -140,7 +144,7 @@ where }; // Check idempotency - let idempotency_ok = match command.no_idempotency_checks { + let idempotency_ok = match command.disable_idempotency_checks { true => { log::info!("ℹ Skipping idempotency check"); true diff --git a/core/src/shared_parameters.rs b/core/src/shared_parameters.rs index 2f766e4cc5e..968d3b255b3 100644 --- a/core/src/shared_parameters.rs +++ b/core/src/shared_parameters.rs @@ -40,6 +40,10 @@ pub struct SharedParams { #[arg(long, default_value = "existing")] pub runtime: Runtime, + /// Whether to disable enforcing the new runtime `spec_name` matches the existing `spec_name`. + #[clap(long, default_value = "false", default_missing_value = "true")] + pub disable_spec_name_check: bool, + /// Type of wasm execution used. #[arg( long = "wasm-execution", diff --git a/core/src/state.rs b/core/src/state.rs index 054f005684e..d04d24866c1 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -140,28 +140,16 @@ pub enum State { Live(LiveState), } -/// Options for [`State::to_ext`] -/// -/// 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 [`State::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, +/// Checks to perform on the given runtime, compared to the existing runtime. +#[derive(Debug)] +pub struct RuntimeChecks { + /// Enforce the `spec_name`s match + pub name_matches: bool, + /// Enforce the `spec_version` of the given is greater or equal to the existing + /// runtime. + pub version_increases: bool, + /// Enforce that the given runtime is compiled with the try-runtime feature. + pub try_runtime_feature_enabled: bool, } impl State { @@ -174,8 +162,7 @@ impl State { shared: &SharedParams, executor: &WasmExecutor, state_snapshot: Option, - try_runtime_check: TryRuntimeFeatureCheck, - spec_version_check: SpecVersionCheck, + runtime_checks: RuntimeChecks, ) -> sc_cli::Result> where Block::Header: DeserializeOwned, @@ -319,25 +306,24 @@ impl State { new_code_hash ); - if new_version.spec_name != old_version.spec_name { - return Err("Spec names must match.".into()); + if runtime_checks.name_matches && new_version.spec_name != old_version.spec_name { + return Err( + "Spec names must match. Use `--disable-spec-name-check` to disable this check." + .into(), + ); } - if spec_version_check == SpecVersionCheck::Check + if runtime_checks.version_increases && 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." - ); + return Err("New runtime spec version must be greater than the on-chain runtime spec version. Use `--disable-spec-version-check` to disable this check.".into()); } } - // whatever runtime we have in store now must have been compiled with try-runtime feature. - if try_runtime_check == TryRuntimeFeatureCheck::Check + if runtime_checks.try_runtime_feature_enabled && !ensure_try_runtime::(executor, &mut ext) { - return Err("given runtime is NOT compiled with try-runtime feature!".into()); + return Err("Given runtime is not compiled with the try-runtime feature.".into()); } Ok(ext) diff --git a/core/tests/on_runtime_upgrade.rs b/core/tests/on_runtime_upgrade.rs index 642824b91c9..2a3d1aa6174 100644 --- a/core/tests/on_runtime_upgrade.rs +++ b/core/tests/on_runtime_upgrade.rs @@ -27,14 +27,16 @@ mod on_runtime_upgrade { fn on_runtime_upgrade( snap_path: &str, runtime_path: &str, - extra_args: &[&str], + command_extra_args: &[&str], + sub_command_extra_args: &[&str], ) -> tokio::process::Child { Command::new(cargo_bin("try-runtime")) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) .arg(format!("--runtime={}", runtime_path)) + .args(command_extra_args) .arg("on-runtime-upgrade") - .args(extra_args) + .args(sub_command_extra_args) .args(["snap", format!("--path={}", snap_path).as_str()]) .kill_on_drop(true) .spawn() @@ -50,10 +52,8 @@ mod on_runtime_upgrade { "{}/tests/runtimes/bridge_hub_rococo_runtime_ok.compact.compressed.wasm", project_root ); - dbg!(&runtime_path, &snap_path); - let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[]); + let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[], &[]); let out = child.wait_with_output().await.unwrap(); - dbg!(&out); assert!(out.status.success()); }) .await; @@ -65,10 +65,10 @@ mod on_runtime_upgrade { let project_root = env!("CARGO_MANIFEST_DIR"); let snap_path = format!("{}/tests/snaps/rococo-bridge-hub.snap", project_root); let runtime_path = format!( - "{}/tests/runtimes/bridge_hub_rococo_runtime_WEIGHT_ISSUE.compact.compressed.wasm", + "{}/tests/runtimes/bridge_hub_rococo_runtime_weight_issue.compact.compressed.wasm", project_root ); - let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[]); + let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[], &[]); let out = child.wait_with_output().await.unwrap(); assert!(!out.status.success()); }) @@ -88,6 +88,7 @@ mod on_runtime_upgrade { let child = on_runtime_upgrade( snap_path.as_str(), runtime_path.as_str(), + &[], &["--no-weight-warnings"], ); let out = child.wait_with_output().await.unwrap(); @@ -106,7 +107,7 @@ mod on_runtime_upgrade { project_root ); - let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[]); + let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[], &[]); let out = child.wait_with_output().await.unwrap(); assert!(!out.status.success()); }) @@ -126,7 +127,8 @@ mod on_runtime_upgrade { let child = on_runtime_upgrade( snap_path.as_str(), runtime_path.as_str(), - &["--no-idempotency-checks"], + &[], + &["--disable-idempotency-checks"], ); let out = child.wait_with_output().await.unwrap(); assert!(out.status.success()); @@ -144,7 +146,7 @@ mod on_runtime_upgrade { project_root ); - let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[]); + let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[], &[]); let out = child.wait_with_output().await.unwrap(); assert!(!out.status.success()); }) @@ -163,11 +165,88 @@ mod on_runtime_upgrade { let child = on_runtime_upgrade( snap_path.as_str(), runtime_path.as_str(), - &["--no-idempotency-checks"], + &[], + &["--disable-idempotency-checks"], ); let out = child.wait_with_output().await.unwrap(); assert!(out.status.success()); }) .await; } + + #[tokio::test] + async fn non_matching_spec_name_fails() { + common::run_with_timeout(Duration::from_secs(300), async move { + let project_root = env!("CARGO_MANIFEST_DIR"); + let snap_path = format!("{}/tests/snaps/rococo-bridge-hub.snap", project_root); + let runtime_path = format!( + "{}/tests/runtimes/bridge_hub_rococo_runtime_bad_spec_name.compact.compressed.wasm", + project_root + ); + + let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[], &[]); + let out = child.wait_with_output().await.unwrap(); + assert!(!out.status.success()); + }) + .await; + } + + #[tokio::test] + async fn non_matching_spec_name_can_be_ignored() { + common::run_with_timeout(Duration::from_secs(300), async move { + let project_root = env!("CARGO_MANIFEST_DIR"); + let snap_path = format!("{}/tests/snaps/rococo-bridge-hub.snap", project_root); + let runtime_path = format!( + "{}/tests/runtimes/bridge_hub_rococo_runtime_bad_spec_name.compact.compressed.wasm", + project_root + ); + let child = on_runtime_upgrade( + snap_path.as_str(), + runtime_path.as_str(), + &["--disable-spec-name-check"], + &[], + ); + let out = child.wait_with_output().await.unwrap(); + assert!(out.status.success()); + }) + .await; + } + + #[tokio::test] + async fn non_incrementing_spec_version_fails() { + common::run_with_timeout(Duration::from_secs(300), async move { + let project_root = env!("CARGO_MANIFEST_DIR"); + let snap_path = format!("{}/tests/snaps/rococo-bridge-hub.snap", project_root); + let runtime_path = format!( + "{}/tests/runtimes/bridge_hub_rococo_runtime_non_incrementing_spec_version.compact.compressed.wasm", + project_root + ); + + let child = on_runtime_upgrade(snap_path.as_str(), runtime_path.as_str(), &[], &[]); + let out = child.wait_with_output().await.unwrap(); + assert!(!out.status.success()); + }) + .await; + } + + #[tokio::test] + async fn non_incrementing_spec_version_can_be_ignored() { + common::run_with_timeout(Duration::from_secs(300), async move { + let project_root = env!("CARGO_MANIFEST_DIR"); + let snap_path = format!("{}/tests/snaps/rococo-bridge-hub.snap", project_root); + let runtime_path = format!( + "{}/tests/runtimes/bridge_hub_rococo_runtime_non_incrementing_spec_version.compact.compressed.wasm", + project_root + ); + let child = on_runtime_upgrade( + snap_path.as_str(), + runtime_path.as_str(), + &[], + &["--disable-spec-version-check"], + ); + let out = child.wait_with_output().await.unwrap(); + assert!(out.status.success()); + }) + .await; + } } diff --git a/core/tests/runtimes/bridge_hub_rococo_runtime_bad_spec_name.compact.compressed.wasm b/core/tests/runtimes/bridge_hub_rococo_runtime_bad_spec_name.compact.compressed.wasm new file mode 100644 index 00000000000..fe4a0b4d479 Binary files /dev/null and b/core/tests/runtimes/bridge_hub_rococo_runtime_bad_spec_name.compact.compressed.wasm differ diff --git a/core/tests/runtimes/bridge_hub_rococo_runtime_non_incrementing_spec_version.compact.compressed.wasm b/core/tests/runtimes/bridge_hub_rococo_runtime_non_incrementing_spec_version.compact.compressed.wasm new file mode 100644 index 00000000000..cc2945187ba Binary files /dev/null and b/core/tests/runtimes/bridge_hub_rococo_runtime_non_incrementing_spec_version.compact.compressed.wasm differ diff --git a/core/tests/runtimes/bridge_hub_rococo_runtime_not_idempotent_execution.compact.compressed.wasm b/core/tests/runtimes/bridge_hub_rococo_runtime_not_idempotent_execution.compact.compressed.wasm index 835cccc0701..a3034cb3268 100644 Binary files a/core/tests/runtimes/bridge_hub_rococo_runtime_not_idempotent_execution.compact.compressed.wasm and b/core/tests/runtimes/bridge_hub_rococo_runtime_not_idempotent_execution.compact.compressed.wasm differ diff --git a/core/tests/runtimes/bridge_hub_rococo_runtime_not_idempotent_state_root.compact.compressed.wasm b/core/tests/runtimes/bridge_hub_rococo_runtime_not_idempotent_state_root.compact.compressed.wasm index c965be9b033..84d81d98cac 100644 Binary files a/core/tests/runtimes/bridge_hub_rococo_runtime_not_idempotent_state_root.compact.compressed.wasm and b/core/tests/runtimes/bridge_hub_rococo_runtime_not_idempotent_state_root.compact.compressed.wasm differ diff --git a/core/tests/runtimes/bridge_hub_rococo_runtime_ok.compact.compressed.wasm b/core/tests/runtimes/bridge_hub_rococo_runtime_ok.compact.compressed.wasm index 4ff6c81fdb0..506f79f229d 100644 Binary files a/core/tests/runtimes/bridge_hub_rococo_runtime_ok.compact.compressed.wasm and b/core/tests/runtimes/bridge_hub_rococo_runtime_ok.compact.compressed.wasm differ diff --git a/core/tests/runtimes/bridge_hub_rococo_runtime_weight_issue.compact.compressed.wasm b/core/tests/runtimes/bridge_hub_rococo_runtime_weight_issue.compact.compressed.wasm index fc03a10bb52..19fdccd972c 100644 Binary files a/core/tests/runtimes/bridge_hub_rococo_runtime_weight_issue.compact.compressed.wasm and b/core/tests/runtimes/bridge_hub_rococo_runtime_weight_issue.compact.compressed.wasm differ diff --git a/core/tests/snaps/rococo-bridge-hub.snap b/core/tests/snaps/rococo-bridge-hub.snap index bb49d8f068b..d13e265a857 100644 Binary files a/core/tests/snaps/rococo-bridge-hub.snap and b/core/tests/snaps/rococo-bridge-hub.snap differ