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

Consolidate Nominator Support in Slashing and Rewards #473

Closed
kianenigma opened this issue Jul 13, 2020 · 8 comments · Fixed by #1189 · May be fixed by paritytech/substrate#13498
Closed

Consolidate Nominator Support in Slashing and Rewards #473

kianenigma opened this issue Jul 13, 2020 · 8 comments · Fixed by #1189 · May be fixed by paritytech/substrate#13498
Labels
I5-enhancement An additional feature request.

Comments

@kianenigma
Copy link
Contributor

Currently, staking is keeping two storage items:

https://github.com/paritytech/substrate/blob/b0eefebc2a85fe18b5bf072910ad8c4c614548e4/frame/staking/src/lib.rs#L1078-L1101

Where the latter is basically equal to the former, but the exposure.others has been clipped to 64. ErasStakers is used for slashing (and generally all purposes), and ErasStakersClipped is only kept around for rewarding.

https://github.com/paritytech/substrate/blob/b0eefebc2a85fe18b5bf072910ad8c4c614548e4/frame/staking/src/lib.rs#L2883-L2894

This is bad because as a nominator you will only get a slice of the reward if you are among the top 64, but you will get slashed pro rata anyhow.

Instead, we should only keep ErasStakersClipped around. Basically, each validator will only be backed by its top 64 supporters. If a validator is not among the top 64, then there is really no point in being exposed to slashing with no chance of being rewarded.

The trend of this change will be toward making participation in staking potentially limited in the future. At some point, we might have too many nominators and too few validators to be able to accompany all of them. In this case we should observe the formation of services from the community for nomination delegation (if that makes any sense -- or nomination pool), where few nominators gather to make their aggregate stake higher, enabling them to make it to the top 64.

@thiolliere we can work on this cooperatively.

@xlc
Copy link
Contributor

xlc commented Jul 13, 2020

We would love to see some nomination pool support from Substrate side, that will greatly simplify Acala staking liquidity protocol implementation, which is basically a cross-chain nomination pool.

@kianenigma
Copy link
Contributor Author

Correction from @rphmeier: We use session::historical for slashing.

Based on this, I really can't find any reason why we should keep both ErasStakers and ErasStakersClipped for the entire last HISTORY_DEPTH. Even if we don't want to have the improvement that I suggested in this issue, we should store ErasStakersClipped for the last HISTORY_DEPTH eras ErasStakers only for the current/active era.

A scan of the usages of eras_stakers(* and ErasStakers<T>:: confirms that we are not using this double map with any other era other than the active/current era.

@gui1117
Copy link
Contributor

gui1117 commented Jul 16, 2020

indeed I see multiple possibilities:

  • currently staking needs to hold ErasStakers only for [active_era, current_era], we could delete the ErasStaker of an era when the era is not active anymore, in end_session.

  • we could factorize to only make use of ErasStakersClipped everywhere:

    • first runtime upgrade: write into ErasStakers the ErasStakersClipped
    • second runtime update (after historical_depth sessions): make use of ErasStakersClipped everywhere and deleted ErasStakers.
  • (also good improvement) we could enforce the limit on the number of nominator per validator when accepting phragmen solutions, this would make the accepted solution score more sensible I think. (the clipping would be done offchain and the score would be taking into account this clipped nominations)

If web3F is ok to use the clipped exposure for slashing (even though the phragmen solution have score computed using full exposure) I can implement the second proposal.

@kianenigma
Copy link
Contributor Author

kianenigma commented Jul 22, 2020

If web3F is ok to use the clipped exposure for slashing (even though the phragmen solution have score computed using full exposure) I can implement the second proposal.

As mentioned in the comment above, slashing happens only with session::historical, so it is good.

Regarding your approach: We do we need two runtime migrations? AFAIK ErasStakersClipped is exactly the same as ErasStakers except for clipping 64 nominators, so with just one upgrade and PR we can:

  • Remove all usage of ErasStakers and replace it with ErasStakersClipped
  • Remove everything in ErasStakers.

@kianenigma
Copy link
Contributor Author

(also good improvement) we could enforce the limit on the number of nominator per validator when accepting phragmen solutions, this would make the accepted solution score more sensible I think. (the clipping would be done offchain and the score would be taking into account this clipped nominations)

Yeah good idea. Let's make this a parallel/follow up issue/PR that I can work on.

@kianenigma
Copy link
Contributor Author

will be closed by paritytech/substrate#11935

@kianenigma
Copy link
Contributor Author

cc @Ank4n

@Ank4n
Copy link
Contributor

Ank4n commented Dec 7, 2022

If web3F is ok to use the clipped exposure for slashing (even though the phragmen solution have score computed using full exposure) I can implement the second proposal.

As mentioned in the comment above, slashing happens only with session::historical, so it is good.

Regarding your approach: We do we need two runtime migrations? AFAIK ErasStakersClipped is exactly the same as ErasStakers except for clipping 64 nominators, so with just one upgrade and PR we can:

  • Remove all usage of ErasStakers and replace it with ErasStakersClipped
  • Remove everything in ErasStakers.

May be the better thing to do would be to implement the multi-block reward payout so we are able to pay all stakers and then get rid of the ErasStakersClipped storage instead.

@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I5-enhancement An additional feature request. and removed J0-enhancement labels Aug 25, 2023
@github-project-automation github-project-automation bot moved this from ⌛️ Sometime-soon to ✅ Done in (Nominated) Proof of Stake Nov 1, 2023
Ank4n added a commit that referenced this issue Nov 1, 2023
…1189)

helps #439.
closes #473.

PR link in the older substrate repository:
paritytech/substrate#13498.

# Context
Rewards payout is processed today in a single block and limited to
`MaxNominatorRewardedPerValidator`. This number is currently 512 on both
Kusama and Polkadot.

This PR tries to scale the nominators payout to an unlimited count in a
multi-block fashion. Exposures are stored in pages, with each page
capped to a certain number (`MaxExposurePageSize`). Starting out, this
number would be the same as `MaxNominatorRewardedPerValidator`, but
eventually, this number can be lowered through new runtime upgrades to
limit the rewardeable nominators per dispatched call instruction.

The changes in the PR are backward compatible.

## How payouts would work like after this change
Staking exposes two calls, 1) the existing `payout_stakers` and 2)
`payout_stakers_by_page`.

### payout_stakers
This remains backward compatible with no signature change. If for a
given era a validator has multiple pages, they can call `payout_stakers`
multiple times. The pages are executed in an ascending sequence and the
runtime takes care of preventing double claims.

### payout_stakers_by_page
Very similar to `payout_stakers` but also accepts an extra param
`page_index`. An account can choose to payout rewards only for an
explicitly passed `page_index`.

**Lets look at an example scenario**
Given an active validator on Kusama had 1100 nominators,
`MaxExposurePageSize` set to 512 for Era e. In order to pay out rewards
to all nominators, the caller would need to call `payout_stakers` 3
times.

- `payout_stakers(origin, stash, e)` => will pay the first 512
nominators.
- `payout_stakers(origin, stash, e)` => will pay the second set of 512
nominators.
- `payout_stakers(origin, stash, e)` => will pay the last set of 76
nominators.
...
- `payout_stakers(origin, stash, e)` => calling it the 4th time would
return an error `InvalidPage`.

The above calls can also be replaced by `payout_stakers_by_page` and
passing a `page_index` explicitly.

## Commission note
Validator commission is paid out in chunks across all the pages where
each commission chunk is proportional to the total stake of the current
page. This implies higher the total stake of a page, higher will be the
commission. If all the pages of a validator's single era are paid out,
the sum of commission paid to the validator across all pages should be
equal to what the commission would have been if we had a non-paged
exposure.

### Migration Note
Strictly speaking, we did not need to bump our storage version since
there is no migration of storage in this PR. But it is still useful to
mark a storage upgrade for the following reasons:

