-
Notifications
You must be signed in to change notification settings - Fork 81
use a partial SMT to represent storage entries in accounts query #1167
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
Conversation
69a1424 to
5c849d3
Compare
b6850b8 to
b64093a
Compare
igamigo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM
|
@igamigo this is currently out of sync with the |
|
I reverted some changes in 0xMiden/miden-node#1158 ( see 0xMiden/miden-node#1158 (comment) ), so this is good to once the former landed. |
Co-authored-by: igamigo <[email protected]>
tomyrd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left a couple of comments. We should wait for the node PR to be merged before this one (and also update the referenced branch).
| partial_storage_smts.iter().try_for_each(|(_slot, partial_smt)| { | ||
| for (key, _value) in partial_smt.entries() { | ||
| // validate all required inner nodes of the merkle path for the specific leaf exist | ||
| let _proof = partial_smt.open(key)?; | ||
| } | ||
| } | ||
| Ok::<_, crate::rpc::RpcError>(()) | ||
| })?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? Checking the internal logic of PartialSmt and Smt, PartialSmt::entries will return an iterator over the tracked leaves and PartialSmt::open will only fail if the leaf isn't tracked.
|
@drahnr, I think we could close this for now, no? |
|
Yes, I think this is obsolete now. We may still come back to the general idea of this, but it'll probably look very different from this PR. |
Issue 0xMiden/miden-node#617
Requires 0xMiden/miden-node#1158