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(x/tx/amino): special case for string represented decimals #22161

Merged
merged 1 commit into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions tests/integration/tx/aminojson/aminojson_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,11 @@ func TestAminoJSON_LegacyParity(t *testing.T) {
"gov/v1_msg_submit_proposal": {
gogo: &gov_v1_types.MsgSubmitProposal{},
},
"gov/v1_params": {
gogo: &gov_v1_types.Params{
Quorum: math.LegacyMustNewDecFromStr("0.33").String(),
},
},
"slashing/params/dec": {
gogo: &slashingtypes.Params{
DowntimeJailDuration: 1e9 + 7,
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 @@ -43,7 +43,7 @@ func cosmosIntEncoder(_ *Encoder, v protoreflect.Value, w io.Writer) error {
}
}

// cosmosDecEncoder provides legacy compatible encoding for cosmos.Dec and cosmos.Int types. These are sometimes
// cosmosDecEncoder provides legacy compatible encoding for cosmos.Dec types. These are sometimes
// represented as strings in pulsar messages and sometimes as bytes. This encoder handles both cases.
func cosmosDecEncoder(_ *Encoder, v protoreflect.Value, w io.Writer) error {
switch val := v.Interface().(type) {
Expand All @@ -54,7 +54,7 @@ func cosmosDecEncoder(_ *Encoder, v protoreflect.Value, w io.Writer) error {
var dec math.LegacyDec
err := dec.Unmarshal([]byte(val))
if err != nil {
return err
return fmt.Errorf("failed to unmarshal for Amino JSON encoding; string %q into Dec: %w", val, err)
}
return jsonMarshal(w, dec.String())
case []byte:
Expand Down
8 changes: 5 additions & 3 deletions x/tx/signing/aminojson/json_marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"cosmossdk.io/x/tx/signing"
)

const cosmosDecType = "cosmos.Dec"

// MessageEncoder is a function that can encode a protobuf protoreflect.Message to JSON.
type MessageEncoder func(*Encoder, protoreflect.Message, io.Writer) error

Expand Down Expand Up @@ -68,8 +70,8 @@ func NewEncoder(options EncoderOptions) Encoder {
}
enc := Encoder{
cosmosProtoScalarEncoders: map[string]FieldEncoder{
"cosmos.Dec": cosmosDecEncoder,
"cosmos.Int": cosmosIntEncoder,
cosmosDecType: cosmosDecEncoder,
"cosmos.Int": cosmosIntEncoder,
},
aminoMessageEncoders: map[string]MessageEncoder{
"key_field": keyFieldEncoder,
Expand Down Expand Up @@ -366,7 +368,7 @@ func (enc Encoder) marshalMessage(msg protoreflect.Message, writer io.Writer) er
}

// encode value
if encoder := enc.getFieldEncoding(f); encoder != nil {
if encoder := enc.getFieldEncoder(f); encoder != nil {
err = encoder(&enc, v, writer)
if err != nil {
return err
Expand Down
26 changes: 25 additions & 1 deletion x/tx/signing/aminojson/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ package aminojson

import (
cosmos_proto "github.com/cosmos/cosmos-proto"
gogo "github.com/cosmos/gogoproto/gogoproto"
gogoproto "github.com/cosmos/gogoproto/proto"
"github.com/iancoleman/strcase"
"github.com/pkg/errors"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/runtime/protoimpl"
"google.golang.org/protobuf/types/descriptorpb"

"cosmossdk.io/api/amino"
)
Expand Down Expand Up @@ -100,7 +103,16 @@ func (enc Encoder) getMessageEncoder(message protoreflect.Message) MessageEncode
return nil
}

func (enc Encoder) getFieldEncoding(field protoreflect.FieldDescriptor) FieldEncoder {
var customTypeExtension = protoimpl.ExtensionInfo{
ExtendedType: (*descriptorpb.FieldOptions)(nil),
ExtensionType: gogo.E_Customtype.ExtensionType,
Field: gogo.E_Customtype.Field,
Name: gogo.E_Customtype.Name,
Tag: gogo.E_Customtype.Tag,
Filename: gogo.E_Customtype.Filename,
}

func (enc Encoder) getFieldEncoder(field protoreflect.FieldDescriptor) FieldEncoder {
opts := field.Options()
if proto.HasExtension(opts, amino.E_Encoding) {
encoding := proto.GetExtension(opts, amino.E_Encoding).(string)
Expand All @@ -110,6 +122,18 @@ func (enc Encoder) getFieldEncoding(field protoreflect.FieldDescriptor) FieldEnc
}
if proto.HasExtension(opts, cosmos_proto.E_Scalar) {
scalar := proto.GetExtension(opts, cosmos_proto.E_Scalar).(string)
// do not handle encoding of fields tagged only with scalar which are not backed by a
// LegacyDec custom type. This types are handled by the default encoding, as they are
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix grammatical error in comment

There's a typo in the comment on line 126. The word "This" should be "These" to correct the grammatical error.

Apply this diff to fix the typo:

-		// This types are handled by the default encoding, as they are
+		// These types are handled by the default encoding, as they are
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// LegacyDec custom type. This types are handled by the default encoding, as they are
// LegacyDec custom type. These types are handled by the default encoding, as they are

// expected to already be encoded as their human readable string representation
// containing a radix, i.e. "1.2345".
// For example:
// https://github.com/cosmos/cosmos-sdk/blob/9076487d035e43d39fe54e8498da1ce31b9c845c/x/gov/proto/cosmos/gov/v1/gov.proto#L274
if scalar == cosmosDecType {
customType := proto.GetExtension(opts, &customTypeExtension)
if customType != "cosmossdk.io/math.LegacyDec" {
return nil
}
}
if fn, ok := enc.cosmosProtoScalarEncoders[scalar]; ok {
return fn
}
Expand Down
Loading