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

Simple Staking Payouts #5406

Merged
merged 32 commits into from
Apr 4, 2020
Merged

Simple Staking Payouts #5406

merged 32 commits into from
Apr 4, 2020

Conversation

shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented Mar 26, 2020

This is a simplified version of Flexible Payouts (#5268).

Here we keep nearly everything about the existing staking pallet the same, reducing the total number of changes introduced by an alternative payout mechanism.

The changes introduced are:

  • We track which specific eras have been rewarded between current_era - history_depth and current_era. This means that rewards can now be claimed out of order without issue.

  • We added a new extrinsic payout_stakers which takes a validator and era as an input, and pays out the validator and all their nominators at once.

  • We allow any signed origin to call payout_stakers. This means any account is able to pay the fees to trigger a payout for a validator and their nominators.

  • We "deprecated" payout_nominator and payout_validator, see the migration story below.

As a result of this PR, everyone who would be paid currently still gets paid. The only difference is that nominators will not need to manually claim their rewards. Validators payouts will do that for them automatically.

  • Comprehensive tests written
  • Documentation Updates
  • Migration Code

EDIT: Migration Story

We will use a "two-update" migration story to implement this new payout system.

The issue is that our current payment mechanism allows for nominators and validators to be out of sync in terms of payments, whereas this payment mechanism will keep them in sync.

When transitioning from the old mechanism to this mechanism, we need to find a way to bring the nominators and validators in sync. One approach would be to make payments to everyone for every era up to the current era, but I felt that this was a heavy and possibly error prone operation.

Rather, I decided to keep the old extrinsics in the codebase for now, but place a check that the old extrinsics could only be called to make payouts for eras before this update. Then, when new eras come, and people will request payouts, they will have to use the new extrinsic.

At some point, the old extrinsics will not be callable anymore (the upgrade era will be older than history depth), then we can do a simple second upgrade to remove the old extrinsics and any old logic.

The one thing we do need to migrate is the last_reward Option to claimed_rewards Vec. We do this for all nominators and validators for now.

When we remove the old extrinsics, we can also go through all the nominators and clean up their claimed_rewards vec, since it won't be used in the new logic.

@shawntabrizi shawntabrizi added the A0-please_review Pull request needs code review. label Mar 26, 2020
frame/staking/src/lib.rs Outdated Show resolved Hide resolved
@shawntabrizi
Copy link
Member Author

I did basic migration verification in the form of a test, but I struggled to set up a production test, and it was eating at some of my time.

Would appreciate any quick help here to verify the migration is sane for Kusama accounts.

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.

looks good to me otherwise

frame/staking/src/lib.rs Outdated Show resolved Hide resolved
frame/staking/src/tests.rs Show resolved Hide resolved
frame/staking/src/lib.rs Outdated Show resolved Hide resolved
@shawntabrizi
Copy link
Member Author

I have updated the original post with our two-step migration plan. This should be ready for final reviews and merge!

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.

lgtm

@gavofyork
Copy link
Member

@shawntabrizi needs resolving.

@gavofyork gavofyork added A8-mergeoncegreen and removed A0-please_review Pull request needs code review. labels Apr 3, 2020
@gavofyork gavofyork merged commit e33dae6 into master Apr 4, 2020
@gavofyork gavofyork deleted the shawntabrizi-simple-payouts branch April 4, 2020 12:50
akru added a commit to airalab/robonomics that referenced this pull request Apr 5, 2020
* Introduce Simple Staking Payout(paritytech/substrate#5406)
* ipci-runtime: version 1 -> 2
* robonomics-runtime: version 70 -> 80
* Additional Prometheus metrics
* Fix node building for latest substrate
* Version bump
@gregzaitsev
Copy link

The only difference is that nominators will not need to manually claim their rewards. Validators payouts will do that for them automatically.

Is it somehow guaranteed that validators call payout_stakers or you just assume that they will call it because their rewards are much larger than average nominator rewards?

@gui1117
Copy link
Contributor

gui1117 commented Apr 9, 2020

Is it somehow guaranteed that validators call payout_stakers or you just assume that they will call it because their rewards are much larger than average nominator rewards?

there is currently no mecanism provided to make it automatic, someone must call it, it can be anybody. Though it is the only way for the validator to gets its reward, but indeed it is also the only way for a nominator to get the part associated.

In the future we could think that the transactions fees are given back to the caller by taking from the payout but for now nothing is planned as far as I know.

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.

8 participants