fix(gnovm): embedded type identity — struct field name + interface method-set flatten

gnovm/adr/pr5739_interface_method_set_flattening.md

@@ -0,0 +1,173 @@
+# Interface method-set flattening and cross-package unexported-method identity
+
+## Context
+
+GnoVM derives an anonymous interface's `TypeID` from its method list. Before
+this change, an *embedded* interface was kept as a single named entry in
+`InterfaceType.Methods` (e.g. `interface{ Stringer }` stored a method-list
+entry named `Stringer` whose type was the embedded interface). That made the
+embedded-interface *name* — including an alias spelling — part of the
+`TypeID`, so GnoVM diverged from Go:
+
+- `interface{ Stringer }` was not identical to `interface{ Str() string }`.
+- `interface{ SAlias }` (where `type SAlias = Stringer`) was not identical to
+  `interface{ Stringer }`.
+
+**Precise behavior on master (verified):** master named the embed entry from the
+*resolved* type — `fillEmbeddedName` does `case *DeclaredType: ft.Name = ct.Name`
+— and never flattened. So the two divergences are not symmetric:
+
+| comparison | master | Go |
+|---|---|---|
+| `interface{ SAlias }` vs `interface{ Stringer }` | **identical** (alias resolves to the same `Stringer` entry name) | identical |
+| `interface{ Stringer }` vs `interface{ Str() string }` | **distinct** (entry named `Stringer` vs method `Str`) | identical |
+
+So on master the alias case was already correct; only embed-vs-explicit
+diverged. No naming policy — resolved or spelled — can fix embed-vs-explicit,
+because the embed is still one named entry rather than its methods. Only
+flattening removes the embed name from identity.
+
+Go computes interface identity from the **flattened method set**; embedding
+contributes methods, not a name. PR #5739 therefore flattens embedded
+interfaces into their constituent methods at type construction
+(`flattenInterfaceMethods`, called from `doOpInterfaceType` and
+`staticTypeFromAST`), so the embed/alias spelling no longer leaks into the
+`TypeID`.
+
+## The cross-package unexported-method problem
+
+Flattening exposed a latent representational gap. An unexported interface
+method's identity in Go is `(pkgpath, name)` — `p.sec` and `q.sec` are
+distinct methods, and a type outside `p` cannot satisfy an interface
+containing `p.sec` (the "sealed interface" pattern). GnoVM encodes that
+package qualification **once**, on the containing `InterfaceType.PkgPath`,
+used by `FieldTypeList.TypeIDForPackage` and by interface-satisfaction gating
+in `VerifyImplementedBy` / `FindEmbeddedFieldType`.
+
+When flattening hoists an unexported method out of an interface defined in
+package `P` into an anonymous interface in package `Q`, that single-pkgpath
+encoding re-qualifies the method to `Q`. A `/challenge` pass reproduced two
+failures (both regressions vs the baseline before this change, confirmed against a
+real-Go oracle):
+
+1. **Identity over-collapse.** `interface{ p.Sec }` (with unexported `p.sec`)
+   became identical to `interface{ sec() int }` declared in `q` — Go treats
+   them as distinct.
+2. **Satisfaction bypass (security-relevant).** A type declared in `q` with a
+   `sec()` method was accepted as satisfying `p.Sec`, bypassing the sealed-
+   interface mechanism. Go correctly rejects it.
+
+Over-collapse is the dangerous direction (type confusion / access-control
+bypass), and it is consensus-relevant in a VM.
+
+## Decision
+
+Make flattening fully Go-faithful by recording each method's **origin
+package** alongside the method, instead of relying on the enclosing
+interface's single `PkgPath`:
+
+- Add `FieldType.PkgPath` — the defining package of an unexported method
+  (empty for exported methods and for legacy/non-flattened entries, which
+  fall back to the enclosing interface's `PkgPath`).
+- `flattenInterfaceMethods` stamps the origin package on each hoisted (and
+  directly-declared) unexported method, and deduplicates on `(pkgpath, name)`
+  so that two same-named unexported methods from different packages coexist.
+- `FieldTypeList.Less` and `FieldTypeList.TypeIDForPackage` key/qualify on the
+  per-method `PkgPath` when set.
+- `VerifyImplementedBy` gates unexported access against the method's origin
+  package, not the enclosing interface's package.
+
+This is fully faithful to Go, including the pathological case of two distinct
+same-package sealed interfaces with identical method sets.
+
+### Cost
+
+`FieldType` is amino-serialized (`gnolang.proto` / `pb3_gen.go`), so adding
+`PkgPath` changes the persisted shape of the type representation — a
+serialization-format change requiring a coordinated chain upgrade, on top of
+the `TypeID` changes flattening already implies. The new field is appended
+(proto field 5) and defaults empty, so legacy-decoded entries behave exactly
+as before.
+
+## Alternatives considered
+
+### Alternative A — do not hoist unsafe embeds (rejected, recorded for trace)
+
+Flatten only when identity is preserved: hoist exported methods and
+unexported methods whose origin package equals the enclosing interface; for an
+embed carrying cross-package unexported methods, keep it as a sub-interface
+entry and reuse the existing recursive satisfaction path.
+
+- **Pro:** sound (no over-collapse, no bypass), small diff, **no serialization
+  change** — `FieldType` shape is untouched.
+- **Con:** conservative — it *over-distinguishes* in one case: two distinct
+  sealed interfaces *in the same package* with identical method sets, embedded
+  from a *third* package, are treated as distinct where Go treats them as
+  identical. Over-distinction is the safe direction (a legitimate equality is
+  missed; satisfaction can never be forged), deterministic across nodes, and
+  the triggering case is exotic.
+
+This was rejected only because we chose full Go-fidelity over avoiding the
+serialization change. If the serialization/migration cost is later judged not
+worth the pathological case, Alternative A is the drop-in fallback: it closes
+the same security hole with no persisted-shape change. This ADR exists so that
+trade-off can be revisited.
+
+### Alternative B — keep the embed-entry representation (rejected)
+
+Name embedded interfaces from the resolved type (the minimal fix that shipped
+first on this branch). Sound, but only equalizes alias-vs-target; leaves
+`interface{ Stringer } != interface{ Str() string }` diverging from Go.
+
+## Consequences
+
+- Interface identity matches Go across embedding, aliasing, multi-level,
+  diamond, order, mixed embed+direct, and cross-package (exported and
+  unexported) method sets.
+- Consensus-breaking: `TypeID`s of anonymous interfaces that embed other
+  interfaces change, and the `FieldType` serialization gains a field. Must
+  land with a chain upgrade (same release class as #5737: coordinated
+  upgrade / fresh genesis).
+- Runtime **assumes flattened** interfaces. The legacy embedded-interface
+  branches in `FindEmbeddedFieldType` / `VerifyImplementedBy` are dropped; an
+  `InterfaceKind` entry in `Methods` (only possible by decoding
+  bytes persisted before this change) is a **hard error** (`panicUnflattened`),
+  enforced where the concern lives: ungated at the **decode boundary**
+  (`fillType`, reached from both type-entry decode and object loads — store
+  bytes are external input, so this check stays in production), and under
+  `-tags debugAssert` at the interior sites (method resolution, satisfaction,
+  `TypeID`), which may assume the invariant on a validated store.
+
+## Rollout: state persisted before this change is unsupported (decided)
+
+Every `InterfaceType` is born one of three ways: **preprocess (AST)**,
+**runtime (`doOpInterfaceType`)**, or **decode**. The first two always flatten;
+decode faithfully reproduces stored bytes. So an unflattened interface can
+reach runtime in exactly one situation: decoding bytes persisted **before
+this change**.
+
+Tolerating those bytes (the recursion branches this PR originally kept) was a
+half-measure: the type's *identity* already moved with this change, so
+resolution/satisfaction would "work" against a type whose equality, type-keyed
+store entries, and hashes are silently split from post-change construction.
+Silent-wrong loses to loud-fail on a chain, so the branches are **dropped**
+and a decoded unflattened interface **panics** with an actionable message.
+
+A network carrying state persisted before this change has two supported paths, both of which
+make unflattened bytes impossible (the panic is then a dead invariant check):
+
+- **Fresh genesis / tx replay** (how gno.land testnets regenesis): all state
+  is reconstructed through the new VM, flattened by construction. The only
+  pre-fork audit is source acceptance — historical txs must still preprocess
+  (this PR tightens one case: cross-package unexported-method selection).
+- **Re-flatten migration** (only if in-place state must survive): walk the
+  type table, flatten every `InterfaceType` (stamping origin `PkgPath`),
+  remap old→new `TypeID`s in all object refs, merge newly-colliding entries,
+  and verify no `InterfaceKind` entry remains.
+
+Conflict handling (same-name, different-signature embedded methods) needs no
+follow-up: `flattenInterfaceMethods` `panic`s, but during preprocessing that is
+wrapped into a positioned `*PreprocessError` (the same idiom as other type
+errors, e.g. "struct has no field"), so it surfaces as a matchable `// Error:`.
+go/types also rejects it as a `// TypeCheckError:`. Both are pinned by
+`iface_embed_conflict.gno`, matching Go's "duplicate method" compile error.

gnovm/pkg/gnolang/gnolang.proto

@@ -596,6 +596,7 @@ message FieldType {
 	google.protobuf.Any type = 2 [json_name = "Type"];
 	bool embedded = 3 [json_name = "Embedded"];
 	string tag = 4 [json_name = "Tag"];
+	string pkg_path = 5 [json_name = "PkgPath"];
 }
 
 message FuncType {

gnovm/pkg/gnolang/op_types.go

@@ -116,9 +116,9 @@ func (m *Machine) doOpStructType() {
 	// allocate (minimum) space for fields
 	fields := make([]FieldType, 0, len(x.Fields))
 	// populate fields
-	for _, ftv := range ftvs {
+	for i, ftv := range ftvs {
 		ft := ftv.V.(TypeValue).Type.(FieldType)
-		fillEmbeddedName(&ft)
+		fillEmbeddedName(&ft, x.Fields[i].Type)
 		fields = append(fields, ft)
 	}
 	// push struct type
@@ -142,9 +142,11 @@ func (m *Machine) doOpInterfaceType() {
 	// pop methods
 	for i := len(x.Methods) - 1; 0 <= i; i-- {
 		ft := m.PopValue().V.(TypeValue).Type.(FieldType)
-		fillEmbeddedName(&ft)
 		methods[i] = ft
 	}
+	// flatten embedded interfaces so identity is the method set, not the
+	// embedded-interface (alias) spelling; see flattenInterfaceMethods.
+	methods = flattenInterfaceMethods(methods, m.Package.PkgPath)
 	// push interface type
 	it := &InterfaceType{
 		PkgPath: m.Package.PkgPath,

gnovm/pkg/gnolang/parity_test.go

@@ -163,5 +163,22 @@ func parityCasesGnolang() []struct {
 
 		// SliceValue referring to no backing Value.
 		{"SliceValue/empty", &SliceValue{Base: nil, Offset: 0, Length: 0, Maxcap: 0}},
+
+		// FieldType.PkgPath (wire field 5) — set on an unexported method that
+		// was flattened out of an interface in package "p". Guards the
+		// genproto2/reflect parity of that field: without the hand-added
+		// field-5 marshal in pb3_gen.go, the reflect codec emits PkgPath while
+		// the fast path drops it, and this case fails.
+		{"FieldType/unexported-pkgpath", &FieldType{Name: "sec", PkgPath: "p", Type: &FuncType{}}},
+		{"FieldType/exported-empty-pkgpath", &FieldType{Name: "M", Type: &FuncType{}}},
+		// InterfaceType carrying both an exported (empty PkgPath) and a
+		// stamped unexported method — the realistic persisted shape.
+		{"InterfaceType/stamped-unexported", &InterfaceType{
+			PkgPath: "q",
+			Methods: []FieldType{
+				{Name: "M", Type: &FuncType{}},
+				{Name: "sec", PkgPath: "p", Type: &FuncType{}},
+			},
+		}},
 	}
 }

gnovm/pkg/gnolang/preprocess.go

@@ -4153,9 +4153,13 @@ func staticTypeFromAST(store Store, last BlockNode, x Expr) (Type, bool) {
 		validateStructFields(st, "<anonymous struct>")
 		return st, true
 	case *InterfaceTypeExpr:
+		pkgPath := packageOf(last).PkgPath
 		it := &InterfaceType{
-			PkgPath: packageOf(last).PkgPath,
-			Methods: buildFieldTypesAST(store, last, x.Methods, true),
+			PkgPath: pkgPath,
+			// Build without embed naming (embed=false): flattenInterfaceMethods
+			// expands embedded interfaces into their method set, making identity
+			// the method set rather than the embedded-interface (alias) spelling.
+			Methods: flattenInterfaceMethods(buildFieldTypesAST(store, last, x.Methods, false), pkgPath),
 			Generic: x.Generic,
 		}
 		validateEmbedDepth(it, "<anonymous interface>")
@@ -4167,8 +4171,9 @@ func staticTypeFromAST(store Store, last BlockNode, x Expr) (Type, bool) {
 
 // buildFieldTypesAST constructs a []FieldType directly from FieldTypeExprs,
 // mirroring doOpFieldType + doOp{Struct,Interface,Func}Type aggregation.
-// embed=true mirrors doOpStructType/doOpInterfaceType which call
-// fillEmbeddedName per field; embed=false mirrors doOpFuncType which does not.
+// embed=true mirrors doOpStructType, which names embedded fields per field;
+// embed=false mirrors doOpFuncType and the interface path (which leaves embeds
+// unnamed and flattens them via flattenInterfaceMethods).
 func buildFieldTypesAST(store Store, last BlockNode, fxs FieldTypeExprs, embed bool) []FieldType {
 	fts := make([]FieldType, len(fxs))
 	for i := range fxs {
@@ -4181,7 +4186,7 @@ func buildFieldTypesAST(store Store, last BlockNode, fxs FieldTypeExprs, embed b
 			ft.Tag = Tag(evalConst(store, last, fx.Tag).GetString())
 		}
 		if embed {
-			fillEmbeddedName(&ft)
+			fillEmbeddedName(&ft, fx.Type)
 		}
 		fts[i] = ft
 	}

gnovm/pkg/gnolang/realm.go

@@ -1509,6 +1509,7 @@ func copyFieldsWithRefs(fields []FieldType) []FieldType {
 			Type:     refOrCopyType(field.Type),
 			Embedded: field.Embedded,
 			Tag:      field.Tag,
+			PkgPath:  field.PkgPath,
 		}
 	}
 	return fieldsCpy
@@ -1819,6 +1820,12 @@ func fillType(store Store, typ Type) Type {
 	case *InterfaceType:
 		for i, mthd := range ct.Methods {
 			ct.Methods[i].Type = fillType(store, mthd.Type)
+			// An embed entry means the bytes predate interface flattening
+			// (unsupported state); reject at this decode boundary, which
+			// sees every stored type. See panicUnflattened.
+			if ct.Methods[i].Type.Kind() == InterfaceKind {
+				ct.panicUnflattened(ct.Methods[i])
+			}
 		}
 		return ct
 	case *TypeType: