This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Store a linked list of nominators in order of stake to avoid OOM when getting top N voters#9055
Closed
coriolinus wants to merge 7 commits intomasterfrom
Closed
Store a linked list of nominators in order of stake to avoid OOM when getting top N voters#9055coriolinus wants to merge 7 commits intomasterfrom
coriolinus wants to merge 7 commits intomasterfrom
Conversation
5 tasks
kianenigma
reviewed
Jun 11, 2021
Contributor
kianenigma
left a comment
There was a problem hiding this comment.
should keep an eye on #9083 (comment).
kianenigma
reviewed
Jun 11, 2021
Comment on lines
+74
to
+77
| pub struct VoterList<T: Config> { | ||
| head: Option<AccountIdOf<T>>, | ||
| tail: Option<AccountIdOf<T>>, | ||
| } |
Contributor
There was a problem hiding this comment.
Suggested change
| pub struct VoterList<T: Config> { | |
| head: Option<AccountIdOf<T>>, | |
| tail: Option<AccountIdOf<T>>, | |
| } | |
| pub struct DoubleLinkedList<T: ...> { | |
| head: Option<T>, | |
| tail: Option<T>, | |
| } |
don't want to make things too hard for you, but this might help in the future. Then you can just call it something generic like DoubleLinkedList.
Contributor
There was a problem hiding this comment.
but I am also fine with doing this only tailored for staking now, and later extract it.
Contributor
Author
There was a problem hiding this comment.
I'm inclined to keep things specific for now and extract it later. Given that we need multiple storage items, I don't yet have a good sense of how we'd generalize this.
Contributor
Author
|
Closing in favor of #9081; that approach seems to have fewer drawbacks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem: there are too many nominators, we can't keep all of them.
Solution: truncate the list by stake.
Problem: an attacker can kick out as many people as they can afford to; those people at the low end would need to re-nominate.
Solution: don't actually take them out of the nominator set, but compute the top N members at runtime.
Problem: computing the top N at runtime is expensive.
Solution: use an ordered linked list which is pre-sorted. Anytime someone wants to nominate, they have to specify where in the list they fall. That's cheap to verify, and the frontend handles the cost of computing it.
Problem: staked amounts can vary due to rewards and slashes.
Solution: add a transaction that anyone can call to update someone's position in the list. Still cheap to verify. Doesn't need an explicit reward, because nominators low on the list have incentive to take down anyone higher than them, to increase their own chance of getting onto the official list of voters when the election is called.
Todo
Do we even need a doubly-linked list? We might be able to get away with just singly-linking it.