-
Notifications
You must be signed in to change notification settings - Fork 478
fix: allow to use blake3 to hash PV #2236
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
1917583
to
0fb977d
Compare
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.
Add more documentation about the cross-collision infeasibility + use a helper function to increase code re-use.
In sp1-verifier
we should also make it such that it checks both, rather than requiring a feature flag.
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.
Left some comments on functions that can be cleaned up. Can you also clean up the PR description?
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.
Left some suggestions for how to polish the documentation to be more readable.
Once addressed, will approve.
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
Motivation
verify()
fails on proofs whensp1-zkvm
blake3
feature is enabled.Solution
Remove the
blake3
feature everywhere except insp1-zkvm
, and when we want to verify PV, check with both Sha256 and Blake3 hashing.