From b51d8bd5db174fa6644df15c55f72a9c5c0b201d Mon Sep 17 00:00:00 2001 From: Luis Carvalho Date: Fri, 11 Oct 2024 09:17:06 +0100 Subject: [PATCH] fix(x/tx): sort with oneof field name in amino-json (#21782) Co-authored-by: Matt Kocubinski (cherry picked from commit 60cbe2db29c4cef12c7a743f3a87060e7f781978) # Conflicts: # tests/integration/tx/aminojson/aminojson_test.go # x/tx/CHANGELOG.md --- .../tx/aminojson/aminojson_test.go | 22 ++++++++ x/tx/CHANGELOG.md | 7 +++ x/tx/signing/aminojson/json_marshal.go | 51 +++++++++++++------ 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/tests/integration/tx/aminojson/aminojson_test.go b/tests/integration/tx/aminojson/aminojson_test.go index eb271308f8d6..adf3ade8dd8e 100644 --- a/tests/integration/tx/aminojson/aminojson_test.go +++ b/tests/integration/tx/aminojson/aminojson_test.go @@ -1,7 +1,12 @@ package aminojson import ( +<<<<<<< HEAD "context" +======= + "bytes" + "encoding/json" +>>>>>>> 60cbe2db2 (fix(x/tx): sort with oneof field name in amino-json (#21782)) "fmt" "reflect" "testing" @@ -573,3 +578,20 @@ func postFixPulsarMessage(msg proto.Message) { } } } + +// sortJson sorts the JSON bytes by way of the side effect of unmarshalling and remarshalling the JSON +// using encoding/json. This hacky way of sorting JSON fields was used by the legacy amino JSON encoding in +// x/auth/migrations/legacytx.StdSignBytes. It is used here ensure the x/tx JSON encoding is equivalent to +// the legacy amino JSON encoding. +func sortJson(bz []byte) ([]byte, error) { + var c any + err := json.Unmarshal(bz, &c) + if err != nil { + return nil, err + } + js, err := json.Marshal(c) + if err != nil { + return nil, err + } + return js, nil +} diff --git a/x/tx/CHANGELOG.md b/x/tx/CHANGELOG.md index 058b9b7d99a2..f835457f0fb6 100644 --- a/x/tx/CHANGELOG.md +++ b/x/tx/CHANGELOG.md @@ -33,6 +33,13 @@ Since v0.13.0, x/tx follows Cosmos SDK semver: https://github.com/cosmos/cosmos- ## [Unreleased] +<<<<<<< HEAD +======= +* [#21782](https://github.com/cosmos/cosmos-sdk/pull/21782) Fix JSON attribute sort order on messages with oneof fields. +* [#21825](https://github.com/cosmos/cosmos-sdk/pull/21825) Fix decimal encoding and field ordering in Amino JSON encoder. +* [#21850](https://github.com/cosmos/cosmos-sdk/pull/21850) Support bytes field as signer. + +>>>>>>> 60cbe2db2 (fix(x/tx): sort with oneof field name in amino-json (#21782)) ## [v0.13.5](https://github.com/cosmos/cosmos-sdk/releases/tag/x/tx/v0.13.5) - 2024-09-18 ### Improvements diff --git a/x/tx/signing/aminojson/json_marshal.go b/x/tx/signing/aminojson/json_marshal.go index 52defcca6357..f53e351b6ea6 100644 --- a/x/tx/signing/aminojson/json_marshal.go +++ b/x/tx/signing/aminojson/json_marshal.go @@ -268,8 +268,11 @@ func (enc Encoder) marshal(value protoreflect.Value, fd protoreflect.FieldDescri } type nameAndIndex struct { - i int - name string + i int + name string + oneof protoreflect.OneofDescriptor + oneofFieldName string + oneofTypeName string } func (enc Encoder) marshalMessage(msg protoreflect.Message, writer io.Writer) error { @@ -300,14 +303,37 @@ func (enc Encoder) marshalMessage(msg protoreflect.Message, writer io.Writer) er indices := make([]*nameAndIndex, 0, fields.Len()) for i := 0; i < fields.Len(); i++ { f := fields.Get(i) - name := getAminoFieldName(f) - indices = append(indices, &nameAndIndex{i: i, name: name}) + entry := &nameAndIndex{ + i: i, + name: getAminoFieldName(f), + oneof: f.ContainingOneof(), + } + + if entry.oneof != nil { + var err error + entry.oneofFieldName, entry.oneofTypeName, err = getOneOfNames(f) + if err != nil { + return err + } + } + + indices = append(indices, entry) } if shouldSortFields := !enc.doNotSortFields; shouldSortFields { sort.Slice(indices, func(i, j int) bool { ni, nj := indices[i], indices[j] - return ni.name < nj.name + niName, njName := ni.name, nj.name + + if indices[i].oneof != nil { + niName = indices[i].oneofFieldName + } + + if indices[j].oneof != nil { + njName = indices[j].oneofFieldName + } + + return niName < njName }) } @@ -316,22 +342,17 @@ func (enc Encoder) marshalMessage(msg protoreflect.Message, writer io.Writer) er name := ni.name f := fields.Get(i) v := msg.Get(f) - oneof := f.ContainingOneof() - isOneOf := oneof != nil - oneofFieldName, oneofTypeName, err := getOneOfNames(f) - if err != nil && isOneOf { - return err - } + isOneOf := ni.oneof != nil writeNil := false if !msg.Has(f) { // msg.WhichOneof(oneof) == nil: no field of the oneof has been set // !emptyOneOfWritten: we haven't written a null for this oneof yet (only write one null per empty oneof) switch { - case isOneOf && msg.WhichOneof(oneof) == nil && !emptyOneOfWritten[oneofFieldName]: - name = oneofFieldName + case isOneOf && msg.WhichOneof(ni.oneof) == nil && !emptyOneOfWritten[ni.oneofFieldName]: + name = ni.oneofFieldName writeNil = true - emptyOneOfWritten[oneofFieldName] = true + emptyOneOfWritten[ni.oneofFieldName] = true case omitEmpty(f): continue case f.Kind() == protoreflect.MessageKind && @@ -349,7 +370,7 @@ func (enc Encoder) marshalMessage(msg protoreflect.Message, writer io.Writer) er } if isOneOf && !writeNil { - _, err = fmt.Fprintf(writer, `"%s":{"type":"%s","value":{`, oneofFieldName, oneofTypeName) + _, err = fmt.Fprintf(writer, `"%s":{"type":"%s","value":{`, ni.oneofFieldName, ni.oneofTypeName) if err != nil { return err }