From 35d3312c3be306591fcba39892223f1244c8d108 Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Wed, 4 May 2022 14:07:02 +0200 Subject: [PATCH] chore: group ORM audit changes (#11842) ## Description ref: #10968 --- ### 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/main/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/main/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/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) --- x/group/internal/orm/genesis.go | 2 +- x/group/internal/orm/index.go | 37 +++++++++++++++------------ x/group/internal/orm/primary_key.go | 4 +-- x/group/internal/orm/spec/01_table.md | 15 ++++++----- 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/x/group/internal/orm/genesis.go b/x/group/internal/orm/genesis.go index 93460dcee8..60a4c3ec78 100644 --- a/x/group/internal/orm/genesis.go +++ b/x/group/internal/orm/genesis.go @@ -4,7 +4,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// TableExportable +// TableExportable defines the methods to import and export a table. type TableExportable interface { // Export stores all the values in the table in the passed // ModelSlicePtr. If the table has an associated sequence, then its diff --git a/x/group/internal/orm/index.go b/x/group/internal/orm/index.go index aff63c6e24..f19de60d3e 100644 --- a/x/group/internal/orm/index.go +++ b/x/group/internal/orm/index.go @@ -132,18 +132,11 @@ func (i MultiKeyIndex) GetPaginated(store sdk.KVStore, searchKey interface{}, pa // // CONTRACT: No writes may happen within a domain while an iterator exists over it. func (i MultiKeyIndex) PrefixScan(store sdk.KVStore, startI interface{}, endI interface{}) (Iterator, error) { - start, err := getPrefixScanKeyBytes(startI) - if err != nil { - return nil, err - } - end, err := getPrefixScanKeyBytes(endI) + start, end, err := getStartEndBz(startI, endI) if err != nil { return nil, err } - if start != nil && end != nil && bytes.Compare(start, end) >= 0 { - return NewInvalidIterator(), sdkerrors.Wrap(errors.ErrORMInvalidArgument, "start must be less than end") - } pStore := prefix.NewStore(store, []byte{i.prefix}) it := pStore.Iterator(start, end) return indexIterator{store: store, it: it, rowGetter: i.rowGetter, indexKey: i.indexKey}, nil @@ -159,23 +152,35 @@ func (i MultiKeyIndex) PrefixScan(store sdk.KVStore, startI interface{}, endI in // // CONTRACT: No writes may happen within a domain while an iterator exists over it. func (i MultiKeyIndex) ReversePrefixScan(store sdk.KVStore, startI interface{}, endI interface{}) (Iterator, error) { - start, err := getPrefixScanKeyBytes(startI) - if err != nil { - return nil, err - } - end, err := getPrefixScanKeyBytes(endI) + start, end, err := getStartEndBz(startI, endI) if err != nil { return nil, err } - if start != nil && end != nil && bytes.Compare(start, end) >= 0 { - return NewInvalidIterator(), sdkerrors.Wrap(errors.ErrORMInvalidArgument, "start must be less than end") - } pStore := prefix.NewStore(store, []byte{i.prefix}) it := pStore.ReverseIterator(start, end) return indexIterator{store: store, it: it, rowGetter: i.rowGetter, indexKey: i.indexKey}, nil } +// getStartEndBz gets the start and end bytes to be passed into the SDK store +// iterator. +func getStartEndBz(startI interface{}, endI interface{}) ([]byte, []byte, error) { + start, err := getPrefixScanKeyBytes(startI) + if err != nil { + return nil, nil, err + } + end, err := getPrefixScanKeyBytes(endI) + if err != nil { + return nil, nil, err + } + + if start != nil && end != nil && bytes.Compare(start, end) >= 0 { + return nil, nil, sdkerrors.Wrap(errors.ErrORMInvalidArgument, "start must be less than end") + } + + return start, end, nil +} + func getPrefixScanKeyBytes(keyI interface{}) ([]byte, error) { var ( key []byte diff --git a/x/group/internal/orm/primary_key.go b/x/group/internal/orm/primary_key.go index 46abfd0dbe..7655c5a9d5 100644 --- a/x/group/internal/orm/primary_key.go +++ b/x/group/internal/orm/primary_key.go @@ -33,9 +33,9 @@ type PrimaryKeyed interface { // the primary key. The PrimaryKey function will encode and concatenate // the fields to build the primary key. // - // PrimaryKey parts can be []byte, string, and integer types. []byte is + // PrimaryKey parts can be []byte, string, and uint64 types. []byte is // encoded with a length prefix, strings are null-terminated, and - // integers are encoded using 8 byte big endian. + // uint64 are encoded using 8 byte big endian. // // IMPORTANT: []byte parts are encoded with a single byte length prefix, // so cannot be longer than 255 bytes. diff --git a/x/group/internal/orm/spec/01_table.md b/x/group/internal/orm/spec/01_table.md index 7e7237f9d0..4c456b1f59 100644 --- a/x/group/internal/orm/spec/01_table.md +++ b/x/group/internal/orm/spec/01_table.md @@ -9,10 +9,11 @@ Regular CRUD operations can be performed on a table, these methods take a `sdk.K The `table` struct does not: -* enforce uniqueness of the `RowID` -* enforce prefix uniqueness of keys, i.e. not allowing one key to be a prefix - of another -* optimize Gas usage conditions +- enforce uniqueness of the `RowID` +- enforce prefix uniqueness of keys, i.e. not allowing one key to be a prefix + of another +- optimize Gas usage conditions + The `table` struct is private, so that we only have custom tables built on top of it, that do satisfy these requirements. `table` provides methods for exporting (using a [`PrefixScan` `Iterator`](03_iterator_pagination.md#iterator)) and importing genesis data. For the import to be successful, objects have to be aware of their primary key by implementing the [`PrimaryKeyed`](#primarykeyed) interface. @@ -42,6 +43,6 @@ The primary key parts can be []byte, string, and `uint64` types. Key parts, except the last part, follow these rules: -* []byte is encoded with a single byte length prefix -* strings are null-terminated -* `uint64` are encoded using 8 byte big endian. +- []byte is encoded with a single byte length prefix (which means the max []byte length is 255) +- strings are null-terminated +- `uint64` are encoded using 8 byte big endian.