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

Bump Nominator Reward Limits #1668

Merged
7 commits merged into from
Sep 16, 2020
Merged

Bump Nominator Reward Limits #1668

7 commits merged into from
Sep 16, 2020

Conversation

kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Sep 1, 2020

Polkadot is already passed this limit and there are half a dozen or so validators who have already been saturated, causing the reward of at least a few hundred nominators to remain unclaimed. Since this limit was arbitrarily set before benchmarks, now we can increase it.

Will re-benchmark to apply the changes to the staking weights.

Companion for: paritytech/substrate#7007

@kianenigma kianenigma added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B7-runtimenoteworthy C3-medium PR touches the given topic and has a medium impact on builders. labels Sep 1, 2020
@shawntabrizi
Copy link
Member

shawntabrizi commented Sep 1, 2020

I would also suggest we reduce the number of historical eras we keep. Right now, the number is quite large, which imo is not needed given the current mechanisms and incentives.

Originally a long history was important cause nominators had to each individually reward themselves, but now that it happens through validators, this timeline could be short.

We would do this through a Root call (so council/democracy)

@wpank
Copy link
Contributor

wpank commented Sep 4, 2020

I think the current Historical Eras (84) is actually very useful for deriving metrics on performance. I would be in favor of keeping it for now.

@gui1117
Copy link
Contributor

gui1117 commented Sep 10, 2020

who will decide on this ?
what is the reason for having more nominator per validator in polkadot than kusama, because of the amount of stakers ?

@kianenigma
Copy link
Contributor Author

what is the reason for having more nominator per validator in polkadot than kusama, because of the amount of stakers ?

Yes, although I am open to other suggestions. The main point for now is to finish paritytech/substrate#7007 sot that we can do it. The value is less relevant to me.

@kianenigma kianenigma added A0-please_review Pull request needs code review. and removed A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). labels Sep 15, 2020
@kianenigma
Copy link
Contributor Author

also made to be the companion for paritytech/substrate#7007

@kianenigma
Copy link
Contributor Author

With 64:
Call staking_payout_stakers (with default args) takes 0.017354368 of the block weight
With 256:
Call staking_payout_stakers (with default args) takes 0.069417472 of the block weight

I think 256 is a good number to go for.


@jacogr Pinging you to update your 64 as well. Although if it is dynamic, then nothing to do from your side.

@jacogr
Copy link
Contributor

jacogr commented Sep 15, 2020

Dynamic based on the constants exposed. However with the weights adjusted would need some tweaks since at the previous there were a max of 40 payout txs in a batch based on tests. So at say 128 it would be (64 / 128) * 40 that can be included in a single batch tx from the UI. (batch is only for a single block)

With weights adjusted will re-calculate that (via testing), but initially it would just adjust as per above, probably fitting in less than the new weights would suggest. (the new "40 number" would need to be determined inside a batch)

runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Sep 16, 2020

Waiting for commit status.

@ghost ghost merged commit 5cbc418 into master Sep 16, 2020
@ghost ghost deleted the kiz-bump-reward-limits branch September 16, 2020 12:47
ordian added a commit that referenced this pull request Sep 16, 2020
* master:
  Bump Nominator Reward Limits (#1668)
ordian added a commit that referenced this pull request Sep 17, 2020
* master:
  Bump Nominator Reward Limits (#1668)
  Update Substrate and bump version to 0.8.24 (#1720)
  `locked` flag added to building section. (#1723)
@gavofyork gavofyork added the A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). label Sep 18, 2020
@gavofyork gavofyork added the D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. label Sep 18, 2020
@redzsina
Copy link

We are done with our review for this PR and don't see any issues with it.

@shawntabrizi shawntabrizi added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Nov 11, 2020
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). C3-medium PR touches the given topic and has a medium impact on builders. D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants