min collator check (#498)

* min collator check

* change statemint/mine min candidates

* Ci pass

* Update pallets/collator-selection/src/lib.rs

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

* Update pallets/collator-selection/src/lib.rs

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

* Apply suggestions from code review

* build fixes

* add error messages to errors

* added validator register check

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

cumulus/pallets/collator-selection/src/lib.rs

@@ -41,6 +41,9 @@
 //! The current implementation resolves congestion of [`Candidates`] in a first-come-first-serve
 //! manner.
 //!
+//! Candidates will not be allowed to get kicked or leave_intent if the total number of candidates
+//! fall below MinCandidates. This is for potential disaster recovery scenarios.
+//!
 //! ### Rewards
 //!
 //! The Collator Selection pallet maintains an on-chain account (the "Pot"). In each block, the
@@ -76,7 +79,7 @@ pub mod pallet {
 		pallet_prelude::*,
 		inherent::Vec,
 		traits::{
-			Currency, ReservableCurrency, EnsureOrigin, ExistenceRequirement::KeepAlive,
+			Currency, ReservableCurrency, EnsureOrigin, ExistenceRequirement::KeepAlive, ValidatorRegistration
 		},
 		PalletId,
 	};
@@ -89,6 +92,7 @@ pub mod pallet {
 		},
 		weights::DispatchClass,
 	};
+	use sp_runtime::traits::Convert;
 	use core::ops::Div;
 	use pallet_session::SessionManager;
 	use sp_staking::SessionIndex;
@@ -127,6 +131,12 @@ pub mod pallet {
 		/// This does not take into account the invulnerables.
 		type MaxCandidates: Get<u32>;
 
+		/// Minimum number of candidates that we should have. This is used for disaster recovery.
+		///
+		/// This does not take into account the invulnerables.
+		type MinCandidates: Get<u32>;
+
+
 		/// Maximum number of invulnerables.
 		///
 		/// Used only for benchmarking.
@@ -135,6 +145,18 @@ pub mod pallet {
 		// Will be kicked if block is not produced in threshold.
 		type KickThreshold: Get<Self::BlockNumber>;
 
+		/// A stable ID for a validator.
+		type ValidatorId: Member + Parameter;
+
+		/// A conversion from account ID to validator ID.
+		///
+		/// Its cost must be at most one storage read.
+		type ValidatorIdOf: Convert<Self::AccountId, Option<Self::ValidatorId>>;
+
+		/// Validate a user is registered
+		type ValidatorRegistration: ValidatorRegistration<Self::ValidatorId>;
+
+
 		/// The weight information of this pallet.
 		type WeightInfo: WeightInfo;
 	}
@@ -238,13 +260,24 @@ pub mod pallet {
 	// Errors inform users that something went wrong.
 	#[pallet::error]
 	pub enum Error<T> {
+		/// Too many candidates
 		TooManyCandidates,
+		/// Too few candidates
+		TooFewCandidates,
+		/// Unknown error
 		Unknown,
+		/// Permission issue
 		Permission,
+		/// User is already a candidate
 		AlreadyCandidate,
+		/// User is not a candidate
 		NotCandidate,
+		/// User is already an Invulnerable
 		AlreadyInvulnerable,
-		InvalidProof,
+		/// Account has no associated validator ID
+		NoAssociatedValidatorId,
+		/// Validator ID is not yet registered
+		ValidatorNotRegistered
 	}
 
 	#[pallet::hooks]
@@ -300,6 +333,9 @@ pub mod pallet {
 			ensure!((length as u32) < Self::desired_candidates(), Error::<T>::TooManyCandidates);
 			ensure!(!Self::invulnerables().contains(&who), Error::<T>::AlreadyInvulnerable);
 
+			let validator_key = T::ValidatorIdOf::convert(who.clone()).ok_or(Error::<T>::NoAssociatedValidatorId)?;
+			ensure!(T::ValidatorRegistration::is_registered(&validator_key), Error::<T>::ValidatorNotRegistered);
+
 			let deposit = Self::candidacy_bond();
 			// First authored block is current block plus kick threshold to handle session delay
 			let incoming = CandidateInfo { who: who.clone(), deposit };
@@ -323,7 +359,7 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::leave_intent(T::MaxCandidates::get()))]
 		pub fn leave_intent(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
 			let who = ensure_signed(origin)?;
-
+			ensure!(Self::candidates().len() as u32 > T::MinCandidates::get(), Error::<T>::TooFewCandidates);
 			let current_count = Self::try_remove_candidate(&who)?;
 
 			Ok(Some(T::WeightInfo::leave_intent(current_count as u32)).into())
@@ -365,7 +401,7 @@ pub mod pallet {
 			let new_candidates = candidates.into_iter().filter_map(|c| {
 				let last_block = <LastAuthoredBlock<T>>::get(c.who.clone());
 				let since_last = now.saturating_sub(last_block);
-				if since_last < kick_threshold {
+				if since_last < kick_threshold || Self::candidates().len() as u32 <= T::MinCandidates::get() {
 					Some(c.who)
 				} else {
 					let outcome = Self::try_remove_candidate(&c.who);

cumulus/pallets/collator-selection/src/mock.rs

@@ -18,7 +18,7 @@ use crate as collator_selection;
 use sp_core::H256;
 use frame_support::{
 	parameter_types, ord_parameter_types,
-	traits::{FindAuthor, GenesisBuild},
+	traits::{FindAuthor, GenesisBuild, ValidatorRegistration},
 	PalletId
 };
 use sp_runtime::{
@@ -188,6 +188,18 @@ parameter_types! {
 	pub const PotId: PalletId = PalletId(*b"PotStake");
 	pub const MaxCandidates: u32 = 20;
 	pub const MaxInvulnerables: u32 = 20;
+	pub const MinCandidates: u32 = 1;
+}
+
+pub struct IsRegistered;
+impl ValidatorRegistration<u64> for IsRegistered {
+	fn is_registered(id: &u64) -> bool {
+		if *id == 7u64 {
+			false
+		} else {
+			true
+		}
+	}
 }
 
 impl Config for Test {
@@ -196,8 +208,12 @@ impl Config for Test {
 	type UpdateOrigin = EnsureSignedBy<RootAccount, u64>;
 	type PotId = PotId;
 	type MaxCandidates = MaxCandidates;
+	type MinCandidates = MinCandidates;
 	type MaxInvulnerables = MaxInvulnerables;
 	type KickThreshold = Period;
+	type ValidatorId = <Self as frame_system::Config>::AccountId;
+	type ValidatorIdOf = IdentityCollator;
+	type ValidatorRegistration = IsRegistered;
 	type WeightInfo = ();
 }
 

cumulus/pallets/collator-selection/src/tests.rs

@@ -106,6 +106,21 @@ fn cannot_register_candidate_if_too_many() {
 	})
 }
 
+#[test]
+fn cannot_unregister_candidate_if_too_few() {
+	new_test_ext().execute_with(|| {
+		// reset desired candidates:
+		<crate::DesiredCandidates<Test>>::put(1);
+		assert_ok!(CollatorSelection::register_as_candidate(Origin::signed(4)));
+
+		// can not remove too few
+		assert_noop!(
+			CollatorSelection::leave_intent(Origin::signed(4)),
+			Error::<Test>::TooFewCandidates,
+		);
+	})
+}
+
 #[test]
 fn cannot_register_as_candidate_if_invulnerable() {
 	new_test_ext().execute_with(|| {
@@ -119,6 +134,17 @@ fn cannot_register_as_candidate_if_invulnerable() {
 	})
 }
 
+#[test]
+fn cannot_register_as_candidate_if_keys_not_registered() {
+	new_test_ext().execute_with(|| {
+		// can't 7 because keys not registered.
+		assert_noop!(
+			CollatorSelection::register_as_candidate(Origin::signed(7)),
+			Error::<Test>::ValidatorNotRegistered
+		);
+	})
+}
+
 #[test]
 fn cannot_register_dupe_candidate() {
 	new_test_ext().execute_with(|| {
@@ -184,6 +210,10 @@ fn leave_intent() {
 		assert_ok!(CollatorSelection::register_as_candidate(Origin::signed(3)));
 		assert_eq!(Balances::free_balance(3), 90);
 
+		// register too so can leave above min candidates
+		assert_ok!(CollatorSelection::register_as_candidate(Origin::signed(5)));
+		assert_eq!(Balances::free_balance(5), 90);
+
 		// cannot leave if not candidate.
 		assert_noop!(
 			CollatorSelection::leave_intent(Origin::signed(4)),
@@ -318,6 +348,34 @@ fn kick_mechanism() {
 	});
 }
 
+#[test]
+fn should_not_kick_mechanism_too_few() {
+	new_test_ext().execute_with(|| {
+		// add a new collator
+		assert_ok!(CollatorSelection::register_as_candidate(Origin::signed(3)));
+		assert_ok!(CollatorSelection::register_as_candidate(Origin::signed(5)));
+		initialize_to_block(10);
+		assert_eq!(CollatorSelection::candidates().len(), 2);
+		initialize_to_block(20);
+		assert_eq!(SessionChangeBlock::get(), 20);
+		// 4 authored this block, 5 gets to stay too few 3 was kicked
+		assert_eq!(CollatorSelection::candidates().len(), 1);
+		// 3 will be kicked after 1 session delay
+		assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 3, 5]);
+		let collator = CandidateInfo {
+			who: 5,
+			deposit: 10,
+		};
+		assert_eq!(CollatorSelection::candidates(), vec![collator]);
+		assert_eq!(CollatorSelection::last_authored_block(4), 20);
+		initialize_to_block(30);
+		// 3 gets kicked after 1 session delay
+		assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 5]);
+		// kicked collator gets funds back
+		assert_eq!(Balances::free_balance(3), 100);
+	});
+}
+
 
 #[test]
 #[should_panic = "duplicate invulnerables in genesis."]

cumulus/polkadot-parachains/statemine-runtime/src/lib.rs

@@ -638,6 +638,7 @@ impl pallet_aura::Config for Runtime {
 parameter_types! {
 	pub const PotId: PalletId = PalletId(*b"PotStake");
 	pub const MaxCandidates: u32 = 1000;
+	pub const MinCandidates: u32 = 5;
 	pub const SessionLength: BlockNumber = 6 * HOURS;
 	pub const MaxInvulnerables: u32 = 100;
 }
@@ -655,9 +656,13 @@ impl pallet_collator_selection::Config for Runtime {
 	type UpdateOrigin = CollatorSelectionUpdateOrigin;
 	type PotId = PotId;
 	type MaxCandidates = MaxCandidates;
+	type MinCandidates = MinCandidates;
 	type MaxInvulnerables = MaxInvulnerables;
 	// should be a multiple of session or things will get inconsistent
 	type KickThreshold = Period;
+	type ValidatorId = <Self as frame_system::Config>::AccountId;
+	type ValidatorIdOf = pallet_collator_selection::IdentityCollator;
+	type ValidatorRegistration = Session;
 	type WeightInfo = weights::pallet_collator_selection::WeightInfo<Runtime>;
 }
 

cumulus/polkadot-parachains/statemint-common/src/impls.rs

@@ -65,7 +65,7 @@ where
 mod tests {
 	use super::*;
 	use frame_support::traits::FindAuthor;
-	use frame_support::{parameter_types, PalletId};
+	use frame_support::{parameter_types, PalletId, traits::ValidatorRegistration};
 	use frame_system::{limits, EnsureRoot};
 	use polkadot_primitives::v1::AccountId;
 	use sp_core::H256;
@@ -74,6 +74,7 @@ mod tests {
 		traits::{BlakeTwo256, IdentityLookup},
 		Perbill,
 	};
+	use pallet_collator_selection::IdentityCollator;
 
 	type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
 	type Block = frame_system::mocking::MockBlock<Test>;
@@ -145,10 +146,18 @@ mod tests {
 		}
 	}
 
+	pub struct IsRegistered;
+	impl ValidatorRegistration<AccountId> for IsRegistered {
+		fn is_registered(_id: &AccountId) -> bool {
+			true
+		}
+	}
+
 	parameter_types! {
 		pub const PotId: PalletId = PalletId(*b"PotStake");
 		pub const MaxCandidates: u32 = 20;
 		pub const MaxInvulnerables: u32 = 20;
+		pub const MinCandidates: u32 = 1;
 	}
 
 	impl pallet_collator_selection::Config for Test {
@@ -157,7 +166,11 @@ mod tests {
 		type UpdateOrigin = EnsureRoot<AccountId>;
 		type PotId = PotId;
 		type MaxCandidates = MaxCandidates;
+		type MinCandidates = MinCandidates;
 		type MaxInvulnerables = MaxInvulnerables;
+		type ValidatorId = <Self as frame_system::Config>::AccountId;
+		type ValidatorIdOf = IdentityCollator;
+		type ValidatorRegistration = IsRegistered;
 		type KickThreshold = ();
 		type WeightInfo = ();
 	}

cumulus/polkadot-parachains/statemint-runtime/src/lib.rs

@@ -579,6 +579,7 @@ impl pallet_aura::Config for Runtime {
 parameter_types! {
 	pub const PotId: PalletId = PalletId(*b"PotStake");
 	pub const MaxCandidates: u32 = 1000;
+	pub const MinCandidates: u32 = 5;
 	pub const SessionLength: BlockNumber = 6 * HOURS;
 	pub const MaxInvulnerables: u32 = 100;
 }
@@ -596,9 +597,13 @@ impl pallet_collator_selection::Config for Runtime {
 	type UpdateOrigin = CollatorSelectionUpdateOrigin;
 	type PotId = PotId;
 	type MaxCandidates = MaxCandidates;
+	type MinCandidates = MinCandidates;
 	type MaxInvulnerables = MaxInvulnerables;
 	// should be a multiple of session or things will get inconsistent
 	type KickThreshold = Period;
+	type ValidatorId = <Self as frame_system::Config>::AccountId;
+	type ValidatorIdOf = pallet_collator_selection::IdentityCollator;
+	type ValidatorRegistration = Session;
 	type WeightInfo = weights::pallet_collator_selection::WeightInfo<Runtime>;
 }
 

cumulus/polkadot-parachains/westmint-runtime/src/lib.rs

@@ -576,6 +576,7 @@ impl pallet_aura::Config for Runtime {
 parameter_types! {
 	pub const PotId: PalletId = PalletId(*b"PotStake");
 	pub const MaxCandidates: u32 = 1000;
+	pub const MinCandidates: u32 = 1;
 	pub const SessionLength: BlockNumber = 6 * HOURS;
 	pub const MaxInvulnerables: u32 = 100;
 }
@@ -586,9 +587,13 @@ impl pallet_collator_selection::Config for Runtime {
 	type UpdateOrigin = EnsureRoot<AccountId>;
 	type PotId = PotId;
 	type MaxCandidates = MaxCandidates;
+	type MinCandidates = MinCandidates;
 	type MaxInvulnerables = MaxInvulnerables;
 	// should be a multiple of session or things will get inconsistent
 	type KickThreshold = Period;
+	type ValidatorId = <Self as frame_system::Config>::AccountId;
+	type ValidatorIdOf = pallet_collator_selection::IdentityCollator;
+	type ValidatorRegistration = Session;
 	type WeightInfo = weights::pallet_collator_selection::WeightInfo<Runtime>;
 }