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

return a Flag on proof recursive verification, instead of Assert() #1429

Open
p4u opened this issue Feb 21, 2025 · 13 comments · May be fixed by #1432
Open

return a Flag on proof recursive verification, instead of Assert() #1429

p4u opened this issue Feb 21, 2025 · 13 comments · May be fixed by #1432

Comments

@p4u
Copy link

p4u commented Feb 21, 2025

Is your feature request related to a problem? Please describe.

We need to recursively verify multiple SNARK proofs, where some proofs may be intentionally invalid. For example, when aggregating up to 100 proofs with only 50 valid ones, or when only one among several circuit proofs must be valid, forcing immediate assertions via api.Assert limits flexibility.

Returning a verification flag (1 for valid, 0 for invalid) in intermediate functions would grant circuit developers finer control over which proofs to enforce.

Describe the solution you'd like.

Introduce a new method (e.g. ProofIsValid) in the std/recursion packages that returns a frontend.Variable flag indicating whether a proof is valid. This method should accumulate a boolean flag (starting at 1 and multiplying by 0 on any failed check) rather than calling assertions directly. The existing AssertProof method can then be implemented as a wrapper that calls ProofIsValid and asserts that the flag equals 1.

Describe alternatives you've considered.

No viable alternatives offer the same level of flexibility. Current approaches force immediate failure with api.Assert, which doesn’t support conditional aggregation of proofs.

Additional context.

This design aligns with practices in other DSLs (such as Circom) and enables the circuit developer to decide later whether to trigger an assertion. It provides a more composable and modular approach in my opinion.

A non-working example implementation of the new Verifier.ProofIsValid() might look like this. This code does not work because the Pairing functions do only support Asserts, we would need to add other methods to receive the true/false flag.

// Legacy version: AssertProof uses ProofIsValid and then asserts that the proof is valid.
func (v *Verifier[FR, G1El, G2El, GtEl]) AssertProof(
	vk VerifyingKey[G1El, G2El, GtEl],
	proof Proof[G1El, G2El],
	witness Witness[FR],
	opts ...VerifierOption,
) error {
	flag := v.ProofIsValid(vk, proof, witness, opts...)
	v.api.AssertIsEqual(flag, 1)
	return nil
}

