gkr_nonnative intial review #1162
Conversation
|
amit0365 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
…ted it in verify -m using emulated.assertequal in verify -m removed unused frombits in element as @arya pointed -m
…ager and populated it in verify" -m "Removed unused fromBits in Element"
Added verifier in ClaimsManager and populated it in verify Using emulated.Assertequal in verify Removed unused fromBits in Element
There was a problem hiding this comment.
I had a partial look at it (not files gkr_nonnative.go and gkr_nonnative_test.go yet). I think in general there may be a confusion what we have meant previously by "native" implementation (which I agree hasn't been the best definition).
Essentially we have been using the term "native" both for the "circuit native field element" and "machine native field element". In the context of GKR, the prover runs completely outside of the circuit working directly with field elements and we do not need to use frontend.Variable and frontend.API at all. Only when we start defining the circuit we need to use frontend.API and frontend.Variable (alternatively, emulated.Field and emulated.Element when working in circuit non-native field).
So, for defining the prover and prover data structures, we shouldn't be needing circuit variables and circuit API at all.
frontend/variable.go
Outdated
| @@ -25,6 +32,521 @@ import ( | |||
| // The only purpose of putting this definition here is to avoid the import cycles (cs/plonk <-> frontend) and (cs/r1cs <-> frontend) | |||
| type Variable interface{} | |||
|
|
|||
| type Element [4]uint64 | |||
There was a problem hiding this comment.
What is the implementation? Usually, when you want to perform witness assignment in gnark then it is sufficient to just assign whatever type you have:
type Circuit struct {
A frontend.Variable
}
func Test(t *testing.T) {
....
assignment := Circuit{A: 10} // also Circuit{A: fr.NewElement(10)} would work etc.
}Maybe I have misunderstood why you have the implementation here?
| @@ -51,6 +51,7 @@ func deriveChallengeProver(fs *cryptofiatshamir.Transcript, challengeNames []str | |||
| return challenge, challengeNames[1:], nil | |||
| } | |||
|
|
|||
| // todo change this bind as limbs instead of bits, ask @arya if necessary | |||
There was a problem hiding this comment.
Good point, but can leave out of scope right now. It would be better to consider it for general case (field and group elements), but group element marshalling is more complicated which we would have to change.
| @@ -88,6 +97,10 @@ func (ee *emuEngine[FR]) Const(i *big.Int) *emulated.Element[FR] { | |||
| return ee.f.NewElement(i) | |||
| } | |||
|
|
|||
| func (ee *emuEngine[FR]) AssertIsEqual(a, b *emulated.Element[FR]) { | |||
There was a problem hiding this comment.
Also remove, does not make sense to have it in ArithEngine which is used for gate evaluation.
| @@ -0,0 +1,203 @@ | |||
| // Copyright 2020 Consensys Software Inc. | |||
There was a problem hiding this comment.
I'm not sure you need pools.
| @@ -51,6 +116,43 @@ func (m MultiLin) fold(api frontend.API, at frontend.Variable) { | |||
| } | |||
| } | |||
|
|
|||
| func (m *MultiLin) FoldParallel(api frontend.API, r frontend.Variable) utils.Task { | |||
There was a problem hiding this comment.
We do not need it - circuit compilation happen sequentially anyway and you cannot parallelize it.
Description
Initial draft of GKR nonnative prover and verifier functions. This can handle arbitrary fields, the variable.go file contains functions like FromBits which takes a variable and casts it as an element to perform necessary computation.
Fixes # (issue)
Type of change
How has this been tested?
Checklist:
golangci-lintdoes not output errors locally