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

PhragMMS election. #6685

Merged
38 commits merged into from
Sep 23, 2020
Merged

PhragMMS election. #6685

38 commits merged into from
Sep 23, 2020

Conversation

kianenigma
Copy link
Contributor

Next step of #6242


This adds the new election algorithm proposed to be used in staking by w3f, PhragMMS. Research paper here.

  • This also revamps the old election implementation to use smart pointers. This makes the code a whole lot easier to maintain.
  • Consequently some simplifications were applied to both staking and elections-phragmen.
  • This does not alter any runtime logic. The election is not yet enabled anywhere. That will be a follow up that can be monitored individually and I didn't wanted to mix that sensitive PR with this one.
  • I have already tested this on remote Kusama and the result looks fine and indeed it is better than seq-phragmen, yet the execution time is very very slow, i.e. a few days ago seq-phragmen with 10 balancing rounds took ~5sec on Kusama, while this took around ~50sec with 2 balancing rounds per candidate (executed in native with TestExternalities backend). This is not unexpected. I put some timings into the code and the main reason why this is slow is balanceing, which runs two iterations per candidate that we pick. In short, the current election (seq-phragmen + balancing) us runing 5-10 balancing rounds now, while this will run staking::validator_count() * N, where I've set N = 2 for now, which is still a damn 800 rounds. This is fine, but we should expect some spikes in CPU usage once it is deployed. I will detail these stuff in the follow up PR.

Some follow ups:

@kianenigma kianenigma added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jul 19, 2020
@kianenigma kianenigma requested a review from andresilva as a code owner July 19, 2020 17:46
@kianenigma kianenigma requested review from gavofyork, bkchr and gui1117 and removed request for andresilva July 19, 2020 17:47
@kianenigma
Copy link
Contributor Author

The changes here might have something to do with the bug being found here #4593

@kianenigma kianenigma added C7-high and removed C1-low PR touches the given topic and has a low impact on builders. labels Jul 20, 2020
@gavofyork
Copy link
Member

@kianenigma needs resolving and doesn't build.

@kianenigma kianenigma added C1-low PR touches the given topic and has a low impact on builders. and removed C7-high labels Jul 26, 2020
frame/elections-phragmen/src/lib.rs Show resolved Hide resolved
frame/staking/src/offchain_election.rs Show resolved Hide resolved
primitives/npos-elections/src/balancing.rs Outdated Show resolved Hide resolved
primitives/npos-elections/src/balancing.rs Outdated Show resolved Hide resolved
primitives/npos-elections/src/balancing.rs Outdated Show resolved Hide resolved
primitives/npos-elections/src/phragmms.rs Outdated Show resolved Hide resolved
primitives/npos-elections/src/phragmms.rs Show resolved Hide resolved
@kianenigma kianenigma added A5-grumble and removed A0-please_review Pull request needs code review. labels Jul 27, 2020
@kianenigma kianenigma changed the title Add PhragMMS election algorithm PhragMMS election. Sep 15, 2020
@kianenigma kianenigma added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Sep 22, 2020
@kianenigma
Copy link
Contributor Author

@thiolliere / @shawntabrizi little things might have changed due to audit. I will finalize and merge this probably tomorrow. A brief re-review could potentially be very very valuable.

@kianenigma kianenigma added A0-please_review Pull request needs code review. and removed A1-onice labels Sep 22, 2020
@kianenigma
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Sep 23, 2020

Waiting for commit status.

@ghost
Copy link

ghost commented Sep 23, 2020

Merge failed: "Required status check \"continuous-integration/gitlab-check-labels\" is pending."

@kianenigma
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Sep 23, 2020

Trying merge.

@ghost ghost merged commit 38d5bb3 into master Sep 23, 2020
@ghost ghost deleted the kiz-impl-phragmms branch September 23, 2020 08:16
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. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

8 participants