From 5560a12416e11b5e98d18ea1d08ed0626a216392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Tue, 1 Sep 2020 15:43:14 +0200 Subject: [PATCH] Fix 02-client message marshal and unmarshal issues (#7217) * add test for encoding issue * add unpackinterfaces for msg create client * add unpackinterface and tests for update client and misbehaviour --- x/ibc/02-client/types/msgs.go | 44 ++++++++ x/ibc/02-client/types/msgs_test.go | 161 +++++++++++++++++++++++++++++ x/ibc/07-tendermint/types/codec.go | 4 +- 3 files changed, 207 insertions(+), 2 deletions(-) diff --git a/x/ibc/02-client/types/msgs.go b/x/ibc/02-client/types/msgs.go index 6344a5b57b..9ff95ffc62 100644 --- a/x/ibc/02-client/types/msgs.go +++ b/x/ibc/02-client/types/msgs.go @@ -1,6 +1,7 @@ package types import ( + codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" @@ -18,6 +19,10 @@ var ( _ sdk.Msg = &MsgCreateClient{} _ sdk.Msg = &MsgUpdateClient{} _ sdk.Msg = &MsgSubmitMisbehaviour{} + + _ codectypes.UnpackInterfacesMessage = MsgCreateClient{} + _ codectypes.UnpackInterfacesMessage = MsgUpdateClient{} + _ codectypes.UnpackInterfacesMessage = MsgSubmitMisbehaviour{} ) // NewMsgCreateClient creates a new MsgCreateClient instance @@ -88,6 +93,23 @@ func (msg MsgCreateClient) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{msg.Signer} } +// UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces +func (msg MsgCreateClient) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { + var clientState exported.ClientState + err := unpacker.UnpackAny(msg.ClientState, &clientState) + if err != nil { + return err + } + + var consensusState exported.ConsensusState + err = unpacker.UnpackAny(msg.ConsensusState, &consensusState) + if err != nil { + return err + } + + return nil +} + // NewMsgUpdateClient creates a new MsgUpdateClient instance func NewMsgUpdateClient(id string, header exported.Header, signer sdk.AccAddress) (*MsgUpdateClient, error) { anyHeader, err := PackHeader(header) @@ -140,6 +162,17 @@ func (msg MsgUpdateClient) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{msg.Signer} } +// UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces +func (msg MsgUpdateClient) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { + var header exported.Header + err := unpacker.UnpackAny(msg.Header, &header) + if err != nil { + return err + } + + return nil +} + // NewMsgSubmitMisbehaviour creates a new MsgSubmitMisbehaviour instance. func NewMsgSubmitMisbehaviour(clientID string, misbehaviour exported.Misbehaviour, signer sdk.AccAddress) (*MsgSubmitMisbehaviour, error) { anyMisbehaviour, err := PackMisbehaviour(misbehaviour) @@ -195,3 +228,14 @@ func (msg MsgSubmitMisbehaviour) GetSignBytes() []byte { func (msg MsgSubmitMisbehaviour) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{msg.Signer} } + +// UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces +func (msg MsgSubmitMisbehaviour) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { + var misbehaviour exported.Misbehaviour + err := unpacker.UnpackAny(msg.Misbehaviour, &misbehaviour) + if err != nil { + return err + } + + return nil +} diff --git a/x/ibc/02-client/types/msgs_test.go b/x/ibc/02-client/types/msgs_test.go index b35f4de0f1..5c7df9a4ad 100644 --- a/x/ibc/02-client/types/msgs_test.go +++ b/x/ibc/02-client/types/msgs_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/golang/protobuf/proto" "github.com/stretchr/testify/suite" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" @@ -30,6 +31,58 @@ func TestTypesTestSuite(t *testing.T) { suite.Run(t, new(TypesTestSuite)) } +// tests that different clients within MsgCreateClient can be marshaled +// and unmarshaled. +func (suite *TypesTestSuite) TestMarshalMsgCreateClient() { + var ( + msg *types.MsgCreateClient + err error + ) + + testCases := []struct { + name string + malleate func() + }{ + { + "solo machine client", func() { + soloMachine := ibctesting.NewSolomachine(suite.T(), "solomachine") + msg, err = types.NewMsgCreateClient(soloMachine.ClientID, soloMachine.ClientState(), soloMachine.ConsensusState(), suite.chain.SenderAccount.GetAddress()) + suite.Require().NoError(err) + }, + }, + { + "tendermint client", func() { + tendermintClient := ibctmtypes.NewClientState(suite.chain.ChainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs()) + msg, err = types.NewMsgCreateClient("tendermint", tendermintClient, suite.chain.CreateTMClientHeader().ConsensusState(), suite.chain.SenderAccount.GetAddress()) + suite.Require().NoError(err) + }, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() + + tc.malleate() + + cdc := suite.chain.App.AppCodec() + + // marshal message + bz, err := cdc.MarshalJSON(msg) + suite.Require().NoError(err) + + // unmarshal message + newMsg := &types.MsgCreateClient{} + err = cdc.UnmarshalJSON(bz, newMsg) + suite.Require().NoError(err) + + suite.Require().True(proto.Equal(msg, newMsg)) + }) + } +} + func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { var ( msg = &types.MsgCreateClient{} @@ -138,6 +191,58 @@ func (suite *TypesTestSuite) TestMsgCreateClient_ValidateBasic() { } } +// tests that different header within MsgUpdateClient can be marshaled +// and unmarshaled. +func (suite *TypesTestSuite) TestMarshalMsgUpdateClient() { + var ( + msg *types.MsgUpdateClient + err error + ) + + testCases := []struct { + name string + malleate func() + }{ + { + "solo machine client", func() { + soloMachine := ibctesting.NewSolomachine(suite.T(), "solomachine") + msg, err = types.NewMsgUpdateClient(soloMachine.ClientID, soloMachine.CreateHeader(), suite.chain.SenderAccount.GetAddress()) + suite.Require().NoError(err) + }, + }, + { + "tendermint client", func() { + msg, err = types.NewMsgUpdateClient("tendermint", suite.chain.CreateTMClientHeader(), suite.chain.SenderAccount.GetAddress()) + suite.Require().NoError(err) + + }, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() + + tc.malleate() + + cdc := suite.chain.App.AppCodec() + + // marshal message + bz, err := cdc.MarshalJSON(msg) + suite.Require().NoError(err) + + // unmarshal message + newMsg := &types.MsgUpdateClient{} + err = cdc.UnmarshalJSON(bz, newMsg) + suite.Require().NoError(err) + + suite.Require().True(proto.Equal(msg, newMsg)) + }) + } +} + func (suite *TypesTestSuite) TestMsgUpdateClient_ValidateBasic() { var ( msg = &types.MsgUpdateClient{} @@ -224,6 +329,62 @@ func (suite *TypesTestSuite) TestMsgUpdateClient_ValidateBasic() { } } +// tests that different misbehaviours within MsgSubmitMisbehaviour can be marshaled +// and unmarshaled. +func (suite *TypesTestSuite) TestMarshalMsgSubmitMisbehaviour() { + var ( + msg *types.MsgSubmitMisbehaviour + err error + ) + + testCases := []struct { + name string + malleate func() + }{ + { + "solo machine client", func() { + soloMachine := ibctesting.NewSolomachine(suite.T(), "solomachine") + msg, err = types.NewMsgSubmitMisbehaviour(soloMachine.ClientID, soloMachine.CreateMisbehaviour(), suite.chain.SenderAccount.GetAddress()) + suite.Require().NoError(err) + }, + }, + { + "tendermint client", func() { + header1 := ibctmtypes.CreateTestHeader(suite.chain.ChainID, suite.chain.CurrentHeader.Height, suite.chain.CurrentHeader.Height-1, suite.chain.CurrentHeader.Time, suite.chain.Vals, suite.chain.Vals, suite.chain.Signers) + header2 := ibctmtypes.CreateTestHeader(suite.chain.ChainID, suite.chain.CurrentHeader.Height, suite.chain.CurrentHeader.Height-1, suite.chain.CurrentHeader.Time.Add(time.Minute), suite.chain.Vals, suite.chain.Vals, suite.chain.Signers) + + misbehaviour := ibctmtypes.NewMisbehaviour("tendermint", suite.chain.ChainID, header1, header2) + msg, err = types.NewMsgSubmitMisbehaviour("tendermint", misbehaviour, suite.chain.SenderAccount.GetAddress()) + suite.Require().NoError(err) + + }, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() + + tc.malleate() + + cdc := suite.chain.App.AppCodec() + + // marshal message + bz, err := cdc.MarshalJSON(msg) + suite.Require().NoError(err) + + // unmarshal message + newMsg := &types.MsgSubmitMisbehaviour{} + err = cdc.UnmarshalJSON(bz, newMsg) + suite.Require().NoError(err) + + suite.Require().True(proto.Equal(msg, newMsg)) + }) + } +} + func (suite *TypesTestSuite) TestMsgSubmitMisbehaviour_ValidateBasic() { var ( msg = &types.MsgSubmitMisbehaviour{} diff --git a/x/ibc/07-tendermint/types/codec.go b/x/ibc/07-tendermint/types/codec.go index 9a8028006e..02ca445247 100644 --- a/x/ibc/07-tendermint/types/codec.go +++ b/x/ibc/07-tendermint/types/codec.go @@ -18,8 +18,8 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { &ConsensusState{}, ) registry.RegisterImplementations( - (*clientexported.Misbehaviour)(nil), - &Misbehaviour{}, + (*clientexported.Header)(nil), + &Header{}, ) registry.RegisterImplementations( (*clientexported.Misbehaviour)(nil),