Feat/gkr api

[Tabaie]

An API for GKR to make it easy to use end-to-end and usable inside gadgets.

[gbotrel]

Mhhh it's a bit hard to review as is, and I think couple of things are needed first.

• lots of pieces should be in gnark-crypto . I don't understand why we define gates here, nor do we deal with memory pool and worker pool at this level.
• in the same spirit the "GkrGateRegistery" and "HashBuilderRegistry" are not documented, not sure I understand the initial objective.
• given that GKR just implement Add/Sub/Mul I see very little benefit to be a "fullblown" frontend.API. we could simplify / improve readability quite a lot by having "typed" object inside the gkr package instead of dealing with frontend.Variable there.
• a "end-to-end" how-to-use gkr example for a new comer (in a _test.go file, ExampleXX(...) would be very useful. That would help understand the proposition here; i.e "here is an example with a MiMC gate (defined in gnark-crypto right?) -> here is how you build the GKR circuit, how you put it in a gnark circuit". And "here is how you would define your own gate or compose more gates... or whatever".
• GKRInfo and other structs in constraint/ package lack documentation; semantically, I don't think a ConstraintSystem should have the AddGkr and GkrInfo bits. It would be cleaner if the GKR circuit description ends up in ... constraint/gkr maybe? I see the constraint/ and ConstraintSystem as representing a list of mathematical constraints. The GKR bits seem to add too much here, with gate description and logic that is closer to... the frontend / "how to write a circuit".
• some key functions on the gkr.API are not documented. When I read gkr := gkr.NewApi(); gkr.Import(...) it's hard to recall what Importdoes :)
• probably taking its roots from gnark-crypto but the "proofSerialization" business is a bit odd; in couple of places found some interface{} and usage of reflect package to optimistically convert to frontend.Variable. Objects used in test cases should be exported by a gnark-crypto pacakge and defined only once. *big.Int plays well with MarshalJSON so no need to put interface{} and do hand-made interpretations. (ie the main thought behind that is I find a lot of code is polluting the logic and could go away)

[Tabaie]

Worker and memory pools are large, reusable resources. That's why I thought it made sense for them to be shared between the solver and prover. Maybe the performance benefit is insignificant, I haven't benchmarked with and without yet, but in that case we can just kill it. Otherwise would a more opaque resources object, so that the gnark-level orchestrator doesn't have to directly touch the pools but can still hand them from the solver to the prover, make more sense?

[Tabaie]

GKR is built on a circuit representation of computation, rather than linear constraints etc. That's why there are gates on the gnark side.

[Tabaie]

The HashRegistry and GateRegistry are imperfect solutions to the problem that every specification in GKR world has to be duplicated once in pure Go (prover side) and once in a SNARK (verifier side.) For example, when the API user writes z := gkr.Mul(x,y) it needs to be translated to a gate that processes frontend.Variable inputs on the SNARK/Verifier side and also to another that processes fr.Element inputs depending on the SNARK curve. So I thought the cleanest way to handle this was to have a string identifier for a gate that would later (based on the registry tables) be translated to an implementation on each side.

Similarly for hashes, the user specifies to the function Solver.Verify what hash they want to use for Fiat-Shamir. That hash will have to be translated to a SNARK implementation on the verifier side and to a pure Go one on the prover side. Afaik we don't have a universal identifier for hash functions in that way, though that may be desirable and usable elsewhere.

Lmk if that makes sense, if so I'll add it to docs.

[Tabaie]

given that GKR just implement Add/Sub/Mul I see very little benefit to be a "fullblown" frontend.API. we could simplify / improve readability quite a lot by having "typed" object inside the gkr package instead of dealing with frontend.Variable there.

If we decide there isn't enough value in being able to pass a GKR API into a gadget, we can kill this (though it would make me very sad, I just think it's a really cool idea.)
The fact that it only implements mul, add, subtract was a bit of laziness on my part. Division is also doable it would just take some extra work. One other thing that the current API doesn't address is that GKR is very conducive to custom gates. I think ultimately the best way is to extract a ComputationRepresentation sub-interface from frontend.API that only deals with these operations so that both GKR and emulated arithmetic can in "honestly" claim to be implementing.

[gbotrel]

given that GKR just implement Add/Sub/Mul I see very little benefit to be a "fullblown" frontend.API. we could simplify / improve readability quite a lot by having "typed" object inside the gkr package instead of dealing with frontend.Variable there.

If we decide there isn't enough value in being able to pass a GKR API into a gadget, we can kill this (though it would make me very sad, I just think it's a really cool idea.) The fact that it only implements mul, add, subtract was a bit of laziness on my part. Division is also doable it would just take some extra work. One other thing that the current API doesn't address is that GKR is very conducive to custom gates. I think ultimately the best way is to extract a ComputationRepresentation sub-interface from frontend.API that only deals with these operations so that both GKR and emulated arithmetic can in "honestly" claim to be implementing.

Well, thing is, do we need to? I think it makes more sense to do a minimal stuff with add / mul / sub, a clear way (as simple as possible) to do the original goal (hash (MiMC / Poseidon maybe) computation). Have a small code size footprint, less tests to write, less code & doc to maintain, specially for the upcoming refactoring that PlonK + custom gates may introduce.

For the GKRInfo thing, maybe that's a good candidate to use @ivokub key value store, I'ld rather the gadget (ie the GKR gadget) hides its business in a std/gkr package, than it to modify the ConstraintSystem pieces.

[Tabaie]

Yes I think allowing custom gates has a significant performance implication (Will test on MiMC soon.)

[CLAassistant]

All committers have signed the CLA.