From 6e1ecade3c569fbc901dc4e644abdae860fc8939 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 7 Jun 2022 15:01:56 +0100 Subject: [PATCH 1/2] Bounded collator selection pallet --- pallets/collator-selection/src/lib.rs | 69 ++++++++++++++------------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/pallets/collator-selection/src/lib.rs b/pallets/collator-selection/src/lib.rs index 0aeb44dc68e..f6ab4fe9682 100644 --- a/pallets/collator-selection/src/lib.rs +++ b/pallets/collator-selection/src/lib.rs @@ -89,7 +89,7 @@ pub mod pallet { ValidatorRegistration, }, weights::DispatchClass, - PalletId, + BoundedVec, PalletId, }; use frame_system::{pallet_prelude::*, Config as SystemConfig}; use pallet_session::SessionManager; @@ -123,8 +123,7 @@ pub mod pallet { /// Account Identifier from which the internal Pot is generated. type PotId: Get; - /// Maximum number of candidates that we should have. This is used for benchmarking and is not - /// enforced. + /// Maximum number of candidates that we should have. This is enforced in code. /// /// This does not take into account the invulnerables. type MaxCandidates: Get; @@ -134,9 +133,7 @@ pub mod pallet { /// This does not take into account the invulnerables. type MinCandidates: Get; - /// Maximum number of invulnerables. - /// - /// Used only for benchmarking. + /// Maximum number of invulnerables. This is enforced in code. type MaxInvulnerables: Get; // Will be kicked if block is not produced in threshold. @@ -158,7 +155,9 @@ pub mod pallet { } /// Basic information about a collation candidate. - #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, scale_info::TypeInfo)] + #[derive( + PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, scale_info::TypeInfo, MaxEncodedLen, + )] pub struct CandidateInfo { /// Account identifier. pub who: AccountId, @@ -168,19 +167,22 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] - #[pallet::without_storage_info] pub struct Pallet(_); /// The invulnerable, fixed collators. #[pallet::storage] #[pallet::getter(fn invulnerables)] - pub type Invulnerables = StorageValue<_, Vec, ValueQuery>; + pub type Invulnerables = + StorageValue<_, BoundedVec, ValueQuery>; /// The (community, limited) collation candidates. #[pallet::storage] #[pallet::getter(fn candidates)] - pub type Candidates = - StorageValue<_, Vec>>, ValueQuery>; + pub type Candidates = StorageValue< + _, + BoundedVec>, T::MaxCandidates>, + ValueQuery, + >; /// Last block authored by collator. #[pallet::storage] @@ -230,10 +232,9 @@ pub mod pallet { "duplicate invulnerables in genesis." ); - assert!( - T::MaxInvulnerables::get() >= (self.invulnerables.len() as u32), - "genesis invulnerables are more than T::MaxInvulnerables", - ); + let bounded_invulnerables = + BoundedVec::<_, T::MaxInvulnerables>::try_from(self.invulnerables.clone()) + .expect("genesis invulnerables are more than T::MaxInvulnerables"); assert!( T::MaxCandidates::get() >= self.desired_candidates, "genesis desired_candidates are more than T::MaxCandidates", @@ -241,7 +242,7 @@ pub mod pallet { >::put(&self.desired_candidates); >::put(&self.candidacy_bond); - >::put(&self.invulnerables); + >::put(bounded_invulnerables); } } @@ -270,6 +271,8 @@ pub mod pallet { AlreadyCandidate, /// User is not a candidate NotCandidate, + /// Too many invulnerables + TooManyInvulnerables, /// User is already an Invulnerable AlreadyInvulnerable, /// Account has no associated validator ID @@ -290,15 +293,11 @@ pub mod pallet { new: Vec, ) -> DispatchResultWithPostInfo { T::UpdateOrigin::ensure_origin(origin)?; - // we trust origin calls, this is just a for more accurate benchmarking - if (new.len() as u32) > T::MaxInvulnerables::get() { - log::warn!( - "invulnerables > T::MaxInvulnerables; you might need to run benchmarks again" - ); - } + let bounded_invulnerables = BoundedVec::<_, T::MaxInvulnerables>::try_from(new) + .map_err(|_| Error::::TooManyInvulnerables)?; // check if the invulnerables have associated validator keys before they are set - for account_id in &new { + for account_id in bounded_invulnerables.iter() { let validator_key = T::ValidatorIdOf::convert(account_id.clone()) .ok_or(Error::::NoAssociatedValidatorId)?; ensure!( @@ -307,8 +306,10 @@ pub mod pallet { ); } - >::put(&new); - Self::deposit_event(Event::NewInvulnerables { invulnerables: new }); + >::put(&bounded_invulnerables); + Self::deposit_event(Event::NewInvulnerables { + invulnerables: bounded_invulnerables.to_vec(), + }); Ok(().into()) } @@ -372,7 +373,7 @@ pub mod pallet { Err(Error::::AlreadyCandidate)? } else { T::Currency::reserve(&who, deposit)?; - candidates.push(incoming); + candidates.try_push(incoming).map_err(|_| Error::::TooManyCandidates)?; >::insert( who.clone(), frame_system::Pallet::::block_number() + T::KickThreshold::get(), @@ -430,17 +431,19 @@ pub mod pallet { /// Assemble the current set of candidates and invulnerables into the next collator set. /// /// This is done on the fly, as frequent as we are told to do so, as the session manager. - pub fn assemble_collators(candidates: Vec) -> Vec { - let mut collators = Self::invulnerables(); + pub fn assemble_collators( + candidates: BoundedVec, + ) -> Vec { + let mut collators = Self::invulnerables().to_vec(); collators.extend(candidates); - collators + collators.to_vec() } /// Kicks out candidates that did not produce a block in the kick threshold /// and refund their deposits. pub fn kick_stale_candidates( - candidates: Vec>>, - ) -> Vec { + candidates: BoundedVec>, T::MaxCandidates>, + ) -> BoundedVec { let now = frame_system::Pallet::::block_number(); let kick_threshold = T::KickThreshold::get(); candidates @@ -461,7 +464,9 @@ pub mod pallet { None } }) - .collect() + .collect::>() + .try_into() + .expect("filter_map operation can't result in a bounded vec larger than its original; qed") } } From 4f80dd11af63e51168cab55e7fd632d27e5aeb9f Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sun, 12 Jun 2022 01:29:45 +0100 Subject: [PATCH 2/2] Update pallets/collator-selection/src/lib.rs Co-authored-by: Squirrel --- pallets/collator-selection/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/collator-selection/src/lib.rs b/pallets/collator-selection/src/lib.rs index f6ab4fe9682..918ec95a3d4 100644 --- a/pallets/collator-selection/src/lib.rs +++ b/pallets/collator-selection/src/lib.rs @@ -436,7 +436,7 @@ pub mod pallet { ) -> Vec { let mut collators = Self::invulnerables().to_vec(); collators.extend(candidates); - collators.to_vec() + collators } /// Kicks out candidates that did not produce a block in the kick threshold