bug: test/Assert (ProverSucceeded, ...) may incorrectly cache a compiled circuit

[gbotrel]

To speed up testing, and avoid recompiling the same circuit when testing against several witnesses, test/Assert have a cache of compiled circuits.
However, the key for the circuits is too weak;

key := curveID.String() + backendID.String() + reflect.TypeOf(circuit).String()

If you consider the case where a circuit is define like so;

type MyCircuit struct {
A frontend.Variable
somethingMutable XXX
}

were somethingMutable could be a hash function or a twisted edwards curve ID, which alters the Define behavior. Then the 2nd compilation after changing this variable will not be reflected.

[ivokub]

There is Run method on assert which takes additional description arguments (for running as subtests). Maybe an easy fix would be to include the subtest name in the cache key?

[gbotrel]

I don't think it'll work ; subtests name are derived from backendID and curveID.
So same problem applies ; a same circuit compile with same backend and curve, but with a behavior that changes will not be correctly added into the cache.

Adding the pointer (unsafe) in the key works, but, I'm also aiming for adding a t.Parallel in the assert.Run, WIP 👍

[ivokub]

I don't think it'll work ; subtests name are derived from backendID and curveID. So same problem applies ; a same circuit compile with same backend and curve, but with a behavior that changes will not be correctly added into the cache.

In general, it is possible to define any additional string context for the subtest (https://pkg.go.dev/github.com/consensys/gnark@v0.6.4/test#Assert.Run), but yes, currently in the most of the cases the context is just the curve and backend.