JSON serialization of embedded types

[alexanderbez]

I'm not sure if this was the intended functionality, but when it comes to JSON marshaling a struct which contains an embedded struct(s), the resulting JSON output is not directly compatible with the Go's JSON stdlib.

Go's JSON library flattens the entire structure and if any conflicting types are found, accepts the last one. Example: https://play.golang.org/p/MZF47ZEKp8t

go-amino does not do this. go-amino takes the embedded struct(s) and nests them under a key (the struct type by default).

This can be an annoyance in some cases. For one, it's not compatible. Secondly, assume I want to extend a struct by adding an additional field but without modifying the actual type (e.g this would cause a state-machine breaking change), so instead I create a new type and embed the original. I expect the resulting JSON to be the same as the original except now with the new type I added. However, this is not the case with go-amino. If I wish to accomplish this with go-amino, I have to implement a custom JSON marshaler which internally defines a type alias and uses the stdlib. eg.:

type DelegationResp struct {
	Delegation
	Balance sdk.Int `json:"balance"`
}

// MarshalJSON implements the json.Marshaler interface. This is so we can
// achieve a flattened structure while embedding other types.
func (d DelegationResp) MarshalJSON() ([]byte, error) {
	type delegationRespAlias DelegationResp
	return json.Marshal((delegationRespAlias)(d))
}

Maybe this is the intended behavior. If so, please close and ignore :)

/cc @mossid @liamsi

[liamsi]

Hmm, amino does't try to be fully compatible with the standard json library but rather with the behaviour of protobuf/jsonpb. So how would embedding a struct in protobuf look like? How would it look like in a protobuf message? Not possible unless you actually copy the fields of the struct to be embedded or introduce a field for that struct (in both cases it would be explicit how to deal with conflicting fields).

@alexanderbez Do you think it is OK to do the workarounds as described for now and not priortize this for now?

[alexanderbez]

I see. Yes @liamsi, I think the workaround is fine for now. Maybe even document it somewhere and clearly note it. Idk how proto behaves in this context.