state_getSize RPC for storage maps #6847
Conversation
kianenigma
added
A0-please_review
tomusdrw
left a comment
tomusdrw
left a comment
There was a problem hiding this comment.
lgtm if doing both queries in parallel is expected
client/rpc/src/state/mod.rs
Outdated
| Box::new(self.storage(block, key) | ||
| .map(|x| x.map(|x| x.0.len() as u64))) | ||
| 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()) |
There was a problem hiding this comment.
Shouldn't that query be done conditionally only if value_len results into None?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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))),
})There was a problem hiding this comment.
I'm pretty sure I tried something like that and failed once, but let me try again and get back.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
The storage_pairs function will return the results directly. You need to do this differently here.
There was a problem hiding this comment.
can you explain more? I don't see the difference in what you suggested.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
kianenigma
added
A6-mustntgrumble
and removed
A0-please_review
kianenigma
added
A0-please_review
client/rpc/src/state/mod.rs
Outdated
| Box::new(self.storage(block, key) | ||
| .map(|x| x.map(|x| x.0.len() as u64))) | ||
| 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()) |
There was a problem hiding this comment.
The storage_pairs function will return the results directly. You need to do this differently here.
Co-authored-by: Bastian Köcher <[email protected]>
… into kiz-rpc-for-map-len
|
with a gratitude to @bkchr: |
|
bot merge |
|
Waiting for commit status. |
|
Checks failed; merge aborted. |
|
bot merge |
|
Trying merge. |
ghost
merged commit 
Currently this rpc endpoint only works for state values.
Now, if no data exists in the given storage key, we check it is a (double) map and any other key-pairs are stored with that prefix. If so, we return the sum of all values.