diff --git a/substrate/frame/contracts/src/benchmarking/mod.rs b/substrate/frame/contracts/src/benchmarking/mod.rs index b98c05f250..048465c1b7 100644 --- a/substrate/frame/contracts/src/benchmarking/mod.rs +++ b/substrate/frame/contracts/src/benchmarking/mod.rs @@ -281,7 +281,7 @@ benchmarks! { #[pov_mode = Measured] migrate { StorageVersion::new(0).put::>(); - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade(); let origin: RawOrigin<::AccountId> = RawOrigin::Signed(whitelisted_caller()); }: { >::migrate(origin.into(), Weight::MAX).unwrap() @@ -294,7 +294,7 @@ benchmarks! { on_runtime_upgrade_noop { assert_eq!(StorageVersion::get::>(), 2); }: { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { assert!(MigrationInProgress::::get().is_none()); } @@ -306,7 +306,7 @@ benchmarks! { let v = vec![42u8].try_into().ok(); MigrationInProgress::::set(v.clone()); }: { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { assert!(MigrationInProgress::::get().is_some()); assert_eq!(MigrationInProgress::::get(), v); @@ -317,7 +317,7 @@ benchmarks! { on_runtime_upgrade { StorageVersion::new(0).put::>(); }: { - as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() + as frame_support::traits::OnRuntimeUpgrade>::on_runtime_upgrade() } verify { assert!(MigrationInProgress::::get().is_some()); } diff --git a/substrate/frame/contracts/src/migration.rs b/substrate/frame/contracts/src/migration.rs index ad85da0477..41fc9753a0 100644 --- a/substrate/frame/contracts/src/migration.rs +++ b/substrate/frame/contracts/src/migration.rs @@ -191,10 +191,14 @@ pub trait MigrateSequence: private::Sealed { } /// Performs all necessary migrations based on `StorageVersion`. -pub struct Migration(PhantomData); +/// +/// If `TEST_ALL_STEPS == true` and `try-runtime` is enabled, this will run all the migrations +/// inside `on_runtime_upgrade`. This should be set to false in tests that want to ensure the step +/// by step migration works. +pub struct Migration(PhantomData); #[cfg(feature = "try-runtime")] -impl Migration { +impl Migration { fn run_all_steps() -> Result<(), TryRuntimeError> { let mut weight = Weight::zero(); let name = >::name(); @@ -221,7 +225,7 @@ impl Migration { } } -impl OnRuntimeUpgrade for Migration { +impl OnRuntimeUpgrade for Migration { fn on_runtime_upgrade() -> Weight { let name = >::name(); let latest_version = >::current_storage_version(); @@ -244,6 +248,7 @@ impl OnRuntimeUpgrade for Migration { "{name}: Migration already in progress {:?}", &storage_version ); + return T::WeightInfo::on_runtime_upgrade_in_progress() } @@ -256,9 +261,11 @@ impl OnRuntimeUpgrade for Migration { MigrationInProgress::::set(Some(cursor)); #[cfg(feature = "try-runtime")] - Self::run_all_steps().unwrap(); + if TEST_ALL_STEPS { + Self::run_all_steps().unwrap(); + } - return T::WeightInfo::on_runtime_upgrade() + T::WeightInfo::on_runtime_upgrade() } #[cfg(feature = "try-runtime")] @@ -305,7 +312,7 @@ pub enum StepResult { Completed { steps_done: u32 }, } -impl Migration { +impl Migration { /// Verify that each migration's step of the [`T::Migrations`] sequence fits into `Cursor`. pub(crate) fn integrity_test() { let max_weight = ::BlockWeights::get().max_block; @@ -460,7 +467,7 @@ impl MigrateSequence for Tuple { return StepResult::Completed{ steps_done } } } - return StepResult::InProgress{cursor: migration.encode().try_into().expect(PROOF_ENCODE), steps_done } + return StepResult::InProgress{cursor: migration.encode().try_into().expect(PROOF_ENCODE), steps_done } } )* ); @@ -569,7 +576,7 @@ mod test { #[test] fn migration_works() { - type TestMigration = Migration; + type TestMigration = Migration; ExtBuilder::default().set_storage_version(0).build().execute_with(|| { assert_eq!(StorageVersion::get::>(), 0); diff --git a/substrate/frame/democracy/src/migrations/unlock_and_unreserve_all_funds.rs b/substrate/frame/democracy/src/migrations/unlock_and_unreserve_all_funds.rs index e23a7c4f5f..1f46070d1d 100644 --- a/substrate/frame/democracy/src/migrations/unlock_and_unreserve_all_funds.rs +++ b/substrate/frame/democracy/src/migrations/unlock_and_unreserve_all_funds.rs @@ -150,8 +150,17 @@ where log::info!(target: LOG_TARGET, "Total accounts: {:?}", all_accounts.len()); log::info!(target: LOG_TARGET, "Total stake to unlock: {:?}", total_stake_to_unlock); - log::info!(target: LOG_TARGET, "Total deposit to unreserve: {:?}", total_deposits_to_unreserve); - log::info!(target: LOG_TARGET, "Bugged deposits: {}/{}", bugged_deposits, account_deposits.len()); + log::info!( + target: LOG_TARGET, + "Total deposit to unreserve: {:?}", + total_deposits_to_unreserve + ); + log::info!( + target: LOG_TARGET, + "Bugged deposits: {}/{}", + bugged_deposits, + account_deposits.len() + ); Ok(account_reserved_before.encode()) } diff --git a/substrate/frame/elections-phragmen/src/migrations/unlock_and_unreserve_all_funds.rs b/substrate/frame/elections-phragmen/src/migrations/unlock_and_unreserve_all_funds.rs index 0115e4148d..f566f84e51 100644 --- a/substrate/frame/elections-phragmen/src/migrations/unlock_and_unreserve_all_funds.rs +++ b/substrate/frame/elections-phragmen/src/migrations/unlock_and_unreserve_all_funds.rs @@ -165,11 +165,20 @@ where let total_stake_to_unlock = account_staked_sums.clone().into_values().sum::>(); let total_deposits_to_unreserve = account_deposited_sums.clone().into_values().sum::>(); - log::info!(target: LOG_TARGET, "Total accounts: {:?}", all_accounts.len()); + log::info!(target: LOG_TARGET, "Total accounts: {:?}", all_accounts.len()); log::info!(target: LOG_TARGET, "Total stake to unlock: {:?}", total_stake_to_unlock); - log::info!(target: LOG_TARGET, "Total deposit to unreserve: {:?}", total_deposits_to_unreserve); + log::info!( + target: LOG_TARGET, + "Total deposit to unreserve: {:?}", + total_deposits_to_unreserve + ); if bugged_deposits > 0 { - log::warn!(target: LOG_TARGET, "Bugged deposits: {}/{}", bugged_deposits, all_accounts.len()); + log::warn!( + target: LOG_TARGET, + "Bugged deposits: {}/{}", + bugged_deposits, + all_accounts.len() + ); } Ok(account_reserved_before.encode()) diff --git a/substrate/frame/im-online/src/migration.rs b/substrate/frame/im-online/src/migration.rs index 33a9bd1479..2f567ce778 100644 --- a/substrate/frame/im-online/src/migration.rs +++ b/substrate/frame/im-online/src/migration.rs @@ -33,7 +33,7 @@ mod v0 { use super::*; use frame_support::traits::WrapperOpaque; - #[derive(Encode, Decode)] + #[derive(Encode, Decode, Default)] pub(super) struct BoundedOpaqueNetworkState { /// PeerId of the local node in SCALE encoded. pub peer_id: Vec, @@ -118,8 +118,7 @@ pub mod v1 { } } -#[cfg(test)] -#[cfg(feature = "try-runtime")] +#[cfg(all(feature = "try-runtime", test))] mod test { use super::*; use crate::mock::{new_test_ext, Runtime as T}; diff --git a/substrate/frame/scheduler/src/migration.rs b/substrate/frame/scheduler/src/migration.rs index 7f23d96206..78313fda66 100644 --- a/substrate/frame/scheduler/src/migration.rs +++ b/substrate/frame/scheduler/src/migration.rs @@ -499,7 +499,7 @@ mod test { // The pre_upgrade hook fails: let err = v3::MigrateToV4::::pre_upgrade().unwrap_err(); - assert!(err == "Call is too large".into()); + assert_eq!(DispatchError::from("Call is too large."), err); // But the migration itself works: let _w = v3::MigrateToV4::::on_runtime_upgrade(); diff --git a/substrate/scripts/ci/gitlab/pipeline/test.yml b/substrate/scripts/ci/gitlab/pipeline/test.yml index 533acd59e0..2e5495cb82 100644 --- a/substrate/scripts/ci/gitlab/pipeline/test.yml +++ b/substrate/scripts/ci/gitlab/pipeline/test.yml @@ -221,7 +221,7 @@ test-linux-stable: --locked --release --verbose - --features runtime-benchmarks + --features runtime-benchmarks,try-runtime --manifest-path ./bin/node/cli/Cargo.toml --partition count:${CI_NODE_INDEX}/${CI_NODE_TOTAL} # we need to update cache only from one job