Make JSONMarshaler methods require proto.Message (#7054)

* Make JSONMarshaler require proto.Message * Use &msg with MarshalJSON * Use *LegacyAmino in queriers instead of JSONMarshaler * Revert ABCIMessageLogs String() and coins tests * Use LegacyAmino in client/debug and fix subspace tests * Use LegacyAmino in all legacy queriers and adapt simulation * Make AminoCodec implement Marshaler and some godoc fixes * Test fixes * Remove unrelevant comment * Use TxConfig.TxJSONEncoder * Use encoding/json in genutil cli migrate/validate genesis cmds * Address simulation related comments * Use JSONMarshaler in cli tests * Use proto.Message as respType in cli tests * Use tmjson for tm GenesisDoc * Update types/module/simulation.go Co-authored-by: Federico Kunze <[email protected]> * Update types/module/module_test.go Co-authored-by: Federico Kunze <[email protected]> * Add godoc comments * Remove unused InsertKeyJSON * Fix tests Co-authored-by: Aaron Craelius <[email protected]> Co-authored-by: Federico Kunze <[email protected]>
cosmos · Aug 26, 2020 · 7f59723 · 7f59723
1 parent b5bc864
commit 7f59723
Show file tree

Hide file tree

Showing 126 changed files with 577 additions and 489 deletions.
diff --git a/client/context.go b/client/context.go
@@ -66,7 +66,7 @@ func (ctx Context) WithJSONMarshaler(m codec.JSONMarshaler) Context {
 	return ctx
 }
 
-// WithCodec returns a copy of the context with an updated codec.
+// WithLegacyAmino returns a copy of the context with an updated LegacyAmino codec.
 // TODO: Deprecated (remove).
 func (ctx Context) WithLegacyAmino(cdc *codec.LegacyAmino) Context {
 	ctx.LegacyAmino = cdc
@@ -212,27 +212,30 @@ func (ctx Context) PrintString(str string) error {
 // either text or json. If text, toPrint will be YAML encoded. Otherwise, toPrint
 // will be JSON encoded using ctx.JSONMarshaler. An error is returned upon failure.
 func (ctx Context) PrintOutput(toPrint proto.Message) error {
-	return ctx.printOutput(toPrint)
+	// always serialize JSON initially because proto json can't be directly YAML encoded
+	out, err := ctx.JSONMarshaler.MarshalJSON(toPrint)
+	if err != nil {
+		return err
+	}
+	return ctx.printOutput(out)
 }
 
 // PrintOutputLegacy is a variant of PrintOutput that doesn't require a proto type
 // and uses amino JSON encoding. It will be removed in the near future!
 func (ctx Context) PrintOutputLegacy(toPrint interface{}) error {
-	return ctx.WithJSONMarshaler(ctx.LegacyAmino).printOutput(toPrint)
-}
-
-func (ctx Context) printOutput(toPrint interface{}) error {
-	// always serialize JSON initially because proto json can't be directly YAML encoded
-	out, err := ctx.JSONMarshaler.MarshalJSON(toPrint)
+	out, err := ctx.LegacyAmino.MarshalJSON(toPrint)
 	if err != nil {
 		return err
 	}
+	return ctx.printOutput(out)
+}
 
+func (ctx Context) printOutput(out []byte) error {
 	if ctx.OutputFormat == "text" {
 		// handle text format by decoding and re-encoding JSON as YAML
 		var j interface{}
 
-		err = json.Unmarshal(out, &j)
+		err := json.Unmarshal(out, &j)
 		if err != nil {
 			return err
 		}
@@ -248,7 +251,7 @@ func (ctx Context) printOutput(toPrint interface{}) error {
 		writer = os.Stdout
 	}
 
-	_, err = writer.Write(out)
+	_, err := writer.Write(out)
 	if err != nil {
 		return err
 	}

diff --git a/client/context_test.go b/client/context_test.go
@@ -71,13 +71,13 @@ x: "10"
 	// amino
 	//
 	amino := testdata.NewTestAmino()
-	ctx = ctx.WithJSONMarshaler(codec.NewAminoCodec(&codec.LegacyAmino{Amino: amino}))
+	ctx = ctx.WithLegacyAmino(&codec.LegacyAmino{Amino: amino})
 
 	// json
 	buf = &bytes.Buffer{}
 	ctx = ctx.WithOutput(buf)
 	ctx.OutputFormat = "json"
-	err = ctx.PrintOutput(hasAnimal)
+	err = ctx.PrintOutputLegacy(hasAnimal)
 	require.NoError(t, err)
 	require.Equal(t,
 		`{"type":"testdata/HasAnimal","value":{"animal":{"type":"testdata/Dog","value":{"size":"big","name":"Spot"}},"x":"10"}}
@@ -87,7 +87,7 @@ x: "10"
 	buf = &bytes.Buffer{}
 	ctx = ctx.WithOutput(buf)
 	ctx.OutputFormat = "text"
-	err = ctx.PrintOutput(hasAnimal)
+	err = ctx.PrintOutputLegacy(hasAnimal)
 	require.NoError(t, err)
 	require.Equal(t,
 		`type: testdata/HasAnimal

diff --git a/client/debug/main.go b/client/debug/main.go
@@ -90,7 +90,7 @@ $ %s debug pubkey cosmos1e0jnq2sun3dzjh8p2xq95kk0expwmd7shwjpfg
 				return fmt.Errorf("invalid pubkey type; expected ED25519")
 			}
 
-			pubKeyJSONBytes, err := clientCtx.JSONMarshaler.MarshalJSON(edPK)
+			pubKeyJSONBytes, err := clientCtx.LegacyAmino.MarshalJSON(edPK)
 			if err != nil {
 				return err
 			}

diff --git a/codec/amino.go b/codec/amino.go
@@ -13,14 +13,12 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec/types"
 )
 
-// deprecated: Codec defines a wrapper for an Amino codec that properly handles protobuf
+// deprecated: LegacyAmino defines a wrapper for an Amino codec that properly handles protobuf
 // types with Any's
 type LegacyAmino struct {
 	Amino *amino.Codec
 }
 
-var _ JSONMarshaler = &LegacyAmino{}
-
 func (cdc *LegacyAmino) Seal() {
 	cdc.Amino.Seal()
 }
@@ -43,8 +41,8 @@ func RegisterEvidences(cdc *LegacyAmino) {
 // MarshalJSONIndent provides a utility for indented JSON encoding of an object
 // via an Amino codec. It returns an error if it cannot serialize or indent as
 // JSON.
-func MarshalJSONIndent(m JSONMarshaler, obj interface{}) ([]byte, error) {
-	bz, err := m.MarshalJSON(obj)
+func MarshalJSONIndent(cdc *LegacyAmino, obj interface{}) ([]byte, error) {
+	bz, err := cdc.MarshalJSON(obj)
 	if err != nil {
 		return nil, err
 	}
@@ -58,8 +56,8 @@ func MarshalJSONIndent(m JSONMarshaler, obj interface{}) ([]byte, error) {
 }
 
 // MustMarshalJSONIndent executes MarshalJSONIndent except it panics upon failure.
-func MustMarshalJSONIndent(m JSONMarshaler, obj interface{}) []byte {
-	bz, err := MarshalJSONIndent(m, obj)
+func MustMarshalJSONIndent(cdc *LegacyAmino, obj interface{}) []byte {
+	bz, err := MarshalJSONIndent(cdc, obj)
 	if err != nil {
 		panic(fmt.Sprintf("failed to marshal JSON: %s", err))
 	}

diff --git a/codec/amino_codec.go b/codec/amino_codec.go
@@ -1,5 +1,7 @@
 package codec
 
+import "github.com/gogo/protobuf/proto"
+
 // AminoCodec defines a codec that utilizes Codec for both binary and JSON
 // encoding.
 type AminoCodec struct {
@@ -8,38 +10,71 @@ type AminoCodec struct {
 
 var _ Marshaler = &AminoCodec{}
 
+// NewAminoCodec returns a reference to a new AminoCodec
 func NewAminoCodec(codec *LegacyAmino) *AminoCodec {
 	return &AminoCodec{LegacyAmino: codec}
 }
 
+// MarshalBinaryBare implements BinaryMarshaler.MarshalBinaryBare method.
 func (ac *AminoCodec) MarshalBinaryBare(o ProtoMarshaler) ([]byte, error) {
 	return ac.LegacyAmino.MarshalBinaryBare(o)
 }
 
+// MustMarshalBinaryBare implements BinaryMarshaler.MustMarshalBinaryBare method.
 func (ac *AminoCodec) MustMarshalBinaryBare(o ProtoMarshaler) []byte {
 	return ac.LegacyAmino.MustMarshalBinaryBare(o)
 }
 
+// MarshalBinaryLengthPrefixed implements BinaryMarshaler.MarshalBinaryLengthPrefixed method.
 func (ac *AminoCodec) MarshalBinaryLengthPrefixed(o ProtoMarshaler) ([]byte, error) {
 	return ac.LegacyAmino.MarshalBinaryLengthPrefixed(o)
 }
 
+// MustMarshalBinaryLengthPrefixed implements BinaryMarshaler.MustMarshalBinaryLengthPrefixed method.
 func (ac *AminoCodec) MustMarshalBinaryLengthPrefixed(o ProtoMarshaler) []byte {
 	return ac.LegacyAmino.MustMarshalBinaryLengthPrefixed(o)
 }
 
+// UnmarshalBinaryBare implements BinaryMarshaler.UnmarshalBinaryBare method.
 func (ac *AminoCodec) UnmarshalBinaryBare(bz []byte, ptr ProtoMarshaler) error {
 	return ac.LegacyAmino.UnmarshalBinaryBare(bz, ptr)
 }
 
+// MustUnmarshalBinaryBare implements BinaryMarshaler.MustUnmarshalBinaryBare method.
 func (ac *AminoCodec) MustUnmarshalBinaryBare(bz []byte, ptr ProtoMarshaler) {
 	ac.LegacyAmino.MustUnmarshalBinaryBare(bz, ptr)
 }
 
+// UnmarshalBinaryLengthPrefixed implements BinaryMarshaler.UnmarshalBinaryLengthPrefixed method.
 func (ac *AminoCodec) UnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler) error {
 	return ac.LegacyAmino.UnmarshalBinaryLengthPrefixed(bz, ptr)
 }
 
+// MustUnmarshalBinaryLengthPrefixed implements BinaryMarshaler.MustUnmarshalBinaryLengthPrefixed method.
 func (ac *AminoCodec) MustUnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler) {
 	ac.LegacyAmino.MustUnmarshalBinaryLengthPrefixed(bz, ptr)
 }
+
+// MarshalJSON implements JSONMarshaler.MarshalJSON method,
+// it marshals to JSON using legacy amino codec.
+func (ac *AminoCodec) MarshalJSON(o proto.Message) ([]byte, error) {
+	return ac.LegacyAmino.MarshalJSON(o)
+}
+
+// MustMarshalJSON implements JSONMarshaler.MustMarshalJSON method,
+// it executes MarshalJSON except it panics upon failure.
+func (ac *AminoCodec) MustMarshalJSON(o proto.Message) []byte {
+	return ac.LegacyAmino.MustMarshalJSON(o)
+}
+
+// UnmarshalJSON implements JSONMarshaler.UnmarshalJSON method,
+// it unmarshals from JSON using legacy amino codec.
+func (ac *AminoCodec) UnmarshalJSON(bz []byte, ptr proto.Message) error {
+	return ac.LegacyAmino.UnmarshalJSON(bz, ptr)
+}
+
+// MustUnmarshalJSON implements JSONMarshaler.MustUnmarshalJSON method,
+// it executes UnmarshalJSON except it panics upon failure.
+func (ac *AminoCodec) MustUnmarshalJSON(bz []byte, ptr proto.Message) {
+	ac.LegacyAmino.MustUnmarshalJSON(bz, ptr)
+}
diff --git a/codec/amino_codec_test.go b/codec/amino_codec_test.go
@@ -29,7 +29,7 @@ func TestAminoCodec(t *testing.T) {
 
 	testCases := []struct {
 		name         string
-		codec        codec.Marshaler
+		codec        *codec.AminoCodec
 		input        codec.ProtoMarshaler
 		recv         codec.ProtoMarshaler
 		marshalErr   bool
@@ -175,7 +175,7 @@ func TestAminoCodecMarshalJSONIndent(t *testing.T) {
 
 			if tc.marshalErr {
 				require.Error(t, err)
-				require.Panics(t, func() { codec.MustMarshalJSONIndent(cdc, tc.input) })
+				require.Panics(t, func() { codec.MustMarshalJSONIndent(cdc.LegacyAmino, tc.input) })
 				return
 			}
 
@@ -184,7 +184,7 @@ func TestAminoCodecMarshalJSONIndent(t *testing.T) {
 			require.Equal(t, bz, []byte(tc.wantJSON))
 
 			var bz2 []byte
-			require.NotPanics(t, func() { bz2 = codec.MustMarshalJSONIndent(cdc, tc.input) })
+			require.NotPanics(t, func() { bz2 = codec.MustMarshalJSONIndent(cdc.LegacyAmino, tc.input) })
 			require.Equal(t, bz2, []byte(tc.wantJSON))
 		})
 	}

diff --git a/codec/codec.go b/codec/codec.go
@@ -10,7 +10,7 @@ type (
 	// Marshaler defines the interface module codecs must implement in order to support
 	// backwards compatibility with Amino while allowing custom Protobuf-based
 	// serialization. Note, Amino can still be used without any dependency on
-	// Protobuf. There are three typical implementations that fulfill this contract:
+	// Protobuf. There are two typical implementations that fulfill this contract:
 	//
 	// 1. AminoCodec: Provides full Amino serialization compatibility.
 	// 2. ProtoCodec: Provides full Protobuf serialization compatibility.
@@ -36,11 +36,11 @@ type (
 	}
 
 	JSONMarshaler interface {
-		MarshalJSON(o interface{}) ([]byte, error)
-		MustMarshalJSON(o interface{}) []byte
+		MarshalJSON(o proto.Message) ([]byte, error)
+		MustMarshalJSON(o proto.Message) []byte
 
-		UnmarshalJSON(bz []byte, ptr interface{}) error
-		MustUnmarshalJSON(bz []byte, ptr interface{})
+		UnmarshalJSON(bz []byte, ptr proto.Message) error
+		MustUnmarshalJSON(bz []byte, ptr proto.Message)
 	}
 
 	// ProtoMarshaler defines an interface a type must implement as protocol buffer

diff --git a/codec/proto_codec.go b/codec/proto_codec.go
@@ -8,6 +8,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec/types"
 
 	"github.com/gogo/protobuf/jsonpb"
+	"github.com/gogo/protobuf/proto"
 )
 
 // ProtoCodec defines a codec that utilizes Protobuf for both binary and JSON
@@ -18,14 +19,17 @@ type ProtoCodec struct {
 
 var _ Marshaler = &ProtoCodec{}
 
+// NewProtoCodec returns a reference to a new ProtoCodec
 func NewProtoCodec(anyUnpacker types.AnyUnpacker) *ProtoCodec {
 	return &ProtoCodec{anyUnpacker: anyUnpacker}
 }
 
+// MarshalBinaryBare implements BinaryMarshaler.MarshalBinaryBare method.
 func (pc *ProtoCodec) MarshalBinaryBare(o ProtoMarshaler) ([]byte, error) {
 	return o.Marshal()
 }
 
+// MustMarshalBinaryBare implements BinaryMarshaler.MustMarshalBinaryBare method.
 func (pc *ProtoCodec) MustMarshalBinaryBare(o ProtoMarshaler) []byte {
 	bz, err := pc.MarshalBinaryBare(o)
 	if err != nil {
@@ -35,6 +39,7 @@ func (pc *ProtoCodec) MustMarshalBinaryBare(o ProtoMarshaler) []byte {
 	return bz
 }
 
+// MarshalBinaryLengthPrefixed implements BinaryMarshaler.MarshalBinaryLengthPrefixed method.
 func (pc *ProtoCodec) MarshalBinaryLengthPrefixed(o ProtoMarshaler) ([]byte, error) {
 	bz, err := pc.MarshalBinaryBare(o)
 	if err != nil {
@@ -46,6 +51,7 @@ func (pc *ProtoCodec) MarshalBinaryLengthPrefixed(o ProtoMarshaler) ([]byte, err
 	return append(sizeBuf[:n], bz...), nil
 }
 
+// MustMarshalBinaryLengthPrefixed implements BinaryMarshaler.MustMarshalBinaryLengthPrefixed method.
 func (pc *ProtoCodec) MustMarshalBinaryLengthPrefixed(o ProtoMarshaler) []byte {
 	bz, err := pc.MarshalBinaryLengthPrefixed(o)
 	if err != nil {
@@ -55,6 +61,7 @@ func (pc *ProtoCodec) MustMarshalBinaryLengthPrefixed(o ProtoMarshaler) []byte {
 	return bz
 }
 
+// UnmarshalBinaryBare implements BinaryMarshaler.UnmarshalBinaryBare method.
 func (pc *ProtoCodec) UnmarshalBinaryBare(bz []byte, ptr ProtoMarshaler) error {
 	err := ptr.Unmarshal(bz)
 	if err != nil {
@@ -67,12 +74,14 @@ func (pc *ProtoCodec) UnmarshalBinaryBare(bz []byte, ptr ProtoMarshaler) error {
 	return nil
 }
 
+// MustUnmarshalBinaryBare implements BinaryMarshaler.MustUnmarshalBinaryBare method.
 func (pc *ProtoCodec) MustUnmarshalBinaryBare(bz []byte, ptr ProtoMarshaler) {
 	if err := pc.UnmarshalBinaryBare(bz, ptr); err != nil {
 		panic(err)
 	}
 }
 
+// UnmarshalBinaryLengthPrefixed implements BinaryMarshaler.UnmarshalBinaryLengthPrefixed method.
 func (pc *ProtoCodec) UnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler) error {
 	size, n := binary.Uvarint(bz)
 	if n < 0 {
@@ -89,13 +98,16 @@ func (pc *ProtoCodec) UnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshale
 	return pc.UnmarshalBinaryBare(bz, ptr)
 }
 
+// MustUnmarshalBinaryLengthPrefixed implements BinaryMarshaler.MustUnmarshalBinaryLengthPrefixed method.
 func (pc *ProtoCodec) MustUnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler) {
 	if err := pc.UnmarshalBinaryLengthPrefixed(bz, ptr); err != nil {
 		panic(err)
 	}
 }
 
-func (pc *ProtoCodec) MarshalJSON(o interface{}) ([]byte, error) {
+// MarshalJSON implements JSONMarshaler.MarshalJSON method,
+// it marshals to JSON using proto codec.
+func (pc *ProtoCodec) MarshalJSON(o proto.Message) ([]byte, error) {
 	m, ok := o.(ProtoMarshaler)
 	if !ok {
 		return nil, fmt.Errorf("cannot protobuf JSON encode unsupported type: %T", o)
@@ -104,7 +116,9 @@ func (pc *ProtoCodec) MarshalJSON(o interface{}) ([]byte, error) {
 	return ProtoMarshalJSON(m)
 }
 
-func (pc *ProtoCodec) MustMarshalJSON(o interface{}) []byte {
+// MustMarshalJSON implements JSONMarshaler.MustMarshalJSON method,
+// it executes MarshalJSON except it panics upon failure.
+func (pc *ProtoCodec) MustMarshalJSON(o proto.Message) []byte {
 	bz, err := pc.MarshalJSON(o)
 	if err != nil {
 		panic(err)
@@ -113,7 +127,9 @@ func (pc *ProtoCodec) MustMarshalJSON(o interface{}) []byte {
 	return bz
 }
 
-func (pc *ProtoCodec) UnmarshalJSON(bz []byte, ptr interface{}) error {
+// UnmarshalJSON implements JSONMarshaler.UnmarshalJSON method,
+// it unmarshals from JSON using proto codec.
+func (pc *ProtoCodec) UnmarshalJSON(bz []byte, ptr proto.Message) error {
 	m, ok := ptr.(ProtoMarshaler)
 	if !ok {
 		return fmt.Errorf("cannot protobuf JSON decode unsupported type: %T", ptr)
@@ -127,12 +143,17 @@ func (pc *ProtoCodec) UnmarshalJSON(bz []byte, ptr interface{}) error {
 	return types.UnpackInterfaces(ptr, pc.anyUnpacker)
 }
 
-func (pc *ProtoCodec) MustUnmarshalJSON(bz []byte, ptr interface{}) {
+// MustUnmarshalJSON implements JSONMarshaler.MustUnmarshalJSON method,
+// it executes UnmarshalJSON except it panics upon failure.
+func (pc *ProtoCodec) MustUnmarshalJSON(bz []byte, ptr proto.Message) {
 	if err := pc.UnmarshalJSON(bz, ptr); err != nil {
 		panic(err)
 	}
 }
 
+// UnpackAny implements AnyUnpacker.UnpackAny method,
+// it unpacks the value in any to the interface pointer passed in as
+// iface.
 func (pc *ProtoCodec) UnpackAny(any *types.Any, iface interface{}) error {
 	return pc.anyUnpacker.UnpackAny(any, iface)
 }