From 518f7dfb0f23508147ea56ea9fa0b0d8b9eda245 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Mon, 15 May 2023 21:19:16 +0200 Subject: [PATCH 01/12] move migration stuffs --- xcm/pallet-xcm/src/lib.rs | 6 ------ xcm/pallet-xcm/src/migration.rs | 14 ++++++++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/xcm/pallet-xcm/src/lib.rs b/xcm/pallet-xcm/src/lib.rs index 16766b7ab90f..a143301400e2 100644 --- a/xcm/pallet-xcm/src/lib.rs +++ b/xcm/pallet-xcm/src/lib.rs @@ -697,12 +697,6 @@ pub mod pallet { } weight_used } - fn on_runtime_upgrade() -> Weight { - // Start a migration (this happens before on_initialize so it'll happen later in this - // block, which should be good enough)... - CurrentMigration::::put(VersionMigrationStage::default()); - T::DbWeight::get().writes(1) - } } pub mod migrations { diff --git a/xcm/pallet-xcm/src/migration.rs b/xcm/pallet-xcm/src/migration.rs index 32bc5e764c57..fe6b4e3118f4 100644 --- a/xcm/pallet-xcm/src/migration.rs +++ b/xcm/pallet-xcm/src/migration.rs @@ -26,6 +26,8 @@ pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); const DEFAULT_PROOF_SIZE: u64 = 64 * 1024; pub mod v1 { + use crate::{CurrentMigration, VersionMigrationStage}; + use super::*; pub struct MigrateToV1(sp_std::marker::PhantomData); @@ -38,8 +40,11 @@ pub mod v1 { } fn on_runtime_upgrade() -> Weight { + CurrentMigration::::put(VersionMigrationStage::default()); + let mut weight = T::DbWeight::get().writes(1); + if StorageVersion::get::>() == 0 { - let mut weight = T::DbWeight::get().reads(1); + weight.saturating_accrue(T::DbWeight::get().reads(1)); let translate = |pre: (u64, u64, u32)| -> Option<(u64, Weight, u32)> { weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1)); @@ -52,12 +57,13 @@ pub mod v1 { log::info!("v1 applied successfully"); STORAGE_VERSION.put::>(); - - weight.saturating_add(T::DbWeight::get().writes(1)) + weight.saturating_add(T::DbWeight::get().writes(1)); } else { log::warn!("skipping v1, should be removed"); - T::DbWeight::get().reads(1) + weight.saturating_accrue(T::DbWeight::get().reads(1)); } + + weight } } } From 35c0800b56fde27038a1347900dfca6f6e587b19 Mon Sep 17 00:00:00 2001 From: PG Herveou Date: Wed, 17 May 2023 14:27:56 +0200 Subject: [PATCH 02/12] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- xcm/pallet-xcm/src/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/pallet-xcm/src/migration.rs b/xcm/pallet-xcm/src/migration.rs index fe6b4e3118f4..9f81fab4d557 100644 --- a/xcm/pallet-xcm/src/migration.rs +++ b/xcm/pallet-xcm/src/migration.rs @@ -56,7 +56,7 @@ pub mod v1 { VersionNotifyTargets::::translate_values(translate); log::info!("v1 applied successfully"); - STORAGE_VERSION.put::>(); + StorageVersion::new(1).put::>(); weight.saturating_add(T::DbWeight::get().writes(1)); } else { log::warn!("skipping v1, should be removed"); From bc41403484fcc6a8d93aa496477ca62a00fe4a54 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Mon, 5 Jun 2023 15:18:04 +0200 Subject: [PATCH 03/12] Fix test --- xcm/pallet-xcm/src/tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 2ad13dced936..4b66e5954ccb 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -1166,7 +1166,10 @@ fn subscription_side_upgrades_work_with_multistage_notify() { AdvertisedXcmVersion::set(3); // A runtime upgrade which alters the version does send notifications. - XcmPallet::on_runtime_upgrade(); + use crate::migration::v1::MigrateToV1; + use frame_support::traits::OnRuntimeUpgrade; + MigrateToV1::::on_runtime_upgrade(); + let mut maybe_migration = CurrentMigration::::take(); let mut counter = 0; while let Some(migration) = maybe_migration.take() { From 011f0176aeccbfd74a69f047e612a34bee32f37f Mon Sep 17 00:00:00 2001 From: pgherveou Date: Mon, 5 Jun 2023 15:43:34 +0200 Subject: [PATCH 04/12] fix --- xcm/pallet-xcm/src/tests.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 4b66e5954ccb..212f47407d28 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -31,6 +31,8 @@ use xcm_executor::{ traits::{Properties, QueryHandler, QueryResponseStatus, ShouldExecute}, XcmExecutor, }; +use crate::migration::v1::MigrateToV1; +use frame_support::traits::OnRuntimeUpgrade; const ALICE: AccountId = AccountId::new([0u8; 32]); const BOB: AccountId = AccountId::new([1u8; 32]); @@ -897,7 +899,7 @@ fn subscription_side_works() { assert_eq!(take_sent_xcm(), vec![(remote.clone(), Xcm(vec![instr]))]); // A runtime upgrade which doesn't alter the version sends no notifications. - XcmPallet::on_runtime_upgrade(); + MigrateToV1::::on_runtime_upgrade(); XcmPallet::on_initialize(1); assert_eq!(take_sent_xcm(), vec![]); @@ -905,7 +907,7 @@ fn subscription_side_works() { AdvertisedXcmVersion::set(2); // A runtime upgrade which alters the version does send notifications. - XcmPallet::on_runtime_upgrade(); + MigrateToV1::::on_runtime_upgrade(); XcmPallet::on_initialize(2); let instr = QueryResponse { query_id: 0, @@ -932,7 +934,7 @@ fn subscription_side_upgrades_work_with_notify() { AdvertisedXcmVersion::set(3); // A runtime upgrade which alters the version does send notifications. - XcmPallet::on_runtime_upgrade(); + MigrateToV1::::on_runtime_upgrade(); XcmPallet::on_initialize(1); let instr1 = QueryResponse { @@ -982,7 +984,7 @@ fn subscription_side_upgrades_work_without_notify() { VersionNotifyTargets::::insert(3, v3_location, (72, Weight::zero(), 2)); // A runtime upgrade which alters the version does send notifications. - XcmPallet::on_runtime_upgrade(); + MigrateToV1::::on_runtime_upgrade(); XcmPallet::on_initialize(1); let mut contents = VersionNotifyTargets::::iter().collect::>(); @@ -1166,8 +1168,6 @@ fn subscription_side_upgrades_work_with_multistage_notify() { AdvertisedXcmVersion::set(3); // A runtime upgrade which alters the version does send notifications. - use crate::migration::v1::MigrateToV1; - use frame_support::traits::OnRuntimeUpgrade; MigrateToV1::::on_runtime_upgrade(); let mut maybe_migration = CurrentMigration::::take(); From a8772b8748dab5cb7906046b0452135383dde480 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Mon, 5 Jun 2023 15:44:27 +0200 Subject: [PATCH 05/12] lint --- xcm/pallet-xcm/src/tests.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 212f47407d28..debac7038b5c 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -15,12 +15,13 @@ // along with Polkadot. If not, see . use crate::{ - mock::*, AssetTraps, CurrentMigration, Error, LatestVersionedMultiLocation, Queries, - QueryStatus, VersionDiscoveryQueue, VersionNotifiers, VersionNotifyTargets, + migration::v1::MigrateToV1, mock::*, AssetTraps, CurrentMigration, Error, + LatestVersionedMultiLocation, Queries, QueryStatus, VersionDiscoveryQueue, VersionNotifiers, + VersionNotifyTargets, }; use frame_support::{ assert_noop, assert_ok, - traits::{Currency, Hooks}, + traits::{Currency, Hooks, OnRuntimeUpgrade}, weights::Weight, }; use polkadot_parachain::primitives::Id as ParaId; @@ -31,8 +32,6 @@ use xcm_executor::{ traits::{Properties, QueryHandler, QueryResponseStatus, ShouldExecute}, XcmExecutor, }; -use crate::migration::v1::MigrateToV1; -use frame_support::traits::OnRuntimeUpgrade; const ALICE: AccountId = AccountId::new([0u8; 32]); const BOB: AccountId = AccountId::new([1u8; 32]); From f07d01a0797c7e939b39e25b45caac22a73f02c9 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Mon, 5 Jun 2023 15:45:24 +0200 Subject: [PATCH 06/12] fix lint --- xcm/pallet-xcm/src/migration.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/xcm/pallet-xcm/src/migration.rs b/xcm/pallet-xcm/src/migration.rs index d8d203b31bc9..616cf6f5a9ef 100644 --- a/xcm/pallet-xcm/src/migration.rs +++ b/xcm/pallet-xcm/src/migration.rs @@ -25,7 +25,6 @@ const DEFAULT_PROOF_SIZE: u64 = 64 * 1024; pub mod v1 { use crate::{CurrentMigration, VersionMigrationStage}; - use super::*; pub struct MigrateToV1(sp_std::marker::PhantomData); From fa9ad9a35f41e37a48babeddf3fe2d28c9c49796 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Mon, 5 Jun 2023 15:46:52 +0200 Subject: [PATCH 07/12] rm extra space --- xcm/pallet-xcm/src/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index debac7038b5c..4827f7393809 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -1168,7 +1168,6 @@ fn subscription_side_upgrades_work_with_multistage_notify() { // A runtime upgrade which alters the version does send notifications. MigrateToV1::::on_runtime_upgrade(); - let mut maybe_migration = CurrentMigration::::take(); let mut counter = 0; while let Some(migration) = maybe_migration.take() { From 6fad87bf21129e3be296be9b90d582dd689346ca Mon Sep 17 00:00:00 2001 From: pgherveou Date: Mon, 5 Jun 2023 16:51:31 +0200 Subject: [PATCH 08/12] fix lint --- xcm/pallet-xcm/src/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/pallet-xcm/src/migration.rs b/xcm/pallet-xcm/src/migration.rs index 616cf6f5a9ef..a8b7f3aafe25 100644 --- a/xcm/pallet-xcm/src/migration.rs +++ b/xcm/pallet-xcm/src/migration.rs @@ -24,8 +24,8 @@ use frame_support::{ const DEFAULT_PROOF_SIZE: u64 = 64 * 1024; pub mod v1 { - use crate::{CurrentMigration, VersionMigrationStage}; use super::*; + use crate::{CurrentMigration, VersionMigrationStage}; pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { From f279726ed3b5f9880ab1f22ab8ab67dcca6bc6e8 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 4 Aug 2023 16:11:45 +0200 Subject: [PATCH 09/12] PR review --- xcm/pallet-xcm/src/migration.rs | 40 +++++++++++++-------------------- 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/xcm/pallet-xcm/src/migration.rs b/xcm/pallet-xcm/src/migration.rs index 4cbe6c063fd5..bdfd47c8d911 100644 --- a/xcm/pallet-xcm/src/migration.rs +++ b/xcm/pallet-xcm/src/migration.rs @@ -34,37 +34,27 @@ pub mod v1 { /// Use experimental [`VersionCheckedMigrateToV1`] instead. pub struct VersionUncheckedMigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for VersionUncheckedMigrateToV1 { - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { - ensure!(StorageVersion::get::>() == 0, "must upgrade linearly"); - - Ok(sp_std::vec::Vec::new()) - } - fn on_runtime_upgrade() -> Weight { CurrentMigration::::put(VersionMigrationStage::default()); - let mut weight = T::DbWeight::get().writes(1); - - if StorageVersion::get::>() == 0 { - weight.saturating_accrue(T::DbWeight::get().reads(1)); + let mut weight = T::DbWeight::get().reads(1); - let translate = |pre: (u64, u64, u32)| -> Option<(u64, Weight, u32)> { - weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1)); - let translated = (pre.0, Weight::from_parts(pre.1, DEFAULT_PROOF_SIZE), pre.2); - log::info!("Migrated VersionNotifyTarget {:?} to {:?}", pre, translated); - Some(translated) - }; - - VersionNotifyTargets::::translate_values(translate); - - log::info!("v1 applied successfully"); - StorageVersion::new(1).put::>(); - weight.saturating_add(T::DbWeight::get().writes(1)); - } else { + if StorageVersion::get::>() != 0 { log::warn!("skipping v1, should be removed"); - weight.saturating_accrue(T::DbWeight::get().reads(1)); + return weight } + let translate = |pre: (u64, u64, u32)| -> Option<(u64, Weight, u32)> { + weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1)); + let translated = (pre.0, Weight::from_parts(pre.1, DEFAULT_PROOF_SIZE), pre.2); + log::info!("Migrated VersionNotifyTarget {:?} to {:?}", pre, translated); + Some(translated) + }; + + VersionNotifyTargets::::translate_values(translate); + + log::info!("v1 applied successfully"); + StorageVersion::new(1).put::>(); + weight.saturating_add(T::DbWeight::get().writes(1)); weight } } From bd6615bfa1ed25e7d70c956491e1bb8a9824a3bd Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 4 Aug 2023 16:14:28 +0200 Subject: [PATCH 10/12] fixes --- xcm/pallet-xcm/src/migration.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/xcm/pallet-xcm/src/migration.rs b/xcm/pallet-xcm/src/migration.rs index bdfd47c8d911..8431475c8be0 100644 --- a/xcm/pallet-xcm/src/migration.rs +++ b/xcm/pallet-xcm/src/migration.rs @@ -35,7 +35,6 @@ pub mod v1 { pub struct VersionUncheckedMigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for VersionUncheckedMigrateToV1 { fn on_runtime_upgrade() -> Weight { - CurrentMigration::::put(VersionMigrationStage::default()); let mut weight = T::DbWeight::get().reads(1); if StorageVersion::get::>() != 0 { @@ -43,6 +42,9 @@ pub mod v1 { return weight } + weight.saturating_accrue(T::DbWeight::get().writes(1)); + CurrentMigration::::put(VersionMigrationStage::default()); + let translate = |pre: (u64, u64, u32)| -> Option<(u64, Weight, u32)> { weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1)); let translated = (pre.0, Weight::from_parts(pre.1, DEFAULT_PROOF_SIZE), pre.2); @@ -53,8 +55,8 @@ pub mod v1 { VersionNotifyTargets::::translate_values(translate); log::info!("v1 applied successfully"); + weight.saturating_accrue(T::DbWeight::get().writes(1)); StorageVersion::new(1).put::>(); - weight.saturating_add(T::DbWeight::get().writes(1)); weight } } From ed26d11fb8a91da15c00eae6f79547fb882e7006 Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 4 Aug 2023 16:15:29 +0200 Subject: [PATCH 11/12] use saturating_accrue in fn --- xcm/pallet-xcm/src/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/pallet-xcm/src/migration.rs b/xcm/pallet-xcm/src/migration.rs index 8431475c8be0..08809f0d2f2e 100644 --- a/xcm/pallet-xcm/src/migration.rs +++ b/xcm/pallet-xcm/src/migration.rs @@ -46,7 +46,7 @@ pub mod v1 { CurrentMigration::::put(VersionMigrationStage::default()); let translate = |pre: (u64, u64, u32)| -> Option<(u64, Weight, u32)> { - weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1)); + weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); let translated = (pre.0, Weight::from_parts(pre.1, DEFAULT_PROOF_SIZE), pre.2); log::info!("Migrated VersionNotifyTarget {:?} to {:?}", pre, translated); Some(translated) From 6f58baf9c4a4f2c76c74301a521da013451691cc Mon Sep 17 00:00:00 2001 From: pgherveou Date: Fri, 4 Aug 2023 20:51:38 +0200 Subject: [PATCH 12/12] fix test --- xcm/pallet-xcm/src/tests.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 4827f7393809..f42eb987876a 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -15,13 +15,13 @@ // along with Polkadot. If not, see . use crate::{ - migration::v1::MigrateToV1, mock::*, AssetTraps, CurrentMigration, Error, - LatestVersionedMultiLocation, Queries, QueryStatus, VersionDiscoveryQueue, VersionNotifiers, + mock::*, AssetTraps, CurrentMigration, Error, LatestVersionedMultiLocation, Queries, + QueryStatus, VersionDiscoveryQueue, VersionMigrationStage, VersionNotifiers, VersionNotifyTargets, }; use frame_support::{ assert_noop, assert_ok, - traits::{Currency, Hooks, OnRuntimeUpgrade}, + traits::{Currency, Hooks}, weights::Weight, }; use polkadot_parachain::primitives::Id as ParaId; @@ -898,7 +898,7 @@ fn subscription_side_works() { assert_eq!(take_sent_xcm(), vec![(remote.clone(), Xcm(vec![instr]))]); // A runtime upgrade which doesn't alter the version sends no notifications. - MigrateToV1::::on_runtime_upgrade(); + CurrentMigration::::put(VersionMigrationStage::default()); XcmPallet::on_initialize(1); assert_eq!(take_sent_xcm(), vec![]); @@ -906,7 +906,7 @@ fn subscription_side_works() { AdvertisedXcmVersion::set(2); // A runtime upgrade which alters the version does send notifications. - MigrateToV1::::on_runtime_upgrade(); + CurrentMigration::::put(VersionMigrationStage::default()); XcmPallet::on_initialize(2); let instr = QueryResponse { query_id: 0, @@ -933,7 +933,7 @@ fn subscription_side_upgrades_work_with_notify() { AdvertisedXcmVersion::set(3); // A runtime upgrade which alters the version does send notifications. - MigrateToV1::::on_runtime_upgrade(); + CurrentMigration::::put(VersionMigrationStage::default()); XcmPallet::on_initialize(1); let instr1 = QueryResponse { @@ -983,7 +983,7 @@ fn subscription_side_upgrades_work_without_notify() { VersionNotifyTargets::::insert(3, v3_location, (72, Weight::zero(), 2)); // A runtime upgrade which alters the version does send notifications. - MigrateToV1::::on_runtime_upgrade(); + CurrentMigration::::put(VersionMigrationStage::default()); XcmPallet::on_initialize(1); let mut contents = VersionNotifyTargets::::iter().collect::>(); @@ -1167,7 +1167,7 @@ fn subscription_side_upgrades_work_with_multistage_notify() { AdvertisedXcmVersion::set(3); // A runtime upgrade which alters the version does send notifications. - MigrateToV1::::on_runtime_upgrade(); + CurrentMigration::::put(VersionMigrationStage::default()); let mut maybe_migration = CurrentMigration::::take(); let mut counter = 0; while let Some(migration) = maybe_migration.take() {