Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move and update codec.MarshalAny functions to codec.Marshaler interface #8080

Merged
merged 22 commits into from
Dec 8, 2020
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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).
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
46 changes: 45 additions & 1 deletion codec/amino_codec.go
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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)
}
121 changes: 13 additions & 108 deletions codec/amino_codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
44 changes: 0 additions & 44 deletions codec/any.go

This file was deleted.

20 changes: 5 additions & 15 deletions codec/any_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package codec_test

import (
"errors"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -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) {
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
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"))
}
5 changes: 5 additions & 0 deletions codec/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading