add call to kick invulnerable candidates

pallets/collator-selection/src/benchmarking.rs

@@ -25,6 +25,7 @@ use frame_benchmarking::{
 use frame_support::{
 	assert_ok,
 	codec::Decode,
+	dispatch::DispatchResult,
 	traits::{Currency, EnsureOrigin, Get},
 };
 use frame_system::{EventRecord, RawOrigin};
@@ -235,6 +236,37 @@ benchmarks! {
 		assert_last_event::<T>(Event::CandidateRemoved{account_id: leaving}.into());
 	}
 
+	remove_invulnerable_candidate {
+		let c in (T::MinEligibleCollators::get() + 1) .. T::MaxCandidates::get();
+		<CandidacyBond<T>>::put(T::Currency::minimum_balance());
+		<DesiredCandidates<T>>::put(c);
+
+		register_validators::<T>(c);
+		register_candidates::<T>(c);
+
+		let leaving = <Candidates<T>>::get().last().unwrap().who.clone();
+		let caller: T::AccountId = whitelisted_caller();
+
+		<Invulnerables<T>>::try_mutate(|invulnerables| -> DispatchResult {
+			match invulnerables.binary_search(&leaving) {
+				Ok(_) => return Ok(()),
+				Err(pos) => invulnerables
+					.try_insert(pos, leaving.clone())
+					.expect("it is short enough"),
+			}
+			Ok(())
+		}).expect("only returns Ok()");
+	}: {
+		assert_ok!(
+			<CollatorSelection<T>>::remove_invulnerable_candidate(
+				RawOrigin::Signed(caller).into(),
+				leaving.clone()
+			)
+		);
+	} verify {
+		assert_last_event::<T>(Event::CandidateRemoved{account_id: leaving}.into());
+	}
+
 	// worse case is paying a non-existing candidate account.
 	note_author {
 		<CandidacyBond<T>>::put(T::Currency::minimum_balance());

pallets/collator-selection/src/lib.rs

@@ -324,6 +324,7 @@ pub mod pallet {
 				// We don't really care about the outcome. If it succeeds, it just tells us how
 				// many candidates are left. If it fails, it's because `who` wasn't a candidate,
 				// which is the state we want anyway.
+				// Don't remove their last authored block.
 				let _outcome = Self::try_remove_candidate(&account_id, false);
 			}
 
@@ -423,9 +424,10 @@ pub mod pallet {
 		pub fn leave_intent(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
 			let who = ensure_signed(origin)?;
 			ensure!(
-				Self::eligible_collators() as u32 > T::MinEligibleCollators::get(),
+				Self::eligible_collators() > T::MinEligibleCollators::get(),
 				Error::<T>::TooFewEligibleCollators
 			);
+			// Do remove their last authored block.
 			let current_count = Self::try_remove_candidate(&who, true)?;
 
 			Ok(Some(T::WeightInfo::leave_intent(current_count as u32)).into())
@@ -452,6 +454,7 @@ pub mod pallet {
 			// `Invulnerables` and `Candidates` are mutually exclusive sets. We don't really care
 			// about the outcome - the only error is that they are not a candidate, which is the
 			// state we want.
+			// Don't remove their last authored block, as they are still a collator.
 			let _outcome = Self::try_remove_candidate(&who, false);
 
 			<Invulnerables<T>>::try_mutate(|invulnerables| -> DispatchResult {
@@ -478,7 +481,7 @@ pub mod pallet {
 			T::UpdateOrigin::ensure_origin(origin)?;
 
 			ensure!(
-				Self::eligible_collators() as u32 > T::MinEligibleCollators::get(),
+				Self::eligible_collators() > T::MinEligibleCollators::get(),
 				Error::<T>::TooFewEligibleCollators
 			);
 
@@ -492,6 +495,22 @@ pub mod pallet {
 			Self::deposit_event(Event::InvulnerableRemoved { account_id: who });
 			Ok(())
 		}
+
+		/// Remove a `Candidate` who is `Invulnerable`, as these sets should be mutually exclusive.
+		///
+		/// Any signed origin can call this.
+		#[pallet::call_index(7)]
+		#[pallet::weight(T::WeightInfo::remove_invulnerable_candidate(T::MaxCandidates::get()))]
+		pub fn remove_invulnerable_candidate(
+			origin: OriginFor<T>,
+			who: T::AccountId,
+		) -> DispatchResultWithPostInfo {
+			let _ = ensure_signed(origin)?;
+			ensure!(Self::invulnerables().contains(&who), Error::<T>::NotInvulnerable);
+			// We don't want to remove the last authored block because they are still a collator.
+			let current_count = Self::try_remove_candidate(&who, false)?;
+			Ok(Some(T::WeightInfo::remove_invulnerable_candidate(current_count as u32)).into())
+		}
 	}
 
 	impl<T: Config> Pallet<T> {
@@ -501,7 +520,11 @@ pub mod pallet {
 		}
 
 		fn eligible_collators() -> u32 {
-			Self::candidates().len().saturating_add(Self::invulnerables().len()).try_into().unwrap()
+			Self::candidates()
+				.len()
+				.saturating_add(Self::invulnerables().len())
+				.try_into()
+				.unwrap()
 		}
 
 		/// Removes a candidate if they exist and sends them back their deposit

pallets/collator-selection/src/mock.rs

@@ -188,9 +188,6 @@ ord_parameter_types! {
 
 parameter_types! {
 	pub const PotId: PalletId = PalletId(*b"PotStake");
-	pub const MaxCandidates: u32 = 20;
-	pub const MaxInvulnerables: u32 = 20;
-	pub const MinEligibleCollators: u32 = 1;
 }
 
 pub struct IsRegistered;
@@ -205,9 +202,9 @@ impl Config for Test {
 	type Currency = Balances;
 	type UpdateOrigin = EnsureSignedBy<RootAccount, u64>;
 	type PotId = PotId;
-	type MaxCandidates = MaxCandidates;
-	type MinEligibleCollators = MinEligibleCollators;
-	type MaxInvulnerables = MaxInvulnerables;
+	type MaxCandidates = ConstU32<20>;
+	type MinEligibleCollators = ConstU32<1>;
+	type MaxInvulnerables = ConstU32<20>;
 	type KickThreshold = Period;
 	type ValidatorId = <Self as frame_system::Config>::AccountId;
 	type ValidatorIdOf = IdentityCollator;

pallets/collator-selection/src/tests.rs

@@ -17,7 +17,8 @@ use crate as collator_selection;
 use crate::{mock::*, CandidateInfo, Error};
 use frame_support::{
 	assert_noop, assert_ok,
-	traits::{Currency, GenesisBuild, OnInitialize},
+	traits::{ConstU32, Currency, GenesisBuild, OnInitialize},
+	BoundedVec,
 };
 use pallet_balances::Error as BalancesError;
 use sp_runtime::{testing::UintAuthorityId, traits::BadOrigin};
@@ -194,7 +195,6 @@ fn remove_invulnerable_works() {
 fn candidate_to_invulnerable_works() {
 	new_test_ext().execute_with(|| {
 		initialize_to_block(1);
-		assert_eq!(<crate::tests::MinEligibleCollators>::get(), 1);
 		assert_eq!(CollatorSelection::desired_candidates(), 2);
 		assert_eq!(CollatorSelection::candidacy_bond(), 10);
 		assert_eq!(CollatorSelection::candidates(), Vec::new());
@@ -298,7 +298,10 @@ fn cannot_unregister_candidate_if_too_few() {
 	new_test_ext().execute_with(|| {
 		assert_eq!(CollatorSelection::candidates(), Vec::new());
 		assert_eq!(CollatorSelection::invulnerables(), vec![1, 2]);
-		assert_ok!(CollatorSelection::remove_invulnerable(RuntimeOrigin::signed(RootAccount::get()), 1));
+		assert_ok!(CollatorSelection::remove_invulnerable(
+			RuntimeOrigin::signed(RootAccount::get()),
+			1
+		));
 		assert_noop!(
 			CollatorSelection::remove_invulnerable(RuntimeOrigin::signed(RootAccount::get()), 2),
 			Error::<Test>::TooFewEligibleCollators,
@@ -309,7 +312,10 @@ fn cannot_unregister_candidate_if_too_few() {
 		assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(4)));
 
 		// now we can remove `2`
-		assert_ok!(CollatorSelection::remove_invulnerable(RuntimeOrigin::signed(RootAccount::get()), 2));
+		assert_ok!(CollatorSelection::remove_invulnerable(
+			RuntimeOrigin::signed(RootAccount::get()),
+			2
+		));
 
 		// can not remove too few
 		assert_noop!(
@@ -541,10 +547,16 @@ fn should_not_kick_mechanism_too_few() {
 		// remove the invulnerables and add new collators 3 and 5
 		assert_eq!(CollatorSelection::candidates(), Vec::new());
 		assert_eq!(CollatorSelection::invulnerables(), vec![1, 2]);
-		assert_ok!(CollatorSelection::remove_invulnerable(RuntimeOrigin::signed(RootAccount::get()), 1));
+		assert_ok!(CollatorSelection::remove_invulnerable(
+			RuntimeOrigin::signed(RootAccount::get()),
+			1
+		));
 		assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)));
 		assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(5)));
-		assert_ok!(CollatorSelection::remove_invulnerable(RuntimeOrigin::signed(RootAccount::get()), 2));
+		assert_ok!(CollatorSelection::remove_invulnerable(
+			RuntimeOrigin::signed(RootAccount::get()),
+			2
+		));
 
 		initialize_to_block(10);
 		assert_eq!(CollatorSelection::candidates().len(), 2);
@@ -567,6 +579,41 @@ fn should_not_kick_mechanism_too_few() {
 	});
 }
 
+#[test]
+fn kick_invulnerable_candidate_works() {
+	new_test_ext().execute_with(|| {
+		// remove the invulnerables and add new collators 3 and 4
+		assert_eq!(CollatorSelection::candidates(), Vec::new());
+		assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)));
+		assert_ok!(CollatorSelection::register_as_candidate(RuntimeOrigin::signed(4)));
+		assert_eq!(Balances::free_balance(3), 90);
+		assert_eq!(Balances::free_balance(4), 90);
+		// force set 3 as invulnerable. we have to force this because the pallet's logic should
+		// prevent it from happening.
+		let force_invulnerables =
+			BoundedVec::<_, ConstU32<20>>::try_from(vec![1_u64, 2_u64, 3_u64])
+				.expect("it's shorter");
+		<crate::Invulnerables<Test>>::put(force_invulnerables);
+
+		let collator_3 = CandidateInfo { who: 3, deposit: 10 };
+		let collator_4 = CandidateInfo { who: 4, deposit: 10 };
+
+		// remove 3 from candidates
+		assert_eq!(CollatorSelection::candidates(), vec![collator_3, collator_4.clone()]);
+		assert_ok!(CollatorSelection::remove_invulnerable_candidate(RuntimeOrigin::signed(1), 3));
+		assert_eq!(CollatorSelection::invulnerables(), vec![1, 2, 3]);
+		assert_eq!(CollatorSelection::candidates(), vec![collator_4]);
+		assert_eq!(Balances::free_balance(3), 100);
+
+		// should not be able to remove 4
+		assert_noop!(
+			CollatorSelection::remove_invulnerable_candidate(RuntimeOrigin::signed(1), 4),
+			Error::<Test>::NotInvulnerable
+		);
+		assert_eq!(Balances::free_balance(4), 90);
+	});
+}
+
 #[test]
 #[should_panic = "duplicate invulnerables in genesis."]
 fn cannot_set_genesis_value_twice() {

pallets/collator-selection/src/weights.rs

@@ -35,6 +35,7 @@ pub trait WeightInfo {
 	fn leave_intent(_c: u32) -> Weight;
 	fn note_author() -> Weight;
 	fn new_session(_c: u32, _r: u32) -> Weight;
+	fn remove_invulnerable_candidate(_c: u32) -> Weight;
 }
 
 /// Weights for pallet_collator_selection using the Substrate node and recommended hardware.
@@ -115,6 +116,25 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 			.saturating_add(T::DbWeight::get().reads(1))
 			.saturating_add(T::DbWeight::get().writes(1))
 	}
+	/// Storage: CollatorSelection Invulnerables (r:1 w:0)
+	/// Proof: CollatorSelection Invulnerables (max_values: Some(1), max_size: Some(641), added: 1136, mode: MaxEncodedLen)
+	/// Storage: CollatorSelection Candidates (r:1 w:1)
+	/// Proof: CollatorSelection Candidates (max_values: Some(1), max_size: Some(4802), added: 5297, mode: MaxEncodedLen)
+	/// Storage: System Account (r:1 w:1)
+	/// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen)
+	/// The range of component `c` is `[5, 100]`.
+	fn remove_invulnerable_candidate(c: u32) -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `364 + c * (49 ±0)`
+		//  Estimated: `6287`
+		// Minimum execution time: 35_081_000 picoseconds.
+		Weight::from_parts(38_468_787, 0)
+			.saturating_add(Weight::from_parts(0, 6287))
+			// Standard Error: 2_002
+			.saturating_add(Weight::from_parts(108_616, 0).saturating_mul(c.into()))
+			.saturating_add(T::DbWeight::get().reads(3))
+			.saturating_add(T::DbWeight::get().writes(2))
+	}
 }
 
 // For backwards compatibility and tests
@@ -194,4 +214,23 @@ impl WeightInfo for () {
 			.saturating_add(RocksDbWeight::get().reads(1))
 			.saturating_add(RocksDbWeight::get().writes(1))
 	}
+	/// Storage: CollatorSelection Invulnerables (r:1 w:0)
+	/// Proof: CollatorSelection Invulnerables (max_values: Some(1), max_size: Some(641), added: 1136, mode: MaxEncodedLen)
+	/// Storage: CollatorSelection Candidates (r:1 w:1)
+	/// Proof: CollatorSelection Candidates (max_values: Some(1), max_size: Some(4802), added: 5297, mode: MaxEncodedLen)
+	/// Storage: System Account (r:1 w:1)
+	/// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen)
+	/// The range of component `c` is `[5, 100]`.
+	fn remove_invulnerable_candidate(c: u32) -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `364 + c * (49 ±0)`
+		//  Estimated: `6287`
+		// Minimum execution time: 35_081_000 picoseconds.
+		Weight::from_parts(38_468_787, 0)
+			.saturating_add(Weight::from_parts(0, 6287))
+			// Standard Error: 2_002
+			.saturating_add(Weight::from_parts(108_616, 0).saturating_mul(c.into()))
+			.saturating_add(RocksDbWeight::get().reads(3))
+			.saturating_add(RocksDbWeight::get().writes(2))
+	}
 }

parachains/runtimes/assets/asset-hub-kusama/src/weights/pallet_collator_selection.rs

@@ -208,4 +208,23 @@ impl<T: frame_system::Config> pallet_collator_selection::WeightInfo for WeightIn
 			.saturating_add(Weight::from_parts(0, 2519).saturating_mul(c.into()))
 			.saturating_add(Weight::from_parts(0, 2602).saturating_mul(r.into()))
 	}
+	/// Storage: CollatorSelection Invulnerables (r:1 w:0)
+	/// Proof: CollatorSelection Invulnerables (max_values: Some(1), max_size: Some(641), added: 1136, mode: MaxEncodedLen)
+	/// Storage: CollatorSelection Candidates (r:1 w:1)
+	/// Proof: CollatorSelection Candidates (max_values: Some(1), max_size: Some(4802), added: 5297, mode: MaxEncodedLen)
+	/// Storage: System Account (r:1 w:1)
+	/// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen)
+	/// The range of component `c` is `[5, 100]`.
+	fn remove_invulnerable_candidate(c: u32, ) -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `364 + c * (49 ±0)`
+		//  Estimated: `6287`
+		// Minimum execution time: 35_081_000 picoseconds.
+		Weight::from_parts(38_468_787, 0)
+			.saturating_add(Weight::from_parts(0, 6287))
+			// Standard Error: 2_002
+			.saturating_add(Weight::from_parts(108_616, 0).saturating_mul(c.into()))
+			.saturating_add(T::DbWeight::get().reads(3))
+			.saturating_add(T::DbWeight::get().writes(2))
+	}
 }

parachains/runtimes/assets/asset-hub-polkadot/src/weights/pallet_collator_selection.rs

@@ -208,4 +208,23 @@ impl<T: frame_system::Config> pallet_collator_selection::WeightInfo for WeightIn
 			.saturating_add(Weight::from_parts(0, 2519).saturating_mul(c.into()))
 			.saturating_add(Weight::from_parts(0, 2602).saturating_mul(r.into()))
 	}
+	/// Storage: CollatorSelection Invulnerables (r:1 w:0)
+	/// Proof: CollatorSelection Invulnerables (max_values: Some(1), max_size: Some(641), added: 1136, mode: MaxEncodedLen)
+	/// Storage: CollatorSelection Candidates (r:1 w:1)
+	/// Proof: CollatorSelection Candidates (max_values: Some(1), max_size: Some(4802), added: 5297, mode: MaxEncodedLen)
+	/// Storage: System Account (r:1 w:1)
+	/// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen)
+	/// The range of component `c` is `[5, 100]`.
+	fn remove_invulnerable_candidate(c: u32, ) -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `364 + c * (49 ±0)`
+		//  Estimated: `6287`
+		// Minimum execution time: 35_081_000 picoseconds.
+		Weight::from_parts(38_468_787, 0)
+			.saturating_add(Weight::from_parts(0, 6287))
+			// Standard Error: 2_002
+			.saturating_add(Weight::from_parts(108_616, 0).saturating_mul(c.into()))
+			.saturating_add(T::DbWeight::get().reads(3))
+			.saturating_add(T::DbWeight::get().writes(2))
+	}
 }

parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_collator_selection.rs

@@ -208,4 +208,23 @@ impl<T: frame_system::Config> pallet_collator_selection::WeightInfo for WeightIn
 			.saturating_add(Weight::from_parts(0, 2519).saturating_mul(c.into()))
 			.saturating_add(Weight::from_parts(0, 2603).saturating_mul(r.into()))
 	}
+	/// Storage: CollatorSelection Invulnerables (r:1 w:0)
+	/// Proof: CollatorSelection Invulnerables (max_values: Some(1), max_size: Some(641), added: 1136, mode: MaxEncodedLen)
+	/// Storage: CollatorSelection Candidates (r:1 w:1)
+	/// Proof: CollatorSelection Candidates (max_values: Some(1), max_size: Some(4802), added: 5297, mode: MaxEncodedLen)
+	/// Storage: System Account (r:1 w:1)
+	/// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen)
+	/// The range of component `c` is `[5, 100]`.
+	fn remove_invulnerable_candidate(c: u32, ) -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `364 + c * (49 ±0)`
+		//  Estimated: `6287`
+		// Minimum execution time: 35_081_000 picoseconds.
+		Weight::from_parts(38_468_787, 0)
+			.saturating_add(Weight::from_parts(0, 6287))
+			// Standard Error: 2_002
+			.saturating_add(Weight::from_parts(108_616, 0).saturating_mul(c.into()))
+			.saturating_add(T::DbWeight::get().reads(3))
+			.saturating_add(T::DbWeight::get().writes(2))
+	}
 }

parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/weights/pallet_collator_selection.rs

@@ -208,4 +208,23 @@ impl<T: frame_system::Config> pallet_collator_selection::WeightInfo for WeightIn
 			.saturating_add(Weight::from_parts(0, 2519).saturating_mul(c.into()))
 			.saturating_add(Weight::from_parts(0, 2602).saturating_mul(r.into()))
 	}
+	/// Storage: CollatorSelection Invulnerables (r:1 w:0)
+	/// Proof: CollatorSelection Invulnerables (max_values: Some(1), max_size: Some(641), added: 1136, mode: MaxEncodedLen)
+	/// Storage: CollatorSelection Candidates (r:1 w:1)
+	/// Proof: CollatorSelection Candidates (max_values: Some(1), max_size: Some(4802), added: 5297, mode: MaxEncodedLen)
+	/// Storage: System Account (r:1 w:1)
+	/// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen)
+	/// The range of component `c` is `[5, 100]`.
+	fn remove_invulnerable_candidate(c: u32, ) -> Weight {
+		// Proof Size summary in bytes:
+		//  Measured:  `364 + c * (49 ±0)`
+		//  Estimated: `6287`
+		// Minimum execution time: 35_081_000 picoseconds.
+		Weight::from_parts(38_468_787, 0)
+			.saturating_add(Weight::from_parts(0, 6287))
+			// Standard Error: 2_002
+			.saturating_add(Weight::from_parts(108_616, 0).saturating_mul(c.into()))
+			.saturating_add(T::DbWeight::get().reads(3))
+			.saturating_add(T::DbWeight::get().writes(2))
+	}
 }