From f2dd873d008434b02fe4d2f769fc6cca83393f35 Mon Sep 17 00:00:00 2001 From: Raphael Date: Tue, 25 Mar 2025 11:09:54 +0100 Subject: [PATCH 1/5] chore: storage migration for bonded coins pallet --- pallets/pallet-bonded-coins/src/lib.rs | 2 + .../pallet-bonded-coins/src/migrations/mod.rs | 1 + .../pallet-bonded-coins/src/migrations/v1.rs | 165 ++++++++++++++++++ 3 files changed, 168 insertions(+) create mode 100644 pallets/pallet-bonded-coins/src/migrations/mod.rs create mode 100644 pallets/pallet-bonded-coins/src/migrations/v1.rs diff --git a/pallets/pallet-bonded-coins/src/lib.rs b/pallets/pallet-bonded-coins/src/lib.rs index 658fcc8fc..8512b50e2 100644 --- a/pallets/pallet-bonded-coins/src/lib.rs +++ b/pallets/pallet-bonded-coins/src/lib.rs @@ -20,6 +20,8 @@ pub use pallet::*; +pub mod migrations; + #[cfg(feature = "runtime-benchmarks")] mod benchmarking; #[cfg(any(test, feature = "runtime-benchmarks"))] diff --git a/pallets/pallet-bonded-coins/src/migrations/mod.rs b/pallets/pallet-bonded-coins/src/migrations/mod.rs new file mode 100644 index 000000000..fbb1de71a --- /dev/null +++ b/pallets/pallet-bonded-coins/src/migrations/mod.rs @@ -0,0 +1 @@ +mod v1; diff --git a/pallets/pallet-bonded-coins/src/migrations/v1.rs b/pallets/pallet-bonded-coins/src/migrations/v1.rs new file mode 100644 index 000000000..0d1754985 --- /dev/null +++ b/pallets/pallet-bonded-coins/src/migrations/v1.rs @@ -0,0 +1,165 @@ +use frame_support::{ + pallet_prelude::*, + storage_alias, + traits::{Get, OnRuntimeUpgrade}, + Twox64Concat, +}; +use parity_scale_codec::{Decode, Encode}; +use scale_info::TypeInfo; +use sp_runtime::{traits::Saturating, SaturatedConversion}; +use sp_std::vec::Vec; + +use crate::{ + curves::Curve, + types::{Locks, PoolStatus}, + BondedCurrenciesSettingsOf, BoundedCurrencyVec, CollateralAssetIdOf, Config, CurveParameterTypeOf, + DepositBalanceOf, FungiblesBalanceOf, +}; + +/// Collection of storage item formats from the previous storage version. +/// +/// Required so we can read values in the v0 storage format during the +/// migration. +mod v0 { + + use super::*; + + // V0 pool details + #[derive(Clone, Encode, Decode, PartialEq, Eq, TypeInfo, MaxEncodedLen, Debug)] + pub struct PoolDetails { + /// The owner of the pool. + pub owner: AccountId, + /// The manager of the pool. If a manager is set, the pool is + /// permissioned. + pub manager: Option, + /// The curve of the pool. + pub curve: ParametrizedCurve, + /// The collateral currency of the pool. + pub collateral: BaseCurrencyId, + /// The bonded currencies of the pool. + pub bonded_currencies: Currencies, + /// The status of the pool. + pub state: PoolStatus, + /// Whether the pool is transferable or not. + pub transferable: bool, + /// The denomination of the pool. + pub denomination: u8, + /// The minimum amount that can be minted/burnt. + pub min_operation_balance: FungiblesBalance, + /// The deposit to be returned upon destruction of this pool. + pub deposit: DepositBalance, + } + + pub type PoolDetailsOf = PoolDetails< + ::AccountId, + Curve>, + BoundedCurrencyVec, + CollateralAssetIdOf, + DepositBalanceOf, + FungiblesBalanceOf, + >; + + /// V0 type for [`crate::Pools`]. + #[storage_alias] + pub type Pools = + StorageMap, Twox64Concat, ::PoolId, PoolDetailsOf, OptionQuery>; +} + +fn v0_to_v1(old_value: v0::PoolDetailsOf) -> crate::PoolDetailsOf { + let v0::PoolDetailsOf:: { + owner, + curve, + manager, + collateral, + bonded_currencies, + state, + transferable, + denomination, + min_operation_balance, + deposit, + } = old_value; + + crate::PoolDetailsOf:: { + owner, + curve, + manager, + collateral, + bonded_currencies, + state, + deposit, + currencies_settings: BondedCurrenciesSettingsOf:: { + denomination, + min_operation_balance, + transferable, + allow_reset_team: true, + }, + } +} + +pub struct InnerMigrateV0ToV1(core::marker::PhantomData); + +impl OnRuntimeUpgrade for InnerMigrateV0ToV1 { + /// Return the existing [`crate::Value`] so we can check that it was + /// correctly set in `InnerMigrateV0ToV1::post_upgrade`. + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + // Access the old value using the `storage_alias` type + let old_value: Vec<(T::PoolId, v0::PoolDetailsOf)> = v0::Pools::::iter().collect(); + // Return it as an encoded `Vec` + Ok(old_value.encode()) + } + + /// Migrate the storage from V0 to V1. + /// + /// - If the value doesn't exist, there is nothing to do. + /// - If the value exists, it is read and then written back to storage + /// inside a [`crate::CurrentAndPreviousValue`]. + fn on_runtime_upgrade() -> frame_support::weights::Weight { + let mut translated = 0u64; + // Read values in-place + crate::Pools::::translate_values::, _>(|old_value| { + translated.saturating_inc(); + Some(v0_to_v1::(old_value)) + }); + + // One read for taking the old value, and one write for setting the new value + T::DbWeight::get().reads_writes(translated, translated) + } + + /// Verifies the storage was migrated correctly. + /// + /// - If there was no old value, the new value should not be set. + /// - If there was an old value, the new value should be a + /// [`crate::CurrentAndPreviousValue`]. + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let old_values = Vec::<(T::PoolId, v0::PoolDetailsOf)>::decode(&mut &state[..]) + .map_err(|_| sp_runtime::TryRuntimeError::Other("Failed to decode old value from storage"))?; + + let prev_count: u32 = old_values.len().saturated_into(); + let post_count: u32 = crate::Pools::::iter().count().saturated_into(); + + ensure!( + prev_count == post_count, + "the pool count before and after the migration should be the same" + ); + + old_values.into_iter().try_for_each(|(pool_id, old_value)| { + let expected_new_value = v0_to_v1::(old_value); + let actual_new_value = crate::Pools::::get(&pool_id); + + ensure!(actual_new_value.is_some(), "New value not set"); + ensure!( + actual_new_value == Some(expected_new_value), + "New value not set correctly" + ); + + ensure!( + actual_new_value.unwrap().currencies_settings.allow_reset_team, + "all migrated pools should have the allow_reset_team flag set to true" + ); + + Ok(()) + }) + } +} From 853a5f7a2471d00299368a180791f7ec3f893324 Mon Sep 17 00:00:00 2001 From: Raphael Date: Wed, 26 Mar 2025 11:23:51 +0100 Subject: [PATCH 2/5] chore: register runtime migration --- pallets/pallet-bonded-coins/src/migrations/mod.rs | 2 +- pallets/pallet-bonded-coins/src/migrations/v1.rs | 12 ++++++++++++ runtimes/peregrine/src/migrations/mod.rs | 1 + 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pallets/pallet-bonded-coins/src/migrations/mod.rs b/pallets/pallet-bonded-coins/src/migrations/mod.rs index fbb1de71a..a3a6d96c3 100644 --- a/pallets/pallet-bonded-coins/src/migrations/mod.rs +++ b/pallets/pallet-bonded-coins/src/migrations/mod.rs @@ -1 +1 @@ -mod v1; +pub mod v1; diff --git a/pallets/pallet-bonded-coins/src/migrations/v1.rs b/pallets/pallet-bonded-coins/src/migrations/v1.rs index 0d1754985..10b1aee32 100644 --- a/pallets/pallet-bonded-coins/src/migrations/v1.rs +++ b/pallets/pallet-bonded-coins/src/migrations/v1.rs @@ -16,6 +16,8 @@ use crate::{ DepositBalanceOf, FungiblesBalanceOf, }; +const LOG_TARGET: &str = "migration::pallet-bonded-coins"; + /// Collection of storage item formats from the previous storage version. /// /// Required so we can read values in the v0 storage format during the @@ -144,6 +146,8 @@ impl OnRuntimeUpgrade for InnerMigrateV0ToV1 { "the pool count before and after the migration should be the same" ); + log::info!(target: LOG_TARGET, "Migrated {} pool entries", post_count); + old_values.into_iter().try_for_each(|(pool_id, old_value)| { let expected_new_value = v0_to_v1::(old_value); let actual_new_value = crate::Pools::::get(&pool_id); @@ -163,3 +167,11 @@ impl OnRuntimeUpgrade for InnerMigrateV0ToV1 { }) } } + +pub type MigrateV0ToV1 = frame_support::migrations::VersionedMigration< + 0, // The migration will only execute when the on-chain storage version is 0 + 1, // The on-chain storage version will be set to 1 after the migration is complete + InnerMigrateV0ToV1, + crate::pallet::Pallet, + ::DbWeight, +>; diff --git a/runtimes/peregrine/src/migrations/mod.rs b/runtimes/peregrine/src/migrations/mod.rs index f6ca110fa..76762e1c0 100644 --- a/runtimes/peregrine/src/migrations/mod.rs +++ b/runtimes/peregrine/src/migrations/mod.rs @@ -32,6 +32,7 @@ pub type RuntimeMigrations = ( frame_support::migrations::RemovePallet::DbWeight>, frame_support::migrations::RemovePallet::DbWeight>, frame_support::migrations::RemovePallet::DbWeight>, + pallet_bonded_coins::migrations::v1::MigrateV0ToV1, ); impl pallet_migration::Config for Runtime { From b5adf8d83e052634b92fe160a82385c41e827fb5 Mon Sep 17 00:00:00 2001 From: Raphael Date: Mon, 31 Mar 2025 13:57:56 +0200 Subject: [PATCH 3/5] fix: migrations imports --- .../pallet-bonded-coins/src/migrations/v1.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pallets/pallet-bonded-coins/src/migrations/v1.rs b/pallets/pallet-bonded-coins/src/migrations/v1.rs index 10b1aee32..e7496da80 100644 --- a/pallets/pallet-bonded-coins/src/migrations/v1.rs +++ b/pallets/pallet-bonded-coins/src/migrations/v1.rs @@ -1,13 +1,8 @@ use frame_support::{ pallet_prelude::*, - storage_alias, traits::{Get, OnRuntimeUpgrade}, - Twox64Concat, }; -use parity_scale_codec::{Decode, Encode}; -use scale_info::TypeInfo; -use sp_runtime::{traits::Saturating, SaturatedConversion}; -use sp_std::vec::Vec; +use sp_runtime::traits::Saturating; use crate::{ curves::Curve, @@ -16,6 +11,12 @@ use crate::{ DepositBalanceOf, FungiblesBalanceOf, }; +#[cfg(feature = "try-runtime")] +use sp_runtime::traits::SaturatedConversion; +#[cfg(feature = "try-runtime")] +use sp_std::vec::Vec; + +#[cfg(feature = "try-runtime")] const LOG_TARGET: &str = "migration::pallet-bonded-coins"; /// Collection of storage item formats from the previous storage version. @@ -23,8 +24,10 @@ const LOG_TARGET: &str = "migration::pallet-bonded-coins"; /// Required so we can read values in the v0 storage format during the /// migration. mod v0 { - use super::*; + use frame_support::{storage_alias, Twox64Concat}; + use parity_scale_codec::{Decode, Encode}; + use scale_info::TypeInfo; // V0 pool details #[derive(Clone, Encode, Decode, PartialEq, Eq, TypeInfo, MaxEncodedLen, Debug)] From ef48bd948da6c9f5dfe5037bbda34199a8df9b85 Mon Sep 17 00:00:00 2001 From: Raphael Date: Mon, 31 Mar 2025 16:09:11 +0200 Subject: [PATCH 4/5] chore: fix docs and improve logging --- .../pallet-bonded-coins/src/migrations/v1.rs | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/pallets/pallet-bonded-coins/src/migrations/v1.rs b/pallets/pallet-bonded-coins/src/migrations/v1.rs index e7496da80..65c4db0b0 100644 --- a/pallets/pallet-bonded-coins/src/migrations/v1.rs +++ b/pallets/pallet-bonded-coins/src/migrations/v1.rs @@ -11,11 +11,6 @@ use crate::{ DepositBalanceOf, FungiblesBalanceOf, }; -#[cfg(feature = "try-runtime")] -use sp_runtime::traits::SaturatedConversion; -#[cfg(feature = "try-runtime")] -use sp_std::vec::Vec; - #[cfg(feature = "try-runtime")] const LOG_TARGET: &str = "migration::pallet-bonded-coins"; @@ -103,22 +98,21 @@ fn v0_to_v1(old_value: v0::PoolDetailsOf) -> crate::PoolDetailsOf< pub struct InnerMigrateV0ToV1(core::marker::PhantomData); -impl OnRuntimeUpgrade for InnerMigrateV0ToV1 { - /// Return the existing [`crate::Value`] so we can check that it was - /// correctly set in `InnerMigrateV0ToV1::post_upgrade`. +impl OnRuntimeUpgrade for InnerMigrateV0ToV1 +where + T::PoolId: sp_std::fmt::Display, +{ + /// Return a vector of existing [`crate::Pools`] values so we can check that + /// they were correctly set in `InnerMigrateV0ToV1::post_upgrade`. #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { // Access the old value using the `storage_alias` type - let old_value: Vec<(T::PoolId, v0::PoolDetailsOf)> = v0::Pools::::iter().collect(); + let old_value: sp_std::vec::Vec<(T::PoolId, v0::PoolDetailsOf)> = v0::Pools::::iter().collect(); // Return it as an encoded `Vec` Ok(old_value.encode()) } /// Migrate the storage from V0 to V1. - /// - /// - If the value doesn't exist, there is nothing to do. - /// - If the value exists, it is read and then written back to storage - /// inside a [`crate::CurrentAndPreviousValue`]. fn on_runtime_upgrade() -> frame_support::weights::Weight { let mut translated = 0u64; // Read values in-place @@ -132,13 +126,11 @@ impl OnRuntimeUpgrade for InnerMigrateV0ToV1 { } /// Verifies the storage was migrated correctly. - /// - /// - If there was no old value, the new value should not be set. - /// - If there was an old value, the new value should be a - /// [`crate::CurrentAndPreviousValue`]. #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { - let old_values = Vec::<(T::PoolId, v0::PoolDetailsOf)>::decode(&mut &state[..]) + fn post_upgrade(state: sp_std::vec::Vec) -> Result<(), sp_runtime::TryRuntimeError> { + use sp_runtime::traits::SaturatedConversion; + + let old_values = sp_std::vec::Vec::<(T::PoolId, v0::PoolDetailsOf)>::decode(&mut &state[..]) .map_err(|_| sp_runtime::TryRuntimeError::Other("Failed to decode old value from storage"))?; let prev_count: u32 = old_values.len().saturated_into(); @@ -155,16 +147,21 @@ impl OnRuntimeUpgrade for InnerMigrateV0ToV1 { let expected_new_value = v0_to_v1::(old_value); let actual_new_value = crate::Pools::::get(&pool_id); - ensure!(actual_new_value.is_some(), "New value not set"); - ensure!( - actual_new_value == Some(expected_new_value), - "New value not set correctly" - ); - - ensure!( - actual_new_value.unwrap().currencies_settings.allow_reset_team, - "all migrated pools should have the allow_reset_team flag set to true" - ); + ensure!(actual_new_value.is_some(), { + log::error!(target: LOG_TARGET, "Expected pool with id {} but found none", &pool_id); + sp_runtime::TryRuntimeError::Other("Pool not migrated") + }); + ensure!(actual_new_value == Some(expected_new_value), { + log::error!(target: LOG_TARGET, "Pool with id {} contains unexpected data", &pool_id); + sp_runtime::TryRuntimeError::Other("Incorrect Pool Data") + }); + + ensure!(actual_new_value.unwrap().currencies_settings.allow_reset_team, { + log::error!(target: LOG_TARGET, "Pool with id {} has allow_reset_team = false", &pool_id); + sp_runtime::TryRuntimeError::Other( + "all migrated pools should have the allow_reset_team flag set to true", + ) + }); Ok(()) }) From 7c6073d7715a6230ac74d9df4f6cb0a46a7e74f4 Mon Sep 17 00:00:00 2001 From: Raphael Date: Tue, 1 Apr 2025 16:01:05 +0200 Subject: [PATCH 5/5] fix: use debug printing instead of display --- pallets/pallet-bonded-coins/src/migrations/v1.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pallets/pallet-bonded-coins/src/migrations/v1.rs b/pallets/pallet-bonded-coins/src/migrations/v1.rs index 65c4db0b0..4f56bf754 100644 --- a/pallets/pallet-bonded-coins/src/migrations/v1.rs +++ b/pallets/pallet-bonded-coins/src/migrations/v1.rs @@ -100,7 +100,7 @@ pub struct InnerMigrateV0ToV1(core::marker::PhantomData); impl OnRuntimeUpgrade for InnerMigrateV0ToV1 where - T::PoolId: sp_std::fmt::Display, + T::PoolId: sp_std::fmt::Debug, { /// Return a vector of existing [`crate::Pools`] values so we can check that /// they were correctly set in `InnerMigrateV0ToV1::post_upgrade`. @@ -148,16 +148,16 @@ where let actual_new_value = crate::Pools::::get(&pool_id); ensure!(actual_new_value.is_some(), { - log::error!(target: LOG_TARGET, "Expected pool with id {} but found none", &pool_id); + log::error!(target: LOG_TARGET, "Expected pool with id {:?} but found none", &pool_id); sp_runtime::TryRuntimeError::Other("Pool not migrated") }); ensure!(actual_new_value == Some(expected_new_value), { - log::error!(target: LOG_TARGET, "Pool with id {} contains unexpected data", &pool_id); + log::error!(target: LOG_TARGET, "Pool with id {:?} contains unexpected data", &pool_id); sp_runtime::TryRuntimeError::Other("Incorrect Pool Data") }); ensure!(actual_new_value.unwrap().currencies_settings.allow_reset_team, { - log::error!(target: LOG_TARGET, "Pool with id {} has allow_reset_team = false", &pool_id); + log::error!(target: LOG_TARGET, "Pool with id {:?} has allow_reset_team = false", &pool_id); sp_runtime::TryRuntimeError::Other( "all migrated pools should have the allow_reset_team flag set to true", )