Gc 304 companion by Tabaie · Pull Request #454 · Consensys/gnark

Tabaie · 2023-02-02T18:18:32Z

Reflecting the changes in Consensys/gnark-crypto#308 including tests and stats related to MiMC and Fiat-Shamir challenge name hashing, as well as things depending on it (EdDSA, Merkle Trees, GKR test vectors)

Tabaie · 2023-02-10T23:32:48Z

Introducing GKR API and making tests pass in light of recent MiMC changes

gbotrel · 2023-02-13T00:13:57Z

staticcheck --> seems this branch is not merged with develop since some time (missing the constraint/ package version for R1CS and SparseR1CS.

Just to clarify, what is the description of the PR? A mix of GKR api (#425) + fix MiMC interface ?

Tabaie · 2023-02-13T00:21:42Z

Yes exactly. It was originally the mimc fixes but since that broke GKR as well I had to merge in the GKR API stuff as well.

Tabaie · 2023-02-13T01:18:56Z

It's currently up to date with develop.

gbotrel · 2023-02-13T01:58:37Z

TestFiatShamir failing locally:

 FAIL: TestFiatShamir (0.77s)
    --- FAIL: TestFiatShamir/bls12_377/groth16 (0.02s)
        assert.go:537: 
                Error Trace:    /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:537
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:299
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:307
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:288
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:74
                Error:          groth16(bls12_377): [assertIsEqual] 2248481674936889848344681279637786769404236779047702341353974298124900307652 == 1804893092514988653572234391207087360545562016624205485159213979627814772119
                                fiat-shamir.(*FiatShamirCircuit).Define
                                        transcript_test.go:78
                            
                                witness:{"Bindings":[[0,0,0,0],[0,1,2,3],[0,2,4,6]],"Challenges":["1804893092514988653572234391207087360545562016624205485159213979627814772119","1367924371156947165337824263043973441016476277020777135048110731039246126174","666493180360088706893004815140289115642905272546227043905595213473311661997"]}
                Test:           TestFiatShamir/bls12_377/groth16
    --- FAIL: TestFiatShamir/bls12_377/plonk (0.02s)
        assert.go:537: 
                Error Trace:    /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:537
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:299
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:307
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:288
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:74
                Error:          plonk(bls12_377): [assertIsEqual] 2248481674936889848344681279637786769404236779047702341353974298124900307652 == 1804893092514988653572234391207087360545562016624205485159213979627814772119
                                fiat-shamir.(*FiatShamirCircuit).Define
                                        transcript_test.go:78
                            
                                witness:{"Bindings":[[0,0,0,0],[0,1,2,3],[0,2,4,6]],"Challenges":["1804893092514988653572234391207087360545562016624205485159213979627814772119","1367924371156947165337824263043973441016476277020777135048110731039246126174","666493180360088706893004815140289115642905272546227043905595213473311661997"]}
                Test:           TestFiatShamir/bls12_377/plonk
    --- FAIL: TestFiatShamir/bls12_377/plonkFRI (0.02s)
        assert.go:537: 
                Error Trace:    /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:537
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:299
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:307
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:288
                                                        /Users/gbotrel/dev/go/src/github.com/consensys/gnark/std/fiat-shamir/assert.go:74
                Error:          plonkFRI(bls12_377): [assertIsEqual] 2248481674936889848344681279637786769404236779047702341353974298124900307652 == 1804893092514988653572234391207087360545562016624205485159213979627814772119
                                fiat-shamir.(*FiatShamirCircuit).Define
                                        transcript_test.go:78
                            
                                witness:{"Bindings":[[0,0,0,0],[0,1,2,3],[0,2,4,6]],"Challenges":["1804893092514988653572234391207087360545562016624205485159213979627814772119","1367924371156947165337824263043973441016476277020777135048110731039246126174","666493180360088706893004815140289115642905272546227043905595213473311661997"]}

gbotrel · 2023-02-13T02:15:42Z

std/accumulator/merkle/verify_test.go

-					r := byte(rand.Int()) //#nosec G404 -- This is a false positive
-					buf.Write([]byte{r})
-				}
+				var leaf fr.Element


test shouldn't depend on bn254.fr.Element

gbotrel · 2023-02-13T02:17:05Z

std/hardcoded_strings/hardcoded_strings.go

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Code generated by gnark DO NOT EDIT


is it generated ?

Yes because of the big switch case

gbotrel · 2023-02-13T02:17:48Z

std/hardcoded_strings/hardcoded_strings.go

+	"math/big"
+)
+
+func String(api frontend.API, str []byte) (frontend.Variable, error) {


don't understand the package name and the function name -- nor while it is a fullblown package in the std ? a package in the stdshould read as "here is a circuit component that anyone can use"

The purpose of this is to cleanly and easily (from the user's POV at least) convert strings to frontend.Variables in a way compatible with what gnark-crypto does. I get that the large switch-case doesn't feel very std-like and that it's not really a "gadget" though.
Does the idea of this function make sense and just not where it's placed?

mmhh if it is something we want to do often, then it makes sense to expose it in its own package, but to me "hardcoded_strings.String" is not the right semantic. Maybe in a bytes package, so that it reads bytes.ToVariable(api, b []byte) ? Or, since it's applicable to constants only, a constant pacakge with constant.FromString() or constant.FromByte().

If we only use it at one place for now, I would make it package private there.

I think constant.FromString() is the best option.

The main reason I avoided more straightforward names was that we already have standard ways of turning a string or []byte into a variable so I wanted to avoid creating confusion between this way of converting and the existing ones. How's constant.HashToVariable?

gbotrel · 2023-02-13T02:18:18Z

std/hash/hash.go

 	Reset()
 }
+
+var BuilderRegistry = make(map[string]func(api frontend.API) (Hash, error))


what is that? comment?

That's for the GKR API. In the specification of a circuit, we don't pass an actual hash object, but a string identifier which can later be used to construct a hash object both in the SNARK (for the GKR verifier) and in pure Go (for the GKR prover.) So this would be used to "register" hash functions in a similar way to hints.

gbotrel · 2023-02-13T02:24:07Z

would it be possible to split this PR in 2? 🙄

for the MiMC stuff and adding a t.Skip() on the GKR tests
for the GKR api and builder / GKR info / algo_utils modifications

I think 1. is straightforward and would unblock gnark v0.8.0 release, and the second one needs a bit more cosmetic work, GKR api is not needed for v0.8.0.

Tabaie · 2023-02-13T14:40:45Z

Yes I can break it up again. The fixes for the GKR tests didn't have anything to do with the API so that merge turned out to be unnecessary.
That being said, I think GKR without the API is close to unusable as the user will have to solve their subcircuit and construct the proof themselves, using gnark-crypto packages and depending what curves their SNARK would be on, might have to import many such packages.

Tabaie added 30 commits December 20, 2022 14:04

refactor: no non-mont on bls12-377

46079a3

refactor: groth16 backend tests pass

0ff8bd5

refactor: MSM takes Montgomery only - Plonk

f515c8c

fix:ToBigIntRegular => BigInt

401993c

fix: more ToBigIntRegular => BigInt

5d56b7a

feat: experiments with solving

3da7cf7

Merge branch 'refactor/no-mont-method' into feat/gkr-api

9f83d5d

Merge branch 'develop' into feat/gkr-api

f042372

chore: some efforts from before christmas break

986f62a

refactor: all in one package

62df360

feat: development done for bn254. to test and generify

be58426

test: fails. pointer issue

b95229e

feat: "generic" top sort

d72713e

refactor: improved, simplified solver; compiler to match

8bdbca7

refactor: use mostly no-ptr data. better information silos

41cb30d

fix: inconsistencies re assignments alignment

743633b

feat: simple compilation test passes

3938c2c

test: solver error found

cae4b07

fix: solving works on the simplest example

0fbf163

test: with dependency. err: inputs are modified

4087f5f

fix: solver works. prover doesn't. possibly deeper gkr issue

defabf6

bench: merkle tree

6097684

refactor: hint-lite, has import cycle

5acf5cb

fix: import cycle

d4b924b

test: basic permutation tests passing

4d29684

test: end-to-end: can't use test engine (for now)

41a1615

fix: propagating gkrInfo

6d041ea

test: "doubling" circuit passes

fc82d97

fix: race condition

97a2001

test: with dependency

e8ed957

Tabaie added 4 commits February 10, 2023 16:10

fix: remove printfs

6a20a0b

Merge branch 'feat/gkr-api' into gc-304-companion

02d2005

refactor: updated gkr test vectors from gc

508d70a

refactor: replace map hash with const hash in gkr tests

9d6d298

Tabaie requested a review from gbotrel February 10, 2023 23:31

Tabaie marked this pull request as ready for review February 10, 2023 23:32

Tabaie added 2 commits February 10, 2023 18:43

Merge branch 'master' into gc-304-companion

56e6d00

style: gofmt

4a181f3

gbotrel added 3 commits February 12, 2023 19:52

build: update go.mod gnark-crypto dev

8feb774

fix: fix Vector pointer type update in witness package

31d74dd

fix: cherry pick witness update commit + ran go generate

e310cbb

gbotrel reviewed Feb 13, 2023

View reviewed changes

Tabaie and others added 6 commits February 13, 2023 10:01

fix: match hardcoded-string prefix with gnark-crypto

179c0b5

revert: no gkr api on this branch!

5ace112

fix: remove fr dependence in Merkle test

c8da378

refactor: hardcoded_strings.String -> constant.HashedBytes

ed69fdf

style: remove unnecessary added new lines

f8e8553

refactor: remove std/utils package from this PR

977eb81

gbotrel merged commit 450364d into develop Feb 14, 2023

gbotrel deleted the gc-304-companion branch February 14, 2023 17:56

Conversation

Tabaie commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tabaie commented Feb 10, 2023

Uh oh!

gbotrel commented Feb 13, 2023

Uh oh!

Tabaie commented Feb 13, 2023

Uh oh!

Tabaie commented Feb 13, 2023

Uh oh!

gbotrel commented Feb 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbotrel commented Feb 13, 2023

Uh oh!

Tabaie commented Feb 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Tabaie commented Feb 2, 2023 •

edited

Loading