From b4f4c005915b8f7a2d7355428531360e203367a1 Mon Sep 17 00:00:00 2001 From: Manuel Mauro Date: Fri, 10 Jan 2025 18:31:31 +0200 Subject: [PATCH] perf: :zap: use a WasInactive storage to keep track of inactive collators --- pallets/parachain-staking/src/lib.rs | 49 ++++++++++++++------------ pallets/parachain-staking/src/tests.rs | 43 ++++++++++++++-------- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/pallets/parachain-staking/src/lib.rs b/pallets/parachain-staking/src/lib.rs index a485dcbae4..721ca6b048 100644 --- a/pallets/parachain-staking/src/lib.rs +++ b/pallets/parachain-staking/src/lib.rs @@ -489,6 +489,8 @@ pub mod pallet { selected_collators_number: collator_count, total_balance: total_staked, }); + // record inactive collators + Self::mark_collators_as_inactive(round.current); // account for Round write weight = weight.saturating_add(T::DbWeight::get().reads_writes(0, 1)); } else { @@ -503,7 +505,7 @@ pub mod pallet { } fn on_finalize(_n: BlockNumberFor) { Self::award_points_to_block_author(); - Self::cleanup_stake_info(); + Self::cleanup_inactive_collator_info(); } } @@ -645,10 +647,10 @@ pub mod pallet { >; #[pallet::storage] - #[pallet::getter(fn had_stake)] - /// Records collator's delegation stake presence at round start. + #[pallet::getter(fn was_inactive)] + /// Records collators' inactivity. /// Data persists for MaxOfflineRounds + 1 rounds before being pruned. - pub type HadStake = + pub type WasInactive = StorageDoubleMap<_, Twox64Concat, RoundIndex, Twox64Concat, T::AccountId, (), OptionQuery>; #[pallet::storage] @@ -1425,17 +1427,9 @@ pub mod pallet { // the collator should be notified as inactive let mut inactive_counter: RoundIndex = 0u32; - // Iter rounds to check - // - // - The collator has AtStake associated and their AwardedPts are zero - // - // If the previous condition is met in all rounds of rounds_to_check, - // the collator is notified as inactive + // Iter rounds and check whether the collator has been inactive for r in rounds_to_check { - let stake = >::get(r, &collator); - let pts = >::get(r, &collator); - - if stake.is_some() && pts.is_zero() { + if >::get(r, &collator).is_some() { inactive_counter = inactive_counter.saturating_add(1); } } @@ -2179,7 +2173,6 @@ pub mod pallet { total: total_counted, }; >::insert(now, account, snapshot); - >::insert(now, account, ()); Self::deposit_event(Event::CollatorChosen { round: now, collator_account: account.clone(), @@ -2342,11 +2335,9 @@ pub mod pallet { }); }; } - } - /// Add reward points to block authors: - /// * 20 points to the block producer for producing a block in the chain - impl Pallet { + /// Add reward points to block authors: + /// * 20 points to the block producer for producing a block in the chain fn award_points_to_block_author() { let author = T::BlockAuthor::get(); let now = >::get().current; @@ -2355,9 +2346,23 @@ pub mod pallet { >::mutate(now, |x| *x = x.saturating_add(20)); } + /// Marks collators as inactive for the previous round if they received zero awarded points. + fn mark_collators_as_inactive(cur: RoundIndex) { + if cur == 0 { + return; + } + + let prev = cur - 1; + for (account, _) in >::iter_prefix(prev) { + if >::get(prev, &account).is_zero() { + >::insert(prev, account, ()); + } + } + } + /// Cleans up historical staking information that is older than MaxOfflineRounds - /// by removing entries from the HadStake storage map. - fn cleanup_stake_info() { + /// by removing entries from the WasIactive storage map. + fn cleanup_inactive_collator_info() { let now = >::get().current; let minimum_rounds_required = T::MaxOfflineRounds::get() + 1; @@ -2365,7 +2370,7 @@ pub mod pallet { return; } - let _ = >::iter_prefix(now - minimum_rounds_required) + let _ = >::iter_prefix(now - minimum_rounds_required) .drain() .next(); } diff --git a/pallets/parachain-staking/src/tests.rs b/pallets/parachain-staking/src/tests.rs index 6384ce3099..78c4c73f4d 100644 --- a/pallets/parachain-staking/src/tests.rs +++ b/pallets/parachain-staking/src/tests.rs @@ -32,7 +32,7 @@ use crate::mock::{ use crate::{ assert_events_emitted, assert_events_emitted_match, assert_events_eq, assert_no_events, AtStake, Bond, CollatorStatus, DelegationScheduledRequests, DelegatorAdded, - EnableMarkingOffline, Error, Event, HadStake, InflationDistributionInfo, Range, + EnableMarkingOffline, Error, Event, InflationDistributionInfo, Range, WasInactive, DELEGATOR_LOCK_ID, }; use frame_support::traits::{Currency, ExistenceRequirement, WithdrawReasons}; @@ -1175,12 +1175,13 @@ fn enable_marking_offline_fails_bad_origin() { } #[test] -fn hadstake_is_cleaned_up_after_max_offline_rounds() { +fn was_inactive_is_cleaned_up_after_max_offline_rounds() { const ACTIVE_COLLATOR: AccountId = 1; + const INACTIVE_COLLATOR: AccountId = 2; ExtBuilder::default() - .with_balances(vec![(1, 20), (2, 20), (3, 20), (4, 20), (5, 20)]) - .with_candidates(vec![(1, 20), (2, 20), (3, 20), (4, 20), (5, 20)]) + .with_balances(vec![(1, 20), (2, 20)]) + .with_candidates(vec![(1, 20), (2, 20)]) .build() .execute_with(|| { assert_eq!(::MaxOfflineRounds::get(), 2); @@ -1192,16 +1193,20 @@ fn hadstake_is_cleaned_up_after_max_offline_rounds() { // Round 2 roll_to_round_begin(2); - // ACTIVE_COLLATOR has a stake in round 1 assert!(>::contains_key(1, ACTIVE_COLLATOR)); - assert!(>::contains_key(1, ACTIVE_COLLATOR)); + assert!(!>::contains_key(1, ACTIVE_COLLATOR)); + + assert!(>::contains_key(1, INACTIVE_COLLATOR)); + assert!(>::contains_key(1, INACTIVE_COLLATOR)); // Round 3 roll_to_round_begin(3); - // ACTIVE_COLLATOR has a stake in round 2 assert!(>::contains_key(2, ACTIVE_COLLATOR)); - assert!(>::contains_key(2, ACTIVE_COLLATOR)); + assert!(!>::contains_key(2, ACTIVE_COLLATOR)); + + assert!(>::contains_key(2, INACTIVE_COLLATOR)); + assert!(>::contains_key(2, INACTIVE_COLLATOR)); // End of round 3 roll_to_round_end(3); @@ -1211,20 +1216,28 @@ fn hadstake_is_cleaned_up_after_max_offline_rounds() { "Active collator should have no stake in round 1 due to the distribution of rewards" ); assert!( - >::contains_key(1, ACTIVE_COLLATOR), - "Active collator should still be in HadStake for round 1" + !>::contains_key(1, INACTIVE_COLLATOR), + "Inactive collator should have no stake in round 1 due to the distribution of rewards" + ); + + assert!( + !>::contains_key(1, ACTIVE_COLLATOR), + "Active collator should not be in WasInactive for round 1" + ); + assert!( + >::contains_key(1, INACTIVE_COLLATOR), + "Inactive collator should still be in WasInactive for round 1" ); // Round 4 roll_to_round_end(4); assert!( - !>::contains_key(1, ACTIVE_COLLATOR), - "Round 1 HadStake should be cleaned up after MaxOfflineRounds" + !>::contains_key(1, INACTIVE_COLLATOR), + "Round 1 WasInactive should be cleaned up after MaxOfflineRounds" ); - assert!(>::contains_key(2, ACTIVE_COLLATOR)); - assert!(>::contains_key(3, ACTIVE_COLLATOR)); - assert!(>::contains_key(4, ACTIVE_COLLATOR)); + assert!(>::contains_key(2, INACTIVE_COLLATOR)); + assert!(>::contains_key(3, INACTIVE_COLLATOR)); }); }