Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use AtStake to dispatch rewards instead of AwardedPts #1896

Merged
merged 12 commits into from
Nov 10, 2022
7 changes: 4 additions & 3 deletions pallets/parachain-staking/src/benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
//! Benchmarking
use crate::{
AwardedPts, BalanceOf, Call, CandidateBondLessRequest, Config, DelegationAction, Pallet,
ParachainBondConfig, ParachainBondInfo, Points, Range, Round, ScheduledRequest, Staked,
TopDelegations,
ParachainBondConfig, ParachainBondInfo, Points, Range, RewardPayment, Round, ScheduledRequest,
Staked, TopDelegations,
};
use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, vec};
use frame_support::traits::{Currency, Get, OnFinalize, OnInitialize};
Expand Down Expand Up @@ -1036,7 +1036,8 @@ benchmarks! {
// TODO: this is an extra read right here (we should whitelist it?)
let payout_info = Pallet::<T>::delayed_payouts(round_for_payout).expect("payout expected");
let result = Pallet::<T>::pay_one_collator_reward(round_for_payout, payout_info);
assert!(result.0.is_some()); // TODO: how to keep this in scope so it can be done in verify block?
// TODO: how to keep this in scope so it can be done in verify block?
assert!(matches!(result.0, RewardPayment::Paid));
}
verify {
// collator should have been paid
Expand Down
49 changes: 27 additions & 22 deletions pallets/parachain-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,17 @@ pub mod pallet {
}
}

/// Represents a payout made via `pay_one_collator_reward`.
pub(crate) enum RewardPayment {
/// A collator was paid
Paid,
/// A collator was skipped for payment. This can happen if they haven't been awarded any
/// points, that is, they did not produce any blocks.
Skipped,
/// All collator payments have been processed.
Finished,
}

impl<T: Config> Pallet<T> {
pub fn is_delegator(acc: &T::AccountId) -> bool {
<DelegatorState<T>>::get(acc).is_some()
Expand Down Expand Up @@ -1518,22 +1529,13 @@ pub mod pallet {

if let Some(payout_info) = <DelayedPayouts<T>>::get(paid_for_round) {
let result = Self::pay_one_collator_reward(paid_for_round, payout_info);
if result.0.is_none() {
// result.0 indicates whether or not a payout was made
// clean up storage items that we no longer need

// clean up storage items that we no longer need
if matches!(result.0, RewardPayment::Finished) {
<DelayedPayouts<T>>::remove(paid_for_round);
<Points<T>>::remove(paid_for_round);

// remove all candidates that did not produce any blocks for
// the given round. The weight is added based on the number of backend
// items removed.
let remove_result = <AtStake<T>>::clear_prefix(paid_for_round, 20, None);
result
.1
.saturating_add(T::DbWeight::get().writes(remove_result.backend as u64))
} else {
result.1 // weight consumed by pay_one_collator_reward
}
result.1 // weight consumed by pay_one_collator_reward
} else {
Weight::from_ref_time(0u64)
}
Expand All @@ -1546,7 +1548,7 @@ pub mod pallet {
pub(crate) fn pay_one_collator_reward(
paid_for_round: RoundIndex,
payout_info: DelayedPayout<BalanceOf<T>>,
) -> (Option<(T::AccountId, BalanceOf<T>)>, Weight) {
) -> (RewardPayment, Weight) {
// TODO: it would probably be optimal to roll Points into the DelayedPayouts storage
// item so that we do fewer reads each block
let total_points = <Points<T>>::get(paid_for_round);
Expand All @@ -1557,22 +1559,25 @@ pub mod pallet {
// 2. we called pay_one_collator_reward when we were actually done with deferred
// payouts
log::warn!("pay_one_collator_reward called with no <Points<T>> for the round!");
return (None, Weight::zero());
return (RewardPayment::Finished, Weight::zero());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this weight fails to capture the <Points<T>> read above, and that may be true for weights returned in other places in this fn.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...it looks like this was the case before this PR...

}

let collator_fee = payout_info.collator_commission;
let collator_issuance = collator_fee * payout_info.round_issuance;

if let Some((collator, pts)) =
<AwardedPts<T>>::iter_prefix(paid_for_round).drain().next()
if let Some((collator, state)) =
<AtStake<T>>::iter_prefix(paid_for_round).drain().next()
{
// Take the awarded points for the collator
let pts = <AwardedPts<T>>::take(paid_for_round, &collator);
if pts == 0 {
return (RewardPayment::Skipped, T::DbWeight::get().reads(1));
}

let mut extra_weight = Weight::zero();
let pct_due = Perbill::from_rational(pts, total_points);
let total_paid = pct_due * payout_info.total_staking_reward;
let mut amt_due = total_paid;
// Take the snapshot of block author and delegations

let state = <AtStake<T>>::take(paid_for_round, &collator);

let num_delegators = state.delegations.len();
if state.delegations.is_empty() {
Expand Down Expand Up @@ -1619,14 +1624,14 @@ pub mod pallet {
}

(
Some((collator, total_paid)),
RewardPayment::Paid,
T::WeightInfo::pay_one_collator_reward(num_delegators as u32)
.saturating_add(extra_weight),
)
} else {
// Note that we don't clean up storage here; it is cleaned up in
// handle_delayed_payouts()
(None, Weight::from_ref_time(0u64.into()))
(RewardPayment::Finished, Weight::from_ref_time(0u64.into()))
}
}

Expand Down
13 changes: 11 additions & 2 deletions pallets/parachain-staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ impl ExtBuilder {
}

/// Rolls forward one block. Returns the new block number.
pub(crate) fn roll_one_block() -> BlockNumber {
fn roll_one_block() -> BlockNumber {
Balances::on_finalize(System::block_number());
System::on_finalize(System::block_number());
System::set_block_number(System::block_number() + 1);
Expand All @@ -261,7 +261,7 @@ pub(crate) fn roll_one_block() -> BlockNumber {
}

/// Rolls to the desired block. Returns the number of blocks played.
pub(crate) fn roll_to(n: BlockNumber) -> BlockNumber {
pub(crate) fn roll_to(n: BlockNumber) -> u32 {
let mut num_blocks = 0;
let mut block = System::block_number();
while block < n {
Expand All @@ -271,6 +271,15 @@ pub(crate) fn roll_to(n: BlockNumber) -> BlockNumber {
num_blocks
}

/// Rolls desired number of blocks. Returns the final block.
pub(crate) fn roll_blocks(num_blocks: u32) -> BlockNumber {
let mut block = System::block_number();
for _ in 0..num_blocks {
block = roll_one_block();
}
block
}

/// Rolls block-by-block to the beginning of the specified round.
/// This will complete the block in which the round change occurs.
/// Returns the number of blocks played.
Expand Down
Loading