Skip to content
This repository has been 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
wants to merge 7 commits into from

Conversation

coriolinus
Copy link
Contributor

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.

@coriolinus coriolinus added A3-in_progress Pull request is in progress. No review needed at this stage. B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Jun 9, 2021
@coriolinus coriolinus self-assigned this Jun 9, 2021
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should keep an eye on #9083 (comment).

Comment on lines +74 to +77
pub struct VoterList<T: Config> {
head: Option<AccountIdOf<T>>,
tail: Option<AccountIdOf<T>>,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I am also fine with doing this only tailored for staking now, and later extract it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@coriolinus
Copy link
Contributor Author

Closing in favor of #9081; that approach seems to have fewer drawbacks.

@coriolinus coriolinus closed this Jun 29, 2021
@louismerlin louismerlin removed the D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited label Oct 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A3-in_progress Pull request is in progress. No review needed at this stage. C1-low PR touches the given topic and has a low impact on builders.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants