fix(schema)!: fixes and key restrictions based on indexer testing (#2…

…1362)

schema/kind.go

@@ -79,6 +79,8 @@ const (
 	// Go Encoding: string which matches the IntegerFormat regex
 	// JSON Encoding: base10 integer string
 	// Canonically encoded values should include no leading zeros.
+	// Equality comparison with integers should be done using numerical equality rather
+	// than string equality.
 	IntegerStringKind
 
 	// DecimalStringKind represents an arbitrary precision decimal or integer number.
@@ -87,6 +89,8 @@ const (
 	// Canonically encoded values should include no leading zeros or trailing zeros,
 	// and exponential notation with a lowercase 'e' should be used for any numbers
 	// with an absolute value less than or equal to 1e-6 or greater than or equal to 1e6.
+	// Equality comparison with decimals should be done using numerical equality rather
+	// than string equality.
 	DecimalStringKind
 
 	// BoolKind represents a boolean true or false value.
@@ -369,6 +373,18 @@ func (t Kind) ValidateValue(value interface{}) error {
 	return nil
 }
 
+// ValidKeyKind returns true if the kind is a valid key kind.
+// All kinds except Float32Kind, Float64Kind, and JSONKind are valid key kinds
+// because they do not define a strict form of equality.
+func (t Kind) ValidKeyKind() bool {
+	switch t {
+	case Float32Kind, Float64Kind, JSONKind:
+		return false
+	default:
+		return true
+	}
+}
+
 var (
 	integerRegex = regexp.MustCompile(IntegerFormat)
 	decimalRegex = regexp.MustCompile(DecimalFormat)

schema/object_type.go

@@ -11,7 +11,9 @@ type ObjectType struct {
 	// KeyFields is a list of fields that make up the primary key of the object.
 	// It can be empty in which case indexers should assume that this object is
 	// a singleton and only has one value. Field names must be unique within the
-	// object between both key and value fields. Key fields CANNOT be nullable.
+	// object between both key and value fields.
+	// Key fields CANNOT be nullable and Float32Kind, Float64Kind, and JSONKind types
+	// are not allowed.
 	KeyFields []Field
 
 	// ValueFields is a list of fields that are not part of the primary key of the object.
@@ -53,6 +55,10 @@ func (o ObjectType) validate(types map[string]Type) error {
 			return fmt.Errorf("invalid key field %q: %v", field.Name, err) //nolint:errorlint // false positive due to using go1.12
 		}
 
+		if !field.Kind.ValidKeyKind() {
+			return fmt.Errorf("key field %q of kind %q uses an invalid key field kind", field.Name, field.Kind)
+		}
+
 		if field.Nullable {
 			return fmt.Errorf("key field %q cannot be nullable", field.Name)
 		}

schema/object_type_test.go

@@ -189,6 +189,45 @@ func TestObjectType_Validate(t *testing.T) {
 			},
 			errContains: "enum \"enum1\" has different values",
 		},
+		{
+			name: "float32 key field",
+			objectType: ObjectType{
+				Name: "o1",
+				KeyFields: []Field{
+					{
+						Name: "field1",
+						Kind: Float32Kind,
+					},
+				},
+			},
+			errContains: "invalid key field kind",
+		},
+		{
+			name: "float64 key field",
+			objectType: ObjectType{
+				Name: "o1",
+				KeyFields: []Field{
+					{
+						Name: "field1",
+						Kind: Float64Kind,
+					},
+				},
+			},
+			errContains: "invalid key field kind",
+		},
+		{
+			name: "json key field",
+			objectType: ObjectType{
+				Name: "o1",
+				KeyFields: []Field{
+					{
+						Name: "field1",
+						Kind: JSONKind,
+					},
+				},
+			},
+			errContains: "invalid key field kind",
+		},
 	}
 
 	for _, tt := range tests {

schema/testing/appdatasim/app_data.go

@@ -8,6 +8,7 @@ import (
 
 	"cosmossdk.io/schema"
 	"cosmossdk.io/schema/appdata"
+	schematesting "cosmossdk.io/schema/testing"
 	"cosmossdk.io/schema/testing/statesim"
 	"cosmossdk.io/schema/view"
 )
@@ -92,10 +93,10 @@ func (a *Simulator) BlockDataGenN(minUpdatesPerBlock, maxUpdatesPerBlock int) *r
 		packets = append(packets, appdata.StartBlockData{Height: a.blockNum + 1})
 
 		updateSet := map[string]bool{}
-		// filter out any updates to the same key from this block, otherwise we can end up with weird errors
+		// filter out any updates to the same key from this block, otherwise we can end up with hard to debug errors
 		updateGen := a.state.UpdateGen().Filter(func(data appdata.ObjectUpdateData) bool {
 			for _, update := range data.Updates {
-				_, existing := updateSet[fmt.Sprintf("%s:%v", data.ModuleName, update.Key)]
+				_, existing := updateSet[a.formatUpdateKey(data.ModuleName, update)]
 				if existing {
 					return false
 				}
@@ -106,7 +107,8 @@ func (a *Simulator) BlockDataGenN(minUpdatesPerBlock, maxUpdatesPerBlock int) *r
 		for i := 0; i < numUpdates; i++ {
 			data := updateGen.Draw(t, fmt.Sprintf("update[%d]", i))
 			for _, update := range data.Updates {
-				updateSet[fmt.Sprintf("%s:%v", data.ModuleName, update.Key)] = true
+				// we need to set the update here each time so that this is used to filter out duplicates in the next round
+				updateSet[a.formatUpdateKey(data.ModuleName, update)] = true
 			}
 			packets = append(packets, data)
 		}
@@ -117,6 +119,21 @@ func (a *Simulator) BlockDataGenN(minUpdatesPerBlock, maxUpdatesPerBlock int) *r
 	})
 }
 
+func (a *Simulator) formatUpdateKey(moduleName string, update schema.ObjectUpdate) string {
+	mod, err := a.state.GetModule(moduleName)
+	if err != nil {
+		panic(err)
+	}
+
+	objColl, err := mod.GetObjectCollection(update.TypeName)
+	if err != nil {
+		panic(err)
+	}
+
+	ks := fmt.Sprintf("%s:%s:%s", moduleName, update.TypeName, schematesting.ObjectKeyString(objColl.ObjectType(), update.Key))
+	return ks
+}
+
 // ProcessBlockData processes the given block data, advancing the app state based on the object updates in the block
 // and forwarding all packets to the attached listener. It is expected that the data passed came from BlockDataGen,
 // however, other data can be passed as long as any StartBlockData packet has the height set to the current height + 1.