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

chainHead_storage: Iterate over keys #14628

Merged
merged 5 commits into from
Jul 25, 2023
Merged

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented Jul 24, 2023

This PR adds support to iterate over (key, value) and (key, hash) pairs.

At the moment, substrate will iterate over at most 10 keys producing at most 10 pairs.
This PR lays the ground for iteration and support for pagination will be added in:

A new tests checks

  • the API can receive multiple items as argument
  • iteration works as expected over (key, value) and (key, hash)

Closes #14548.

// @paritytech/subxt-team

@lexnv lexnv added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit B1-note_worthy Changes should be noted in the release notes T2-API This PR/Issue is related to APIs. labels Jul 24, 2023
@lexnv lexnv requested review from jsdw and skunert July 24, 2023 20:29
@lexnv lexnv self-assigned this Jul 24, 2023
Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

result: StorageResultType::Value(hex_string(&storage_data.0)),
})
})
QueryResult::Ok(opt.map(|storage_data| StorageResult::<String> {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I personally would just use Ok instead of QueryResult::Ok, but I guess that is just personal preference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It might be that QueryResult::Ok is used here to convey the type information that comes along with QueryResult :))

Copy link
Contributor

@jsdw jsdw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lookk clean to me; good work!

Random question; is there a reason for picking 10 as the number of storage items to send back before needing a "continue"?

@lexnv
Copy link
Contributor Author

lexnv commented Jul 25, 2023

Thanks a lot for the review!

is there a reason for picking 10 as the number of storage items to send back before needing a "continue"?

I thought of it as a hard limit to ensure we don't iterate over the whole DB until we implement the continue and reachedLimit from the new spec. As for the number, that's randomly small :D

@lexnv
Copy link
Contributor Author

lexnv commented Jul 25, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

@lexnv
Copy link
Contributor Author

lexnv commented Jul 25, 2023

bot rebase

@paritytech-processbot
Copy link

Branch is already up-to-date

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T2-API This PR/Issue is related to APIs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RPC-Spec-V2] Storage: Add iteration support for storage queries
4 participants