-
Notifications
You must be signed in to change notification settings - Fork 19
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.
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
schemas: PoV compatible changes #1743
Conversation
⏳ Running benchmarks and calculating weights. DO NOT MERGE! A new commit will be added upon completion... |
Codecov Report
@@ Coverage Diff @@
## main #1743 +/- ##
==========================================
- Coverage 88.00% 87.60% -0.41%
==========================================
Files 51 52 +1
Lines 4270 4339 +69
==========================================
+ Hits 3758 3801 +43
- Misses 512 538 +26
📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
✅ Finished running benchmarks. Updated weights have been committed to this PR branch in commit afe3711. |
61723a0
to
0629206
Compare
⏳ Running benchmarks and calculating weights. DO NOT MERGE! A new commit will be added upon completion... |
✅ Finished running benchmarks. Updated weights have been committed to this PR branch in commit 344b1a7. |
344b1a7
to
965da26
Compare
Looks pretty good. Perhaps add/update e2e tests for all pallets to use the new RPC? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good now
⏳ Running benchmarks and calculating weights. DO NOT MERGE! A new commit will be added upon completion... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 fantastic!
✅ Finished running benchmarks. Updated weights have been committed to this PR branch in commit 8df2685. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid.
[Read through code, ran tests,
caveat: I don't have much experience with storage migration, but nothing smells off]
This PoV improvement would not affect extrinsic weights in this pallet, but it would directly affect any | ||
pallet that is dependent on **Schemas** pallet. Some of these pallets are **Messages** and | ||
**Stateful-Storage**. After these changes we are expecting see to see around 30-60KiB decrease in PoV | ||
for `MaxEncodedLen` mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Goal
The goal of this PR is to split schemas and the model into 2 separate storages so that we can limit the size of PoV being accessed from other pallets.
Closes #1742
Checklist