From 4901c0c07f2aa1735ce6e766a84432b3d37c2f66 Mon Sep 17 00:00:00 2001 From: koushiro Date: Thu, 16 Sep 2021 14:24:35 +0800 Subject: [PATCH 1/8] Migrate session-historical to the new pallet macro Signed-off-by: koushiro --- frame/session/benchmarking/src/lib.rs | 4 +- frame/session/src/historical/mod.rs | 146 +++++++++++++---------- frame/session/src/historical/offchain.rs | 4 +- 3 files changed, 88 insertions(+), 66 deletions(-) diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs index c0131957c8732..9f043ca8c3843 100644 --- a/frame/session/benchmarking/src/lib.rs +++ b/frame/session/benchmarking/src/lib.rs @@ -22,6 +22,7 @@ mod mock; +use sp_runtime::traits::{One, StaticLookup}; use sp_std::{prelude::*, vec}; use frame_benchmarking::{benchmarks, impl_benchmark_test_suite}; @@ -30,12 +31,11 @@ use frame_support::{ traits::{KeyOwnerProofSystem, OnInitialize}, }; use frame_system::RawOrigin; -use pallet_session::{historical::Module as Historical, Pallet as Session, *}; +use pallet_session::{historical::Pallet as Historical, Pallet as Session, *}; use pallet_staking::{ benchmarking::create_validator_with_nominators, testing_utils::create_validators, RewardDestination, }; -use sp_runtime::traits::{One, StaticLookup}; const MAX_VALIDATORS: u32 = 1000; diff --git a/frame/session/src/historical/mod.rs b/frame/session/src/historical/mod.rs index 0801b2aca1701..dc8876f482192 100644 --- a/frame/session/src/historical/mod.rs +++ b/frame/session/src/historical/mod.rs @@ -26,62 +26,84 @@ //! These roots and proofs of inclusion can be generated at any time during the current session. //! Afterwards, the proofs can be fed to a consensus module when reporting misbehavior. -use super::{Pallet as SessionModule, SessionIndex}; +pub mod offchain; +pub mod onchain; +mod shared; + use codec::{Decode, Encode}; -use frame_support::{ - decl_module, decl_storage, print, - traits::{ValidatorSet, ValidatorSetWithIdentification}, - Parameter, -}; use sp_runtime::{ traits::{Convert, OpaqueKeys}, KeyTypeId, }; use sp_session::{MembershipProof, ValidatorCount}; +use sp_staking::SessionIndex; use sp_std::prelude::*; use sp_trie::{ trie_types::{TrieDB, TrieDBMut}, MemoryDB, Recorder, Trie, TrieMut, EMPTY_PREFIX, }; -pub mod offchain; -pub mod onchain; -mod shared; +use frame_support::{ + print, + traits::{KeyOwnerProofSystem, StorageVersion, ValidatorSet, ValidatorSetWithIdentification}, + Parameter, +}; -/// Config necessary for the historical module. -pub trait Config: super::Config { - /// Full identification of the validator. - type FullIdentification: Parameter; - - /// A conversion from validator ID to full identification. - /// - /// This should contain any references to economic actors associated with the - /// validator, since they may be outdated by the time this is queried from a - /// historical trie. - /// - /// It must return the identification for the current session index. - type FullIdentificationOf: Convert>; -} +use crate::{self as pallet_session, Pallet as Session}; -decl_storage! { - trait Store for Module as Session { - /// Mapping from historical session indices to session-data root hash and validator count. - HistoricalSessions get(fn historical_root): - map hasher(twox_64_concat) SessionIndex => Option<(T::Hash, ValidatorCount)>; - /// The range of historical sessions we store. [first, last) - StoredRange: Option<(SessionIndex, SessionIndex)>; - /// Deprecated. - CachedObsolete: - map hasher(twox_64_concat) SessionIndex - => Option>; +pub use pallet::*; + +#[frame_support::pallet] +pub mod pallet { + use super::*; + use frame_support::pallet_prelude::*; + + /// The current storage version. + const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + #[pallet::storage_version(STORAGE_VERSION)] + pub struct Pallet(_); + + /// Config necessary for the historical pallet. + #[pallet::config] + pub trait Config: pallet_session::Config + frame_system::Config { + /// Full identification of the validator. + type FullIdentification: Parameter; + + /// A conversion from validator ID to full identification. + /// + /// This should contain any references to economic actors associated with the + /// validator, since they may be outdated by the time this is queried from a + /// historical trie. + /// + /// It must return the identification for the current session index. + type FullIdentificationOf: Convert>; } -} -decl_module! { - pub struct Module for enum Call where origin: T::Origin {} + /// Mapping from historical session indices to session-data root hash and validator count. + #[pallet::storage] + #[pallet::getter(fn historical_root)] + pub type HistoricalSessions = + StorageMap<_, Twox64Concat, SessionIndex, (T::Hash, ValidatorCount), OptionQuery>; + + /// The range of historical sessions we store. [first, last) + #[pallet::storage] + pub type StoredRange = StorageValue<_, (SessionIndex, SessionIndex), OptionQuery>; + + /// Deprecated. + #[pallet::storage] + pub type CachedObsolete = StorageMap< + _, + Twox64Concat, + SessionIndex, + Vec<(T::ValidatorId, T::FullIdentification)>, + OptionQuery, + >; } -impl Module { +impl Pallet { /// Prune historical stored session roots up to (but not including) /// `up_to`. pub fn prune_up_to(up_to: SessionIndex) { @@ -109,7 +131,7 @@ impl Module { } } -impl ValidatorSet for Module { +impl ValidatorSet for Pallet { type ValidatorId = T::ValidatorId; type ValidatorIdOf = T::ValidatorIdOf; @@ -122,7 +144,7 @@ impl ValidatorSet for Module { } } -impl ValidatorSetWithIdentification for Module { +impl ValidatorSetWithIdentification for Pallet { type Identification = T::FullIdentification; type IdentificationOf = T::FullIdentificationOf; } @@ -130,7 +152,7 @@ impl ValidatorSetWithIdentification for Module { /// Specialization of the crate-level `SessionManager` which returns the set of full identification /// when creating a new session. pub trait SessionManager: - crate::SessionManager + pallet_session::SessionManager { /// If there was a validator set change, its returns the set of new validators along with their /// full identifications. @@ -150,7 +172,7 @@ pub struct NoteHistoricalRoot(sp_std::marker::PhantomData<(T, I)>); impl> NoteHistoricalRoot { fn do_new_session(new_index: SessionIndex, is_genesis: bool) -> Option> { - StoredRange::mutate(|range| { + >::mutate(|range| { range.get_or_insert_with(|| (new_index, new_index)).1 = new_index + 1; }); @@ -183,7 +205,7 @@ impl> NoteHi } } -impl crate::SessionManager for NoteHistoricalRoot +impl pallet_session::SessionManager for NoteHistoricalRoot where I: SessionManager, { @@ -207,7 +229,7 @@ where /// A tuple of the validator's ID and their full identification. pub type IdentificationTuple = - (::ValidatorId, ::FullIdentification); + (::ValidatorId, ::FullIdentification); /// A trie instance for checking and generating proofs. pub struct ProvingTrie { @@ -227,7 +249,7 @@ impl ProvingTrie { let mut trie = TrieDBMut::new(&mut db, &mut root); for (i, (validator, full_id)) in validators.into_iter().enumerate() { let i = i as u32; - let keys = match >::load_keys(&validator) { + let keys = match >::load_keys(&validator) { None => continue, Some(k) => k, }; @@ -304,15 +326,13 @@ impl ProvingTrie { } } -impl> frame_support::traits::KeyOwnerProofSystem<(KeyTypeId, D)> - for Module -{ +impl> KeyOwnerProofSystem<(KeyTypeId, D)> for Pallet { type Proof = MembershipProof; type IdentificationTuple = IdentificationTuple; fn prove(key: (KeyTypeId, D)) -> Option { - let session = >::current_index(); - let validators = >::validators() + let session = >::current_index(); + let validators = >::validators() .into_iter() .filter_map(|validator| { T::FullIdentificationOf::convert(validator.clone()) @@ -335,10 +355,10 @@ impl> frame_support::traits::KeyOwnerProofSystem<(KeyT fn check_proof(key: (KeyTypeId, D), proof: Self::Proof) -> Option> { let (id, data) = key; - if proof.session == >::current_index() { - >::key_owner(id, data.as_ref()).and_then(|owner| { + if proof.session == >::current_index() { + >::key_owner(id, data.as_ref()).and_then(|owner| { T::FullIdentificationOf::convert(owner.clone()).and_then(move |id| { - let count = >::validators().len() as ValidatorCount; + let count = >::validators().len() as ValidatorCount; if count != proof.validator_count { return None @@ -374,7 +394,7 @@ pub(crate) mod tests { BasicExternalities, }; - type Historical = Module; + type Historical = Pallet; pub(crate) fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); @@ -386,7 +406,9 @@ pub(crate) mod tests { frame_system::Pallet::::inc_providers(k); } }); - crate::GenesisConfig:: { keys }.assimilate_storage(&mut t).unwrap(); + pallet_session::GenesisConfig:: { keys } + .assimilate_storage(&mut t) + .unwrap(); sp_io::TestExternalities::new(t) } @@ -436,27 +458,27 @@ pub(crate) mod tests { Session::on_initialize(i); } - assert_eq!(StoredRange::get(), Some((0, 100))); + assert_eq!(>::get(), Some((0, 100))); for i in 0..100 { assert!(Historical::historical_root(i).is_some()) } Historical::prune_up_to(10); - assert_eq!(StoredRange::get(), Some((10, 100))); + assert_eq!(>::get(), Some((10, 100))); Historical::prune_up_to(9); - assert_eq!(StoredRange::get(), Some((10, 100))); + assert_eq!(>::get(), Some((10, 100))); for i in 10..100 { assert!(Historical::historical_root(i).is_some()) } Historical::prune_up_to(99); - assert_eq!(StoredRange::get(), Some((99, 100))); + assert_eq!(>::get(), Some((99, 100))); Historical::prune_up_to(100); - assert_eq!(StoredRange::get(), None); + assert_eq!(>::get(), None); for i in 99..199u64 { set_next_validators(vec![i]); @@ -466,14 +488,14 @@ pub(crate) mod tests { Session::on_initialize(i); } - assert_eq!(StoredRange::get(), Some((100, 200))); + assert_eq!(>::get(), Some((100, 200))); for i in 100..200 { assert!(Historical::historical_root(i).is_some()) } Historical::prune_up_to(9999); - assert_eq!(StoredRange::get(), None); + assert_eq!(>::get(), None); for i in 100..200 { assert!(Historical::historical_root(i).is_none()) diff --git a/frame/session/src/historical/offchain.rs b/frame/session/src/historical/offchain.rs index b646ecc2764f7..0b292b57658d0 100644 --- a/frame/session/src/historical/offchain.rs +++ b/frame/session/src/historical/offchain.rs @@ -140,7 +140,7 @@ pub fn keep_newest(n_to_keep: usize) { mod tests { use super::*; use crate::{ - historical::{onchain, Module}, + historical::{onchain, Pallet}, mock::{force_new_session, set_next_validators, Session, System, Test, NEXT_VALIDATORS}, }; @@ -156,7 +156,7 @@ mod tests { BasicExternalities, }; - type Historical = Module; + type Historical = Pallet; pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default() From 2d98c6f7d353cad4d47dc401094ebd7d98174b27 Mon Sep 17 00:00:00 2001 From: koushiro Date: Tue, 28 Sep 2021 15:04:20 +0800 Subject: [PATCH 2/8] pallet-session: Migrate the historical part to the new pallet macro Signed-off-by: koushiro --- frame/session/src/historical/mod.rs | 2 +- frame/session/src/lib.rs | 1 + frame/session/src/migrations/mod.rs | 24 ++++ frame/session/src/migrations/v1.rs | 211 ++++++++++++++++++++++++++++ frame/session/src/tests.rs | 27 ++++ 5 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 frame/session/src/migrations/mod.rs create mode 100644 frame/session/src/migrations/v1.rs diff --git a/frame/session/src/historical/mod.rs b/frame/session/src/historical/mod.rs index dc8876f482192..b5b90468697db 100644 --- a/frame/session/src/historical/mod.rs +++ b/frame/session/src/historical/mod.rs @@ -59,7 +59,7 @@ pub mod pallet { use frame_support::pallet_prelude::*; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index 2742d302ce439..076e3e22a7e88 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -108,6 +108,7 @@ #[cfg(feature = "historical")] pub mod historical; +pub mod migrations; #[cfg(test)] mod mock; #[cfg(test)] diff --git a/frame/session/src/migrations/mod.rs b/frame/session/src/migrations/mod.rs new file mode 100644 index 0000000000000..98ab06fce5d61 --- /dev/null +++ b/frame/session/src/migrations/mod.rs @@ -0,0 +1,24 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/// Version 1. +/// +/// For backward compatability reasons, session historical pallet uses `Session` +/// for storage module prefix before calling this migration. +/// After calling this migration, it will get replaced with own storage identifier. +#[cfg(feature = "historical")] +pub mod v1; diff --git a/frame/session/src/migrations/v1.rs b/frame/session/src/migrations/v1.rs new file mode 100644 index 0000000000000..6da8842fc3316 --- /dev/null +++ b/frame/session/src/migrations/v1.rs @@ -0,0 +1,211 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use sp_io::hashing::twox_128; +use sp_std::str; + +use frame_support::{ + storage::{generator::StorageValue, StoragePrefixedMap}, + traits::{ + Get, GetStorageVersion, PalletInfoAccess, StorageVersion, + STORAGE_VERSION_STORAGE_KEY_POSTFIX, + }, + weights::Weight, +}; + +use crate::historical as pallet_session_historical; + +/// Migrate the entire storage of this pallet to a new prefix. +/// +/// This new prefix must be the same as the one set in construct_runtime. +/// +/// The migration will look into the storage version in order not to trigger a migration on an up +/// to date storage. Thus the on chain storage version must be less than 1 in order to trigger the +/// migration. +pub fn migrate< + T: pallet_session_historical::Config, + P: GetStorageVersion + PalletInfoAccess, + N: AsRef, +>( + old_pallet_name: N, +) -> Weight { + let old_pallet_name = old_pallet_name.as_ref(); + let new_pallet_name =

