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.

Sign up for GitHub

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

Jump to bottom

Multisig pallet has outdated WeightInfo trait #7204

Closed
2 tasks done
Moliholy opened this issue Jan 16, 2025 · 1 comment · Fixed by #6789
Closed
2 tasks done

Multisig pallet has outdated WeightInfo trait #7204

Moliholy opened this issue Jan 16, 2025 · 1 comment · Fixed by #6789
Assignees
@mordamax
Labels
I2-bug The node fails to follow expected behavior. I10-unconfirmed Issue might be valid, but it's not yet known.

Comments

@Moliholy
Copy link
Contributor

Moliholy commented Jan 16, 2025 •
edited
Loading

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Description of bug

Currently multisig pallet's benchmarks include a parameter z that is however not reflected in the corresponding WeightInfo trait.

This can be solved by simply running the benchmarks, which will generate weights that depend on such parameter, and update the corresponding usages.

The current problem right now is that when parachains perform the corresponding benchmarks, the resulting WeightInfo is not the expected one and is definitely not compatible, which forces them to manually tweak the benchmarks to meet the outdated WeightInfo interface.

I'd happily fix the issue myself and submit the corresponding PR, but I guess there's a well-known infrastructure to run the benchmarks from a given reference hardware.

Steps to reproduce

Simply run the benchmarks in a parachain and the resulting WeightInfo trait won't be compatible with the multibatching pallet.
@Moliholy Moliholy added I10-unconfirmed Issue might be valid, but it's not yet known. I2-bug The node fails to follow expected behavior. labels Jan 16, 2025
@bkchr
Copy link
Member

bkchr commented Jan 16, 2025

@mordamax while we are waiting for your auto-weight update, could you do one pr that updates all the pallets?

bgallois added a commit to duniter/duniter-polkadot-sdk that referenced this issue Jan 21, 2025
@bgallois
hotfix paritytech#7204
20b4cc3 
Since we do not use Substrate-generated weights, only fix the library and trait definitions so we can generate our own weights.
@mordamax mordamax self-assigned this Jan 23, 2025
@mordamax mordamax mentioned this issue Jan 23, 2025
Merged
github-merge-queue bot pushed a commit that referenced this issue Jan 24, 2025
@mordamax @github-actions @ggwpez
bench all weekly - and fix for pallet_multisig lib (#6789)
f845a9f 
Closes #6196 
Closes #7204

Example of PR: #6816

Every sunday 01:00 AM it's going to start to benchmark (with /cmd bench)
all runtimes and all pallets
Then diff total will be pushed to a branch and PR open,. I assume
review-bot is going assign required reviewers per changed files

I afraid each weeks will be too much to review & merge, but we can
adjust later

Bonus: fix for pallet_multisig lib and
substrate/.maintain/frame-weight-template.hbs , which didn't let to
compile new weights

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
@mordamax mordamax mentioned this issue Jan 24, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mordamax mordamax

Labels
I2-bug The node fails to follow expected behavior. I10-unconfirmed Issue might be valid, but it's not yet known.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

bench all weekly - and fix for pallet_multisig lib paritytech/polkadot-sdk
3 participants
@mordamax @Moliholy @bkchr