Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(client/v2): fix marshalling of queries with any #18309

Merged
merged 10 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why favouring interface registry over the proto.HybridResolver?

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-rc.1 // indirect
github.com/cosmos/ics23/go v0.10.0 // indirect
github.com/cosmos/ledger-cosmos-go v0.13.3 // indirect
Expand Down
6 changes: 6 additions & 0 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 defauls to msg type url.

## v0.11.0

### Improvements
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 {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
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