feat: added frontend/schema to dynamically create circuit/witness Schema

[gbotrel]

This PR uses new APIs introduced in Go 1.17 to clean the circuit structure parser.

This parser moved from internal/parser to frontend/schema .

Additionally, this parser now returns a Schema struct when walking through a circuit.

A Schema can then be instantiated into a typed object using reflect.StructOf and reflect.StructFields[] . See schema_test.go for examples.

This PR is necessary for witness.ToJSON / FromJSON.

• feat: added frontend/schema to build circuit and witness schemas

• fix: remove embbed struct tag

• Will clean up the error returns from the parser

[gbotrel]

@ThomasPiellard / @ivokub I think latest refactor removed some tests on struct tags, can't find them. (I believe they were in frontend/)

[ivokub]

Suggested edit:

diff --git a/backend/witness/witness.go b/backend/witness/witness.go
index 91082ce8..286a656e 100644
--- a/backend/witness/witness.go
+++ b/backend/witness/witness.go
@@ -198,14 +198,7 @@ func WriteSequence(w io.Writer, circuit frontend.Circuit) error {
 // If it can't fully re-construct the witness from the reader, returns an error
 // if the provided witness has 0 public Variables this function returns 0, nil
 func ReadPublicFrom(r io.Reader, curveID ecc.ID, witness frontend.Circuit) (int64, error) {
-	nbPublic := 0
-	collectHandler := func(visibility compiled.Visibility, name string, tInput reflect.Value) error {
-		if visibility == compiled.Public {
-			nbPublic++
-		}
-		return nil
-	}
-	_, _ = schema.Parse(witness, tVariable, collectHandler)
+	_, nbPublic := schema.Count(witness, tVariable)
 
 	if nbPublic == 0 {
 		return 0, nil
@@ -255,17 +248,7 @@ func ReadPublicFrom(r io.Reader, curveID ecc.ID, witness frontend.Circuit) (int6
 // If it can't fully re-construct the witness from the reader, returns an error
 // if the provided witness has 0 public Variables and 0 secret Variables this function returns 0, nil
 func ReadFullFrom(r io.Reader, curveID ecc.ID, witness frontend.Circuit) (int64, error) {
-	nbPublic := 0
-	nbSecrets := 0
-	collectHandler := func(visibility compiled.Visibility, name string, tInput reflect.Value) error {
-		if visibility == compiled.Public {
-			nbPublic++
-		} else if visibility == compiled.Secret {
-			nbSecrets++
-		}
-		return nil
-	}
-	_, _ = schema.Parse(witness, tVariable, collectHandler)
+	nbSecrets, nbPublic := schema.Count(witness, tVariable)
 
 	if nbPublic == 0 && nbSecrets == 0 {
 		return 0, nil
diff --git a/frontend/schema/schema.go b/frontend/schema/schema.go
index 9232adf0..fec6d637 100644
--- a/frontend/schema/schema.go
+++ b/frontend/schema/schema.go
@@ -214,7 +214,7 @@ func parse(r []Field, input interface{}, target reflect.Type, parentName, parent
 	if tValue.Kind() == reflect.Slice || tValue.Kind() == reflect.Array {
 		if tValue.Len() == 0 {
 			if reflect.SliceOf(target) == tValue.Type() {
-				fmt.Printf("ignoring unitizalized slice: %s %s\n", parentName, reflect.SliceOf(target).String())
+				fmt.Printf("ignoring uninitizalized slice: %s %s\n", parentName, reflect.SliceOf(target).String())
 			}
 			return r, nil
 		}

[ivokub]

In every invocation of schema.Parse the argument tLeaf is set to tVariable which is runtime reflection of frontend.Variable type. Maybe we can omit the argument (as here Parse internally calls parse, where it makes more sense to have the ability to change the leaf type filter) here? I see that it creates import cycles, but maybe reflection can help? By looking at the names of struct field type?

Also, handler is always set in usage. Make it an argument instead of keeping it as vararg? And if nil, then is not called.

[gbotrel]

no handler is not set when you just want the schema (for now, only in schema_test.go) . We can put it as arg and check if it's nil to not call it, works well too.

For the tLeaf, indeed, it's to avoid import cycles, (same as for why circuit is interface{} and not frontend.Circuit)

[ivokub]

Also - would it make sense to separate Go dependency upgrade (min 1.16 to 1.17) to separate PR?

[ivokub]

@ThomasPiellard / @ivokub I think latest refactor removed some tests on struct tags, can't find them. (I believe they were in frontend/)

I think I found it. Do you mean ce768f5#diff-1ce0aba23ab0f7f8bbf288be3a45b5bda4c1231ffbf3e1475f6d80b4d10be747?

[ivokub]

Suggested edit:

diff --git a/frontend/variable_test.go b/frontend/variable_test.go
new file mode 100644
index 00000000..206d3943
--- /dev/null
+++ b/frontend/variable_test.go
@@ -0,0 +1,166 @@
+package frontend
+
+import (
+	"errors"
+	"fmt"
+	"reflect"
+	"runtime/debug"
+	"testing"
+
+	"github.com/consensys/gnark/frontend/schema"
+	"github.com/consensys/gnark/internal/backend/compiled"
+)
+
+func TestStructTags(t *testing.T) {
+
+	testParseType := func(input interface{}, expected map[string]compiled.Visibility) {
+		collected := make(map[string]compiled.Visibility)
+		var collectHandler schema.LeafHandler = func(visibility compiled.Visibility, name string, tInput reflect.Value) error {
+			if _, ok := collected[name]; ok {
+				return errors.New("duplicate name collected")
+			}
+			collected[name] = visibility
+			return nil
+		}
+		if _, err := schema.Parse(input, tVariable, collectHandler); err != nil {
+			t.Log(string(debug.Stack()))
+			t.Fatal(err)
+		}
+
+		for k, v := range expected {
+			if v2, ok := collected[k]; !ok {
+				fmt.Println(collected)
+				t.Fatal("failed to collect", k)
+			} else if v2 != v {
+				t.Fatal("collected", k, "but visibility is wrong got", v2, "expected", v)
+			}
+			delete(collected, k)
+		}
+		if len(collected) != 0 {
+			t.Fatal("collected more Variable than expected")
+		}
+
+	}
+
+	{
+		s := struct {
+			A, B Variable
+		}{}
+		expected := make(map[string]compiled.Visibility)
+		expected["A"] = compiled.Secret
+		expected["B"] = compiled.Secret
+		testParseType(&s, expected)
+	}
+
+	{
+		s := struct {
+			A Variable `gnark:"a, public"`
+			B Variable
+		}{}
+		expected := make(map[string]compiled.Visibility)
+		expected["a"] = compiled.Public
+		expected["B"] = compiled.Secret
+		testParseType(&s, expected)
+	}
+
+	{
+		s := struct {
+			A Variable `gnark:",public"`
+			B Variable
+		}{}
+		expected := make(map[string]compiled.Visibility)
+		expected["A"] = compiled.Public
+		expected["B"] = compiled.Secret
+		testParseType(&s, expected)
+	}
+
+	{
+		s := struct {
+			A Variable `gnark:"-"`
+			B Variable
+		}{}
+		expected := make(map[string]compiled.Visibility)
+		expected["B"] = compiled.Secret
+		testParseType(&s, expected)
+	}
+
+	{
+		s := struct {
+			A Variable `gnark:",public"`
+			B Variable
+			C struct {
+				D Variable
+			}
+		}{}
+		expected := make(map[string]compiled.Visibility)
+		expected["A"] = compiled.Public
+		expected["B"] = compiled.Secret
+		expected["C_D"] = compiled.Secret
+		testParseType(&s, expected)
+	}
+
+	// hierarchy of structs
+	{
+		type grandchild struct {
+			D Variable `gnark:"grandchild"`
+		}
+		type child struct {
+			D Variable `gnark:",public"` // parent visibility is secret so public here is ignored
+			G grandchild
+		}
+		s := struct {
+			A Variable `gnark:",public"`
+			B Variable
+			C child
+		}{}
+		expected := make(map[string]compiled.Visibility)
+		expected["A"] = compiled.Public
+		expected["B"] = compiled.Secret
+		expected["C_D"] = compiled.Secret
+		expected["C_G_grandchild"] = compiled.Secret
+		testParseType(&s, expected)
+	}
+
+	// ignore embedded structs (not exported)
+	{
+		type embedded struct {
+			D Variable
+		}
+		type child struct {
+			embedded // this is not exported and ignored
+		}
+
+		s := struct {
+			C child
+			A Variable `gnark:",public"`
+			B Variable
+		}{}
+		expected := make(map[string]compiled.Visibility)
+		expected["A"] = compiled.Public
+		expected["B"] = compiled.Secret
+		testParseType(&s, expected)
+	}
+
+	// array
+	{
+		s := struct {
+			A [2]Variable `gnark:",public"`
+		}{}
+		expected := make(map[string]compiled.Visibility)
+		expected["A_0"] = compiled.Public
+		expected["A_1"] = compiled.Public
+		testParseType(&s, expected)
+	}
+
+	// slice
+	{
+		s := struct {
+			A []Variable `gnark:",public"`
+		}{A: make([]Variable, 2)}
+		expected := make(map[string]compiled.Visibility)
+		expected["A_0"] = compiled.Public
+		expected["A_1"] = compiled.Public
+		testParseType(&s, expected)
+	}
+
+}