fix(amino): panic when registering types with the same name

tm2/pkg/amino/README.md

@@ -84,8 +84,8 @@ var Package = amino.RegisterPackage(
 )
 ```
 
-You can still override global registrations with local \*amino.Codec state.
-This is used by genproto.P3Context, which may help development while writing
+You can still override global registrations with local `*amino.Codec` state.
+This is used by `genproto.P3Context`, which may help development while writing
 migration scripts.  Feedback welcome in the issues section.
 
 ## Unsupported types

tm2/pkg/amino/amino.go

@@ -888,17 +888,25 @@ func (cdc *Codec) MarshalJSONIndent(o interface{}, prefix, indent string) ([]byt
 // ----------------------------------------
 // Other
 
+// Given amino package `pi`, register it with the global codec.
 // NOTE: do not modify the result.
 func RegisterPackage(pi *pkg.Package) *Package {
 	gcdc.RegisterPackage(pi)
 	return pi
 }
 
+// Create an unregistered amino package with args:
+// - (gopkg string) The Go package path, e.g. "github.com/gnolang/gno/tm2/pkg/std"
+// - (p3pkg string) The (shorter) Proto3 package path (no slashes), e.g. "std"
+// - (dirname string) Package directory this is called from. Typical is to use `amino.GetCallersDirname()`
 func NewPackage(gopkg string, p3pkg string, dirname string) *Package {
 	return pkg.NewPackage(gopkg, p3pkg, dirname)
 }
 
-// NOTE: duplicated in pkg/pkg.go
+// Get caller's package directory.
+// Implementation uses `filepath.Dir(runtime.Caller(1))`.
+// NOTE: duplicated in pkg/pkg.go; given what it does and how,
+// both are probably needed.
 func GetCallersDirname() string {
 	dirname := "" // derive from caller.
 	_, filename, _, ok := runtime.Caller(1)

tm2/pkg/amino/codec_test.go

@@ -3,6 +3,7 @@ package amino_test
 import (
 	"bytes"
 	"encoding/binary"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -236,4 +237,69 @@ func TestCodecSeal(t *testing.T) {
 	assert.Panics(t, func() { cdc.RegisterPackage(tests.Package) })
 }
 
-// XXX Test registering duplicate names or concrete types not in a package.
+func TestDupTypesMustPanic(t *testing.T) {
+	// duplicate types must panic
+	t.Parallel()
+
+	pkg := amino.NewPackage(
+		reflect.TypeOf(SimpleStruct{}).PkgPath(),
+		"amino_test",
+		amino.GetCallersDirname(),
+	)
+	assert.PanicsWithError(
+		t,
+		"type amino_test.SimpleStruct already registered with package",
+		func() {
+			pkg.WithTypes(
+				SimpleStruct{},
+				SimpleStruct{},
+			)
+		})
+}
+
+func TestTypesOutsidePackageMustPanic(t *testing.T) {
+	// adding concrete types from within a different package must panic
+	// (use dependency instead)
+	t.Parallel()
+
+	makepkg := func() *amino.Package {
+		return amino.NewPackage(
+			reflect.TypeOf(tests.EmptyStruct{}).PkgPath(),
+			"amino_test",
+			amino.GetCallersDirname(),
+		)
+	}
+
+	makepkg().WithTypes(tests.PrimitivesStruct{}) // from same package ✓
+
+	assert.Panics(t, func() {
+		makepkg().WithTypes(
+			SimpleStruct{}, // from another package ✗
+		)
+	})
+}
+
+func TestDupNamesMustPanic(t *testing.T) {
+	// adding types with the same names must panic
+	t.Parallel()
+
+	makepkg := func() *amino.Package {
+		return amino.NewPackage(
+			reflect.TypeOf(tests.EmptyStruct{}).PkgPath(),
+			"amino_test",
+			amino.GetCallersDirname(),
+		)
+	}
+	makepkg().WithTypes(
+		tests.EmptyStruct{}, "A",
+		tests.PrimitivesStruct{}, "B",
+		tests.ShortArraysStruct{}, "C",
+	)
+	assert.Panics(t, func() {
+		makepkg().WithTypes(
+			tests.EmptyStruct{}, "A",
+			tests.PrimitivesStruct{}, "B",
+			tests.ShortArraysStruct{}, "A", // Same name!
+		)
+	})
+}

tm2/pkg/amino/pkg/pkg.go

@@ -118,11 +118,24 @@ func (pkg *Package) WithGoPkgName(name string) *Package {
 	return pkg
 }
 
+// Package dependencies need to be declared (for now).
+// If a package has no dependency, it is conventional to
+// use `.WithDependencies()` with no arguments.
 func (pkg *Package) WithDependencies(deps ...*Package) *Package {
 	pkg.Dependencies = append(pkg.Dependencies, deps...)
 	return pkg
 }
 
+// WithType specifies which types are encoded and decoded by the package.
+// You must provide a list of instantiated objects in the arguments.
+// Each type declaration may be optionally followed by a string which is then
+// used as its name.
+//
+//	pkg.WithTypes(
+//		StructA{},
+//		&StructB{}, // If pointer receivers are preferred when decoding to interfaces.
+//		NoInputsError{}, "NoInputsError", // Named
+//	)
 func (pkg *Package) WithTypes(objs ...interface{}) *Package {
 	var lastType *Type = nil
 	for _, obj := range objs {
@@ -183,6 +196,17 @@ func (pkg *Package) WithTypes(objs ...interface{}) *Package {
 		}
 		pkg.Types = append(pkg.Types, lastType)
 	}
+	seen := make(map[string]struct{})
+	for _, tp := range pkg.Types {
+		// tp.Name is "" for cases like nativePkg, containing go native types.
+		if tp.Name == "" {
+			continue
+		}
+		if _, ok := seen[tp.Name]; ok {
+			panic("duplicate type name " + tp.Name)
+		}
+		seen[tp.Name] = struct{}{}
+	}
 	return pkg
 }