Membership pallet benchmark

changed milestone to %runtime-500

added RN-runtime label

mentioned in merge request !138 (merged)

added 14 commits

• 483ed3da...0bfd3e4d - 13 commits from branch nodes/rust:master
• a5fc0514 - wip add membership benchmark

Compare with previous version

The membership pallet has two instances:

• Instance1 with type CheckCallAllowed = Wot; and type MetaData = (); for Membership
• Instance2 with type CheckCallAllowed = SmithsSubWot; and type MetaData = SmithsMembershipMetaData<opaque::SessionKeysWrapper>; for SmithsMembership

Several problems arise when trying to write the benchmark:

1. request_membership is not benchmarkable for Instance1 because https://git.duniter.org/nodes/rust/duniter-v2s/-/blob/master/pallets/duniter-wot/src/lib.rs#L245 T::IsSubWot::get() is always False so there is no way to run this extrinsic with the runtime configured as such, hence to perform a benchmark.
2. To benchmark the pallet, we need T::MetaData::default() (or at least a mean to generate some T::MetaData for the benchmark) to be able to call the extrinsic using this type. In the case of the Instance2, it must be implemented https://git.duniter.org/nodes/rust/duniter-v2s/-/blob/master/runtime/common/src/entities.rs#L109 for SmithsMembershipMetaData that seems quite complicated, moreover since they are hardly used in the pallet.

I find 1 particularly confusing regarding the documentation https://git.duniter.org/nodes/rust/duniter-v2s/-/blob/master/docs/api/runtime-calls.md that states:

	Main Wot	Sub Wot
request_membership	Automatic	Available
claim_membership	Automatic	Available
revoke_membership	Automatic	Available

But request_membership is the only extrinsic that explicitly checks the type of Wot with T::IsSubWot::get(); the others can be run successfully even with Instance1 where T::IsSubWot::get() is False.

Either we can (from better to worse):

• Fix 1 and fix 2 to have a benchmark that can be run on the two instances to produce two weights.rs files.
• If all the extrinsics except force_request_membership are not intended to be used in Main Wot, i.e., with the Instance1, we can fix 2 and use one unique weights.rs for the two instances that will be generated using the Instance2.
• Fix 1 and use one unique weights.rs for the two instances that will be generated using the Instance1 avoiding to touch 2.

@HugoTrentesaux @tuxmain

I tried a branch hugo-membership-benchmark where I declare the call manually and ignore its output.

It crashes the runtime, and we can ignore this with --no-verify:

./target/release/duniter benchmark pallet --chain=dev --steps=3 --repeat=2 --pallet=common_runtime::membership --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=. --no-verify

Still digging. Thanks for https://substrate.stackexchange.com/questions/8215/benchmark-failing-extrinsics

---

[edit] I now only check the presence of the event when the call is allowed.

---

claim_membership / automatic

That's what I understood from the BaseCallFilter but I have to check that.

added 1 commit

• acd5f55c - feat: add pallet membership benchmark

Compare with previous version

resolved all threads

added 2 commits

• 4d3a7158 - feat(pallet_membership): add weights info
• d08cb1d3 - feat(runtimes): use our benchmarks for pallet membership

Compare with previous version

added 3 commits

• ca6f0867 - feat: add pallet membership benchmark
• 4a415952 - feat(pallet_membership): add weights info
• e663d431 - feat(runtimes): use our benchmarks for pallet membership

Compare with previous version

added 3 commits

• 50233873 - feat: add pallet membership benchmark
• c791f42b - feat(pallet_membership): add weights info
• 3cab44eb - feat(runtimes): use our benchmarks for pallet membership

Compare with previous version

approved this merge request

added 1 commit

• b72f2423 - add comment warning about hardcoded smith metadata

Compare with previous version

Looks good to me. The failing cucumber test is only a timeout, it succeeds with a larger timeout:

DUNITER_END2END_TESTS_SPAWN_NODE_TIMEOUT=300 cargo cucumber -i account_creation*

The only annoying thing in my opinion is the hardcoding of Dave key in metadata. I added a comment about this and hope we can remove it soon.

marked this merge request as ready

added 20 commits

• b72f2423...70ac32cd - 15 commits from branch nodes/rust:master
• 6a69d631 - feat: add pallet membership benchmark
• e3864552 - feat(pallet_membership): add weights info
• 14030b41 - feat(runtimes): use our benchmarks for pallet membership
• e6293136 - add comment warning about hardcoded smith metadata
• 46ec59a4 - fix after rebase

Compare with previous version

I rebased on master and fixed conflicts introduced by !148 (merged). This should be ok for merge.

enabled an automatic merge when the pipeline for 46ec59a4 succeeds

mentioned in commit 10170f2a