Skip to content

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.

@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

@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

4 participants