Skip to content
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

bug: Groth16Commitments is empty after trusted setup #1046

Open
mattstam opened this issue Feb 9, 2024 · 5 comments
Open

bug: Groth16Commitments is empty after trusted setup #1046

mattstam opened this issue Feb 9, 2024 · 5 comments

Comments

@mattstam
Copy link

mattstam commented Feb 9, 2024

Description

semaphore-mtb-setup allows you to run an MPC ceremony for Groth16. They've previously used Gnark v0.8.0 (because at the time v0.9.0 was unaudited). Since then, I have been attempting to update it to v0.9.1.

This repo provides custom functions for exporting the pk and vk:

The purpose of these functions is to export the keys in a way that can be read using Gnark's provided ReadFrom() functions. However, when the exported PK is read in this way after migrating to v0.9.1., it throws an error.

The particular error thrown is "must have as many value vectors as bases" from https://github.com/Consensys/gnark-crypto/blob/2e4aaaaefdbfdf06515663986ed884fed1b2177e/internal/generator/pedersen/template/pedersen.go.tmpl#L104 - I've manually checked the values here, its len(pk) == 1 vs. len(values) == 0, causing the error.

This led me to debug the cause of why the length of the passed in values [][]fr.Element are 0, and it turns out that when bn254.Prove() runs:

commitmentInfo := r1cs.CommitmentInfo.(constraint.Groth16Commitments)
it has an empty commitmentInfo.

Expected Behavior

It's expected that commitmentInfo is non-empty, and that the passed in length of the values privateCommittedValues matches the pk.CommitmentKeys

Actual Behavior

commitmentInfo is empty

Possible Fix

In this PR to semaphore-mtb-setup, updating from v0.8 to v0.9 required adjustments due to changes in Gnark's API.

Most of these changes can be found in utils.go and keys.go -- if any of these changes from v0.8 -> v0.9 are not 1:1, it could be lead to the unexpected behavior observed.

Steps to Reproduce

  1. git clone https://github.com/worldcoin/semaphore-mtb-setup.git && cd semaphore-mtb-setup
  2. git fetch origin pull/2/head:mattstam-update-gnark
  3. git checkout mattstam-update-gnark
  4. go test ./...

This will lead to error panic: must have as many value vectors as base

Context

In semaphore-mtb-setup I was attempting to migrate v0.8.0 to 0.9.1 in PR. Although the issue described is not directly in https://github.com/Consensys/gnark, semaphore-mtb-setup is a very useful utility for anyone working with Groth16.

Your Environment

  • gnark version used: v0.9.1
  • gnark-crypto version used: v0.12.2-0.20231013160410-1f65e75b6dfb
  • go version: 1.19
  • Operating System and version: MacOS 13.4.1
@mattstam mattstam changed the title Groth16Commitments is empty after trusted setup bug: Groth16Commitments is empty after trusted setup Feb 9, 2024
@ivokub
Copy link
Collaborator

ivokub commented Feb 11, 2024

Hmm, there may be several aspects here which may be related, but would have to debug further to figure out the issue:

  • commitment info is quite recent addition to gnark, and it may be that for circuits compiled with older versions of gnark the compiled circuit info doesn't contain the data.
  • gnark included MPC imo doesnt support commitments, but would have to double check
  • I see you referenced icicle backend. We have the code path included and ran the tests at merge time, but we're not running continuous tests for it. There may be some updates to gnark CPU backends not merged to GPU backend.

All in all, will look into it, but relies on several non-default code paths.

@ivokub
Copy link
Collaborator

ivokub commented Feb 13, 2024

@mattstam - looking at the failing test TestProveAndVerify, I see that the proving and verification key is deserialized from file. For which circuit the keys have been generated? Is there a test for generating the keys also?

@mattstam
Copy link
Author

@mattstam - looking at the failing test TestProveAndVerify, I see that the proving and verification key is deserialized from file. For which circuit the keys have been generated? Is there a test for generating the keys also?

Sorry it's a little confusing, the keys are generated here https://github.com/worldcoin/semaphore-mtb-setup/blob/d46ef6be3eb0c43303d7e817f7d0c005530addf0/test/example_test.go#L106 in TestSetup's ExtractKeys().

So the files from TestSetup are used during TestProveAndVerify, the latter of which demonstrates a standard trusted setup procedure using the program.

@ivokub
Copy link
Collaborator

ivokub commented Feb 23, 2024

But in this case, how does the proving key and verification key depend on the circuit? In TestSetup

// Compile the circuit
	var myCircuit Circuit
	ccs, err := frontend.Compile(bn254.ID.ScalarField(), r1cs.NewBuilder, &myCircuit)
	if err != nil {
		t.Error(err)
	}
	writer, err := os.Create("circuit.r1cs")
	if err != nil {
		t.Error(err)
	}
	defer writer.Close()
	ccs.WriteTo(writer)

is the only place either writer or ccs is used. Most importantly, when we write files pk and vk they do not depend on the circuit at all?

Now in TestProveAndVerify, the line which fails is https://github.com/worldcoin/semaphore-mtb-setup/blob/d46ef6be3eb0c43303d7e817f7d0c005530addf0/test/example_test.go#L131C22-L131C27
(using proving key for proving a compiled circuit), but it cannot match to the circuit itself?

@ivokub
Copy link
Collaborator

ivokub commented Feb 23, 2024

All in all, I do not understand the MPC setup code very well. But something seems to be off here, maybe the proving key is generated for some particular circuit? I'll try to have a look, but I'm not very hopeful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants