Issue 871 Wrong weight calculation for acknowledge_proposal by Marky-Shi · Pull Request #877 · litentry/heima

Marky-Shi · 2022-10-09T02:55:26Z

This is about issue 871's pr, modifying the incorrect benchmark code in the bridge pallet

codecov-commenter · 2022-10-09T03:06:47Z

Codecov Report

Merging #877 (297559a) into dev (c95ce3a) will decrease coverage by 0.07%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##              dev     #877      +/-   ##
==========================================
- Coverage   92.53%   92.45%   -0.08%     
==========================================
  Files          27       27              
  Lines        7155     7161       +6     
==========================================
  Hits         6621     6621              
- Misses        534      540       +6

Impacted Files	Coverage Δ
pallets/bridge/src/lib.rs	`84.25% <0.00%> (-2.41%)`	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Kailai-Wang

Unfortunately I still think it's a bit inaccurate:

we should use the auto-close keyword to link and close the issue
normally we just add the call's weight to the extrinsic, I don't think it's necessary to use the it as the parameter (and it will be incorrect as we can have any kind of calls), please check the link in the issue description for reference
I think you only have to benchmark the pallet-bridge again, so instead of using * when running GHA

Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com>

Kailai-Wang · 2022-10-09T09:28:25Z

pallets/bridge/src/lib.rs

-		#[pallet::weight(<T as Config>::WeightInfo::set_threshold(*threshold as u32))]
+		#[pallet::weight({
+		<T as Config>::WeightInfo::set_threshold()
+		.saturating_add(*threshold as Weight)


We don't need this saturating_add here

Kailai-Wang · 2022-10-09T09:36:18Z

pallets/bridge/src/lib.rs

+		#[pallet::weight({
+		let di = call.get_dispatch_info();
+		< T as Config >::WeightInfo::acknowledge_proposal()
+		.saturating_add(di.weight)


let's include di.class too

Kailai-Wang · 2022-10-09T09:36:29Z

pallets/bridge/src/lib.rs

+		#[pallet::weight({
+		let di = prop.get_dispatch_info();
+		< T as Config >::WeightInfo::eval_vote_state()
+		.saturating_add(di.weight)


same as above

Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com>

