From adc3432ba24f9f2ea5382f62d356ec921c7096ea Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 25 Mar 2024 05:45:15 -0400 Subject: [PATCH] fix(x/tx): default to using gogoproto.HybridResolver wherever possible (#19845) Co-authored-by: marbar3778 --- store/database.go | 6 +++--- store/internal/encoding/changeset_test.go | 3 ++- store/storage/pebbledb/batch.go | 4 ++-- store/storage/pebbledb/db.go | 4 ++-- store/storage/sqlite/batch.go | 4 ++-- store/storage/sqlite/db_test.go | 4 +--- x/tx/CHANGELOG.md | 16 ++++++++++------ x/tx/signing/aminojson/aminojson.go | 3 ++- x/tx/signing/aminojson/json_marshal.go | 3 ++- x/tx/signing/context.go | 3 ++- x/tx/signing/textual/handler.go | 3 ++- 11 files changed, 30 insertions(+), 23 deletions(-) diff --git a/store/database.go b/store/database.go index 9dc48d8db49b..0ff83ef3401f 100644 --- a/store/database.go +++ b/store/database.go @@ -18,7 +18,7 @@ type Reader interface { // // Note: is safe to modify and read after calling Get. // The returned byte slice is safe to read, but cannot be modified. - Get(storeKey []byte, key []byte) ([]byte, error) + Get(storeKey, key []byte) ([]byte, error) } // Writer wraps the Set method of a backing data store. @@ -26,12 +26,12 @@ type Writer interface { // Set inserts the given value into the key-value data store. // // Note: are safe to modify and read after calling Set. - Set(storeKey []byte, key, value []byte) error + Set(storeKey, key, value []byte) error // Delete removes the key from the backing key-value data store. // // Note: is safe to modify and read after calling Delete. - Delete(storeKey []byte, key []byte) error + Delete(storeKey, key []byte) error } // Database contains all the methods required to allow handling different diff --git a/store/internal/encoding/changeset_test.go b/store/internal/encoding/changeset_test.go index ae6cb4e2d4a4..03313936b9ea 100644 --- a/store/internal/encoding/changeset_test.go +++ b/store/internal/encoding/changeset_test.go @@ -3,8 +3,9 @@ package encoding import ( "testing" - corestore "cosmossdk.io/core/store" "github.com/stretchr/testify/require" + + corestore "cosmossdk.io/core/store" ) func TestChangesetMarshal(t *testing.T) { diff --git a/store/storage/pebbledb/batch.go b/store/storage/pebbledb/batch.go index fed8c8a1b34c..101986878c81 100644 --- a/store/storage/pebbledb/batch.go +++ b/store/storage/pebbledb/batch.go @@ -57,11 +57,11 @@ func (b *Batch) set(storeKey []byte, tombstone uint64, key, value []byte) error return nil } -func (b *Batch) Set(storeKey []byte, key, value []byte) error { +func (b *Batch) Set(storeKey, key, value []byte) error { return b.set(storeKey, 0, key, value) } -func (b *Batch) Delete(storeKey []byte, key []byte) error { +func (b *Batch) Delete(storeKey, key []byte) error { return b.set(storeKey, b.version, key, []byte(tombstoneVal)) } diff --git a/store/storage/pebbledb/db.go b/store/storage/pebbledb/db.go index 054237bbb487..bc6b4c988f0d 100644 --- a/store/storage/pebbledb/db.go +++ b/store/storage/pebbledb/db.go @@ -319,7 +319,7 @@ func storePrefix(storeKey []byte) []byte { return append([]byte(StorePrefixTpl), storeKey...) } -func prependStoreKey(storeKey []byte, key []byte) []byte { +func prependStoreKey(storeKey, key []byte) []byte { return append(storePrefix(storeKey), key...) } @@ -362,7 +362,7 @@ func valTombstoned(value []byte) bool { return true } -func getMVCCSlice(db *pebble.DB, storeKey []byte, key []byte, version uint64) ([]byte, error) { +func getMVCCSlice(db *pebble.DB, storeKey, key []byte, version uint64) ([]byte, error) { // end domain is exclusive, so we need to increment the version by 1 if version < math.MaxUint64 { version++ diff --git a/store/storage/sqlite/batch.go b/store/storage/sqlite/batch.go index ceb5e29ee625..783b597e04af 100644 --- a/store/storage/sqlite/batch.go +++ b/store/storage/sqlite/batch.go @@ -62,13 +62,13 @@ func (b *Batch) Reset() error { return nil } -func (b *Batch) Set(storeKey []byte, key, value []byte) error { +func (b *Batch) Set(storeKey, key, value []byte) error { b.size += len(key) + len(value) b.ops = append(b.ops, batchOp{action: batchActionSet, storeKey: storeKey, key: key, value: value}) return nil } -func (b *Batch) Delete(storeKey []byte, key []byte) error { +func (b *Batch) Delete(storeKey, key []byte) error { b.size += len(key) b.ops = append(b.ops, batchOp{action: batchActionDel, storeKey: storeKey, key: key}) return nil diff --git a/store/storage/sqlite/db_test.go b/store/storage/sqlite/db_test.go index cf11c3dec780..96c50e9b56fa 100644 --- a/store/storage/sqlite/db_test.go +++ b/store/storage/sqlite/db_test.go @@ -13,9 +13,7 @@ import ( "cosmossdk.io/store/v2/storage" ) -var ( - storeKey1 = []byte("store1") -) +var storeKey1 = []byte("store1") func TestStorageTestSuite(t *testing.T) { s := &storage.StorageTestSuite{ diff --git a/x/tx/CHANGELOG.md b/x/tx/CHANGELOG.md index 4d475d2c64d0..714127d73bb4 100644 --- a/x/tx/CHANGELOG.md +++ b/x/tx/CHANGELOG.md @@ -31,6 +31,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### Improvments + +* [#19845](https://github.com/cosmos/cosmos-sdk/pull/19845) Use hybrid resolver instead of only protov2 registry + ## v0.13.1 ### Features @@ -113,18 +117,18 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements * [#15871](https://github.com/cosmos/cosmos-sdk/pull/15871) - * `HandlerMap` now has a `DefaultMode()` getter method - * Textual types use `signing.ProtoFileResolver` instead of `protoregistry.Files` + * `HandlerMap` now has a `DefaultMode()` getter method + * Textual types use `signing.ProtoFileResolver` instead of `protoregistry.Files` ## v0.6.0 ### API Breaking * [#15709](https://github.com/cosmos/cosmos-sdk/pull/15709): - * `GetSignersContext` has been renamed to `signing.Context` - * `GetSigners` now returns `[][]byte` instead of `[]string` - * `GetSignersOptions` has been renamed to `signing.Options` and requires `address.Codec`s for account and validator addresses - * `GetSignersOptions.ProtoFiles` has been renamed to `signing.Options.FileResolver` + * `GetSignersContext` has been renamed to `signing.Context` + * `GetSigners` now returns `[][]byte` instead of `[]string` + * `GetSignersOptions` has been renamed to `signing.Options` and requires `address.Codec`s for account and validator addresses + * `GetSignersOptions.ProtoFiles` has been renamed to `signing.Options.FileResolver` ### Bug Fixes diff --git a/x/tx/signing/aminojson/aminojson.go b/x/tx/signing/aminojson/aminojson.go index 5b0d9e4d0547..abcb25d5e818 100644 --- a/x/tx/signing/aminojson/aminojson.go +++ b/x/tx/signing/aminojson/aminojson.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + gogoproto "github.com/cosmos/gogoproto/proto" "google.golang.org/protobuf/reflect/protoregistry" signingv1beta1 "cosmossdk.io/api/cosmos/tx/signing/v1beta1" @@ -31,7 +32,7 @@ type SignModeHandlerOptions struct { func NewSignModeHandler(options SignModeHandlerOptions) *SignModeHandler { h := &SignModeHandler{} if options.FileResolver == nil { - h.fileResolver = protoregistry.GlobalFiles + h.fileResolver = gogoproto.HybridResolver } else { h.fileResolver = options.FileResolver } diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index 800dfc7fd320..e6c6cad6f250 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/x/tx/signing/aminojson/json_marshal.go @@ -7,6 +7,7 @@ import ( "io" "sort" + gogoproto "github.com/cosmos/gogoproto/proto" "github.com/pkg/errors" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" @@ -55,7 +56,7 @@ type Encoder struct { // rules. func NewEncoder(options EncoderOptions) Encoder { if options.FileResolver == nil { - options.FileResolver = protoregistry.GlobalFiles + options.FileResolver = gogoproto.HybridResolver } if options.TypeResolver == nil { options.TypeResolver = protoregistry.GlobalTypes diff --git a/x/tx/signing/context.go b/x/tx/signing/context.go index d90fb4a5cb30..7aca6fd2b1cd 100644 --- a/x/tx/signing/context.go +++ b/x/tx/signing/context.go @@ -5,6 +5,7 @@ import ( "fmt" cosmos_proto "github.com/cosmos/cosmos-proto" + gogoproto "github.com/cosmos/gogoproto/proto" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protodesc" "google.golang.org/protobuf/reflect/protoreflect" @@ -79,7 +80,7 @@ type ProtoFileResolver interface { func NewContext(options Options) (*Context, error) { protoFiles := options.FileResolver if protoFiles == nil { - protoFiles = protoregistry.GlobalFiles + protoFiles = gogoproto.HybridResolver } protoTypes := options.TypeResolver diff --git a/x/tx/signing/textual/handler.go b/x/tx/signing/textual/handler.go index 67ab53a5c095..e31981b9286e 100644 --- a/x/tx/signing/textual/handler.go +++ b/x/tx/signing/textual/handler.go @@ -8,6 +8,7 @@ import ( "reflect" cosmos_proto "github.com/cosmos/cosmos-proto" + gogoproto "github.com/cosmos/gogoproto/proto" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" @@ -71,7 +72,7 @@ func NewSignModeHandler(o SignModeOptions) (*SignModeHandler, error) { return nil, errors.New("coinMetadataQuerier must be non-empty") } if o.FileResolver == nil { - o.FileResolver = protoregistry.GlobalFiles + o.FileResolver = gogoproto.HybridResolver } if o.TypeResolver == nil { o.TypeResolver = protoregistry.GlobalTypes