bug: plonk: Compile() corrupts the circuit object

Description

When frontend.Compile(..., &circuit) is called on a circuit := myCircuitDefinition{ ... } object, subsequent call to frontend.NewWitness(&circuit, ...) panics with can't set fr.Element from type expr.Term. I prepared a test case illustrating the problem. The test fails for every curve. I believe it shouldn't fail regardless the order in which Compile() and NewWitness() are called.

Here's the code snippet in case the link above stops working one day:

func TestCompileDoesntCorruptCircuit(t *testing.T) {
    assert := assert.New(t)
    for _, curve := range getCurves() {
        // This is deliberately instantiated inside the loop so each iteration gets a fresh object
        circuit := commitmentCircuit{X: 3}

        t.Run(
            curve.String(), func(t *testing.T) {
                ccs, err := frontend.Compile(curve.ScalarField(), scs.NewBuilder, &circuit)
                assert.NoError(err)
                assert.NotNil(ccs)

                // NewWitness fails if Compile is called on &circuit first.
                // It works ok if Compile is called after NewWitness.
                w, err := frontend.NewWitness(&circuit, curve.ScalarField())
                if err != nil {
                    t.Fatalf(
                        "frontend.Compile() corrupted circuit for curve %v; err: %v",
                        curve.String(), err
                    )
                }
                assert.NotNil(w)
            },
        )
    }
}

Expected Behavior

The proposed test case doesn't fail. It doesn't matter if NewWitness() is called before or after Compile().

Actual Behavior

The proposed test case fails. NewWitness() must be called before Compile() to avoid gnark panicking.

Possible Fix

Steps to Reproduce

Fork my branch or paste the test above to backend/plonk/plonk_test.go
Run the test, it fails
Change the order in which Compile() and NewWitness() are called
Run the test again, it passes

Context

I was writing a simple application proving and verifying a trivial circuit using Plonk and KZG. I was getting panic() in NewWitness so I played around and I noticed that the problem goes away if I generate private witness before I compile the circuit.

To be frank I'm not 100% sure this is a bug. Another way to avoid the problem is to call Compile() on one instance of the circuit (e.g. on an empty one, like &myCircuitDefinition{}) and NewWitness() on another. On the other hand I don't see a reason Compile() would mutate the object so I guess this may be a bug.

Your Environment

gnark version used: today's master, d11824fa
gnark-crypto version used: v0.12.2-0.20231221171913-5d5eded6bb15
go version: 1.21.5 darwin/arm64
Operating System and version: macOS Sonoma 14.1 (23B2073)

Hi,

Yes, Compile() can mutate the circuit - this happens for example when we use field emulation where we compute the overflows. You should by default instantiate different Circuit instances for compilation and witness assignment.

There is another issue if you use already assigned Circuit for compile and witness assignment. When you have set X=3 by doing circuit := commitmentCircuit{X: 3}, then the compiler can deduce that in this circuit X is a constant and perform different logic (for R1CS we encode the value as a constant and in PLONK we use another selector). Now, if you try to assign witness to the same variable, then witness assignment doesn't work as the variable is optimized out.

If you want to be able to always assign X as a witness, then you should not compile with X set to non-nil value.

For example, see KZG recursion examples:

https://github.com/Consensys/gnark/blob/master/std/commitments/kzg/verifier_test.go#L924-L931 we fix the verification key for recursion. This performs the precomputation ahead of time, reducing the size of the circuit significantly.
https://github.com/Consensys/gnark/blob/master/std/commitments/kzg/verifier_test.go#L351-L358 here we don't, the recursion verification key is not provided at compile time and needs to be provided through witness assignment.

Consensys / gnark