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

feat(x/tx): add enum as string encoder option #19618

Merged
merged 8 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions client/v2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* [#19618](https://github.com/cosmos/cosmos-sdk/pull/19618) Marshal enum as string in queries.
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
* [#19060](https://github.com/cosmos/cosmos-sdk/pull/19060) Use client context from root (or enhanced) command in autocli commands.
* Note, the given command must have a `client.Context` in its context.
* [#19216](https://github.com/cosmos/cosmos-sdk/pull/19216) Do not overwrite TxConfig, use directly the one provided in context. TxConfig should always be set in the `client.Context` in `root.go` of an app.
Expand Down
1 change: 1 addition & 0 deletions client/v2/autocli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ func (b *Builder) BuildQueryMethodCommand(ctx context.Context, descriptor protor
outputType := util.ResolveMessageType(b.TypeResolver, descriptor.Output())
encoderOptions := aminojson.EncoderOptions{
Indent: " ",
EnumAsString: true,
DoNotSortFields: true,
TypeResolver: b.TypeResolver,
FileResolver: b.FileResolver,
Expand Down
22 changes: 14 additions & 8 deletions x/tx/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,20 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### Bug Fixes
## v0.13.1

* [#19265](https://github.com/cosmos/cosmos-sdk/pull/19265) Reject denoms that contain a comma.
### Features

* [#19618](https://github.com/cosmos/cosmos-sdk/pull/19618) Add enum as string option to encoder.

### Improvements

* [#18857](https://github.com/cosmos/cosmos-sdk/pull/18857) Moved `FormatCoins` from `core/coins` to this package under `signing/textual`.

### Bug Fixes

* [#19265](https://github.com/cosmos/cosmos-sdk/pull/19265) Reject denoms that contain a comma.

## v0.13.0

### Improvements
Expand Down Expand Up @@ -107,18 +113,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
6 changes: 3 additions & 3 deletions x/tx/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ require (
)

require (
buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.32.0-20230509103710-5e5b9fdd0180.1 // indirect
buf.build/gen/go/tendermint/tendermint/protocolbuffers/go v1.32.0-20231117195010-33ed361a9051.1 // 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 All @@ -37,4 +35,6 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace cosmossdk.io/api => ../../api
// NOTE: we do not want to replace to the latest API yet
// Until https://github.com/cosmos/cosmos-sdk/issues/19228 is resolved
// We are tagging x/tx from main and must keep using released versions of x/tx dependencies
6 changes: 2 additions & 4 deletions x/tx/go.sum
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.32.0-20230509103710-5e5b9fdd0180.1 h1:7LKjxs607BNfGhtKLf+bi3SDJgpiGuTgOvemojsH8Hc=
buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.32.0-20230509103710-5e5b9fdd0180.1/go.mod h1:5GqIYthcy/ASmnKcaT26APpxMhZirnIHXHKki69zjWI=
buf.build/gen/go/tendermint/tendermint/protocolbuffers/go v1.32.0-20231117195010-33ed361a9051.1 h1:VooqQ3rklp3PwMTAE890M76w/8Z01OPa7RdgU9posFE=
buf.build/gen/go/tendermint/tendermint/protocolbuffers/go v1.32.0-20231117195010-33ed361a9051.1/go.mod h1:9KmeMJUsSG3IiIwK63Lh1ipZJrwd7KHrWZseJeHukcs=
cosmossdk.io/api v0.7.3 h1:V815i8YOwOAQa1rLCsSMjVG5Gnzs02JLq+l7ks8s1jk=
cosmossdk.io/api v0.7.3/go.mod h1:IcxpYS5fMemZGqyYtErK7OqvdM0C8kdW3dq8Q/XIG38=
cosmossdk.io/core v0.11.0 h1:vtIafqUi+1ZNAE/oxLOQQ7Oek2n4S48SWLG8h/+wdbo=
cosmossdk.io/core v0.11.0/go.mod h1:LaTtayWBSoacF5xNzoF8tmLhehqlA9z1SWiPuNC6X1w=
cosmossdk.io/errors v1.0.1 h1:bzu+Kcr0kS/1DuPBtUFdWjzLqyUuCiyHjyJB6srBV/0=
Expand Down
1 change: 1 addition & 0 deletions x/tx/signing/aminojson/aminojson.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func NewSignModeHandler(options SignModeHandlerOptions) *SignModeHandler {
h.encoder = NewEncoder(EncoderOptions{
FileResolver: options.FileResolver,
TypeResolver: options.TypeResolver,
EnumAsString: false, // ensure enum as string is disabled
})
} else {
h.encoder = *options.Encoder
Expand Down
4 changes: 2 additions & 2 deletions x/tx/signing/aminojson/encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func nullSliceAsEmptyEncoder(enc *Encoder, v protoreflect.Value, w io.Writer) er
_, err := io.WriteString(w, "[]")
return err
}
return enc.marshalList(list, w)
return enc.marshalList(list, nil /* no field descriptor available here */, w)
default:
return fmt.Errorf("unsupported type %T", list)
}
Expand Down Expand Up @@ -161,7 +161,7 @@ func thresholdStringEncoder(enc *Encoder, msg protoreflect.Message, w io.Writer)
pubkeysField := fields.ByName("public_keys")
pubkeys := msg.Get(pubkeysField).List()

err = enc.marshalList(pubkeys, w)
err = enc.marshalList(pubkeys, pubkeysField, w)
if err != nil {
return err
}
Expand Down
30 changes: 23 additions & 7 deletions x/tx/signing/aminojson/json_marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ type EncoderOptions struct {
Indent string
// DoNotSortFields when set turns off sorting of field names.
DoNotSortFields bool
// EnumAsString when set will encode enums as strings instead of integers.
// Caution: Enabling this option produce different sign bytes.
EnumAsString 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.
Expand All @@ -45,6 +48,7 @@ type Encoder struct {
typeResolver protoregistry.MessageTypeResolver
doNotSortFields bool
indent string
enumsAsString bool
}

// NewEncoder returns a new Encoder capable of serializing protobuf messages to JSON using the Amino JSON encoding
Expand Down Expand Up @@ -78,6 +82,7 @@ func NewEncoder(options EncoderOptions) Encoder {
typeResolver: options.TypeResolver,
doNotSortFields: options.DoNotSortFields,
indent: options.Indent,
enumsAsString: options.EnumAsString,
}
return enc
}
Expand Down Expand Up @@ -189,7 +194,7 @@ func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAn
}
}

err := enc.marshal(protoreflect.ValueOfMessage(msg), writer)
err := enc.marshal(protoreflect.ValueOfMessage(msg), nil /* no field descriptor needed here */, writer)
if err != nil {
return err
}
Expand All @@ -204,7 +209,7 @@ func (enc Encoder) beginMarshal(msg protoreflect.Message, writer io.Writer, isAn
return nil
}

func (enc Encoder) marshal(value protoreflect.Value, writer io.Writer) error {
func (enc Encoder) marshal(value protoreflect.Value, fd protoreflect.FieldDescriptor, writer io.Writer) error {
switch val := value.Interface().(type) {
case protoreflect.Message:
err := enc.marshalMessage(val, writer)
Expand All @@ -218,9 +223,20 @@ func (enc Encoder) marshal(value protoreflect.Value, writer io.Writer) error {
_, err := io.WriteString(writer, "null")
return err
}
return enc.marshalList(val, writer)
return enc.marshalList(val, fd, writer)

case string, bool, int32, uint32, []byte:
return jsonMarshal(writer, val)

case protoreflect.EnumNumber:
if enc.enumsAsString && fd != nil {
desc := fd.Enum().Values().ByNumber(val)
if desc != nil {
_, err := io.WriteString(writer, fmt.Sprintf(`"%s"`, desc.Name()))
return err
}
}

case string, bool, int32, uint32, []byte, protoreflect.EnumNumber:
return jsonMarshal(writer, val)

case uint64, int64:
Expand Down Expand Up @@ -342,7 +358,7 @@ func (enc Encoder) marshalMessage(msg protoreflect.Message, writer io.Writer) er
return err
}
} else {
err = enc.marshal(v, writer)
err = enc.marshal(v, f, writer)
if err != nil {
return err
}
Expand Down Expand Up @@ -371,7 +387,7 @@ func jsonMarshal(w io.Writer, v interface{}) error {
return err
}

func (enc Encoder) marshalList(list protoreflect.List, writer io.Writer) error {
func (enc Encoder) marshalList(list protoreflect.List, fd protoreflect.FieldDescriptor, writer io.Writer) error {
n := list.Len()

_, err := io.WriteString(writer, "[")
Expand All @@ -389,7 +405,7 @@ func (enc Encoder) marshalList(list protoreflect.List, writer io.Writer) error {
}
first = false

err = enc.marshal(list.Get(i), writer)
err = enc.marshal(list.Get(i), fd, writer)
if err != nil {
return err
}
Expand Down
59 changes: 59 additions & 0 deletions x/tx/signing/aminojson/json_marshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,62 @@ func TestIndent(t *testing.T) {
}
}`, string(bz))
}

func TestEnumAsString(t *testing.T) {
encoder := aminojson.NewEncoder(aminojson.EncoderOptions{Indent: " ", EnumAsString: 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": "ABitOfEverything",
"value": {
"bool": true,
"bytes": "AAECAw==",
"enum": "ONE",
"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))
}
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Loading
Loading