diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index d72a558e1dd28..cb8a90085aaf4 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -201,8 +201,13 @@ impl frame_system::Config for Runtime { type OnSetCode = (); } +parameter_types! { + pub const MaxAuthorities: u32 = 100; +} + impl pallet_aura::Config for Runtime { type AuthorityId = AuraId; + type MaxAuthorities = MaxAuthorities; } impl pallet_grandpa::Config for Runtime { @@ -221,6 +226,8 @@ impl pallet_grandpa::Config for Runtime { type HandleEquivocation = (); + type MaxAuthorities = MaxAuthorities; + type WeightInfo = (); } @@ -394,7 +401,7 @@ impl_runtime_apis! { } fn authorities() -> Vec { - Aura::authorities() + Aura::authorities().to_vec() } } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 648bbff633046..cb59c8c5961cd 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -332,8 +332,14 @@ parameter_types! { pub const ExpectedBlockTime: Moment = MILLISECS_PER_BLOCK; pub const ReportLongevity: u64 = BondingDuration::get() as u64 * SessionsPerEra::get() as u64 * EpochDuration::get(); + // The maximum number of authorities / validators for this runtime system. + // This is should be shared across all pallets using session handlers. + pub const MaxAuthorities: u32 = 100; } +// Just a convenient rename... +type MaxValidators = MaxAuthorities; + impl pallet_babe::Config for Runtime { type EpochDuration = EpochDuration; type ExpectedBlockTime = ExpectedBlockTime; @@ -354,6 +360,8 @@ impl pallet_babe::Config for Runtime { type HandleEquivocation = pallet_babe::EquivocationHandler; + type MaxAuthorities = MaxAuthorities; + type WeightInfo = (); } @@ -446,6 +454,7 @@ impl pallet_session::Config for Runtime { type SessionHandler = ::KeyTypeIdProviders; type Keys = SessionKeys; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; + type MaxValidators = MaxValidators; type WeightInfo = pallet_session::weights::SubstrateWeight; } @@ -496,6 +505,7 @@ impl pallet_staking::Config for Runtime { type NextNewSession = Session; type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; type ElectionProvider = ElectionProviderMultiPhase; + type MaxValidators = MaxValidators; type WeightInfo = pallet_staking::weights::SubstrateWeight; } @@ -891,6 +901,7 @@ impl pallet_im_online::Config for Runtime { type ValidatorSet = Historical; type ReportUnresponsiveness = Offences; type UnsignedPriority = ImOnlineUnsignedPriority; + type MaxAuthorityKeys = MaxAuthorities; type WeightInfo = pallet_im_online::weights::SubstrateWeight; } @@ -925,6 +936,8 @@ impl pallet_grandpa::Config for Runtime { type HandleEquivocation = pallet_grandpa::EquivocationHandler; + type MaxAuthorities = MaxAuthorities; + type WeightInfo = (); } @@ -1294,7 +1307,7 @@ impl_runtime_apis! { slot_duration: Babe::slot_duration(), epoch_length: EpochDuration::get(), c: BABE_GENESIS_EPOCH_CONFIG.c, - genesis_authorities: Babe::authorities(), + genesis_authorities: Babe::authorities().to_vec(), randomness: Babe::randomness(), allowed_slots: BABE_GENESIS_EPOCH_CONFIG.allowed_slots, } diff --git a/frame/aura/src/lib.rs b/frame/aura/src/lib.rs index a9b91737235ae..d07a0ca32a33f 100644 --- a/frame/aura/src/lib.rs +++ b/frame/aura/src/lib.rs @@ -37,10 +37,14 @@ #![cfg_attr(not(feature = "std"), no_std)] -use sp_std::prelude::*; +use sp_std::{ + prelude::*, + convert::TryInto, +}; use codec::{Encode, Decode}; use frame_support::{ - Parameter, traits::{Get, FindAuthor, OneSessionHandler, OnTimestampSet}, ConsensusEngineId, + Parameter, BoundedVec, ConsensusEngineId, + traits::{Get, FindAuthor, OneSessionHandler, OnTimestampSet}, }; use sp_runtime::{ RuntimeAppPublic, @@ -64,11 +68,21 @@ pub mod pallet { pub trait Config: pallet_timestamp::Config + frame_system::Config { /// The identifier type for an authority. type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + MaybeSerializeDeserialize; + + /// The maximum number of authorities that can be registered in this pallet. + type MaxAuthorities: Get; } #[pallet::pallet] pub struct Pallet(sp_std::marker::PhantomData); + // Errors inform users that something went wrong. + #[pallet::error] + pub enum Error { + /// You are trying to add more authorities than allowed in the pallet configuration. + TooManyAuthorities, + } + #[pallet::hooks] impl Hooks> for Pallet { fn on_initialize(_: T::BlockNumber) -> Weight { @@ -93,7 +107,11 @@ pub mod pallet { /// The current authority set. #[pallet::storage] #[pallet::getter(fn authorities)] - pub(super) type Authorities = StorageValue<_, Vec, ValueQuery>; + pub(super) type Authorities = StorageValue< + _, + BoundedVec, + ValueQuery, + >; /// The current slot of this block. /// @@ -123,12 +141,12 @@ pub mod pallet { } impl Pallet { - fn change_authorities(new: Vec) { + fn change_authorities(new: BoundedVec) { >::put(&new); let log: DigestItem = DigestItem::Consensus( AURA_ENGINE_ID, - ConsensusLog::AuthoritiesChange(new).encode() + ConsensusLog::AuthoritiesChange(new.to_vec()).encode() ); >::deposit_log(log.into()); } @@ -136,7 +154,11 @@ impl Pallet { fn initialize_authorities(authorities: &[T::AuthorityId]) { if !authorities.is_empty() { assert!(>::get().is_empty(), "Authorities are already initialized!"); - >::put(authorities); + let bounded_authorities: BoundedVec = authorities + .to_vec() + .try_into() + .expect("Too many initial authorities!"); + >::put(&bounded_authorities); } } @@ -165,7 +187,7 @@ impl sp_runtime::BoundToRuntimeAppPublic for Pallet { type Public = T::AuthorityId; } -impl OneSessionHandler for Pallet { +impl OneSessionHandler for Pallet { type Key = T::AuthorityId; fn on_genesis_session<'a, I: 'a>(validators: I) @@ -181,9 +203,15 @@ impl OneSessionHandler for Pallet { // instant changes if changed { let next_authorities = validators.map(|(_, k)| k).collect::>(); + // Truncate any extra that would not fit into storage... + let bounded_next_authorities = BoundedVec::::truncating_from( + next_authorities, + Some("Aura New Session"), + ); + let last_authorities = Self::authorities(); - if next_authorities != last_authorities { - Self::change_authorities(next_authorities); + if bounded_next_authorities != last_authorities { + Self::change_authorities(bounded_next_authorities); } } } diff --git a/frame/aura/src/mock.rs b/frame/aura/src/mock.rs index 26d5a2754974f..b41d5ad0656f6 100644 --- a/frame/aura/src/mock.rs +++ b/frame/aura/src/mock.rs @@ -80,8 +80,13 @@ impl pallet_timestamp::Config for Test { type WeightInfo = (); } +parameter_types! { + pub const MaxAuthorities: u32 = 10; +} + impl pallet_aura::Config for Test { type AuthorityId = AuthorityId; + type MaxAuthorities = MaxAuthorities; } pub fn new_test_ext(authorities: Vec) -> sp_io::TestExternalities { diff --git a/frame/aura/src/tests.rs b/frame/aura/src/tests.rs index 18e14e802bd32..9f4a01e81945c 100644 --- a/frame/aura/src/tests.rs +++ b/frame/aura/src/tests.rs @@ -20,6 +20,8 @@ #![cfg(test)] use crate::mock::{Aura, new_test_ext}; +use sp_runtime::testing::UintAuthorityId; +use frame_support::traits::OneSessionHandler; #[test] fn initial_values() { @@ -28,3 +30,30 @@ fn initial_values() { assert_eq!(Aura::authorities().len(), 4); }); } + +// Should not be able to put more authorities than allowed in genesis. +#[test] +#[should_panic(expected = "Too many initial authorities!")] +fn too_many_initial_fails() { + new_test_ext((0..100).collect::>()); +} + +// Session change should truncate the new authorities to fit into the limits +// of the Aura pallet. +#[test] +fn session_change_truncates() { + new_test_ext(vec![0, 1, 2, 3]).execute_with(|| { + Aura::on_new_session( + true, + (0..100) + .map(|x| { + let auth_id = UintAuthorityId(x).to_public_key(); + (&0, auth_id) + }) + .collect::>() + .into_iter(), + vec![].into_iter(), + ); + assert_eq!(Aura::authorities().len(), 10); + }); +} diff --git a/frame/authority-discovery/src/lib.rs b/frame/authority-discovery/src/lib.rs index 6b7608b10c3bd..4b250e1440630 100644 --- a/frame/authority-discovery/src/lib.rs +++ b/frame/authority-discovery/src/lib.rs @@ -23,8 +23,8 @@ // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] -use sp_std::prelude::*; -use frame_support::traits::OneSessionHandler; +use sp_std::{prelude::*, convert::TryInto}; +use frame_support::{traits::OneSessionHandler, BoundedVec}; #[cfg(feature = "std")] use frame_support::traits::GenesisBuild; use sp_authority_discovery::AuthorityId; @@ -50,16 +50,16 @@ pub mod pallet { /// Keys of the current authority set. pub(super) type Keys = StorageValue< _, - Vec, + BoundedVec, ValueQuery, >; - + #[pallet::storage] #[pallet::getter(fn next_keys)] /// Keys of the next authority set. pub(super) type NextKeys = StorageValue< _, - Vec, + BoundedVec, ValueQuery, >; @@ -79,7 +79,9 @@ pub mod pallet { #[pallet::genesis_build] impl GenesisBuild for GenesisConfig { fn build(&self) { - Pallet::::initialize_keys(&self.keys) + let bounded_keys: BoundedVec:: = + self.keys.clone().try_into().expect("Too many genesis keys!"); + Pallet::::initialize_keys(&bounded_keys) } } @@ -94,8 +96,8 @@ impl Pallet { /// Retrieve authority identifiers of the current and next authority set /// sorted and deduplicated. pub fn authorities() -> Vec { - let mut keys = Keys::::get(); - let next = NextKeys::::get(); + let mut keys = Keys::::get().to_vec(); + let next = NextKeys::::get().to_vec(); keys.extend(next); keys.sort(); @@ -106,19 +108,21 @@ impl Pallet { /// Retrieve authority identifiers of the current authority set in the original order. pub fn current_authorities() -> Vec { - Keys::::get() + Keys::::get().to_vec() } /// Retrieve authority identifiers of the next authority set in the original order. pub fn next_authorities() -> Vec { - NextKeys::::get() + NextKeys::::get().to_vec() } fn initialize_keys(keys: &[AuthorityId]) { if !keys.is_empty() { assert!(Keys::::get().is_empty(), "Keys are already initialized!"); - Keys::::put(keys); - NextKeys::::put(keys); + let bounded_keys: BoundedVec = + keys.to_vec().try_into().expect("Too many initial keys!"); + Keys::::put(&bounded_keys); + NextKeys::::put(&bounded_keys); } } } @@ -127,7 +131,7 @@ impl sp_runtime::BoundToRuntimeAppPublic for Pallet { type Public = AuthorityId; } -impl OneSessionHandler for Pallet { +impl OneSessionHandler for Pallet { type Key = AuthorityId; fn on_genesis_session<'a, I: 'a>(authorities: I) @@ -143,10 +147,20 @@ impl OneSessionHandler for Pallet { { // Remember who the authorities are for the new and next session. if changed { - let keys = validators.map(|x| x.1); - Keys::::put(keys.collect::>()); - let next_keys = queued_validators.map(|x| x.1); - NextKeys::::put(next_keys.collect::>()); + let keys = validators.map(|x| x.1).collect::>(); + // Truncate any extra that would not fit in storage... + let bounded_keys = BoundedVec::::truncating_from( + keys, + Some("Authority Discovery New Session Keys"), + ); + Keys::::put(bounded_keys); + let next_keys = queued_validators.map(|x| x.1).collect::>(); + // Truncate any extra that would not fit in storage... + let bounded_next_keys = BoundedVec::::truncating_from( + next_keys, + Some("Authority Discovery New Session Next Keys"), + ); + NextKeys::::put(bounded_next_keys); } } @@ -211,6 +225,7 @@ mod tests { type ValidatorIdOf = ConvertInto; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; type NextSessionRotation = pallet_session::PeriodicSessions; + type MaxValidators = MaxValidators; type WeightInfo = (); } @@ -256,8 +271,12 @@ mod tests { type OnSetCode = (); } + parameter_types! { + pub const MaxValidators: u32 = 10; + } + pub struct TestSessionHandler; - impl pallet_session::SessionHandler for TestSessionHandler { + impl pallet_session::SessionHandler for TestSessionHandler { const KEY_TYPE_IDS: &'static [KeyTypeId] = &[key_types::DUMMY]; fn on_new_session( diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index fb1e32e5350b5..df37bc1fd00bc 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -23,6 +23,7 @@ use codec::{Decode, Encode}; use frame_support::{ + BoundedVec, dispatch::DispatchResultWithPostInfo, traits::{FindAuthor, Get, KeyOwnerProofSystem, OneSessionHandler, OnTimestampSet}, weights::{Pays, Weight}, @@ -34,7 +35,7 @@ use sp_runtime::{ ConsensusEngineId, KeyTypeId, Percent, }; use sp_session::{GetSessionNumber, GetValidatorCount}; -use sp_std::prelude::*; +use sp_std::{prelude::*, convert::TryInto}; use sp_consensus_babe::{ digests::{NextConfigDescriptor, NextEpochDescriptor, PreDigest}, @@ -93,7 +94,7 @@ impl EpochChangeTrigger for SameAuthoritiesForever { let authorities = >::authorities(); let next_authorities = authorities.clone(); - >::enact_epoch_change(authorities, next_authorities); + >::enact_epoch_change(authorities.to_vec(), next_authorities.to_vec()); } } } @@ -161,6 +162,9 @@ pub mod pallet { /// definition. type HandleEquivocation: HandleEquivocation; + /// Maximum number of authorities handled by the session manager. + type MaxAuthorities: Get; + type WeightInfo: WeightInfo; } @@ -182,7 +186,11 @@ pub mod pallet { /// Current epoch authorities. #[pallet::storage] #[pallet::getter(fn authorities)] - pub type Authorities = StorageValue<_, Vec<(AuthorityId, BabeAuthorityWeight)>, ValueQuery>; + pub type Authorities = StorageValue< + _, + BoundedVec<(AuthorityId, BabeAuthorityWeight), T::MaxAuthorities>, + ValueQuery, + >; /// The slot at which the first epoch actually started. This is 0 /// until the first block of the chain. @@ -222,9 +230,9 @@ pub mod pallet { /// Next epoch authorities. #[pallet::storage] - pub(super) type NextAuthorities = StorageValue< + pub(super) type NextAuthorities = StorageValue< _, - Vec<(AuthorityId, BabeAuthorityWeight)>, + BoundedVec<(AuthorityId, BabeAuthorityWeight), T::MaxAuthorities>, ValueQuery, >; @@ -518,7 +526,15 @@ impl Pallet { .expect("epoch indices will never reach 2^64 before the death of the universe; qed"); EpochIndex::::put(epoch_index); - Authorities::::put(authorities); + + let bounded_authorities = BoundedVec::< + (AuthorityId, BabeAuthorityWeight), + T::MaxAuthorities, + >::truncating_from( + authorities, + Some("Babe Enact Epoch Change"), + ); + Authorities::::put(bounded_authorities); // Update epoch randomness. let next_epoch_index = epoch_index @@ -531,7 +547,14 @@ impl Pallet { Randomness::::put(randomness); // Update the next epoch authorities. - NextAuthorities::::put(&next_authorities); + let bounded_next_authorities = BoundedVec::< + (AuthorityId, BabeAuthorityWeight), + T::MaxAuthorities, + >::truncating_from( + next_authorities, + Some("Babe Enact Epoch Change"), + ); + NextAuthorities::::put(&bounded_next_authorities); // Update the start blocks of the previous and new current epoch. >::mutate(|(previous_epoch_start_block, current_epoch_start_block)| { @@ -544,7 +567,7 @@ impl Pallet { let next_randomness = NextRandomness::::get(); let next_epoch = NextEpochDescriptor { - authorities: next_authorities, + authorities: bounded_next_authorities.to_vec(), randomness: next_randomness, }; Self::deposit_consensus(ConsensusLog::NextEpochData(next_epoch)); @@ -575,7 +598,7 @@ impl Pallet { epoch_index: EpochIndex::::get(), start_slot: Self::current_epoch_start(), duration: T::EpochDuration::get(), - authorities: Self::authorities(), + authorities: Self::authorities().to_vec(), randomness: Self::randomness(), config: EpochConfig::::get().expect("EpochConfig is initialized in genesis; we never `take` or `kill` it; qed"), } @@ -593,7 +616,7 @@ impl Pallet { epoch_index: next_epoch_index, start_slot: Self::epoch_start(next_epoch_index), duration: T::EpochDuration::get(), - authorities: NextAuthorities::::get(), + authorities: NextAuthorities::::get().to_vec(), randomness: NextRandomness::::get(), config: NextEpochConfig::::get().unwrap_or_else(|| { EpochConfig::::get().expect("EpochConfig is initialized in genesis; we never `take` or `kill` it; qed") @@ -667,7 +690,7 @@ impl Pallet { // we use the same values as genesis because we haven't collected any // randomness yet. let next = NextEpochDescriptor { - authorities: Self::authorities(), + authorities: Self::authorities().to_vec(), randomness: Self::randomness(), }; @@ -749,8 +772,10 @@ impl Pallet { fn initialize_authorities(authorities: &[(AuthorityId, BabeAuthorityWeight)]) { if !authorities.is_empty() { assert!(Authorities::::get().is_empty(), "Authorities are already initialized!"); - Authorities::::put(authorities); - NextAuthorities::::put(authorities); + let bounded_authorities: BoundedVec<(AuthorityId, BabeAuthorityWeight), T::MaxAuthorities> = + authorities.to_vec().try_into().expect("Too many initial authorities!"); + Authorities::::put(&bounded_authorities); + NextAuthorities::::put(&bounded_authorities); } } @@ -868,7 +893,7 @@ impl sp_runtime::BoundToRuntimeAppPublic for Pallet { type Public = AuthorityId; } -impl OneSessionHandler for Pallet { +impl OneSessionHandler for Pallet { type Key = AuthorityId; fn on_genesis_session<'a, I: 'a>(validators: I) diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 39831eceb75ba..0e0aaf684adfa 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -109,6 +109,10 @@ impl_opaque_keys! { } } +parameter_types! { + pub const MaxValidators: u32 = 10; +} + impl pallet_session::Config for Test { type Event = Event; type ValidatorId = ::AccountId; @@ -119,6 +123,7 @@ impl pallet_session::Config for Test { type SessionHandler = ::KeyTypeIdProviders; type Keys = MockSessionKeys; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; + type MaxValidators = MaxValidators; type WeightInfo = (); } @@ -211,6 +216,7 @@ impl pallet_staking::Config for Test { type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; type NextNewSession = Session; type ElectionProvider = onchain::OnChainSequentialPhragmen; + type MaxValidators = MaxValidators; type WeightInfo = (); } @@ -251,6 +257,8 @@ impl Config for Test { type HandleEquivocation = super::EquivocationHandler; + type MaxAuthorities = MaxValidators; + type WeightInfo = (); } diff --git a/frame/babe/src/tests.rs b/frame/babe/src/tests.rs index 6aa80e9697339..999fa0b9146b2 100644 --- a/frame/babe/src/tests.rs +++ b/frame/babe/src/tests.rs @@ -104,7 +104,7 @@ fn first_block_epoch_zero_start() { let consensus_log = sp_consensus_babe::ConsensusLog::NextEpochData( sp_consensus_babe::digests::NextEpochDescriptor { - authorities: Babe::authorities(), + authorities: Babe::authorities().to_vec(), randomness: Babe::randomness(), } ); @@ -413,7 +413,7 @@ fn report_equivocation_current_session_works() { start_era(1); let authorities = Babe::authorities(); - let validators = Session::validators(); + let validators = Session::validators().to_vec(); // make sure that all authorities have the same balance for validator in &validators { diff --git a/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index eb3dc4f110acb..f7a80974790e4 100644 --- a/frame/grandpa/src/lib.rs +++ b/frame/grandpa/src/lib.rs @@ -40,8 +40,10 @@ use fg_primitives::{ GRANDPA_ENGINE_ID, }; use frame_support::{ - decl_error, decl_event, decl_module, decl_storage, dispatch::DispatchResultWithPostInfo, - storage, traits::{OneSessionHandler, KeyOwnerProofSystem}, weights::{Pays, Weight}, Parameter, + decl_error, decl_event, decl_module, decl_storage, storage, + dispatch::DispatchResultWithPostInfo, + traits::{OneSessionHandler, KeyOwnerProofSystem}, weights::{Pays, Weight}, + pallet_prelude::Get, Parameter, }; use frame_system::{ensure_none, ensure_root, ensure_signed}; use sp_runtime::{ @@ -98,6 +100,9 @@ pub trait Config: frame_system::Config { /// definition. type HandleEquivocation: HandleEquivocation; + /// The maximum number of authorities for the session handler. + type MaxAuthorities: Get; + /// Weights for this pallet. type WeightInfo: WeightInfo; } @@ -587,7 +592,7 @@ impl sp_runtime::BoundToRuntimeAppPublic for Module { type Public = AuthorityId; } -impl OneSessionHandler for Module +impl OneSessionHandler for Module where T: pallet_session::Config { type Key = AuthorityId; diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index d59d0d19d0e87..f51887f31c6b8 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -112,6 +112,7 @@ parameter_types! { pub const Period: u64 = 1; pub const Offset: u64 = 0; pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(17); + pub const MaxValidators: u32 = 10; } /// Custom `SessionHandler` since we use `TestSessionKeys` as `Keys`. @@ -125,6 +126,7 @@ impl pallet_session::Config for Test { type SessionHandler = ::KeyTypeIdProviders; type Keys = TestSessionKeys; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; + type MaxValidators = MaxValidators; type WeightInfo = (); } @@ -217,6 +219,7 @@ impl pallet_staking::Config for Test { type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; type NextNewSession = Session; type ElectionProvider = onchain::OnChainSequentialPhragmen; + type MaxValidators = MaxValidators; type WeightInfo = (); } @@ -253,6 +256,8 @@ impl Config for Test { type HandleEquivocation = super::EquivocationHandler; + type MaxAuthorities = MaxValidators; + type WeightInfo = (); } diff --git a/frame/grandpa/src/tests.rs b/frame/grandpa/src/tests.rs index 92d2c6c751a24..7763764548beb 100644 --- a/frame/grandpa/src/tests.rs +++ b/frame/grandpa/src/tests.rs @@ -364,7 +364,7 @@ fn report_equivocation_current_set_works() { start_era(1); let authorities = Grandpa::grandpa_authorities(); - let validators = Session::validators(); + let validators = Session::validators().to_vec(); // make sure that all validators have the same balance for validator in &validators { @@ -453,7 +453,7 @@ fn report_equivocation_old_set_works() { start_era(1); let authorities = Grandpa::grandpa_authorities(); - let validators = Session::validators(); + let validators = Session::validators().to_vec(); let equivocation_authority_index = 0; let equivocation_key = &authorities[equivocation_authority_index].0; diff --git a/frame/im-online/src/benchmarking.rs b/frame/im-online/src/benchmarking.rs index 287a2c6fd3a73..6fa1864aecf20 100644 --- a/frame/im-online/src/benchmarking.rs +++ b/frame/im-online/src/benchmarking.rs @@ -31,17 +31,18 @@ use frame_support::traits::UnfilteredDispatchable; use crate::Module as ImOnline; -const MAX_KEYS: u32 = 1000; const MAX_EXTERNAL_ADDRESSES: u32 = 100; pub fn create_heartbeat(k: u32, e: u32) -> Result<(crate::Heartbeat, ::Signature), &'static str> { let mut keys = Vec::new(); + assert!(k <= T::MaxAuthorityKeys::get(), "Trying to add too many keys!"); for _ in 0..k { keys.push(T::AuthorityId::generate_pair(None)); } - Keys::::put(keys.clone()); + let bounded_keys: BoundedVec::<_, T::MaxAuthorityKeys> = keys.try_into().unwrap(); + Keys::::put(&bounded_keys); let network_state = OpaqueNetworkState { peer_id: OpaquePeerId::default(), @@ -52,11 +53,11 @@ pub fn create_heartbeat(k: u32, e: u32) -> network_state, session_index: 0, authority_index: k-1, - validators_len: keys.len() as u32, + validators_len: bounded_keys.len() as u32, }; let encoded_heartbeat = input_heartbeat.encode(); - let authority_id = keys.get((k-1) as usize).ok_or("out of range")?; + let authority_id = bounded_keys.get((k-1) as usize).ok_or("out of range")?; let signature = authority_id.sign(&encoded_heartbeat).ok_or("couldn't make signature")?; Ok((input_heartbeat, signature)) @@ -65,14 +66,14 @@ pub fn create_heartbeat(k: u32, e: u32) -> benchmarks! { #[extra] heartbeat { - let k in 1 .. MAX_KEYS; + let k in 1 .. T::MaxAuthorityKeys::get(); let e in 1 .. MAX_EXTERNAL_ADDRESSES; let (input_heartbeat, signature) = create_heartbeat::(k, e)?; }: _(RawOrigin::None, input_heartbeat, signature) #[extra] validate_unsigned { - let k in 1 .. MAX_KEYS; + let k in 1 .. T::MaxAuthorityKeys::get(); let e in 1 .. MAX_EXTERNAL_ADDRESSES; let (input_heartbeat, signature) = create_heartbeat::(k, e)?; let call = Call::heartbeat(input_heartbeat, signature); @@ -81,7 +82,7 @@ benchmarks! { } validate_unsigned_and_then_heartbeat { - let k in 1 .. MAX_KEYS; + let k in 1 .. T::MaxAuthorityKeys::get(); let e in 1 .. MAX_EXTERNAL_ADDRESSES; let (input_heartbeat, signature) = create_heartbeat::(k, e)?; let call = Call::heartbeat(input_heartbeat, signature); diff --git a/frame/im-online/src/lib.rs b/frame/im-online/src/lib.rs index d8f3fdc854b16..6cfc8173608a3 100644 --- a/frame/im-online/src/lib.rs +++ b/frame/im-online/src/lib.rs @@ -98,7 +98,7 @@ use frame_support::{ EstimateNextSessionRotation, Get, OneSessionHandler, ValidatorSet, ValidatorSetWithIdentification, }, - Parameter, + Parameter, BoundedVec, }; use frame_system::{ensure_none, offchain::{SendTransactionTypes, SubmitTransaction}}; pub use weights::WeightInfo; @@ -270,6 +270,9 @@ pub trait Config: SendTransactionTypes> + frame_system::Config { /// multiple pallets send unsigned transactions. type UnsignedPriority: Get; + /// The maximum number of authority keys that can be stored by this pallet. + type MaxAuthorityKeys: Get; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -304,7 +307,7 @@ decl_storage! { HeartbeatAfter get(fn heartbeat_after): T::BlockNumber; /// The current set of keys that may issue a heartbeat. - Keys get(fn keys): Vec; + Keys get(fn keys): BoundedVec; /// For each session index, we keep a mapping of `AuthIndex` to /// `offchain::OpaqueNetworkState`. @@ -638,12 +641,16 @@ impl Module { fn initialize_keys(keys: &[T::AuthorityId]) { if !keys.is_empty() { assert!(Keys::::get().is_empty(), "Keys are already initialized!"); - Keys::::put(keys); + let bounded_keys: BoundedVec = keys + .to_vec() + .try_into() + .expect("Too many initial keys!"); + Keys::::put(bounded_keys); } } #[cfg(test)] - fn set_keys(keys: Vec) { + fn set_keys(keys: BoundedVec) { Keys::::put(&keys) } } @@ -652,7 +659,7 @@ impl sp_runtime::BoundToRuntimeAppPublic for Module { type Public = T::AuthorityId; } -impl OneSessionHandler for Module { +impl OneSessionHandler for Module { type Key = T::AuthorityId; fn on_genesis_session<'a, I: 'a>(validators: I) @@ -673,7 +680,13 @@ impl OneSessionHandler for Module { >::put(block_number + half_session); // Remember who the authorities are for the new session. - Keys::::put(validators.map(|x| x.1).collect::>()); + let next_validators = validators.map(|x| x.1).collect::>(); + + let bounded_validators = BoundedVec::::truncating_from( + next_validators, + Some("Im Online New Session"), + ); + Keys::::put(bounded_validators); } fn on_before_session_ending() { diff --git a/frame/im-online/src/mock.rs b/frame/im-online/src/mock.rs index 4f21012abc510..20dee12f455fd 100644 --- a/frame/im-online/src/mock.rs +++ b/frame/im-online/src/mock.rs @@ -21,9 +21,10 @@ use std::cell::RefCell; -use frame_support::{parameter_types, weights::Weight}; +use frame_support::{parameter_types, weights::Weight, BoundedVec}; use pallet_session::historical as pallet_session_historical; use sp_core::H256; +use sp_std::convert::TryInto; use sp_runtime::testing::{Header, TestXt, UintAuthorityId}; use sp_runtime::traits::{BlakeTwo256, ConvertInto, IdentityLookup}; use sp_runtime::{Perbill, Percent}; @@ -150,6 +151,7 @@ parameter_types! { parameter_types! { pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(33); + pub const MaxValidators: u32 = 10; } impl pallet_session::Config for Runtime { @@ -158,6 +160,7 @@ impl pallet_session::Config for Runtime { type SessionHandler = (ImOnline, ); type ValidatorId = u64; type ValidatorIdOf = ConvertInto; + type MaxValidators = MaxValidators; type Keys = UintAuthorityId; type Event = Event; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; @@ -227,6 +230,7 @@ impl Config for Runtime { type NextSessionRotation = TestNextSessionRotation; type ReportUnresponsiveness = OffenceHandler; type UnsignedPriority = UnsignedPriority; + type MaxAuthorityKeys = MaxValidators; type WeightInfo = (); } @@ -241,7 +245,8 @@ pub fn advance_session() { let now = System::block_number().max(1); System::set_block_number(now + 1); Session::rotate_session(); - let keys = Session::validators().into_iter().map(UintAuthorityId).collect(); - ImOnline::set_keys(keys); + let keys: Vec = Session::validators().into_iter().map(UintAuthorityId).collect(); + let bounded_keys: BoundedVec = keys.try_into().unwrap(); + ImOnline::set_keys(bounded_keys); assert_eq!(Session::current_index(), (now / Period::get()) as u32); } diff --git a/frame/im-online/src/tests.rs b/frame/im-online/src/tests.rs index f447a2ade5481..548ad0e89b454 100644 --- a/frame/im-online/src/tests.rs +++ b/frame/im-online/src/tests.rs @@ -89,7 +89,7 @@ fn should_report_offline_validators() { // should not report when heartbeat is sent for (idx, v) in validators.into_iter().take(4).enumerate() { - let _ = heartbeat(block, 3, idx as u32, v.into(), Session::validators()).unwrap(); + let _ = heartbeat(block, 3, idx as u32, v.into(), Session::validators().to_vec()).unwrap(); } advance_session(); @@ -148,19 +148,19 @@ fn should_mark_online_validator_when_heartbeat_is_received() { advance_session(); // given VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3, 4, 5, 6])); - assert_eq!(Session::validators(), Vec::::new()); + assert_eq!(Session::validators().to_vec(), Vec::::new()); // enact the change and buffer another one advance_session(); assert_eq!(Session::current_index(), 2); - assert_eq!(Session::validators(), vec![1, 2, 3]); + assert_eq!(Session::validators().to_vec(), vec![1, 2, 3]); assert!(!ImOnline::is_online(0)); assert!(!ImOnline::is_online(1)); assert!(!ImOnline::is_online(2)); // when - let _ = heartbeat(1, 2, 0, 1.into(), Session::validators()).unwrap(); + let _ = heartbeat(1, 2, 0, 1.into(), Session::validators().to_vec()).unwrap(); // then assert!(ImOnline::is_online(0)); @@ -168,7 +168,7 @@ fn should_mark_online_validator_when_heartbeat_is_received() { assert!(!ImOnline::is_online(2)); // and when - let _ = heartbeat(1, 2, 2, 3.into(), Session::validators()).unwrap(); + let _ = heartbeat(1, 2, 2, 3.into(), Session::validators().to_vec()).unwrap(); // then assert!(ImOnline::is_online(0)); @@ -183,16 +183,16 @@ fn late_heartbeat_and_invalid_keys_len_should_fail() { advance_session(); // given VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3, 4, 5, 6])); - assert_eq!(Session::validators(), Vec::::new()); + assert_eq!(Session::validators().to_vec(), Vec::::new()); // enact the change and buffer another one advance_session(); assert_eq!(Session::current_index(), 2); - assert_eq!(Session::validators(), vec![1, 2, 3]); + assert_eq!(Session::validators().to_vec(), vec![1, 2, 3]); // when - assert_noop!(heartbeat(1, 3, 0, 1.into(), Session::validators()), "Transaction is outdated"); - assert_noop!(heartbeat(1, 1, 0, 1.into(), Session::validators()), "Transaction is outdated"); + assert_noop!(heartbeat(1, 3, 0, 1.into(), Session::validators().to_vec()), "Transaction is outdated"); + assert_noop!(heartbeat(1, 1, 0, 1.into(), Session::validators().to_vec()), "Transaction is outdated"); // invalid validators_len assert_noop!(heartbeat(1, 2, 0, 1.into(), vec![]), "invalid validators len"); @@ -252,16 +252,16 @@ fn should_cleanup_received_heartbeats_on_session_end() { advance_session(); VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3])); - assert_eq!(Session::validators(), Vec::::new()); + assert_eq!(Session::validators().to_vec(), Vec::::new()); // enact the change and buffer another one advance_session(); assert_eq!(Session::current_index(), 2); - assert_eq!(Session::validators(), vec![1, 2, 3]); + assert_eq!(Session::validators().to_vec(), vec![1, 2, 3]); // send an heartbeat from authority id 0 at session 2 - let _ = heartbeat(1, 2, 0, 1.into(), Session::validators()).unwrap(); + let _ = heartbeat(1, 2, 0, 1.into(), Session::validators().to_vec()).unwrap(); // the heartbeat is stored assert!(!ImOnline::received_heartbeats(&2, &0).is_none()); @@ -283,12 +283,12 @@ fn should_mark_online_validator_when_block_is_authored() { advance_session(); // given VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3, 4, 5, 6])); - assert_eq!(Session::validators(), Vec::::new()); + assert_eq!(Session::validators().to_vec(), Vec::::new()); // enact the change and buffer another one advance_session(); assert_eq!(Session::current_index(), 2); - assert_eq!(Session::validators(), vec![1, 2, 3]); + assert_eq!(Session::validators().to_vec(), vec![1, 2, 3]); for i in 0..3 { assert!(!ImOnline::is_online(i)); @@ -320,11 +320,11 @@ fn should_not_send_a_report_if_already_online() { advance_session(); // given VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3, 4, 5, 6])); - assert_eq!(Session::validators(), Vec::::new()); + assert_eq!(Session::validators().to_vec(), Vec::::new()); // enact the change and buffer another one advance_session(); assert_eq!(Session::current_index(), 2); - assert_eq!(Session::validators(), vec![1, 2, 3]); + assert_eq!(Session::validators().to_vec(), vec![1, 2, 3]); ImOnline::note_author(2); ImOnline::note_uncle(3, 0); diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index a0a09e0fbb897..f01544728f404 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -101,7 +101,7 @@ sp_runtime::impl_opaque_keys! { } pub struct TestSessionHandler; -impl pallet_session::SessionHandler for TestSessionHandler { +impl pallet_session::SessionHandler for TestSessionHandler { const KEY_TYPE_IDS: &'static [sp_runtime::KeyTypeId] = &[]; fn on_genesis_session(_validators: &[(AccountId, Ks)]) {} @@ -118,6 +118,7 @@ impl pallet_session::SessionHandler for TestSessionHandler { parameter_types! { pub const Period: u64 = 1; pub const Offset: u64 = 0; + pub const MaxValidators: u32 = 10; } impl pallet_session::Config for Test { @@ -130,6 +131,7 @@ impl pallet_session::Config for Test { type ValidatorId = AccountId; type ValidatorIdOf = pallet_staking::StashOf; type DisabledValidatorsThreshold = (); + type MaxValidators = MaxValidators; type WeightInfo = (); } @@ -176,6 +178,7 @@ impl pallet_staking::Config for Test { type NextNewSession = Session; type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; type ElectionProvider = onchain::OnChainSequentialPhragmen; + type MaxValidators = MaxValidators; type WeightInfo = (); } @@ -186,6 +189,7 @@ impl pallet_im_online::Config for Test { type NextSessionRotation = pallet_session::PeriodicSessions; type ReportUnresponsiveness = Offences; type UnsignedPriority = (); + type MaxAuthorityKeys = MaxValidators; type WeightInfo = (); } diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index cf2fa8a07cfe0..145f26a84ac20 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -103,7 +103,7 @@ sp_runtime::impl_opaque_keys! { } pub struct TestSessionHandler; -impl pallet_session::SessionHandler for TestSessionHandler { +impl pallet_session::SessionHandler for TestSessionHandler { const KEY_TYPE_IDS: &'static [sp_runtime::KeyTypeId] = &[]; fn on_genesis_session(_validators: &[(AccountId, Ks)]) {} @@ -117,6 +117,9 @@ impl pallet_session::SessionHandler for TestSessionHandler { fn on_disabled(_: usize) {} } +parameter_types! { + pub const MaxValidators: u32 = 10; +} impl pallet_session::Config for Test { type SessionManager = pallet_session::historical::NoteHistoricalRoot; type Keys = SessionKeys; @@ -127,6 +130,7 @@ impl pallet_session::Config for Test { type ValidatorId = AccountId; type ValidatorIdOf = pallet_staking::StashOf; type DisabledValidatorsThreshold = (); + type MaxValidators = MaxValidators; type WeightInfo = (); } pallet_staking_reward_curve::build! { @@ -181,6 +185,7 @@ impl pallet_staking::Config for Test { type NextNewSession = Session; type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; type ElectionProvider = onchain::OnChainSequentialPhragmen; + type MaxValidators = MaxValidators; type WeightInfo = (); } diff --git a/frame/session/src/historical/mod.rs b/frame/session/src/historical/mod.rs index 8902ebe551f6c..d37e169df289f 100644 --- a/frame/session/src/historical/mod.rs +++ b/frame/session/src/historical/mod.rs @@ -113,7 +113,7 @@ impl ValidatorSet for Module { } fn validators() -> Vec { - super::Module::::validators() + super::Module::::validators().to_vec() } } diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index cbe70598a91b3..06ea13f80fb5c 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -95,7 +95,7 @@ //! use pallet_session as session; //! //! fn validators() -> Vec<::ValidatorId> { -//! >::validators() +//! >::validators().to_vec() //! } //! # fn main(){} //! ``` @@ -114,7 +114,7 @@ mod tests; pub mod historical; pub mod weights; -use sp_std::{prelude::*, marker::PhantomData, ops::{Sub, Rem}}; +use sp_std::{prelude::*, marker::PhantomData, ops::{Sub, Rem}, convert::TryInto}; use codec::Decode; use sp_runtime::{ traits::{AtLeast32BitUnsigned, Convert, Member, One, OpaqueKeys, Zero}, @@ -122,7 +122,8 @@ use sp_runtime::{ }; use sp_staking::SessionIndex; use frame_support::{ - ensure, decl_module, decl_event, decl_storage, decl_error, ConsensusEngineId, Parameter, + ensure, decl_module, decl_event, decl_storage, decl_error, + ConsensusEngineId, Parameter, BoundedVec, traits::{ Get, FindAuthor, ValidatorRegistration, EstimateNextSessionRotation, EstimateNextNewSession, OneSessionHandler, ValidatorSet, @@ -256,7 +257,7 @@ impl SessionManager for () { } /// Handler for session life cycle events. -pub trait SessionHandler { +pub trait SessionHandler { /// All the key type ids this session handler can process. /// /// The order must be the same as it expects them in @@ -291,8 +292,8 @@ pub trait SessionHandler { } #[impl_trait_for_tuples::impl_for_tuples(1, 30)] -#[tuple_types_custom_trait_bound(OneSessionHandler)] -impl SessionHandler for Tuple { +#[tuple_types_custom_trait_bound(OneSessionHandler)] +impl> SessionHandler for Tuple { for_tuples!( const KEY_TYPE_IDS: &'static [KeyTypeId] = &[ #( ::ID ),* ]; ); @@ -338,7 +339,7 @@ impl SessionHandler for Tuple { /// `SessionHandler` for tests that use `UintAuthorityId` as `Keys`. pub struct TestSessionHandler; -impl SessionHandler for TestSessionHandler { +impl> SessionHandler for TestSessionHandler { const KEY_TYPE_IDS: &'static [KeyTypeId] = &[sp_runtime::key_types::DUMMY]; fn on_genesis_session(_: &[(AId, Ks)]) {} @@ -368,6 +369,9 @@ pub trait Config: frame_system::Config { /// Its cost must be at most one storage read. type ValidatorIdOf: Convert>; + /// The maximum number of validators supported by the session handler. + type MaxValidators: Get; + /// Indicator for when to end the session. type ShouldEndSession: ShouldEndSession; @@ -380,7 +384,7 @@ pub trait Config: frame_system::Config { type SessionManager: SessionManager; /// Handler when a session has changed. - type SessionHandler: SessionHandler; + type SessionHandler: SessionHandler; /// The keys. type Keys: OpaqueKeys + Member + Parameter + Default; @@ -398,7 +402,7 @@ pub trait Config: frame_system::Config { decl_storage! { trait Store for Module as Session { /// The current set of validators. - Validators get(fn validators): Vec; + Validators get(fn validators): BoundedVec; /// Current index of the session. CurrentIndex get(fn current_index): SessionIndex; @@ -409,7 +413,7 @@ decl_storage! { /// The queued keys for the next session. When the next session begins, these keys /// will be used to determine the validator's session keys. - QueuedKeys get(fn queued_keys): Vec<(T::ValidatorId, T::Keys)>; + QueuedKeys get(fn queued_keys): BoundedVec<(T::ValidatorId, T::Keys), T::MaxValidators>; /// Indices of disabled validators. /// @@ -455,10 +459,12 @@ decl_storage! { session config keys to generate initial validator set."); config.keys.iter().map(|x| x.1.clone()).collect() }); - assert!(!initial_validators_0.is_empty(), "Empty validator set for session 0 in genesis block!"); + let bounded_initial_validators_0: BoundedVec + = initial_validators_0.clone().try_into().expect("Too many initial validators!"); + assert!(!bounded_initial_validators_0.is_empty(), "Empty validator set for session 0 in genesis block!"); let initial_validators_1 = T::SessionManager::new_session(1) - .unwrap_or_else(|| initial_validators_0.clone()); + .unwrap_or_else(|| initial_validators_0); assert!(!initial_validators_1.is_empty(), "Empty validator set for session 1 in genesis block!"); let queued_keys: Vec<_> = initial_validators_1 @@ -469,12 +475,13 @@ decl_storage! { >::load_keys(&v).unwrap_or_default(), )) .collect(); - + let bounded_queued_keys: BoundedVec<(T::ValidatorId, T::Keys), T::MaxValidators> + = queued_keys.try_into().expect("Too many queued keys!"); // Tell everyone about the genesis session keys - T::SessionHandler::on_genesis_session::(&queued_keys); + T::SessionHandler::on_genesis_session::(&bounded_queued_keys); - >::put(initial_validators_0); - >::put(queued_keys); + >::put(bounded_initial_validators_0); + >::put(bounded_queued_keys); T::SessionManager::start_session(0); }); @@ -589,6 +596,12 @@ impl Module { let validators = session_keys.iter() .map(|(validator, _)| validator.clone()) .collect::>(); + // Note this should never truncate because queued keys is also bounded to `MaxValidators`, + // but we do so defensively. + let validators = BoundedVec::::truncating_from( + validators, + Some("Session Rotate Session"), + ); >::put(&validators); if changed { @@ -607,6 +620,10 @@ impl Module { let (next_validators, next_identities_changed) = if let Some(validators) = maybe_next_validators { + let validators = BoundedVec::::truncating_from( + validators, + Some("Session Rotate Session Maybe Next Validators"), + ); // NOTE: as per the documentation on `OnSessionEnding`, we consider // the validator set as having changed even if the validators are the // same as before, as underlying economic conditions may have changed. @@ -641,6 +658,12 @@ impl Module { (a, k) }) .collect::>(); + // Should never truncate since next_validators should already be the right length, + // but we do so defensively. + let queued_amalgamated = BoundedVec::<(T::ValidatorId, T::Keys), T::MaxValidators>::truncating_from( + queued_amalgamated, + Some("Session Rotate Session"), + ); (queued_amalgamated, changed) }; @@ -735,9 +758,16 @@ impl Module { let _ = >::translate::, _>( |k| { - k.map(|k| k.into_iter() - .map(|(val, old_keys)| (val.clone(), upgrade(val, old_keys))) - .collect::>()) + k.map(|k| { + let keys = k.into_iter() + .map(|(val, old_keys)| (val.clone(), upgrade(val, old_keys))) + .collect::>(); + // Should never truncate since queued keys is already bounded. + BoundedVec::<(T::ValidatorId, T::Keys), T::MaxValidators>::truncating_from( + keys, + Some("Session Upgrade Keys"), + ) + }) } ); } @@ -846,7 +876,7 @@ impl ValidatorSet for Module { } fn validators() -> Vec { - Module::::validators() + Module::::validators().to_vec() } } diff --git a/frame/session/src/mock.rs b/frame/session/src/mock.rs index 3459ab73d6afe..a3dbe9fff89dc 100644 --- a/frame/session/src/mock.rs +++ b/frame/session/src/mock.rs @@ -119,7 +119,7 @@ impl ShouldEndSession for TestShouldEndSession { } pub struct TestSessionHandler; -impl SessionHandler for TestSessionHandler { +impl SessionHandler for TestSessionHandler { const KEY_TYPE_IDS: &'static [sp_runtime::KeyTypeId] = &[UintAuthorityId::ID]; fn on_genesis_session(_validators: &[(u64, T)]) {} fn on_new_session( @@ -262,6 +262,7 @@ impl pallet_timestamp::Config for Test { parameter_types! { pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(33); + pub const MaxValidators: u32 = 10; } impl Config for Test { @@ -273,6 +274,7 @@ impl Config for Test { type SessionHandler = TestSessionHandler; type ValidatorId = u64; type ValidatorIdOf = ConvertInto; + type MaxValidators = MaxValidators; type Keys = MockSessionKeys; type Event = Event; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; diff --git a/frame/session/src/tests.rs b/frame/session/src/tests.rs index f48388b5a002c..767d594351939 100644 --- a/frame/session/src/tests.rs +++ b/frame/session/src/tests.rs @@ -40,7 +40,7 @@ fn initialize_block(block: u64) { fn simple_setup_should_work() { new_test_ext().execute_with(|| { assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2), UintAuthorityId(3)]); - assert_eq!(Session::validators(), vec![1, 2, 3]); + assert_eq!(Session::validators().to_vec(), vec![1, 2, 3]); }); } @@ -56,7 +56,7 @@ fn put_get_keys() { fn keys_cleared_on_kill() { let mut ext = new_test_ext(); ext.execute_with(|| { - assert_eq!(Session::validators(), vec![1, 2, 3]); + assert_eq!(Session::validators().to_vec(), vec![1, 2, 3]); assert_eq!(Session::load_keys(&1), Some(UintAuthorityId(1).into())); let id = DUMMY; @@ -79,22 +79,22 @@ fn authorities_should_track_validators() { set_next_validators(vec![1, 2]); force_new_session(); initialize_block(1); - assert_eq!(Session::queued_keys(), vec![ + assert_eq!(Session::queued_keys().to_vec(), vec![ (1, UintAuthorityId(1).into()), (2, UintAuthorityId(2).into()), ]); - assert_eq!(Session::validators(), vec![1, 2, 3]); + assert_eq!(Session::validators().to_vec(), vec![1, 2, 3]); assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2), UintAuthorityId(3)]); assert!(before_session_end_called()); reset_before_session_end_called(); force_new_session(); initialize_block(2); - assert_eq!(Session::queued_keys(), vec![ + assert_eq!(Session::queued_keys().to_vec(), vec![ (1, UintAuthorityId(1).into()), (2, UintAuthorityId(2).into()), ]); - assert_eq!(Session::validators(), vec![1, 2]); + assert_eq!(Session::validators().to_vec(), vec![1, 2]); assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2)]); assert!(before_session_end_called()); reset_before_session_end_called(); @@ -103,23 +103,23 @@ fn authorities_should_track_validators() { assert_ok!(Session::set_keys(Origin::signed(4), UintAuthorityId(4).into(), vec![])); force_new_session(); initialize_block(3); - assert_eq!(Session::queued_keys(), vec![ + assert_eq!(Session::queued_keys().to_vec(), vec![ (1, UintAuthorityId(1).into()), (2, UintAuthorityId(2).into()), (4, UintAuthorityId(4).into()), ]); - assert_eq!(Session::validators(), vec![1, 2]); + assert_eq!(Session::validators().to_vec(), vec![1, 2]); assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2)]); assert!(before_session_end_called()); force_new_session(); initialize_block(4); - assert_eq!(Session::queued_keys(), vec![ + assert_eq!(Session::queued_keys().to_vec(), vec![ (1, UintAuthorityId(1).into()), (2, UintAuthorityId(2).into()), (4, UintAuthorityId(4).into()), ]); - assert_eq!(Session::validators(), vec![1, 2, 4]); + assert_eq!(Session::validators().to_vec(), vec![1, 2, 4]); assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2), UintAuthorityId(4)]); }); } @@ -437,7 +437,7 @@ fn upgrade_keys() { // Check queued keys. assert_eq!( - Session::queued_keys(), + Session::queued_keys().to_vec(), vec![ (1, mock_keys_for(1)), (2, mock_keys_for(2)), diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 1d8a5c1fd6451..4c0691391ce4e 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -29,7 +29,6 @@ pub use frame_benchmarking::{ const SEED: u32 = 0; const MAX_SPANS: u32 = 100; -const MAX_VALIDATORS: u32 = 1000; const MAX_SLASHES: u32 = 1000; // Add slashing spans to a user account. Not relevant for actual use, only to benchmark @@ -300,7 +299,7 @@ benchmarks! { } set_validator_count { - let validator_count = MAX_VALIDATORS; + let validator_count = T::MaxValidators::get(); }: _(RawOrigin::Root, validator_count) verify { assert_eq!(ValidatorCount::get(), validator_count); @@ -317,7 +316,7 @@ benchmarks! { // Worst case scenario, the list of invulnerables is very long. set_invulnerables { - let v in 0 .. MAX_VALIDATORS; + let v in 0 .. T::MaxValidators::get(); let mut invulnerables = Vec::new(); for i in 0 .. v { invulnerables.push(account("invulnerable", i, SEED)); diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index c938dceb76e49..ffc310fdd0f62 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -284,11 +284,11 @@ use sp_std::{ result, prelude::*, collections::btree_map::BTreeMap, - convert::From, + convert::{From, TryInto}, }; use codec::{HasCompact, Encode, Decode}; use frame_support::{ - decl_module, decl_event, decl_storage, ensure, decl_error, + decl_module, decl_event, decl_storage, ensure, decl_error, BoundedVec, weights::{ Weight, WithPostDispatchInfo, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}, @@ -648,7 +648,10 @@ impl SessionInterface<::AccountId> for T w FullIdentification = Exposure<::AccountId, BalanceOf>, FullIdentificationOf = ExposureOf, >, - T::SessionHandler: pallet_session::SessionHandler<::AccountId>, + T::SessionHandler: pallet_session::SessionHandler< + ::AccountId, + ::MaxValidators, + >, T::SessionManager: pallet_session::SessionManager<::AccountId>, T::ValidatorIdOf: Convert<::AccountId, Option<::AccountId>>, @@ -658,7 +661,7 @@ impl SessionInterface<::AccountId> for T w } fn validators() -> Vec<::AccountId> { - >::validators() + >::validators().to_vec() } fn prune_historical_up_to(up_to: SessionIndex) { @@ -785,6 +788,9 @@ pub trait Config: frame_system::Config + SendTransactionTypes> { /// their reward. This used to limit the i/o cost for the nominator payout. type MaxNominatorRewardedPerValidator: Get; + /// The maximum number of validators supported by the session handler and should be selected by the staking system. + type MaxValidators: Get; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -848,7 +854,7 @@ decl_storage! { /// Any validators that may never be slashed or forcibly kicked. It's a Vec since they're /// easy to initialize and the performance hit is minimal (we expect no more than four /// invulnerables) and restricted to testnets. - pub Invulnerables get(fn invulnerables) config(): Vec; + pub Invulnerables get(fn invulnerables): BoundedVec; /// Map from all locked "stash" accounts to the controller account. pub Bonded get(fn bonded): map hasher(twox_64_concat) T::AccountId => Option; @@ -952,6 +958,7 @@ decl_storage! { pub CanceledSlashPayout get(fn canceled_payout) config(): BalanceOf; /// All unapplied slashes that are queued for later. + // TODO: BOUND THIS VEC pub UnappliedSlashes: map hasher(twox_64_concat) EraIndex => Vec>>; @@ -998,7 +1005,11 @@ decl_storage! { add_extra_genesis { config(stakers): Vec<(T::AccountId, T::AccountId, BalanceOf, StakerStatus)>; + config(invulnerables): Vec; build(|config: &GenesisConfig| { + let bounded_invulnerables: BoundedVec + = config.invulnerables.clone().try_into().expect("Too many invulnerables!"); + Invulnerables::::put(bounded_invulnerables); for &(ref stash, ref controller, balance, ref status) in &config.stakers { assert!( T::Currency::free_balance(&stash) >= balance, @@ -1149,6 +1160,8 @@ decl_error! { TooManyTargets, /// A nomination target was supplied that was blocked or otherwise not a validator. BadTarget, + /// Validator count cannot be greater than the configured `MaxValidators`. + TooManyValidators, } } @@ -1621,6 +1634,7 @@ decl_module! { #[weight = T::WeightInfo::set_validator_count()] fn set_validator_count(origin, #[compact] new: u32) { ensure_root(origin)?; + ensure!(new <= T::MaxValidators::get(), Error::::TooManyValidators); ValidatorCount::put(new); } @@ -1634,7 +1648,11 @@ decl_module! { #[weight = T::WeightInfo::set_validator_count()] fn increase_validator_count(origin, #[compact] additional: u32) { ensure_root(origin)?; - ValidatorCount::mutate(|n| *n += additional); + ValidatorCount::try_mutate(|n| -> DispatchResult { + *n += additional; + ensure!(*n <= T::MaxValidators::get(), Error::::TooManyValidators); + Ok(()) + })?; } /// Scale up the ideal number of validators by a factor. @@ -1647,7 +1665,11 @@ decl_module! { #[weight = T::WeightInfo::set_validator_count()] fn scale_validator_count(origin, factor: Percent) { ensure_root(origin)?; - ValidatorCount::mutate(|n| *n += factor * *n); + ValidatorCount::try_mutate(|n| -> DispatchResult { + *n += factor * *n; + ensure!(*n <= T::MaxValidators::get(), Error::::TooManyValidators); + Ok(()) + })?; } /// Force there to be no new eras indefinitely. @@ -1692,7 +1714,9 @@ decl_module! { #[weight = T::WeightInfo::set_invulnerables(invulnerables.len() as u32)] fn set_invulnerables(origin, invulnerables: Vec) { ensure_root(origin)?; - >::put(invulnerables); + let bounded_invulnerables: BoundedVec + = invulnerables.try_into().map_err(|_| Error::::TooManyValidators)?; + >::put(bounded_invulnerables); } /// Force a current staker to become completely unstaked, immediately. @@ -2754,7 +2778,10 @@ where FullIdentification = Exposure<::AccountId, BalanceOf>, FullIdentificationOf = ExposureOf, >, - T::SessionHandler: pallet_session::SessionHandler<::AccountId>, + T::SessionHandler: pallet_session::SessionHandler< + ::AccountId, + ::MaxValidators, + >, T::SessionManager: pallet_session::SessionManager<::AccountId>, T::ValidatorIdOf: Convert< ::AccountId, diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 188eda801095e..fa1cabb001900 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -51,7 +51,7 @@ thread_local! { /// Another session handler struct to test on_disabled. pub struct OtherSessionHandler; -impl OneSessionHandler for OtherSessionHandler { +impl OneSessionHandler for OtherSessionHandler { type Key = UintAuthorityId; fn on_genesis_session<'a, I: 'a>(_: I) @@ -180,6 +180,7 @@ impl pallet_session::Config for Test { type ValidatorIdOf = crate::StashOf; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; type NextSessionRotation = pallet_session::PeriodicSessions; + type MaxValidators = MaxValidators; type WeightInfo = (); } @@ -240,6 +241,11 @@ impl onchain::Config for Test { type Accuracy = Perbill; type DataProvider = Staking; } + +parameter_types! { + pub const MaxValidators: u32 = 10; +} + impl Config for Test { const MAX_NOMINATIONS: u32 = 16; type Currency = Balances; @@ -258,6 +264,7 @@ impl Config for Test { type NextNewSession = Session; type MaxNominatorRewardedPerValidator = MaxNominatorRewardedPerValidator; type ElectionProvider = onchain::OnChainSequentialPhragmen; + type MaxValidators = MaxValidators; type WeightInfo = (); } @@ -445,7 +452,7 @@ impl ExtBuilder { let mut ext = sp_io::TestExternalities::from(storage); ext.execute_with(|| { - let validators = Session::validators(); + let validators = Session::validators().to_vec(); SESSION.with(|x| *x.borrow_mut() = (validators.clone(), HashSet::new())); }); diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 05eb6fdc5e028..06507007987cf 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -3099,7 +3099,7 @@ fn six_session_delay() { ExtBuilder::default().initialize_first_session(false).build_and_execute(|| { use pallet_session::SessionManager; - let val_set = Session::validators(); + let val_set = Session::validators().to_vec(); let init_session = Session::current_index(); let init_active_era = Staking::active_era().unwrap().index; diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index 9fcfe4035294f..b4e8cf3420e88 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -20,6 +20,8 @@ use sp_std::prelude::*; use sp_std::{convert::TryFrom, marker::PhantomData}; +#[cfg(feature = "std")] +use serde::{Serialize, Deserialize}; use codec::{FullCodec, Encode, EncodeLike, Decode}; use crate::{ traits::Get, @@ -38,6 +40,7 @@ impl BoundedVecValue for T {} /// As the name suggests, the length of the queue is always bounded. All internal operations ensure /// this bound is respected. #[derive(Encode, Decode, crate::DefaultNoBound, crate::CloneNoBound, crate::DebugNoBound)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub struct BoundedVec>(Vec, PhantomData); // NOTE: we could also implement this as: @@ -65,6 +68,21 @@ impl> BoundedVec { Self(t, Default::default()) } + /// Create self from `t` in a lossy way. In case too many items exist, a log with the given + /// scope is emitted and `t` is truncated to fit the size. + pub fn truncating_from(mut t: Vec, scope: Option<&'static str>) -> Self { + if t.len() > Self::bound() { + log::warn!( + target: crate::LOG_TARGET, + "length of a bounded vector in scope {} is not respected. Truncating...", + scope.unwrap_or("UNKNOWN"), + ); + t.truncate(Self::bound()) + } + + Self::unchecked_from(t) + } + /// Create `Self` from `t` without any checks. Logs warnings if the bound is not being /// respected. The additional scope can be used to indicate where a potential overflow is /// happening. @@ -76,7 +94,7 @@ impl> BoundedVec { if t.len() > Self::bound() { log::warn!( target: crate::LOG_TARGET, - "length of a bounded vector in scope {} is not respected.", + "length of a bounded vector in scope {} is not respected. Forcing...", scope.unwrap_or("UNKNOWN"), ); } @@ -467,4 +485,10 @@ pub mod test { assert_eq!(bounded.len(), 7); assert!(bounded.try_mutate(|v| v.push(8)).is_none()); } + + #[test] + fn truncating_from_works() { + let bounded = BoundedVec::::truncating_from(vec![1, 2, 3, 4, 5, 6], None); + assert_eq!(*bounded, vec![1, 2, 3, 4]); + } } diff --git a/frame/support/src/traits/validation.rs b/frame/support/src/traits/validation.rs index 900be7bb8e7e2..c3ab7c9649858 100644 --- a/frame/support/src/traits/validation.rs +++ b/frame/support/src/traits/validation.rs @@ -72,7 +72,7 @@ pub trait VerifySeal { } /// A session handler for specific key type. -pub trait OneSessionHandler: BoundToRuntimeAppPublic { +pub trait OneSessionHandler: BoundToRuntimeAppPublic { /// The key type expected. type Key: Decode + Default + RuntimeAppPublic; diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 150bc403732c7..ef2a798596366 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -529,6 +529,7 @@ impl pallet_timestamp::Config for Runtime { parameter_types! { pub const EpochDuration: u64 = 6; pub const ExpectedBlockTime: u64 = 10_000; + pub const MaxAuthorities: u32 = 100; } impl pallet_babe::Config for Runtime { @@ -551,6 +552,8 @@ impl pallet_babe::Config for Runtime { type HandleEquivocation = (); + type MaxAuthorities = MaxAuthorities; + type WeightInfo = (); }