-
Notifications
You must be signed in to change notification settings - Fork 26
Generate proofs for gindices #50
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
base: main
Are you sure you want to change the base?
Conversation
* added scratch proof generation for FixedVector and VariableLists * changes * changes
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.
Partial review, I need to dig into this more
let proof = vec.compute_proof_for_gindex(1); | ||
assert!(proof.is_ok()); | ||
if let Ok(proof) = proof { | ||
assert_eq!(proof.len(), 0); |
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.
I think the proof should probably be length 1 here, and be exactly equal to the tree hash root?
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.
I was assuming that no one would ever request for a proof for the root. it's just a small change I can add that. do you think we should add it?
let proof = vec.compute_proof_for_gindex(2); | ||
assert!(proof.is_ok()); | ||
if let Ok(proof) = proof { | ||
assert!(!proof.is_empty()); |
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.
We could assert something more detailed here:
- First element of the proof should be the tree_hash_root of the full tree
- Second element should be the length
Alternatively we should write a function to check a merkle proof given a gindex, a leaf and a root
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.
We could probably use the merkle_proof
library from Lighthouse for this actually
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.
(would have to put these tests in Lighthouse, until LEAP happens and all these libraries are co-located in one repo)
for gindex in gindices { | ||
let proof = vec.compute_proof_for_gindex(gindex); |
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.
We could iterate through VecIndex<I, N>
here instead, to check we can do proofs for all leaves
where | ||
T: tree_hash::TreeHash |
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.
where | |
T: tree_hash::TreeHash | |
where | |
T: tree_hash::prototype::MerkleProof |
Ideally we want to be able to combine proofs in a nested way. E.g. List<List<u64, U10>, U20>
should work.
No description provided.