Skip to content

Commit

Permalink
feat(orm): allow bytes keys longer than 255 bytes (#11522)
Browse files Browse the repository at this point in the history
## 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)
  • Loading branch information
aaronc authored Apr 1, 2022
1 parent 8591cce commit 44a1293
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 31 deletions.
36 changes: 19 additions & 17 deletions orm/encoding/ormfield/bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
10 changes: 0 additions & 10 deletions orm/encoding/ormfield/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions orm/internal/testutil/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -39,7 +39,7 @@ var TestFieldSpecs = []TestFieldSpec{
},
{
"bz",
rapid.SliceOfN(rapid.Byte(), 0, 255),
rapid.SliceOfN(rapid.Byte(), 0, math.MaxUint32),
},
{
"i32",
Expand Down
1 change: 0 additions & 1 deletion orm/types/ormerrors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 44a1293

Please sign in to comment.