Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(orm)!: timestamp encoding doesn't handle nil values properly #12273

Merged
merged 15 commits into from
Feb 9, 2023
Merged
8 changes: 6 additions & 2 deletions orm/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have docs, upgrading tooling on that. Otherwise, it feels a bit like "good luck".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm planning to push another PR for ORM migrations that will automate this


- [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.
7 changes: 6 additions & 1 deletion orm/encoding/encodeutil/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"encoding/binary"
"io"
"reflect"

"google.golang.org/protobuf/reflect/protoreflect"
)
Expand Down Expand Up @@ -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)
}
Expand Down
15 changes: 10 additions & 5 deletions orm/encoding/ormfield/bool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
26 changes: 23 additions & 3 deletions orm/encoding/ormfield/bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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))
Expand All @@ -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)
}
35 changes: 35 additions & 0 deletions orm/encoding/ormfield/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import (
"bytes"
"fmt"
"testing"
"time"

"google.golang.org/protobuf/types/known/timestamppb"

"github.com/cosmos/cosmos-sdk/orm/encoding/ormfield"

Expand Down Expand Up @@ -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))
}
14 changes: 11 additions & 3 deletions orm/encoding/ormfield/enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion orm/encoding/ormfield/int32.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
14 changes: 11 additions & 3 deletions orm/encoding/ormfield/int64.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
30 changes: 26 additions & 4 deletions orm/encoding/ormfield/string.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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) {
Expand All @@ -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
}

Expand All @@ -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) {
Expand All @@ -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 {
Expand All @@ -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)
}
Loading