* Types import and web3 signature verification for IMP mock (#850) * update rust toolchain for llvm15 * add web3 verification skeleton * implement verification * fix ecdsa verify * clippy fix and comments * minor comment update * debug: relocate migration file (#844) * debug: relocate migration file * debug: change runtime methpd public to private * debug: correct chain spec * debug: correct chain spec Co-authored-by: Kai <7630809+Kailai-Wang@users.noreply.github.com> * Add tests for IMP mock (#856) * remove MrenclaveType * use oaep padding * add more testcases * add tests for verifying web3 identities * clippy fix * add cargofix * add GHA for benchmarking machine (#870) * Refactor benchmark yml (#868) * separate docker-build and benchmark * add if condition * remove stop instance for debugging * ssh debug * retry * more debugging * go back to normal routine * use reachability test for AWS instance * fix syntax * Initial VCMP impl (#857) * move tee-related primitives to primitives/ * rename and use ALICE * skeleton of vc-management pallet * add basic tests * minor comment update * add vcmp to runtime * cargo update * cargo update * use cargo pkgid for full-qualified pallet names * add benchmark at bridge and bridge transfer pallet (#854) * Fix some events triggered by token-bridge * fix some syntax * issue 794 add benchmark code * fix benchmark code * fix some syntax * about benchmark code * Chengcheng Shi * fix some syntax * add bridge-transfer weight file * add benchmark code at bridge and bridge transfer pallet * fix some syntax * add benchmark code at litentry and litmus * fix asset manager pallet benchmark code * temporarily chose to comment pallet_collator_selection out * Comment out the pallet_collator_selection module in the toml file * fix parachain-staking benchmark code * fix staking benchmark code * fix staking benchmark code * fix staking benchmark code * fix all benchmarks code * common the staking benchmark code * fix some syntax * commond the staking benchmarks * common staking benchmarks * fix syntax * Temporarily comment out the staking benchmark * Temporarily comment out the staking benchmark * Temporarily comment out the staking benchmark * separate docker-build and benchmark * add if condition * remove stop instance for debugging * ssh debug * retry * more debugging * go back to normal routine * use reachability test for AWS instance * fix syntax * [benchmarking bot] Auto commit generated weights files (#869) Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com> * Change the bridge-related weight configuration in runtime * fix bridge benchmark code * [benchmarking bot] Auto commit generated weights files (#875) Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com> Co-authored-by: Chengcheng Shi <root@LAPTOP-73QQ3GCK.localdomain> Co-authored-by: Kailai Wang <Kailai.Wang@hotmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com> Co-authored-by: Kai <7630809+Kailai-Wang@users.noreply.github.com> * Issue 871 Wrong weight calculation for acknowledge_proposal (#877) * fix bridge benchmarks code * fix some syntax * [benchmarking bot] Auto commit generated weights files (#878) Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com> * Change how weight is calculated * Change the weight * [benchmarking bot] Auto commit generated weights files (#879) Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com> Co-authored-by: Chengcheng Shi <root@LAPTOP-73QQ3GCK.localdomain> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com> * Bump serde_json from 1.0.85 to 1.0.86 (#882) Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.85 to 1.0.86. - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](serde-rs/json@v1.0.85...v1.0.86) --- updated-dependencies: - dependency-name: serde_json dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add rococo for local dev (#880) * bug fix about chain identity * add CI/scripts for rococo * add rococo ts-tests * add --allow-fail to benchmark machine Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Kai <7630809+Kailai-Wang@users.noreply.github.com> Co-authored-by: SCC <87012234+Jinsipang@users.noreply.github.com> Co-authored-by: Chengcheng Shi <root@LAPTOP-73QQ3GCK.localdomain> Co-authored-by: Kailai Wang <Kailai.Wang@hotmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Chengcheng Shi added 2 commits October 9, 2022 10:50

fix bridge benchmarks code

645c110

fix some syntax

75d7001

Marky-Shi mentioned this pull request Oct 9, 2022

Wrong weight calculation for acknowledge_proposal #871

Closed

Marky-Shi changed the title ~~Issue 871~~ Issue 871 Wrong weight calculation for acknowledge_proposal Oct 9, 2022

Marky-Shi requested a review from Kailai-Wang October 9, 2022 03:03

Marky-Shi added D2-bug Something isn't working I3-high should be completed within 5 working days labels Oct 9, 2022

Marky-Shi removed the I3-high should be completed within 5 working days label Oct 9, 2022

Kailai-Wang requested changes Oct 9, 2022

View reviewed changes

github-actions bot and others added 2 commits October 9, 2022 14:50

[benchmarking bot] Auto commit generated weights files (#878)

38cea98

Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com>

Change how weight is calculated

fae73d4

Marky-Shi requested a review from Kailai-Wang October 9, 2022 09:25

Kailai-Wang requested changes Oct 9, 2022

View reviewed changes

SCC and others added 2 commits October 9, 2022 17:54

Change the weight

cc8d2e2

[benchmarking bot] Auto commit generated weights files (#879)

297559a

Co-authored-by: Jinsipang <Jinsipang@users.noreply.github.com>

Kailai-Wang approved these changes Oct 9, 2022

View reviewed changes

Marky-Shi merged commit 93ca0f9 into dev Oct 10, 2022

Marky-Shi deleted the issue_871 branch October 10, 2022 07:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 871 Wrong weight calculation for acknowledge_proposal#877

Issue 871 Wrong weight calculation for acknowledge_proposal#877
Marky-Shi merged 6 commits intodevfrom
issue_871

Marky-Shi commented Oct 9, 2022

Uh oh!

codecov-commenter commented Oct 9, 2022 •

edited

Loading

Uh oh!

Kailai-Wang left a comment

Uh oh!

Kailai-Wang Oct 9, 2022

Uh oh!

Kailai-Wang Oct 9, 2022

Uh oh!

Kailai-Wang Oct 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Marky-Shi commented Oct 9, 2022

Uh oh!

codecov-commenter commented Oct 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Kailai-Wang left a comment

Choose a reason for hiding this comment

Uh oh!

Kailai-Wang Oct 9, 2022

Choose a reason for hiding this comment

Uh oh!

Kailai-Wang Oct 9, 2022

Choose a reason for hiding this comment

Uh oh!

Kailai-Wang Oct 9, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Oct 9, 2022 •

edited

Loading