Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
d723499
wip: poc
wischli Jul 20, 2022
82da1d9
refactor: extrinsics and inherents
wischli Jul 28, 2022
03cd34d
refactor: remove storage redundancy
wischli Jul 29, 2022
766c777
tests: fix existing ones for staking
wischli Jul 29, 2022
791ef9c
tests: 80%
wischli Aug 1, 2022
38e92dd
feat: rm delegate_another_candidate and more
wischli Aug 1, 2022
d8bb955
docs: remove deprecated weight
wischli Aug 1, 2022
24dc4bf
Apply suggestions from code review
wischli Aug 2, 2022
7789cda
tests: 100%
wischli Aug 2, 2022
870b534
fix: benchmarks
wischli Aug 3, 2022
8926e4c
feat: remove multiple delegations (#391)
wischli Aug 8, 2022
3dd9c6d
Merge remote-tracking branch 'origin/wf/1930-staking-refactor' into w…
wischli Aug 8, 2022
05f520e
Merge branch 'develop' into wf/1930-staking-refactor
wischli Aug 8, 2022
f8c1419
Apply suggestions from code review
wischli Aug 9, 2022
ad13650
fix: suggestions from @ntn-x2 review
wischli Aug 9, 2022
adbcd46
fmt
wischli Aug 9, 2022
ddebe3a
fix: typos
wischli Aug 9, 2022
28e47ef
Merge remote-tracking branch 'origin/develop' into wf/1930-staking-re…
wischli Aug 10, 2022
7b9fda3
fix: docker file
wischli Aug 11, 2022
3e21d0c
Merge remote-tracking branch 'origin/develop' into wf/1930-staking-re…
wischli Sep 7, 2022
e70c044
feat: add staking rewards runtime api
wischli Sep 8, 2022
b1de42a
docs: add new extrinsics to staking header
wischli Sep 8, 2022
6f73296
feat: add staking rates api
wischli Sep 8, 2022
20fd523
tests: add api
wischli Sep 8, 2022
0f8160b
fix: unify staking runtime api
wischli Sep 9, 2022
420170d
refactor: stricter claiming
wischli Sep 15, 2022
25f1878
Merge remote-tracking branch 'origin/develop' into wf/1930-staking-re…
wischli Sep 15, 2022
a3b2631
fix: easy suggestions from code review
wischli Sep 16, 2022
e703493
tests: comp api claim with claiming
wischli Sep 19, 2022
02eb407
stlye: fmt
wischli Sep 19, 2022
dfbd9e3
Merge remote-tracking branch 'origin/develop' into wf/1930-staking-re…
wischli Oct 27, 2022
bc0890f
Merge remote-tracking branch 'origin/develop' into wf/1930-staking-re…
wischli Oct 28, 2022
36e4abd
tests: add safety check to API rewards
wischli Oct 31, 2022
56a5539
style: fmt
wischli Oct 31, 2022
98eab08
Merge branch 'develop' into wf/1930-staking-refactor
wischli Oct 31, 2022
30044dd
feat: write delegator state migration
wischli Sep 16, 2022
30be731
refactor: Delegator interface functions
wischli Nov 1, 2022
55c6ac3
refactor: rm Option for Delegator.owner
wischli Nov 1, 2022
cd9684e
style: apply turbofish to staking
wischli Nov 1, 2022
f459050
fix: replace RewardCount with two counters
wischli Nov 2, 2022
39e3716
refactor: rm col input for del stake adjustment
wischli Nov 2, 2022
9102cfb
Update pallets/parachain-staking/src/lib.rs
wischli Nov 3, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
tests: fix existing ones for staking
  • Loading branch information
wischli committed Jul 29, 2022
commit 766c7778383b6e0be2fc345e90e3d8f41efdee27
107 changes: 84 additions & 23 deletions pallets/parachain-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ pub mod pallet {

// set rewards for all collators and delegators due
CandidatePool::<T>::iter().for_each(|(id, state)| {
Self::inc_collator_reward(&id, state.stake);
Self::do_inc_collator_reward(&id, state.stake);
});

Self::deposit_event(Event::RoundInflationSet(
Expand Down Expand Up @@ -937,7 +937,6 @@ pub mod pallet {
/// - Reads: [Origin Account], Round
/// - Writes: Round
/// # </weight>
// TODO: Maybe refactor to set rewards for all network participants
#[pallet::weight(<T as pallet::Config>::WeightInfo::set_blocks_per_round())]
pub fn set_blocks_per_round(origin: OriginFor<T>, new: T::BlockNumber) -> DispatchResult {
ensure_root(origin)?;
Expand Down Expand Up @@ -1397,7 +1396,7 @@ pub mod pallet {
CandidatePool::<T>::insert(&collator, state);

// increment rewards for origin + their delegators and reset reward counter
Self::inc_collator_reward(&collator, before_stake);
Self::do_inc_collator_reward(&collator, before_stake);

Self::deposit_event(Event::CollatorStakedMore(collator, before_stake, after_stake));
Ok(Some(<T as pallet::Config>::WeightInfo::candidate_stake_more(
Expand Down Expand Up @@ -1474,7 +1473,7 @@ pub mod pallet {
CandidatePool::<T>::insert(&collator, state);

// increment rewards for origin + their delegators and reset reward counter
Self::inc_collator_reward(&collator, before_stake);
Self::do_inc_collator_reward(&collator, before_stake);

Self::deposit_event(Event::CollatorStakedLess(collator, before_stake, after));
Ok(Some(<T as pallet::Config>::WeightInfo::candidate_stake_less(
Expand Down Expand Up @@ -1950,7 +1949,7 @@ pub mod pallet {
};

// set rewards and reset reward counter
Self::inc_delegator_reward(&delegator, stake_after.saturating_sub(more), &candidate);
Self::do_inc_delegator_reward(&delegator, stake_after.saturating_sub(more), &candidate);

CandidatePool::<T>::insert(&candidate, collator);
DelegatorState::<T>::insert(&delegator, delegations);
Expand Down Expand Up @@ -2042,7 +2041,7 @@ pub mod pallet {
};

// set rewards and reset reward counter
Self::inc_delegator_reward(&delegator, stake_after.saturating_add(less), &candidate);
Self::do_inc_delegator_reward(&delegator, stake_after.saturating_add(less), &candidate);

CandidatePool::<T>::insert(&candidate, collator);
DelegatorState::<T>::insert(&delegator, delegations);
Expand Down Expand Up @@ -2080,15 +2079,30 @@ pub mod pallet {
Ok(Some(<T as pallet::Config>::WeightInfo::unlock_unstaked(unstaking_len)).into())
}

// TODO: Benchmark, unit tests, docs
/// Claim block authoring rewards for the target address.
///
/// Requires `Rewards` to be set beforehand, which can by triggered by
/// any of the following options
/// * Calling increment_{collator, delegator}_rewards (active)
/// * Altering your stake (active)
/// * Leaving the network as a collator (active)
/// * Revoking a delegation as a delegator (active)
/// * Being a delegator whose collator left the network, altered their
/// stake or incremented rewards (passive)
///
/// The dispatch origin can be any signed one, e.g., anyone can claim
/// for anyone.
///
/// Emits `Rewarded`.
// TODO: Benchmark, unit tests
#[pallet::weight(0)]
pub fn claim_rewards_for(origin: OriginFor<T>, target: <T::Lookup as StaticLookup>::Source) -> DispatchResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific reason why we would allow anyone to trigger a payout for anyone else? I still don't understand what would be the use case for this, if I want to have control over when I would claim my rewards. If I want to delegate, I could set up a proxy,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, the main reason would be to cleanup storage and consume tiny rewards that have not been claimed by potentially exited users. I understand that's a very unlikely scenario and each user has an incentive to claim their rewards.

Moreover, in frame staking this is also possible by triggering a payout for a validator which rewards their nominators as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am completely fine changing this and would be interested in @weichweich 's opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think Parity does that because otherwise users would just forget and keep piling up stuff, I think. Which would be fine in our case if that is the assumption. But of course the downside is that I can't simply decide to trigger my reward at any time, because someone else inside my delegation pool might do it for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's first start with the more restrictive version. We might be able to change that later if we feel the need for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By more restrictive you are referring to Antonio's proposal, e.g. origins can only claim for themselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ensure_signed(origin)?;
let target = T::Lookup::lookup(target)?;

// we could kill the storage entry but let's be safe in case the deposit fails
let rewards = Rewards::<T>::get(&target);
ensure!(rewards.is_zero(), Error::<T>::RewardsNotFound);
ensure!(!rewards.is_zero(), Error::<T>::RewardsNotFound);

// mint into target and reset rewards
let rewards = T::Currency::deposit_into_existing(&target, rewards)?;
Expand All @@ -2099,6 +2113,56 @@ pub mod pallet {
Ok(())
}

/// Actively increment the rewards of a collator and their delegators.
///
/// The same effect is triggered by changing the stake or leaving the
/// network.
///
/// The dispatch origin must be a collator.
// TODO: Benchmark, unit tests
#[pallet::weight(0)]
pub fn increment_collator_rewards(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
let collator = ensure_signed(origin)?;
let state = CandidatePool::<T>::get(&collator).ok_or(Error::<T>::CandidateNotFound)?;

// early exit
let reward_count = RewardCount::<T>::get(&collator);
ensure!(!reward_count.is_zero(), Error::<T>::RewardsNotFound);

let weight = Self::do_inc_collator_reward(&collator, state.stake);
Ok(Some(weight).into())
}

/// Actively increment the rewards of a delegator for all their
/// delegations.
///
/// The same effect is triggered by changing the stake or revoking
/// delegations.
///
/// The dispatch origin must be a delegator.
// TODO: Benchmark, unit tests
#[pallet::weight(0)]
pub fn increment_delegator_rewards(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
let delegator = ensure_signed(origin)?;
let state = DelegatorState::<T>::get(&delegator).ok_or(Error::<T>::DelegatorNotFound)?;

// early exit
let reward_count = RewardCount::<T>::take(&delegator);
ensure!(!reward_count.is_zero(), Error::<T>::RewardsNotFound);

// iterate delegations
let mut post_weight: Weight = 0;
for delegation in state.delegations.into_iter() {
post_weight = post_weight.saturating_add(Self::do_inc_delegator_reward(
&delegator,
delegation.amount,
&delegation.owner,
));
}

Ok(Some(post_weight).into())
}

/// Executes the annual reduction of the reward rates for collators and
/// delegators. Moreover, sets rewards for all collators and delegators
/// before adjusting the inflation.
Expand Down Expand Up @@ -2140,7 +2204,7 @@ pub mod pallet {

// set rewards for all collators and delegators before updating reward rates
CandidatePool::<T>::iter().for_each(|(id, state)| {
Self::inc_collator_reward(&id, state.stake);
Self::do_inc_collator_reward(&id, state.stake);
});

// update inflation config
Expand Down Expand Up @@ -2436,7 +2500,7 @@ pub mod pallet {
let new_total = state.total;

// set rewards
Self::inc_delegator_reward(&delegator, delegator_stake, &collator);
Self::do_inc_delegator_reward(&delegator, delegator_stake, &collator);

// we don't unlock immediately
Self::prep_unstake(&delegator, delegator_stake, false)?;
Expand Down Expand Up @@ -2588,7 +2652,7 @@ pub mod pallet {
state.total = state.total.saturating_sub(stake_to_remove.amount);

// set rewards for kicked delegator
Self::inc_delegator_reward(&stake_to_remove.owner, stake_to_remove.amount, &state.id);
Self::do_inc_delegator_reward(&stake_to_remove.owner, stake_to_remove.amount, &state.id);

// update storage of kicked delegator
let kicked_delegator = Self::prep_kick_delegator(&stake_to_remove, &state.id)?;
Expand Down Expand Up @@ -2727,8 +2791,6 @@ pub mod pallet {
) -> DispatchResult {
// iterate over delegators
for stake in &state.delegators[..] {
// // increment rewards of delegator
// Self::inc_delegator_reward(&stake.owner, stake.amount, collator);
// prepare unstaking of delegator
Self::prep_unstake(&stake.owner, stake.amount, true)?;
// remove delegation from delegator state
Expand All @@ -2748,7 +2810,7 @@ pub mod pallet {
// *** No Fail beyond this point ***

// increment rewards of collator and their delegators
Self::inc_collator_reward(collator, state.stake);
Self::do_inc_collator_reward(collator, state.stake);

// disable validator for next session if they were in the set of validators
pallet_session::Pallet::<T>::validators()
Expand Down Expand Up @@ -2921,20 +2983,20 @@ pub mod pallet {
///
/// Resets all reward counters of the collator and their delegators to
/// zero.
fn inc_collator_reward(collator: &T::AccountId, stake: BalanceOf<T>) -> Weight {
fn do_inc_collator_reward(collator: &T::AccountId, stake: BalanceOf<T>) -> Weight {
let mut post_weight = Weight::zero();
// get reward counters
let col_reward_count = RewardCount::<T>::get(collator);

// set reward data for collator
Rewards::<T>::mutate(collator, |reward| {
reward.saturating_add(Self::calc_block_rewards_collator(stake, col_reward_count.into()))
*reward = reward.saturating_add(Self::calc_block_rewards_collator(stake, col_reward_count.into()));
});

// set reward data for delegators
if let Some(state) = CandidatePool::<T>::get(collator.clone()) {
for Stake { owner, amount } in state.delegators {
post_weight = post_weight.saturating_add(Self::inc_delegator_reward(&owner, amount, collator));
post_weight = post_weight.saturating_add(Self::do_inc_delegator_reward(&owner, amount, collator));
// Reset delegator counter since collator counter will be reset
RewardCount::<T>::insert(owner, 0);
post_weight = post_weight.saturating_add(T::DbWeight::get().reads(1));
Expand All @@ -2951,7 +3013,7 @@ pub mod pallet {
/// Increment the accumulated rewards of a delegator by consuming their
/// current rewards counter. The counter will be reset to the collator
/// counter.
fn inc_delegator_reward(acc: &T::AccountId, stake: BalanceOf<T>, col: &T::AccountId) -> Weight {
fn do_inc_delegator_reward(acc: &T::AccountId, stake: BalanceOf<T>, col: &T::AccountId) -> Weight {
// get reward counters
let del_reward_count = RewardCount::<T>::get(acc);
let col_reward_count = RewardCount::<T>::get(col);
Expand All @@ -2960,7 +3022,7 @@ pub mod pallet {
// only update if collator has higher reward count
if diff > 0 {
Rewards::<T>::mutate(acc, |reward| {
reward.saturating_add(Self::calc_block_rewards_delegator(stake, diff.into()))
*reward = reward.saturating_add(Self::calc_block_rewards_delegator(stake, diff.into()));
});
// TODO: Investigate whether we want to set this to some input variable to
// improve `add_collator_reward`
Expand Down Expand Up @@ -2990,17 +3052,16 @@ pub mod pallet {
where
T: Config + pallet_authorship::Config + pallet_session::Config,
{
// TODO: Docs
/// Increments the reward counter of the block author by the number of
/// collators.
/// Increments the reward counter of the block author by the current
/// number of collators in the session.
fn note_author(author: T::AccountId) {
// should always include state except if the collator has been forcedly removed
// via `force_remove_candidate` in the current or previous round
if CandidatePool::<T>::get(author.clone()).is_some() {
// necessary to compensate for a potentially fluctuating number of collators
let authors = pallet_session::Pallet::<T>::validators();
RewardCount::<T>::mutate(&author, |count| {
count.saturating_add(authors.len().saturated_into::<u32>())
*count = count.saturating_add(authors.len().saturated_into::<u32>());
});
}

Expand Down
26 changes: 25 additions & 1 deletion pallets/parachain-staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use super::*;
use crate::{self as stake, types::NegativeImbalanceOf};
use frame_support::{
construct_runtime, parameter_types,
assert_ok, construct_runtime, parameter_types,
traits::{Currency, GenesisBuild, OnFinalize, OnInitialize, OnUnbalanced},
weights::Weight,
};
Expand Down Expand Up @@ -370,6 +370,30 @@ pub(crate) fn roll_to(n: BlockNumber, authors: Vec<Option<AccountId>>) {
}
}

#[allow(unused_must_use)]
pub(crate) fn roll_to_claim_rewards(n: BlockNumber, authors: Vec<Option<AccountId>>) {
while System::block_number() < n {
if let Some(Some(author)) = authors.get((System::block_number()) as usize) {
StakePallet::note_author(*author);
// author must convert RewardCount to Rewards before claiming
assert_ok!(StakePallet::increment_collator_rewards(Origin::signed(*author)));
// author claims rewards
assert_ok!(StakePallet::claim_rewards_for(Origin::signed(*author), *author));

// claim rewards for delegators
let col_state = StakePallet::candidate_pool(author).expect("Block author must be candidate");
for delegation in col_state.delegators {
// NOTE: cannot use assert_ok! as we sometimes expect zero rewards for
// delegators such that the claiming would throw
StakePallet::claim_rewards_for(Origin::signed(*author), delegation.owner);
}
}
<AllPalletsReversedWithSystemFirst as OnFinalize<u64>>::on_finalize(System::block_number());
System::set_block_number(System::block_number() + 1);
<AllPalletsReversedWithSystemFirst as OnInitialize<u64>>::on_initialize(System::block_number());
}
}

pub(crate) fn last_event() -> Event {
System::events().pop().expect("Event expected").event
}
Expand Down
Loading