Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve runtime check configurability #58

Merged
merged 10 commits into from Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Expand Up @@ -4,7 +4,7 @@ resolver = "2"
members = ["cli", "core"]

[workspace.package]
version = "0.4.0"
version = "0.5.0"
authors = ["Parity Technologies <admin@parity.io>"]
description = "Substrate's programmatic testing framework."
edition = "2021"
Expand Down
15 changes: 7 additions & 8 deletions core/src/commands/create_snapshot.rs
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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(())
Expand Down
15 changes: 7 additions & 8 deletions core/src/commands/execute_block.rs
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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
Expand Down
15 changes: 7 additions & 8 deletions core/src/commands/fast_forward.rs
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 7 additions & 8 deletions core/src/commands/follow_chain.rs
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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);
}
Expand Down
20 changes: 9 additions & 11 deletions core/src/commands/offchain_worker.rs
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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?;

Expand All @@ -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)
Expand Down
24 changes: 14 additions & 10 deletions core/src/commands/on_runtime_upgrade.rs
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions core/src/shared_parameters.rs
Expand Up @@ -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",
Expand Down
54 changes: 20 additions & 34 deletions core/src/state.rs
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down