A refusal travels up, instead of being written as an empty struct

This projection promises that an op whose value cannot cross gets no method, because a client that compiles and lies is worse than one that says the operation is out of reach. Three ways it broke that promise, all found generating clients for node's ninety-four ops: A field whose own type had no wire form was replaced by struct{} and the op kept its method. The call succeeded and the payload was gone. node's admin, info and platformvm all had []struct{} where a value should be. A field that cannot cross now refuses the type holding it, which refuses the op, reported once at the field. On an EMBEDDED field that substitute is not Go — an embedded field must be a name — so the package did not parse, SDK returned an error, and the service got no client at all. One op of xvm's ten has that shape. And the memo that remembers a refused type answered "no" and said nothing, so the SECOND op reaching it had no method and no gap; the only way to notice was to count. platformvm's get_tx_rewards vanished exactly that way. The refusal is remembered with its reason and reported again against each op that meets it. Two causes join the vocabulary: nested, and embedded. Co-authored-by: Hanzo Dev <[email protected]>
2026-10-19 15:38:57 +00:00 · 2026-09-01 11:58:04 -07:00
parent 158735fcf0
commit fe4bc7dce8
2 changed files with 191 additions and 24 deletions
@@ -79,7 +79,12 @@ func (a *App) SDK(pkg string) (*SDK, error) {
 	if !validIdent(pkg) {
 		return nil, fmt.Errorf("zip: %q is not a Go package name", pkg)
 	}
-	g := &render{sdk: &SDK{Package: pkg}, named: map[reflect.Type]string{}, taken: map[string]bool{}}
+	g := &render{
+		sdk:   &SDK{Package: pkg},
+		named: map[reflect.Type]string{},
+		why:   map[reflect.Type]Gap{},
+		taken: map[string]bool{},
+	}

 	ops := append([]*registeredOp(nil), a.Registry()...)
 	sort.Slice(ops, func(i, j int) bool { return opName(ops[i]) < opName(ops[j]) })
@@ -116,7 +121,12 @@ type call struct {
 type render struct {
 	sdk   *SDK
 	named map[reflect.Type]string // type → the name declared for it, "" while claiming
-	taken map[string]bool         // every name claimed at package scope
+	// why is the refusal remembered for a type that has none. Without it the
+	// SECOND op reaching a refused type is dropped in silence: the memo below
+	// answers "no" and says nothing, so the op has no method and no gap, and
+	// the only way to notice is to count the methods.
+	why   map[reflect.Type]Gap
+	taken map[string]bool // every name claimed at package scope
 	calls []call
 	decls []string
 }
@@ -155,13 +165,17 @@ func (g *render) declare(t reflect.Type, op string, fields map[string]string) (s
 		return "", true
 	}
 	if name, seen := g.named[t]; seen {
-		return name, name != ""
+		if name == "" {
+			// Refused before, by another op. Say so again, against this one.
+			had := g.why[t]
+			g.gap(op, had.Field, had.Go, had.Cause)
+			return "", false
+		}
+		return name, true
 	}
 	shape, err := zapenc.LayoutOf(t)
 	if err != nil {
-		g.named[t] = ""
-		g.gap(op, goName(t), t.String(), causeOf(err))
-		return "", false
+		return "", g.refuse(t, op, goName(t), t.String(), causeOf(err))
 	}
 	// A layout that is right is not the same as a value that crosses. A fixed
 	// array — every id in the fleet — has an offset and a width, so the schema
@@ -177,9 +191,7 @@ func (g *render) declare(t reflect.Type, op string, fields map[string]string) (s
 	// whole answer. It is the same fact [Schema.Coded] reports.
 	for _, s := range shape.Slots {
 		if strings.HasPrefix(s.Type, "bytes_fixed[") || strings.HasPrefix(s.Elem, "bytes_fixed[") {
-			g.named[t] = ""
-			g.gap(op, goName(t)+"."+s.Name, s.Type, causeCodec)
-			return "", false
+			return "", g.refuse(t, op, goName(t)+"."+s.Name, s.Type, causeCodec)
 		}
 	}

@@ -213,10 +225,24 @@ func (g *render) declare(t reflect.Type, op string, fields map[string]string) (s
 		if d := fields[t.Name()+"."+jsonFieldName(f)]; d != "" {
 			prose(&b, "", d)
 		}
-		goType := g.typeOf(f.Type, op, t.Name()+"."+f.Name, fields)
+		at := t.Name() + "." + f.Name
+		goType, ok := g.typeOf(f.Type, op, at, fields)
+		if !ok {
+			// A field that cannot cross refuses the type that holds it, which
+			// refuses the op. Substituting an empty struct here instead — which
+			// is what this did — emitted a client that compiles, calls, and
+			// arrives with the value gone. Reported once, at the field.
+			return "", g.refuse(t, op, at, f.Type.String(), causeNested)
+		}
 		if f.Anonymous {
 			// An embedded field IS its type's name, and the wire cares: it takes
-			// one nested slot, where the flattened form would take several.
+			// one nested slot, where the flattened form would take several. A
+			// spelling that is not a name — the empty struct an unnameable type
+			// renders as — is not Go there, so the type is refused rather than
+			// written out unparseable.
+			if !isIdent(goType) {
+				return "", g.refuse(t, op, at, f.Type.String(), causeEmbedded)
+			}
 			fmt.Fprintf(&b, "\t%s %s\n", goType, tagOf(f))
 			continue
 		}
@@ -230,36 +256,65 @@ func (g *render) declare(t reflect.Type, op string, fields map[string]string) (s
 // typeOf spells one field's Go type in the generated package: the same type it
 // has here, with any struct it reaches renamed to the declaration this package
 // makes for it.
-func (g *render) typeOf(t reflect.Type, op, at string, fields map[string]string) string {
+func (g *render) typeOf(t reflect.Type, op, at string, fields map[string]string) (string, bool) {
 	switch t.Kind() {
 	case reflect.Pointer:
-		return "*" + g.typeOf(t.Elem(), op, at, fields)
+		elem, ok := g.typeOf(t.Elem(), op, at, fields)
+		return "*" + elem, ok
 	case reflect.Slice:
-		return "[]" + g.typeOf(t.Elem(), op, at, fields)
+		elem, ok := g.typeOf(t.Elem(), op, at, fields)
+		return "[]" + elem, ok
 	case reflect.Array:
-		return "[" + strconv.Itoa(t.Len()) + "]" + g.typeOf(t.Elem(), op, at, fields)
+		elem, ok := g.typeOf(t.Elem(), op, at, fields)
+		return "[" + strconv.Itoa(t.Len()) + "]" + elem, ok
 	case reflect.Map:
 		// A map has no layout: a key set is not a set of offsets. LayoutOf has
 		// already refused the struct that holds it, so this only renders where
 		// the map sits behind something the wire never reaches.
-		return "map[" + g.typeOf(t.Key(), op, at, fields) + "]" + g.typeOf(t.Elem(), op, at, fields)
+		key, kok := g.typeOf(t.Key(), op, at, fields)
+		elem, eok := g.typeOf(t.Elem(), op, at, fields)
+		return "map[" + key + "]" + elem, kok && eok
 	case reflect.Struct:
 		if t.NumField() == 0 {
-			return "struct{}"
+			return "struct{}", true
 		}
-		name, ok := g.declare(t, op, fields)
-		if !ok {
-			return "struct{}"
-		}
-		return name
+		return g.declare(t, op, fields)
 	case reflect.Interface:
+		// An interface field names no type, so there is nothing to declare and
+		// nothing that could cross. It refuses the type that holds it.
 		g.gap(op, at, t.String(), CauseAny)
-		return "any"
+		return "", false
 	}
 	// A basic kind keeps its own spelling — including a named one's underlying
 	// type, because the name belongs to the service's package and the width is
 	// what the wire reads.
-	return t.Kind().String()
+	return t.Kind().String(), true
+}
+
+// refuse records why a type has no wire form, remembers it for the next op to
+// reach the same type, and reports false so the caller can stop.
+func (g *render) refuse(t reflect.Type, op, field, goType, cause string) bool {
+	g.named[t] = ""
+	g.why[t] = Gap{Field: field, Go: goType, Cause: cause}
+	g.gap(op, field, goType, cause)
+	return false
+}
+
+// isIdent says whether a spelling can stand where Go wants a type name, which
+// is the only thing an embedded field may be.
+func isIdent(s string) bool {
+	if s == "" {
+		return false
+	}
+	for i, r := range s {
+		switch {
+		case r == '_' || (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z'):
+		case i > 0 && r >= '0' && r <= '9':
+		default:
+			return false
+		}
+	}
+	return true
 }

 // claim reserves one package-scope name, so a type called Client or Dial cannot
@@ -461,6 +516,13 @@ func validIdent(s string) bool {
 const (
 	causeUnnamed = "no name"  // an op whose id spells no Go identifier
 	causeCodec   = "no codec" // a fixed array: the layout is right, reflection refuses it
+	// causeNested is a field whose own type has no wire form. The type holding
+	// it has none either: a struct is its fields, and one that cannot cross
+	// cannot be quietly left out of a value the caller thinks it sent.
+	causeNested = "nested"
+	// causeEmbedded is an embedded field whose type this package cannot name.
+	// Go requires a name there, so there is nothing to write that parses.
+	causeEmbedded = "embedded"
 )

 var goReserved = map[string]bool{
@@ -485,3 +485,108 @@ func TestSDK_OneOperationOneMethodName(t *testing.T) {
 		}
 	}
 }
+
+// An id is a fixed array, which the layout states and reflection refuses. When
+// one sits on an EMBEDDED field the substitute this used to write —
+// "struct{} `json:\"...\"`" — is not Go: an embedded field must be a name. The
+// package did not parse, App.SDK returned an error, and the app got no client
+// at all. Found generating node's xvm service, where one op of ten has it.
+func TestSDK_AnEmbeddedUnnameableTypeIsAGapAndNotASyntaxError(t *testing.T) {
+	// Exported, because an embedded unexported type is on no wire at all: the
+	// renderer and the layout both walk exported fields only, and skip it
+	// together. node's is FormattedAssetID.
+	type AssetID struct {
+		ID [32]byte `json:"id"`
+	}
+	type asset struct {
+		AssetID `json:"assetID"`
+		Name    string `json:"name"`
+	}
+	app := zip.New(zip.Config{AppName: "x", DisableStartupMessage: true})
+	zip.Get(app, "/v1/asset", func(_ context.Context, in *asset) (*asset, error) { return in, nil },
+		zip.WithOperationID("x_asset"))
+	zip.Get(app, "/v1/height", func(_ context.Context, in *Height) (*Height, error) { return in, nil },
+		zip.WithOperationID("x_height"))
+
+	sdk, err := app.SDK("x")
+	if err != nil {
+		t.Fatalf("SDK: %v", err)
+	}
+	if strings.Contains(string(sdk.Source), "struct{} `json") {
+		t.Errorf("an embedded field was written as a type with no name:\n%s", sdk.Source)
+	}
+	if !strings.Contains(string(sdk.Source), "func (c *Client) XHeight") {
+		t.Error("the op that CAN cross lost its method to the one that cannot")
+	}
+	if strings.Contains(string(sdk.Source), "XAsset") {
+		t.Error("the op the wire refuses got a method")
+	}
+	if len(sdk.Gaps) == 0 {
+		t.Fatal("no gap reported for the refused op")
+	}
+}
+
+// A field whose own type cannot cross used to be replaced by an empty struct,
+// and the op kept its method: the call succeeded and the value was gone.
+// That is the "compiles and lies" outcome this projection exists to prevent,
+// and it reached real clients — node's admin, info and platformvm all had
+// []struct{} where a payload should be.
+func TestSDK_ANestedRefusalRefusesTheOp(t *testing.T) {
+	type inner struct {
+		ID [32]byte `json:"id"`
+	}
+	type outer struct {
+		Name  string  `json:"name"`
+		Items []inner `json:"items"`
+	}
+	app := zip.New(zip.Config{AppName: "n", DisableStartupMessage: true})
+	zip.Post(app, "/v1/outer", func(_ context.Context, in *outer) (*outer, error) { return in, nil },
+		zip.WithOperationID("n_outer"))
+
+	sdk, err := app.SDK("n")
+	if err != nil {
+		t.Fatalf("SDK: %v", err)
+	}
+	if sdk.Ops() != 0 {
+		t.Errorf("an op whose payload cannot cross got %d method(s):\n%s", sdk.Ops(), sdk.Source)
+	}
+	if strings.Contains(string(sdk.Source), "[]struct{}") {
+		t.Errorf("a field that cannot cross was written as an empty struct:\n%s", sdk.Source)
+	}
+	if len(sdk.Gaps) == 0 {
+		t.Fatal("no gap reported")
+	}
+}
+
+// Two ops sharing one refused type are two ops with no method, so they are two
+// gaps. The memo that remembers the refusal used to answer "no" and say
+// nothing, so the second op vanished from the package with nothing recorded and
+// the only way to notice was to count the methods. node's platformvm had
+// get_tx_rewards disappear exactly this way.
+func TestSDK_EveryOpRefusedIsAnOpReported(t *testing.T) {
+	type shared struct {
+		ID [32]byte `json:"id"`
+	}
+	app := zip.New(zip.Config{AppName: "s", DisableStartupMessage: true})
+	zip.Get(app, "/v1/one", func(_ context.Context, in *shared) (*shared, error) { return in, nil },
+		zip.WithOperationID("s_one"))
+	zip.Get(app, "/v1/two", func(_ context.Context, in *shared) (*shared, error) { return in, nil },
+		zip.WithOperationID("s_two"))
+
+	sdk, err := app.SDK("s")
+	if err != nil {
+		t.Fatalf("SDK: %v", err)
+	}
+	if sdk.Ops() != 0 {
+		t.Fatalf("got %d method(s), want none:\n%s", sdk.Ops(), sdk.Source)
+	}
+	named := map[string]bool{}
+	for _, g := range sdk.Gaps {
+		named[g.Op] = true
+	}
+	for _, op := range []string{"s_one", "s_two"} {
+		if !named[op] {
+			t.Errorf("%s has no method and no gap; gaps are %+v", op, sdk.Gaps)
+		}
+	}
+}