From 5ab100b25a34fda07878b17edb2f45bc8d438c4d Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Thu, 25 Apr 2024 18:14:42 +0200 Subject: [PATCH] fix: outdated storage versions on Demo (#1820) * chore: update block rewards init migration * fix: add missing demo storage version bumps * fix: pre-emptive version bump * refactor: nuke some pallets * fmt: clippy --- pallets/block-rewards/src/migrations.rs | 120 +++++++++++++----------- runtime/development/src/migrations.rs | 34 +++++++ 2 files changed, 99 insertions(+), 55 deletions(-) diff --git a/pallets/block-rewards/src/migrations.rs b/pallets/block-rewards/src/migrations.rs index 3306e98cbf..c6de5570a9 100644 --- a/pallets/block-rewards/src/migrations.rs +++ b/pallets/block-rewards/src/migrations.rs @@ -12,12 +12,10 @@ use cfg_traits::TimeAsSecs; use frame_support::{ - pallet_prelude::{StorageVersion, Weight}, + pallet_prelude::Weight, traits::{Get, GetStorageVersion, OnRuntimeUpgrade}, }; #[cfg(feature = "try-runtime")] -use num_traits::Zero; -#[cfg(feature = "try-runtime")] use parity_scale_codec::{Decode, Encode}; use sp_runtime::FixedPointNumber; #[cfg(feature = "try-runtime")] @@ -31,9 +29,8 @@ fn inflation_rate(percent: u32) -> T::Rate { } pub mod init { - #[cfg(feature = "try-runtime")] - use cfg_traits::rewards::AccountRewards; - use cfg_traits::rewards::CurrencyGroupChange; + use cfg_traits::rewards::{AccountRewards, CurrencyGroupChange, GroupRewards}; + use num_traits::Zero; use sp_runtime::{BoundedVec, SaturatedConversion}; use super::*; @@ -60,17 +57,12 @@ pub mod init { for InitBlockRewards where T: frame_system::Config + Config + pallet_collator_selection::Config, - ::Balance: From, - CollatorReward: Get<::Balance>, + T::Balance: From, + CollatorReward: Get, AnnualTreasuryInflationPercent: Get, { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { - assert_eq!( - Pallet::::on_chain_storage_version(), - StorageVersion::new(0), - "On-chain storage version should be 0 (default)" - ); let collators = get_collators::(); assert!(!collators.is_empty()); @@ -83,19 +75,46 @@ pub mod init { // Weight: 2 + collator_count reads and writes fn on_runtime_upgrade() -> Weight { - if Pallet::::on_chain_storage_version() == StorageVersion::new(0) { - log::info!("{LOG_PREFIX} Initiating migration"); - let mut weight: Weight = Weight::zero(); + log::info!("{LOG_PREFIX} Initiating migration"); + + log::info!("{LOG_PREFIX} Checking whether group is ready"); + let group_is_ready = T::Rewards::is_ready(T::StakeGroupId::get()); + + log::info!("{LOG_PREFIX} Checking whether session data is set"); + let session_data_already_set = + pallet::ActiveSessionData::::get() != Default::default(); + + log::info!("{LOG_PREFIX} Getting list of collators"); + let collators = get_collators::(); + + let collators_are_staked = collators.clone().into_iter().all(|c| { + log::info!("{LOG_PREFIX} Checking stake of collator {c:?}"); + !T::Rewards::account_stake(T::StakeCurrencyId::get(), &c).is_zero() + }); + let mut weight = + T::DbWeight::get().reads(2u64.saturating_add(collators.len().saturated_into())); + + if group_is_ready && session_data_already_set && collators_are_staked { + log::info!( + "{LOG_PREFIX} Migration not necessary because all data is already initialized" + ); + return weight; + } + + if !group_is_ready { + log::info!("{LOG_PREFIX} Attaching currency to collator group"); + + T::Rewards::attach_currency(T::StakeCurrencyId::get(), T::StakeGroupId::get()) + .map_err(|e| { + log::error!("Failed to attach currency to collator group: {:?}", e) + }) + .ok(); - let collators = get_collators::(); - weight.saturating_accrue(T::DbWeight::get().reads(2)); + weight.saturating_accrue(T::DbWeight::get().writes(1)); + } - ::Rewards::attach_currency( - ::StakeCurrencyId::get(), - ::StakeGroupId::get(), - ) - .map_err(|e| log::error!("Failed to attach currency to collator group: {:?}", e)) - .ok(); + if !session_data_already_set { + log::info!("{LOG_PREFIX} Setting session data"); pallet::ActiveSessionData::::set(SessionData:: { collator_count: collators.len().saturated_into(), @@ -106,25 +125,30 @@ pub mod init { last_update: T::Time::now(), }); weight.saturating_accrue(T::DbWeight::get().writes(1)); + } + if !collators_are_staked { for collator in collators.iter() { - // NOTE: Benching not required as num of collators <= 10. - Pallet::::do_init_collator(collator) - .map_err(|e| { - log::error!("Failed to init genesis collators for rewards: {:?}", e); - }) - .ok(); - weight.saturating_accrue(T::DbWeight::get().reads_writes(6, 6)); + if T::Rewards::account_stake(T::StakeCurrencyId::get(), collator).is_zero() { + log::info!("{LOG_PREFIX} Adding stake for collator {collator:?}"); + // NOTE: Benching not required as num of collators <= 10. + Pallet::::do_init_collator(collator) + .map_err(|e| { + log::error!( + "Failed to init genesis collators for rewards: {:?}", + e + ); + }) + .ok(); + weight.saturating_accrue(T::DbWeight::get().reads_writes(6, 6)); + } } - Pallet::::current_storage_version().put::>(); - weight.saturating_add(T::DbWeight::get().writes(1)) - } else { - // wrong storage version - log::info!( - "{LOG_PREFIX} Migration did not execute. This probably should be removed" - ); - T::DbWeight::get().reads_writes(1, 0) } + + log::info!("{LOG_PREFIX} Migration complete"); + + Pallet::::current_storage_version().put::>(); + weight.saturating_add(T::DbWeight::get().writes(1)) } #[cfg(feature = "try-runtime")] @@ -137,24 +161,10 @@ pub mod init { let collators: Vec = Decode::decode(&mut pre_state.as_slice()) .expect("pre_upgrade provides a valid state; qed"); - assert_eq!( - Pallet::::active_session_data(), - SessionData:: { - collator_count: collators.len().saturated_into(), - collator_reward: CollatorReward::get(), - treasury_inflation_rate: inflation_rate::( - AnnualTreasuryInflationPercent::get() - ), - last_update: T::Time::now(), - } - ); + assert_ne!(Pallet::::active_session_data(), Default::default()); for collator in collators.iter() { - assert!(!::Rewards::account_stake( - ::StakeCurrencyId::get(), - collator, - ) - .is_zero()) + assert!(!T::Rewards::account_stake(T::StakeCurrencyId::get(), collator,).is_zero()) } log::info!("{LOG_PREFIX} Post migration checks successful"); diff --git a/runtime/development/src/migrations.rs b/runtime/development/src/migrations.rs index 38680a593a..105b1ac17e 100644 --- a/runtime/development/src/migrations.rs +++ b/runtime/development/src/migrations.rs @@ -10,11 +10,45 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. +use sp_core::parameter_types; +parameter_types! { + pub const CollatorReward: cfg_primitives::Balance = cfg_primitives::constants::CFG; + pub const AnnualTreasuryInflationPercent: u32 = 3; +} + /// The migration set for Development & Demo. /// It includes all the migrations that have to be applied on that chain. pub type UpgradeDevelopment1047 = ( pallet_collator_selection::migration::v1::MigrateToV1, cleanup_foreign_investments::Migration, + // v0 -> v1 + pallet_multisig::migrations::v1::MigrateToV1, + // v0 -> v1 + pallet_balances::migration::MigrateToTrackInactive, + // v0 -> v1 + runtime_common::migrations::nuke::ResetPallet, + // v0 -> v1 + runtime_common::migrations::nuke::ResetPallet, + // v0 -> v1 + pallet_xcm::migration::v1::VersionUncheckedMigrateToV1, + runtime_common::migrations::increase_storage_version::Migration, + runtime_common::migrations::increase_storage_version::Migration, + runtime_common::migrations::increase_storage_version::Migration, + runtime_common::migrations::increase_storage_version::Migration, + runtime_common::migrations::increase_storage_version::Migration, + runtime_common::migrations::increase_storage_version::Migration< + crate::OraclePriceCollection, + 0, + 1, + >, + runtime_common::migrations::increase_storage_version::Migration, + // Reset Block rewards + runtime_common::migrations::nuke::ResetPallet, + pallet_block_rewards::migrations::init::InitBlockRewards< + crate::Runtime, + CollatorReward, + AnnualTreasuryInflationPercent, + >, ); mod cleanup_foreign_investments {