fix(orm)!: timestamp encoding doesn't handle nil values properly (cos…

…mos#12273)

Co-authored-by: Julien Robert <julien@rbrt.fr>

orm/CHANGELOG.md

@@ -36,6 +36,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
-### API-Breaking Changes
+### API Breaking Changes
 
-- [14822](https://github.com/cosmos/cosmos-sdk/pull/14822) Migrate to cosmossdk.io/core genesis API
+- [14822](https://github.com/cosmos/cosmos-sdk/pull/14822) Migrate to cosmossdk.io/core genesis API
+
+### State-machine Breaking Changes
+
+- [12273](https://github.com/cosmos/cosmos-sdk/pull/12273) The timestamp key encoding was reworked to properly handle nil values. Existing users will need to manually migrate their data to the new encoding before upgrading.

orm/encoding/encodeutil/util.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"io"
+	"reflect"
 
 	"google.golang.org/protobuf/reflect/protoreflect"
 )
@@ -42,7 +43,11 @@ func ValuesOf(values ...interface{}) []protoreflect.Value {
 		value := values[i]
 		switch value.(type) {
 		case protoreflect.ProtoMessage:
-			value = value.(protoreflect.ProtoMessage).ProtoReflect()
+			if !reflect.ValueOf(value).IsNil() {
+				value = value.(protoreflect.ProtoMessage).ProtoReflect()
+			} else {
+				value = nil
+			}
 		}
 		res[i] = protoreflect.ValueOf(value)
 	}

orm/encoding/ormfield/bool.go

@@ -21,17 +21,22 @@ var (
 
 func (b BoolCodec) Encode(value protoreflect.Value, w io.Writer) error {
 	var err error
-	if value.Bool() {
-		_, err = w.Write(oneBz)
-	} else {
+	if !value.IsValid() || !value.Bool() {
 		_, err = w.Write(zeroBz)
+	} else {
+		_, err = w.Write(oneBz)
 	}
 	return err
 }
 
 func (b BoolCodec) Compare(v1, v2 protoreflect.Value) int {
-	b1 := v1.Bool()
-	b2 := v2.Bool()
+	var b1, b2 bool
+	if v1.IsValid() {
+		b1 = v1.Bool()
+	}
+	if v2.IsValid() {
+		b2 = v2.Bool()
+	}
 	if b1 == b2 {
 		return 0
 	} else if b1 {

orm/encoding/ormfield/bytes.go

@@ -22,6 +22,9 @@ func (b BytesCodec) ComputeBufferSize(value protoreflect.Value) (int, error) {
 }
 
 func bytesSize(value protoreflect.Value) int {
+	if !value.IsValid() {
+		return 0
+	}
 	return len(value.Bytes())
 }
 
@@ -35,12 +38,15 @@ func (b BytesCodec) Decode(r Reader) (protoreflect.Value, error) {
 }
 
 func (b BytesCodec) Encode(value protoreflect.Value, w io.Writer) error {
+	if !value.IsValid() {
+		return nil
+	}
 	_, err := w.Write(value.Bytes())
 	return err
 }
 
 func (b BytesCodec) Compare(v1, v2 protoreflect.Value) int {
-	return bytes.Compare(v1.Bytes(), v2.Bytes())
+	return compareBytes(v1, v2)
 }
 
 // NonTerminalBytesCodec encodes bytes as raw bytes length prefixed by a single
@@ -69,7 +75,7 @@ func (b NonTerminalBytesCodec) IsOrdered() bool {
 }
 
 func (b NonTerminalBytesCodec) Compare(v1, v2 protoreflect.Value) int {
-	return bytes.Compare(v1.Bytes(), v2.Bytes())
+	return compareBytes(v1, v2)
 }
 
 func (b NonTerminalBytesCodec) Decode(r Reader) (protoreflect.Value, error) {
@@ -88,7 +94,10 @@ func (b NonTerminalBytesCodec) Decode(r Reader) (protoreflect.Value, error) {
 }
 
 func (b NonTerminalBytesCodec) Encode(value protoreflect.Value, w io.Writer) error {
-	bz := value.Bytes()
+	var bz []byte
+	if value.IsValid() {
+		bz = value.Bytes()
+	}
 	n := len(bz)
 	var prefix [binary.MaxVarintLen64]byte
 	prefixLen := binary.PutUvarint(prefix[:], uint64(n))
@@ -99,3 +108,14 @@ func (b NonTerminalBytesCodec) Encode(value protoreflect.Value, w io.Writer) err
 	_, err = w.Write(bz)
 	return err
 }
+
+func compareBytes(v1, v2 protoreflect.Value) int {
+	var bz1, bz2 []byte
+	if v1.IsValid() {
+		bz1 = v1.Bytes()
+	}
+	if v2.IsValid() {
+		bz2 = v2.Bytes()
+	}
+	return bytes.Compare(bz1, bz2)
+}

orm/encoding/ormfield/codec_test.go

@@ -4,6 +4,9 @@ import (
 	"bytes"
 	"fmt"
 	"testing"
+	"time"
+
+	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"github.com/cosmos/cosmos-sdk/orm/encoding/ormfield"
 
@@ -169,3 +172,35 @@ func TestCompactUInt64(t *testing.T) {
 		assert.Equal(t, y, y2)
 	})
 }
+
+func TestTimestamp(t *testing.T) {
+	cdc := ormfield.TimestampCodec{}
+
+	// nil value
+	buf := &bytes.Buffer{}
+	assert.NilError(t, cdc.Encode(protoreflect.Value{}, buf))
+	assert.Equal(t, 1, len(buf.Bytes()))
+	val, err := cdc.Decode(buf)
+	assert.NilError(t, err)
+	assert.Assert(t, !val.IsValid())
+
+	// no nanos
+	ts := timestamppb.New(time.Date(2022, 1, 1, 12, 30, 15, 0, time.UTC))
+	val = protoreflect.ValueOfMessage(ts.ProtoReflect())
+	buf = &bytes.Buffer{}
+	assert.NilError(t, cdc.Encode(val, buf))
+	assert.Equal(t, 6, len(buf.Bytes()))
+	val2, err := cdc.Decode(buf)
+	assert.NilError(t, err)
+	assert.Equal(t, 0, cdc.Compare(val, val2))
+
+	// nanos
+	ts = timestamppb.New(time.Date(2022, 1, 1, 12, 30, 15, 235809753, time.UTC))
+	val = protoreflect.ValueOfMessage(ts.ProtoReflect())
+	buf = &bytes.Buffer{}
+	assert.NilError(t, cdc.Encode(val, buf))
+	assert.Equal(t, 9, len(buf.Bytes()))
+	val2, err = cdc.Decode(buf)
+	assert.NilError(t, err)
+	assert.Equal(t, 0, cdc.Compare(val, val2))
+}

orm/encoding/ormfield/enum.go

@@ -16,16 +16,24 @@ func (e EnumCodec) Decode(r Reader) (protoreflect.Value, error) {
 }
 
 func (e EnumCodec) Encode(value protoreflect.Value, w io.Writer) error {
-	x := value.Enum()
+	var x protoreflect.EnumNumber
+	if value.IsValid() {
+		x = value.Enum()
+	}
 	buf := make([]byte, binary.MaxVarintLen32)
 	n := binary.PutVarint(buf, int64(x))
 	_, err := w.Write(buf[:n])
 	return err
 }
 
 func (e EnumCodec) Compare(v1, v2 protoreflect.Value) int {
-	x := v1.Enum()
-	y := v2.Enum()
+	var x, y protoreflect.EnumNumber
+	if v1.IsValid() {
+		x = v1.Enum()
+	}
+	if v2.IsValid() {
+		y = v2.Enum()
+	}
 	if x == y {
 		return 0
 	} else if x < y {

orm/encoding/ormfield/int32.go

@@ -27,7 +27,10 @@ func (i Int32Codec) Decode(r Reader) (protoreflect.Value, error) {
 }
 
 func (i Int32Codec) Encode(value protoreflect.Value, w io.Writer) error {
-	x := value.Int()
+	var x int64
+	if value.IsValid() {
+		x = value.Int()
+	}
 	x += int32Offset
 	return binary.Write(w, binary.BigEndian, uint32(x))
 }

orm/encoding/ormfield/int64.go

@@ -29,7 +29,10 @@ func (i Int64Codec) Decode(r Reader) (protoreflect.Value, error) {
 }
 
 func (i Int64Codec) Encode(value protoreflect.Value, w io.Writer) error {
-	x := value.Int()
+	var x int64
+	if value.IsValid() {
+		x = value.Int()
+	}
 	if x >= -1 {
 		y := uint64(x) + int64Max + 1
 		return binary.Write(w, binary.BigEndian, y)
@@ -57,8 +60,13 @@ func (i Int64Codec) ComputeBufferSize(protoreflect.Value) (int, error) {
 }
 
 func compareInt(v1, v2 protoreflect.Value) int {
-	x := v1.Int()
-	y := v2.Int()
+	var x, y int64
+	if v1.IsValid() {
+		x = v1.Int()
+	}
+	if v2.IsValid() {
+		y = v2.Int()
+	}
 	if x == y {
 		return 0
 	} else if x < y {

orm/encoding/ormfield/string.go

@@ -16,6 +16,10 @@ func (s StringCodec) FixedBufferSize() int {
 }
 
 func (s StringCodec) ComputeBufferSize(value protoreflect.Value) (int, error) {
+	if !value.IsValid() {
+		return 0, nil
+	}
+
 	return len(value.String()), nil
 }
 
@@ -24,7 +28,7 @@ func (s StringCodec) IsOrdered() bool {
 }
 
 func (s StringCodec) Compare(v1, v2 protoreflect.Value) int {
-	return strings.Compare(v1.String(), v2.String())
+	return compareStrings(v1, v2)
 }
 
 func (s StringCodec) Decode(r Reader) (protoreflect.Value, error) {
@@ -33,7 +37,11 @@ func (s StringCodec) Decode(r Reader) (protoreflect.Value, error) {
 }
 
 func (s StringCodec) Encode(value protoreflect.Value, w io.Writer) error {
-	_, err := w.Write([]byte(value.String()))
+	var x string
+	if value.IsValid() {
+		x = value.String()
+	}
+	_, err := w.Write([]byte(x))
 	return err
 }
 
@@ -54,7 +62,7 @@ func (s NonTerminalStringCodec) IsOrdered() bool {
 }
 
 func (s NonTerminalStringCodec) Compare(v1, v2 protoreflect.Value) int {
-	return strings.Compare(v1.String(), v2.String())
+	return compareStrings(v1, v2)
 }
 
 func (s NonTerminalStringCodec) Decode(r Reader) (protoreflect.Value, error) {
@@ -69,7 +77,10 @@ func (s NonTerminalStringCodec) Decode(r Reader) (protoreflect.Value, error) {
 }
 
 func (s NonTerminalStringCodec) Encode(value protoreflect.Value, w io.Writer) error {
-	str := value.String()
+	var str string
+	if value.IsValid() {
+		str = value.String()
+	}
 	bz := []byte(str)
 	for _, b := range bz {
 		if b == 0 {
@@ -85,3 +96,14 @@ func (s NonTerminalStringCodec) Encode(value protoreflect.Value, w io.Writer) er
 }
 
 var nullTerminator = []byte{0}
+
+func compareStrings(v1, v2 protoreflect.Value) int {
+	var x, y string
+	if v1.IsValid() {
+		x = v1.String()
+	}
+	if v2.IsValid() {
+		y = v2.String()
+	}
+	return strings.Compare(x, y)
+}