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

state_getSize RPC for storage maps #6847

Merged
19 commits merged into from
Aug 11, 2020
Merged

state_getSize RPC for storage maps #6847

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
261b95c
Fancy compact encode/decode impl for compact solution
kianenigma Jul 23, 2020
973bd25
Merge branch 'master' of github.com:paritytech/substrate into kiz-cod…
kianenigma Aug 3, 2020
0ff952c
Make it optional
kianenigma Aug 3, 2020
8c7b07a
Remove extra file
kianenigma Aug 3, 2020
41ad894
Update primitives/npos-elections/compact/src/lib.rs
kianenigma Aug 6, 2020
03ce657
Final fixes.
kianenigma Aug 6, 2020
82641f9
Merge branch 'kiz-codec-for-compact' of github.com:paritytech/substra…
kianenigma Aug 6, 2020
1524c5b
Merge branch 'master' of github.com:paritytech/substrate into kiz-cod…
kianenigma Aug 6, 2020
6fcb09e
getSize rpc should work for maps as well
kianenigma Aug 7, 2020
ba19fd3
Fix future types
kianenigma Aug 10, 2020
ba3eacd
Remove minimum_validator_count stale const
kianenigma Aug 10, 2020
b889970
Update client/rpc/src/state/mod.rs
kianenigma Aug 11, 2020
66586a4
"Optimize" `storage_size`
bkchr Aug 11, 2020
754098a
Remove unused import
bkchr Aug 11, 2020
85532e1
Merge branch 'kiz-rpc-for-map-len' of github.com:paritytech/substrate…
kianenigma Aug 11, 2020
11ff7ee
Update doc
kianenigma Aug 11, 2020
fcc7f0c
Merge branch 'master' of github.com:paritytech/substrate into kiz-cod…
kianenigma Aug 11, 2020
41663f4
Merge branch 'kiz-rpc-for-map-len' of github.com:paritytech/substrate…
kianenigma Aug 11, 2020
e12e6c9
Merge branch 'master' of github.com:paritytech/substrate into kiz-rpc…
kianenigma Aug 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions client/rpc/src/state/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,26 @@ pub trait StateBackend<Block: BlockT, Client>: Send + Sync + 'static
block: Option<Block::Hash>,
key: StorageKey,
) -> FutureResult<Option<u64>> {
Box::new(self.storage(block, key)
.map(|x| x.map(|x| x.0.len() as u64)))
use rpc::futures::{future::Either, done};

let value_len = self.storage(block, key.clone()).map(|x| x.map(|x| x.0.len() as u64));
let pairs_len = self.storage_pairs(block, key.clone())
Copy link
Contributor

@tomusdrw tomusdrw Aug 7, 2020

Choose a reason for hiding this comment

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

Shouldn't that query be done conditionally only if value_len results into None?

Copy link
Contributor Author

@kianenigma kianenigma Aug 7, 2020

Choose a reason for hiding this comment

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

yeah that makes more sense and I initially started with that, but failed to get it to work. Ideally I want: only if the first one is Ok(None), then try the second one.

But I am not sure if this is even possible? I am but new to futures.

Copy link
Contributor

@tomusdrw tomusdrw Aug 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Depends on the futures version we have here, but it seems it's 0.1 and I think it should be something like this:

value_len.and_then(|result| match result {
  None => Either::A(self.storage_pairs(...)...),
  Some(v) => Either:B(future::done(Ok(Some(v))),
})

Copy link
Contributor Author

@kianenigma kianenigma Aug 7, 2020

Choose a reason for hiding this comment

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

I'm pretty sure I tried something like that and failed once, but let me try again and get back.

Copy link
Contributor Author

@kianenigma kianenigma Aug 10, 2020

Choose a reason for hiding this comment

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

Okay so this was the first time I was working with futures and the main point of confusion was the versioning going on (which makes me appreciate more and more the standardised async/await). I wasn't aware that the rpc us using and re-exporting 0.1 and the current crate is using 0.3, and I was getting strange errors all over the place.

Copy link
Member

@bkchr bkchr Aug 10, 2020

Choose a reason for hiding this comment

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

The storage_pairs function will return the results directly. You need to do this differently here.

Copy link
Contributor Author

@kianenigma kianenigma Aug 11, 2020

Choose a reason for hiding this comment

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

can you explain more? I don't see the difference in what you suggested.

Copy link
Member

@bkchr bkchr Aug 11, 2020

Choose a reason for hiding this comment

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

When you call a function that returns a future, you can also directly return the result. This means the first time the future is polled the result is ready. However this also means that the operation is always executed, in this case the scanning of the storage. I propose that you put the scanning call into a closure and only call this closure if the first storage call returned a none. This means you will only scan the storage when the value does not exists vs query it always.

Copy link
Contributor Author

@kianenigma kianenigma Aug 11, 2020

Choose a reason for hiding this comment

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

I exactly wanted to do as you explain: only query the second if the first one is None. This is how I thought: calling self.storage_pairs and storing it in pairs_len executes nothing, because it is not yet polled. I thought that also afterwards when I pass it to and_then, it is would be only polled in the None arm, but apparently not.

.map(|kv| {
let item_sum = kv.iter().map(|(_, v)| v.0.len() as u64).sum::<u64>();
if item_sum > 0 {
Some(item_sum)
} else {
None
}
});

// if value size is there, use that, else execute another future that looks for map len.
let value_or_map = value_len.and_then(|result| match result {
Some(size) => Either::A(done(Ok(Some(size)))),
None => Either::B(pairs_len),
});

Box::new(value_or_map)
}

/// Returns the runtime metadata as an opaque blob.
Expand Down
8 changes: 7 additions & 1 deletion client/rpc/src/state/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ fn should_return_storage() {
let client = TestClientBuilder::new()
.add_extra_storage(KEY.to_vec(), VALUE.to_vec())
.add_extra_child_storage(&child_info, KEY.to_vec(), CHILD_VALUE.to_vec())
// similar to a map with two keys
.add_extra_storage(b":map:acc1".to_vec(), vec![1, 2])
.add_extra_storage(b":map:acc2".to_vec(), vec![1, 2, 3])
.build();
let genesis_hash = client.genesis_hash();
let (client, child) = new_full(Arc::new(client), SubscriptionManager::new(Arc::new(TaskExecutor)));
Expand All @@ -72,6 +75,10 @@ fn should_return_storage() {
client.storage_size(key.clone(), None).wait().unwrap().unwrap() as usize,
VALUE.len(),
);
assert_eq!(
client.storage_size(StorageKey(b":map".to_vec()), None).wait().unwrap().unwrap() as usize,
2 + 3,
);
assert_eq!(
executor::block_on(
child.storage(prefixed_storage_key(), key, Some(genesis_hash).into())
Expand All @@ -80,7 +87,6 @@ fn should_return_storage() {
).unwrap().unwrap() as usize,
CHILD_VALUE.len(),
);

}

#[test]
Expand Down