bug: Using Range Checking component leads to incorrect # of inputs in Solidity verifier

[puma314]

Description

In a simple circuit when using the range checking component (as shown in a simple test example below), the Verifier.sol that is exported has 4 public inputs, even though the circuit only has 3 public inputs. When generating a proof and outputting the public witness to verify the proof in Solidity the public witness has only has 3 public inputs (which makes sense). What is the 4th public input that should be passed to the on-chain verifier?

Note that the generated proof still verifies in go when using groth16.Verify(proof, vk, publicWitness). If the rangecheck component is removed, then the generated Verifier.sol has the correct verifyProof function signature with 3 public inputs.

type MyCircuit struct {
	X frontend.Variable `gnark:",public"`
	Y frontend.Variable `gnark:",public"`
	Z frontend.Variable `gnark:",public"`
	A frontend.Variable `gnark:"-"`
}

func (circuit *MyCircuit) Define(api frontend.API) error {
	api.AssertIsEqual(circuit.Z, api.Add(circuit.X, circuit.Y))
	rangeChecker := rangecheck.New(api)
	rangeChecker.Check(circuit.X, 100)
	return nil
}

func TestSanity(t *testing.T) {
	circuit := MyCircuit{}

	r1cs, err := frontend.Compile(ecc.BN254.ScalarField(), r1cs.NewBuilder, &circuit)
	if err != nil {
		panic(err)
	}
	_, vk, err := groth16.Setup(r1cs)
	if err != nil {
		panic(err)
	}

	buf := new(bytes.Buffer)
	err = vk.ExportSolidity(buf)
	if err != nil {
		panic(err)
	}
	content := buf.String()

	contractFile, err := os.Create("Verifier.sol")
	if err != nil {
		panic(err)
	}
	w := bufio.NewWriter(contractFile)
	// write the new content to the writer
	_, err = w.Write([]byte(content))
	if err != nil {
		panic(err)
	}
	contractFile.Close()
}

Generated Verifier.sol function signature:

 function verifyProof(
        uint256[8] calldata proof,
        uint256[4] calldata input
    ) public view {

[ivokub]

Hi, this is a known issue: #524

In short, we use something called commitment API in range checking which on a high level is a way to compute an efficient in-circuit challenge. The verifier has to do a bit more work to actually verify the correctness of the challenge and we haven't implemented it yet in the Solidity Groth16 verifier. It is there though for PLONK verifier.

It is quite high on our list, we just have few other more pressing priorities right now.

[puma314]

@ivokub thanks for the quick response! I apologize--I didn't realize that this was already an issue. Is there a easy way to add this to the Solidity groth16 verifier? Would it be basically re-implementing the logic here in Solidity?

I believe this issue would also need to be resolved around including this information in the serialized vkey as well?

In particular, for our circuit we notice that the groth16 version has 6 million constraints, but the Plonk version has 20 million. Also the pkey size is significantly smaller with groth16 as well as proof generation time, which is why we're motivated to use groth16 if possible. (Although the constraint count grows significantly if we don't use the range-checker, which is why we want to use the range-checker + groth16).

[ivokub]

No need to apologize -- that issue was a bit poorly worded and lacks issue description. I added it mostly for backreference :)

Yup, there is about 4x blowup from Groth16 to PLONK. But usually the deciding factor to go with PLONK is that it doesn't require circuit-specific trusted setup (can reuse existing setup for KZG SRS). For Groth16 trusted setup is needed though (but there is an MPC implementation for generating the trusted setup in distributed manner. But its a bit slow for now).

In essence, yes, you should follow the verifier logic in the native verifier. Additionally you need hash-to-field in Solidity what we have already implemented for PLONK verifier (see here).

I'm right now not 100% sure, but I think #580 may already be obsolete.

Pinging @gbotrel and @Tabaie, maybe you have anything to add? We're actually quite interested in having the commitment verification in Groth16 Solidity verifier, but probably won't be able to get to it in a few months, unfortunately.

[Tabaie]

Thanks @ivokub yes I think you covered everything.

[puma314]

By the way, @ivokub has there been any update on implementation of a Solidity verifier for groth16 using the logup range checking component? We profiled a groth16 circuit with and without the logup range check and found that the proving time was 5x faster using logup, so it would be great to be able to use logup +groth16 with onchain verification!

[gbotrel]

see --> #1063

[ivokub]

Implemented in #1063