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

Remove without_storage_info from pallets #1696

Closed
aramikm opened this issue Oct 4, 2023 · 0 comments
Closed

Remove without_storage_info from pallets #1696

aramikm opened this issue Oct 4, 2023 · 0 comments
Assignees
@aramikm
Labels
benchmark Extrensic benchmark needed change/major Major Changes in this PR tech-debt

Comments

@aramikm
Copy link
Collaborator

aramikm commented Oct 4, 2023 •
edited
Loading

Description

Based on the discussions in https://substrate.stackexchange.com/questions/3891/benchmarks-command-error/3894#3894 we should remove without_storage_info usages so that we can get accurate PoV benchmarks

Currently its been used in Messages and Schema pallets

Acceptence Criterea

  • Remove without_storage_info from Messages and Schema pallets and fix all compile issues
  • Do migrations from unbounded to bounded types for these pallets
  • Rerun benchmarks to include PoV size

Additional details

Examples for migration and other changes from substrate:
paritytech/polkadot-sdk#323
@aramikm aramikm added change/major Major Changes in this PR tech-debt labels Oct 4, 2023
@aramikm aramikm changed the title Copy of Upgrade to Polkadot Release v1.0.0 Remove without_storage_info from pallets Oct 4, 2023
@aramikm aramikm added the benchmark Extrensic benchmark needed label Oct 4, 2023
@aramikm aramikm self-assigned this Oct 5, 2023
@aramikm aramikm mentioned this issue Oct 6, 2023
Merged
2 tasks
aramikm added a commit that referenced this issue Oct 23, 2023
@aramikm
Removed without_storage_info from messages and schema pallets (#1702)
4736b69 
# Goal
The goal of this PR is to remove `without_storage_info` which is
necessary to be able to calculate max encoded length for PoV.

Closes <!-- issue # -->
#1696 

# Changes
- Since we can not submit a `MaxEncodedLen` PoV for messages due to
oversized PoV we would need to use `measured` until we reworked the
messages pallet.
- Reduced the `MessagesMaxPayloadSizeBytes` from 50K to 3K to reduce the
chance of Oversized PoV even for measured mode until the rework of
messages pallet.
- Reduced the `MessagesMaxPerBlock` from 7000 to 200 to reduce the
chance of Oversized PoV even for measured mode until the rework of
messages pallet.

# Checklist
- [X] Chain spec updated
- [X] Weights updated

# Notes
I checked all the submitted messages on rococo and main-net and there
are no messages that violate these restrctions
Here is list of all messages on rococo and there are none on main-net.
The biggest one has less than 700 bytes


[messages.txt](https://github.com/LibertyDSNP/frequency/files/13033855/messages.txt)

# Merge order
This most probably will get merged after node upgrade PR unless
something changes
@aramikm aramikm closed this as completed Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aramikm aramikm

Labels
benchmark Extrensic benchmark needed change/major Major Changes in this PR tech-debt
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mattheworris @enddynayn @aramikm