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

[sudipghimire533]

Closes: #11902 Polkadot companion: https://github.com/paritytech/polkadot/pull/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 #11790

[cla-bot-2021[bot]]

User @sudipghimire533, please sign the CLA here.

[kianenigma]

Looks mostly good, need to make companions.

[sudipghimire533]

Polkadot companion: https://github.com/paritytech/polkadot/pull/5815

[Szegoo]

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

[sudipghimire533]

@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]

@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: https://github.com/paritytech/substrate/blob/54d6f2cbae12fa314f2bcaf6d57fba585162438f/frame/staking/src/benchmarking.rs#L815

[sudipghimire533]

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]

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]

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]

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    

[Szegoo]

@sudipghimire533 The errors are now fixed :)

[sudipghimire533]

@sudipghimire533 The errors are now fixed :)

Indeed, a smart wizard named Szegoo same and fixed everything.. Thanks a lot for helping out mate :)

[Szegoo]

@sudipghimire533 The errors are now fixed :)

Indeed, a smart wizard named Szegoo same and fixed everything.. Thanks a lot for helping out mate :)

NP, note that you should put the polkadot companion in the PR description so that the companion CI checks pass. It should look like this: polkadot companion: https://github.com/paritytech/polkadot/pull/5815

[sudipghimire533]

@sudipghimire533 The errors are now fixed :)

Indeed, a smart wizard named Szegoo same and fixed everything.. Thanks a lot for helping out mate :)

NP, note that you should put the polkadot companion in the PR description so that the companion CI checks pass. It should look like this: polkadot companion: paritytech/polkadot#5815

https://github.com/paritytech/polkadot/pull/5815

This looks ok?

[Szegoo]

@sudipghimire533 The errors are now fixed :)

Indeed, a smart wizard named Szegoo same and fixed everything.. Thanks a lot for helping out mate :)

NP, note that you should put the polkadot companion in the PR description so that the companion CI checks pass. It should look like this: polkadot companion: paritytech/polkadot#5815

paritytech/polkadot#5815

This looks ok?

You should put it inside the PR description, but don't put it in a list as you did. Here is an example: # 11831

[Szegoo]

@sudipghimire533 Are you working on this, or may I finish this?

[sudipghimire533]

@sudipghimire533 Are you working on this, or may I finish this?

I seems quite busy on my office work. You can take over. Thanks @Szegoo

[Szegoo]

Needs to be able to set smaller values for testing, otherwise running tests and benchmarks of this pallet would take ages.

You can try yourself by doing cargo test -p pallet-elections-phragmen --features runtime-benchmarks

@kianenigma I don't think that is right. Please correct me if I am wrong, but in the code, the TEST_MAX_VOTERS and TEST_MAX_CANDIDATES constants are removed, so for the tests and benchmarks the values are used from the Test implementation of Config. I have put the values in the Test configuration to be the same as the deleted test constants. So the benchmarks don't take a long time to execute. The non-test values are specified in the runtime config and don't impact the benchmarks.

[Szegoo]

@kianenigma I have updated the docs.

[kianenigma]

companion needs an update too.

[sudipghimire533]

companion needs an update too.

Can you mention what needed to me updated

[ggwpez]

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

[command-bot[bot]]

@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[bot]]

@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]

bot rebase

[paritytech-processbot[bot]]

Rebased

[kianenigma]

bot merge

[kianenigma]

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

[Szegoo]

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]

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