::name(); + + if new_pallet_name == old_pallet_name { + log::info!( + target: "runtime::session_historical", + "New pallet name is equal to the old prefix. No migration needs to be done.", + ); + return 0 + } + + let on_chain_storage_version =

::on_chain_storage_version(); + log::info!( + target: "runtime::session_historical", + "Running migration to v1 for session_historical with storage version {:?}", + on_chain_storage_version, + ); + + if on_chain_storage_version < 1 { + let storage_prefix = pallet_session_historical::HistoricalSessions::::storage_prefix(); + frame_support::storage::migration::move_storage_from_pallet( + storage_prefix, + old_pallet_name.as_bytes(), + new_pallet_name.as_bytes(), + ); + log_migration("migration", storage_prefix, old_pallet_name, new_pallet_name); + + let storage_prefix = pallet_session_historical::StoredRange::::storage_prefix(); + frame_support::storage::migration::move_storage_from_pallet( + storage_prefix, + old_pallet_name.as_bytes(), + new_pallet_name.as_bytes(), + ); + log_migration("migration", storage_prefix, old_pallet_name, new_pallet_name); + + StorageVersion::new(1).put::

(); + ::BlockWeights::get().max_block + } else { + log::warn!( + target: "runtime::session_historical", + "Attempted to apply migration to v1 but failed because storage version is {:?}", + on_chain_storage_version, + ); + 0 + } +} + +/// Some checks prior to migration. This can be linked to +/// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing. +/// +/// Panics if anything goes wrong. +pub fn pre_migrate< + T: pallet_session_historical::Config, + P: GetStorageVersion + PalletInfoAccess, + N: AsRef, +>( + old_pallet_name: N, +) { + let old_pallet_name = old_pallet_name.as_ref(); + let new_pallet_name =

