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

Properly benchmark set_validation_data #6772

Open
skunert opened this issue Dec 5, 2024 · 4 comments · May be fixed by #7336
Open

Properly benchmark set_validation_data #6772

skunert opened this issue Dec 5, 2024 · 4 comments · May be fixed by #7336
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I5-enhancement An additional feature request.

Comments

@skunert
Copy link
Contributor

skunert commented Dec 5, 2024

On parachains, we have the set_validation_data inherent that needs to be present in every block.
It is returning some weight at the moment, but there is no proper benchmark and I assume that the Weight return is not accurate.

Location:

polkadot-sdk/cumulus/pallets/parachain-system/src/lib.rs

Lines 568 to 571 in f4a196a

#[pallet::call_index(0)]
#[pallet::weight((0, DispatchClass::Mandatory))]
// TODO: This weight should be corrected.
pub fn set_validation_data(

@skunert skunert added I5-enhancement An additional feature request. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Dec 5, 2024
@Doordashcon
Copy link
Contributor

Let me take a crack at it @skunert

@skunert
Copy link
Contributor Author

skunert commented Dec 5, 2024

Thank you! Feel free to take a stab at it!

@skunert skunert added this to SDK Node Dec 13, 2024
@github-project-automation github-project-automation bot moved this to backlog in SDK Node Dec 13, 2024
@skunert
Copy link
Contributor Author

skunert commented Jan 8, 2025

Hey @Doordashcon , do you still plan to take a look into this?

@Doordashcon
Copy link
Contributor

yes @skunert

@gui1117 gui1117 mentioned this issue Jan 24, 2025
Merged
@Doordashcon Doordashcon linked a pull request Jan 24, 2025 that will close this issue
Draft
github-merge-queue bot pushed a commit that referenced this issue Jan 25, 2025
@gui1117
set_validation_data register weight manually, do not use refund whe…
682f8cd 
…n the pre dispatch is zero. (#7327)

Related #6772

For an extrinsic, in the post dispatch info, the actual weight is only
used to reclaim unused weight. If the actual weight is more than the pre
dispatch weight, then the extrinsic is using the minimum, e.g., the
weight used registered in pre dispatch.

In parachain-system pallet one call is `set_validation_data`. This call
is returning an actual weight, but the pre-dispatch weight is 0.

This PR fix the disregard of actual weight of `set_validation_data` by
registering it manually.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I5-enhancement An additional feature request.
Projects
SDK Node
Status: backlog
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

SVD Benchmark Doordashcon/polkadot-sdk
2 participants
@skunert @Doordashcon