kick patch

pallets/collator-selection/src/benchmarking.rs

@@ -134,21 +134,21 @@ benchmarks! {
 
 	// worse case is paying a non-existing candidate account.
 	note_author {
-		let c in 1 .. T::MaxCandidates::get();
 		<CandidacyBond<T>>::put(T::Currency::minimum_balance());
-		<DesiredCandidates<T>>::put(c);
-		register_candidates::<T>(c);
-
 		T::Currency::make_free_balance_be(
 			&<CollatorSelection<T>>::account_id(),
-			T::Currency::minimum_balance() * 2u32.into(),
+			T::Currency::minimum_balance() * 4u32.into(),
 		);
 		let author = account("author", 0, SEED);
+		let new_block: T::BlockNumber = 10u32.into();
+
+		frame_system::Pallet::<T>::set_block_number(new_block);
 		assert!(T::Currency::free_balance(&author) == 0u32.into());
 	}: {
 		<CollatorSelection<T> as EventHandler<_, _>>::note_author(author.clone())
 	} verify {
 		assert!(T::Currency::free_balance(&author) > 0u32.into());
+		assert_eq!(frame_system::Pallet::<T>::block_number(), new_block);
 	}
 
 	// worse case is on new session.
@@ -164,16 +164,16 @@ benchmarks! {
 
 		let new_block: T::BlockNumber = 20u32.into();
 
-		let mut candidates = <Candidates<T>>::get();
+		let candidates = <Candidates<T>>::get();
 		let non_removals = if c > r { c - r } else { 0 };
 
 		for i in 0..non_removals {
-			candidates[i as usize].last_block = new_block;
+			<LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), new_block);
 		}
 		<Candidates<T>>::put(candidates.clone());
 
 		let pre_length = <Candidates<T>>::get().len();
-		frame_system::Pallet::<T>::set_block_number(new_block.clone());
+		frame_system::Pallet::<T>::set_block_number(new_block);
 
 		assert!(<Candidates<T>>::get().len() == c as usize);
 

pallets/collator-selection/src/lib.rs

