Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run benchmarks and update weights for pallet_collective once benchmarks are fixed for external pallets #706

Closed
wilwade opened this issue Nov 17, 2022 · 7 comments · Fixed by #923 or #936
Assignees

Comments

@wilwade
Copy link
Collaborator

wilwade commented Nov 17, 2022

The following Benchmarks in pallet versions 0.9.29 are failing:

  1. pallet_collective - A PR is merged, don't know what branch it will go in

We need to keep an eye out for these fixes, run the benchmarks for the pallets and then use our weights in the relevant next frequency update.

@wilwade wilwade added this to the Upgrade to v0.9.32 milestone Nov 17, 2022
@enddynayn
Copy link
Collaborator

Do we need to wait for a new release from substrate?

@wilwade wilwade added the blocked Blocked by another issue label Nov 17, 2022
@wilwade
Copy link
Collaborator Author

wilwade commented Nov 17, 2022

@enddynayn Yes. I added it to the v0.9.32 milestone as that is when I expect it to be available, but I missed the blocked label.

@saraswatpuneet saraswatpuneet self-assigned this Dec 7, 2022
@saraswatpuneet saraswatpuneet removed the blocked Blocked by another issue label Dec 7, 2022
@p5150j p5150j removed this from the Upgrade to v0.9.32 milestone Jan 4, 2023
@rlaferla
Copy link
Contributor

rlaferla commented Jan 17, 2023

From what I see in the run_all_benchmarks.sh script, the pallet_collective and pallet_collator_selection are not yet included in the list of EXTERNAL_PALLETS because at least one was failing. Therefore, they needed to be added, tested and fixed (if necessary). Is this correct?

@rlaferla
Copy link
Contributor

Benchmarking for both pallets appears to be still failing. I am researching what (if anything) we can do about it.

pallet_collective (from Substrate)

2023-01-17 13:19:42 New members count (10) exceeds maximum amount of members expected (3).    
2023-01-17 13:19:42 New members count (10) exceeds maximum amount of members expected (3).    
Starting benchmark: pallet_collective::disapprove_proposal
Starting benchmark: pallet_collective::set_members
Running Benchmark: pallet_collective.set_members(3 args) 63/150 1/1
Starting benchmark: pallet_collective::execute
Starting benchmark: pallet_collective::propose_execute
Starting benchmark: pallet_collective::propose_proposed
Running Benchmark: pallet_collective.propose_proposed(3 args) 10/150 1/1
Running Benchmark: pallet_collective.propose_proposed(3 args) 126/150 1/1
Starting benchmark: pallet_collective::vote
Error: Input("`low` cannot be higher than `high`")
❌ 💔

pallet_collator_selection (from Cumulus)

2023-01-17 13:40:16 max > T::MaxCandidates; you might need to run benchmarks again    
2023-01-17 13:40:16 panicked at 'called `Result::unwrap()` on an `Err` value: <wasm:stripped>', .../cumulus-59522f43471fa161/6abd385/pallets/collator-selection/src/benchmarking.rs:103:86    
Error: Input("Error executing and verifying runtime benchmark: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed\nWASM backtrace:\n\n

@rlaferla
Copy link
Contributor

I increased the maximum number of tech committee members from 3 to 10 (same as max council members) which fixed the benchmarking.

@rlaferla
Copy link
Contributor

As this issue is for the collective pallet, I will not re-enable the collator selection pallet benchmarks. That can be a separate issue if desired.

rlaferla added a commit that referenced this issue Jan 18, 2023
…hmarks are fixed for external pallets (#923)

# Goal
The goal of this PR is to enable the benchmarks for the collective
pallet.

Closes #706

Co-authored-by: Jenkins <[email protected]>
@rlaferla
Copy link
Contributor

The PR was missing a couple of items which should now be fixed in the next PR.

@rlaferla rlaferla reopened this Jan 19, 2023
JoeCap08055 pushed a commit that referenced this issue Jan 19, 2023
…hmarks are fixed for external pallets (#923)

The goal of this PR is to enable the benchmarks for the collective
pallet.

Closes #706

Co-authored-by: Jenkins <[email protected]>
JoeCap08055 pushed a commit that referenced this issue Jan 19, 2023
…hmarks are fixed for external pallets (#923)

The goal of this PR is to enable the benchmarks for the collective
pallet.

Closes #706

Co-authored-by: Jenkins <[email protected]>
rlaferla added a commit that referenced this issue Jan 23, 2023
…hmarks are fixed for external pallets (#936)

# Goal
The goal of this PR is enable benchmarks for the collective pallet

Closes #706

Co-authored-by: Jenkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment