Skip to content
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

Implemented Batched Proof for multiple openings and Path Pruning #130

Merged
merged 39 commits into from
Mar 25, 2024

Conversation

intx4
Copy link
Contributor

@intx4 intx4 commented Jan 17, 2024

Description

This PR implements an optimized struct MultiPath which holds multiple proofs for multiple leaves. A MultiPath is generated via MerkleTree::generate_multi_proof(...) which takes an iterable of leaf indexes. MultiPath optimizes the proof size by compressing multiple paths using Front Encoding. To verify, MultiPath exposes the same API as Path, with a verify(...) method which takes an iterable of leaf hashes instead of a single hash. For verification, MultiPath optimizes runtime by using a lookup table to avoid recomputing redundant hashes of nodes.
The only caveat to MultiPath is that, when calling generate_multi_proof, the provided array of indexes is internally sorted to maximize compression efficiency. The array of leaves hashes provided at verification time should then match this order. The documentation provides an example code snippet.
Files affected:

  • src/merkle_tree/mod.rs
  • src/merkle_tree/tests/mod.rs

closes: #124


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (main)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@intx4 intx4 requested a review from a team as a code owner January 17, 2024 18:22
@intx4 intx4 requested review from z-tech, Pratyush and mmagician and removed request for a team January 17, 2024 18:22
src/merkle_tree/mod.rs Outdated Show resolved Hide resolved
src/merkle_tree/mod.rs Outdated Show resolved Hide resolved
src/merkle_tree/mod.rs Outdated Show resolved Hide resolved
src/merkle_tree/mod.rs Outdated Show resolved Hide resolved
src/merkle_tree/mod.rs Outdated Show resolved Hide resolved
src/merkle_tree/mod.rs Outdated Show resolved Hide resolved
@mmagician
Copy link
Member

That's a great addition, thanks! I left minor comments for now, though haven't gone over the actual algorithm yet.
Did you run any benches to compare performance to having individual paths?

src/merkle_tree/mod.rs Outdated Show resolved Hide resolved
src/merkle_tree/mod.rs Outdated Show resolved Hide resolved
intx4 and others added 7 commits January 18, 2024 12:40
Co-authored-by: Pratyush Mishra <pratyush795@gmail.com>
Co-authored-by: Pratyush Mishra <pratyush795@gmail.com>
Co-authored-by: Marcin <marcin.gorny.94@protonmail.com>
Co-authored-by: Marcin <marcin.gorny.94@protonmail.com>
Co-authored-by: Marcin <marcin.gorny.94@protonmail.com>
Co-authored-by: Marcin <marcin.gorny.94@protonmail.com>
Co-authored-by: Marcin <marcin.gorny.94@protonmail.com>
@intx4
Copy link
Contributor Author

intx4 commented Jan 18, 2024

@mmagician I fixed the code in decompress to avoid the extra loop and removed the decompress function entirely to merge the decompression step directly into verification, thus removing another loop. According to benches this has decreased the runtime of around 7% w.r.t the previous implementation with the extra loops.

EDIT: did a similar optimization for compression. I removed compress and pushed compression into generation. This speeds up things a lot (70%)

Copy link
Member

@mmagician mmagician left a comment

Choose a reason for hiding this comment

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

It is very strange that batch verification is slower than naive - if anything, by using the lookup table I'd expect a speedup. Were you able to identify which part of the new batch verify is causing this?

@intx4
Copy link
Contributor Author

intx4 commented Feb 2, 2024

It is very strange that batch verification is slower than naive - if anything, by using the lookup table I'd expect a speedup. Were you able to identify which part of the new batch verify is causing this?

the PR has been updated, now in the single threaded case MultiProof Verification is faster (verification test runs in ~2.8s on my thinkpad X1 Gen10 against ~3.8s of single proof verification). For reference, multiproof is only slightly slower for creation of the proof (tests for generation run in ~600ms against ~400ms for single proof generation), but on the other size the proof size is smaller and verification is much faster.

@intx4
Copy link
Contributor Author

intx4 commented Mar 25, 2024

@mmagician the changes proposed by @Cesar199999 have been merged to the main branch of my fork

Copy link
Member

@mmagician mmagician left a comment

Choose a reason for hiding this comment

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

That's great!
I think the checks are failing for the same reason as in other repos. @autquis already addressed these in this repo in #136.

@intx4
Copy link
Contributor Author

intx4 commented Mar 25, 2024

That's great! I think the checks are failing for the same reason as in other repos. @autquis already addressed these in this repo in #136.

I was literally about to ask about the failing tests :D

@intx4
Copy link
Contributor Author

intx4 commented Mar 25, 2024

That's great! I think the checks are failing for the same reason as in other repos. @autquis already addressed these in this repo in #136.

@mmagician I followed @autquis commit to fix the tests, all good

@mmagician mmagician added this pull request to the merge queue Mar 25, 2024
Merged via the queue into arkworks-rs:main with commit fc1c949 Mar 25, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement path pruning and multiple openings
4 participants