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.

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

fetch more storage keys than the maximum limit set by substrate #602

Merged
merged 8 commits into from
Jul 5, 2023

Conversation

echevrier
Copy link
Contributor

@echevrier echevrier commented Jun 29, 2023

Add get_all_storage_keys_paged_up_to_count to fetch required number of keys, independantly of substrate limit:

  • call several times the substrate RPC call with the max limited number (1000), if required

The default call, get_storage_keys_paged performs according to substrate: if required number of keys> 1000, an error is thrown. The documentation warns the user.

This does not completely fix #588. To fetch all pages, you need to know how many keys are stored. Currently, count is mandatory in state_getKeysPaged and substrate doesn't support the feature for obtaining all keys.

Copy link
Contributor

@Niederb Niederb left a comment

Choose a reason for hiding this comment

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

I'm not quite sure if it makes sense to have get_all_storage_keys_paged_up_to_count as a separate function. It can do everything that get_storage_keys_paged can, so for me it should become the new get_storage_keys_paged and get_storage_keys_paged could become an internal helper method.

src/api/rpc_api/state.rs Outdated Show resolved Hide resolved
src/api/rpc_api/state.rs Outdated Show resolved Hide resolved
src/api/rpc_api/state.rs Outdated Show resolved Hide resolved
src/api/rpc_api/state.rs Outdated Show resolved Hide resolved
src/api/rpc_api/state.rs Outdated Show resolved Hide resolved
testing/examples/state_tests.rs Show resolved Hide resolved
testing/examples/state_tests.rs Outdated Show resolved Hide resolved
src/api/rpc_api/state.rs Show resolved Hide resolved
@echevrier
Copy link
Contributor Author

I'm not quite sure if it makes sense to have get_all_storage_keys_paged_up_to_count as a separate function. It can do everything that get_storage_keys_paged can, so for me it should become the new get_storage_keys_paged and get_storage_keys_paged could become an internal helper method.

We provide both methods. See: #588 (comment)

@echevrier echevrier force-pushed the ec/fetch_all_pages_fix branch from c4973d0 to 8444dfc Compare July 4, 2023 11:32
@echevrier echevrier requested review from Niederb and haerdib July 4, 2023 12:25
Copy link
Contributor

@Niederb Niederb left a comment

Choose a reason for hiding this comment

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

LGTM

src/api/rpc_api/state.rs Outdated Show resolved Hide resolved
testing/examples/state_tests.rs Show resolved Hide resolved
examples/examples/get_storage.rs Outdated Show resolved Hide resolved
@echevrier echevrier requested a review from haerdib July 5, 2023 07:04
Copy link
Contributor

@haerdib haerdib left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a lot!

/// If `start_key` is passed, return next keys in storage in lexicographic order.
///
/// `at_block`: the state is queried at this block, set to `None` to get the state from the latest known block.
// See https://github.com/paritytech/substrate/blob/9f6fecfeea15345c983629af275b1f1702a50004/client/rpc/src/state/mod.rs#L54
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@echevrier echevrier merged commit 2e092a3 into master Jul 5, 2023
@echevrier echevrier deleted the ec/fetch_all_pages_fix branch July 5, 2023 07:51
@haerdib haerdib added the F8-newfeature Introduces a new feature label Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E2-breaksapi F8-newfeature Introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fetch all pages until end for get_storage_keys_paged
3 participants