LaconicNetwork/cosmos-sdk
9
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix(orm): fix failing tests (#16023)

Browse Source
This commit is contained in:
Aaron Craelius 2023-05-04 11:26:57 -04:00 committed by GitHub
parent 96730a5f2b
commit b0c6941deb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 216 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.golangci.yml
View File

@ -117,3 +117,4 @@ linters-settings:
disabled-checks:
- regexpMust
- appendAssign
- ifElseChain

4
orm/CHANGELOG.md
View File

@ -49,3 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [#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.
* [#15138](https://github.com/cosmos/cosmos-sdk/pull/15138) The duration key encoding was reworked to properly handle nil values. Existing users will need to manually migrate their data to the new encoding before upgrading.
### Bug Fixes
* [#16023](https://github.com/cosmos/cosmos-sdk/pull/16023) Fix bugs introduced by lack of CI tests in [#15138](https://github.com/cosmos/cosmos-sdk/pull/15138) and [#15813](https://github.com/cosmos/cosmos-sdk/pull/15813). This changes the duration encoding in [#15138](https://github.com/cosmos/cosmos-sdk/pull/15138) to correctly order values with negative nanos.

16
orm/encoding/ormfield/duration.go
View File

@ -14,6 +14,18 @@ const (
DurationNanosMax = 999999999
)
// DurationCodec encodes google.protobuf.Duration values with the following
// encoding:
// - nil is encoded as []byte{0xFF}
// - seconds (which can range from -315,576,000,000 to +315,576,000,000) is encoded as 5 fixed bytes
// - nanos (which can range from 0 to 999,999,999 or -999,999,999 to 0 if seconds is negative) is encoded as:
// - []byte{0x0} for zero nanos
// - 4 fixed bytes with the bit mask 0xC0 applied to the first byte, with negative nanos scaled so that -999,999,999
// is encoded as 1 and -1 is encoded as 999,999,999
//
// When iterating over timestamp indexes, nil values will always be ordered last.
//
// Values for seconds and nanos outside the ranges specified by google.protobuf.Duration will be rejected.
type DurationCodec struct{}
func (d DurationCodec) Encode(value protoreflect.Value, w io.Writer) error {
@ -46,7 +58,7 @@ func (d DurationCodec) Encode(value protoreflect.Value, w io.Writer) error {
if nanosInt < DurationNanosMin || nanosInt > 0 {
return fmt.Errorf("negative duration nanos is out of range %d, must be between %d and %d", nanosInt, DurationNanosMin, 0)
}
nanosInt = -nanosInt
nanosInt = DurationNanosMax + nanosInt + 1
} else if nanosInt < 0 || nanosInt > DurationNanosMax {
return fmt.Errorf("duration nanos is out of range %d, must be between %d and %d", nanosInt, 0, DurationNanosMax)
}
@ -78,7 +90,7 @@ func (d DurationCodec) Decode(r Reader) (protoreflect.Value, error) {
}
if negative {
nanos = -nanos
nanos = nanos - DurationNanosMax - 1
}
msg.Set(durationNanosField, protoreflect.ValueOfInt32(nanos))

259
orm/encoding/ormfield/duration_test.go
View File

@ -12,79 +12,81 @@ import (
"cosmossdk.io/orm/encoding/ormfield"
)
func TestDurationNil(t *testing.T) {
t.Parallel()
cdc := ormfield.DurationCodec{}
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())
}
func TestDuration(t *testing.T) {
t.Parallel()
cdc := ormfield.DurationCodec{}
// nil value
t.Run("nil value", func(t *testing.T) {
t.Parallel()
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())
})
tt := []struct {
name string
seconds int64
nanos int32
wantLen int
}{
{
"no nanos",
100,
0,
6,
},
{
"with nanos",
3,
879468295,
9,
},
{
"min seconds, -1 nanos",
-315576000000,
-1,
9,
},
{
"min value",
-315576000000,
-999999999,
9,
},
{
"max value",
315576000000,
999999999,
9,
},
{
"max seconds, 1 nanos",
315576000000,
1,
9,
},
}
// no nanos
t.Run("no nanos", func(t *testing.T) {
t.Parallel()
dur, err := time.ParseDuration("100s")
assert.NilError(t, err)
durPb := durationpb.New(dur)
val := protoreflect.ValueOfMessage(durPb.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))
})
t.Run("nanos", func(t *testing.T) {
t.Parallel()
dur, err := time.ParseDuration("3879468295ns")
assert.NilError(t, err)
durPb := durationpb.New(dur)
val := protoreflect.ValueOfMessage(durPb.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))
})
t.Run("min value", func(t *testing.T) {
t.Parallel()
durPb := &durationpb.Duration{
Seconds: -315576000000,
Nanos: -999999999,
}
val := protoreflect.ValueOfMessage(durPb.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))
})
t.Run("max value", func(t *testing.T) {
t.Parallel()
durPb := &durationpb.Duration{
Seconds: 315576000000,
Nanos: 999999999,
}
val := protoreflect.ValueOfMessage(durPb.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))
})
for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
durPb := &durationpb.Duration{
Seconds: tc.seconds,
Nanos: tc.nanos,
}
val := protoreflect.ValueOfMessage(durPb.ProtoReflect())
buf := &bytes.Buffer{}
assert.NilError(t, cdc.Encode(val, buf))
assert.Equal(t, tc.wantLen, len(buf.Bytes()))
val2, err := cdc.Decode(buf)
assert.NilError(t, err)
assert.Equal(t, 0, cdc.Compare(val, val2))
})
}
}
func TestDurationOutOfRange(t *testing.T) {
@ -156,3 +158,124 @@ func TestDurationOutOfRange(t *testing.T) {
})
}
}
func TestDurationCompare(t *testing.T) {
t.Parallel()
cdc := ormfield.DurationCodec{}
tt := []struct {
name string
dur1 *durationpb.Duration
dur2 *durationpb.Duration
want int
}{
{
name: "equal",
dur1: &durationpb.Duration{
Seconds: 1,
Nanos: 1,
},
dur2: &durationpb.Duration{
Seconds: 1,
Nanos: 1,
},
want: 0,
},
{
name: "seconds equal, dur1 nanos less than dur2 nanos",
dur1: &durationpb.Duration{
Seconds: 1,
Nanos: 1,
},
dur2: &durationpb.Duration{
Seconds: 1,
Nanos: 2,
},
want: -1,
},
{
name: "seconds equal, dur1 nanos greater than dur2 nanos",
dur1: &durationpb.Duration{
Seconds: 1,
Nanos: 2,
},
dur2: &durationpb.Duration{
Seconds: 1,
Nanos: 1,
},
want: 1,
},
{
name: "seconds less than",
dur1: &durationpb.Duration{
Seconds: 1,
Nanos: 1,
},
dur2: &durationpb.Duration{
Seconds: 2,
Nanos: 1,
},
want: -1,
},
{
name: "seconds greater than",
dur1: &durationpb.Duration{
Seconds: 2,
Nanos: 1,
},
dur2: &durationpb.Duration{
Seconds: 1,
Nanos: 1,
},
want: 1,
},
{
name: "negative seconds equal, dur1 nanos less than dur2 nanos",
dur1: &durationpb.Duration{
Seconds: -1,
Nanos: -2,
},
dur2: &durationpb.Duration{
Seconds: -1,
Nanos: -1,
},
want: -1,
},
}
for _, tc := range tt {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
val1 := protoreflect.ValueOfMessage(tc.dur1.ProtoReflect())
val2 := protoreflect.ValueOfMessage(tc.dur2.ProtoReflect())
got := cdc.Compare(val1, val2)
assert.Equal(t, tc.want, got, "Compare(%v, %v)", tc.dur1, tc.dur2)
bz1 := encodeValue(t, cdc, val1)
bz2 := encodeValue(t, cdc, val2)
assert.Equal(t, tc.want, bytes.Compare(bz1, bz2), "bytes.Compare(%v, %v)", bz1, bz2)
})
}
t.Run("nanos", func(t *testing.T) {
t.Parallel()
dur, err := time.ParseDuration("3879468295ns")
assert.NilError(t, err)
durPb := durationpb.New(dur)
val := protoreflect.ValueOfMessage(durPb.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))
})
}
func encodeValue(t *testing.T, cdc ormfield.Codec, val protoreflect.Value) []byte {
buf := &bytes.Buffer{}
assert.NilError(t, cdc.Encode(val, buf))
return buf.Bytes()
}

