From bfce676f08df19f16bbe795c511807d5dc3cf800 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 21 Sep 2022 14:08:30 +0200 Subject: [PATCH 01/23] [Feature] Sequential migration execution for try-runtime --- frame/support/src/traits/hooks.rs | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index eb1106e4c383f..e2d81f2ab400f 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -23,7 +23,7 @@ use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; #[cfg(feature = "try-runtime")] -use codec::{Decode, Encode}; +use codec::Decode; /// The block initialization trait. /// @@ -165,6 +165,7 @@ pub trait OnRuntimeUpgrade { #[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] #[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] impl OnRuntimeUpgrade for Tuple { + #[cfg(not(feature = "try-runtime"))] fn on_runtime_upgrade() -> Weight { let mut weight = Weight::zero(); for_tuples!( #( weight = weight.saturating_add(Tuple::on_runtime_upgrade()); )* ); @@ -172,20 +173,29 @@ impl OnRuntimeUpgrade for Tuple { } #[cfg(feature = "try-runtime")] + /// We are executing pre and post checks sequentially in order to be able to test seveal + /// consecutive migrations for the same pallet without errors. Therefore pre and post upgrade + /// hooks for tuples are a noop. + fn on_runtime_upgrade() -> Weight { + let mut state: Vec = Vec::default(); + let mut weight = Weight::zero(); + for_tuples!( #( + state = Tuple::pre_upgrade().unwrap(); + weight = weight.saturating_add(Tuple::on_runtime_upgrade()); + Tuple::post_upgrade(state).unwrap(); + )* ); + weight + } + + #[cfg(feature = "try-runtime")] + /// noop fn pre_upgrade() -> Result, &'static str> { - let mut state: Vec> = Vec::default(); - for_tuples!( #( state.push(Tuple::pre_upgrade()?); )* ); - Ok(state.encode()) + Ok(Vec::new()) } #[cfg(feature = "try-runtime")] + /// noop fn post_upgrade(state: Vec) -> Result<(), &'static str> { - let state: Vec> = Decode::decode(&mut state.as_slice()) - .expect("the state parameter should be the same as pre_upgrade generated"); - let mut state_iter = state.into_iter(); - for_tuples!( #( Tuple::post_upgrade( - state_iter.next().expect("the state parameter should be the same as pre_upgrade generated") - )?; )* ); Ok(()) } } From efc3d29c399e6d1529eda0bc9757a39ff0bff03e Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 21 Sep 2022 14:13:33 +0200 Subject: [PATCH 02/23] remove unused --- frame/support/src/traits/hooks.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index e2d81f2ab400f..4362f54f194b2 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -22,9 +22,6 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; -#[cfg(feature = "try-runtime")] -use codec::Decode; - /// The block initialization trait. /// /// Implementing this lets you express what should happen for your pallet when the block is From 5de382e89702b12de17febeab5a1c11651808ca2 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 21 Sep 2022 14:19:26 +0200 Subject: [PATCH 03/23] guards --- frame/executive/src/lib.rs | 8 -------- frame/support/src/traits/hooks.rs | 2 ++ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 01b71aab745d5..a41c82da5757c 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -287,15 +287,7 @@ where /// /// This should only be used for testing. pub fn try_runtime_upgrade() -> Result { - // ensure both `pre_upgrade` and `post_upgrade` won't change the storage root - let state = { - let _guard = frame_support::StorageNoopGuard::default(); - <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap() - }; let weight = Self::execute_on_runtime_upgrade(); - let _guard = frame_support::StorageNoopGuard::default(); - <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade(state) - .unwrap(); Ok(weight) } } diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 4362f54f194b2..7c21623099e12 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -177,8 +177,10 @@ impl OnRuntimeUpgrade for Tuple { let mut state: Vec = Vec::default(); let mut weight = Weight::zero(); for_tuples!( #( + let _guard = frame_support::StorageNoopGuard::default(); state = Tuple::pre_upgrade().unwrap(); weight = weight.saturating_add(Tuple::on_runtime_upgrade()); + let _guard = frame_support::StorageNoopGuard::default(); Tuple::post_upgrade(state).unwrap(); )* ); weight From 3f3fdda7b2e82d42f130b6eb92472857658718ff Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 21 Sep 2022 15:08:52 +0200 Subject: [PATCH 04/23] reinstate encode/decode --- frame/support/src/traits/hooks.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 7c21623099e12..50cfbba1e10dc 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -22,6 +22,9 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; +#[cfg(feature = "try-runtime")] +use codec::{Decode, Encode}; + /// The block initialization trait. /// /// Implementing this lets you express what should happen for your pallet when the block is From 2c4b85691cb4ce7076a7f9f6b7aedf0c8ffa114b Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 21 Sep 2022 16:27:54 +0200 Subject: [PATCH 05/23] proper feature gate --- frame/support/src/traits/hooks.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 50cfbba1e10dc..d835f43d75984 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -22,6 +22,7 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; +#[cfg(feature = "test")] #[cfg(feature = "try-runtime")] use codec::{Decode, Encode}; From 4c455a8b9c85fc133dffb06ffb6df48ce247e648 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 21 Sep 2022 16:37:53 +0200 Subject: [PATCH 06/23] proper test feature gate --- frame/support/src/traits/hooks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index d835f43d75984..8e5d11f3cc9c4 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -22,7 +22,7 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; -#[cfg(feature = "test")] +#[cfg(test)] #[cfg(feature = "try-runtime")] use codec::{Decode, Encode}; From 7522aacfbc0ff21bfe0bf89cdfd6c8ad22551b89 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 21 Sep 2022 16:48:57 +0200 Subject: [PATCH 07/23] Update frame/support/src/traits/hooks.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/support/src/traits/hooks.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 8e5d11f3cc9c4..9eac174e913a4 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -183,6 +183,7 @@ impl OnRuntimeUpgrade for Tuple { for_tuples!( #( let _guard = frame_support::StorageNoopGuard::default(); state = Tuple::pre_upgrade().unwrap(); + drop(_guard); weight = weight.saturating_add(Tuple::on_runtime_upgrade()); let _guard = frame_support::StorageNoopGuard::default(); Tuple::post_upgrade(state).unwrap(); From 2e1323697cbd3cfe54059eeefe5082776fb40b02 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Wed, 21 Sep 2022 16:49:05 +0200 Subject: [PATCH 08/23] Update frame/support/src/traits/hooks.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/support/src/traits/hooks.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 9eac174e913a4..926e403900216 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -187,6 +187,7 @@ impl OnRuntimeUpgrade for Tuple { weight = weight.saturating_add(Tuple::on_runtime_upgrade()); let _guard = frame_support::StorageNoopGuard::default(); Tuple::post_upgrade(state).unwrap(); + drop(_guard); )* ); weight } From 551f04a134649403bdd55c7ec304373b055ca11d Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 22 Sep 2022 09:57:07 +0200 Subject: [PATCH 09/23] fix tests --- frame/support/src/traits/hooks.rs | 61 ++++++++++++------------------- 1 file changed, 24 insertions(+), 37 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 926e403900216..a9d27eea52869 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -436,12 +436,17 @@ mod tests { struct Test2; struct Test3; + let mut test1_assertions = 0; + let mut test2_assertions = 0; + let mut test3_assertions = 0; + impl OnRuntimeUpgrade for Test1 { fn pre_upgrade() -> Result, &'static str> { Ok("Test1".encode()) } fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: String = Decode::decode(&mut state.as_slice()).unwrap(); + test1_assertions += 1; assert_eq!(s, "Test1"); Ok(()) } @@ -452,6 +457,7 @@ mod tests { } fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: u32 = Decode::decode(&mut state.as_slice()).unwrap(); + test2_assertions += 1; assert_eq!(s, 100); Ok(()) } @@ -463,59 +469,40 @@ mod tests { fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: bool = Decode::decode(&mut state.as_slice()).unwrap(); assert_eq!(s, true); + test3_assertions += 1; Ok(()) } } type TestEmpty = (); let origin_state = ::pre_upgrade().unwrap(); - let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); - assert!(states.is_empty()); + assert!(origin_state.is_empty()); ::post_upgrade(origin_state).unwrap(); type Test1Tuple = (Test1,); let origin_state = ::pre_upgrade().unwrap(); - let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); - assert_eq!(states.len(), 1); - assert_eq!( - ::decode(&mut states[0].as_slice()).unwrap(), - "Test1".to_owned() - ); + assert!(origin_state.is_empty()); ::post_upgrade(origin_state).unwrap(); + assert_eq!(test1_assertions, 0); + ::on_runtime_upgrade(); + assert_eq!(test1_assertions, 1); type Test123 = (Test1, Test2, Test3); - let origin_state = ::pre_upgrade().unwrap(); - let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); - assert_eq!( - ::decode(&mut states[0].as_slice()).unwrap(), - "Test1".to_owned() - ); - assert_eq!(::decode(&mut states[1].as_slice()).unwrap(), 100u32); - assert_eq!(::decode(&mut states[2].as_slice()).unwrap(), true); - ::post_upgrade(origin_state).unwrap(); + ::on_runtime_upgrade(); + assert_eq!(test1_assertions, 2); + assert_eq!(test2_assertions, 1); + assert_eq!(test3_assertions, 1); type Test321 = (Test3, Test2, Test1); - let origin_state = ::pre_upgrade().unwrap(); - let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); - assert_eq!(::decode(&mut states[0].as_slice()).unwrap(), true); - assert_eq!(::decode(&mut states[1].as_slice()).unwrap(), 100u32); - assert_eq!( - ::decode(&mut states[2].as_slice()).unwrap(), - "Test1".to_owned() - ); - ::post_upgrade(origin_state).unwrap(); + ::on_runtime_upgrade(); + assert_eq!(test1_assertions, 3); + assert_eq!(test2_assertions, 2); + assert_eq!(test3_assertions, 2); type TestNested123 = (Test1, (Test2, Test3)); - let origin_state = ::pre_upgrade().unwrap(); - let states: Vec> = Decode::decode(&mut origin_state.as_slice()).unwrap(); - assert_eq!( - ::decode(&mut states[0].as_slice()).unwrap(), - "Test1".to_owned() - ); - // nested state for (Test2, Test3) - let nested_states: Vec> = Decode::decode(&mut states[1].as_slice()).unwrap(); - assert_eq!(::decode(&mut nested_states[0].as_slice()).unwrap(), 100u32); - assert_eq!(::decode(&mut nested_states[1].as_slice()).unwrap(), true); - ::post_upgrade(origin_state).unwrap(); + ::on_runtime_upgrade(); + assert_eq!(test1_assertions, 4); + assert_eq!(test2_assertions, 3); + assert_eq!(test3_assertions, 3); } } From 7bd6c1735385672a65e887a089cd1d799f8c1419 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 22 Sep 2022 10:57:33 +0200 Subject: [PATCH 10/23] redo Tuple tests --- frame/support/src/traits/hooks.rs | 85 +++++++++++++++++-------------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index a9d27eea52869..20ebd7c87b032 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -357,7 +357,9 @@ mod tests { #[test] fn on_initialize_and_on_runtime_upgrade_weight_merge_works() { + use sp_io::TestExternalities; struct Test; + impl OnInitialize for Test { fn on_initialize(_n: u8) -> Weight { Weight::from_ref_time(10) @@ -369,8 +371,10 @@ mod tests { } } - assert_eq!(<(Test, Test)>::on_initialize(0), Weight::from_ref_time(20)); - assert_eq!(<(Test, Test)>::on_runtime_upgrade(), Weight::from_ref_time(40)); + TestExternalities::default().execute_with(|| { + assert_eq!(<(Test, Test)>::on_initialize(0), Weight::from_ref_time(20)); + assert_eq!(<(Test, Test)>::on_runtime_upgrade(), Weight::from_ref_time(40)); + }); } #[test] @@ -432,13 +436,16 @@ mod tests { #[cfg(feature = "try-runtime")] #[test] fn on_runtime_upgrade_tuple() { + use sp_io::TestExternalities; + use sp_std::sync::atomic::{AtomicUsize, Ordering}; + struct Test1; struct Test2; struct Test3; - let mut test1_assertions = 0; - let mut test2_assertions = 0; - let mut test3_assertions = 0; + static TEST1_ASSERTIONS: AtomicUsize = AtomicUsize::new(0); + static TEST2_ASSERTIONS: AtomicUsize = AtomicUsize::new(0); + static TEST3_ASSERTIONS: AtomicUsize = AtomicUsize::new(0); impl OnRuntimeUpgrade for Test1 { fn pre_upgrade() -> Result, &'static str> { @@ -446,7 +453,7 @@ mod tests { } fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: String = Decode::decode(&mut state.as_slice()).unwrap(); - test1_assertions += 1; + TEST1_ASSERTIONS.fetch_add(1, Ordering::SeqCst); assert_eq!(s, "Test1"); Ok(()) } @@ -457,7 +464,7 @@ mod tests { } fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: u32 = Decode::decode(&mut state.as_slice()).unwrap(); - test2_assertions += 1; + TEST2_ASSERTIONS.fetch_add(1, Ordering::SeqCst); assert_eq!(s, 100); Ok(()) } @@ -468,41 +475,43 @@ mod tests { } fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: bool = Decode::decode(&mut state.as_slice()).unwrap(); + TEST3_ASSERTIONS.fetch_add(1, Ordering::SeqCst); assert_eq!(s, true); - test3_assertions += 1; Ok(()) } } - type TestEmpty = (); - let origin_state = ::pre_upgrade().unwrap(); - assert!(origin_state.is_empty()); - ::post_upgrade(origin_state).unwrap(); - - type Test1Tuple = (Test1,); - let origin_state = ::pre_upgrade().unwrap(); - assert!(origin_state.is_empty()); - ::post_upgrade(origin_state).unwrap(); - assert_eq!(test1_assertions, 0); - ::on_runtime_upgrade(); - assert_eq!(test1_assertions, 1); - - type Test123 = (Test1, Test2, Test3); - ::on_runtime_upgrade(); - assert_eq!(test1_assertions, 2); - assert_eq!(test2_assertions, 1); - assert_eq!(test3_assertions, 1); - - type Test321 = (Test3, Test2, Test1); - ::on_runtime_upgrade(); - assert_eq!(test1_assertions, 3); - assert_eq!(test2_assertions, 2); - assert_eq!(test3_assertions, 2); - - type TestNested123 = (Test1, (Test2, Test3)); - ::on_runtime_upgrade(); - assert_eq!(test1_assertions, 4); - assert_eq!(test2_assertions, 3); - assert_eq!(test3_assertions, 3); + TestExternalities::default().execute_with(|| { + type TestEmpty = (); + let origin_state = ::pre_upgrade().unwrap(); + assert!(origin_state.is_empty()); + ::post_upgrade(origin_state).unwrap(); + + type Test1Tuple = (Test1,); + let origin_state = ::pre_upgrade().unwrap(); + assert!(origin_state.is_empty()); + ::post_upgrade(origin_state).unwrap(); + assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 0); + ::on_runtime_upgrade(); + assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 1); + + type Test123 = (Test1, Test2, Test3); + ::on_runtime_upgrade(); + assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 2); + assert_eq!(TEST2_ASSERTIONS.load(Ordering::SeqCst), 1); + assert_eq!(TEST3_ASSERTIONS.load(Ordering::SeqCst), 1); + + type Test321 = (Test3, Test2, Test1); + ::on_runtime_upgrade(); + assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 3); + assert_eq!(TEST2_ASSERTIONS.load(Ordering::SeqCst), 2); + assert_eq!(TEST3_ASSERTIONS.load(Ordering::SeqCst), 2); + + type TestNested123 = (Test1, (Test2, Test3)); + ::on_runtime_upgrade(); + assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 4); + assert_eq!(TEST2_ASSERTIONS.load(Ordering::SeqCst), 3); + assert_eq!(TEST3_ASSERTIONS.load(Ordering::SeqCst), 3); + }); } } From 7676a47a88051a97cee59455923fb7a63468e690 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 22 Sep 2022 13:35:52 +0200 Subject: [PATCH 11/23] Update frame/support/src/traits/hooks.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/support/src/traits/hooks.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 20ebd7c87b032..2e0e846603ffe 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -178,14 +178,17 @@ impl OnRuntimeUpgrade for Tuple { /// consecutive migrations for the same pallet without errors. Therefore pre and post upgrade /// hooks for tuples are a noop. fn on_runtime_upgrade() -> Weight { - let mut state: Vec = Vec::default(); let mut weight = Weight::zero(); for_tuples!( #( let _guard = frame_support::StorageNoopGuard::default(); - state = Tuple::pre_upgrade().unwrap(); + // expected unwrap: we want to panic if any checks fail right here right now. + let state = Tuple::pre_upgrade().unwrap(); drop(_guard); + weight = weight.saturating_add(Tuple::on_runtime_upgrade()); + let _guard = frame_support::StorageNoopGuard::default(); + // expected unwrap: we want to panic if any checks fail right here right now. Tuple::post_upgrade(state).unwrap(); drop(_guard); )* ); From da7dd03202b2f9508385587e8cdd0f491256746b Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 22 Sep 2022 13:47:13 +0200 Subject: [PATCH 12/23] use parameter_types for testing --- frame/support/src/traits/hooks.rs | 38 ++++++++++++++++--------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 2e0e846603ffe..648f06a66b658 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -439,16 +439,18 @@ mod tests { #[cfg(feature = "try-runtime")] #[test] fn on_runtime_upgrade_tuple() { + use frame_support::parameter_types; use sp_io::TestExternalities; - use sp_std::sync::atomic::{AtomicUsize, Ordering}; struct Test1; struct Test2; struct Test3; - static TEST1_ASSERTIONS: AtomicUsize = AtomicUsize::new(0); - static TEST2_ASSERTIONS: AtomicUsize = AtomicUsize::new(0); - static TEST3_ASSERTIONS: AtomicUsize = AtomicUsize::new(0); + parameter_types! { + static Test1Assertions: u8 = 0; + static Test2Assertions: u8 = 0; + static Test3Assertions: u8 = 0; + } impl OnRuntimeUpgrade for Test1 { fn pre_upgrade() -> Result, &'static str> { @@ -456,7 +458,7 @@ mod tests { } fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: String = Decode::decode(&mut state.as_slice()).unwrap(); - TEST1_ASSERTIONS.fetch_add(1, Ordering::SeqCst); + Test1Assertions::mutate(|val| *val += 1); assert_eq!(s, "Test1"); Ok(()) } @@ -467,7 +469,7 @@ mod tests { } fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: u32 = Decode::decode(&mut state.as_slice()).unwrap(); - TEST2_ASSERTIONS.fetch_add(1, Ordering::SeqCst); + Test2Assertions::mutate(|val| *val += 1); assert_eq!(s, 100); Ok(()) } @@ -478,7 +480,7 @@ mod tests { } fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: bool = Decode::decode(&mut state.as_slice()).unwrap(); - TEST3_ASSERTIONS.fetch_add(1, Ordering::SeqCst); + Test3Assertions::mutate(|val| *val += 1); assert_eq!(s, true); Ok(()) } @@ -494,27 +496,27 @@ mod tests { let origin_state = ::pre_upgrade().unwrap(); assert!(origin_state.is_empty()); ::post_upgrade(origin_state).unwrap(); - assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 0); + assert_eq!(Test1Assertions::get(), 0); ::on_runtime_upgrade(); - assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 1); + assert_eq!(Test1Assertions::take(), 1); type Test123 = (Test1, Test2, Test3); ::on_runtime_upgrade(); - assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 2); - assert_eq!(TEST2_ASSERTIONS.load(Ordering::SeqCst), 1); - assert_eq!(TEST3_ASSERTIONS.load(Ordering::SeqCst), 1); + assert_eq!(Test1Assertions::take(), 1); + assert_eq!(Test2Assertions::take(), 1); + assert_eq!(Test3Assertions::take(), 1); type Test321 = (Test3, Test2, Test1); ::on_runtime_upgrade(); - assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 3); - assert_eq!(TEST2_ASSERTIONS.load(Ordering::SeqCst), 2); - assert_eq!(TEST3_ASSERTIONS.load(Ordering::SeqCst), 2); + assert_eq!(Test1Assertions::take(), 1); + assert_eq!(Test2Assertions::take(), 1); + assert_eq!(Test3Assertions::take(), 1); type TestNested123 = (Test1, (Test2, Test3)); ::on_runtime_upgrade(); - assert_eq!(TEST1_ASSERTIONS.load(Ordering::SeqCst), 4); - assert_eq!(TEST2_ASSERTIONS.load(Ordering::SeqCst), 3); - assert_eq!(TEST3_ASSERTIONS.load(Ordering::SeqCst), 3); + assert_eq!(Test1Assertions::take(), 1); + assert_eq!(Test2Assertions::take(), 1); + assert_eq!(Test3Assertions::take(), 1); }); } } From d2efbba8035b888e52b6a1c5057244d7f7ef4a73 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 22 Sep 2022 13:50:16 +0200 Subject: [PATCH 13/23] lint fix --- frame/support/src/traits/hooks.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 648f06a66b658..6af8a1b5e1d66 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -184,9 +184,9 @@ impl OnRuntimeUpgrade for Tuple { // expected unwrap: we want to panic if any checks fail right here right now. let state = Tuple::pre_upgrade().unwrap(); drop(_guard); - + weight = weight.saturating_add(Tuple::on_runtime_upgrade()); - + let _guard = frame_support::StorageNoopGuard::default(); // expected unwrap: we want to panic if any checks fail right here right now. Tuple::post_upgrade(state).unwrap(); From 626c5bf91927edb9ffef1a3b8ec58c69b3676445 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 22 Sep 2022 16:08:25 +0200 Subject: [PATCH 14/23] Update frame/support/src/traits/hooks.rs Co-authored-by: Jegor Sidorenko <5252494+jsidorenko@users.noreply.github.com> --- frame/support/src/traits/hooks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 6af8a1b5e1d66..50f8be8881ad9 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -174,7 +174,7 @@ impl OnRuntimeUpgrade for Tuple { } #[cfg(feature = "try-runtime")] - /// We are executing pre and post checks sequentially in order to be able to test seveal + /// We are executing pre- and post-checks sequentially in order to be able to test several /// consecutive migrations for the same pallet without errors. Therefore pre and post upgrade /// hooks for tuples are a noop. fn on_runtime_upgrade() -> Weight { From 20c0793c3e29759d39ea13e47b9327423de6548c Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 22 Sep 2022 16:09:49 +0200 Subject: [PATCH 15/23] Update frame/support/src/traits/hooks.rs Co-authored-by: Oliver Tale-Yazdi --- frame/support/src/traits/hooks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 50f8be8881ad9..e2021c109faea 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -203,7 +203,7 @@ impl OnRuntimeUpgrade for Tuple { #[cfg(feature = "try-runtime")] /// noop - fn post_upgrade(state: Vec) -> Result<(), &'static str> { + fn post_upgrade(_state: Vec) -> Result<(), &'static str> { Ok(()) } } From 3d2098a6df4f2ac1b7c0d64e30202535faa77cb1 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Thu, 22 Sep 2022 18:17:26 +0200 Subject: [PATCH 16/23] eloquent feature gate --- frame/support/src/traits/hooks.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index e2021c109faea..a5bfd3b0ac61d 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -22,8 +22,7 @@ use impl_trait_for_tuples::impl_for_tuples; use sp_runtime::traits::AtLeast32BitUnsigned; use sp_std::prelude::*; -#[cfg(test)] -#[cfg(feature = "try-runtime")] +#[cfg(all(feature = "try-runtime", test))] use codec::{Decode, Encode}; /// The block initialization trait. From d7102dc989fe99c381b11f9b9a544cdae15dd8d9 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 23 Sep 2022 09:53:36 +0200 Subject: [PATCH 17/23] redo tests --- frame/support/src/traits/hooks.rs | 38 +++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index a5bfd3b0ac61d..0616b40c5d4a2 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -435,6 +435,23 @@ mod tests { } } + #[test] + fn on_runtime_upgrade_tuple_no_try_runtime() { + struct Test1; + + impl OnRuntimeUpgrade for Test1 { + fn pre_upgrade() -> Result, &'static str> { + panic!("should not be called") + } + fn post_upgrade(state: Vec) -> Result<(), &'static str> { + panic!("should not be called") + } + } + + type Test1Tuple = (Test1,); + ::on_runtime_upgrade(); + } + #[cfg(feature = "try-runtime")] #[test] fn on_runtime_upgrade_tuple() { @@ -449,6 +466,8 @@ mod tests { static Test1Assertions: u8 = 0; static Test2Assertions: u8 = 0; static Test3Assertions: u8 = 0; + static EnableSequentialTest: bool = false; + static SequentialAssertions: u8 = 0; } impl OnRuntimeUpgrade for Test1 { @@ -458,10 +477,14 @@ mod tests { fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: String = Decode::decode(&mut state.as_slice()).unwrap(); Test1Assertions::mutate(|val| *val += 1); + if EnableSequentialTest::get() { + SequentialAssertions::mutate(|val| *val += 1); + } assert_eq!(s, "Test1"); Ok(()) } } + impl OnRuntimeUpgrade for Test2 { fn pre_upgrade() -> Result, &'static str> { Ok(100u32.encode()) @@ -469,10 +492,15 @@ mod tests { fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: u32 = Decode::decode(&mut state.as_slice()).unwrap(); Test2Assertions::mutate(|val| *val += 1); + if EnableSequentialTest::get() { + assert_eq!(SequentialAssertions::get(), 1); + SequentialAssertions::mutate(|val| *val += 1); + } assert_eq!(s, 100); Ok(()) } } + impl OnRuntimeUpgrade for Test3 { fn pre_upgrade() -> Result, &'static str> { Ok(true.encode()) @@ -480,6 +508,10 @@ mod tests { fn post_upgrade(state: Vec) -> Result<(), &'static str> { let s: bool = Decode::decode(&mut state.as_slice()).unwrap(); Test3Assertions::mutate(|val| *val += 1); + if EnableSequentialTest::get() { + assert_eq!(SequentialAssertions::get(), 2); + SequentialAssertions::mutate(|val| *val += 1); + } assert_eq!(s, true); Ok(()) } @@ -505,12 +537,18 @@ mod tests { assert_eq!(Test2Assertions::take(), 1); assert_eq!(Test3Assertions::take(), 1); + // enable sequential tests + EnableSequentialTest::mutate(|val| *val = true); + type Test321 = (Test3, Test2, Test1); ::on_runtime_upgrade(); assert_eq!(Test1Assertions::take(), 1); assert_eq!(Test2Assertions::take(), 1); assert_eq!(Test3Assertions::take(), 1); + // reset assertions + SequentialAssertions::take(); + type TestNested123 = (Test1, (Test2, Test3)); ::on_runtime_upgrade(); assert_eq!(Test1Assertions::take(), 1); From 1537c993032ee7832c6823332178b6629a6c9db6 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 23 Sep 2022 14:37:54 +0200 Subject: [PATCH 18/23] more fixes --- frame/support/src/traits/hooks.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 0616b40c5d4a2..1f8321cf32a84 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -180,15 +180,15 @@ impl OnRuntimeUpgrade for Tuple { let mut weight = Weight::zero(); for_tuples!( #( let _guard = frame_support::StorageNoopGuard::default(); - // expected unwrap: we want to panic if any checks fail right here right now. - let state = Tuple::pre_upgrade().unwrap(); + // we want to panic if any checks fail right here right now. + let state = Tuple::pre_upgrade().expect("PreUpgrade failed for migration #{}", Tuple::ID); drop(_guard); weight = weight.saturating_add(Tuple::on_runtime_upgrade()); let _guard = frame_support::StorageNoopGuard::default(); - // expected unwrap: we want to panic if any checks fail right here right now. - Tuple::post_upgrade(state).unwrap(); + // we want to panic if any checks fail right here right now. + Tuple::post_upgrade(state).expect("PostUpgrade failed for migration #{}", Tuple::ID); drop(_guard); )* ); weight @@ -454,6 +454,7 @@ mod tests { #[cfg(feature = "try-runtime")] #[test] + #[allow(dead_code)] fn on_runtime_upgrade_tuple() { use frame_support::parameter_types; use sp_io::TestExternalities; From 6eb790e87f34aafa53d707a745685e1d318d92bd Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 23 Sep 2022 15:04:49 +0200 Subject: [PATCH 19/23] properly handle pre/post errors --- frame/support/src/traits/hooks.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 1f8321cf32a84..3a99a2379f625 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -178,18 +178,20 @@ impl OnRuntimeUpgrade for Tuple { /// hooks for tuples are a noop. fn on_runtime_upgrade() -> Weight { let mut weight = Weight::zero(); + let mut i = 0; for_tuples!( #( let _guard = frame_support::StorageNoopGuard::default(); // we want to panic if any checks fail right here right now. - let state = Tuple::pre_upgrade().expect("PreUpgrade failed for migration #{}", Tuple::ID); + let state = Tuple::pre_upgrade().expect("PreUpgrade failed for migration #{}", i); drop(_guard); weight = weight.saturating_add(Tuple::on_runtime_upgrade()); let _guard = frame_support::StorageNoopGuard::default(); // we want to panic if any checks fail right here right now. - Tuple::post_upgrade(state).expect("PostUpgrade failed for migration #{}", Tuple::ID); + Tuple::post_upgrade(state).expect("PostUpgrade failed for migration #{}", i); drop(_guard); + i += 1; )* ); weight } From d7d6078208db5f498c8ded2ddbbb65de1c3c3268 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 23 Sep 2022 15:28:11 +0200 Subject: [PATCH 20/23] remove some tests and fix the others --- frame/support/src/traits/hooks.rs | 33 +++++++++---------------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 3a99a2379f625..028506ab4f95d 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -178,19 +178,21 @@ impl OnRuntimeUpgrade for Tuple { /// hooks for tuples are a noop. fn on_runtime_upgrade() -> Weight { let mut weight = Weight::zero(); - let mut i = 0; + // migration index in the tuple, start with 1 for better readability + let mut i = 1; for_tuples!( #( let _guard = frame_support::StorageNoopGuard::default(); // we want to panic if any checks fail right here right now. - let state = Tuple::pre_upgrade().expect("PreUpgrade failed for migration #{}", i); + let state = Tuple::pre_upgrade().expect(&format!("PreUpgrade failed for migration #{}", i)); drop(_guard); weight = weight.saturating_add(Tuple::on_runtime_upgrade()); let _guard = frame_support::StorageNoopGuard::default(); // we want to panic if any checks fail right here right now. - Tuple::post_upgrade(state).expect("PostUpgrade failed for migration #{}", i); + Tuple::post_upgrade(state).expect(&format!("PostUpgrade failed for migration #{}", i)); drop(_guard); + i += 1; )* ); weight @@ -437,23 +439,6 @@ mod tests { } } - #[test] - fn on_runtime_upgrade_tuple_no_try_runtime() { - struct Test1; - - impl OnRuntimeUpgrade for Test1 { - fn pre_upgrade() -> Result, &'static str> { - panic!("should not be called") - } - fn post_upgrade(state: Vec) -> Result<(), &'static str> { - panic!("should not be called") - } - } - - type Test1Tuple = (Test1,); - ::on_runtime_upgrade(); - } - #[cfg(feature = "try-runtime")] #[test] #[allow(dead_code)] @@ -534,8 +519,8 @@ mod tests { ::on_runtime_upgrade(); assert_eq!(Test1Assertions::take(), 1); - type Test123 = (Test1, Test2, Test3); - ::on_runtime_upgrade(); + type Test321 = (Test3, Test2, Test1); + ::on_runtime_upgrade(); assert_eq!(Test1Assertions::take(), 1); assert_eq!(Test2Assertions::take(), 1); assert_eq!(Test3Assertions::take(), 1); @@ -543,8 +528,8 @@ mod tests { // enable sequential tests EnableSequentialTest::mutate(|val| *val = true); - type Test321 = (Test3, Test2, Test1); - ::on_runtime_upgrade(); + type Test123 = (Test1, Test2, Test3); + ::on_runtime_upgrade(); assert_eq!(Test1Assertions::take(), 1); assert_eq!(Test2Assertions::take(), 1); assert_eq!(Test3Assertions::take(), 1); From 57d0f0b97c3c45cbb9015ccb6e0e80db93c9dc43 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 23 Sep 2022 15:45:33 +0200 Subject: [PATCH 21/23] add format import --- frame/support/src/traits/hooks.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index 028506ab4f95d..e343c14db691d 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -24,6 +24,7 @@ use sp_std::prelude::*; #[cfg(all(feature = "try-runtime", test))] use codec::{Decode, Encode}; +use scale_info::prelude::format; /// The block initialization trait. /// From 11c2a6be9fc7694907c4f5419125a5d94be72ce7 Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 23 Sep 2022 15:52:10 +0200 Subject: [PATCH 22/23] import fix --- frame/support/src/traits/hooks.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index e343c14db691d..f4fc0f550f5dc 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -24,6 +24,8 @@ use sp_std::prelude::*; #[cfg(all(feature = "try-runtime", test))] use codec::{Decode, Encode}; + +#[cfg(feature = "try-runtime")] use scale_info::prelude::format; /// The block initialization trait. From 759b8516ad23abc2ece3acefc0ffa1d70cc44bbe Mon Sep 17 00:00:00 2001 From: Roman Useinov Date: Fri, 23 Sep 2022 15:56:58 +0200 Subject: [PATCH 23/23] more import fixes --- frame/support/src/traits/hooks.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frame/support/src/traits/hooks.rs b/frame/support/src/traits/hooks.rs index f4fc0f550f5dc..3415682c0b382 100644 --- a/frame/support/src/traits/hooks.rs +++ b/frame/support/src/traits/hooks.rs @@ -25,9 +25,6 @@ use sp_std::prelude::*; #[cfg(all(feature = "try-runtime", test))] use codec::{Decode, Encode}; -#[cfg(feature = "try-runtime")] -use scale_info::prelude::format; - /// The block initialization trait. /// /// Implementing this lets you express what should happen for your pallet when the block is @@ -180,6 +177,8 @@ impl OnRuntimeUpgrade for Tuple { /// consecutive migrations for the same pallet without errors. Therefore pre and post upgrade /// hooks for tuples are a noop. fn on_runtime_upgrade() -> Weight { + use scale_info::prelude::format; + let mut weight = Weight::zero(); // migration index in the tuple, start with 1 for better readability let mut i = 1;