Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Pass max-total to RewardRemainder on end_era #5697

Merged
merged 3 commits into from
Apr 20, 2020
Merged

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Apr 19, 2020

Alternative to #5696 since it currently affects Kusama, no era rewards remainders are received

  • Adds the missing calc & pass from the previous version as found at
    let rest = max_payout.saturating_sub(total_payout);
    Self::deposit_event(RawEvent::Reward(total_payout, rest));
    T::Reward::on_unbalanced(total_imbalance);
    T::RewardRemainder::on_unbalanced(T::Currency::issue(rest));
  • Always done on end_era, no depth delay before treasury has the funds

@jacogr jacogr requested a review from kianenigma as a code owner April 19, 2020 06:15
@jacogr jacogr added the A0-please_review Pull request needs code review. label Apr 19, 2020
@jacogr
Copy link
Contributor Author

jacogr commented Apr 19, 2020

Tested it against a dev chain, getting the following at the end of the era -

image

@bkchr
Copy link
Member

bkchr commented Apr 19, 2020

If this pr is accepted, we should update the tests and make sure that this code is not removed a second time ;)

@gui1117
Copy link
Contributor

gui1117 commented Apr 19, 2020

ok indeed Staking::RewardRemainder::on_unbalance was called with reward_for_max_inflation - reward_for_current_inflation.

So maybe in a following PR but I think we should improve doc on the trait saying that

  • validator-payout = yearly_inflation(npos_token_staked / total_tokens) * total_tokens / era_per_year
  • minted = yearly_inflation_maxium * total_tokens / era_per_year
  • RewardRemainder is used for minted - validator-reward

Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

I approve if merged as is I'll make a new PR with tests and doc, I'm currently working on pushing test on this branch

@gui1117
Copy link
Contributor

gui1117 commented Apr 20, 2020

also in the previous code an event was send with this value maybe better to revive it as well.

Self::deposit_event(RawEvent::Reward(total_payout, rest));

@gui1117 gui1117 self-requested a review April 20, 2020 10:07
@gui1117
Copy link
Contributor

gui1117 commented Apr 20, 2020

I add doc and test, someone should review this few new lines and then we can merge

@gui1117 gui1117 requested a review from shawntabrizi April 20, 2020 10:24
@shawntabrizi shawntabrizi added A8-mergeoncegreen and removed A0-please_review Pull request needs code review. labels Apr 20, 2020
@bkchr bkchr merged commit 1590165 into master Apr 20, 2020
@bkchr bkchr deleted the jg-rewardRemainder branch April 20, 2020 11:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants