GKR Gate Registry #652

Tabaie · 2025-02-25T19:43:06Z

Change Gate from an interface to a concrete type, containing its fan-in, function reference, degree, and whether there is a variable of degree 1 (will be useful in later optims)
A thread-safe gate registry as the sole way to create gates
Options for automatic detection or verification of the total degree and 1-degree var index
Remove the unsound function TestGateDegree.

Copilot

Pull Request Overview

This PR changes the implementation of polynomial interpolation across multiple ECC packages and updates the GKR module to use a thread‐safe gate registry with automatic detection/verification of gate degrees while removing the unsound TestGateDegree function.

Implements an Interpolate function via Gaussian elimination in several polynomial.go files.
Refactors GKR tests and gate initialization to use GetGate and helper functions (such as setRandomSlice) for consistency.

Reviewed Changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
ecc/grumpkin/fr/polynomial/polynomial.go	Added error import and a new Interpolate function via Gaussian elimination.
ecc/bw6-633/fr/polynomial/polynomial.go	Same modifications as above supporting the new gate registry design.
ecc/bls24-317/fr/polynomial/polynomial.go	Introduced Interpolate and setRandom changes for consistency.
ecc/bls24-315/fr/polynomial/polynomial.go	Updated polynomial interpolation with error handling.
ecc/bn254/fr/polynomial/polynomial.go	Added Interpolate with proper error checks.
ecc/bw6-761/fr/polynomial/polynomial.go	Same as above with error import and interpolation update.
ecc/bls12-381/fr/polynomial/polynomial.go	Similar Interpolate function update.
ecc/bls12-377/fr/polynomial/polynomial.go	Interpolate function added with error handling.
Various GKR test files (e.g. ecc/bn254/fr/gkr/gkr_test.go, ecc/bls24-317/fr/gkr/gkr_test.go, etc.)	Updated gate instantiation to use GetGate("name"), replaced setRandom with setRandomSlice, and removed direct use of unsound functions.
ecc/bls12-377/fr/poseidon2/gkrgates/gkrgates.go	Refactored gate registrations to use the RegisterGate API with unverified degree options.

ecc/grumpkin/fr/polynomial/polynomial.go

ecc/bls12-377/fr/polynomial/polynomial.go

ecc/bls12-377/fr/gkr/gkr.go

ecc/bls12-377/fr/gkr/gkr_test.go

ivokub · 2025-03-14T09:16:19Z

Suggested edit:

diff --git a/ecc/bls12-377/fr/polynomial/polynomial.go b/ecc/bls12-377/fr/polynomial/polynomial.go
index 9aee8b405..64f4c2ba5 100644
--- a/ecc/bls12-377/fr/polynomial/polynomial.go
+++ b/ecc/bls12-377/fr/polynomial/polynomial.go
@@ -212,7 +212,9 @@ func (p Polynomial) Text(base int) string {
 
 // InterpolateOnRange maps vector v to polynomial f
 // such that f(i) = v[i] for 0 ≤ i < len(v).
-// len(f) = len(v) and deg(f) ≤ len(v) - 1
+// len(f) = len(v) and deg(f) ≤ len(v) - 1.
+//
+// See [Interpolate] for interpolating arbitrary points.
 func InterpolateOnRange(v []fr.Element) Polynomial {
 	nEvals := uint8(len(v))
 	if int(nEvals) != len(v) {
@@ -309,8 +311,8 @@ func computeLagrangeBasis(domainSize uint8) []Polynomial {
 	return res
 }
 
-// Interpolate fits a polynomial of degree len(X) - 1 = len(Y) - 1 to the points (X[i], Y[i])
-// Note that the runtime is O(len(X)³)
+// Interpolate fits a polynomial of degree len(X) - 1 = len(Y) - 1 to the points (X[i], Y[i]).
+// Note that the runtime is O(len(X)³). For a more efficient interpolation method, see [InterpolateOnRange].
 func Interpolate(X, Y []fr.Element) (Polynomial, error) {
 	if len(X) != len(Y) {
 		return nil, errors.New("X and Y must have the same length")

Copilot

Pull Request Overview

This PR refactors the GKR gate registry by replacing the old interface‐based Gate abstraction with a concrete type and a unified, thread‑safe registry. Key changes include:

Converting Gate from an interface to a concrete type that encapsulates its fan‑in, evaluation function, degree, and solvable variable information.
Introducing helper options (e.g. WithDegree, WithUnverifiedDegree, WithNoSolvableVar) to enable automatic detection or explicit verification of gate degree and solvability.
Updating multiple submodules (bw6-633, bls24-315, bls12-381, bls12-377, bn254, and bls24-317) and their corresponding tests to use the new registry API and naming conventions.

Reviewed Changes

Copilot reviewed 32 out of 41 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
ecc/bw6-633/fr/gkr/registry.go	Implements the new concrete Gate type, degree detection/verification, and thread‑safe registry.
ecc/bls24-315/fr/gkr/registry.go	Mirrors changes for the BLS24‑315 curve with updated gate registrations.
ecc/bls12-381/fr/gkr/registry.go	Refactors gate registration and verification code in line with new standards.
ecc/bls12-377/fr/gkr/registry.go	Applies new registry API and gate options for degree and solvability.
ecc/bn254/fr/gkr/registry.go	Adopts the new concrete Gate API and registration options for the BN254 curve.
ecc/bls24-317/fr/gkr/registry.go	Updates the BLs24‑317 implementation to use GetGate and the new options.
Various gkr_test.go files across curves	Updates tests to use GetGate, renames helper functions (e.g. setRandomSlice), and aligns gate usage with the updated registry.

Files not reviewed (9)

ecc/bw6-761/fr/gkr/gkr.go: Language not supported
ecc/bn254/fr/gkr/gkr.go: Language not supported
ecc/bls12-377/fr/gkr/gkr.go: Language not supported
ecc/bls12-381/fr/gkr/gkr.go: Language not supported
ecc/bls12-377/fr/poseidon2/gkrgates/gkrgates.go: Language not supported
ecc/bls12-381/fr/gkr/gkr_test.go: Language not supported
ecc/bls24-317/fr/gkr/gkr.go: Language not supported
ecc/bw6-633/fr/gkr/gkr.go: Language not supported
ecc/bls24-315/fr/gkr/gkr.go: Language not supported

Comments suppressed due to low confidence (2)

ecc/bw6-633/fr/gkr/registry.go:55

The use of the literal '-1' to indicate the absence of a solvable variable may be unclear. Consider using a named constant or adding a clarifying comment to improve code readability.

settings.solvableVar = -1

ecc/bls24-317/fr/gkr/gkr_test.go:373

[nitpick] The introduced helper function 'setRandomSlice' should be named consistently with similar utilities across the codebase. Consider reviewing naming conventions to ensure uniformity.

func setRandomSlice(slice []fr.Element) {

Copilot · 2025-03-23T16:35:51Z

ecc/bw6-633/fr/gkr/registry.go

+
+	if s.degree == -1 { // find a degree
+		if s.noDegreeVerification {
+			panic("invalid settings")


Using panic here in production code can cause unexpected crashes if invalid settings are provided. Consider returning an error instead to allow graceful handling of such cases.

Suggested change

panic("invalid settings")

return fmt.Errorf("invalid settings: no degree verification is enabled but no degree is provided")

ivokub

A few comments, but I think in general good to merge.

ivokub · 2025-03-25T12:04:58Z

ecc/bls12-377/fr/gkr/registry.go

+}
+
+const (
+	IdentityGateName GateName = "identity"


Documentation describing the gates would be good. It is quite self-explaining, but doesn't hurt.

I would remove the Name suffix from all constants (maybe even full GateName) - it is implied from the constant name. And in circuits etc the gates are associated with GateName, not with the actual raw Gate type, so imo we don't need to make the distinction here. The goal of having shorter constant names is to reduce the clutter:

... GetGate(IdentityGateName) ...

vs

GetGate(Identity)

ecc/bls12-377/fr/gkr/gkr.go

ecc/bls12-377/fr/gkr/gkr_test.go

ivokub · 2025-03-25T13:04:55Z

ecc/bls12-377/fr/gkr/gkr_test.go

+		return x[0]
+	}, 3, 1)
+
+	testGate("add2", func(x ...fr.Element) fr.Element {


Here and elsewhere can use the gate name constants

ivokub · 2025-03-25T13:10:03Z

ecc/bls12-377/fr/gkr/registry.go

+}
+
+// fitPoly tries to fit a polynomial of degree less than degreeBound to f.
+// degreeBound must be a power of 2.


Bound must be power of two, but we don't check it? In fft.NewDomain we automatically take anyway next power of two of the degreeBound, so it can be arbitrary though?

ivokub · 2025-03-25T13:11:10Z

ecc/bls12-377/fr/gkr/registry.go

+
+// fitPoly tries to fit a polynomial of degree less than degreeBound to f.
+// degreeBound must be a power of 2.
+// It returns the polynomial if successful, nil otherwise


What would happen at the edge case where the gate is constant 0

ivokub · 2025-03-25T13:18:20Z

ecc/bls12-377/fr/gkr/registry.go

+// Failure could be due to the degree being higher than max or the function not being a polynomial at all.
+func (f GateFunction) FindDegree(max, nbIn int) int {
+	bound := uint64(max) + 1
+	for degreeBound := uint64(4); degreeBound <= bound; degreeBound *= 2 {


Maybe bigger increase factor? With 2 we most most definitely run the loop mulltiple times. But for any practical gates if we would have 8-16 etc then would have a single loop.

Tabaie added 5 commits February 25, 2025 11:13

feat: interpolate

9a79951

refactor: GKR gate registry and automatic degree detection for bn254

721f563

chore: generify polynomial stuff

3e24c87

chore: generify gkr changes

ed999a9

chore: codegen rough edges

214d91e

Tabaie requested review from gbotrel, yelhousni and ivokub February 25, 2025 19:43

Tabaie marked this pull request as draft February 25, 2025 19:48

Tabaie added 2 commits February 25, 2025 13:51

refactor: rename f to Evaluate

8348df6

fix: poseidon2 permutations

17b431f

Tabaie marked this pull request as ready for review February 25, 2025 21:31

Tabaie mentioned this pull request Feb 25, 2025

Don't require user reporting of GKR gate degrees Consensys/gnark#1411

Open

Tabaie added 2 commits March 4, 2025 11:59

refactor: make registerGateOption public

a15b428

fix: intKeySBox2 degree

3313753

Tabaie marked this pull request as draft March 4, 2025 20:56

Tabaie added 2 commits March 4, 2025 15:21

refactor: rename linearVar to solvableVar

0e61ecb

chore: generify linearVar refactor

ca2d409

Tabaie marked this pull request as ready for review March 4, 2025 21:35

Tabaie added 2 commits March 5, 2025 15:08

fix: IsAdditive allow f to modify input slice

8cd59b5

perf: explicitly assign solvable vars in poseidon2 gkr

01ad8f3

Tabaie mentioned this pull request Mar 5, 2025

GKR Gate Registry Consensys/gnark#1442

Open

fix: no solvable vars in mul2

d5afd4f

gbotrel requested review from Copilot and removed request for gbotrel March 13, 2025 14:25

Copilot AI reviewed Mar 13, 2025

View reviewed changes

ecc/grumpkin/fr/polynomial/polynomial.go Outdated Show resolved Hide resolved

ivokub requested changes Mar 14, 2025

View reviewed changes

Tabaie added 2 commits March 14, 2025 11:17

revert: remove isLinear

5678e1d

perf: Interpolate to use Lagrange rather than Gauss

7366adf

Tabaie added 10 commits March 18, 2025 11:57

chore: generify Lagrange interpolation

d372a1e

revert: gaussian interpolation

826a175

revert: adapt test to gaussian method

5871cdd

revert: back to lagrange

b1e296b

revert: remove interpolate; use FFT instead

a8a4d51

fix: use of FFTInverse

e461ff3

chore: generify changes

a45ed95

fix: separate fitPoly impl for small rational

522ef76

fix: small rational fitPoly interface

892d390

revert: restore RegisterGate modifications

282d3f1

Tabaie marked this pull request as draft March 19, 2025 04:37

Tabaie added 9 commits March 22, 2025 11:03

chore: only generate gkr for fields with fft

cf8775f

Merge branch 'master' into refactor/gkr-gate-registry

834e426

feat: separate codegen for registry

ffacd69

feat: adapt codegen for FFT availability

b584699

fix: unused import

9219476

refactor: remove setRandom

dd5838e

refactor: register.go

54bc10f

refactor: expose degree and solvability detection; unexpose gate removal

24627ea

refactor: remove removeGate

be041b0

Tabaie requested review from ivokub and Copilot March 23, 2025 16:32

Tabaie marked this pull request as ready for review March 23, 2025 16:32

Copilot AI reviewed Mar 23, 2025

View reviewed changes

ivokub approved these changes Mar 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GKR Gate Registry #652

GKR Gate Registry #652

Tabaie commented Feb 25, 2025

Copilot AI left a comment

ivokub commented Mar 14, 2025

Copilot AI left a comment

Copilot AI Mar 23, 2025

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

ivokub left a comment

ivokub Mar 25, 2025

ivokub Mar 25, 2025

ivokub Mar 25, 2025

ivokub Mar 25, 2025

ivokub Mar 25, 2025

ivokub Mar 25, 2025

	panic("invalid settings")
	return fmt.Errorf("invalid settings: no degree verification is enabled but no degree is provided")

GKR Gate Registry #652

Are you sure you want to change the base?

GKR Gate Registry #652

Conversation

Tabaie commented Feb 25, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

ivokub commented Mar 14, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Copilot AI Mar 23, 2025

Choose a reason for hiding this comment

ivokub left a comment

Choose a reason for hiding this comment

ivokub Mar 25, 2025

Choose a reason for hiding this comment

ivokub Mar 25, 2025

Choose a reason for hiding this comment

ivokub Mar 25, 2025

Choose a reason for hiding this comment

ivokub Mar 25, 2025

Choose a reason for hiding this comment

ivokub Mar 25, 2025

Choose a reason for hiding this comment

ivokub Mar 25, 2025

Choose a reason for hiding this comment