Implements approval voting to use as a `NposSolver` #13367

gpestana · 2023-02-12T21:04:48Z

Implements an approval voting election scheme where each voter backs a limited set of candidates, without requiring splitting the stake among the selected candidates.

…ting

gpestana · 2023-02-12T21:57:36Z

/cmd queue -c bench-bot $ pallet dev pallet_election_provider_support

paritytech-cicd-pr · 2023-02-12T22:08:16Z

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2380291

gpestana · 2023-02-13T14:07:11Z

/cmd queue -c bench-bot $ pallet dev pallet_election_provider_support

primitives/npos-elections/src/approval_voting.rs

kianenigma

beautifully done.

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

gpestana · 2023-02-14T05:50:32Z

/cmd queue -c bench-bot $ pallet dev pallet_election_provider_support

primitives/npos-elections/src/approval_voting.rs

kianenigma · 2023-02-14T12:19:55Z

primitives/npos-elections/src/approval_voting.rs

+) -> Result<ElectionResult<AccountId, P>, crate::Error> {
+	let to_elect = to_elect.min(candidates.len());
+
+	let (mut candidates, mut voters) = setup_inputs(candidates, voters);


ahh interesting, I didn't recall that setup_inputs calculates approval stakes. Does it too much other stuff too? then we should perhaps extract the approval stake calculation to optimize.

We get a few things from fn setup_inputs:

Build a Vec of Rc<Ref<Candidates>> from a vec of candidate account IDs. Using pointers here helps to optimise the the subsequent computation;

Removes duplicate vote edges, if existent.

Calculate approval state per candidate;

I think it won't be easy to optimise this further for the approval voting case.

primitives/npos-elections/src/approval_voting.rs

gpestana · 2023-02-14T13:48:48Z

bot rebase

…estana/approval_voting

paritytech-processbot · 2023-02-14T13:49:03Z

Rebased

gpestana · 2023-02-15T00:38:33Z

bot rebase

paritytech-processbot · 2023-02-15T00:38:38Z

Branch is already up-to-date

gpestana · 2023-02-15T00:39:45Z

bot rebase

…estana/approval_voting

paritytech-processbot · 2023-02-15T00:39:51Z

Rebased

primitives/npos-elections/src/tests.rs

gpestana · 2023-02-15T00:51:32Z

Once the two outstanding comments above are ack'ed/addressed, I will merge this PR into #12588 and take it from there.

gpestana · 2023-02-16T11:23:12Z

/cmd queue -c bench-bot $ pallet dev pallet_election_provider_support

ruseinov · 2023-02-18T09:15:16Z

primitives/npos-elections/src/approval_voting.rs

+			w.borrow_mut().elected = true;
+			w
+		})
+		.map(|w_ptr| (w_ptr.borrow().who.clone(), w_ptr.borrow().approval_stake))


nit: Curious if we need to borrow here to clone who.

ruseinov · 2023-02-18T09:19:07Z

primitives/npos-elections/src/approval_voting.rs

+///   ExtendedBalance and may be slightly different that what will be computed from the support map,
+///   due to accuracy loss.
+///
+/// This can only fail of the normalization fails. This can happen if for any of the resulting


ruseinov · 2023-02-18T09:20:48Z

A couple of nits, otherwise found nothing wrong with this PR

* 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` (#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 (#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>

* 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>

gpestana added 2 commits February 9, 2023 13:36

Implements the approval voting methods in sp_npos_elections

401a2f0

Replaces phragmen elections in pallet-elections tests for approval vo…

5e5d0ff

…ting

gpestana requested a review from kianenigma as a code owner February 12, 2023 21:04

gpestana marked this pull request as draft February 12, 2023 21:04

gpestana self-assigned this Feb 12, 2023

github-actions bot added the A0-please_review Pull request needs code review. label Feb 12, 2023

gpestana requested review from rossbulat and Ank4n February 12, 2023 21:06

gpestana added the C1-low PR touches the given topic and has a low impact on builders. label Feb 12, 2023

gpestana added 2 commits February 12, 2023 18:50

nits for checks

5782216

rfmt

a7643af

paritytech deleted a comment from paritytech-cicd-pr Feb 12, 2023

Uses ApprovalVoting in node runtime

e22b87c

kianenigma reviewed Feb 14, 2023

View reviewed changes

primitives/npos-elections/src/approval_voting.rs Outdated Show resolved Hide resolved

kianenigma approved these changes Feb 14, 2023

View reviewed changes

Update primitives/npos-elections/src/approval_voting.rs

46697cc

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

gpestana marked this pull request as ready for review February 14, 2023 05:25

github-actions bot added the A0-pleasereview label Feb 14, 2023

kianenigma reviewed Feb 14, 2023

View reviewed changes

primitives/npos-elections/src/approval_voting.rs Outdated Show resolved Hide resolved

kianenigma reviewed Feb 14, 2023

View reviewed changes

primitives/npos-elections/src/approval_voting.rs Outdated Show resolved Hide resolved

reverts num max voters tests

0b4a52d

Merge remote-tracking branch 'origin/gpestana8250_npossolver' into gp…

198082a

…estana/approval_voting

gpestana added 3 commits February 14, 2023 22:24

Adds test

87503ee

Merge branch 'master' into gpestana/approval_voting

3e001b2

Removes vote normalization for approval voting

186cf8d

gpestana requested a review from koute as a code owner February 15, 2023 00:37

Merge remote-tracking branch 'origin/gpestana8250_npossolver' into gp…

f4a0f34

…estana/approval_voting

gpestana commented Feb 15, 2023

View reviewed changes

primitives/npos-elections/src/tests.rs Show resolved Hide resolved

gpestana mentioned this pull request Feb 15, 2023

Abstracts elections-phragmen pallet to use NposSolver #12588

Merged

6 tasks

gpestana removed the D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. label Feb 17, 2023

gpestana merged commit 5343a6c into gpestana8250_npossolver Feb 17, 2023

gpestana deleted the gpestana/approval_voting branch February 17, 2023 16:17

ruseinov reviewed Feb 18, 2023

View reviewed changes

ruseinov self-requested a review February 18, 2023 09:20

gpestana restored the gpestana/approval_voting branch April 27, 2023 14:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements approval voting to use as a `NposSolver` #13367

Implements approval voting to use as a `NposSolver` #13367

gpestana commented Feb 12, 2023

gpestana commented Feb 12, 2023

paritytech-cicd-pr commented Feb 12, 2023

gpestana commented Feb 13, 2023

kianenigma left a comment

gpestana commented Feb 14, 2023

kianenigma Feb 14, 2023

gpestana Feb 14, 2023

gpestana commented Feb 14, 2023

paritytech-processbot bot commented Feb 14, 2023

gpestana commented Feb 15, 2023

paritytech-processbot bot commented Feb 15, 2023

gpestana commented Feb 15, 2023

paritytech-processbot bot commented Feb 15, 2023

gpestana commented Feb 15, 2023

gpestana commented Feb 16, 2023

ruseinov Feb 18, 2023

ruseinov Feb 18, 2023

ruseinov commented Feb 18, 2023

Implements approval voting to use as a NposSolver #13367

Implements approval voting to use as a NposSolver #13367

Conversation

gpestana commented Feb 12, 2023

gpestana commented Feb 12, 2023

paritytech-cicd-pr commented Feb 12, 2023

gpestana commented Feb 13, 2023

kianenigma left a comment

Choose a reason for hiding this comment

gpestana commented Feb 14, 2023

kianenigma Feb 14, 2023

Choose a reason for hiding this comment

gpestana Feb 14, 2023

Choose a reason for hiding this comment

gpestana commented Feb 14, 2023

paritytech-processbot bot commented Feb 14, 2023

gpestana commented Feb 15, 2023

paritytech-processbot bot commented Feb 15, 2023

gpestana commented Feb 15, 2023

paritytech-processbot bot commented Feb 15, 2023

gpestana commented Feb 15, 2023

gpestana commented Feb 16, 2023

ruseinov Feb 18, 2023

Choose a reason for hiding this comment

ruseinov Feb 18, 2023

Choose a reason for hiding this comment

ruseinov commented Feb 18, 2023

Implements approval voting to use as a `NposSolver` #13367

Implements approval voting to use as a `NposSolver` #13367