Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[FRAME] Paginated Storage Primitives #14747

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

[FRAME] Paginated Storage Primitives #14747

wants to merge 6 commits into from

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Aug 10, 2023

The paged-list pallet provides a quite narrow solution to the problem of paginated data structures.
This MR implements StorageList and StoragePagedNMap. Only the latter contains code, the former is an adapter type of it. Alike it should be possible to use it as StoragePagedDoubleMap and StoragePagedMap. All variants are counted, so the Counted prefix is omitted.

Metadata

It currently uses fudged metadata by setting the metadata of a StoragePagedList<Value> to the metadata of a StorageMap<u32, Vec<Value>>. This is correct on the storage level, since it maps the page-index (u32) to a page (Vec<Value>), but when reading the page, it is unclear to the reader which offset to use within the page.
FRAME metadata V16 would be needed to fix this properly so that the reader is aware of the metadata field in storage.

Changes

  • The paged-list pallet now uses a proper heap and supports len().
  • TODO: explain

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added A3-in_progress Pull request is in progress. No review needed at this stage. C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. labels Aug 10, 2023
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3359288

@jasl
Copy link
Contributor

jasl commented Aug 10, 2023

All variants are counted

I have yet to learn the source code, but I am curious about the counter of DoubleMap or NMap.
There's a common scenario, for example, DoubleMap<CollectionId, ItemId>, I would like to list items in a collection, and it's useful to have the counter of how many items are in a collection.

Does this is the scope of the PR aim? Does the counter support multi-level?

Does the pagination ensure the results are ordered for map structures?

@ggwpez
Copy link
Member Author

ggwpez commented Aug 10, 2023

@jasl after reading your comment irealize that the name of the storage primitives is badly chosen. Its not really a "Map" in the common sense; its rather a List structure that can be accessed through a map key.
So for example a StoargePagedNMap has the functions append_one(key, value), iter(key) and len(key) (and some other).
All operations always need the full key. This means that these prefix operations like on normal NMaps are not possible... It would therefore not support sub-level counting operations.
The full key is AFAIK always needed, since the metadata needs to be updated.

@jasl
Copy link
Contributor

jasl commented Aug 10, 2023

Thank you for clarifying!

So this feature is for optimizing iterating a full list of storage? like when doing migration?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A3-in_progress Pull request is in progress. No review needed at this stage. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants