-
Notifications
You must be signed in to change notification settings - Fork 222
Remove GKR from gnark-crypto #670
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
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 removes the GKR components from gnark-crypto in preparation for moving them to gnark.
- Removed all sumcheck and GKR registry files for various curves (bn254, bls24-317, bls24-315, bls12-381, bls12-377)
- Removed related test vector utilities and Poseidon2 gkrgate implementations
Reviewed Changes
Copilot reviewed 96 out of 96 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ecc/bn254/fr/sumcheck/sumcheck_test.go | Removed GKR-related sumcheck test code for bn254 |
| ecc/bn254/fr/sumcheck/sumcheck.go | Removed GKR-related sumcheck implementation for bn254 |
| ecc/bn254/fr/gkr/registry.go | Removed GKR registry file for bn254 |
| ecc/bls24-317/fr/test_vector_utils/test_vector_utils.go | Removed test vector utilities for bls24-317 |
| ecc/bls24-317/fr/sumcheck/sumcheck_test.go | Removed sumcheck tests for bls24-317 |
| ecc/bls24-317/fr/sumcheck/sumcheck.go | Removed sumcheck code for bls24-317 |
| ecc/bls24-317/fr/gkr/registry.go | Removed GKR registry for bls24-317 |
| ecc/bls24-315/fr/test_vector_utils/test_vector_utils.go | Removed test vector utilities for bls24-315 |
| ecc/bls24-315/fr/sumcheck/sumcheck_test.go | Removed sumcheck tests for bls24-315 |
| ecc/bls24-315/fr/sumcheck/sumcheck.go | Removed sumcheck code for bls24-315 |
| ecc/bls24-315/fr/gkr/registry.go | Removed GKR registry for bls24-315 |
| ecc/bls12-381/fr/test_vector_utils/test_vector_utils.go | Removed test vector utilities for bls12-381 |
| ecc/bls12-381/fr/sumcheck/sumcheck_test.go | Removed sumcheck tests for bls12-381 |
| ecc/bls12-381/fr/sumcheck/sumcheck.go | Removed sumcheck code for bls12-381 |
| ecc/bls12-381/fr/gkr/registry.go | Removed GKR registry for bls12-381 |
| ecc/bls12-377/fr/test_vector_utils/test_vector_utils.go | Removed test vector utilities for bls12-377 |
| ecc/bls12-377/fr/sumcheck/sumcheck_test.go | Removed sumcheck tests for bls12-377 |
| ecc/bls12-377/fr/sumcheck/sumcheck.go | Removed sumcheck code for bls12-377 |
| ecc/bls12-377/fr/gkr/registry.go | Removed GKR registry for bls12-377 |
| ecc/bls12-377/fr/poseidon2/gkrgates/gkrgates.go | Removed GKR gate implementations for Poseidon2 on bls12-377 |
| ecc/bls12-377/fr/gkr/registry.go | Removed additional GKR registry file for bls12-377 |
gbotrel
left a comment
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.
so, I'm going to approve, but I think the polynomial package should only be defined in gnark-crypto and not "duplicated with different API" in gnark..
i.e. sumcheck / gkr in gnark ok, I think while we iterate on it it makes sense. But the low level stuff like polynomial should be stable enough, sturdy in gnark-crypto imo.
|
Thanks @gbotrel. Will merge once Consensys/gnark#1467 is approved. |
As a proof system, it is better housed in gnark. Companion gnark PR: Consensys/gnark#1467