- New storage items are introduced in this PR while some older storage
items are deprecated.
- For the next `HistoryDepth` eras, the exposure would be incrementally
migrated to its corresponding paged storage item.
- Runtimes using staking pallet would strictly need to wait at least
`HistoryDepth` eras with current upgraded version (14) for the migration
to complete. At some era `E` such that `E >
era_at_which_V14_gets_into_effect + HistoryDepth`, we will upgrade to
version X which will remove the deprecated storage items.
In other words, it is a strict requirement that E<sub>x</sub> -
E<sub>14</sub> > `HistoryDepth`, where
E<sub>x</sub> = Era at which deprecated storages are removed from
runtime,
E<sub>14</sub> = Era at which runtime is upgraded to version 14.
- For Polkadot and Kusama, there is a [tracker
ticket](#433) to clean
up the deprecated storage items.

### Storage Changes

#### Added
- ErasStakersOverview
- ClaimedRewards
- ErasStakersPaged

#### Deprecated
The following can be cleaned up after 84 eras which is tracked
[here](#433).

- ErasStakers.
- ErasStakersClipped.
- StakingLedger.claimed_rewards, renamed to
StakingLedger.legacy_claimed_rewards.

### Config Changes
- Renamed MaxNominatorRewardedPerValidator to MaxExposurePageSize.

### TODO
- [x] Tracker ticket for cleaning up the old code after 84 eras.
- [x] Add companion.
- [x] Redo benchmarks before merge.
- [x] Add Changelog for pallet_staking.
- [x] Pallet should be configurable to enable/disable paged rewards.
- [x] Commission payouts are distributed across pages.
- [x] Review documentation thoroughly.
- [x] Rename `MaxNominatorRewardedPerValidator` ->
`MaxExposurePageSize`.
- [x] NMap for `ErasStakersPaged`.
- [x] Deprecate ErasStakers.
- [x] Integrity tests.

### Followup issues
[Runtime api for deprecated ErasStakers storage
item](#426)

---------

Co-authored-by: Javier Viola <[email protected]>
Co-authored-by: Ross Bulat <[email protected]>
Co-authored-by: command-bot <>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this issue Mar 25, 2024
…aritytech#1189)

helps paritytech#439.
closes paritytech#473.

PR link in the older substrate repository:
paritytech/substrate#13498.

# Context
Rewards payout is processed today in a single block and limited to
`MaxNominatorRewardedPerValidator`. This number is currently 512 on both
Kusama and Polkadot.

This PR tries to scale the nominators payout to an unlimited count in a
multi-block fashion. Exposures are stored in pages, with each page
capped to a certain number (`MaxExposurePageSize`). Starting out, this
number would be the same as `MaxNominatorRewardedPerValidator`, but
eventually, this number can be lowered through new runtime upgrades to
limit the rewardeable nominators per dispatched call instruction.

The changes in the PR are backward compatible.

## How payouts would work like after this change
Staking exposes two calls, 1) the existing `payout_stakers` and 2)
`payout_stakers_by_page`.

### payout_stakers
This remains backward compatible with no signature change. If for a
given era a validator has multiple pages, they can call `payout_stakers`
multiple times. The pages are executed in an ascending sequence and the
runtime takes care of preventing double claims.

### payout_stakers_by_page
Very similar to `payout_stakers` but also accepts an extra param
`page_index`. An account can choose to payout rewards only for an
explicitly passed `page_index`.

**Lets look at an example scenario**
Given an active validator on Kusama had 1100 nominators,
`MaxExposurePageSize` set to 512 for Era e. In order to pay out rewards
to all nominators, the caller would need to call `payout_stakers` 3
times.

- `payout_stakers(origin, stash, e)` => will pay the first 512
nominators.
- `payout_stakers(origin, stash, e)` => will pay the second set of 512
nominators.
- `payout_stakers(origin, stash, e)` => will pay the last set of 76
nominators.
...
- `payout_stakers(origin, stash, e)` => calling it the 4th time would
return an error `InvalidPage`.

The above calls can also be replaced by `payout_stakers_by_page` and
passing a `page_index` explicitly.

## Commission note
Validator commission is paid out in chunks across all the pages where
each commission chunk is proportional to the total stake of the current
page. This implies higher the total stake of a page, higher will be the
commission. If all the pages of a validator's single era are paid out,
the sum of commission paid to the validator across all pages should be
equal to what the commission would have been if we had a non-paged
exposure.

### Migration Note
Strictly speaking, we did not need to bump our storage version since
there is no migration of storage in this PR. But it is still useful to
mark a storage upgrade for the following reasons:

- New storage items are introduced in this PR while some older storage
items are deprecated.
- For the next `HistoryDepth` eras, the exposure would be incrementally
migrated to its corresponding paged storage item.
- Runtimes using staking pallet would strictly need to wait at least
`HistoryDepth` eras with current upgraded version (14) for the migration
to complete. At some era `E` such that `E >
era_at_which_V14_gets_into_effect + HistoryDepth`, we will upgrade to
version X which will remove the deprecated storage items.
In other words, it is a strict requirement that E<sub>x</sub> -
E<sub>14</sub> > `HistoryDepth`, where
E<sub>x</sub> = Era at which deprecated storages are removed from
runtime,
E<sub>14</sub> = Era at which runtime is upgraded to version 14.
- For Polkadot and Kusama, there is a [tracker
ticket](paritytech#433) to clean
up the deprecated storage items.

### Storage Changes

#### Added
- ErasStakersOverview
- ClaimedRewards
- ErasStakersPaged

#### Deprecated
The following can be cleaned up after 84 eras which is tracked
[here](paritytech#433).

- ErasStakers.
- ErasStakersClipped.
- StakingLedger.claimed_rewards, renamed to
StakingLedger.legacy_claimed_rewards.

### Config Changes
- Renamed MaxNominatorRewardedPerValidator to MaxExposurePageSize.

### TODO
- [x] Tracker ticket for cleaning up the old code after 84 eras.
- [x] Add companion.
- [x] Redo benchmarks before merge.
- [x] Add Changelog for pallet_staking.
- [x] Pallet should be configurable to enable/disable paged rewards.
- [x] Commission payouts are distributed across pages.
- [x] Review documentation thoroughly.
- [x] Rename `MaxNominatorRewardedPerValidator` ->
`MaxExposurePageSize`.
- [x] NMap for `ErasStakersPaged`.
- [x] Deprecate ErasStakers.
- [x] Integrity tests.

### Followup issues
[Runtime api for deprecated ErasStakers storage
item](paritytech#426)

---------

Co-authored-by: Javier Viola <[email protected]>
Co-authored-by: Ross Bulat <[email protected]>
Co-authored-by: command-bot <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request.
Projects
Status: Done
5 participants