Merged
Conversation
gbotrel
approved these changes
May 28, 2025
Collaborator
gbotrel
left a comment
There was a problem hiding this comment.
Looks good to me, some minor cosmetic name change suggestion
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds configurable hash functionality to the Vortex prover, enabling users to choose between the default SIS/Poseidon2 and alternative hash functions (e.g. SHA256) for both column hashing and Merkle tree construction. The changes include new options in the parameters, adjustments in the commitment and verification flows, and additional tests covering non-default hash scenarios.
- Updated verifier and prover logic to pass configurable hash functions.
- Introduced Option functions (WithMerkleHash and WithNoSis) to configure which hash to use.
- Added tests to validate behavior when using alternate hash functions.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| field/koalabear/vortex/verifier.go | Updated to pass the configurable merkle hash function for verification. |
| field/koalabear/vortex/transversal_hash.go | Introduced support for a generic hash function when processing codeword columns; note a naming inconsistency. |
| field/koalabear/vortex/prover_test.go | Added new tests for non-default hashing options (NoSis and MerkleHash). |
| field/koalabear/vortex/prover.go | Updated the commitment step to use the configurable hash functions. |
| field/koalabear/vortex/params.go | Added configuration options and option functions to select hash functions. |
| field/koalabear/vortex/merkle_test.go | Updated tests to run Merkle tree construction and verification with both defaults and SHA256. |
| field/koalabear/vortex/merkle.go | Modified the Merkle tree functions to accept a configurable hash and to use a generic hashing method when provided. |
Comments suppressed due to low confidence (1)
field/koalabear/vortex/transversal_hash.go:21
- The function name 'transveralHashGeneric' appears to be misspelled. Consider renaming it to 'transversalHashGeneric' for consistency with other similar functions.
func transveralHashGeneric(codewords []koalabear.Element, newHash NewHash, sizeCodeWord int) []koalabear.Element {
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The Vortex prover now supports options for choosing between different hashes. The default hash is still poseidon2. This feature is needed for the linea prover.
Type of change
Please delete options that are not relevant.
How has this been tested?
Checklist:
golangci-lintdoes not output errors locally