Skip to content

Commit

Permalink
fix(x/tx): sort with oneof field name in amino-json (#21782)
Browse files Browse the repository at this point in the history
Co-authored-by: Matt Kocubinski <mkocubinski@gmail.com>
(cherry picked from commit 60cbe2d)

# Conflicts:
#	tests/integration/tx/aminojson/aminojson_test.go
#	x/tx/CHANGELOG.md
  • Loading branch information
maharifu authored and mergify[bot] committed Oct 11, 2024
1 parent 955304a commit b51d8bd
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 15 deletions.
22 changes: 22 additions & 0 deletions tests/integration/tx/aminojson/aminojson_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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
}
7 changes: 7 additions & 0 deletions x/tx/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 36 additions & 15 deletions x/tx/signing/aminojson/json_marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
})
}

Expand All @@ -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 &&
Expand All @@ -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
}
Expand Down

0 comments on commit b51d8bd

Please sign in to comment.