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

Benchmark Collective Pallet #5343

Merged
merged 11 commits into from
Mar 24, 2020
Merged

Benchmark Collective Pallet #5343

merged 11 commits into from
Mar 24, 2020

Conversation

marcio-diaz
Copy link
Contributor

@marcio-diaz marcio-diaz commented Mar 21, 2020

One of the problems I found is that the module is instantiated. So it seems I should call it from Council like this:

b"pallet-collective" | b"collective" => Council::run_benchmark(
),

but run_benchmark from the benchmarking macro is not implemented for Module<T, I>, only for Module<T>.

@marcio-diaz marcio-diaz added the A3-in_progress Pull request is in progress. No review needed at this stage. label Mar 21, 2020
@marcio-diaz marcio-diaz added this to the 2.0 milestone Mar 21, 2020
@shawntabrizi
Copy link
Member

The balances module also has instantiation. Why was there no issue there?

@marcio-diaz
Copy link
Contributor Author

marcio-diaz commented Mar 21, 2020

The balances module also has instantiation. Why was there no issue there?

I guess the code generated by the macro is different when it's actually instantiated. But yeah, good point, I'll check deeper. \cc @thiolliere

@gui1117
Copy link
Contributor

gui1117 commented Mar 23, 2020

I guess it can related to the fact that node-runtime only use balance with default instance whereas collective is used with non default instance.

@shawntabrizi
Copy link
Member

@thiolliere Do you see a cleaner way to solve this problem? Or do you agree this is a good hack for now?

Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

Make sure you populate the Proposals vec with when doing these benchmarks.

@marcio-diaz marcio-diaz changed the title Benchmark collective pallet Benchmark Collective Pallet Mar 23, 2020
@marcio-diaz marcio-diaz mentioned this pull request Mar 24, 2020
14 tasks
@marcio-diaz marcio-diaz added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Mar 24, 2020
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

lgtm

@shawntabrizi shawntabrizi added A8-mergeoncegreen and removed A0-please_review Pull request needs code review. labels Mar 24, 2020
@gavofyork gavofyork merged commit f4460ef into master Mar 24, 2020
@gavofyork gavofyork deleted the md-benchmark-collective branch March 24, 2020 17:06
rakanalh pushed a commit to rakanalh/substrate that referenced this pull request Mar 25, 2020
* Init

* Fix execute.

* Duplicate macro for instances temporarilly

* Add propose.

* Add vote, close.

* Propose from own module

* Add old members to set_members.

* Add previous proposals to propose.

* Compress a bit the macro.

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants