make MAX_VOTERS and MAX_CANDIDATES in elections-phragmen configurable. Fix: #11092 #11908

sudipghimire533 · 2022-07-25T15:27:46Z

Closes: #11902
Polkadot companion: paritytech/polkadot#5815

Update

Allow max_voters & max_candidates to be configurable from Config trait of pallet_elections_phragmen
Put value 1000 (for max_candidates) & 10_000 (for max_voters) while creating runtime from /bin/node/runtime. Value taken as was in PR prep council election pallet for being dissolved #11790

Polkadot address: 126X27SbhrV19mBFawys3ovkyBS87SGfYwtwa8J2FjHrtbmA

…voters

cla-bot-2021 · 2022-07-25T15:27:52Z

User @sudipghimire533, please sign the CLA here.

kianenigma · 2022-07-25T16:29:49Z

frame/elections-phragmen/src/lib.rs

@@ -395,9 +392,10 @@ pub mod pallet {
 		) -> DispatchResultWithPostInfo {
 			let who = ensure_signed(origin)?;

+			let max_candidate = <T as Config>::MaxCandidates::get();


Suggested change

let max_candidate = <T as Config>::MaxCandidates::get();

kianenigma · 2022-07-25T16:29:59Z

frame/elections-phragmen/src/lib.rs

 			let actual_count = <Candidates<T>>::decode_len().unwrap_or(0) as u32;
 			ensure!(actual_count <= candidate_count, Error::<T>::InvalidWitnessData);
-			ensure!(actual_count <= MAX_CANDIDATES, Error::<T>::TooManyCandidates);
+			ensure!(actual_count <= max_candidate, Error::<T>::TooManyCandidates);


Suggested change

ensure!(actual_count <= max_candidate, Error::<T>::TooManyCandidates);

ensure!(actual_count <= <T as Config>::MaxCandidates::get(), Error::<T>::TooManyCandidates);

kianenigma · 2022-07-25T16:32:50Z

Looks mostly good, need to make companions.

sudipghimire533 · 2022-07-25T17:04:11Z

Polkadot companion: paritytech/polkadot#5815

Szegoo · 2022-07-26T07:40:43Z

@sudipghimire533 Note that you have to update the benchmarking.rs file also.

sudipghimire533 · 2022-07-26T09:34:59Z

@Szegoo maybe you can help me here:

error: expected one of `:`, `;`, `=`, `@`, or `|`, found keyword `in`
   --> frame/elections-phragmen/src/benchmarking.rs:394:9
    |
394 |         let e in T::MaxVoters::get() .. T::MaxVoters * MAXIMUM_VOTE as u32;
    |               ^^ expected one of `:`, `;`, `=`, `@`, or `|`

error: expected one of `:`, `;`, `=`, `@`, or `|`, found keyword `in`
   --> frame/elections-phragmen/src/benchmarking.rs:429:9
    |
429 |         let e in T::MaxVoters::get() .. T::MaxVoters::get() * MAXIMUM_VOTE as u32;
    |               ^^ expected one of `:`, `;`, `=`, `@`, or `|`

kianenigma · 2022-07-26T13:06:18Z

@Szegoo maybe you can help me here:

error: expected one of `:`, `;`, `=`, `@`, or `|`, found keyword `in`
   --> frame/elections-phragmen/src/benchmarking.rs:394:9
    |
394 |         let e in T::MaxVoters::get() .. T::MaxVoters * MAXIMUM_VOTE as u32;
    |               ^^ expected one of `:`, `;`, `=`, `@`, or `|`

error: expected one of `:`, `;`, `=`, `@`, or `|`, found keyword `in`
   --> frame/elections-phragmen/src/benchmarking.rs:429:9
    |
429 |         let e in T::MaxVoters::get() .. T::MaxVoters::get() * MAXIMUM_VOTE as u32;
    |               ^^ expected one of `:`, `;`, `=`, `@`, or `|`

I think you need to wrap it in a parenthesis:

substrate/frame/staking/src/benchmarking.rs

Line 815 in 54d6f2c

let v in (MaxValidators::<T>::get() / 2) .. MaxValidators::<T>::get();

sudipghimire533 · 2022-07-26T13:37:28Z

Not working same error and neither can I assign it as variable and re-use. @kianenigma do you think you can help me with benchmarking.rs ?

Szegoo · 2022-07-26T13:43:56Z

Not working same error and neither can I assign it as variable and re-use. @kianenigma do you think you can help me with benchmarking.rs ?

I have run into the same issue when working on this, the suggested solution from @kianenigma should work. Could you give me access to your branch, so maybe I manage to get it working?

sudipghimire533 · 2022-07-26T13:59:39Z

Not working same error and neither can I assign it as variable and re-use. @kianenigma do you think you can help me with benchmarking.rs ?

I have run into the same issue when working on this, the suggested solution from @kianenigma should work. Could you give me access to your branch, so maybe I manage to get it working?

@Szegoo I have invited you to my fork. Also I tried adding () as well. Not good from my side. Let's see

sudipghimire533 · 2022-07-26T14:22:15Z

Not working same error and neither can I assign it as variable and re-use. @kianenigma do you think you can help me with benchmarking.rs ?

I have run into the same issue when working on this, the suggested solution from @kianenigma should work. Could you give me access to your branch, so maybe I manage to get it working?

@Szegoo I have invited you to my fork. Also I tried adding () as well. Not good from my side. Let's see

Ahh you guys meant to only wrap the get. Gotcha but looks like benchmark verification failing no?

Jul 26 20:05:52.022 ERROR runtime::elections-phragmen: Failed to run election. Number of voters exceeded

frame/elections-phragmen/src/lib.rs

Szegoo · 2022-07-26T07:03:20Z

frame/elections-phragmen/src/lib.rs

@@ -397,7 +394,7 @@ pub mod pallet {

 			let actual_count = <Candidates<T>>::decode_len().unwrap_or(0) as u32;
 			ensure!(actual_count <= candidate_count, Error::<T>::InvalidWitnessData);
-			ensure!(actual_count <= MAX_CANDIDATES, Error::<T>::TooManyCandidates);
+			ensure!(actual_count <= <T as Config>::MaxCandidates::get(), Error::<T>::TooManyCandidates);


Suggested change

ensure!(actual_count <= <T as Config>::MaxCandidates::get(), Error::<T>::TooManyCandidates);

ensure!(actual_count <= T::MaxCandidates::get(), Error::<T>::TooManyCandidates);

ggwpez · 2022-08-08T09:03:54Z

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

command-bot · 2022-08-08T09:03:57Z

@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1726285 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_elections_phragmen. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment /cmd cancel 10-705eccf4-9bf2-4e1f-aed1-46615cdc9dbf to cancel this command or /cmd cancel to cancel all commands in this pull request.

command-bot · 2022-08-08T13:14:19Z

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_elections_phragmen has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1726285 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1726285/artifacts/download.

kianenigma · 2022-08-14T17:00:20Z

bot rebase

paritytech-processbot · 2022-08-14T17:00:26Z

Rebased

kianenigma · 2022-08-14T18:04:58Z

bot merge

kianenigma · 2022-08-14T18:05:44Z

https://github.com/sudipghimire533 @Szegoo can you both paste your Polkadot address for a tip here?

Szegoo · 2022-08-14T18:30:50Z

https://github.com/sudipghimire533 @Szegoo can you both paste your Polkadot address for a tip here?

I can't edit the PR description, should I provide it somehow else?

sudipghimire533 · 2022-08-15T02:15:17Z

https://github.com/sudipghimire533 @Szegoo can you both paste your Polkadot address for a tip here?

I can't edit the PR description, should I provide it somehow else?

If it need to be in PR description you can put it here and I will put in the Description

sudipghimire533 · 2022-08-15T02:31:40Z

Mine is: 1FNfKQFHFLp2Fin8Xqz1KhtZ5nVr4RqCM4Z5SGtv6QctCA9

Szegoo · 2022-08-15T05:30:45Z

Polkadot address: 126X27SbhrV19mBFawys3ovkyBS87SGfYwtwa8J2FjHrtbmA

kianenigma · 2022-08-16T07:54:13Z

/tip small

substrate-tip-bot · 2022-08-16T07:54:17Z

@kianenigma A small tip was successfully submitted for sudipghimire533 (1FNfKQFHFLp2Fin8Xqz1KhtZ5nVr4RqCM4Z5SGtv6QctCA9 on polkadot).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/treasury/tips

kianenigma · 2022-08-16T07:54:34Z

/tip small

substrate-tip-bot · 2022-08-16T07:54:37Z

@kianenigma A small tip was successfully submitted for sudipghimire533 (126X27SbhrV19mBFawys3ovkyBS87SGfYwtwa8J2FjHrtbmA on polkadot).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/treasury/tips

Szegoo · 2022-08-17T07:35:14Z

/tip small

It doesn't seem like this worked, only the first tip was submitted if I am not mistaken.

sudipghimire533 · 2022-08-17T07:43:56Z

On my address as well there we no balance whatsoever. Maybe because mine was freshly created address and it didn't even had existential deposit

Szegoo · 2022-08-17T07:50:09Z

On my address as well there we no balance whatsoever. Maybe because mine was freshly created address and it didn't even had existential deposit

Yours was submitted successfully, you can check it on https://www.dotreasury.com/dot/tips

sudipghimire533 · 2022-08-17T08:23:01Z

https://www.dotreasury.com/dot/tips/11627463_0xeed4b3487ce0c15cb0cc66f0293eece4f70001ca6f75951d9b7880d8bce3822a

You can see: value: 0

Szegoo · 2022-08-17T08:30:00Z

That is because no one has yet declared a tip value, you can check how this works in the tips pallet(https://github.com/paritytech/substrate/tree/master/frame/tips/)

sudipghimire533 · 2022-08-17T09:54:32Z

That is because no one has yet declared a tip value, you can check how this works in the tips pallet(https://github.com/paritytech/substrate/tree/master/frame/tips/)

Is that something I need to do or parity team?

Szegoo · 2022-08-17T11:03:28Z

That is because no one has yet declared a tip value, you can check how this works in the tips pallet(https://github.com/paritytech/substrate/tree/master/frame/tips/)

Is that something I need to do or parity team?

You don't have to do anything, you need to wait for the tippers to declare the tip value.

…Fix: paritytech#11092 (paritytech#11908) * make MAX_VOTERS and MAX_CANDIDATES in elections-phragmen configurable * Configure election-phragmen in node bin configuring max candidates & voters * Add document comment for added Config parameter * Incorporate suggestion * fix benchmarks * Update frame/elections-phragmen/src/lib.rs * Update frame/elections-phragmen/src/lib.rs * fix wrong values * fix typo * docs * more detailed docs * fmt * ".git/.scripts/bench-bot.sh" pallet dev pallet_elections_phragmen Co-authored-by: Szegoo <sakacszergej@gmail.com> Co-authored-by: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Co-authored-by: command-bot <>

sudipghimire533 added 2 commits July 25, 2022 21:08

make MAX_VOTERS and MAX_CANDIDATES in elections-phragmen configurable

7daf33d

Configure election-phragmen in node bin configuring max candidates & …

80fe2e0

…voters

sudipghimire533 marked this pull request as ready for review July 25, 2022 15:38

sudipghimire533 requested a review from kianenigma as a code owner July 25, 2022 15:38

Add document comment for added Config parameter

84f3ed3

kianenigma reviewed Jul 25, 2022

View reviewed changes

Incorporate suggestion

77a690f

sudipghimire533 mentioned this pull request Jul 25, 2022

Incorporate changes from substrate PR #11908 paritytech/polkadot#5815

Merged

Merge branch 'paritytech:master' into master

4bdd466

sudipghimire533 requested a review from kianenigma July 26, 2022 05:40

Merge branch 'paritytech:master' into master

a80efdf

fix benchmarks

4d31857

Szegoo reviewed Jul 28, 2022

View reviewed changes

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

1c4a7ef

truestorybaby approved these changes Aug 8, 2022

View reviewed changes

".git/.scripts/bench-bot.sh" pallet dev pallet_elections_phragmen

7faf313

ggwpez approved these changes Aug 8, 2022

View reviewed changes

Merge remote-tracking branch 'origin/master'

e1326ad

paritytech-processbot bot merged commit 34a0621 into paritytech:master Aug 14, 2022

	ensure!(actual_count <= max_candidate, Error::<T>::TooManyCandidates);
	ensure!(actual_count <= <T as Config>::MaxCandidates::get(), Error::<T>::TooManyCandidates);

	ensure!(actual_count <= <T as Config>::MaxCandidates::get(), Error::<T>::TooManyCandidates);
	ensure!(actual_count <= T::MaxCandidates::get(), Error::<T>::TooManyCandidates);

make MAX_VOTERS and MAX_CANDIDATES in elections-phragmen configurable. Fix: #11092 #11908

make MAX_VOTERS and MAX_CANDIDATES in elections-phragmen configurable. Fix: #11092 #11908

Conversation

sudipghimire533 commented Jul 25, 2022 • edited by kianenigma Loading

Update

cla-bot-2021 bot commented Jul 25, 2022

kianenigma Jul 25, 2022

Choose a reason for hiding this comment

kianenigma Jul 25, 2022

Choose a reason for hiding this comment

kianenigma commented Jul 25, 2022

sudipghimire533 commented Jul 25, 2022

Szegoo commented Jul 26, 2022

sudipghimire533 commented Jul 26, 2022 • edited Loading

kianenigma commented Jul 26, 2022

sudipghimire533 commented Jul 26, 2022

Szegoo commented Jul 26, 2022 • edited Loading

sudipghimire533 commented Jul 26, 2022

sudipghimire533 commented Jul 26, 2022

Szegoo Jul 26, 2022

Choose a reason for hiding this comment

ggwpez commented Aug 8, 2022

command-bot bot commented Aug 8, 2022 • edited Loading

command-bot bot commented Aug 8, 2022

kianenigma commented Aug 14, 2022

paritytech-processbot bot commented Aug 14, 2022

kianenigma commented Aug 14, 2022

kianenigma commented Aug 14, 2022

Szegoo commented Aug 14, 2022 • edited Loading

sudipghimire533 commented Aug 15, 2022

sudipghimire533 commented Aug 15, 2022

Szegoo commented Aug 15, 2022

kianenigma commented Aug 16, 2022

substrate-tip-bot bot commented Aug 16, 2022

kianenigma commented Aug 16, 2022

substrate-tip-bot bot commented Aug 16, 2022

Szegoo commented Aug 17, 2022

sudipghimire533 commented Aug 17, 2022

Szegoo commented Aug 17, 2022

sudipghimire533 commented Aug 17, 2022

Szegoo commented Aug 17, 2022

sudipghimire533 commented Aug 17, 2022

Szegoo commented Aug 17, 2022

sudipghimire533 commented Jul 25, 2022 •

edited by kianenigma

Loading

sudipghimire533 commented Jul 26, 2022 •

edited

Loading

Szegoo commented Jul 26, 2022 •

edited

Loading

command-bot bot commented Aug 8, 2022 •

edited

Loading

Szegoo commented Aug 14, 2022 •

edited

Loading