From 6fa391d1b4920b189d56c27cb57244514db9f339 Mon Sep 17 00:00:00 2001 From: Tomasz Polaczyk Date: Tue, 20 Feb 2024 13:54:52 +0100 Subject: [PATCH 1/5] Add flag to see storage diff if idemptotent check fails --- core/Cargo.toml | 1 + core/src/commands/on_runtime_upgrade.rs | 37 +++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/core/Cargo.toml b/core/Cargo.toml index 0562c028513..3b2fde14e9c 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -53,6 +53,7 @@ sp-weights = { workspace = true } substrate-rpc-client = { workspace = true } paris = "1.5.15" +similar-asserts = "1.5.0" [dev-dependencies] assert_cmd = { workspace = true } diff --git a/core/src/commands/on_runtime_upgrade.rs b/core/src/commands/on_runtime_upgrade.rs index 4e1298f55af..d3058ae10c8 100644 --- a/core/src/commands/on_runtime_upgrade.rs +++ b/core/src/commands/on_runtime_upgrade.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{fmt::Debug, str::FromStr}; +use std::{collections::BTreeMap, fmt::Debug, str::FromStr}; use bytesize::ByteSize; use frame_try_runtime::UpgradeCheckSelect; @@ -24,7 +24,7 @@ use parity_scale_codec::Encode; use sc_executor::sp_wasm_interface::HostFunctions; use sp_core::{hexdisplay::HexDisplay, Hasher}; use sp_runtime::traits::{Block as BlockT, HashingFor, NumberFor}; -use sp_state_machine::{CompactProof, StorageProof}; +use sp_state_machine::{CompactProof, OverlayedChanges, StorageProof}; use crate::{ build_executor, @@ -69,6 +69,11 @@ pub struct Command { /// Whether to disable migration idempotency checks #[clap(long, default_value = "false", default_missing_value = "true")] pub disable_idempotency_checks: bool, + + /// When migrations are detected as not idempotent, enabling this will output a diff of the + /// storage before and after running the same set of migrations the second time. + #[clap(long, default_value = "false", default_missing_value = "true")] + pub print_storage_diff: bool, } // Runs the `on-runtime-upgrade` command. @@ -187,6 +192,7 @@ where colorize_string("-------------------------------------------------------------------") ); let (oc_pre_root, _) = overlayed_changes.storage_root(&ext.backend, ext.state_version); + overlayed_changes.start_transaction(); match state_machine_call_with_proof::( &ext, &mut overlayed_changes, @@ -202,6 +208,19 @@ where overlayed_changes.storage_root(&ext.backend, ext.state_version); if oc_pre_root != oc_post_root { log::error!("❌ Migrations are not idempotent. Selectively remove migrations from Executive until you find the culprit."); + if command.print_storage_diff { + log::info!("Changed storage keys:"); + let changes_after = + collect_storage_changes_as_hex::(&overlayed_changes); + overlayed_changes.rollback_transaction().unwrap(); + let changes_before = + collect_storage_changes_as_hex::(&overlayed_changes); + + similar_asserts::assert_eq!(changes_before, changes_after); + } else { + log::error!("Run with --print-storage-diff to see list of changed storage keys."); + } + false } else { // Exeuction was OK and state root didn't change @@ -366,3 +385,17 @@ fn analyse_ref_time(ref_time_results: RefTimeInfo, no_weight_warnings: bool) -> WeightSafety::ProbablySafe } } + +fn collect_storage_changes_as_hex( + overlayed_changes: &OverlayedChanges>, +) -> BTreeMap { + overlayed_changes + .changes() + .filter_map(|(key, entry)| { + Some(( + HexDisplay::from(key).to_string(), + HexDisplay::from(entry.value_ref().as_ref()?).to_string(), + )) + }) + .collect() +} From 947f9cf7bee2782f2866c3bf5b5bbe70cea37369 Mon Sep 17 00:00:00 2001 From: tmpolaczyk <44604217+tmpolaczyk@users.noreply.github.com> Date: Mon, 26 Feb 2024 17:38:15 +0100 Subject: [PATCH 2/5] Update core/src/commands/on_runtime_upgrade.rs Co-authored-by: Oliver Tale-Yazdi --- core/src/commands/on_runtime_upgrade.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/commands/on_runtime_upgrade.rs b/core/src/commands/on_runtime_upgrade.rs index d3058ae10c8..fe4d250d156 100644 --- a/core/src/commands/on_runtime_upgrade.rs +++ b/core/src/commands/on_runtime_upgrade.rs @@ -212,7 +212,7 @@ where log::info!("Changed storage keys:"); let changes_after = collect_storage_changes_as_hex::(&overlayed_changes); - overlayed_changes.rollback_transaction().unwrap(); + overlayed_changes.rollback_transaction().expect("Migrations must not rollback transactions that they did not open"); let changes_before = collect_storage_changes_as_hex::(&overlayed_changes); From 7169c5096934b0befb2508fde524e3e52c5fbeb1 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 27 Feb 2024 16:19:42 +1100 Subject: [PATCH 3/5] fmt --- core/src/commands/on_runtime_upgrade.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/commands/on_runtime_upgrade.rs b/core/src/commands/on_runtime_upgrade.rs index fe4d250d156..38086994e89 100644 --- a/core/src/commands/on_runtime_upgrade.rs +++ b/core/src/commands/on_runtime_upgrade.rs @@ -212,7 +212,9 @@ where log::info!("Changed storage keys:"); let changes_after = collect_storage_changes_as_hex::(&overlayed_changes); - overlayed_changes.rollback_transaction().expect("Migrations must not rollback transactions that they did not open"); + overlayed_changes.rollback_transaction().expect( + "Migrations must not rollback transactions that they did not open", + ); let changes_before = collect_storage_changes_as_hex::(&overlayed_changes); From 203a7a9d636d5141fdbd571f7ed2906df7e74fa0 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 27 Feb 2024 16:20:02 +1100 Subject: [PATCH 4/5] update cargo.lock --- Cargo.lock | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d32e9667b18..a550d2aee7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -588,7 +588,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "00ad3f3a942eee60335ab4342358c161ee296829e0d16ff42fc1d6cb07815467" dependencies = [ "anstyle", - "bstr", + "bstr 1.9.0", "doc-comment", "predicates 3.1.0", "predicates-core", @@ -965,6 +965,17 @@ dependencies = [ "tinyvec", ] +[[package]] +name = "bstr" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba3569f383e8f1598449f1a423e72e99569137b47740b1da11ef19af3d5c3223" +dependencies = [ + "lazy_static", + "memchr", + "regex-automata 0.1.10", +] + [[package]] name = "bstr" version = "1.9.0" @@ -2927,7 +2938,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57da3b9b5b85bd66f31093f8c408b90a74431672542466497dcbdfdc02034be1" dependencies = [ "aho-corasick", - "bstr", + "bstr 1.9.0", "log", "regex-automata 0.4.4", "regex-syntax 0.8.2", @@ -9087,6 +9098,26 @@ dependencies = [ "wide", ] +[[package]] +name = "similar" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32fea41aca09ee824cc9724996433064c89f7777e60762749a4170a14abbfa21" +dependencies = [ + "bstr 0.2.17", + "unicode-segmentation", +] + +[[package]] +name = "similar-asserts" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e041bb827d1bfca18f213411d51b665309f1afb37a04a5d1464530e13779fc0f" +dependencies = [ + "console", + "similar", +] + [[package]] name = "siphasher" version = "0.3.11" @@ -11122,6 +11153,7 @@ dependencies = [ "sc-service", "serde", "serde_json", + "similar-asserts", "sp-api", "sp-consensus-aura", "sp-consensus-babe", @@ -11211,6 +11243,12 @@ dependencies = [ "tinyvec", ] +[[package]] +name = "unicode-segmentation" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4c87d22b6e3f4a18d4d40ef354e97c90fcb14dd91d7dc0aa9d8a1172ebf7202" + [[package]] name = "unicode-width" version = "0.1.11" From 239172f31376b4bf1904e346c339198a5115efda Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 27 Feb 2024 16:20:19 +1100 Subject: [PATCH 5/5] bump version --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a550d2aee7a..1522c7806f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11080,7 +11080,7 @@ checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" [[package]] name = "try-runtime-cli" -version = "0.5.4" +version = "0.6.0" dependencies = [ "clap", "env_logger", @@ -11133,7 +11133,7 @@ dependencies = [ [[package]] name = "try-runtime-core" -version = "0.5.4" +version = "0.6.0" dependencies = [ "assert_cmd", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 5f8f10a6343..78e65cf87b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ resolver = "2" members = ["cli", "core"] [workspace.package] -version = "0.5.4" +version = "0.6.0" authors = ["Parity Technologies "] description = "Substrate's programmatic testing framework." edition = "2021"