// ProofIsValid returns a frontend.Variable flag equal to 1 if the proof is valid and 0 otherwise.
func (v *Verifier[FR, G1El, G2El, GtEl]) ProofIsValid(
	vk VerifyingKey[G1El, G2El, GtEl],
	proof Proof[G1El, G2El],
	witness Witness[FR],
	opts ...VerifierOption,
) frontend.Variable {
	// Start with a valid flag of 1.
	valid := frontend.Variable(1)

	// Check commitment lengths.
	if len(vk.CommitmentKeys) != len(proof.Commitments) {
		valid = v.api.Mul(valid, 0)
	}
	if len(vk.CommitmentKeys) != len(vk.PublicAndCommitmentCommitted) {
		valid = v.api.Mul(valid, 0)
	}

	var fr FR
	nbPublicVars := len(vk.G1.K) - len(vk.PublicAndCommitmentCommitted)
	if len(witness.Public) != nbPublicVars-1 {
		valid = v.api.Mul(valid, 0)
	}

	// Build the input arrays for MSM.
	inP := make([]*G1El, len(vk.G1.K)-1) // skip the one-wire (handled later)
	for i := range inP {
		inP[i] = &vk.G1.K[i+1]
	}
	inS := make([]*emulated.Element[FR], len(witness.Public)+len(vk.PublicAndCommitmentCommitted))
	for i := range witness.Public {
		inS[i] = &witness.Public[i]
	}

	opt, err := newCfg(opts...)
	if err != nil {
		valid = v.api.Mul(valid, 0)
	}
	hashToField, err := recursion.NewHash(v.api, fr.Modulus(), true)
	if err != nil {
		valid = v.api.Mul(valid, 0)
	}

	// Compute maximum number of public committed elements (not used further here).
	maxNbPublicCommitted := 0
	for _, s := range vk.PublicAndCommitmentCommitted {
		if len(s) > maxNbPublicCommitted {
			maxNbPublicCommitted = len(s)
		}
	}

	commitmentAuxData := make([]*emulated.Element[FR], len(vk.PublicAndCommitmentCommitted))
	for i := range vk.PublicAndCommitmentCommitted {
		hashToField.Write(v.curve.MarshalG1(proof.Commitments[i].G1El)...)
		for j := range vk.PublicAndCommitmentCommitted[i] {
			hashToField.Write(v.curve.MarshalScalar(*inS[vk.PublicAndCommitmentCommitted[i][j]-1])...)
		}

		h := hashToField.Sum()
		hashToField.Reset()

		res := v.scalarApi.FromBits(v.api.ToBinary(h)...)
		inS[nbPublicVars-1+i] = res
		commitmentAuxData[i] = res
	}

	switch len(vk.CommitmentKeys) {
	case 0:
		// No commitment to verify.
	case 1:
		if err = v.commitment.AssertCommitment(proof.Commitments[0], proof.CommitmentPok, vk.CommitmentKeys[0], opt.pedopt...); err != nil {
			valid = v.api.Mul(valid, 0)
		}
	default:
		// Multiple commitments are not supported.
		valid = v.api.Mul(valid, 0)
	}

	kSum, err := v.curve.MultiScalarMul(inP, inS, opt.algopt...)
	if err != nil {
		valid = v.api.Mul(valid, 0)
	}
	kSum = v.curve.Add(kSum, &vk.G1.K[0])
	for i := range proof.Commitments {
		kSum = v.curve.Add(kSum, &proof.Commitments[i].G1El)
	}

	if opt.forceSubgroupCheck {
		valid = v.api.Mul(valid, v.pairing.IsOnG1(&proof.Ar))
		valid = v.api.Mul(valid, v.pairing.IsOnG1(&proof.Krs))
		valid = v.api.Mul(valid, v.pairing.IsOnG2(&proof.Bs))
	}
	pairing, err := v.pairing.Pair(
		[]*G1El{kSum, &proof.Krs, &proof.Ar},
		[]*G2El{&vk.G2.GammaNeg, &vk.G2.DeltaNeg, &proof.Bs},
	)
	if err != nil {
		valid = v.api.Mul(valid, 0)
	}

	valid = v.api.Mul(valid, v.pairing.IsEqual(pairing, &vk.E))

	return valid
}
@p4u
Copy link
Author

p4u commented Feb 21, 2025

I would like to know the feasibility of implementing this to understand if we wait for it, or we try to find some other workaround. @ivokub @gbotrel Thanks.

@p4u
Copy link
Author

p4u commented Feb 21, 2025

I'm thinking that instead of returning a flag, it could simply return an assert error (but do no actually Assert).

@ivokub
Copy link
Collaborator

ivokub commented Feb 21, 2025

That is very interesting proposal, and I think it would be worth it. It could actually enable a few use-cases for making rollups more gas-efficient using "optimistic zk-rollup" approach where the PLONK/Groth16 proofs are not always verified on-chain but the challengers could post proof-of-invalidity for some posted proofs. I recall someone has proposed the idea, but I cannot find it now.

The issue is however, that the proof can fail in several places, so we would need to account for that. And for example when the proof verification fails early, then we need to swap the rest of proof checking to work on dummy variables so that we would have a solvable witness to be able to complete the circuit. I haven't checked the proposed change in detail, but just keep in mind.

Another idea - you can provide the expected verification result (succcess/failure) as an argument, as it should be part of the witness anyway imo. And then inside the assertion method you use a hint to deduce what is the actual failure reason, and then implement the corresponding cases. We have done something very similar in ECRecover precompile for Linea, see https://github.com/Consensys/gnark/blob/master/std/evmprecompiles/01-ecrecover.go as we also need to prove ECRecover calls with invalid inputs (in which case the transaction is reverted, but we need to prove the revert). It actually required quite a lot of thinking to handle all the failure cases properly. I think in general for Groth16 it should be straightforward as we only do a pairing check essentially, but for PLONK we actually have several asserts inside the recursive verifier.

@ivokub
Copy link
Collaborator

ivokub commented Feb 21, 2025

In the example you have, in cases:

