Skip to content

Commit

Permalink
perf: ⚡ use a WasInactive storage to keep track of inactive collators
Browse files Browse the repository at this point in the history
  • Loading branch information
manuelmauro committed Jan 10, 2025
1 parent d66eca5 commit b4f4c00
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 37 deletions.
49 changes: 27 additions & 22 deletions pallets/parachain-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -503,7 +505,7 @@ pub mod pallet {
}
fn on_finalize(_n: BlockNumberFor<T>) {
Self::award_points_to_block_author();
Self::cleanup_stake_info();
Self::cleanup_inactive_collator_info();
}
}

Expand Down Expand Up @@ -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<T: Config> =
pub type WasInactive<T: Config> =
StorageDoubleMap<_, Twox64Concat, RoundIndex, Twox64Concat, T::AccountId, (), OptionQuery>;

#[pallet::storage]
Expand Down Expand Up @@ -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 = <HadStake<T>>::get(r, &collator);
let pts = <AwardedPts<T>>::get(r, &collator);

if stake.is_some() && pts.is_zero() {
if <WasInactive<T>>::get(r, &collator).is_some() {
inactive_counter = inactive_counter.saturating_add(1);
}
}
Expand Down Expand Up @@ -2179,7 +2173,6 @@ pub mod pallet {
total: total_counted,
};
<AtStake<T>>::insert(now, account, snapshot);
<HadStake<T>>::insert(now, account, ());
Self::deposit_event(Event::CollatorChosen {
round: now,
collator_account: account.clone(),
Expand Down Expand Up @@ -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<T: Config> Pallet<T> {
/// 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 = <Round<T>>::get().current;
Expand All @@ -2355,17 +2346,31 @@ pub mod pallet {
<Points<T>>::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 <AtStake<T>>::iter_prefix(prev) {
if <AwardedPts<T>>::get(prev, &account).is_zero() {
<WasInactive<T>>::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 = <Round<T>>::get().current;
let minimum_rounds_required = T::MaxOfflineRounds::get() + 1;

if now < minimum_rounds_required {
return;
}

let _ = <HadStake<T>>::iter_prefix(now - minimum_rounds_required)
let _ = <WasInactive<T>>::iter_prefix(now - minimum_rounds_required)
.drain()
.next();
}
Expand Down
43 changes: 28 additions & 15 deletions pallets/parachain-staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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!(<Test as crate::Config>::MaxOfflineRounds::get(), 2);
Expand All @@ -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!(<AtStake<Test>>::contains_key(1, ACTIVE_COLLATOR));
assert!(<HadStake<Test>>::contains_key(1, ACTIVE_COLLATOR));
assert!(!<WasInactive<Test>>::contains_key(1, ACTIVE_COLLATOR));

assert!(<AtStake<Test>>::contains_key(1, INACTIVE_COLLATOR));
assert!(<WasInactive<Test>>::contains_key(1, INACTIVE_COLLATOR));

// Round 3
roll_to_round_begin(3);

// ACTIVE_COLLATOR has a stake in round 2
assert!(<AtStake<Test>>::contains_key(2, ACTIVE_COLLATOR));
assert!(<HadStake<Test>>::contains_key(2, ACTIVE_COLLATOR));
assert!(!<WasInactive<Test>>::contains_key(2, ACTIVE_COLLATOR));

assert!(<AtStake<Test>>::contains_key(2, INACTIVE_COLLATOR));
assert!(<WasInactive<Test>>::contains_key(2, INACTIVE_COLLATOR));

// End of round 3
roll_to_round_end(3);
Expand All @@ -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!(
<HadStake<Test>>::contains_key(1, ACTIVE_COLLATOR),
"Active collator should still be in HadStake for round 1"
!<AtStake<Test>>::contains_key(1, INACTIVE_COLLATOR),
"Inactive collator should have no stake in round 1 due to the distribution of rewards"
);

assert!(
!<WasInactive<Test>>::contains_key(1, ACTIVE_COLLATOR),
"Active collator should not be in WasInactive for round 1"
);
assert!(
<WasInactive<Test>>::contains_key(1, INACTIVE_COLLATOR),
"Inactive collator should still be in WasInactive for round 1"
);

// Round 4
roll_to_round_end(4);

assert!(
!<HadStake<Test>>::contains_key(1, ACTIVE_COLLATOR),
"Round 1 HadStake should be cleaned up after MaxOfflineRounds"
!<WasInactive<Test>>::contains_key(1, INACTIVE_COLLATOR),
"Round 1 WasInactive should be cleaned up after MaxOfflineRounds"
);
assert!(<HadStake<Test>>::contains_key(2, ACTIVE_COLLATOR));
assert!(<HadStake<Test>>::contains_key(3, ACTIVE_COLLATOR));
assert!(<HadStake<Test>>::contains_key(4, ACTIVE_COLLATOR));
assert!(<WasInactive<Test>>::contains_key(2, INACTIVE_COLLATOR));
assert!(<WasInactive<Test>>::contains_key(3, INACTIVE_COLLATOR));
});
}

Expand Down

0 comments on commit b4f4c00

Please sign in to comment.