::name(); + + let storage_prefix_historical_sessions = + pallet_session_historical::HistoricalSessions::::storage_prefix(); + let storage_prefix_stored_range = pallet_session_historical::StoredRange::::storage_prefix(); + + log_migration( + "pre-migration", + storage_prefix_historical_sessions, + old_pallet_name, + new_pallet_name, + ); + log_migration("pre-migration", storage_prefix_stored_range, old_pallet_name, new_pallet_name); + + if new_pallet_name == old_pallet_name { + return + } + + let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); + let storage_version_key = twox_128(STORAGE_VERSION_STORAGE_KEY_POSTFIX); + + let mut new_pallet_prefix_iter = frame_support::storage::KeyPrefixIterator::new( + new_pallet_prefix.to_vec(), + new_pallet_prefix.to_vec(), + |key| Ok(key.to_vec()), + ); + + // Ensure nothing except the storage_version_key is stored in the new prefix. + assert!(new_pallet_prefix_iter.all(|key| key == storage_version_key)); + + assert!(

::on_chain_storage_version() < 1); +} + +/// Some checks for after migration. This can be linked to +/// [`frame_support::traits::OnRuntimeUpgrade::post_upgrade`] for further testing. +/// +/// Panics if anything goes wrong. +pub fn post_migrate< + T: pallet_session_historical::Config, + P: GetStorageVersion + PalletInfoAccess, + N: AsRef, +>( + old_pallet_name: N, +) { + let old_pallet_name = old_pallet_name.as_ref(); + let new_pallet_name =

::name(); + + let storage_prefix_historical_sessions = + pallet_session_historical::HistoricalSessions::::storage_prefix(); + let storage_prefix_stored_range = pallet_session_historical::StoredRange::::storage_prefix(); + + log_migration( + "post-migration", + storage_prefix_historical_sessions, + old_pallet_name, + new_pallet_name, + ); + log_migration("post-migration", storage_prefix_stored_range, old_pallet_name, new_pallet_name); + + if new_pallet_name == old_pallet_name { + return + } + + // Assert that no `HistoricalSessions` and `StoredRange` storages remains at the old prefix. + let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); + let old_historical_sessions_key = + [&old_pallet_prefix, &twox_128(storage_prefix_historical_sessions)[..]].concat(); + let old_historical_sessions_key_iter = frame_support::storage::KeyPrefixIterator::new( + old_historical_sessions_key.to_vec(), + old_historical_sessions_key.to_vec(), + |_| Ok(()), + ); + assert_eq!(old_historical_sessions_key_iter.count(), 0); + + let old_stored_range_key = + [&old_pallet_prefix, &twox_128(storage_prefix_stored_range)[..]].concat(); + let old_stored_range_key_iter = frame_support::storage::KeyPrefixIterator::new( + old_stored_range_key.to_vec(), + old_stored_range_key.to_vec(), + |_| Ok(()), + ); + assert_eq!(old_stored_range_key_iter.count(), 0); + + // Assert that the `HistoricalSessions` and `StoredRange` storages (if they exist) have been + // moved to the new prefix. + // NOTE: storage_version_key is already in the new prefix. + let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); + let new_pallet_prefix_iter = frame_support::storage::KeyPrefixIterator::new( + new_pallet_prefix.to_vec(), + new_pallet_prefix.to_vec(), + |_| Ok(()), + ); + assert!(new_pallet_prefix_iter.count() >= 1); + + assert_eq!(