@@ -85,7 +85,7 @@ pub mod pallet {
 	use frame_support::{
 		sp_runtime::{
 			RuntimeDebug,
-			traits::{AccountIdConversion, CheckedSub, Zero},
+			traits::{AccountIdConversion, CheckedSub, Zero, Saturating},
 		},
 		weights::DispatchClass,
 	};
@@ -141,13 +141,11 @@ pub mod pallet {
 
 	/// Basic information about a collation candidate.
 	#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)]
-	pub struct CandidateInfo<AccountId, Balance, BlockNumber> {
+	pub struct CandidateInfo<AccountId, Balance> {
 		/// Account identifier.
 		pub who: AccountId,
 		/// Reserved deposit.
 		pub deposit: Balance,
-		/// Last block at which they authored a block.
-		pub last_block: BlockNumber,
 	}
 
 	#[pallet::pallet]
@@ -164,10 +162,15 @@ pub mod pallet {
 	#[pallet::getter(fn candidates)]
 	pub type Candidates<T: Config> = StorageValue<
 		_,
-		Vec<CandidateInfo<T::AccountId, BalanceOf<T>, T::BlockNumber>>,
+		Vec<CandidateInfo<T::AccountId, BalanceOf<T>>>,
 		ValueQuery,
 	>;
 
+	/// Last block authored by collator.
+	#[pallet::storage]
+	#[pallet::getter(fn last_authored_block)]
+	pub type LastAuthoredBlock<T: Config> = StorageMap<_, Twox64Concat, T::AccountId, T::BlockNumber, ValueQuery>;
+
 	/// Desired number of candidates.
 	///
 	/// This should ideally always be less than [`Config::MaxCandidates`] for weights to be correct.
@@ -298,7 +301,8 @@ pub mod pallet {
 			ensure!(!Self::invulnerables().contains(&who), Error::<T>::AlreadyInvulnerable);
 
 			let deposit = Self::candidacy_bond();
-			let incoming = CandidateInfo { who: who.clone(), deposit, last_block: frame_system::Pallet::<T>::block_number() };
+			// First authored block is current block plus kick threshold to handle session delay
+			let incoming = CandidateInfo { who: who.clone(), deposit };
 
 			let current_count =
 				<Candidates<T>>::try_mutate(|candidates| -> Result<usize, DispatchError> {
@@ -307,6 +311,7 @@ pub mod pallet {
 					} else {
 						T::Currency::reserve(&who, deposit)?;
 						candidates.push(incoming);
+						<LastAuthoredBlock<T>>::insert(who.clone(), frame_system::Pallet::<T>::block_number() + T::KickThreshold::get());
 						Ok(candidates.len())
 					}
 				})?;
@@ -336,6 +341,7 @@ pub mod pallet {
 				let index = candidates.iter().position(|candidate| candidate.who == *who).ok_or(Error::<T>::NotCandidate)?;
 				T::Currency::unreserve(&who, candidates[index].deposit);
 				candidates.remove(index);
+				<LastAuthoredBlock<T>>::remove(who.clone());
 				Ok(candidates.len())
 			});
 			Self::deposit_event(Event::CandidateRemoved(who.clone()));
@@ -353,11 +359,12 @@ pub mod pallet {
 			collators
 		}
 		/// Kicks out and candidates that did not produce a block in the kick threshold.
-		pub fn kick_stale_candidates(candidates: Vec<CandidateInfo<T::AccountId, BalanceOf<T>, T::BlockNumber>>) -> Vec<T::AccountId> {
+		pub fn kick_stale_candidates(candidates: Vec<CandidateInfo<T::AccountId, BalanceOf<T>>>) -> Vec<T::AccountId> {
 			let now = frame_system::Pallet::<T>::block_number();
 			let kick_threshold = T::KickThreshold::get();
 			let new_candidates = candidates.into_iter().filter_map(|c| {
-				let since_last = now - c.last_block;
+				let last_block = <LastAuthoredBlock<T>>::get(c.who.clone());
+				let since_last = now.saturating_sub(last_block);
 				if since_last < kick_threshold {
 					Some(c.who)
 				} else {
@@ -385,14 +392,10 @@ pub mod pallet {
 			// `reward` is half of pot account minus ED, this should never fail.
 			let _success = T::Currency::transfer(&pot, &author, reward, KeepAlive);
 			debug_assert!(_success.is_ok());
-			let candidates_len = <Candidates<T>>::mutate(|candidates| -> usize {
-				if let Some(found) = candidates.iter_mut().find(|candidate| candidate.who == author) {
-					found.last_block = frame_system::Pallet::<T>::block_number();
-				}
-				candidates.len()
-			});
+			<LastAuthoredBlock<T>>::insert(author, frame_system::Pallet::<T>::block_number());
+
 			frame_system::Pallet::<T>::register_extra_weight_unchecked(
-				T::WeightInfo::note_author(candidates_len as u32),
+				T::WeightInfo::note_author(),
 				DispatchClass::Mandatory,
 			);
 		}

pallets/collator-selection/src/tests.rs

@@ -124,8 +124,9 @@ fn cannot_register_dupe_candidate() {
 	new_test_ext().execute_with(|| {
 		// can add 3 as candidate
 		assert_ok!(CollatorSelection::register_as_candidate(Origin::signed(3)));
-		let addition = CandidateInfo { who: 3, deposit: 10, last_block: 0 };
+		let addition = CandidateInfo { who: 3, deposit: 10 };
 		assert_eq!(CollatorSelection::candidates(), vec![addition]);
+		assert_eq!(CollatorSelection::last_authored_block(3), 10);
 		assert_eq!(Balances::free_balance(3), 90);
 
 		// but no more
@@ -192,6 +193,7 @@ fn leave_intent() {
 		// bond is returned
 		assert_ok!(CollatorSelection::leave_intent(Origin::signed(3)));
 		assert_eq!(Balances::free_balance(3), 100);
+		assert_eq!(CollatorSelection::last_authored_block(3), 0);
 	});
 }
 
@@ -211,10 +213,10 @@ fn authorship_event_handler() {
 		let collator = CandidateInfo {
 			who: 4,
 			deposit: 10,
-			last_block: 0
 		};
 
 		assert_eq!(CollatorSelection::candidates(), vec![collator]);
+		assert_eq!(CollatorSelection::last_authored_block(4), 0);
 
 		// half of the pot goes to the collator who's the author (4 in tests).
 		assert_eq!(Balances::free_balance(4), 140);
@@ -240,11 +242,10 @@ fn fees_edgecases() {
 		let collator = CandidateInfo {
 			who: 4,
 			deposit: 10,
-			last_block: 0
 		};
 
 		assert_eq!(CollatorSelection::candidates(), vec![collator]);
-
+		assert_eq!(CollatorSelection::last_authored_block(4), 0);
 		// Nothing received
 		assert_eq!(Balances::free_balance(4), 90);
 		// all fee stays
@@ -295,18 +296,23 @@ fn kick_mechanism() {
 		// add a new collator
 		assert_ok!(CollatorSelection::register_as_candidate(Origin::signed(3)));
 		assert_ok!(CollatorSelection::register_as_candidate(Origin::signed(4)));
+		initialize_to_block(10);
 		assert_eq!(CollatorSelection::candidates().len(), 2);
-		initialize_to_block(21);
+		initialize_to_block(20);
 		assert_eq!(SessionChangeBlock::get(), 20);
 		// 4 authored this block, gets to stay 3 was kicked
 		assert_eq!(CollatorSelection::candidates().len(), 1);
-		assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 4]);
+		// 3 will be kicked after 1 session delay
+		assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 3, 4]);
 		let collator = CandidateInfo {
 			who: 4,
 			deposit: 10,
-			last_block: 21
 		};
 		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, 4]);
 		// kicked collator gets funds back
 		assert_eq!(Balances::free_balance(3), 100);
 	});

pallets/collator-selection/src/weights.rs

@@ -28,7 +28,7 @@ pub trait WeightInfo {
 	fn set_candidacy_bond() -> Weight;
 	fn register_as_candidate(_c: u32) -> Weight;
 	fn leave_intent(_c: u32) -> Weight;
-	fn note_author(_c: u32) -> Weight;
+	fn note_author() -> Weight;
 	fn new_session(_c: u32, _r: u32) -> Weight;
 }
 
@@ -63,11 +63,9 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
-	fn note_author(c: u32, ) -> Weight {
-		(108_730_000 as Weight)
-			// Standard Error: 3_000
-			.saturating_add((286_000 as Weight).saturating_mul(c as Weight))
-			.saturating_add(T::DbWeight::get().reads(4 as Weight))
+	fn note_author() -> Weight {
+		(106_000_000 as Weight)
+			.saturating_add(T::DbWeight::get().reads(3 as Weight))
 			.saturating_add(T::DbWeight::get().writes(4 as Weight))
 	}
 	fn new_session(r: u32, c: u32, ) -> Weight {
@@ -112,11 +110,9 @@ impl WeightInfo for () {
 			.saturating_add(RocksDbWeight::get().reads(1 as Weight))
 			.saturating_add(RocksDbWeight::get().writes(1 as Weight))
 	}
-	fn note_author(c: u32, ) -> Weight {
-		(108_730_000 as Weight)
-			// Standard Error: 3_000
-			.saturating_add((286_000 as Weight).saturating_mul(c as Weight))
-			.saturating_add(RocksDbWeight::get().reads(4 as Weight))
+	fn note_author() -> Weight {
+		(106_000_000 as Weight)
+			.saturating_add(RocksDbWeight::get().reads(3 as Weight))
 			.saturating_add(RocksDbWeight::get().writes(4 as Weight))
 	}
 	fn new_session(r: u32, c: u32, ) -> Weight {

runtime/statemine/src/weights/pallet_collator_selection.rs

@@ -2,7 +2,7 @@
 //! Autogenerated weights for pallet_collator_selection
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0
-//! DATE: 2021-05-04, STEPS: `[20, ]`, REPEAT: 10, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! DATE: 2021-05-28, STEPS: `[20, ]`, REPEAT: 10, LOW RANGE: `[]`, HIGH RANGE: `[]`
 //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("statemine-dev"), DB CACHE: 128
 
 // Executed Command:
@@ -37,47 +37,46 @@ use sp_std::marker::PhantomData;
 pub struct WeightInfo<T>(PhantomData<T>);
 impl<T: frame_system::Config> pallet_collator_selection::WeightInfo for WeightInfo<T> {
 	fn set_invulnerables(b: u32, ) -> Weight {
-		(34_775_000 as Weight)
-			// Standard Error: 2_000
-			.saturating_add((159_000 as Weight).saturating_mul(b as Weight))
+		(28_206_000 as Weight)
+			// Standard Error: 1_000
+			.saturating_add((127_000 as Weight).saturating_mul(b as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	fn set_desired_candidates() -> Weight {
-		(29_000_000 as Weight)
+		(25_000_000 as Weight)
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	fn set_candidacy_bond() -> Weight {
-		(30_000_000 as Weight)
+		(26_000_000 as Weight)
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 	fn register_as_candidate(c: u32, ) -> Weight {
-		(116_537_000 as Weight)
-			// Standard Error: 4_000
-			.saturating_add((330_000 as Weight).saturating_mul(c as Weight))
+		(97_732_000 as Weight)
+			// Standard Error: 2_000
+			.saturating_add((272_000 as Weight).saturating_mul(c as Weight))
 			.saturating_add(T::DbWeight::get().reads(4 as Weight))
-			.saturating_add(T::DbWeight::get().writes(1 as Weight))
+			.saturating_add(T::DbWeight::get().writes(2 as Weight))
 	}
 	fn leave_intent(c: u32, ) -> Weight {
-		(66_542_000 as Weight)
-			// Standard Error: 0
-			.saturating_add((335_000 as Weight).saturating_mul(c as Weight))
+		(71_267_000 as Weight)
+			// Standard Error: 2_000
+			.saturating_add((229_000 as Weight).saturating_mul(c as Weight))
 			.saturating_add(T::DbWeight::get().reads(1 as Weight))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
-	fn note_author(c: u32, ) -> Weight {
-		(118_918_000 as Weight)
-			// Standard Error: 1_000
-			.saturating_add((334_000 as Weight).saturating_mul(c as Weight))
-			.saturating_add(T::DbWeight::get().reads(4 as Weight))
+	fn note_author() -> Weight {
+		(106_000_000 as Weight)
+			.saturating_add(T::DbWeight::get().reads(3 as Weight))
 			.saturating_add(T::DbWeight::get().writes(4 as Weight))
 	}
 	fn new_session(r: u32, c: u32, ) -> Weight {
-		(45_567_000 as Weight)
-			// Standard Error: 3_000
-			.saturating_add((16_000 as Weight).saturating_mul(r as Weight))
-			// Standard Error: 3_000
-			.saturating_add((359_000 as Weight).saturating_mul(c as Weight))
+		(0 as Weight)
+			// Standard Error: 114_000
+			.saturating_add((890_000 as Weight).saturating_mul(r as Weight))
+			// Standard Error: 114_000
+			.saturating_add((10_928_000 as Weight).saturating_mul(c as Weight))
 			.saturating_add(T::DbWeight::get().reads(3 as Weight))
+			.saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(c as Weight)))
 			.saturating_add(T::DbWeight::get().writes(1 as Weight))
 	}
 }