From 5ec09c691daf4a8d373087ab6adac1d5563833e6 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 13 Sep 2024 13:59:50 +0200 Subject: [PATCH 1/6] feat(x/tx,client/v2/autocli): add `aminoNameAsTypeURL` option in aminojson encoder --- client/v2/CHANGELOG.md | 4 ++++ client/v2/autocli/query.go | 11 ++++++----- x/tx/CHANGELOG.md | 1 + x/tx/signing/aminojson/any.go | 4 ++-- x/tx/signing/aminojson/json_marshal.go | 23 +++++++++++++++-------- x/tx/signing/aminojson/options.go | 6 +++++- 6 files changed, 33 insertions(+), 16 deletions(-) diff --git a/client/v2/CHANGELOG.md b/client/v2/CHANGELOG.md index 5fae2c385bdc..35cd31c1b395 100644 --- a/client/v2/CHANGELOG.md +++ b/client/v2/CHANGELOG.md @@ -43,6 +43,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#18626](https://github.com/cosmos/cosmos-sdk/pull/18626) Support for off-chain signing and verification of a file. * [#18461](https://github.com/cosmos/cosmos-sdk/pull/18461) Support governance proposals. +### Improvements + +* [#](https://github.com/cosmos/cosmos-sdk/pull/) Marshal `type` field as proto message url in queries instead of amino name. + ### API Breaking Changes * [#17709](https://github.com/cosmos/cosmos-sdk/pull/17709) Address codecs have been removed from `autocli.AppOptions` and `flag.Builder`. Instead client/v2 uses the address codecs present in the context (introduced in [#17503](https://github.com/cosmos/cosmos-sdk/pull/17503)). diff --git a/client/v2/autocli/query.go b/client/v2/autocli/query.go index dc268b573654..166be5efebdc 100644 --- a/client/v2/autocli/query.go +++ b/client/v2/autocli/query.go @@ -119,11 +119,12 @@ func (b *Builder) BuildQueryMethodCommand(ctx context.Context, descriptor protor methodName := fmt.Sprintf("/%s/%s", serviceDescriptor.FullName(), descriptor.Name()) outputType := util.ResolveMessageType(b.TypeResolver, descriptor.Output()) encoderOptions := aminojson.EncoderOptions{ - Indent: " ", - EnumAsString: true, - DoNotSortFields: true, - TypeResolver: b.TypeResolver, - FileResolver: b.FileResolver, + Indent: " ", + EnumAsString: true, + DoNotSortFields: true, + AminoNameAsTypeURL: true, + TypeResolver: b.TypeResolver, + FileResolver: b.FileResolver, } cmd, err := b.buildMethodCommandCommon(descriptor, options, func(cmd *cobra.Command, input protoreflect.Message) error { diff --git a/x/tx/CHANGELOG.md b/x/tx/CHANGELOG.md index 7dfbc54a8a73..4d8d581a9fbd 100644 --- a/x/tx/CHANGELOG.md +++ b/x/tx/CHANGELOG.md @@ -35,6 +35,7 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos- ### Improvements +* [#](https://github.com/cosmos/cosmos-sdk/pull/) Add `AminoNameAsTypeURL` option to Amino JSON encoder. * [#21073](https://github.com/cosmos/cosmos-sdk/pull/21073) In Context use sync.Map `getSignersFuncs` map from concurrent writes, we also need to call Validate when using the legacy app. ## [v0.13.3](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.3) - 2024-04-22 diff --git a/x/tx/signing/aminojson/any.go b/x/tx/signing/aminojson/any.go index c7e304a46977..474b89f5fd83 100644 --- a/x/tx/signing/aminojson/any.go +++ b/x/tx/signing/aminojson/any.go @@ -50,7 +50,7 @@ func marshalAny(enc *Encoder, message protoreflect.Message, writer io.Writer) er protoMessage = valueMsg.ProtoReflect() } - return enc.beginMarshal(protoMessage, writer, true) + return enc.beginMarshal(protoMessage, writer, true, enc.aminoNameAsTypeURL) } const ( @@ -73,5 +73,5 @@ func marshalDynamic(enc *Encoder, message protoreflect.Message, writer io.Writer return err } - return enc.beginMarshal(valueMsg.ProtoReflect(), writer, true) + return enc.beginMarshal(valueMsg.ProtoReflect(), writer, true, enc.aminoNameAsTypeURL) } diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index f276cb39d23e..48e0f626342c 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/x/tx/signing/aminojson/json_marshal.go @@ -32,6 +32,9 @@ type EncoderOptions struct { // EnumAsString when set will encode enums as strings instead of integers. // Caution: Enabling this option produce different sign bytes. EnumAsString bool + // AminoNameAsTypeURL when set will use the amino name as the type URL in the JSON output. + // It is useful when using the Amino JSON encoder for non Amino purposes + AminoNameAsTypeURL bool // TypeResolver is used to resolve protobuf message types by TypeURL when marshaling any packed messages. TypeResolver signing.TypeResolver // FileResolver is used to resolve protobuf file descriptors TypeURL when TypeResolver fails. @@ -50,6 +53,7 @@ type Encoder struct { doNotSortFields bool indent string enumsAsString bool + aminoNameAsTypeURL bool } // NewEncoder returns a new Encoder capable of serializing protobuf messages to JSON using the Amino JSON encoding @@ -80,11 +84,12 @@ func NewEncoder(options EncoderOptions) Encoder { "google.protobuf.Duration": marshalDuration, "google.protobuf.Any": marshalAny, }, - fileResolver: options.FileResolver, - typeResolver: options.TypeResolver, - doNotSortFields: options.DoNotSortFields, - indent: options.Indent, - enumsAsString: options.EnumAsString, + fileResolver: options.FileResolver, + typeResolver: options.TypeResolver, + doNotSortFields: options.DoNotSortFields, + indent: options.Indent, + enumsAsString: options.EnumAsString, + aminoNameAsTypeURL: options.AminoNameAsTypeURL, } return enc } @@ -163,7 +168,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, false) + err := enc.beginMarshal(message.ProtoReflect(), buf, false, enc.aminoNameAsTypeURL) if err != nil { return nil, err } @@ -180,13 +185,15 @@ func (enc Encoder) Marshal(message proto.Message) ([]byte, error) { return buf.Bytes(), nil } -func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAny bool) error { +func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAny bool, useTypeUrl bool) error { var ( name string named bool ) - if isAny { + if useTypeUrl { + name, named = getMessageTypeURL(msg), true + } else if isAny { name, named = getMessageAminoNameAny(msg), true } else { name, named = getMessageAminoName(msg) diff --git a/x/tx/signing/aminojson/options.go b/x/tx/signing/aminojson/options.go index 0eecedb88731..cf9110aef3ae 100644 --- a/x/tx/signing/aminojson/options.go +++ b/x/tx/signing/aminojson/options.go @@ -25,7 +25,6 @@ func getMessageAminoName(msg protoreflect.Message) (string, bool) { // 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) { @@ -33,6 +32,11 @@ func getMessageAminoNameAny(msg protoreflect.Message) string { return name.(string) } + return getMessageTypeURL(msg) +} + +// getMessageTypeURL returns the msg url. +func getMessageTypeURL(msg protoreflect.Message) string { msgURL := "/" + string(msg.Descriptor().FullName()) if msgURL != "/" { return msgURL From 067ebba10e86026379a66754f6fab40ca13d42fa Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 13 Sep 2024 14:04:08 +0200 Subject: [PATCH 2/6] changelog + test --- client/v2/CHANGELOG.md | 2 +- x/tx/CHANGELOG.md | 2 +- x/tx/signing/aminojson/json_marshal_test.go | 59 +++++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/client/v2/CHANGELOG.md b/client/v2/CHANGELOG.md index 35cd31c1b395..831ee40ce922 100644 --- a/client/v2/CHANGELOG.md +++ b/client/v2/CHANGELOG.md @@ -45,7 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements -* [#](https://github.com/cosmos/cosmos-sdk/pull/) Marshal `type` field as proto message url in queries instead of amino name. +* [#21712](https://github.com/cosmos/cosmos-sdk/pull/21712) Marshal `type` field as proto message url in queries instead of amino name. ### API Breaking Changes diff --git a/x/tx/CHANGELOG.md b/x/tx/CHANGELOG.md index 4d8d581a9fbd..d75a44787385 100644 --- a/x/tx/CHANGELOG.md +++ b/x/tx/CHANGELOG.md @@ -35,7 +35,7 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos- ### Improvements -* [#](https://github.com/cosmos/cosmos-sdk/pull/) Add `AminoNameAsTypeURL` option to Amino JSON encoder. +* [#21712](https://github.com/cosmos/cosmos-sdk/pull/21712) Add `AminoNameAsTypeURL` option to Amino JSON encoder. * [#21073](https://github.com/cosmos/cosmos-sdk/pull/21073) In Context use sync.Map `getSignersFuncs` map from concurrent writes, we also need to call Validate when using the legacy app. ## [v0.13.3](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.3) - 2024-04-22 diff --git a/x/tx/signing/aminojson/json_marshal_test.go b/x/tx/signing/aminojson/json_marshal_test.go index a51b83bb49da..29d2f8cf8f59 100644 --- a/x/tx/signing/aminojson/json_marshal_test.go +++ b/x/tx/signing/aminojson/json_marshal_test.go @@ -355,3 +355,62 @@ func TestEnumAsString(t *testing.T) { } }`, string(bz)) } + +func TestAminoNameAsTypeURL(t *testing.T) { + encoder := aminojson.NewEncoder(aminojson.EncoderOptions{Indent: " ", AminoNameAsTypeURL: true}) + + msg := &testpb.ABitOfEverything{ + Message: &testpb.NestedMessage{ + Foo: "test", + Bar: 0, // this is the default value and should be omitted from output + }, + Enum: testpb.AnEnum_ONE, + Repeated: []int32{3, -7, 2, 6, 4}, + Str: `abcxyz"foo"def`, + Bool: true, + Bytes: []byte{0, 1, 2, 3}, + I32: -15, + F32: 1001, + U32: 1200, + Si32: -376, + Sf32: -1000, + I64: 14578294827584932, + F64: 9572348124213523654, + U64: 4759492485, + Si64: -59268425823934, + Sf64: -659101379604211154, + } + + bz, err := encoder.Marshal(msg) + require.NoError(t, err) + fmt.Println(string(bz)) + require.Equal(t, `{ + "type": "/testpb.ABitOfEverything", + "value": { + "bool": true, + "bytes": "AAECAw==", + "enum": 1, + "f32": 1001, + "f64": "9572348124213523654", + "i32": -15, + "i64": "14578294827584932", + "message": { + "foo": "test" + }, + "repeated": [ + 3, + -7, + 2, + 6, + 4 + ], + "sf32": -1000, + "sf64": "-659101379604211154", + "si32": -376, + "si64": "-59268425823934", + "str": "abcxyz\"foo\"def", + "u32": 1200, + "u64": "4759492485" + } +}`, string(bz)) +} From a3007af3a6fe051cf382073a5d92e69e0a99a301 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 13 Sep 2024 15:02:55 +0200 Subject: [PATCH 3/6] lint --- x/tx/signing/aminojson/json_marshal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index 48e0f626342c..16069daefb54 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/x/tx/signing/aminojson/json_marshal.go @@ -185,7 +185,7 @@ func (enc Encoder) Marshal(message proto.Message) ([]byte, error) { return buf.Bytes(), nil } -func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAny bool, useTypeUrl bool) error { +func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAny, useTypeUrl bool) error { var ( name string named bool From 28868c58177226922ca5223783c162c7d048b5f8 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 13 Sep 2024 17:35:06 +0200 Subject: [PATCH 4/6] suggestion Co-authored-by: Matt Kocubinski --- x/tx/signing/aminojson/json_marshal.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index 16069daefb54..cdc6559b3eaf 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/x/tx/signing/aminojson/json_marshal.go @@ -33,7 +33,8 @@ type EncoderOptions struct { // Caution: Enabling this option produce different sign bytes. EnumAsString bool // AminoNameAsTypeURL when set will use the amino name as the type URL in the JSON output. - // It is useful when using the Amino JSON encoder for non Amino purposes + // It is useful when using the Amino JSON encoder for non Amino purposes, + // such as JSON RPC. AminoNameAsTypeURL bool // TypeResolver is used to resolve protobuf message types by TypeURL when marshaling any packed messages. TypeResolver signing.TypeResolver From 4503c30f5affcda9f760f7606719ba31adc12668 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 13 Sep 2024 17:35:59 +0200 Subject: [PATCH 5/6] use switch --- x/tx/signing/aminojson/json_marshal.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index cdc6559b3eaf..9b73d5f06f2e 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/x/tx/signing/aminojson/json_marshal.go @@ -192,11 +192,12 @@ func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAn named bool ) - if useTypeUrl { + switch { + case useTypeUrl: name, named = getMessageTypeURL(msg), true - } else if isAny { + case isAny: name, named = getMessageAminoNameAny(msg), true - } else { + default: name, named = getMessageAminoName(msg) } From 48c4bbdcee624ea5439d0bf8e8c40c54e846dc0e Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 18 Sep 2024 11:06:19 +0200 Subject: [PATCH 6/6] fixes --- x/tx/signing/aminojson/any.go | 4 ++-- x/tx/signing/aminojson/json_marshal.go | 21 +++++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/x/tx/signing/aminojson/any.go b/x/tx/signing/aminojson/any.go index 474b89f5fd83..c7e304a46977 100644 --- a/x/tx/signing/aminojson/any.go +++ b/x/tx/signing/aminojson/any.go @@ -50,7 +50,7 @@ func marshalAny(enc *Encoder, message protoreflect.Message, writer io.Writer) er protoMessage = valueMsg.ProtoReflect() } - return enc.beginMarshal(protoMessage, writer, true, enc.aminoNameAsTypeURL) + return enc.beginMarshal(protoMessage, writer, true) } const ( @@ -73,5 +73,5 @@ func marshalDynamic(enc *Encoder, message protoreflect.Message, writer io.Writer return err } - return enc.beginMarshal(valueMsg.ProtoReflect(), writer, true, enc.aminoNameAsTypeURL) + return enc.beginMarshal(valueMsg.ProtoReflect(), writer, true) } diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index 9b73d5f06f2e..52defcca6357 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/x/tx/signing/aminojson/json_marshal.go @@ -169,7 +169,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, false, enc.aminoNameAsTypeURL) + err := enc.beginMarshal(message.ProtoReflect(), buf, false) if err != nil { return nil, err } @@ -186,19 +186,24 @@ func (enc Encoder) Marshal(message proto.Message) ([]byte, error) { return buf.Bytes(), nil } -func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAny, useTypeUrl bool) error { +func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAny bool) error { var ( name string named bool ) - switch { - case useTypeUrl: - name, named = getMessageTypeURL(msg), true - case isAny: - name, named = getMessageAminoNameAny(msg), true - default: + if isAny { + if enc.aminoNameAsTypeURL { + name, named = getMessageTypeURL(msg), true + } else { + name, named = getMessageAminoNameAny(msg), true + } + } else { name, named = getMessageAminoName(msg) + if enc.aminoNameAsTypeURL { + // do not override named + name = getMessageTypeURL(msg) + } } if named {