Add simple collator election mechanism (#1340)

Fixes https://github.com/paritytech/polkadot-sdk/issues/106 Port of cumulus PR https://github.com/paritytech/cumulus/pull/2960 This PR adds the ability to bid for collator slots even after the max number of collators have already registered. This eliminates the first come, first served mechanism that was in place before. Key changes: - added `update_bond` extrinsic to allow registered candidates to adjust their bonds in order to dynamically control their bids - added `take_candidate_slot` extrinsic to try to replace an already existing candidate by bidding more than them - candidates are now kept in a sorted list in the pallet storage, where the top `DesiredCandidates` out of `MaxCandidates` candidates in the list will be selected by the session pallet as collators - if the candidacy bond is increased through a `set_candidacy_bond` call, candidates which don't meet the new bond requirements are kicked # Checklist - [ ] My PR includes a detailed description as outlined in the "Description" section above - [ ] My PR follows the [labeling requirements](https://github.com/paritytech/polkadot-sdk/blob/master/docs/CONTRIBUTING.md#process) of this project (at minimum one label for `T` required) - [ ] I have made corresponding changes to the documentation (if applicable) - [ ] I have added tests that prove my fix is effective or that my feature works (if applicable) - [ ] If this PR alters any external APIs or interfaces used by Polkadot, the corresponding Polkadot PR is ready as well as the corresponding Cumulus PR (optional) --------- Signed-off-by: georgepisaltu <george.pisaltu@parity.io>
2026-05-31 16:51:02 +00:00 · 2023-11-14 18:22:30 +02:00
parent cd38ccff7f
commit 7d735fc8ae
13 changed files with 1731 additions and 163 deletions
@@ -25,14 +25,11 @@ use codec::Decode;
 use frame_benchmarking::{
 	account, impl_benchmark_test_suite, v2::*, whitelisted_caller, BenchmarkError,
 };
-use frame_support::{
-	dispatch::DispatchResult,
-	traits::{Currency, EnsureOrigin, Get, ReservableCurrency},
-};
+use frame_support::traits::{Currency, EnsureOrigin, Get, ReservableCurrency};
 use frame_system::{pallet_prelude::BlockNumberFor, EventRecord, RawOrigin};
 use pallet_authorship::EventHandler;
 use pallet_session::{self as session, SessionManager};
-use sp_std::prelude::*;
+use sp_std::{cmp, prelude::*};

 pub type BalanceOf<T> =
 	<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
@@ -94,7 +91,7 @@ fn register_candidates<T: Config>(count: u32) {
 	assert!(<CandidacyBond<T>>::get() > 0u32.into(), "Bond cannot be zero!");

 	for who in candidates {
-		T::Currency::make_free_balance_be(&who, <CandidacyBond<T>>::get() * 2u32.into());
+		T::Currency::make_free_balance_be(&who, <CandidacyBond<T>>::get() * 3u32.into());
 		<CollatorSelection<T>>::register_as_candidate(RawOrigin::Signed(who).into()).unwrap();
 	}
 }
@@ -107,8 +104,11 @@ fn min_candidates<T: Config>() -> u32 {

 fn min_invulnerables<T: Config>() -> u32 {
 	let min_collators = T::MinEligibleCollators::get();
-	let candidates_length = <Candidates<T>>::get().len();
-	min_collators.saturating_sub(candidates_length.try_into().unwrap())
+	let candidates_length = <CandidateList<T>>::decode_len()
+		.unwrap_or_default()
+		.try_into()
+		.unwrap_or_default();
+	min_collators.saturating_sub(candidates_length)
 }

 #[benchmarks(where T: pallet_authorship::Config + session::Config)]
@@ -160,22 +160,19 @@ mod benchmarks {
 				.unwrap();
 		}
 		// ... and register them.
-		for (who, _) in candidates {
+		for (who, _) in candidates.iter() {
 			let deposit = <CandidacyBond<T>>::get();
-			T::Currency::make_free_balance_be(&who, deposit * 1000_u32.into());
-			let incoming = CandidateInfo { who: who.clone(), deposit };
-			<Candidates<T>>::try_mutate(|candidates| -> DispatchResult {
-				if !candidates.iter().any(|candidate| candidate.who == who) {
-					T::Currency::reserve(&who, deposit)?;
-					candidates.try_push(incoming).expect("we've respected the bounded vec limit");
-					<LastAuthoredBlock<T>>::insert(
-						who.clone(),
-						frame_system::Pallet::<T>::block_number() + T::KickThreshold::get(),
-					);
-				}
-				Ok(())
+			T::Currency::make_free_balance_be(who, deposit * 1000_u32.into());
+			<CandidateList<T>>::try_mutate(|list| {
+				list.try_push(CandidateInfo { who: who.clone(), deposit }).unwrap();
+				Ok::<(), BenchmarkError>(())
 			})
-			.expect("only returns ok");
+			.unwrap();
+			T::Currency::reserve(who, deposit)?;
+			<LastAuthoredBlock<T>>::insert(
+				who.clone(),
+				frame_system::Pallet::<T>::block_number() + T::KickThreshold::get(),
+			);
 		}

 		// now we need to fill up invulnerables
@@ -226,10 +223,27 @@ mod benchmarks {
 	}

 	#[benchmark]
-	fn set_candidacy_bond() -> Result<(), BenchmarkError> {
-		let bond_amount: BalanceOf<T> = T::Currency::minimum_balance() * 10u32.into();
+	fn set_candidacy_bond(
+		c: Linear<0, { T::MaxCandidates::get() }>,
+		k: Linear<0, { T::MaxCandidates::get() }>,
+	) -> Result<(), BenchmarkError> {
+		let initial_bond_amount: BalanceOf<T> = T::Currency::minimum_balance() * 2u32.into();
+		<CandidacyBond<T>>::put(initial_bond_amount);
+		register_validators::<T>(c);
+		register_candidates::<T>(c);
+		let kicked = cmp::min(k, c);
 		let origin =
 			T::UpdateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
+		let bond_amount = if k > 0 {
+			<CandidateList<T>>::mutate(|candidates| {
+				for info in candidates.iter_mut().skip(kicked as usize) {
+					info.deposit = T::Currency::minimum_balance() * 3u32.into();
+				}
+			});
+			T::Currency::minimum_balance() * 3u32.into()
+		} else {
+			T::Currency::minimum_balance()
+		};

 		#[extrinsic_call]
 		_(origin as T::RuntimeOrigin, bond_amount);
@@ -238,6 +252,35 @@ mod benchmarks {
 		Ok(())
 	}

+	#[benchmark]
+	fn update_bond(
+		c: Linear<{ min_candidates::<T>() + 1 }, { T::MaxCandidates::get() }>,
+	) -> Result<(), BenchmarkError> {
+		<CandidacyBond<T>>::put(T::Currency::minimum_balance());
+		<DesiredCandidates<T>>::put(c);
+
+		register_validators::<T>(c);
+		register_candidates::<T>(c);
+
+		let caller = <CandidateList<T>>::get()[0].who.clone();
+		v2::whitelist!(caller);
+
+		let bond_amount: BalanceOf<T> =
+			T::Currency::minimum_balance() + T::Currency::minimum_balance();
+
+		#[extrinsic_call]
+		_(RawOrigin::Signed(caller.clone()), bond_amount);
+
+		assert_last_event::<T>(
+			Event::CandidateBondUpdated { account_id: caller, deposit: bond_amount }.into(),
+		);
+		assert!(
+			<CandidateList<T>>::get().iter().last().unwrap().deposit ==
+				T::Currency::minimum_balance() * 2u32.into()
+		);
+		Ok(())
+	}
+
 	// worse case is when we have all the max-candidate slots filled except one, and we fill that
 	// one.
 	#[benchmark]
@@ -267,6 +310,36 @@ mod benchmarks {
 		);
 	}

+	#[benchmark]
+	fn take_candidate_slot(c: Linear<{ min_candidates::<T>() + 1 }, { T::MaxCandidates::get() }>) {
+		<CandidacyBond<T>>::put(T::Currency::minimum_balance());
+		<DesiredCandidates<T>>::put(1);
+
+		register_validators::<T>(c);
+		register_candidates::<T>(c);
+
+		let caller: T::AccountId = whitelisted_caller();
+		let bond: BalanceOf<T> = T::Currency::minimum_balance() * 10u32.into();
+		T::Currency::make_free_balance_be(&caller, bond);
+
+		<session::Pallet<T>>::set_keys(
+			RawOrigin::Signed(caller.clone()).into(),
+			keys::<T>(c + 1),
+			Vec::new(),
+		)
+		.unwrap();
+
+		let target = <CandidateList<T>>::get().iter().last().unwrap().who.clone();
+
+		#[extrinsic_call]
+		_(RawOrigin::Signed(caller.clone()), bond / 2u32.into(), target.clone());
+
+		assert_last_event::<T>(
+			Event::CandidateReplaced { old: target, new: caller, deposit: bond / 2u32.into() }
+				.into(),
+		);
+	}
+
 	// worse case is the last candidate leaving.
 	#[benchmark]
 	fn leave_intent(c: Linear<{ min_candidates::<T>() + 1 }, { T::MaxCandidates::get() }>) {
@@ -276,7 +349,7 @@ mod benchmarks {
 		register_validators::<T>(c);
 		register_candidates::<T>(c);

-		let leaving = <Candidates<T>>::get().last().unwrap().who.clone();
+		let leaving = <CandidateList<T>>::get().iter().last().unwrap().who.clone();
 		v2::whitelist!(leaving);

 		#[extrinsic_call]
@@ -323,31 +396,37 @@ mod benchmarks {

 		let new_block: BlockNumberFor<T> = 1800u32.into();
 		let zero_block: BlockNumberFor<T> = 0u32.into();
-		let candidates = <Candidates<T>>::get();
+		let candidates: Vec<T::AccountId> = <CandidateList<T>>::get()
+			.iter()
+			.map(|candidate_info| candidate_info.who.clone())
+			.collect();

 		let non_removals = c.saturating_sub(r);

 		for i in 0..c {
-			<LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), zero_block);
+			<LastAuthoredBlock<T>>::insert(candidates[i as usize].clone(), zero_block);
 		}

 		if non_removals > 0 {
 			for i in 0..non_removals {
-				<LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), new_block);
+				<LastAuthoredBlock<T>>::insert(candidates[i as usize].clone(), new_block);
 			}
 		} else {
 			for i in 0..c {
-				<LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), new_block);
+				<LastAuthoredBlock<T>>::insert(candidates[i as usize].clone(), new_block);
 			}
 		}

 		let min_candidates = min_candidates::<T>();
-		let pre_length = <Candidates<T>>::get().len();
+		let pre_length = <CandidateList<T>>::decode_len().unwrap_or_default();

 		frame_system::Pallet::<T>::set_block_number(new_block);

-		assert!(<Candidates<T>>::get().len() == c as usize);
-
+		let current_length: u32 = <CandidateList<T>>::decode_len()
+			.unwrap_or_default()
+			.try_into()
+			.unwrap_or_default();
+		assert!(c == current_length);
 		#[block]
 		{
 			<CollatorSelection<T> as SessionManager<_>>::new_session(0);
@@ -357,16 +436,20 @@ mod benchmarks {
 			// candidates > removals and remaining candidates > min candidates
 			// => remaining candidates should be shorter than before removal, i.e. some were
 			//    actually removed.
-			assert!(<Candidates<T>>::get().len() < pre_length);
+			assert!(<CandidateList<T>>::decode_len().unwrap_or_default() < pre_length);
 		} else if c > r && non_removals < min_candidates {
 			// candidates > removals and remaining candidates would be less than min candidates
 			// => remaining candidates should equal min candidates, i.e. some were removed up to
 			//    the minimum, but then any more were "forced" to stay in candidates.
-			assert!(<Candidates<T>>::get().len() == min_candidates as usize);
+			let current_length: u32 = <CandidateList<T>>::decode_len()
+				.unwrap_or_default()
+				.try_into()
+				.unwrap_or_default();
+			assert!(min_candidates == current_length);
 		} else {
 			// removals >= candidates, non removals must == 0
 			// can't remove more than exist
-			assert!(<Candidates<T>>::get().len() == pre_length);
+			assert!(<CandidateList<T>>::decode_len().unwrap_or_default() == pre_length);
 		}
 	}