Add support for verifying compact multiproofs #153
Conversation
|
hi @rista404 this is great, thank you! can you address the linter errors? otherwise I'd generally lean to merge this soon |
There was a problem hiding this comment.
generally looks good!
I don't fully get yet how this is different from the other multiproof code, but I think I need to go check the other resources you linked in your first comment
edit: to be clear, I think we can make some of the bit munging more efficient but happy to merge this as-is and refactor later
| } | ||
|
|
||
| pub fn verify_compact_merkle_multiproof( | ||
| nodes: &[Node], |
There was a problem hiding this comment.
nodes here is the leaf layer from a given merkle tree?
|
|
||
| pub fn verify_compact_merkle_multiproof( | ||
| nodes: &[Node], | ||
| descriptor: &[u8], |
There was a problem hiding this comment.
what do you think about using a bitvec type here instead? then we can skip a lot of bool parsing elsewhere in this module
| } | ||
| } | ||
|
|
||
| fn compute_bits_from_proof_descriptor(descriptor: &[u8]) -> Result<Vec<bool>, Error> { |
There was a problem hiding this comment.
could make this much more efficient with just some bit-level arithmetic going byte-by-byte in descriptor and can return a bitvec type that would be much more compact than Vec<bool>
fine to merge this for now and make an issue to refactor
|
@ralexstokes thanks for the review, I'll go back and make those changes in the same PR! I also highly recommend looking into the spec to get a better understanding of the compact multiproofs. |
|
@ralexstokes on a second thought, could we make an issue to refactor the code to use Still very keen to optimise but I think I would need some more time. If the rest looks okay and you've had the time to look into the spec PR, feel free to merge 🙏🏻 Thanks again! |
|
@ralexstokes ping on this! anything else I can do to help get this merged? |
Hi! Thanks for maintaining this great library. We were using it in a (soon to be open-source) project but found ourselves missing the compact multiproof support. This PR adds the support for verifying such proofs.
You can read the spec here: ethereum/consensus-specs#3148
You can also see a full TypeScript implementation here: ChainSafe/ssz#292
I think those two PRs explain why compact proofs are useful! Lodestar, which we interact with, is already using them in production as the only format for certain proofs.
I've marked the implementation as experimental and added some very basic tests. I could do a follow-up PR that adds proof generation as well as more tests. Let me know if something needs changing!