From 44a129330ea854f3869d65792e73342cf75867da Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Fri, 1 Apr 2022 14:01:11 -0400 Subject: [PATCH] feat(orm): allow bytes keys longer than 255 bytes (#11522) ## Description Closes: #11381 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- orm/encoding/ormfield/bytes.go | 36 +++++++++++++++-------------- orm/encoding/ormfield/codec_test.go | 10 -------- orm/internal/testutil/testutil.go | 6 ++--- orm/types/ormerrors/errors.go | 1 - 4 files changed, 22 insertions(+), 31 deletions(-) diff --git a/orm/encoding/ormfield/bytes.go b/orm/encoding/ormfield/bytes.go index 9f03f0905d5..2c984b113d1 100644 --- a/orm/encoding/ormfield/bytes.go +++ b/orm/encoding/ormfield/bytes.go @@ -2,11 +2,10 @@ package ormfield import ( "bytes" + "encoding/binary" "io" "google.golang.org/protobuf/reflect/protoreflect" - - "github.com/cosmos/cosmos-sdk/orm/types/ormerrors" ) // BytesCodec encodes bytes as raw bytes. It errors if the byte array is longer @@ -17,17 +16,13 @@ func (b BytesCodec) FixedBufferSize() int { return -1 } +// ComputeBufferSize returns the bytes size of the value. func (b BytesCodec) ComputeBufferSize(value protoreflect.Value) (int, error) { - return bytesSize(value) + return bytesSize(value), nil } -func bytesSize(value protoreflect.Value) (int, error) { - bz := value.Bytes() - n := len(bz) - if n > 255 { - return -1, ormerrors.BytesFieldTooLong - } - return n, nil +func bytesSize(value protoreflect.Value) int { + return len(value.Bytes()) } func (b BytesCodec) IsOrdered() bool { @@ -56,9 +51,17 @@ func (b NonTerminalBytesCodec) FixedBufferSize() int { return -1 } +// ComputeBufferSize returns the bytes size of the value plus the length of the +// varint length-prefix. func (b NonTerminalBytesCodec) ComputeBufferSize(value protoreflect.Value) (int, error) { - n, err := bytesSize(value) - return n + 1, err + n := bytesSize(value) + prefixLen := 1 + // we use varint, if the first bit of a byte is 1 then we need to signal continuation + for n >= 0x80 { + prefixLen++ + n >>= 7 + } + return n + prefixLen, nil } func (b NonTerminalBytesCodec) IsOrdered() bool { @@ -70,7 +73,7 @@ func (b NonTerminalBytesCodec) Compare(v1, v2 protoreflect.Value) int { } func (b NonTerminalBytesCodec) Decode(r Reader) (protoreflect.Value, error) { - n, err := r.ReadByte() + n, err := binary.ReadUvarint(r) if err != nil { return protoreflect.Value{}, err } @@ -87,10 +90,9 @@ func (b NonTerminalBytesCodec) Decode(r Reader) (protoreflect.Value, error) { func (b NonTerminalBytesCodec) Encode(value protoreflect.Value, w io.Writer) error { bz := value.Bytes() n := len(bz) - if n > 255 { - return ormerrors.BytesFieldTooLong - } - _, err := w.Write([]byte{byte(n)}) + var prefix [binary.MaxVarintLen64]byte + prefixLen := binary.PutUvarint(prefix[:], uint64(n)) + _, err := w.Write(prefix[:prefixLen]) if err != nil { return err } diff --git a/orm/encoding/ormfield/codec_test.go b/orm/encoding/ormfield/codec_test.go index 94da353dc46..4e7d1574d7d 100644 --- a/orm/encoding/ormfield/codec_test.go +++ b/orm/encoding/ormfield/codec_test.go @@ -76,16 +76,6 @@ func TestUnsupportedFields(t *testing.T) { assert.ErrorContains(t, err, ormerrors.UnsupportedKeyField.Error()) } -func TestNTBytesTooLong(t *testing.T) { - cdc, err := ormfield.GetCodec(testutil.GetTestField("bz"), true) - assert.NilError(t, err) - buf := &bytes.Buffer{} - bz := protoreflect.ValueOfBytes(make([]byte, 256)) - assert.ErrorContains(t, cdc.Encode(bz, buf), ormerrors.BytesFieldTooLong.Error()) - _, err = cdc.ComputeBufferSize(bz) - assert.ErrorContains(t, err, ormerrors.BytesFieldTooLong.Error()) -} - func TestCompactUInt32(t *testing.T) { var lastBz []byte testEncodeDecode := func(x uint32, expectedLen int) { diff --git a/orm/internal/testutil/testutil.go b/orm/internal/testutil/testutil.go index f98b7e33476..f220cc89790 100644 --- a/orm/internal/testutil/testutil.go +++ b/orm/internal/testutil/testutil.go @@ -2,11 +2,11 @@ package testutil import ( "fmt" + "math" "strings" - "google.golang.org/protobuf/types/known/durationpb" - "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/timestamppb" "pgregory.net/rapid" @@ -39,7 +39,7 @@ var TestFieldSpecs = []TestFieldSpec{ }, { "bz", - rapid.SliceOfN(rapid.Byte(), 0, 255), + rapid.SliceOfN(rapid.Byte(), 0, math.MaxUint32), }, { "i32", diff --git a/orm/types/ormerrors/errors.go b/orm/types/ormerrors/errors.go index bcb432a7410..73a3418e61d 100644 --- a/orm/types/ormerrors/errors.go +++ b/orm/types/ormerrors/errors.go @@ -26,7 +26,6 @@ var ( AutoIncrementKeyAlreadySet = errors.New(codespace, 12, "can't create with auto-increment primary key already set") CantFindIndex = errors.New(codespace, 13, "can't find index") UnexpectedDecodePrefix = errors.New(codespace, 14, "unexpected prefix while trying to decode an entry") - BytesFieldTooLong = errors.New(codespace, 15, "bytes field is longer than 255 bytes") UnsupportedOperation = errors.New(codespace, 16, "unsupported operation") BadDecodeEntry = errors.New(codespace, 17, "bad decode entry") IndexOutOfBounds = errors.New(codespace, 18, "index out of bounds")