_, err := something()
if err != nil {
    valid = v.api.Mul(valid, 0)
}

you don't have to add the assertions - such errors are circuit compile time errors (a la input length mismatch etc.), not solving time errors. So it is safe to do instead

go
_, err := something()
if err != nil {
    return err
}

@p4u
Copy link
Author

p4u commented Feb 21, 2025

In the example you have, in cases:

_, err := something()
if err != nil {
valid = v.api.Mul(valid, 0)
}
you don't have to add the assertions - such errors are circuit compile time errors (a la input length mismatch etc.), not solving time errors. So it is safe to do instead

go
_, err := something()
if err != nil {
    return err
}

Right! Well, that was only a very quick&dirty example to provide better understanding of the proposal.

I do believe the big challenge is what you mention of when the proof verification fails early, then we need to swap the rest of proof checking to work on dummy variables so that we would have a solvable witness to be able to complete the circuit.

@ivokub
Copy link
Collaborator

ivokub commented Feb 21, 2025

And one more thing - essentially when doing a PairingCheck, then we compute the pairing result and compare it against 1 in the target group. It is just that when we call PairingCheck, then we don't do the full finalexp, but only part of it, saving a lot of constraints for the full successful recursive verifier. But I think something similar could be done when we only want to check if the pairing corresponds to 1 or not.

@ivokub
Copy link
Collaborator

ivokub commented Feb 21, 2025

Pinging @yelhousni to loop in.

@ivokub
Copy link
Collaborator

ivokub commented Feb 21, 2025

So, I definitely recommend posting as a PR and then we could start working from that. And for simplicity I think Groth16 at first and later can figure out PLONK.

@p4u
Copy link
Author

p4u commented Feb 21, 2025

In general, not having direct Assertions within the Gnark standard library would improve control-flow on the final circuit design. I.e, Circom uses this approach, and they do have control-flow with if/else statements and so on, which is quite useful.
https://docs.circom.io/circom-language/control-flow/

Of course, this is a big change to the design of the framework, so it's maybe something that can be worked slowly with time.

Related #81

@ivokub
Copy link
Collaborator

ivokub commented Feb 21, 2025

In general, not having direct Assertions within the Gnark standard library would improve control-flow on the final circuit design. I.e, Circom uses this approach, and they do have control-flow with if/else statements and so on, which is quite useful. https://docs.circom.io/circom-language/control-flow/

Of course, this is a big change to the design of the framework, so it's maybe something that can be worked slowly with time.

Related #81

Indeed, it would be a big change. Our problem is that gnark is essentially Go API, but for such control flow changes we would have to have a DSL or do source code processing from AST etc.

@p4u
Copy link
Author

p4u commented Feb 21, 2025

So, I definitely recommend posting as a PR and then we could start working from that. And for simplicity I think Groth16 at first and later can figure out PLONK.

I'm not sure if I can do that. I can work at the Circuit API level, but modifying the Pairing Check requires some understanding that I might not have. Furthermore, I'm a bit lost on what part of the Gnark code is built with a Generator and which is manually coded, tbh. Are the algebra/pairing functions autogenerated somehow? Where is the source?

@ivokub
Copy link
Collaborator

ivokub commented Feb 21, 2025

I think for the in-circuit implementation we don't use autogenerated code. The interfaces and implementations are defined in https://github.com/Consensys/gnark/blob/master/std/algebra/interfaces.go and https://github.com/Consensys/gnark/tree/master/std/algebra (see native for 2-chain implementation and emulated for emulated implementations).

For out-circuit computation it is good enough to compute Pair of the inputs, initialize target group 1 with https://pkg.go.dev/github.com/consensys/gnark-crypto@v0.16.0/ecc/bls12-377/internal/fptower#E12.SetOne and then https://pkg.go.dev/github.com/consensys/gnark-crypto@v0.16.0/ecc/bls12-377/internal/fptower#E12.Equal for checking if the pairing check succeeded.

@yelhousni
Copy link
Contributor

Pinging @yelhousni to loop in.

Indeed PairingCheck computes the final exp only once and shares the squares of all the Miller loops à la 'FE((f_1 * ... *f_n)^2).

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