From 1f9d09d597a250e11f8f742b2d6d2cc9ae911c6c Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 20 Jan 2020 17:26:53 +0100 Subject: [PATCH] Pallet session new API (#4609) * Initial work * Fix most things * fix test * fix old comment * migration * fix * remove useless stuff * fix * less spaghetti implementation * fix initial session * fix --- bin/node/runtime/src/lib.rs | 3 +- frame/authority-discovery/src/lib.rs | 10 +-- frame/babe/src/mock.rs | 3 +- frame/im-online/src/mock.rs | 22 ++--- frame/session/src/historical.rs | 129 ++++++++++++--------------- frame/session/src/lib.rs | 84 ++++++++--------- frame/session/src/mock.rs | 25 +++--- frame/staking/src/lib.rs | 63 +++++++------ frame/staking/src/mock.rs | 3 +- frame/staking/src/tests.rs | 12 +-- 10 files changed, 159 insertions(+), 195 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index d769be7b505d5..e158cbe2cbb88 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -236,14 +236,13 @@ parameter_types! { } impl pallet_session::Trait for Runtime { - type OnSessionEnding = Staking; + type SessionManager = Staking; type SessionHandler = ::KeyTypeIdProviders; type ShouldEndSession = Babe; type Event = Event; type Keys = SessionKeys; type ValidatorId = ::AccountId; type ValidatorIdOf = pallet_staking::StashOf; - type SelectInitialValidators = Staking; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/authority-discovery/src/lib.rs b/frame/authority-discovery/src/lib.rs index b3911859f4787..c427043397075 100644 --- a/frame/authority-discovery/src/lib.rs +++ b/frame/authority-discovery/src/lib.rs @@ -109,26 +109,18 @@ mod tests { pub struct Test; impl Trait for Test {} - pub struct TestOnSessionEnding; - impl pallet_session::OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(_: SessionIndex, _: SessionIndex) -> Option> { - None - } - } - parameter_types! { pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(33); } impl pallet_session::Trait for Test { - type OnSessionEnding = TestOnSessionEnding; + type SessionManager = (); type Keys = UintAuthorityId; type ShouldEndSession = pallet_session::PeriodicSessions; type SessionHandler = TestSessionHandler; type Event = (); type ValidatorId = AuthorityId; type ValidatorIdOf = ConvertInto; - type SelectInitialValidators = (); type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 3f0c42a6cb9c6..e65f305dc4de2 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -79,9 +79,8 @@ impl pallet_session::Trait for Test { type ValidatorId = ::AccountId; type ShouldEndSession = Babe; type SessionHandler = (Babe,Babe,); - type OnSessionEnding = (); + type SessionManager = (); type ValidatorIdOf = (); - type SelectInitialValidators = (); type Keys = MockSessionKeys; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/im-online/src/mock.rs b/frame/im-online/src/mock.rs index 387ff47c09fa5..5c428c38582ff 100644 --- a/frame/im-online/src/mock.rs +++ b/frame/im-online/src/mock.rs @@ -43,28 +43,25 @@ thread_local! { pub static VALIDATORS: RefCell>> = RefCell::new(Some(vec![1, 2, 3])); } -pub struct TestOnSessionEnding; -impl pallet_session::OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(_ending_index: SessionIndex, _will_apply_at: SessionIndex) - -> Option> - { +pub struct TestSessionManager; +impl pallet_session::SessionManager for TestSessionManager { + fn new_session(_new_index: SessionIndex) -> Option> { VALIDATORS.with(|l| l.borrow_mut().take()) } + fn end_session(_: SessionIndex) {} } -impl pallet_session::historical::OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(_ending_index: SessionIndex, _will_apply_at: SessionIndex) - -> Option<(Vec, Vec<(u64, u64)>)> - { +impl pallet_session::historical::SessionManager for TestSessionManager { + fn new_session(_new_index: SessionIndex) -> Option> { VALIDATORS.with(|l| l .borrow_mut() .take() .map(|validators| { - let full_identification = validators.iter().map(|v| (*v, *v)).collect(); - (validators, full_identification) + validators.iter().map(|v| (*v, *v)).collect() }) ) } + fn end_session(_: SessionIndex) {} } /// An extrinsic type used for tests. @@ -131,13 +128,12 @@ parameter_types! { impl pallet_session::Trait for Runtime { type ShouldEndSession = pallet_session::PeriodicSessions; - type OnSessionEnding = pallet_session::historical::NoteHistoricalRoot; + type SessionManager = pallet_session::historical::NoteHistoricalRoot; type SessionHandler = (ImOnline, ); type ValidatorId = u64; type ValidatorIdOf = ConvertInto; type Keys = UintAuthorityId; type Event = (); - type SelectInitialValidators = (); type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/session/src/historical.rs b/frame/session/src/historical.rs index 939e6133e8520..e67a2ae1b0eab 100644 --- a/frame/session/src/historical.rs +++ b/frame/session/src/historical.rs @@ -48,8 +48,7 @@ pub trait Trait: super::Trait { /// validator, since they may be outdated by the time this is queried from a /// historical trie. /// - /// This mapping is expected to remain stable in between calls to - /// `Self::OnSessionEnding::on_session_ending` which return new validators. + /// It must return the identification for the current session index. type FullIdentificationOf: Convert>; } @@ -57,16 +56,19 @@ 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 SessionIndex => Option<(T::Hash, ValidatorCount)>; - /// Queued full identifications for queued sessions whose validators have become obsolete. - CachedObsolete get(fn cached_obsolete): map SessionIndex - => Option>; /// The range of historical sessions we store. [first, last) StoredRange: Option<(SessionIndex, SessionIndex)>; + /// Deprecated. + CachedObsolete: map SessionIndex => Option>; } } decl_module! { - pub struct Module for enum Call where origin: T::Origin { } + pub struct Module for enum Call where origin: T::Origin { + fn on_initialize(_n: T::BlockNumber) { + CachedObsolete::::remove_all(); + } + } } impl Module { @@ -97,52 +99,52 @@ impl Module { } } -/// Specialization of the crate-level `OnSessionEnding` which returns the old -/// set of full identification when changing the validator set. -pub trait OnSessionEnding: crate::OnSessionEnding { - /// If there was a validator set change, its returns the set of new validators along with the - /// old validators and their full identifications. - fn on_session_ending(ending: SessionIndex, will_apply_at: SessionIndex) - -> Option<(Vec, Vec<(ValidatorId, FullIdentification)>)>; +/// Specialization of the crate-level `SessionManager` which returns the set of full identification +/// when creating a new session. +pub trait SessionManager: crate::SessionManager { + /// If there was a validator set change, its returns the set of new validators along with their + /// full identifications. + fn new_session(new_index: SessionIndex) -> Option>; + fn end_session(end_index: SessionIndex); } -/// An `OnSessionEnding` implementation that wraps an inner `I` and also +/// An `SessionManager` implementation that wraps an inner `I` and also /// sets the historical trie root of the ending session. pub struct NoteHistoricalRoot(sp_std::marker::PhantomData<(T, I)>); -impl crate::OnSessionEnding for NoteHistoricalRoot - where I: OnSessionEnding +impl crate::SessionManager for NoteHistoricalRoot + where I: SessionManager { - fn on_session_ending(ending: SessionIndex, applied_at: SessionIndex) -> Option> { + fn new_session(new_index: SessionIndex) -> Option> { StoredRange::mutate(|range| { - range.get_or_insert_with(|| (ending, ending)).1 = ending + 1; + range.get_or_insert_with(|| (new_index, new_index)).1 = new_index + 1; }); - // do all of this _before_ calling the other `on_session_ending` impl - // so that we have e.g. correct exposures from the _current_. + let new_validators_and_id = >::new_session(new_index); + let new_validators = new_validators_and_id.as_ref().map(|new_validators| { + new_validators.iter().map(|(v, _id)| v.clone()).collect() + }); - let count = >::validators().len() as u32; - match ProvingTrie::::generate_for(ending) { - Ok(trie) => >::insert(ending, &(trie.root, count)), - Err(reason) => { - print("Failed to generate historical ancestry-inclusion proof."); - print(reason); + if let Some(new_validators) = new_validators_and_id { + let count = new_validators.len() as u32; + match ProvingTrie::::generate_for(new_validators) { + Ok(trie) => >::insert(new_index, &(trie.root, count)), + Err(reason) => { + print("Failed to generate historical ancestry-inclusion proof."); + print(reason); + } + }; + } else { + let previous_index = new_index.saturating_sub(1); + if let Some(previous_session) = >::get(previous_index) { + >::insert(new_index, previous_session); } - }; - - // trie has been generated for this session, so it's no longer queued. - >::remove(&ending); - - let (new_validators, old_exposures) = >::on_session_ending(ending, applied_at)?; - - // every session from `ending+1 .. applied_at` now has obsolete `FullIdentification` - // now that a new validator election has occurred. - // we cache these in the trie until those sessions themselves end. - for obsolete in (ending + 1) .. applied_at { - >::insert(obsolete, &old_exposures); } - Some(new_validators) + new_validators + } + fn end_session(end_index: SessionIndex) { + >::end_session(end_index) } } @@ -158,15 +160,14 @@ pub struct ProvingTrie { } impl ProvingTrie { - fn generate_for(now: SessionIndex) -> Result { + fn generate_for(validators: I) -> Result + where I: IntoIterator + { let mut db = MemoryDB::default(); let mut root = Default::default(); - fn build(root: &mut T::Hash, db: &mut MemoryDB>, validators: I) - -> Result<(), &'static str> - where I: IntoIterator)> { - let mut trie = TrieDBMut::new(db, root); + 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) { @@ -174,11 +175,7 @@ impl ProvingTrie { Some(k) => k, }; - let full_id = full_id.or_else(|| T::FullIdentificationOf::convert(validator.clone())); - let full_id = match full_id { - None => return Err("no full identification for a current validator"), - Some(full) => (validator, full), - }; + let full_id = (validator, full_id); // map each key to the owner index. for key_id in T::Keys::key_ids() { @@ -194,17 +191,6 @@ impl ProvingTrie { let _ = i.using_encoded(|k| full_id.using_encoded(|v| trie.insert(k, v))) .map_err(|_| "failed to insert into trie")?; } - - Ok(()) - } - - // if the current session's full identifications are obsolete but cached, - // use those. - if let Some(obsolete) = >::get(&now) { - build::(&mut root, &mut db, obsolete.into_iter().map(|(v, f)| (v, Some(f))))? - } else { - let validators = >::validators(); - build::(&mut root, &mut db, validators.into_iter().map(|v| (v, None)))? } Ok(ProvingTrie { @@ -281,7 +267,12 @@ impl> frame_support::traits::KeyOwnerProofSystem<(KeyTy fn prove(key: (KeyTypeId, D)) -> Option { let session = >::current_index(); - let trie = ProvingTrie::::generate_for(session).ok()?; + let validators = >::validators().into_iter() + .filter_map(|validator| { + T::FullIdentificationOf::convert(validator.clone()) + .map(|full_id| (validator, full_id)) + }); + let trie = ProvingTrie::::generate_for(validators).ok()?; let (id, data) = key; @@ -348,13 +339,9 @@ mod tests { set_next_validators(vec![1, 2, 4]); force_new_session(); - assert!(Historical::cached_obsolete(&(proof.session + 1)).is_none()); - System::set_block_number(2); Session::on_initialize(2); - assert!(Historical::cached_obsolete(&(proof.session + 1)).is_some()); - assert!(Historical::historical_root(proof.session).is_some()); assert!(Session::current_index() > proof.session); @@ -366,15 +353,13 @@ mod tests { force_new_session(); System::set_block_number(3); Session::on_initialize(3); - - assert!(Historical::cached_obsolete(&(proof.session + 1)).is_none()); }); } #[test] fn prune_up_to_works() { new_test_ext().execute_with(|| { - for i in 1..101u64 { + for i in 1..99u64 { set_next_validators(vec![i]); force_new_session(); @@ -385,7 +370,7 @@ mod tests { assert_eq!(StoredRange::get(), Some((0, 100))); - for i in 1..100 { + for i in 0..100 { assert!(Historical::historical_root(i).is_some()) } @@ -405,7 +390,7 @@ mod tests { Historical::prune_up_to(100); assert_eq!(StoredRange::get(), None); - for i in 101..201u64 { + for i in 99..199u64 { set_next_validators(vec![i]); force_new_session(); @@ -416,14 +401,14 @@ mod tests { assert_eq!(StoredRange::get(), Some((100, 200))); - for i in 101..200 { + for i in 100..200 { assert!(Historical::historical_root(i).is_some()) } Historical::prune_up_to(9999); assert_eq!(StoredRange::get(), None); - for i in 101..200 { + for i in 100..200 { assert!(Historical::historical_root(i).is_none()) } }); diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index 2200221af42fe..098b5330779ef 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -162,29 +162,30 @@ impl< } } -/// An event handler for when the session is ending. -/// TODO [slashing] consider renaming to OnSessionStarting -pub trait OnSessionEnding { - /// Handle the fact that the session is ending, and optionally provide the new validator set. +/// A trait for managing creation of new validator set. +pub trait SessionManager { + /// Plan a new session, and optionally provide the new validator set. /// /// Even if the validator-set is the same as before, if any underlying economic /// conditions have changed (i.e. stake-weights), the new validator set must be returned. /// This is necessary for consensus engines making use of the session module to /// issue a validator-set change so misbehavior can be provably associated with the new /// economic conditions as opposed to the old. + /// The returned validator set, if any, will not be applied until `new_index`. + /// `new_index` is strictly greater than from previous call. /// - /// `ending_index` is the index of the currently ending session. - /// The returned validator set, if any, will not be applied until `will_apply_at`. - /// `will_apply_at` is guaranteed to be at least `ending_index + 1`, since session indices don't - /// repeat, but it could be some time after in case we are staging authority set changes. - fn on_session_ending( - ending_index: SessionIndex, - will_apply_at: SessionIndex - ) -> Option>; + /// The first session start at index 0. + fn new_session(new_index: SessionIndex) -> Option>; + /// End the session. + /// + /// Because the session pallet can queue validator set the ending session can be lower than the + /// last new session index. + fn end_session(end_index: SessionIndex); } -impl OnSessionEnding for () { - fn on_session_ending(_: SessionIndex, _: SessionIndex) -> Option> { None } +impl SessionManager for () { + fn new_session(_: SessionIndex) -> Option> { None } + fn end_session(_: SessionIndex) {} } /// Handler for session lifecycle events. @@ -214,7 +215,7 @@ pub trait SessionHandler { /// A notification for end of the session. /// - /// Note it is triggered before any `OnSessionEnding` handlers, + /// Note it is triggered before any `SessionManager::end_session` handlers, /// so we can still affect the validator set. fn on_before_session_ending() {} @@ -248,7 +249,7 @@ pub trait OneSessionHandler: BoundToRuntimeAppPublic { /// A notification for end of the session. /// - /// Note it is triggered before any `OnSessionEnding` handlers, + /// Note it is triggered before any `SessionManager::end_session` handlers, /// so we can still affect the validator set. fn on_before_session_ending() {} @@ -318,21 +319,6 @@ impl SessionHandler for TestSessionHandler { fn on_disabled(_: usize) {} } -/// Handler for selecting the genesis validator set. -pub trait SelectInitialValidators { - /// Returns the initial validator set. If `None` is returned - /// all accounts that have session keys set in the genesis block - /// will be validators. - fn select_initial_validators() -> Option>; -} - -/// Implementation of `SelectInitialValidators` that does nothing. -impl SelectInitialValidators for () { - fn select_initial_validators() -> Option> { - None - } -} - impl ValidatorRegistration for Module { fn is_registered(id: &T::ValidatorId) -> bool { Self::load_keys(id).is_some() @@ -352,8 +338,8 @@ pub trait Trait: frame_system::Trait { /// Indicator for when to end the session. type ShouldEndSession: ShouldEndSession; - /// Handler for when a session is about to end. - type OnSessionEnding: OnSessionEnding; + /// Handler for managing new session. + type SessionManager: SessionManager; /// Handler when a session has changed. type SessionHandler: SessionHandler; @@ -366,9 +352,6 @@ pub trait Trait: frame_system::Trait { /// After the threshold is reached `disabled` method starts to return true, /// which in combination with `pallet_staking` forces a new era. type DisabledValidatorsThreshold: Get; - - /// Select initial validators. - type SelectInitialValidators: SelectInitialValidators; } const DEDUP_KEY_PREFIX: &[u8] = b":session:keys"; @@ -435,12 +418,19 @@ decl_storage! { .expect("genesis config must not contain duplicates; qed"); } - let initial_validators = T::SelectInitialValidators::select_initial_validators() - .unwrap_or_else(|| config.keys.iter().map(|(ref v, _)| v.clone()).collect()); + let initial_validators_0 = T::SessionManager::new_session(0) + .unwrap_or_else(|| { + frame_support::print("No initial validator provided by `SessionManager`, use \ + session config keys to generate initial validator set."); + config.keys.iter().map(|(ref v, _)| v.clone()).collect() + }); + assert!(!initial_validators_0.is_empty(), "Empty validator set for session 0 in genesis block!"); - assert!(!initial_validators.is_empty(), "Empty validator set in genesis block!"); + let initial_validators_1 = T::SessionManager::new_session(1) + .unwrap_or_else(|| initial_validators_0.clone()); + assert!(!initial_validators_1.is_empty(), "Empty validator set for session 1 in genesis block!"); - let queued_keys: Vec<_> = initial_validators + let queued_keys: Vec<_> = initial_validators_1 .iter() .cloned() .map(|v| ( @@ -452,7 +442,7 @@ decl_storage! { // Tell everyone about the genesis session keys T::SessionHandler::on_genesis_session::(&queued_keys); - >::put(initial_validators); + >::put(initial_validators_0); >::put(queued_keys); }); } @@ -545,10 +535,14 @@ impl Module { DisabledValidators::take(); } - let applied_at = session_index + 2; + T::SessionManager::end_session(session_index); + + // Increment session index. + let session_index = session_index + 1; + CurrentIndex::put(session_index); // Get next validator set. - let maybe_next_validators = T::OnSessionEnding::on_session_ending(session_index, applied_at); + let maybe_next_validators = T::SessionManager::new_session(session_index + 1); let (next_validators, next_identities_changed) = if let Some(validators) = maybe_next_validators { @@ -560,10 +554,6 @@ impl Module { (>::get(), false) }; - // Increment session index. - let session_index = session_index + 1; - CurrentIndex::put(session_index); - // Queue next session keys. let (queued_amalgamated, next_changed) = { // until we are certain there has been a change, iterate the prior diff --git a/frame/session/src/mock.rs b/frame/session/src/mock.rs index 7fe9cd01f43bf..ff84743a61596 100644 --- a/frame/session/src/mock.rs +++ b/frame/session/src/mock.rs @@ -88,9 +88,10 @@ impl SessionHandler for TestSessionHandler { } } -pub struct TestOnSessionEnding; -impl OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(_: SessionIndex, _: SessionIndex) -> Option> { +pub struct TestSessionManager; +impl SessionManager for TestSessionManager { + fn end_session(_: SessionIndex) {} + fn new_session(_: SessionIndex) -> Option> { if !TEST_SESSION_CHANGED.with(|l| *l.borrow()) { VALIDATORS.with(|v| { let mut v = v.borrow_mut(); @@ -108,14 +109,13 @@ impl OnSessionEnding for TestOnSessionEnding { } #[cfg(feature = "historical")] -impl crate::historical::OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(ending_index: SessionIndex, will_apply_at: SessionIndex) - -> Option<(Vec, Vec<(u64, u64)>)> +impl crate::historical::SessionManager for TestSessionManager { + fn end_session(_: SessionIndex) {} + fn new_session(new_index: SessionIndex) + -> Option> { - let pair_with_ids = |vals: &[u64]| vals.iter().map(|&v| (v, v)).collect::>(); - >::on_session_ending(ending_index, will_apply_at) - .map(|vals| (pair_with_ids(&vals), vals)) - .map(|(ids, vals)| (vals, ids)) + >::new_session(new_index) + .map(|vals| vals.into_iter().map(|val| (val, val)).collect()) } } @@ -190,15 +190,14 @@ parameter_types! { impl Trait for Test { type ShouldEndSession = TestShouldEndSession; #[cfg(feature = "historical")] - type OnSessionEnding = crate::historical::NoteHistoricalRoot; + type SessionManager = crate::historical::NoteHistoricalRoot; #[cfg(not(feature = "historical"))] - type OnSessionEnding = TestOnSessionEnding; + type SessionManager = TestSessionManager; type SessionHandler = TestSessionHandler; type ValidatorId = u64; type ValidatorIdOf = ConvertInto; type Keys = MockSessionKeys; type Event = (); - type SelectInitialValidators = (); type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index c547545c389a5..7f1ad37b32cf6 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -265,7 +265,7 @@ use frame_support::{ WithdrawReasons, OnUnbalanced, Imbalance, Get, Time } }; -use pallet_session::{historical::OnSessionEnding, SelectInitialValidators}; +use pallet_session::historical::SessionManager; use sp_runtime::{ Perbill, RuntimeDebug, @@ -575,8 +575,7 @@ impl SessionInterface<::AccountId> for T whe FullIdentificationOf = ExposureOf, >, T::SessionHandler: pallet_session::SessionHandler<::AccountId>, - T::OnSessionEnding: pallet_session::OnSessionEnding<::AccountId>, - T::SelectInitialValidators: pallet_session::SelectInitialValidators<::AccountId>, + T::SessionManager: pallet_session::SessionManager<::AccountId>, T::ValidatorIdOf: Convert<::AccountId, Option<::AccountId>> { fn disable_validator(validator: &::AccountId) -> Result { @@ -1346,11 +1345,8 @@ impl Module { imbalance } - /// Session has just ended. Provide the validator set for the next session if it's an era-end, along - /// with the exposure of the prior validator set. - fn new_session(session_index: SessionIndex) - -> Option<(Vec, Vec<(T::AccountId, Exposure>)>)> - { + /// Session has just ended. Provide the validator set for the next session if it's an era-end. + fn new_session(session_index: SessionIndex) -> Option> { let era_length = session_index.checked_sub(Self::current_era_start_session_index()).unwrap_or(0); match ForceEra::get() { Forcing::ForceNew => ForceEra::kill(), @@ -1358,12 +1354,17 @@ impl Module { Forcing::NotForcing if era_length >= T::SessionsPerEra::get() => (), _ => return None, } - let validators = T::SessionInterface::validators(); - let prior = validators.into_iter() - .map(|v| { let e = Self::stakers(&v); (v, e) }) - .collect(); - Self::new_era(session_index).map(move |new| (new, prior)) + Self::new_era(session_index) + } + + /// Initialise the first session (and consequently the first era) + fn initial_session() -> Option> { + // note: `CurrentEraStart` is set in `on_finalize` of the first block because now is not + // available yet. + CurrentEraStartSessionIndex::put(0); + BondedEras::mutate(|bonded| bonded.push((0, 0))); + Self::select_validators().1 } /// The era has changed - enact new staking set. @@ -1646,19 +1647,30 @@ impl Module { } } -impl pallet_session::OnSessionEnding for Module { - fn on_session_ending(_ending: SessionIndex, start_session: SessionIndex) -> Option> { +impl pallet_session::SessionManager for Module { + fn new_session(new_index: SessionIndex) -> Option> { Self::ensure_storage_upgraded(); - Self::new_session(start_session - 1).map(|(new, _old)| new) + if new_index == 0 { + return Self::initial_session(); + } + Self::new_session(new_index - 1) } + fn end_session(_end_index: SessionIndex) {} } -impl OnSessionEnding>> for Module { - fn on_session_ending(_ending: SessionIndex, start_session: SessionIndex) - -> Option<(Vec, Vec<(T::AccountId, Exposure>)>)> +impl SessionManager>> for Module { + fn new_session(new_index: SessionIndex) + -> Option>)>> { - Self::ensure_storage_upgraded(); - Self::new_session(start_session - 1) + >::new_session(new_index).map(|validators| { + validators.into_iter().map(|v| { + let exposure = >::get(&v); + (v, exposure) + }).collect() + }) + } + fn end_session(end_index: SessionIndex) { + >::end_session(end_index) } } @@ -1707,12 +1719,6 @@ impl Convert> } } -impl SelectInitialValidators for Module { - fn select_initial_validators() -> Option> { - >::select_validators().1 - } -} - /// This is intended to be used with `FilterHistoricalOffences`. impl OnOffenceHandler> for Module where T: pallet_session::Trait::AccountId>, @@ -1721,8 +1727,7 @@ impl OnOffenceHandler, >, T::SessionHandler: pallet_session::SessionHandler<::AccountId>, - T::OnSessionEnding: pallet_session::OnSessionEnding<::AccountId>, - T::SelectInitialValidators: pallet_session::SelectInitialValidators<::AccountId>, + T::SessionManager: pallet_session::SessionManager<::AccountId>, T::ValidatorIdOf: Convert<::AccountId, Option<::AccountId>> { fn on_offence( diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 3c238b56ed317..8aa20c19c6e29 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -162,14 +162,13 @@ parameter_types! { pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(25); } impl pallet_session::Trait for Test { - type OnSessionEnding = pallet_session::historical::NoteHistoricalRoot; + type SessionManager = pallet_session::historical::NoteHistoricalRoot; type Keys = UintAuthorityId; type ShouldEndSession = pallet_session::PeriodicSessions; type SessionHandler = TestSessionHandler; type Event = (); type ValidatorId = AccountId; type ValidatorIdOf = crate::StashOf; - type SelectInitialValidators = Staking; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index bb1fe32025ebc..43a2d0079164f 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -365,7 +365,7 @@ fn less_than_needed_candidates_works() { #[test] fn no_candidate_emergency_condition() { ExtBuilder::default() - .minimum_validator_count(10) + .minimum_validator_count(1) .validator_count(15) .num_validators(4) .validator_pool(true) @@ -374,21 +374,21 @@ fn no_candidate_emergency_condition() { .execute_with(|| { // initial validators assert_eq_uvec!(validator_controllers(), vec![10, 20, 30, 40]); + let prefs = ValidatorPrefs { commission: Perbill::one() }; + ::Validators::insert(11, prefs.clone()); // set the minimum validator count. ::MinimumValidatorCount::put(10); - ::ValidatorCount::put(15); - assert_eq!(Staking::validator_count(), 15); let _ = Staking::chill(Origin::signed(10)); // trigger era - System::set_block_number(1); - Session::on_initialize(System::block_number()); + start_era(1); // Previous ones are elected. chill is invalidates. TODO: #2494 assert_eq_uvec!(validator_controllers(), vec![10, 20, 30, 40]); - assert_eq!(Staking::current_elected().len(), 0); + // Though the validator preferences has been removed. + assert!(Staking::validators(11) != prefs); }); }