Improve runtime check configurability (#58)

## Breaking changes
- Rename `--no-idempotency-checks` `--disable-idempotency-checks`
- Changes severity of failing a spec version check from a warning to
execution failure (closes
#45)
- Check spec name by default

## Other changes

- Introduce new shared param `check_spec_name`
- Introduce new `OnRuntimeUpgrade` param `check_spec_version`
- Simplify internal handling of runtime checks 

TODO
- [x] Tests

Cargo.toml

@@ -4,7 +4,7 @@ resolver = "2"
 members = ["cli", "core"]
 
 [workspace.package]
-version = "0.4.0"
+version = "0.5.0"
 authors = ["Parity Technologies <[email protected]>"]
 description = "Substrate's programmatic testing framework."
 edition = "2021"

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::<HostFns>(&shared);
+    let runtime_checks = RuntimeChecks {
+        name_matches: false,
+        version_increases: false,
+        try_runtime_feature_enabled: false,
+    };
     let _ = State::Live(command.from)
-        .to_ext::<Block, HostFns>(
-            &shared,
-            &executor,
-            Some(path.into()),
-            TryRuntimeFeatureCheck::Skip,
-            SpecVersionCheck::Skip,
-        )
+        .to_ext::<Block, HostFns>(&shared, &executor, Some(path.into()), runtime_checks)
         .await?;
 
     Ok(())

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::<Block>().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::<Block, HostFns>(
-            &shared,
-            &executor,
-            None,
-            TryRuntimeFeatureCheck::Check,
-            SpecVersionCheck::Skip,
-        )
+        .to_ext::<Block, HostFns>(&shared, &executor, None, runtime_checks)
         .await?;
 
     // Execute the desired block on top of it

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::<HostFns>(&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::<Block, HostFns>(
-            &shared,
-            &executor,
-            None,
-            TryRuntimeFeatureCheck::Check,
-            SpecVersionCheck::Skip,
-        )
+        .to_ext::<Block, HostFns>(&shared, &executor, None, runtime_checks)
         .await?;
 
     if command.run_migrations {

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::<Block, HostFns>(
-                    &shared,
-                    &executor,
-                    None,
-                    TryRuntimeFeatureCheck::Check,
-                    SpecVersionCheck::Skip,
-                )
+                .to_ext::<Block, HostFns>(&shared, &executor, None, runtime_checks)
                 .await?;
             maybe_state_ext = Some(ext);
         }

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
     <NumberFor<Block> as FromStr>::Err: Debug,
     HostFns: HostFunctions,
 {
-    let executor = build_executor::<HostFns>(&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::<Block>()?;
 
-    let prev_block_live_state = live_state.to_prev_block_live_state::<Block>().await?;
-
     // Get state for the prev block
+    let prev_block_live_state = live_state.to_prev_block_live_state::<Block>().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::<Block, HostFns>(
-            &shared,
-            &executor,
-            None,
-            TryRuntimeFeatureCheck::Check,
-            SpecVersionCheck::Skip,
-        )
+        .to_ext::<Block, HostFns>(&shared, &executor, None, runtime_checks)
         .await?;
 
     let header = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, execute_at)

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::<Block, HostFns>(
-            &shared,
-            &executor,
-            None,
-            TryRuntimeFeatureCheck::Check,
-            SpecVersionCheck::Check,
-        )
+        .to_ext::<Block, HostFns>(&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

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",

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<HostFns>,
         state_snapshot: Option<SnapshotConfig>,
-        try_runtime_check: TryRuntimeFeatureCheck,
-        spec_version_check: SpecVersionCheck,
+        runtime_checks: RuntimeChecks,
     ) -> sc_cli::Result<RemoteExternalities<Block>>
     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::<Block, HostFns>(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)