7
orm/encoding/ormkv/key_codec.go
View File

@ -247,20 +247,19 @@ func (cdc KeyCodec) CheckValidRangeIterationKeys(start, end []protoreflect.Value
y := end[i]
cmp = fieldCdc.Compare(x, y)
switch {
case cmp > 0:
if cmp > 0 {
return ormerrors.InvalidRangeIterationKeys.Wrapf(
"start must be before end for field %s",
cdc.fieldDescriptors[i].FullName(),
)
case !fieldCdc.IsOrdered() && cmp != 0:
} else if !fieldCdc.IsOrdered() && cmp != 0 {
descriptor := cdc.fieldDescriptors[i]
return ormerrors.InvalidRangeIterationKeys.Wrapf(
"field %s of kind %s doesn't support ordered range iteration",
descriptor.FullName(),
descriptor.Kind(),
)
case cmp < 0:
} else if cmp < 0 {
break
}
}

5
orm/encoding/ormkv/unique_key.go
View File

@ -160,9 +160,8 @@ func (u UniqueKeyCodec) EncodeEntry(entry Entry) (k, v []byte, err error) {
if !fieldOrder.inKey {
// goes in values because it is not present in the index key otherwise
values = append(values, value)
}
// does not go in values, but we need to verify that the value in index values matches the primary key value
if u.keyCodec.fieldCodecs[fieldOrder.i].Compare(value, indexEntry.IndexValues[fieldOrder.i]) != 0 {
} else if u.keyCodec.fieldCodecs[fieldOrder.i].Compare(value, indexEntry.IndexValues[fieldOrder.i]) != 0 {
// does not go in values, but we need to verify that the value in index values matches the primary key value
return nil, nil, ormerrors.BadDecodeEntry.Wrapf("value in primary key does not match corresponding value in index key")
}
}

2
x/group/internal/orm/index_property_test.go
View File

@ -46,7 +46,7 @@ func TestPrefixRangeProperty(t *testing.T) {
// index, one greater at overflow and 0 from
// then on
for i, b := range start {
if i < overflowIndex { //nolint:gocritic // ifElseChain: rewrite if-else to switch statement
if i < overflowIndex {
require.Equal(t, b, end[i])
} else if i == overflowIndex {
require.Equal(t, b+1, end[i])