Consensys / gnark

gnark is a fast zk-SNARK library that offers a high-level API to design circuits. The library is open source and developed under the Apache 2.0 license
https://hackmd.io/@gnark
Apache License 2.0
1.43k stars 369 forks source link

Error when generating a proof for a circuit to which I did not add an extra initial public variable #544

Open ilitteri opened 1 year ago

ilitteri commented 1 year ago

The next piece of code proves that $x \cdot y = z$ where $x, z$ are public variables and $y$ is a private variable (witness):

func Example() {
    // [Y, Z]
    publicVariables := []fr_bn254.Element{fr_bn254.NewElement(2), fr_bn254.NewElement(6)}
    // [X]
    secretVariables := []fr_bn254.Element{fr_bn254.NewElement(3)

    /* R1CS Building */

    // (X * Y) == Z
    // X is secret
    // Y is public
    // Z is public
    r1cs := cs_bn254.NewR1CS(1)

    // Variables
    _ = r1cs.AddPublicVariable("1") // the ONE_WIRE
    Y := r1cs.AddPublicVariable("Y")
    Z := r1cs.AddPublicVariable("Z")
    X := r1cs.AddSecretVariable("X")

    // Coefficients
    COEFFICIENT_ONE := r1cs.FromInterface(1)

    // Constraints
    // (1 * X) * (1 * Y) == (1 * Z)
    constraint := constraint.R1C{
        L: constraint.LinearExpression{r1cs.MakeTerm(&COEFFICIENT_ONE, X)}, // 1 * X
        R: constraint.LinearExpression{r1cs.MakeTerm(&COEFFICIENT_ONE, Y)}, // 1 * Y
        O: constraint.LinearExpression{r1cs.MakeTerm(&COEFFICIENT_ONE, Z)}, // 1 * Z
    }
    r1cs.AddConstraint(constraint)

    constraints, r := r1cs.GetConstraints()

    for _, r1c := range constraints {
        fmt.Println(r1c.String(r))
    }

    /* Universal SRS Generation */

    pk, vk, _ := groth16.Setup(r1cs)

    /* Proving */

    rightWitness := buildWitnesses(r1cs, publicVariables, secretVariables)

    p, _ := groth16.Prove(r1cs, pk, rightWitness)

    /* Verification */

    publicWitness, _ := rightWitness.Public()

    verifies := groth16.Verify(p, vk, publicWitness)

    fmt.Println("Verifies with the right public values :", verifies == nil)

    wrongPublicVariables := []fr_bn254.Element{fr_bn254.NewElement(1), fr_bn254.NewElement(5)}
    wrongWitness := buildWitnesses(r1cs, wrongPublicVariables, secretVariables)
    wrongPublicWitness, _ := wrongWitness.Public()
    verifies = groth16.Verify(p, vk, wrongPublicWitness)

    fmt.Println("Verifies with the wrong public values :", verifies == nil)
}

For you to be able to run this, you'll need the buildWitness function:

func buildWitnesses(r1cs *cs_bn254.R1CS, publicVariables fr_bn254.Vector, privateVariables fr_bn254.Vector) witness.Witness {
    witnessValues := make(chan any)

    go func() {
        defer close(witnessValues)
        for _, publicVariable := range publicVariables {
            witnessValues <- publicVariable
        }
        for _, privateVariable := range privateVariables {
            witnessValues <- privateVariable
        }
    }()

    witness, err := witness.New(r1cs.CurveID().ScalarField())
    if err != nil {
        log.Fatal(err)
    }

    // -1 because the first variable is the ONE_WIRE.
    // This is a patch for the bug that I'm issuing that let this code work.
    witness.Fill(r1cs.GetNbPublicVariables()-1, r1cs.GetNbSecretVariables(), witnessValues)

    return witness
}

This code works but only because I added manually the following line:

_ = r1cs.AddPublicVariable("1") // the ONE_WIRE

If you remove this line and the -1 in the patch from the line:

witness.Fill(r1cs.GetNbPublicVariables()-1, r1cs.GetNbSecretVariables(), witnessValues)

you'll get the following error when proving:

18:32:36 ERR error="invalid witness size, got 3, expected 2 = 1 (public) + 1 (secret)" backend=groth16 nbConstraints=1

The error says that we are specting 2 variables (1 public and 1 private) when this is wrong. We've already declared 3 variables (2 public and 1 private).

Users mustn't be responsible for handling this like that.

gbotrel commented 1 year ago

first, thanks for the nice reproductible snippet 👍

Now: (keep in mind; this constraint/ package is quite recent, up until last month, it was only used internally by gnark frontend / backend.)

  1. The "one_wire" bit of R1CS is hidden from gnark end-user; they don't care about it when they write a circuit nor when they create a witness, serialize or deserialize a witness. So forcing the NewR1CS method to do that behind the scenes is reasonable 👍
  2. schema.NbPublic is different than r1cs.GetNbPublicVariables both return what they should; the schema cares about the structure of the circuit, and returns the number of public variables there. The r1cs cares about the actual number of public variables it has (including the one wire).
  3. also witness.Fill may not be the best choice; if the witness is generated outside of gnark, it can directly be decoded (UnmarshalBinary)
    --> i.e; I think it's not too much to ask to a power user (i.e doing something like using only the frontend / only the backend of gnark) to take care of this "one_wire" special case. I suspect the scenario is "I want to write a new frontend for gnark, so I need to generate the witness in the correct format for the constraint system to be solvable" ;