Skip to content

Commit

Permalink
fix(client/v2): fix marshalling of queries with any (#18309)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt authored Nov 3, 2023
1 parent 9e91c7b commit 2caf00d
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 28 deletions.
1 change: 1 addition & 0 deletions client/debug/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ func getCodecInterfaceImpls() *cobra.Command {
Short: "List the registered type URLs for the provided interface",
Long: "List the registered type URLs that can be used for the provided interface name using the application codec",
Example: fmt.Sprintf("%s debug codec list-implementations cosmos.crypto.PubKey", version.AppName),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx := client.GetClientContextFromCmd(cmd)
impls := clientCtx.Codec.InterfaceRegistry().ListImplementations(args[0])
Expand Down
3 changes: 1 addition & 2 deletions client/v2/autocli/app.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package autocli

import (
"github.com/cosmos/gogoproto/proto"
"github.com/spf13/cobra"
"google.golang.org/grpc"
"google.golang.org/protobuf/reflect/protoregistry"
Expand Down Expand Up @@ -66,7 +65,7 @@ func (appOptions AppOptions) EnhanceRootCommand(rootCmd *cobra.Command) error {
builder := &Builder{
Builder: flag.Builder{
TypeResolver: protoregistry.GlobalTypes,
FileResolver: proto.HybridResolver,
FileResolver: appOptions.ClientCtx.InterfaceRegistry,
Keyring: appOptions.Keyring,
AddressCodec: appOptions.ClientCtx.AddressCodec,
ValidatorAddressCodec: appOptions.ClientCtx.ValidatorAddressCodec,
Expand Down
2 changes: 1 addition & 1 deletion client/v2/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ require (
github.com/cockroachdb/errors v1.11.1
github.com/cosmos/cosmos-proto v1.0.0-beta.3
github.com/cosmos/cosmos-sdk v0.51.0
github.com/cosmos/gogoproto v1.4.11
github.com/spf13/cobra v1.7.0
github.com/spf13/pflag v1.0.5
google.golang.org/grpc v1.59.0
Expand Down Expand Up @@ -47,6 +46,7 @@ require (
github.com/cosmos/cosmos-db v1.0.0 // indirect
github.com/cosmos/go-bip39 v1.0.0 // indirect
github.com/cosmos/gogogateway v1.2.0 // indirect
github.com/cosmos/gogoproto v1.4.11 // indirect
github.com/cosmos/iavl v1.0.0 // indirect
github.com/cosmos/ics23/go v0.10.0 // indirect
github.com/cosmos/ledger-cosmos-go v0.13.3 // indirect
Expand Down
18 changes: 12 additions & 6 deletions x/tx/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

## v0.12.0

### Improvements

* [#18309](https://github.com/cosmos/cosmos-sdk/pull/18309) Update encoder so that amino types default to msg type url.

## v0.11.0

### Improvements
Expand Down Expand Up @@ -87,18 +93,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

Expand Down
2 changes: 1 addition & 1 deletion x/tx/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
cosmossdk.io/errors v1.0.0
cosmossdk.io/math v1.1.3-rc.1
github.com/cosmos/cosmos-proto v1.0.0-beta.3
github.com/cosmos/gogoproto v1.4.11
github.com/google/go-cmp v0.6.0
github.com/google/gofuzz v1.2.0
github.com/iancoleman/strcase v0.3.0
Expand All @@ -20,7 +21,6 @@ require (
)

require (
github.com/cosmos/gogoproto v1.4.11 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
Expand Down
16 changes: 2 additions & 14 deletions x/tx/signing/aminojson/any.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,7 @@ func marshalAny(enc *Encoder, message protoreflect.Message, writer io.Writer) er
protoMessage = valueMsg.ProtoReflect()
}

_, named := getMessageAminoName(protoMessage.Descriptor().Options())
if !named {
return fmt.Errorf("message %s is packed into an any field, so requires an amino.name annotation",
anyMsg.TypeUrl)
}

return enc.beginMarshal(protoMessage, writer)
return enc.beginMarshal(protoMessage, writer, true)
}

const (
Expand All @@ -73,17 +67,11 @@ func marshalDynamic(enc *Encoder, message protoreflect.Message, writer io.Writer
return errors.Wrapf(err, "can't resolve type URL %s", msgName)
}

_, named := getMessageAminoName(desc.Options())
if !named {
return fmt.Errorf("message %s is packed into an any field, so requires an amino.name annotation",
msgName)
}

valueMsg := dynamicpb.NewMessageType(desc.(protoreflect.MessageDescriptor)).New().Interface()
err = proto.Unmarshal(msgBytes, valueMsg)
if err != nil {
return err
}

return enc.beginMarshal(valueMsg.ProtoReflect(), writer)
return enc.beginMarshal(valueMsg.ProtoReflect(), writer, true)
}
16 changes: 13 additions & 3 deletions x/tx/signing/aminojson/json_marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (enc Encoder) DefineTypeEncoding(typeURL string, encoder MessageEncoder) En
// Marshal serializes a protobuf message to JSON.
func (enc Encoder) Marshal(message proto.Message) ([]byte, error) {
buf := &bytes.Buffer{}
err := enc.beginMarshal(message.ProtoReflect(), buf)
err := enc.beginMarshal(message.ProtoReflect(), buf, false)

if enc.indent != "" {
indentBuf := &bytes.Buffer{}
Expand All @@ -170,8 +170,18 @@ func (enc Encoder) Marshal(message proto.Message) ([]byte, error) {
return buf.Bytes(), err
}

func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer) error {
name, named := getMessageAminoName(msg.Descriptor().Options())
func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAny bool) error {
var (
name string
named bool
)

if isAny {
name, named = getMessageAminoNameAny(msg), true
} else {
name, named = getMessageAminoName(msg)
}

if named {
_, err := fmt.Fprintf(writer, `{"type":"%s","value":`, name)
if err != nil {
Expand Down
27 changes: 26 additions & 1 deletion x/tx/signing/aminojson/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aminojson

import (
cosmos_proto "github.com/cosmos/cosmos-proto"
gogoproto "github.com/cosmos/gogoproto/proto"
"github.com/iancoleman/strcase"
"github.com/pkg/errors"
"google.golang.org/protobuf/proto"
Expand All @@ -12,14 +13,38 @@ import (

// getMessageAminoName returns the amino name of a message if it has been set by the `amino.name` option.
// If the message does not have an amino name, then the function returns false.
func getMessageAminoName(messageOptions proto.Message) (string, bool) {
func getMessageAminoName(msg protoreflect.Message) (string, bool) {
messageOptions := msg.Descriptor().Options()
if proto.HasExtension(messageOptions, amino.E_Name) {
name := proto.GetExtension(messageOptions, amino.E_Name)
return name.(string), true
}

return "", false
}

// getMessageAminoName returns the amino name of a message if it has been set by the `amino.name` option.
// If the message does not have an amino name, then it returns the msg url.
// If it cannot get the msg url, then it returns false.
func getMessageAminoNameAny(msg protoreflect.Message) string {
messageOptions := msg.Descriptor().Options()
if proto.HasExtension(messageOptions, amino.E_Name) {
name := proto.GetExtension(messageOptions, amino.E_Name)
return name.(string)
}

msgURL := "/" + string(msg.Descriptor().FullName())
if msgURL != "/" {
return msgURL
}

if m, ok := msg.(gogoproto.Message); ok {
return "/" + gogoproto.MessageName(m)
}

return ""
}

// omitEmpty returns true if the field should be omitted if empty. Empty field omission is the default behavior.
func omitEmpty(field protoreflect.FieldDescriptor) bool {
opts := field.Options()
Expand Down
30 changes: 30 additions & 0 deletions x/tx/signing/aminojson/options_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package aminojson

import (
"testing"

"github.com/stretchr/testify/require"

"cosmossdk.io/x/tx/signing/aminojson/internal/testpb"
)

func Test_getMessageAminoName(t *testing.T) {
msg := &testpb.ABitOfEverything{}
name, ok := getMessageAminoName(msg.ProtoReflect())
require.True(t, ok)
require.Equal(t, "ABitOfEverything", name)

secondMsg := &testpb.Duration{}
_, ok = getMessageAminoName(secondMsg.ProtoReflect())
require.False(t, ok)
}

func Test_getMessageAminoNameAny(t *testing.T) {
msg := &testpb.ABitOfEverything{}
name := getMessageAminoNameAny(msg.ProtoReflect())
require.Equal(t, "ABitOfEverything", name)

secondMsg := &testpb.Duration{}
name = getMessageAminoNameAny(secondMsg.ProtoReflect())
require.Equal(t, "/testpb.Duration", name)
}

0 comments on commit 2caf00d

Please sign in to comment.