-
Notifications
You must be signed in to change notification settings - Fork 443
Feat: GKR-Hashers #1512
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
base: feat/gkr/add-instance
Are you sure you want to change the base?
Feat: GKR-Hashers #1512
Conversation
This reverts commit 4b43ce3.
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.
Pull Request Overview
This PR adds full hash implementations for Poseidon2 and MiMC, including GKR‐based compressors and parameter management for multiple curves, and updates related tests and benchmarks.
- Introduce
Parameters
type withGetDefaultParameters
to unify Poseidon2 parameters across curves - Refactor GKR compressor for Poseidon2 and add a new GKR MiMC compressor
- Update hash wrappers/tests to use new APIs and remove hardcoded curve assumptions
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
std/permutation/poseidon2/poseidon2.go | Export Parameters fields, add GetDefaultParameters , unify constructors |
std/permutation/poseidon2/gkr-poseidon2/gkr-poseidon2.go | Refactor NewGkrCompressor , generic gate registration & circuit definition |
std/permutation/poseidon2/gkr-poseidon2/gkr-poseidon2_test.go | Update test helper signature, fix loop and assignment logic |
std/permutation/gkr-mimc/gkr-mimc.go | New GKR‐MiMC compressor implementation |
std/permutation/poseidon2/gkr-poseidon2/... (hash wrappers/tests) | Update Poseidon2 hash wrappers and tests for new APIs |
internal/gkr/... | Simplify Println error handling across backends |
Comments suppressed due to low confidence (3)
std/permutation/poseidon2/gkr-poseidon2/gkr-poseidon2_test.go:20
- You cannot range over an
int
. Usefor i := 0; i < n; i++
orfor i := range ins
instead.
for i := range n {
std/hash/poseidon2/poseidon2_test.go:43
- You cannot use
range
on anint
. Change tofor i := 0; i < nbInputs; i++
.
for i := range nbInputs {
std/permutation/poseidon2/gkr-poseidon2/gkr-poseidon2.go:235
- You cannot use
range
on an integer. Replace with a standardfor i := 0; i < p.NbFullRounds/2; i++
loop.
for i := range p.NbFullRounds / 2 {
RoundKeys [][]big.Int | ||
} | ||
|
||
func GetDefaultParameters(curve ecc.ID) (Parameters, error) { |
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.
[nitpick] The switch cases that copy RoundKeys
for each curve are nearly identical. Consider extracting the loop into a helper to reduce duplication and ease future maintenance.
Copilot uses AI. Check for mistakes.
StateStorer
interface.Benchmark: For 27M constraints for 52K permutations on BLS12-377, using degree 17 gates. Attempts to use degree 2 and 4 gates proved counterproductive, resulting in 32M and 31M constraints, respectively.