Add hash.Hash wrapper to goldenposeidon #59
Conversation
This file contains 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
This approach makes the test vector reusable in another files in the package. Signed-off-by: Wojciech Zmuda <[email protected]>
Similarly to the other poseidon implementation, add hash.Hash interface wrapper to poseidon over goldilocks. Unit tests for this interface use the same test vector as the non-wrapped version, to ensure the wrapper doesn't introduce bugs. Signed-off-by: Wojciech Zmuda <[email protected]>
|
HI @rjfraize80, can I do something with this PR to help with merging? On one hand I see you approved the changes, on the other github still says that 1 approving review is required. Please let me know if some action is required from me. |
|
Hi @wzmuda! Thanks for your PR, I will review it. |
OBrezhniev
requested changes
Dec 20, 2023
There was a problem hiding this comment.
@wzmuda Please add tests and test vectors (with a link to sources) for byte array hashing with goldilocks poseidon. Maybe zkEVM or plonky2 have such.
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.
The Poseidon (non-goldilocks) implementation already has the hash.Hash interface, which makes it usable with other libraries that utilize external hash functions. This PR adds an equivalent feature to the goldenposeidon implementation. The majority of the wrapper is basically a conversion between two uint64 arrays (as expected by the non-wrapped goldenposeidon hash function) and slice of bytes, as expected by the hash.Hash interface.
Goldenoseidon's test were refactored a little. They already contain a nice vector of test data. The vector was moved out of test function, so it can be used in the wrapper tests. Both wrapped and non-wrapped goldenposeidons are the same hash function, only with different interfaces, so the wrapped function should behave exactly the same as the non-wrapped function. Unified testing data make it easier to check if the interface introduced bugs, if it fails with the same test case that passes in the raw-interface version.