::on_chain_storage_version(), 1); +} + +fn log_migration(stage: &str, storage_prefix: &[u8], old_pallet_name: &str, new_pallet_name: &str) { + log::info!( + target: "runtime::session_historical", + "{} prefix of storage '{}': '{}' ==> '{}'", + stage, + str::from_utf8(storage_prefix).unwrap_or(""), + old_pallet_name, + new_pallet_name, + ); +} diff --git a/frame/session/src/tests.rs b/frame/session/src/tests.rs index 47152042d204f..400991e8ce921 100644 --- a/frame/session/src/tests.rs +++ b/frame/session/src/tests.rs @@ -427,3 +427,30 @@ fn upgrade_keys() { } }) } + +#[cfg(feature = "historical")] +#[test] +fn test_migration_v1() { + use crate::{ + historical::{HistoricalSessions, StoredRange}, + mock::Historical, + }; + use frame_support::traits::PalletInfoAccess; + + new_test_ext().execute_with(|| { + assert!(>::iter_values().count() > 0); + assert!(>::exists()); + + let old_pallet = "Session"; + let new_pallet = ::name(); + frame_support::storage::migration::move_pallet( + new_pallet.as_bytes(), + old_pallet.as_bytes(), + ); + StorageVersion::new(0).put::(); + + crate::migrations::v1::pre_migrate::(old_pallet); + crate::migrations::v1::migrate::(old_pallet); + crate::migrations::v1::post_migrate::(old_pallet); + }); +} From c4372654f49151a244a574b901fc7214044bbb78 Mon Sep 17 00:00:00 2001 From: koushiro Date: Wed, 29 Sep 2021 16:35:13 +0800 Subject: [PATCH 3/8] Fix staking test runtime Signed-off-by: koushiro --- frame/staking/src/mock.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 06c9be9c01e11..853479034c71c 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -104,6 +104,7 @@ frame_support::construct_runtime!( Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, Staking: pallet_staking::{Pallet, Call, Config, Storage, Event}, Session: pallet_session::{Pallet, Call, Storage, Event, Config}, + Historical: pallet_session::historical::{Pallet, Storage}, BagsList: pallet_bags_list::{Pallet, Call, Storage, Event}, } ); From 6d71796f5a5c75621f6f7fe808c9e3abea580b20 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Thu, 28 Oct 2021 11:57:09 +0200 Subject: [PATCH 4/8] Update frame/session/src/historical/mod.rs --- frame/session/src/historical/mod.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/frame/session/src/historical/mod.rs b/frame/session/src/historical/mod.rs index b5b90468697db..c548fa222b44c 100644 --- a/frame/session/src/historical/mod.rs +++ b/frame/session/src/historical/mod.rs @@ -92,15 +92,6 @@ pub mod pallet { #[pallet::storage] pub type StoredRange = StorageValue<_, (SessionIndex, SessionIndex), OptionQuery>; - /// Deprecated. - #[pallet::storage] - pub type CachedObsolete = StorageMap< - _, - Twox64Concat, - SessionIndex, - Vec<(T::ValidatorId, T::FullIdentification)>, - OptionQuery, - >; } impl Pallet { From 9202ea49ea175ef50b71b0c567b55c747eba83d1 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Thu, 28 Oct 2021 11:57:27 +0200 Subject: [PATCH 5/8] Update frame/session/src/historical/mod.rs --- frame/session/src/historical/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/session/src/historical/mod.rs b/frame/session/src/historical/mod.rs index c548fa222b44c..a3e64f4f9efa4 100644 --- a/frame/session/src/historical/mod.rs +++ b/frame/session/src/historical/mod.rs @@ -91,7 +91,6 @@ pub mod pallet { /// The range of historical sessions we store. [first, last) #[pallet::storage] pub type StoredRange = StorageValue<_, (SessionIndex, SessionIndex), OptionQuery>; - } impl Pallet { From bc2655dcc3b343aeaa7b146309220446885ae693 Mon Sep 17 00:00:00 2001 From: koushiro Date: Thu, 28 Oct 2021 18:16:13 +0800 Subject: [PATCH 6/8] update migration doc Signed-off-by: koushiro --- frame/session/src/migrations/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/session/src/migrations/mod.rs b/frame/session/src/migrations/mod.rs index 98ab06fce5d61..ccc5ee3c2e525 100644 --- a/frame/session/src/migrations/mod.rs +++ b/frame/session/src/migrations/mod.rs @@ -17,8 +17,8 @@ /// Version 1. /// -/// For backward compatability reasons, session historical pallet uses `Session` -/// for storage module prefix before calling this migration. -/// After calling this migration, it will get replaced with own storage identifier. +/// In version 0 session historical pallet uses `Session` for storage module prefix. +/// In version 1 it uses its name as configured in `construct_runtime`. +/// This migration moves session historical pallet storages from old prefix to new prefix. #[cfg(feature = "historical")] pub mod v1; From 3d98af4fee3baacec18d167f78c7aae2fc559fc2 Mon Sep 17 00:00:00 2001 From: koushiro Date: Thu, 28 Oct 2021 22:58:03 +0800 Subject: [PATCH 7/8] use hardcoded prefix for migration v1 Signed-off-by: koushiro --- frame/session/src/migrations/v1.rs | 51 ++++++++++-------------------- frame/session/src/tests.rs | 6 ++-- 2 files changed, 20 insertions(+), 37 deletions(-) diff --git a/frame/session/src/migrations/v1.rs b/frame/session/src/migrations/v1.rs index 6da8842fc3316..1de199fe7bedd 100644 --- a/frame/session/src/migrations/v1.rs +++ b/frame/session/src/migrations/v1.rs @@ -29,6 +29,8 @@ use frame_support::{ use crate::historical as pallet_session_historical; +const OLD_PREFIX: &str = "Session"; + /// Migrate the entire storage of this pallet to a new prefix. /// /// This new prefix must be the same as the one set in construct_runtime. @@ -36,17 +38,11 @@ use crate::historical as pallet_session_historical; /// The migration will look into the storage version in order not to trigger a migration on an up /// to date storage. Thus the on chain storage version must be less than 1 in order to trigger the /// migration. -pub fn migrate< - T: pallet_session_historical::Config, - P: GetStorageVersion + PalletInfoAccess, - N: AsRef, ->( - old_pallet_name: N, +pub fn migrate( ) -> Weight { - let old_pallet_name = old_pallet_name.as_ref(); let new_pallet_name =

