From 57503658ecbe17eb8af928571fa383722f941cb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 4 Dec 2022 22:05:58 +0100 Subject: [PATCH] pallet-balances: Fix inactive funds migration Fixes the inactive funds migration. It was missing to set the `storage_version` attribute for the `Pallet` struct. Besides that it also removes the old `StorageVersion` representation and adds support for instances of pallet-balances. --- frame/balances/src/lib.rs | 29 +++--------- frame/balances/src/migration.rs | 79 ++++++++++++++++++--------------- 2 files changed, 48 insertions(+), 60 deletions(-) diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index c4a8456c22c67..381a0ffceeb85 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -246,8 +246,13 @@ pub mod pallet { type ReserveIdentifier: Parameter + Member + MaxEncodedLen + Ord + Copy; } + /// The current storage version. + const STORAGE_VERSION: frame_support::traits::StorageVersion = + frame_support::traits::StorageVersion::new(1); + #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(PhantomData<(T, I)>); #[pallet::call] @@ -556,13 +561,6 @@ pub mod pallet { ValueQuery, >; - /// Storage version of the pallet. - /// - /// This is set to v2.0.0 for new networks. - #[pallet::storage] - pub(super) type StorageVersion, I: 'static = ()> = - StorageValue<_, Releases, ValueQuery>; - #[pallet::genesis_config] pub struct GenesisConfig, I: 'static = ()> { pub balances: Vec<(T::AccountId, T::Balance)>, @@ -581,8 +579,6 @@ pub mod pallet { let total = self.balances.iter().fold(Zero::zero(), |acc: T::Balance, &(_, n)| acc + n); >::put(total); - >::put(Releases::V2_0_0); - for (_, balance) in &self.balances { assert!( *balance >= >::ExistentialDeposit::get(), @@ -727,21 +723,6 @@ impl AccountData { } } -// A value placed in storage that represents the current version of the Balances storage. -// This value is used by the `on_runtime_upgrade` logic to determine whether we run -// storage migration logic. This should match directly with the semantic versions of the Rust crate. -#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, MaxEncodedLen, TypeInfo)] -enum Releases { - V1_0_0, - V2_0_0, -} - -impl Default for Releases { - fn default() -> Self { - Releases::V1_0_0 - } -} - pub struct DustCleaner, I: 'static = ()>( Option<(T::AccountId, NegativeImbalance)>, ); diff --git a/frame/balances/src/migration.rs b/frame/balances/src/migration.rs index 1dd3e6f51f1be..08e1d8c7a2c74 100644 --- a/frame/balances/src/migration.rs +++ b/frame/balances/src/migration.rs @@ -15,50 +15,57 @@ // along with Polkadot. If not, see . use super::*; -use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade, weights::Weight}; +use frame_support::{ + pallet_prelude::*, + traits::{OnRuntimeUpgrade, PalletInfoAccess}, + weights::Weight, +}; -// NOTE: This must be used alongside the account whose balance is expected to be inactive. -// Generally this will be used for the XCM teleport checking account. -pub struct MigrateToTrackInactive(PhantomData<(T, A)>); -impl> OnRuntimeUpgrade for MigrateToTrackInactive { - fn on_runtime_upgrade() -> Weight { - let current_version = Pallet::::current_storage_version(); - let onchain_version = Pallet::::on_chain_storage_version(); +fn migrate_v0_to_v1, I: 'static>(accounts: &[T::AccountId]) -> Weight { + let onchain_version = Pallet::::on_chain_storage_version(); + + if onchain_version == 0 { + let total = accounts + .iter() + .map(|a| Pallet::::total_balance(a)) + .fold(T::Balance::zero(), |a, e| a.saturating_add(e)); + Pallet::::deactivate(total); + + // Remove the old `StorageVersion` type. + frame_support::storage::unhashed::kill(&frame_support::storage::storage_prefix( + Pallet::::name().as_bytes(), + "StorageVersion".as_bytes(), + )); - if onchain_version == 0 && current_version == 1 { - let b = Pallet::::total_balance(&A::get()); - Pallet::::deactivate(b); - current_version.put::>(); - log::info!(target: "runtime::balances", "Storage to version {:?}", current_version); - T::DbWeight::get().reads_writes(4, 3) - } else { - log::info!(target: "runtime::balances", "Migration did not execute. This probably should be removed"); - T::DbWeight::get().reads(2) - } + // Set storage version to `1`. + StorageVersion::new(1).put::>(); + + log::info!(target: "runtime::balances", "Storage to version 1"); + T::DbWeight::get().reads_writes(2 + accounts.len() as u64, 3) + } else { + log::info!(target: "runtime::balances", "Migration did not execute. This probably should be removed"); + T::DbWeight::get().reads(1) } } // NOTE: This must be used alongside the account whose balance is expected to be inactive. // Generally this will be used for the XCM teleport checking account. -pub struct MigrateManyToTrackInactive(PhantomData<(T, A)>); -impl>> OnRuntimeUpgrade for MigrateManyToTrackInactive { +pub struct MigrateToTrackInactive(PhantomData<(T, A, I)>); +impl, A: Get, I: 'static> OnRuntimeUpgrade + for MigrateToTrackInactive +{ fn on_runtime_upgrade() -> Weight { - let current_version = Pallet::::current_storage_version(); - let onchain_version = Pallet::::on_chain_storage_version(); + migrate_v0_to_v1::(&[A::get()]) + } +} - if onchain_version == 0 && current_version == 1 { - let accounts = A::get(); - let total = accounts - .iter() - .map(|a| Pallet::::total_balance(a)) - .fold(T::Balance::zero(), |a, e| a.saturating_add(e)); - Pallet::::deactivate(total); - current_version.put::>(); - log::info!(target: "runtime::balances", "Storage to version {:?}", current_version); - T::DbWeight::get().reads_writes(3 + accounts.len() as u64, 3) - } else { - log::info!(target: "runtime::balances", "Migration did not execute. This probably should be removed"); - T::DbWeight::get().reads(2) - } +// NOTE: This must be used alongside the accounts whose balance is expected to be inactive. +// Generally this will be used for the XCM teleport checking accounts. +pub struct MigrateManyToTrackInactive(PhantomData<(T, A, I)>); +impl, A: Get>, I: 'static> OnRuntimeUpgrade + for MigrateManyToTrackInactive +{ + fn on_runtime_upgrade() -> Weight { + migrate_v0_to_v1::(&A::get()) } }