diff --git a/pallets/collator-selection/src/benchmarking.rs b/pallets/collator-selection/src/benchmarking.rs index c757090..1c2c52f 100644 --- a/pallets/collator-selection/src/benchmarking.rs +++ b/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(); >::put(T::Currency::minimum_balance()); - >::put(c); - register_candidates::(c); - T::Currency::make_free_balance_be( &>::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::::set_block_number(new_block); assert!(T::Currency::free_balance(&author) == 0u32.into()); }: { as EventHandler<_, _>>::note_author(author.clone()) } verify { assert!(T::Currency::free_balance(&author) > 0u32.into()); + assert_eq!(frame_system::Pallet::::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 = >::get(); + let candidates = >::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; + >::insert(candidates[i as usize].who.clone(), new_block); } >::put(candidates.clone()); let pre_length = >::get().len(); - frame_system::Pallet::::set_block_number(new_block.clone()); + frame_system::Pallet::::set_block_number(new_block); assert!(>::get().len() == c as usize); diff --git a/pallets/collator-selection/src/lib.rs b/pallets/collator-selection/src/lib.rs index c7c9f27..8285c67 100644 --- a/pallets/collator-selection/src/lib.rs +++ b/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 { + pub struct CandidateInfo { /// 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 = StorageValue< _, - Vec, T::BlockNumber>>, + Vec>>, ValueQuery, >; + /// Last block authored by collator. + #[pallet::storage] + #[pallet::getter(fn last_authored_block)] + pub type LastAuthoredBlock = 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::::AlreadyInvulnerable); let deposit = Self::candidacy_bond(); - let incoming = CandidateInfo { who: who.clone(), deposit, last_block: frame_system::Pallet::::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 = >::try_mutate(|candidates| -> Result { @@ -307,6 +311,7 @@ pub mod pallet { } else { T::Currency::reserve(&who, deposit)?; candidates.push(incoming); + >::insert(who.clone(), frame_system::Pallet::::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::::NotCandidate)?; T::Currency::unreserve(&who, candidates[index].deposit); candidates.remove(index); + >::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, T::BlockNumber>>) -> Vec { + pub fn kick_stale_candidates(candidates: Vec>>) -> Vec { let now = frame_system::Pallet::::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 = >::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 = >::mutate(|candidates| -> usize { - if let Some(found) = candidates.iter_mut().find(|candidate| candidate.who == author) { - found.last_block = frame_system::Pallet::::block_number(); - } - candidates.len() - }); + >::insert(author, frame_system::Pallet::::block_number()); + frame_system::Pallet::::register_extra_weight_unchecked( - T::WeightInfo::note_author(candidates_len as u32), + T::WeightInfo::note_author(), DispatchClass::Mandatory, ); } diff --git a/pallets/collator-selection/src/tests.rs b/pallets/collator-selection/src/tests.rs index a1f0071..47157fd 100644 --- a/pallets/collator-selection/src/tests.rs +++ b/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); }); diff --git a/pallets/collator-selection/src/weights.rs b/pallets/collator-selection/src/weights.rs index 5c50995..b35cb08 100644 --- a/pallets/collator-selection/src/weights.rs +++ b/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 WeightInfo for SubstrateWeight { .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 { diff --git a/runtime/statemine/src/weights/pallet_collator_selection.rs b/runtime/statemine/src/weights/pallet_collator_selection.rs index dfca2ab..ab3b0ab 100644 --- a/runtime/statemine/src/weights/pallet_collator_selection.rs +++ b/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(PhantomData); impl pallet_collator_selection::WeightInfo for WeightInfo { 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)) } } diff --git a/runtime/statemint/src/weights/pallet_collator_selection.rs b/runtime/statemint/src/weights/pallet_collator_selection.rs index fad9de8..4b04246 100644 --- a/runtime/statemint/src/weights/pallet_collator_selection.rs +++ b/runtime/statemint/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("statemint-dev"), DB CACHE: 128 // Executed Command: @@ -37,47 +37,46 @@ use sp_std::marker::PhantomData; pub struct WeightInfo(PhantomData); impl pallet_collator_selection::WeightInfo for WeightInfo { fn set_invulnerables(b: u32, ) -> Weight { - (36_147_000 as Weight) - // Standard Error: 22_000 - .saturating_add((50_000 as Weight).saturating_mul(b as Weight)) + (30_139_000 as Weight) + // Standard Error: 26_000 + .saturating_add((195_000 as Weight).saturating_mul(b as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn set_desired_candidates() -> Weight { - (25_000_000 as Weight) + (29_000_000 as Weight) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn set_candidacy_bond() -> Weight { - (26_000_000 as Weight) + (28_000_000 as Weight) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn register_as_candidate(c: u32, ) -> Weight { - (90_380_000 as Weight) - // Standard Error: 8_000 - .saturating_add((314_000 as Weight).saturating_mul(c as Weight)) + (104_312_000 as Weight) + // Standard Error: 10_000 + .saturating_add((309_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 { - (65_617_000 as Weight) - // Standard Error: 4_000 - .saturating_add((325_000 as Weight).saturating_mul(c as Weight)) + (75_851_000 as Weight) + // Standard Error: 3_000 + .saturating_add((216_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 { - (138_742_000 as Weight) - // Standard Error: 4_000 - .saturating_add((255_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 { - (59_461_000 as Weight) - // Standard Error: 4_000 - .saturating_add((2_000 as Weight).saturating_mul(r as Weight)) - // Standard Error: 4_000 - .saturating_add((291_000 as Weight).saturating_mul(c as Weight)) + (0 as Weight) + // Standard Error: 120_000 + .saturating_add((819_000 as Weight).saturating_mul(r as Weight)) + // Standard Error: 120_000 + .saturating_add((10_090_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)) } } diff --git a/runtime/westmint/src/weights/pallet_collator_selection.rs b/runtime/westmint/src/weights/pallet_collator_selection.rs index fad9de8..4b04246 100644 --- a/runtime/westmint/src/weights/pallet_collator_selection.rs +++ b/runtime/westmint/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("statemint-dev"), DB CACHE: 128 // Executed Command: @@ -37,47 +37,46 @@ use sp_std::marker::PhantomData; pub struct WeightInfo(PhantomData); impl pallet_collator_selection::WeightInfo for WeightInfo { fn set_invulnerables(b: u32, ) -> Weight { - (36_147_000 as Weight) - // Standard Error: 22_000 - .saturating_add((50_000 as Weight).saturating_mul(b as Weight)) + (30_139_000 as Weight) + // Standard Error: 26_000 + .saturating_add((195_000 as Weight).saturating_mul(b as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn set_desired_candidates() -> Weight { - (25_000_000 as Weight) + (29_000_000 as Weight) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn set_candidacy_bond() -> Weight { - (26_000_000 as Weight) + (28_000_000 as Weight) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn register_as_candidate(c: u32, ) -> Weight { - (90_380_000 as Weight) - // Standard Error: 8_000 - .saturating_add((314_000 as Weight).saturating_mul(c as Weight)) + (104_312_000 as Weight) + // Standard Error: 10_000 + .saturating_add((309_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 { - (65_617_000 as Weight) - // Standard Error: 4_000 - .saturating_add((325_000 as Weight).saturating_mul(c as Weight)) + (75_851_000 as Weight) + // Standard Error: 3_000 + .saturating_add((216_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 { - (138_742_000 as Weight) - // Standard Error: 4_000 - .saturating_add((255_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 { - (59_461_000 as Weight) - // Standard Error: 4_000 - .saturating_add((2_000 as Weight).saturating_mul(r as Weight)) - // Standard Error: 4_000 - .saturating_add((291_000 as Weight).saturating_mul(c as Weight)) + (0 as Weight) + // Standard Error: 120_000 + .saturating_add((819_000 as Weight).saturating_mul(r as Weight)) + // Standard Error: 120_000 + .saturating_add((10_090_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)) } }