Move and update codec.MarshalAny functions to codec.Marshaler interf…

…ace (#8080) * Changelog update * Rename codec.MarshalAny * move codec.MarshalInterface to codec.Marshaler * fix tests * Update amino_codec for compliance with MarshalerInterface * update tests and comments * add tests * change order of args in UnmarshalInterface to a canonical one * uplift MarshalInterface to take ProtoMessage as an argument * wip * add nil check * make tests working * tests cleanup * add support for *JSON methods * Update changelog * linter fixes * fix test types * update evidence genesis_test * adding test * review updates Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
cosmos · Dec 8, 2020 · b219c54 · b219c54
1 parent da6b7f7
commit b219c54
Show file tree

Hide file tree

Showing 29 changed files with 374 additions and 361 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,13 +51,20 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (version) [\#7848](https://github.com/cosmos/cosmos-sdk/pull/7848) [\#7941](https://github.com/cosmos/cosmos-sdk/pull/7941) `version --long` output now shows the list of build dependencies and replaced build dependencies.
 
 ### State Machine Breaking Changes
-
-* (x/upgrade) [\#7979](https://github.com/cosmos/cosmos-sdk/pull/7979) keeper pubkey storage serialization migration from bech32 to protobuf. 
+* (x/upgrade) [\#7979](https://github.com/cosmos/cosmos-sdk/pull/7979) keeper pubkey storage serialization migration from bech32 to protobuf.
 
 ### Bug Fixes
 
 * (crypto) [\#7966](https://github.com/cosmos/cosmos-sdk/issues/7966) `Bip44Params` `String()` function now correctly returns the absolute HD path by adding the `m/` prefix.
 
+
+### API Breaking
+
+* [\#8080](https://github.com/cosmos/cosmos-sdk/pull/8080) Updated the `codec.Marshaler` interface
+  * Moved `MarshalAny` and `UnmarshalAny` helper functions to `codec.Marshaler` and renamed to `MarshalInterface` and `UnmarshalInterface` respectively. These functions must take interface as a parameter (not a concrete type nor `Any` object). Underneath they use `Any` wrapping for correct protobuf serialization.
+
+
+
 ## [v0.40.0-rc3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.40.0-rc3) - 2020-11-06
 
 ### Client Breaking
@@ -74,7 +81,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Client Breaking
 
 * (x/upgrade) [#7697](https://github.com/cosmos/cosmos-sdk/pull/7697) Rename flag name "--time" to "--upgrade-time", "--info" to "--upgrade-info", to keep it consistent with help message.
-* (x/auth) [#7788](https://github.com/cosmos/cosmos-sdk/pull/7788) Remove `tx auth` subcommands, all auth subcommands exist as `tx <subcommand>` 
+* (x/auth) [#7788](https://github.com/cosmos/cosmos-sdk/pull/7788) Remove `tx auth` subcommands, all auth subcommands exist as `tx <subcommand>`
 
 ### API Breaking
 
@@ -93,6 +100,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
   * All outward facing APIs will now check that capability is not nil and name is not empty before performing any state-machine changes
   * `SetIndex` has been renamed to `InitializeIndex`
 
+
 ### Features
 
 * (tx) [\#7688](https://github.com/cosmos/cosmos-sdk/pull/7688) Add a new Tx gRPC service with methods `Simulate` and `GetTx` (by hash).
@@ -134,7 +142,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Features
 
 * (modules) [\#7540](https://github.com/cosmos/cosmos-sdk/issues/7540) Protobuf service definitions can now be used for
-packing `Msg`s in transactions as defined in [ADR 031](./docs/architecture/adr-031-msg-service.md). All modules now 
+packing `Msg`s in transactions as defined in [ADR 031](./docs/architecture/adr-031-msg-service.md). All modules now
 define a `Msg` protobuf service.
 * (codec) [\#7519](https://github.com/cosmos/cosmos-sdk/pull/7519) `InterfaceRegistry` now inherits `jsonpb.AnyResolver`, and has a `RegisterCustomTypeURL` method to support ADR 031 packing of `Any`s. `AnyResolver` is now a required parameter to `RejectUnknownFields`.
 * (baseapp) [\#7519](https://github.com/cosmos/cosmos-sdk/pull/7519) Add `ServiceMsgRouter` to BaseApp to handle routing of protobuf service `Msg`s. The two new types defined in ADR 031, `sdk.ServiceMsg` and `sdk.MsgRequest` are introduced with this router.
@@ -738,7 +746,7 @@ generalized genesis accounts through the `GenesisAccount` interface.
 * (sdk) [\#4758](https://github.com/cosmos/cosmos-sdk/issues/4758) update `x/genaccounts` to match module spec
 * (simulation) [\#4824](https://github.com/cosmos/cosmos-sdk/issues/4824) `PrintAllInvariants` flag will print all failed invariants
 * (simulation) [\#4490](https://github.com/cosmos/cosmos-sdk/issues/4490) add `InitialBlockHeight` flag to resume a simulation from a given block
-  
+
   * Support exporting the simulation stats to a given JSON file
 * (simulation) [\#4847](https://github.com/cosmos/cosmos-sdk/issues/4847), [\#4838](https://github.com/cosmos/cosmos-sdk/pull/4838) and [\#4869](https://github.com/cosmos/cosmos-sdk/pull/4869) `SimApp` and simulation refactors:
   * Implement `SimulationManager` for executing modules' simulation functionalities in a modularized way
@@ -1052,7 +1060,7 @@ that error is that the account doesn't exist.
 * (simulation) PrintAllInvariants flag will print all failed invariants
 * (simulation) Add `InitialBlockHeight` flag to resume a simulation from a given block
 * (simulation) [\#4670](https://github.com/cosmos/cosmos-sdk/issues/4670) Update simulation statistics to JSON format
-  
+
   - Support exporting the simulation stats to a given JSON file
 * [\#4775](https://github.com/cosmos/cosmos-sdk/issues/4775) Refactor CI config
 * Upgrade IAVL to v0.12.4
@@ -1658,9 +1666,9 @@ BREAKING CHANGES
 FEATURES
 
 * Gaia REST API
-  
+
 * [\#2358](https://github.com/cosmos/cosmos-sdk/issues/2358) Add distribution module REST interface
-  
+
 * Gaia CLI  (`gaiacli`)
   * [\#3429](https://github.com/cosmos/cosmos-sdk/issues/3429) Support querying
   for all delegator distribution rewards.

diff --git a/codec/amino_codec.go b/codec/amino_codec.go
@@ -1,6 +1,8 @@
 package codec
 
-import "github.com/gogo/protobuf/proto"
+import (
+	"github.com/gogo/protobuf/proto"
+)
 
 // AminoCodec defines a codec that utilizes Codec for both binary and JSON
 // encoding.
@@ -78,3 +80,45 @@ func (ac *AminoCodec) UnmarshalJSON(bz []byte, ptr proto.Message) error {
 func (ac *AminoCodec) MustUnmarshalJSON(bz []byte, ptr proto.Message) {
 	ac.LegacyAmino.MustUnmarshalJSON(bz, ptr)
 }
+
+// MarshalInterface is a convenience function for amino marshaling interfaces.
+// The `i` must be an interface.
+// NOTE: to marshal a concrete type, you should use MarshalBinaryBare instead
+func (ac *AminoCodec) MarshalInterface(i proto.Message) ([]byte, error) {
+	if err := assertNotNil(i); err != nil {
+		return nil, err
+	}
+	return ac.LegacyAmino.MarshalBinaryBare(i)
+}
+
+// UnmarshalInterface is a convenience function for amino unmarshaling interfaces.
+// `ptr` must be a pointer to an interface.
+// NOTE: to unmarshal a concrete type, you should use UnmarshalBinaryBare instead
+//
+// Example:
+//   var x MyInterface
+//   err := cdc.UnmarshalInterface(bz, &x)
+func (ac *AminoCodec) UnmarshalInterface(bz []byte, ptr interface{}) error {
+	return ac.LegacyAmino.UnmarshalBinaryBare(bz, ptr)
+}
+
+// MarshalInterfaceJSON is a convenience function for amino marshaling interfaces.
+// The `i` must be an interface.
+// NOTE: to marshal a concrete type, you should use MarshalJSON instead
+func (ac *AminoCodec) MarshalInterfaceJSON(i proto.Message) ([]byte, error) {
+	if err := assertNotNil(i); err != nil {
+		return nil, err
+	}
+	return ac.LegacyAmino.MarshalJSON(i)
+}
+
+// UnmarshalInterfaceJSON is a convenience function for amino unmarshaling interfaces.
+// `ptr` must be a pointer to an interface.
+// NOTE: to unmarshal a concrete type, you should use UnmarshalJSON instead
+//
+// Example:
+//   var x MyInterface
+//   err := cdc.UnmarshalInterfaceJSON(bz, &x)
+func (ac *AminoCodec) UnmarshalInterfaceJSON(bz []byte, ptr interface{}) error {
+	return ac.LegacyAmino.UnmarshalJSON(bz, ptr)
+}
diff --git a/codec/amino_codec_test.go b/codec/amino_codec_test.go
@@ -15,121 +15,26 @@ import (
 
 func createTestCodec() *codec.LegacyAmino {
 	cdc := codec.NewLegacyAmino()
-
 	cdc.RegisterInterface((*testdata.Animal)(nil), nil)
-	cdc.RegisterConcrete(testdata.Dog{}, "testdata/Dog", nil)
-	cdc.RegisterConcrete(testdata.Cat{}, "testdata/Cat", nil)
+	// NOTE: since we unmarshal interface using pointers, we need to register a pointer
+	// types here.
+	cdc.RegisterConcrete(&testdata.Dog{}, "testdata/Dog", nil)
+	cdc.RegisterConcrete(&testdata.Cat{}, "testdata/Cat", nil)
 
 	return cdc
 }
 
-func TestAminoCodec(t *testing.T) {
-	any, err := types.NewAnyWithValue(&testdata.Dog{Name: "rufus"})
-	require.NoError(t, err)
-
-	testCases := []struct {
-		name         string
-		codec        *codec.AminoCodec
-		input        codec.ProtoMarshaler
-		recv         codec.ProtoMarshaler
-		marshalErr   bool
-		unmarshalErr bool
-	}{
-		{
-			"valid encoding and decoding",
-			codec.NewAminoCodec(createTestCodec()),
-			&testdata.Dog{Name: "rufus"},
-			&testdata.Dog{},
-			false,
-			false,
-		},
-		{
-			"invalid decode type",
-			codec.NewAminoCodec(createTestCodec()),
-			&testdata.Dog{Name: "rufus"},
-			&testdata.Cat{},
-			false,
-			true,
-		},
-		{
-			"any marshaling",
-			codec.NewAminoCodec(createTestCodec()),
-			&testdata.HasAnimal{Animal: any},
-			&testdata.HasAnimal{Animal: any},
-			false,
-			false,
-		},
-	}
-
-	for _, tc := range testCases {
-		tc := tc
-
-		t.Run(tc.name, func(t *testing.T) {
-			bz, err := tc.codec.MarshalBinaryBare(tc.input)
-
-			if tc.marshalErr {
-				require.Error(t, err)
-				require.Panics(t, func() { tc.codec.MustMarshalBinaryBare(tc.input) })
-			} else {
-				var bz2 []byte
-				require.NoError(t, err)
-				require.NotPanics(t, func() { bz2 = tc.codec.MustMarshalBinaryBare(tc.input) })
-				require.Equal(t, bz, bz2)
-
-				err := tc.codec.UnmarshalBinaryBare(bz, tc.recv)
-				if tc.unmarshalErr {
-					require.Error(t, err)
-					require.Panics(t, func() { tc.codec.MustUnmarshalBinaryBare(bz, tc.recv) })
-				} else {
-					require.NoError(t, err)
-					require.NotPanics(t, func() { tc.codec.MustUnmarshalBinaryBare(bz, tc.recv) })
-					require.Equal(t, tc.input, tc.recv)
-				}
-			}
+func TestAminoMarsharlInterface(t *testing.T) {
+	cdc := codec.NewAminoCodec(createTestCodec())
+	m := interfaceMarshaler{cdc.MarshalInterface, cdc.UnmarshalInterface}
+	testInterfaceMarshaling(require.New(t), m, true)
 
-			bz, err = tc.codec.MarshalBinaryLengthPrefixed(tc.input)
-			if tc.marshalErr {
-				require.Error(t, err)
-				require.Panics(t, func() { tc.codec.MustMarshalBinaryLengthPrefixed(tc.input) })
-			} else {
-				var bz2 []byte
-				require.NoError(t, err)
-				require.NotPanics(t, func() { bz2 = tc.codec.MustMarshalBinaryLengthPrefixed(tc.input) })
-				require.Equal(t, bz, bz2)
-
-				err := tc.codec.UnmarshalBinaryLengthPrefixed(bz, tc.recv)
-				if tc.unmarshalErr {
-					require.Error(t, err)
-					require.Panics(t, func() { tc.codec.MustUnmarshalBinaryLengthPrefixed(bz, tc.recv) })
-				} else {
-					require.NoError(t, err)
-					require.NotPanics(t, func() { tc.codec.MustUnmarshalBinaryLengthPrefixed(bz, tc.recv) })
-					require.Equal(t, tc.input, tc.recv)
-				}
-			}
+	m = interfaceMarshaler{cdc.MarshalInterfaceJSON, cdc.UnmarshalInterfaceJSON}
+	testInterfaceMarshaling(require.New(t), m, false)
+}
 
-			bz, err = tc.codec.MarshalJSON(tc.input)
-			if tc.marshalErr {
-				require.Error(t, err)
-				require.Panics(t, func() { tc.codec.MustMarshalJSON(tc.input) })
-			} else {
-				var bz2 []byte
-				require.NoError(t, err)
-				require.NotPanics(t, func() { bz2 = tc.codec.MustMarshalJSON(tc.input) })
-				require.Equal(t, bz, bz2)
-
-				err := tc.codec.UnmarshalJSON(bz, tc.recv)
-				if tc.unmarshalErr {
-					require.Error(t, err)
-					require.Panics(t, func() { tc.codec.MustUnmarshalJSON(bz, tc.recv) })
-				} else {
-					require.NoError(t, err)
-					require.NotPanics(t, func() { tc.codec.MustUnmarshalJSON(bz, tc.recv) })
-					require.Equal(t, tc.input, tc.recv)
-				}
-			}
-		})
-	}
+func TestAminoCodec(t *testing.T) {
+	testMarshaling(t, codec.NewAminoCodec(createTestCodec()))
 }
 
 func TestAminoCodecMarshalJSONIndent(t *testing.T) {

diff --git a/codec/any.go b/codec/any.go
diff --git a/codec/any_test.go b/codec/any_test.go
@@ -1,7 +1,6 @@
 package codec_test
 
 import (
-	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -28,38 +27,29 @@ func TestMarshalAny(t *testing.T) {
 	cdc := codec.NewProtoCodec(registry)
 
 	kitty := &testdata.Cat{Moniker: "Kitty"}
-	bz, err := codec.MarshalAny(cdc, kitty)
+	bz, err := cdc.MarshalInterface(kitty)
 	require.NoError(t, err)
 
 	var animal testdata.Animal
 
 	// empty registry should fail
-	err = codec.UnmarshalAny(cdc, &animal, bz)
+	err = cdc.UnmarshalInterface(bz, &animal)
 	require.Error(t, err)
 
 	// wrong type registration should fail
 	registry.RegisterImplementations((*testdata.Animal)(nil), &testdata.Dog{})
-	err = codec.UnmarshalAny(cdc, &animal, bz)
+	err = cdc.UnmarshalInterface(bz, &animal)
 	require.Error(t, err)
 
 	// should pass
 	registry = NewTestInterfaceRegistry()
 	cdc = codec.NewProtoCodec(registry)
-	err = codec.UnmarshalAny(cdc, &animal, bz)
+	err = cdc.UnmarshalInterface(bz, &animal)
 	require.NoError(t, err)
 	require.Equal(t, kitty, animal)
 
 	// nil should fail
 	registry = NewTestInterfaceRegistry()
-	err = codec.UnmarshalAny(cdc, nil, bz)
+	err = cdc.UnmarshalInterface(bz, nil)
 	require.Error(t, err)
 }
-
-func TestMarshalAnyNonProtoErrors(t *testing.T) {
-	registry := types.NewInterfaceRegistry()
-	cdc := codec.NewProtoCodec(registry)
-
-	_, err := codec.MarshalAny(cdc, 29)
-	require.Error(t, err)
-	require.Equal(t, err, errors.New("can't proto marshal int"))
-}
diff --git a/codec/codec.go b/codec/codec.go
@@ -32,12 +32,17 @@ type (
 		UnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler) error
 		MustUnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler)
 
+		MarshalInterface(i proto.Message) ([]byte, error)
+		UnmarshalInterface(bz []byte, ptr interface{}) error
+
 		types.AnyUnpacker
 	}
 
 	JSONMarshaler interface {
 		MarshalJSON(o proto.Message) ([]byte, error)
 		MustMarshalJSON(o proto.Message) []byte
+		MarshalInterfaceJSON(i proto.Message) ([]byte, error)
+		UnmarshalInterfaceJSON(bz []byte, ptr interface{}) error
 
 		UnmarshalJSON(bz []byte, ptr proto.Message) error
 		MustUnmarshalJSON(bz []byte, ptr proto.Message)