ccs.GetConstraints readable output
#244
Conversation
| @@ -14,8 +14,10 @@ import ( | |||
| {{ template "import_curve" . }} | |||
| ) | |||
|
|
|||
| // ErrUnsatisfiedConstraint can be generated when solving a R1CS | |||
| var ErrUnsatisfiedConstraint = errors.New("constraint is not satisfied") | |||
| // errUnsatisfiedConstraint can be generated when solving a R1CS | |||
There was a problem hiding this comment.
Previously, as ErrUnsatisfiedConstraint was a instantiated type, it could be checked using errors.Is. Now, it is essentially a utility function to call fmt.Errorf. When looking at the places it is called, it makes difficult to understand where the error actually happened (grepping leads here, not to the actual place the error happened).
Maybe remove this function and construct new errors where necessary? If it is necessary to assert this error, then a conventional way would be to define this as a type (satisfying the error interface), and use errors.As for assertion.
| @@ -103,9 +102,9 @@ func (cs *R1CS) Solve(witness, a, b, c []fr.Element, opt backend.ProverConfig) ( | |||
| if !check.Equal(&c[i]) { | |||
| if dID, ok := cs.MDebug[i]; ok { | |||
| debugInfoStr := solution.logValue(cs.DebugInfo[dID]) | |||
| return solution.values, fmt.Errorf("%w: %s", ErrUnsatisfiedConstraint, debugInfoStr) | |||
| return solution.values, fmt.Errorf("%w: %s", errUnsatisfiedConstraint(i), debugInfoStr) | |||
There was a problem hiding this comment.
As the error is not assertable anymore, then it doesn't make sense to wrap the error with "%w" anymore. Imo internally it returns a struct which contains the wrapped error and the message, but this overhead doesn't matter as the current function returns early.
| @@ -99,9 +99,15 @@ func (cs *SparseR1CS) Solve(witness []fr.Element, opt backend.ProverConfig) ([]f | |||
| if err := cs.checkConstraint(cs.Constraints[i], &solution); err != nil { | |||
| if dID, ok := cs.MDebug[i]; ok { | |||
| debugInfoStr := solution.logValue(cs.DebugInfo[dID]) | |||
| return solution.values, fmt.Errorf("%w: %s", ErrUnsatisfiedConstraint, debugInfoStr) | |||
| if debug.Debug { | |||
There was a problem hiding this comment.
See the same comments as for r1cs.go.tmpl
| // [1] = qR⋅xb | ||
| // [2] = qO⋅xc | ||
| // [3] = qM⋅(xaxb) | ||
| // [4] = qC | ||
| func (cs *SparseR1CS) GetConstraints() [][]string { | ||
| var r [][]string |
There was a problem hiding this comment.
We know the number of returned strings ahead of time. We could allocate the whole slice to reduce the number of allocations in the loop.
| sbb.WriteByte('0') | ||
| return sbb.String() | ||
|
|
||
| sbb.Reset() |
There was a problem hiding this comment.
Builder is already initialized.
internal/backend/circuits/cmp.go
Outdated
| @@ -55,5 +55,5 @@ func init() { | |||
| }, | |||
| } | |||
|
|
|||
| addNewEntry("cmp", &cmpCircuit{}, good, bad, []ecc.ID{ecc.BW6_761}) | |||
| addNewEntry("cmp", &cmpCircuit{}, good, bad, []ecc.ID{ecc.BN254}) | |||
There was a problem hiding this comment.
Is there a reason for changing the curve for this test? Why not ecc.Implemented()?
There was a problem hiding this comment.
I suspect we missed it in PR #233 --> I like to run go test -short and with BW6 hard coded here, it takes 10x more time :)
This PR removes the
ccs.ToHTMLdeadcode. It's notgnarkjob to produce HTML, the constraint system returns a list of formatted constraints in a[][]stringand a user (like play.gnark.io ) can then render it.ccs.GetConstraintsreturns for PlonK the constraints in the formqL⋅xa + qR⋅xb + qO⋅xc + qM⋅(xaxb) + qC == 0(as in the paper) and for Groth16L⋅R == O.Additionally, this PR adds a constraint ID number in the message for a unsatisfied constraint at solving time.