Add simple collator election mechanism (#1340)

Fixes #106

Port of cumulus PR paritytech/cumulus#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>

Author: georgepisaltu

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

@@ -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);
 		}
 	}