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

Abstracts elections-phragmen pallet to use NposSolver #12588

Merged
merged 50 commits into from
Feb 23, 2023
Merged

Abstracts elections-phragmen pallet to use NposSolver #12588

merged 50 commits into from
Feb 23, 2023

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented Oct 31, 2022 •
edited
Loading

This PR refactors the current elections-phragmen pallet to use an NposSolver instead of hardcoding one flavor of phragmen. In addition, it implements an approval voting election scheme where each voter backs a limited set of candidates, without requiring splitting the stake among the selected candidates.

When using the pallet-elections, one should consider switching to a simpler NposSolver type like the approvals voting scheme, which is lighter than phragmen and reasonable in most cases. Users of this pallet should keep in mind that the election happens periodically on_initialize, and thus ensure that the election weight fits within a block (there is an integrity test to ensure the runtime configs comply).

Note that, although this PR renamed the old pallet-elections-phragmen to pallet-elections, the pallet name in the runtimes did not change in order to avoid a runtime upgrade (we could use move_pallet but that's not recommended). Otherwise a runtime upgrade would have been required.

Finally, this PR removes the need for a separate benchmarking crate for elections-provider-support. This way, it is possible to use the CI bench bot to correctly calculate and commit weights (all changes related to this merged and more info about it in this PR #13431).

  • rename elections-phragmen to elections-generic
  • change name of the pallet's dir from elections-phragmen to elections
  • comments on changing the name of the pallet (but not the lockid)
  • implement an approval voting election algorithm and NposSolver wrapper (WIP Implements approval voting to use as a NposSolver #13367)
  • refactor election-provider-support benchmarking and update pallet and runtime weights with CI bench bot.
  • try with follow-chain on a block that performs a council election

polkadot companion: paritytech/polkadot#6278

Closes #8250
Follow up of #12563

@gpestana gpestana self-assigned this Oct 31, 2022
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Oct 31, 2022
@gpestana gpestana marked this pull request as draft October 31, 2022 08:03
@gpestana gpestana marked this pull request as ready for review October 31, 2022 08:03
kianenigma
kianenigma reviewed Nov 8, 2022
View reviewed changes
Copy link
Contributor

@kianenigma kianenigma left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks broadly good. Now we need to think about deployment: If we change the prefix of the pallet in construct_runtime!, then the whole pallet storage needs to be moved to the new prefix name.

So I see a few action items:

  1. We should provide that migration script for simplicity.
  2. There should be a clear update note on the PR stating that any chain has two options:
  • rename the pallet and run the migratin
  • keep the old pallet prefix in construct_runtime!
  1. we should put the appropriate labels on the PR to make sure it goes into the release notes.
  2. Would be good if we run this PR with follow-chain on a block that actually does a council election

and

a few housekeeping notes :D

  • as per contribution guide, avoiding force-pushes is advised.
  • label and project similarly advised

@kianenigma
Copy link
Contributor

I think we should rename the pallet as well fwiw.

kianenigma
kianenigma reviewed Nov 8, 2022
View reviewed changes
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.

The main challenge now is how you will reflect this change in making kitchensink-runtime compile, and next the polkadot companion.

gpestana and others added 2 commits November 8, 2022 23:17
@gpestana @kianenigma
Update frame/elections-phragmen/src/lib.rs
452292f 
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@gpestana @kianenigma
Update frame/elections-phragmen/src/lib.rs
bf8d1c2 
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@gpestana
Copy link
Contributor Author

gpestana commented Nov 8, 2022

I think we should rename the pallet as well fwiw.

Yes, was leaving that to the end. renaming now

@gpestana
Copy link
Contributor Author

bot rebase

@gpestana gpestana mentioned this pull request Nov 13, 2022
Open
@gpestana gpestana added A3-in_progress Pull request is in progress. No review needed at this stage. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Nov 14, 2022
@paritytech-processbot
Copy link

Rebased

@gpestana
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Branch is already up-to-date

@gpestana
Copy link
Contributor Author

bot merge

@paritytech-processbot
Copy link

Error: Github API says paritytech/polkadot#6278 is not mergeable

@gpestana
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 4be5dd2 into master Feb 23, 2023
@paritytech-processbot paritytech-processbot bot deleted the gpestana8250_npossolver branch February 23, 2023 11:21
gpestana added a commit that referenced this pull request Feb 23, 2023
@gpestana
Revert "Abstracts elections-phragmen pallet to use NposSolver (#12588)"
d49421d 
This reverts commit 4be5dd2.
@gpestana gpestana mentioned this pull request Feb 23, 2023
Merged
kianenigma pushed a commit that referenced this pull request Feb 23, 2023
@gpestana
Revert "Abstracts elections-phragmen pallet to use NposSolver (#12588)…
fbddfbd 
…" (#13451)

This reverts commit 4be5dd2.
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@gpestana @kianenigma @ark0f
Abstracts elections-phragmen pallet to use NposSolver (paritytech#12588)
fdb4b39 
* Abstracts elections-phragmen pallet to use NposSolver

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* changes the name of the pallet; adds changelog

* update changelog

* Adds weight testing

* Adds log macro_rules

* renames elections-phragment dir to elections

* weights rename

* fixes typo in cargo toml

* pre/post solve weight scafolding

* refactor do_post_election

* refactors into pre and post election solve for independent benchmarking

* deconstructs PreElectionResults struct

* updates benchmarking pre and post election solve; mock weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* addresses PR comments

* adds pre_solve and post_sove weights

* Adds comments on election pallet id param name change

* ".git/.scripts/bench-bot.sh" pallet dev pallet_elections

* Finishes pre-post solve weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Addresses PR comments: no panic in on_init path; nits

* Fixes node build

* Implements approval voting to use as a `NposSolver` (paritytech#13367)

* Implements the approval voting methods in sp_npos_elections

* fmt

* remove unecessary file

* comment clarification

* re-run weights

* fix typo

* updates MaxVoters in tests for integrity_tests to pass

* Refactors election provider support benchmarks outside its own crate (paritytech#13431)

* Refactors election provider support benchmarks outside its own crate
---------

Co-authored-by: command-bot <>

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: parity-processbot <>
Co-authored-by: Ross Bulat <ross@parity.io>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@gpestana @ark0f
Revert "Abstracts elections-phragmen pallet to use NposSolver (parity…
18ac78e 
…tech#12588)" (paritytech#13451)

This reverts commit 4be5dd2.
@kiltbot kiltbot mentioned this pull request Apr 3, 2023
Closed
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/mis-configuring-pallet-elections-phragmen/2502/1

ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 10, 2023
@gpestana @kianenigma @ukint-vs
Abstracts elections-phragmen pallet to use NposSolver (paritytech#12588)
dac1a80 
* Abstracts elections-phragmen pallet to use NposSolver

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* changes the name of the pallet; adds changelog

* update changelog

* Adds weight testing

* Adds log macro_rules

* renames elections-phragment dir to elections

* weights rename

* fixes typo in cargo toml

* pre/post solve weight scafolding

* refactor do_post_election

* refactors into pre and post election solve for independent benchmarking

* deconstructs PreElectionResults struct

* updates benchmarking pre and post election solve; mock weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* addresses PR comments

* adds pre_solve and post_sove weights

* Adds comments on election pallet id param name change

* ".git/.scripts/bench-bot.sh" pallet dev pallet_elections

* Finishes pre-post solve weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Addresses PR comments: no panic in on_init path; nits

* Fixes node build

* Implements approval voting to use as a `NposSolver` (paritytech#13367)

* Implements the approval voting methods in sp_npos_elections

* fmt

* remove unecessary file

* comment clarification

* re-run weights

* fix typo

* updates MaxVoters in tests for integrity_tests to pass

* Refactors election provider support benchmarks outside its own crate (paritytech#13431)

* Refactors election provider support benchmarks outside its own crate
---------

Co-authored-by: command-bot <>

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: parity-processbot <>
Co-authored-by: Ross Bulat <ross@parity.io>
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 10, 2023
@gpestana @ukint-vs
Revert "Abstracts elections-phragmen pallet to use NposSolver (parity…
aa78c85 
…tech#12588)" (paritytech#13451)

This reverts commit 4be5dd2.
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 13, 2023
@gpestana @kianenigma @ukint-vs
Abstracts elections-phragmen pallet to use NposSolver (paritytech#12588)
4442710 
* Abstracts elections-phragmen pallet to use NposSolver

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* changes the name of the pallet; adds changelog

* update changelog

* Adds weight testing

* Adds log macro_rules

* renames elections-phragment dir to elections

* weights rename

* fixes typo in cargo toml

* pre/post solve weight scafolding

* refactor do_post_election

* refactors into pre and post election solve for independent benchmarking

* deconstructs PreElectionResults struct

* updates benchmarking pre and post election solve; mock weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* addresses PR comments

* adds pre_solve and post_sove weights

* Adds comments on election pallet id param name change

* ".git/.scripts/bench-bot.sh" pallet dev pallet_elections

* Finishes pre-post solve weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Addresses PR comments: no panic in on_init path; nits

* Fixes node build

* Implements approval voting to use as a `NposSolver` (paritytech#13367)

* Implements the approval voting methods in sp_npos_elections

* fmt

* remove unecessary file

* comment clarification

* re-run weights

* fix typo

* updates MaxVoters in tests for integrity_tests to pass

* Refactors election provider support benchmarks outside its own crate (paritytech#13431)

* Refactors election provider support benchmarks outside its own crate
---------

Co-authored-by: command-bot <>

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: parity-processbot <>
Co-authored-by: Ross Bulat <ross@parity.io>
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 13, 2023
@gpestana @ukint-vs
Revert "Abstracts elections-phragmen pallet to use NposSolver (parity…
0786d80 
…tech#12588)" (paritytech#13451)

This reverts commit 4be5dd2.
breathx pushed a commit to gear-tech/substrate that referenced this pull request Apr 22, 2023
@gpestana @kianenigma @breathx
Abstracts elections-phragmen pallet to use NposSolver (paritytech#12588)
779ed89 
* Abstracts elections-phragmen pallet to use NposSolver

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* changes the name of the pallet; adds changelog

* update changelog

* Adds weight testing

* Adds log macro_rules

* renames elections-phragment dir to elections

* weights rename

* fixes typo in cargo toml

* pre/post solve weight scafolding

* refactor do_post_election

* refactors into pre and post election solve for independent benchmarking

* deconstructs PreElectionResults struct

* updates benchmarking pre and post election solve; mock weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* addresses PR comments

* adds pre_solve and post_sove weights

* Adds comments on election pallet id param name change

* ".git/.scripts/bench-bot.sh" pallet dev pallet_elections

* Finishes pre-post solve weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Addresses PR comments: no panic in on_init path; nits

* Fixes node build

* Implements approval voting to use as a `NposSolver` (paritytech#13367)

* Implements the approval voting methods in sp_npos_elections

* fmt

* remove unecessary file

* comment clarification

* re-run weights

* fix typo

* updates MaxVoters in tests for integrity_tests to pass

* Refactors election provider support benchmarks outside its own crate (paritytech#13431)

* Refactors election provider support benchmarks outside its own crate
---------

Co-authored-by: command-bot <>

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: parity-processbot <>
Co-authored-by: Ross Bulat <ross@parity.io>
breathx pushed a commit to gear-tech/substrate that referenced this pull request Apr 22, 2023
@gpestana @breathx
Revert "Abstracts elections-phragmen pallet to use NposSolver (parity…
9a71f87 
…tech#12588)" (paritytech#13451)

This reverts commit 4be5dd2.
@kianenigma kianenigma mentioned this pull request Jun 12, 2023
Open
@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@gpestana @kianenigma @nathanwhit
Abstracts elections-phragmen pallet to use NposSolver (paritytech#12588)
41bd196 
* Abstracts elections-phragmen pallet to use NposSolver

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* changes the name of the pallet; adds changelog

* update changelog

* Adds weight testing

* Adds log macro_rules

* renames elections-phragment dir to elections

* weights rename

* fixes typo in cargo toml

* pre/post solve weight scafolding

* refactor do_post_election

* refactors into pre and post election solve for independent benchmarking

* deconstructs PreElectionResults struct

* updates benchmarking pre and post election solve; mock weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* addresses PR comments

* adds pre_solve and post_sove weights

* Adds comments on election pallet id param name change

* ".git/.scripts/bench-bot.sh" pallet dev pallet_elections

* Finishes pre-post solve weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Addresses PR comments: no panic in on_init path; nits

* Fixes node build

* Implements approval voting to use as a `NposSolver` (paritytech#13367)

* Implements the approval voting methods in sp_npos_elections

* fmt

* remove unecessary file

* comment clarification

* re-run weights

* fix typo

* updates MaxVoters in tests for integrity_tests to pass

* Refactors election provider support benchmarks outside its own crate (paritytech#13431)

* Refactors election provider support benchmarks outside its own crate
---------

Co-authored-by: command-bot <>

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: parity-processbot <>
Co-authored-by: Ross Bulat <ross@parity.io>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@gpestana @nathanwhit
Revert "Abstracts elections-phragmen pallet to use NposSolver (parity…
9ce0166 
…tech#12588)" (paritytech#13451)

This reverts commit 4be5dd2.
@gpestana gpestana mentioned this pull request Oct 31, 2022
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kianenigma kianenigma kianenigma approved these changes

@Ank4n Ank4n Ank4n approved these changes

@rossbulat rossbulat rossbulat approved these changes

Assignees

@gpestana gpestana

Labels
B1-note_worthy Changes should be noted in the 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 T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
(Nominated) Proof of Stake
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use ElectionProvider for pallet-elections-phragmen
7 participants
@gpestana @kianenigma @rossbulat @Polkadot-Forum @Ank4n @jakoblell @the-right-joyce