From 51874f282aa0b9bc0cdcb23bff5f59b105aefc3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 26 Oct 2022 17:03:16 +0200 Subject: [PATCH 01/10] refactor: SerailzeCosmosTx does not require a fully implemented sdk.Msg for serialization --- .../controller/keeper/msg_server_test.go | 3 +- .../controller/keeper/relay_test.go | 7 +-- .../controller/types/msgs_test.go | 5 ++- .../host/client/cli/tx.go | 9 ++-- .../host/ibc_module_test.go | 4 +- .../host/keeper/relay_test.go | 27 +++++------ .../27-interchain-accounts/types/codec.go | 3 +- .../types/codec_test.go | 45 ++++++++++--------- modules/apps/29-fee/ica_test.go | 3 +- 9 files changed, 58 insertions(+), 48 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go b/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go index 013e2053901..0a9e77a130b 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go @@ -5,6 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/gogo/protobuf/proto" "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/keeper" "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/types" @@ -167,7 +168,7 @@ func (suite *KeeperTestSuite) TestSubmitTx() { Amount: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))), } - data, err := icatypes.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{icaMsg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.Codec, []proto.Message{icaMsg}) suite.Require().NoError(err) packetData := icatypes.InterchainAccountPacketData{ diff --git a/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go b/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go index 8a8a95afbe1..faa13f80365 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go @@ -3,6 +3,7 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/gogo/protobuf/proto" icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types" clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" @@ -34,7 +35,7 @@ func (suite *KeeperTestSuite) TestSendTx() { Amount: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))), } - data, err := icatypes.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) packetData = icatypes.InterchainAccountPacketData{ @@ -50,7 +51,7 @@ func (suite *KeeperTestSuite) TestSendTx() { interchainAccountAddr, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID) suite.Require().True(found) - msgsBankSend := []sdk.Msg{ + msgsBankSend := []proto.Message{ &banktypes.MsgSend{ FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), @@ -121,7 +122,7 @@ func (suite *KeeperTestSuite) TestSendTx() { Amount: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))), } - data, err := icatypes.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) packetData = icatypes.InterchainAccountPacketData{ diff --git a/modules/apps/27-interchain-accounts/controller/types/msgs_test.go b/modules/apps/27-interchain-accounts/controller/types/msgs_test.go index 933798560ce..6e6487c0b66 100644 --- a/modules/apps/27-interchain-accounts/controller/types/msgs_test.go +++ b/modules/apps/27-interchain-accounts/controller/types/msgs_test.go @@ -5,6 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/require" "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/types" @@ -155,7 +156,7 @@ func TestMsgSendTxValidateBasic(t *testing.T) { Amount: ibctesting.TestCoins, } - data, err := icatypes.SerializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, []sdk.Msg{msgBankSend}) + data, err := icatypes.SerializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, []proto.Message{msgBankSend}) require.NoError(t, err) packetData := icatypes.InterchainAccountPacketData{ @@ -191,7 +192,7 @@ func TestMsgSendTxGetSigners(t *testing.T) { Amount: ibctesting.TestCoins, } - data, err := icatypes.SerializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, []sdk.Msg{msgBankSend}) + data, err := icatypes.SerializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, []proto.Message{msgBankSend}) require.NoError(t, err) packetData := icatypes.InterchainAccountPacketData{ diff --git a/modules/apps/27-interchain-accounts/host/client/cli/tx.go b/modules/apps/27-interchain-accounts/host/client/cli/tx.go index b72769bcda1..fba73700daa 100644 --- a/modules/apps/27-interchain-accounts/host/client/cli/tx.go +++ b/modules/apps/27-interchain-accounts/host/client/cli/tx.go @@ -8,6 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/version" + proto "github.com/gogo/protobuf/proto" "github.com/spf13/cobra" icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types" @@ -100,7 +101,7 @@ func generatePacketData(cdc *codec.ProtoCodec, msgBytes []byte, memo string) ([] // convertBytesIntoSdkMessages returns a list of sdk messages from bytes. The bytes can be in the form of a single // message, or a json array of messages. -func convertBytesIntoSdkMessages(cdc *codec.ProtoCodec, msgBytes []byte) ([]sdk.Msg, error) { +func convertBytesIntoSdkMessages(cdc *codec.ProtoCodec, msgBytes []byte) ([]proto.Message, error) { var rawMessages []json.RawMessage if err := json.Unmarshal(msgBytes, &rawMessages); err != nil { // if we fail to unmarshal a list of messages, we assume we are just dealing with a single message. @@ -110,10 +111,10 @@ func convertBytesIntoSdkMessages(cdc *codec.ProtoCodec, msgBytes []byte) ([]sdk. return nil, err } - return []sdk.Msg{msg}, nil + return []proto.Message{msg}, nil } - sdkMessages := make([]sdk.Msg, len(rawMessages)) + sdkMessages := make([]proto.Message, len(rawMessages)) for i, anyJSON := range rawMessages { var msg sdk.Msg if err := cdc.UnmarshalInterfaceJSON(anyJSON, &msg); err != nil { @@ -127,7 +128,7 @@ func convertBytesIntoSdkMessages(cdc *codec.ProtoCodec, msgBytes []byte) ([]sdk. } // generateIcaPacketDataFromSdkMessages generates ica packet data as bytes from a given set of sdk messages and a memo. -func generateIcaPacketDataFromSdkMessages(cdc *codec.ProtoCodec, sdkMessages []sdk.Msg, memo string) ([]byte, error) { +func generateIcaPacketDataFromSdkMessages(cdc *codec.ProtoCodec, sdkMessages []proto.Message, memo string) ([]byte, error) { icaPacketDataBytes, err := icatypes.SerializeCosmosTx(cdc, sdkMessages) if err != nil { return nil, err diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 5bd6f916df5..02c085b4106 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -446,7 +446,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount, } - data, err := icatypes.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.Codec, []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -655,7 +655,7 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose() Amount: tokenAmt, } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go index e4378373fc5..50af32bd7c3 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go @@ -8,6 +8,7 @@ import ( govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/gogo/protobuf/proto" "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types" @@ -55,7 +56,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { Option: govtypes.OptionYes, } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -82,7 +83,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { Amount: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))), } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -110,7 +111,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { Amount: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(5000)), } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -144,7 +145,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { Amount: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(5000)), } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msgDelegate, msgUndelegate}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msgDelegate, msgUndelegate}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -179,7 +180,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { Proposer: interchainAccountAddr, } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -221,7 +222,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { Option: govtypes.OptionYes, } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -247,7 +248,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { Depositor: interchainAccountAddr, } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -273,7 +274,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { WithdrawAddress: suite.chainB.SenderAccount.GetAddress().String(), } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -312,7 +313,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { TimeoutTimestamp: uint64(0), } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -351,7 +352,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { { "invalid packet type - UNSPECIFIED", func() { - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{&banktypes.MsgSend{}}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{&banktypes.MsgSend{}}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -368,7 +369,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { func() { path.EndpointA.ChannelConfig.PortID = "invalid-port-id" - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{&banktypes.MsgSend{}}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{&banktypes.MsgSend{}}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -389,7 +390,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { Amount: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))), } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ @@ -410,7 +411,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { Amount: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))), } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msg}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ diff --git a/modules/apps/27-interchain-accounts/types/codec.go b/modules/apps/27-interchain-accounts/types/codec.go index 4c9fdea401d..852922ee12b 100644 --- a/modules/apps/27-interchain-accounts/types/codec.go +++ b/modules/apps/27-interchain-accounts/types/codec.go @@ -6,6 +6,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + proto "github.com/gogo/protobuf/proto" ) // ModuleCdc references the global interchain accounts module codec. Note, the codec @@ -25,7 +26,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { // SerializeCosmosTx serializes a slice of sdk.Msg's using the CosmosTx type. The sdk.Msg's are // packed into Any's and inserted into the Messages field of a CosmosTx. The proto marshaled CosmosTx // bytes are returned. Only the ProtoCodec is supported for serializing messages. -func SerializeCosmosTx(cdc codec.BinaryCodec, msgs []sdk.Msg) (bz []byte, err error) { +func SerializeCosmosTx(cdc codec.BinaryCodec, msgs []proto.Message) (bz []byte, err error) { // only ProtoCodec is supported if _, ok := cdc.(*codec.ProtoCodec); !ok { return nil, sdkerrors.Wrap(ErrInvalidCodec, "only ProtoCodec is supported for receiving messages on the host chain") diff --git a/modules/apps/27-interchain-accounts/types/codec_test.go b/modules/apps/27-interchain-accounts/types/codec_test.go index f058053cdf4..01a2d91bd46 100644 --- a/modules/apps/27-interchain-accounts/types/codec_test.go +++ b/modules/apps/27-interchain-accounts/types/codec_test.go @@ -5,6 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" + proto "github.com/gogo/protobuf/proto" "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types" "github.com/cosmos/ibc-go/v6/testing/simapp" @@ -46,12 +47,12 @@ func (mockSdkMsg) GetSigners() []sdk.AccAddress { func (suite *TypesTestSuite) TestSerializeAndDeserializeCosmosTx() { testCases := []struct { name string - msgs []sdk.Msg + msgs []proto.Message expPass bool }{ { "single msg", - []sdk.Msg{ + []proto.Message{ &banktypes.MsgSend{ FromAddress: TestOwnerAddress, ToAddress: TestOwnerAddress, @@ -62,7 +63,7 @@ func (suite *TypesTestSuite) TestSerializeAndDeserializeCosmosTx() { }, { "multiple msgs, same types", - []sdk.Msg{ + []proto.Message{ &banktypes.MsgSend{ FromAddress: TestOwnerAddress, ToAddress: TestOwnerAddress, @@ -78,7 +79,7 @@ func (suite *TypesTestSuite) TestSerializeAndDeserializeCosmosTx() { }, { "multiple msgs, different types", - []sdk.Msg{ + []proto.Message{ &banktypes.MsgSend{ FromAddress: TestOwnerAddress, ToAddress: TestOwnerAddress, @@ -93,14 +94,14 @@ func (suite *TypesTestSuite) TestSerializeAndDeserializeCosmosTx() { }, { "unregistered msg type", - []sdk.Msg{ + []proto.Message{ &mockSdkMsg{}, }, false, }, { "multiple unregistered msg types", - []sdk.Msg{ + []proto.Message{ &mockSdkMsg{}, &mockSdkMsg{}, &mockSdkMsg{}, @@ -109,29 +110,31 @@ func (suite *TypesTestSuite) TestSerializeAndDeserializeCosmosTx() { }, } - testCasesAny := []caseRawBytes{} - for _, tc := range testCases { - bz, err := types.SerializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, tc.msgs) - suite.Require().NoError(err, tc.name) - - testCasesAny = append(testCasesAny, caseRawBytes{tc.name, bz, tc.expPass}) - } + tc := tc - for i, tc := range testCasesAny { - msgs, err := types.DeserializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, tc.bz) - if tc.expPass { + suite.Run(tc.name, func() { + bz, err := types.SerializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, tc.msgs) suite.Require().NoError(err, tc.name) - suite.Require().Equal(testCases[i].msgs, msgs, tc.name) - } else { - suite.Require().Error(err, tc.name) - } + + msgs, err := types.DeserializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, bz) + if tc.expPass { + suite.Require().NoError(err, tc.name) + } else { + suite.Require().Error(err, tc.name) + } + + for i, msg := range msgs { + suite.Require().Equal(tc.msgs[i], msg) + } + }) } // test deserializing unknown bytes msgs, err := types.DeserializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, []byte("invalid")) suite.Require().Error(err) suite.Require().Empty(msgs) + } // unregistered bytes causes amino to panic. @@ -141,7 +144,7 @@ func (suite *TypesTestSuite) TestDeserializeAndSerializeCosmosTxWithAmino() { cdc := codec.NewLegacyAmino() marshaler := codec.NewAminoCodec(cdc) - msgs, err := types.SerializeCosmosTx(marshaler, []sdk.Msg{&banktypes.MsgSend{}}) + msgs, err := types.SerializeCosmosTx(marshaler, []proto.Message{&banktypes.MsgSend{}}) suite.Require().Error(err) suite.Require().Empty(msgs) diff --git a/modules/apps/29-fee/ica_test.go b/modules/apps/29-fee/ica_test.go index 331f5e85abf..f77eb0ffca1 100644 --- a/modules/apps/29-fee/ica_test.go +++ b/modules/apps/29-fee/ica_test.go @@ -4,6 +4,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/gogo/protobuf/proto" icahosttypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types" @@ -146,7 +147,7 @@ func (suite *FeeTestSuite) TestFeeInterchainAccounts() { Amount: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(5000)), } - data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []sdk.Msg{msgDelegate}) + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msgDelegate}) suite.Require().NoError(err) icaPacketData := icatypes.InterchainAccountPacketData{ From 123801ba49cc66effcb373ad9b958e5780b2e347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 26 Oct 2022 17:04:27 +0200 Subject: [PATCH 02/10] update docs --- docs/apps/interchain-accounts/auth-modules.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/apps/interchain-accounts/auth-modules.md b/docs/apps/interchain-accounts/auth-modules.md index dbf917d11d4..2c35062aaca 100644 --- a/docs/apps/interchain-accounts/auth-modules.md +++ b/docs/apps/interchain-accounts/auth-modules.md @@ -241,7 +241,7 @@ if !found { // The appropriate serialization function should be called. The host chain must be able to deserialize the transaction. // If the host chain is using the ibc-go host module, `SerializeCosmosTx` should be used. msg := &banktypes.MsgSend{FromAddress: fromAddr, ToAddress: toAddr, Amount: amt} -data, err := icatypes.SerializeCosmosTx(keeper.cdc, []sdk.Msg{msg}) +data, err := icatypes.SerializeCosmosTx(keeper.cdc, []proto.Message{msg}) if err != nil { return err } From c590486c521e9da87cde4638bc3bd294e5412b61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 26 Oct 2022 17:12:15 +0200 Subject: [PATCH 03/10] add changelog entry and migration docs --- CHANGELOG.md | 1 + docs/migrations/v5-to-v6.md | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53b9e388ac7..9e85ca7d323 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking +* (apps/27-interchain-accounts) [\#2607](https://github.com/cosmos/ibc-go/pull/2607) `SerializeCosmosTx` now takes in a `[]proto.Message` instead of `[]sdk.Msg`. * (apps/transfer) [\#2446](https://github.com/cosmos/ibc-go/pull/2446) Remove `SendTransfer` function in favor of a private `sendTransfer` function. All IBC transfers must be initiated with `MsgTransfer`. * (apps/29-fee) [\#2395](https://github.com/cosmos/ibc-go/pull/2395) Remove param space from ics29 NewKeeper function. The field was unused. * (apps/27-interchain-accounts) [\#2133](https://github.com/cosmos/ibc-go/pull/2133) Generates genesis protos in a separate directory to avoid circular import errors. The protobuf package name has changed for the genesis types. diff --git a/docs/migrations/v5-to-v6.md b/docs/migrations/v5-to-v6.md index b7795ea7d56..104dc93b650 100644 --- a/docs/migrations/v5-to-v6.md +++ b/docs/migrations/v5-to-v6.md @@ -123,6 +123,8 @@ func DefaultParams() Params { #### API breaking changes +`SerializeCosmosTx` takes in a `[]proto.Message` instead of `[]sdk.Message`. This allows for the serialization of proto messages without requiring the fulfillment of the `sdk.Msg` interface. + The `27-interchain-accounts` genesis types have been moved to their own package: `modules/apps/27-interchain-acccounts/genesis/types`. This change facilitates the addition of the ICS27 controller submodule `MsgServer` and avoids cyclic imports. This should have minimal disruption to chain developers integrating `27-interchain-accounts`. @@ -138,6 +140,7 @@ func NewKeeper( ) Keeper ``` + ### ICS29 - `NewKeeper` API change The `NewKeeper` function of ICS29 has been updated to remove the `paramSpace` parameter as it was unused. From 6a6d9cad1fde2d296b13ae00c26c6be335f4f97f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 26 Oct 2022 17:12:46 +0200 Subject: [PATCH 04/10] Update docs/migrations/v5-to-v6.md --- docs/migrations/v5-to-v6.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/migrations/v5-to-v6.md b/docs/migrations/v5-to-v6.md index 104dc93b650..023356d3f89 100644 --- a/docs/migrations/v5-to-v6.md +++ b/docs/migrations/v5-to-v6.md @@ -140,7 +140,6 @@ func NewKeeper( ) Keeper ``` - ### ICS29 - `NewKeeper` API change The `NewKeeper` function of ICS29 has been updated to remove the `paramSpace` parameter as it was unused. From d2c28fbe7a105009b90d61667d4d4cda1099ef6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 26 Oct 2022 17:13:17 +0200 Subject: [PATCH 05/10] Update modules/apps/27-interchain-accounts/host/client/cli/tx.go --- modules/apps/27-interchain-accounts/host/client/cli/tx.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/host/client/cli/tx.go b/modules/apps/27-interchain-accounts/host/client/cli/tx.go index fba73700daa..c4d1f6aa594 100644 --- a/modules/apps/27-interchain-accounts/host/client/cli/tx.go +++ b/modules/apps/27-interchain-accounts/host/client/cli/tx.go @@ -8,7 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/version" - proto "github.com/gogo/protobuf/proto" + "github.com/gogo/protobuf/proto" "github.com/spf13/cobra" icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types" From 350169156bc619863700348df03c193ca1ce7fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 26 Oct 2022 17:14:01 +0200 Subject: [PATCH 06/10] Update modules/apps/27-interchain-accounts/types/codec_test.go --- modules/apps/27-interchain-accounts/types/codec_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/types/codec_test.go b/modules/apps/27-interchain-accounts/types/codec_test.go index 01a2d91bd46..d6d785438b7 100644 --- a/modules/apps/27-interchain-accounts/types/codec_test.go +++ b/modules/apps/27-interchain-accounts/types/codec_test.go @@ -5,7 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - proto "github.com/gogo/protobuf/proto" + "github.com/gogo/protobuf/proto" "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types" "github.com/cosmos/ibc-go/v6/testing/simapp" From 0b01fb31bfaca3791762243138818a4a4e3a02c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 26 Oct 2022 17:14:26 +0200 Subject: [PATCH 07/10] Update modules/apps/27-interchain-accounts/types/codec_test.go --- modules/apps/27-interchain-accounts/types/codec_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/types/codec_test.go b/modules/apps/27-interchain-accounts/types/codec_test.go index d6d785438b7..c2f6e64e632 100644 --- a/modules/apps/27-interchain-accounts/types/codec_test.go +++ b/modules/apps/27-interchain-accounts/types/codec_test.go @@ -134,7 +134,6 @@ func (suite *TypesTestSuite) TestSerializeAndDeserializeCosmosTx() { msgs, err := types.DeserializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, []byte("invalid")) suite.Require().Error(err) suite.Require().Empty(msgs) - } // unregistered bytes causes amino to panic. From faf254f8318b76baa21fdf62878e239558eba000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 26 Oct 2022 17:14:35 +0200 Subject: [PATCH 08/10] Update modules/apps/27-interchain-accounts/types/codec.go --- modules/apps/27-interchain-accounts/types/codec.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/types/codec.go b/modules/apps/27-interchain-accounts/types/codec.go index 852922ee12b..521971e06bc 100644 --- a/modules/apps/27-interchain-accounts/types/codec.go +++ b/modules/apps/27-interchain-accounts/types/codec.go @@ -6,7 +6,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - proto "github.com/gogo/protobuf/proto" + "github.com/gogo/protobuf/proto" ) // ModuleCdc references the global interchain accounts module codec. Note, the codec From 893b3d32d470805d8a5f3b2cc0dbb6deb12b321c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 27 Oct 2022 16:25:49 +0200 Subject: [PATCH 09/10] chore: apply review suggestions --- .../host/client/cli/tx.go | 12 +++++------ .../host/keeper/relay_test.go | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/modules/apps/27-interchain-accounts/host/client/cli/tx.go b/modules/apps/27-interchain-accounts/host/client/cli/tx.go index fba73700daa..d549ca33f6a 100644 --- a/modules/apps/27-interchain-accounts/host/client/cli/tx.go +++ b/modules/apps/27-interchain-accounts/host/client/cli/tx.go @@ -91,17 +91,17 @@ which submits pre-built packet data containing messages to be executed on the ho // generatePacketData takes in message bytes and a memo and serializes the message into an // instance of InterchainAccountPacketData which is returned as bytes. func generatePacketData(cdc *codec.ProtoCodec, msgBytes []byte, memo string) ([]byte, error) { - sdkMessages, err := convertBytesIntoSdkMessages(cdc, msgBytes) + protoMessages, err := convertBytesIntoProtoMessages(cdc, msgBytes) if err != nil { return nil, err } - return generateIcaPacketDataFromSdkMessages(cdc, sdkMessages, memo) + return generateIcaPacketDataFromProtoMessages(cdc, protoMessages, memo) } -// convertBytesIntoSdkMessages returns a list of sdk messages from bytes. The bytes can be in the form of a single +// convertBytesIntoProtoMessages returns a list of proto messages from bytes. The bytes can be in the form of a single // message, or a json array of messages. -func convertBytesIntoSdkMessages(cdc *codec.ProtoCodec, msgBytes []byte) ([]proto.Message, error) { +func convertBytesIntoProtoMessages(cdc *codec.ProtoCodec, msgBytes []byte) ([]proto.Message, error) { var rawMessages []json.RawMessage if err := json.Unmarshal(msgBytes, &rawMessages); err != nil { // if we fail to unmarshal a list of messages, we assume we are just dealing with a single message. @@ -127,8 +127,8 @@ func convertBytesIntoSdkMessages(cdc *codec.ProtoCodec, msgBytes []byte) ([]prot return sdkMessages, nil } -// generateIcaPacketDataFromSdkMessages generates ica packet data as bytes from a given set of sdk messages and a memo. -func generateIcaPacketDataFromSdkMessages(cdc *codec.ProtoCodec, sdkMessages []proto.Message, memo string) ([]byte, error) { +// generateIcaPacketDataFromProtoMessages generates ica packet data as bytes from a given set of proto encoded sdk messages and a memo. +func generateIcaPacketDataFromProtoMessages(cdc *codec.ProtoCodec, sdkMessages []proto.Message, memo string) ([]byte, error) { icaPacketDataBytes, err := icatypes.SerializeCosmosTx(cdc, sdkMessages) if err != nil { return nil, err diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go index 50af32bd7c3..1b488541f48 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go @@ -328,6 +328,26 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { }, true, }, + { + "unregistered sdk.Msg", + func() { + msg := &banktypes.MsgSendResponse{} + + data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}) + suite.Require().NoError(err) + + icaPacketData := icatypes.InterchainAccountPacketData{ + Type: icatypes.EXECUTE_TX, + Data: data, + } + + packetData = icaPacketData.GetBytes() + + params := types.NewParams(true, []string{"/" + proto.MessageName(msg)}) + suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params) + }, + false, + }, { "cannot unmarshal interchain account packet data", func() { From 0cdcc652340fb8327f8d79f3abe51e25964122d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 27 Oct 2022 16:32:38 +0200 Subject: [PATCH 10/10] chore: add test case for non sdk.Msg type --- modules/apps/27-interchain-accounts/types/codec_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/modules/apps/27-interchain-accounts/types/codec_test.go b/modules/apps/27-interchain-accounts/types/codec_test.go index c2f6e64e632..301488b9ec7 100644 --- a/modules/apps/27-interchain-accounts/types/codec_test.go +++ b/modules/apps/27-interchain-accounts/types/codec_test.go @@ -130,6 +130,15 @@ func (suite *TypesTestSuite) TestSerializeAndDeserializeCosmosTx() { }) } + // test serializing non sdk.Msg type + bz, err := types.SerializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, []proto.Message{&banktypes.MsgSendResponse{}}) + suite.Require().NoError(err) + suite.Require().NotEmpty(bz) + + // test deserializing unknown bytes + _, err = types.DeserializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, bz) + suite.Require().Error(err) // unregistered type + // test deserializing unknown bytes msgs, err := types.DeserializeCosmosTx(simapp.MakeTestEncodingConfig().Marshaler, []byte("invalid")) suite.Require().Error(err)