::name(); - if new_pallet_name == old_pallet_name { + if new_pallet_name == OLD_PREFIX { log::info!( target: "runtime::session_historical", "New pallet name is equal to the old prefix. No migration needs to be done.", @@ -65,18 +61,18 @@ pub fn migrate< let storage_prefix = pallet_session_historical::HistoricalSessions::::storage_prefix(); frame_support::storage::migration::move_storage_from_pallet( storage_prefix, - old_pallet_name.as_bytes(), + OLD_PREFIX.as_bytes(), new_pallet_name.as_bytes(), ); - log_migration("migration", storage_prefix, old_pallet_name, new_pallet_name); + log_migration("migration", storage_prefix, OLD_PREFIX, new_pallet_name); let storage_prefix = pallet_session_historical::StoredRange::::storage_prefix(); frame_support::storage::migration::move_storage_from_pallet( storage_prefix, - old_pallet_name.as_bytes(), + OLD_PREFIX.as_bytes(), new_pallet_name.as_bytes(), ); - log_migration("migration", storage_prefix, old_pallet_name, new_pallet_name); + log_migration("migration", storage_prefix, OLD_PREFIX, new_pallet_name); StorageVersion::new(1).put::

(); ::BlockWeights::get().max_block @@ -97,26 +93,17 @@ pub fn migrate< pub fn pre_migrate< T: pallet_session_historical::Config, P: GetStorageVersion + PalletInfoAccess, - N: AsRef, ->( - old_pallet_name: N, -) { - let old_pallet_name = old_pallet_name.as_ref(); +>() { let new_pallet_name =

::name(); let storage_prefix_historical_sessions = pallet_session_historical::HistoricalSessions::::storage_prefix(); let storage_prefix_stored_range = pallet_session_historical::StoredRange::::storage_prefix(); - log_migration( - "pre-migration", - storage_prefix_historical_sessions, - old_pallet_name, - new_pallet_name, - ); - log_migration("pre-migration", storage_prefix_stored_range, old_pallet_name, new_pallet_name); + log_migration("pre-migration", storage_prefix_historical_sessions, OLD_PREFIX, new_pallet_name); + log_migration("pre-migration", storage_prefix_stored_range, OLD_PREFIX, new_pallet_name); - if new_pallet_name == old_pallet_name { + if new_pallet_name == OLD_PREFIX { return } @@ -142,11 +129,7 @@ pub fn pre_migrate< pub fn post_migrate< T: pallet_session_historical::Config, P: GetStorageVersion + PalletInfoAccess, - N: AsRef, ->( - old_pallet_name: N, -) { - let old_pallet_name = old_pallet_name.as_ref(); +>() { let new_pallet_name =

::name(); let storage_prefix_historical_sessions = @@ -156,17 +139,17 @@ pub fn post_migrate< log_migration( "post-migration", storage_prefix_historical_sessions, - old_pallet_name, + OLD_PREFIX, new_pallet_name, ); - log_migration("post-migration", storage_prefix_stored_range, old_pallet_name, new_pallet_name); + log_migration("post-migration", storage_prefix_stored_range, OLD_PREFIX, new_pallet_name); - if new_pallet_name == old_pallet_name { + if new_pallet_name == OLD_PREFIX { return } // Assert that no `HistoricalSessions` and `StoredRange` storages remains at the old prefix. - let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); + let old_pallet_prefix = twox_128(OLD_PREFIX.as_bytes()); let old_historical_sessions_key = [&old_pallet_prefix, &twox_128(storage_prefix_historical_sessions)[..]].concat(); let old_historical_sessions_key_iter = frame_support::storage::KeyPrefixIterator::new( diff --git a/frame/session/src/tests.rs b/frame/session/src/tests.rs index 083555e18cbc1..cc0606edf499d 100644 --- a/frame/session/src/tests.rs +++ b/frame/session/src/tests.rs @@ -475,8 +475,8 @@ fn test_migration_v1() { ); StorageVersion::new(0).put::(); - crate::migrations::v1::pre_migrate::(old_pallet); - crate::migrations::v1::migrate::(old_pallet); - crate::migrations::v1::post_migrate::(old_pallet); + crate::migrations::v1::pre_migrate::(); + crate::migrations::v1::migrate::(); + crate::migrations::v1::post_migrate::(); }); } From 97b8affb5729e09620860c44f4b36bdc2ef51dfd Mon Sep 17 00:00:00 2001 From: koushiro Date: Tue, 16 Nov 2021 13:32:30 +0800 Subject: [PATCH 8/8] cargo +nightly-2021-11-08 fmt Signed-off-by: koushiro --- client/consensus/babe/src/verification.rs | 4 ++-- client/network/src/protocol/notifications/behaviour.rs | 4 ++-- client/network/src/protocol/sync/blocks.rs | 2 +- client/network/src/service/tests.rs | 2 +- client/network/src/transactions.rs | 4 ++-- frame/election-provider-multi-phase/src/lib.rs | 2 +- frame/support/procedural/src/pallet/parse/pallet_struct.rs | 4 ++-- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/client/consensus/babe/src/verification.rs b/client/consensus/babe/src/verification.rs index af118312dd07c..1554fa6de31be 100644 --- a/client/consensus/babe/src/verification.rs +++ b/client/consensus/babe/src/verification.rs @@ -114,7 +114,7 @@ where ); check_secondary_plain_header::(pre_hash, secondary, sig, &epoch)?; - } + }, PreDigest::SecondaryVRF(secondary) if epoch.config.allowed_slots.is_secondary_vrf_slots_allowed() => { @@ -125,7 +125,7 @@ where ); check_secondary_vrf_header::(pre_hash, secondary, sig, &epoch)?; - } + }, _ => return Err(babe_err(Error::SecondarySlotAssignmentsDisabled)), } diff --git a/client/network/src/protocol/notifications/behaviour.rs b/client/network/src/protocol/notifications/behaviour.rs index 01138e3207570..f66f1fbe9e95a 100644 --- a/client/network/src/protocol/notifications/behaviour.rs +++ b/client/network/src/protocol/notifications/behaviour.rs @@ -712,7 +712,7 @@ impl Notifications { timer: delay_id, timer_deadline: *backoff, }; - } + }, // Disabled => Enabled PeerState::Disabled { mut connections, backoff_until } => { @@ -2085,7 +2085,7 @@ impl NetworkBehaviour for Notifications { .boxed(), ); } - } + }, // We intentionally never remove elements from `delays`, and it may // thus contain obsolete entries. This is a normal situation. diff --git a/client/network/src/protocol/sync/blocks.rs b/client/network/src/protocol/sync/blocks.rs index 30ba7ffafeffc..ce4535dc0b45f 100644 --- a/client/network/src/protocol/sync/blocks.rs +++ b/client/network/src/protocol/sync/blocks.rs @@ -203,7 +203,7 @@ impl BlockCollection { { *downloading -= 1; false - } + }, Some(&mut BlockRangeState::Downloading { .. }) => true, _ => false, }; diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index 69b172d07edfe..87e481dc87f2d 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -530,7 +530,7 @@ fn fallback_name_working() { { assert_eq!(negotiated_fallback, Some(PROTOCOL_NAME)); break - } + }, _ => {}, }; } diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index 99350f603a375..6d190651160f0 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -336,13 +336,13 @@ impl TransactionsHandler { }, ); debug_assert!(_was_in.is_none()); - } + }, Event::NotificationStreamClosed { remote, protocol } if protocol == self.protocol_name => { let _peer = self.peers.remove(&remote); debug_assert!(_peer.is_some()); - } + }, Event::NotificationsReceived { remote, messages } => { for (protocol, message) in messages { diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 80a13aa99fb70..4c4de82af592f 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -772,7 +772,7 @@ pub mod pallet { Self::on_initialize_open_unsigned(enabled, now); T::WeightInfo::on_initialize_open_unsigned() } - } + }, _ => T::WeightInfo::on_initialize_nothing(), } } diff --git a/frame/support/procedural/src/pallet/parse/pallet_struct.rs b/frame/support/procedural/src/pallet/parse/pallet_struct.rs index 278f46e13818e..c528faf669ee3 100644 --- a/frame/support/procedural/src/pallet/parse/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/parse/pallet_struct.rs @@ -130,12 +130,12 @@ impl PalletStructDef { if generate_storage_info.is_none() => { generate_storage_info = Some(span); - } + }, PalletStructAttr::StorageVersion { storage_version, .. } if storage_version_found.is_none() => { storage_version_found = Some(storage_version); - } + }, attr => { let msg = "Unexpected duplicated attribute"; return Err(syn::Error::new(attr.span(), msg))