This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
[WIP][Society] Adding benchmarking to the v2. #11776
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kianenigma
reviewed
Jul 10, 2022
| pub const MaxStrikes: u32 = 10; | ||
| pub const RotationPeriod: BlockNumber = 80 * HOURS; | ||
| pub const GraceStrikes: u32 = 10; | ||
| pub const SocietyVotingPeriod: BlockNumber = 80 * HOURS; |
There was a problem hiding this comment.
Easier to express this with DAYS?
There was a problem hiding this comment.
I just kept how RotationPeriod is on master.
A heads up that 80 HOURS in DAYS is 3.333.... To use an integer number of DAYS we'd have to decrease it to 72 HOURS or increase it to 96 HOURS.
(maybe I'm missing something here =/)
There was a problem hiding this comment.
I explicitly made it this way to avoid giving one global region preference in daylight times.
kianenigma
reviewed
Jul 10, 2022
kianenigma
reviewed
Jul 10, 2022
| let caller: T::AccountId = whitelisted_caller(); | ||
| let deposit: BalanceOf<T, I> = T::Currency::minimum_balance(); | ||
| T::Currency::make_free_balance_be(&caller, BalanceOf::<T, I>::max_value()); | ||
| }: _(RawOrigin::Signed(caller.clone()), 10u32.into()) |
There was a problem hiding this comment.
You should bit with a multiple of minimum_balance. This value is smaller than the existential deposit in all real chains and might have unforeseen consequences.
kianenigma
reviewed
Jul 10, 2022
| // Storage: Society MemberByIndex (r:0 w:1) | ||
| // Storage: Society Rules (r:0 w:1) | ||
| // Storage: Society Members (r:0 w:1) | ||
| fn dissolve() -> Weight { |
There was a problem hiding this comment.
a bit too good to be true. The dissolve benchmark should assume "worse case", which means there should be maximum number of votes and other data structures also around to clean.
There was a problem hiding this comment.
It makes sense, will do that.
kianenigma
reviewed
Jul 10, 2022
|
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
A5-stale
stale
bot
removed
the
A5-stale
|
This one is ready for review. |
|
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
A5-stale
stale
bot
removed
the
A5-stale
gavofyork
added a commit
that referenced
this pull request
Jun 18, 2023
* New Society * More logic drafting * More work * Building * Some tests * Fixes * Improvements to the voting process * More tests * Test number 20 * Tests * 30 tests * Another test] * All tests enabled * Minor stuff * generate_storage_alias: Rewrite as proc macro attribute This rewrites the `generate_storage_alias!` declarative macro as proc-macro attribute. While doing this the name is changed to `storage_alias`. The prefix can now also be the name of a pallet. This makes storage aliases work in migrations for all kind of chains and not just for the ones that use predefined prefixes. * Maintenance operations don't pay fee * Fix compilation and FMT * Moare fixes * Migrations * Fix tests and add migration testing * Introduce lazy-cleanup and avoid unbounded prefix removal * Fixes * Fixes * [WIP][Society] Adding benchmarking to the v2. (#11776) * [Society] Adding benchmarking to the v2. * [Society] Code review. * [Society] Better code. * Using clear() + clear_prefix() and adding more tests. * Benchmarking again... * Fix Cargo * Fixes * Fixes * Spelling * Fix benchmarks * Another fix * Remove println --------- Co-authored-by: Bastian Köcher <info@kchr.de> Co-authored-by: Artur Gontijo <arturgontijo@users.noreply.github.com>
rossbulat
pushed a commit
that referenced
this pull request
Jun 19, 2023
* New Society * More logic drafting * More work * Building * Some tests * Fixes * Improvements to the voting process * More tests * Test number 20 * Tests * 30 tests * Another test] * All tests enabled * Minor stuff * generate_storage_alias: Rewrite as proc macro attribute This rewrites the `generate_storage_alias!` declarative macro as proc-macro attribute. While doing this the name is changed to `storage_alias`. The prefix can now also be the name of a pallet. This makes storage aliases work in migrations for all kind of chains and not just for the ones that use predefined prefixes. * Maintenance operations don't pay fee * Fix compilation and FMT * Moare fixes * Migrations * Fix tests and add migration testing * Introduce lazy-cleanup and avoid unbounded prefix removal * Fixes * Fixes * [WIP][Society] Adding benchmarking to the v2. (#11776) * [Society] Adding benchmarking to the v2. * [Society] Code review. * [Society] Better code. * Using clear() + clear_prefix() and adding more tests. * Benchmarking again... * Fix Cargo * Fixes * Fixes * Spelling * Fix benchmarks * Another fix * Remove println --------- Co-authored-by: Bastian Köcher <info@kchr.de> Co-authored-by: Artur Gontijo <arturgontijo@users.noreply.github.com>
nathanwhit
pushed a commit
to nathanwhit/substrate
that referenced
this pull request
Jul 19, 2023
* New Society * More logic drafting * More work * Building * Some tests * Fixes * Improvements to the voting process * More tests * Test number 20 * Tests * 30 tests * Another test] * All tests enabled * Minor stuff * generate_storage_alias: Rewrite as proc macro attribute This rewrites the `generate_storage_alias!` declarative macro as proc-macro attribute. While doing this the name is changed to `storage_alias`. The prefix can now also be the name of a pallet. This makes storage aliases work in migrations for all kind of chains and not just for the ones that use predefined prefixes. * Maintenance operations don't pay fee * Fix compilation and FMT * Moare fixes * Migrations * Fix tests and add migration testing * Introduce lazy-cleanup and avoid unbounded prefix removal * Fixes * Fixes * [WIP][Society] Adding benchmarking to the v2. (paritytech#11776) * [Society] Adding benchmarking to the v2. * [Society] Code review. * [Society] Better code. * Using clear() + clear_prefix() and adding more tests. * Benchmarking again... * Fix Cargo * Fixes * Fixes * Spelling * Fix benchmarks * Another fix * Remove println --------- Co-authored-by: Bastian Köcher <info@kchr.de> Co-authored-by: Artur Gontijo <arturgontijo@users.noreply.github.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PRs tracks #11324 and adds
benchmarking.rsto it.I had to touch on some other files in order to make it compile.