[Staking] Rotate era reward pots through a fixed-size pool#11930
Conversation
Ank4n
changed the title
Ank4n
changed the title
| /// Must be strictly greater than [`Config::HistoryDepth`] so that a slot is only | ||
| /// reused after its previous era has been pruned and drained. The | ||
| /// [`integrity_test`] enforces this invariant at runtime startup. | ||
| pub(crate) const POT_POOL_SIZE: u32 = 200; |
There was a problem hiding this comment.
Didn't expose this as config since it doesn't practically need to be configured. Only requirement is that it's bigger than HistoryDepth (enforced by integrity_test). Also intended to keep it independent of HistoryDepth so changing it doesn't affect pot account assignment.
| )] | ||
| pub enum RewardKind { | ||
| /// Staker rewards (nominators + validators). | ||
| #[codec(index = 0)] |
There was a problem hiding this comment.
Pinned variant indices to be safe from any future mishappenings
| &pot_account, | ||
| remaining, | ||
| Precision::BestEffort, | ||
| Preservation::Expendable, |
There was a problem hiding this comment.
I was worried for a sec that this would cause the account to die, while it still has a provider, and it cannot due, causing us to not be able to drain the remaining balance. I still think this part is tricky, and should be double checked.
An existing test is surely validating that it works.
ATM I think Fortitude::Force is basically saving us, otherwise on paper this transfer should fail: Account has a provider, but is requesting all of its funds to be drained.
All in all, the intersection of Consumer/Provider and Preservation/Fortitude is quite tricky.
There was a problem hiding this comment.
Its okay even if it dies, no? At create, we check if account has a provider and if not add one. The intention for adding an explicit provider was to ensure no funds gets cleaned up from here as dust while the pot is active.
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
| This ensure we only use a fixed size of pot accounts for the lifetime of the | ||
| chain rather than growing per era. | ||
|
|
||
| An `integrity_test` enforces `POT_POOL_SIZE > HistoryDepth` so a slot is only |
There was a problem hiding this comment.
Since HistoryDepth is 84 for Westend / Kusama / Polkadot, isn't 200 too high? Why not 100 maybe?
There was a problem hiding this comment.
I was looking to make it at least 2x to protect against history depth changing, which tbh is unlikely in itself. That said, I think 200 is a safe number. Much larger than history depth, and still gives the bounded protection.
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
5 participants
Era reward pot accounts are now drawn from a fixed pool of
POT_POOL_SIZE = 200accounts, indexed byera % POT_POOL_SIZE, instead of one fresh account per era.This ensure we only use a fixed size of pot accounts for the lifetime of the chain rather than growing per era.