Rename DeserializeTx, enforce []sdk.Msg usage in SerializeCosmosTx (#457

) * rename DeserializeTx to DeserializeCosmosTx, simply serialization logic * improve godoc wording
cosmos · Oct 6, 2021 · 064aa42 · 064aa42
1 parent e24294c
commit 064aa42
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 19 deletions.
diff --git a/modules/apps/27-interchain-accounts/keeper/keeper.go b/modules/apps/27-interchain-accounts/keeper/keeper.go
@@ -58,17 +58,7 @@ func NewKeeper(
 }
 
 // SerializeCosmosTx marshals data to bytes using the provided codec
-func (k Keeper) SerializeCosmosTx(cdc codec.BinaryCodec, data interface{}) ([]byte, error) {
-	msgs := make([]sdk.Msg, 0)
-	switch data := data.(type) {
-	case sdk.Msg:
-		msgs = append(msgs, data)
-	case []sdk.Msg:
-		msgs = append(msgs, data...)
-	default:
-		return nil, types.ErrInvalidOutgoingData
-	}
-
+func (k Keeper) SerializeCosmosTx(cdc codec.BinaryCodec, msgs []sdk.Msg) ([]byte, error) {
 	msgAnys := make([]*codectypes.Any, len(msgs))
 
 	for i, msg := range msgs {

diff --git a/modules/apps/27-interchain-accounts/keeper/relay.go b/modules/apps/27-interchain-accounts/keeper/relay.go
@@ -46,9 +46,20 @@ func (k Keeper) createOutgoingPacket(
 		return []byte{}, types.ErrInvalidOutgoingData
 	}
 
-	txBytes, err := k.SerializeCosmosTx(k.cdc, data)
+	var (
+		txBytes []byte
+		err     error
+	)
+
+	switch data := data.(type) {
+	case []sdk.Msg:
+		txBytes, err = k.SerializeCosmosTx(k.cdc, data)
+	default:
+		return nil, sdkerrors.Wrapf(types.ErrInvalidOutgoingData, "message type %T is not supported", data)
+	}
+
 	if err != nil {
-		return []byte{}, sdkerrors.Wrap(err, "invalid packet data or codec")
+		return nil, sdkerrors.Wrap(err, "serialization of transaction data failed")
 	}
 
 	channelCap, ok := k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(sourcePort, sourceChannel))
@@ -86,7 +97,9 @@ func (k Keeper) createOutgoingPacket(
 	return k.ComputeVirtualTxHash(packetData.Data, packet.Sequence), k.channelKeeper.SendPacket(ctx, channelCap, packet)
 }
 
-func (k Keeper) DeserializeTx(_ sdk.Context, txBytes []byte) ([]sdk.Msg, error) {
+// DeserializeCosmosTx unmarshals and unpacks a slice of transaction bytes
+// into a slice of sdk.Msg's.
+func (k Keeper) DeserializeCosmosTx(_ sdk.Context, txBytes []byte) ([]sdk.Msg, error) {
 	var txBody types.IBCTxBody
 
 	if err := k.cdc.Unmarshal(txBytes, &txBody); err != nil {
@@ -191,7 +204,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) error
 
 	switch data.Type {
 	case types.EXECUTE_TX:
-		msgs, err := k.DeserializeTx(ctx, data.Data)
+		msgs, err := k.DeserializeCosmosTx(ctx, data.Data)
 		if err != nil {
 			return err
 		}

diff --git a/modules/apps/27-interchain-accounts/keeper/relay_test.go b/modules/apps/27-interchain-accounts/keeper/relay_test.go
@@ -29,11 +29,11 @@ func (suite *KeeperTestSuite) TestTrySendTx() {
 			"success", func() {
 				amount, _ := sdk.ParseCoinsNormalized("100stake")
 				interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID)
-				msg = &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
+				msg = []sdk.Msg{&banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}}
 			}, true,
 		},
 		{
-			"success with []sdk.Message", func() {
+			"success with multiple sdk.Msg", func() {
 				amount, _ := sdk.ParseCoinsNormalized("100stake")
 				interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID)
 				msg1 := &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
@@ -46,6 +46,13 @@ func (suite *KeeperTestSuite) TestTrySendTx() {
 				msg = []byte{}
 			}, false,
 		},
+		{
+			"incorrect outgoing data - []sdk.Msg is not used", func() {
+				amount, _ := sdk.ParseCoinsNormalized("100stake")
+				interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID)
+				msg = &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
+			}, false,
+		},
 		{
 			"active channel not found", func() {
 				amount, _ := sdk.ParseCoinsNormalized("100stake")
@@ -122,7 +129,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID)
 				msg = &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
 				// build packet data
-				txBytes, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, msg)
+				txBytes, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg})
 				suite.Require().NoError(err)
 
 				data := types.InterchainAccountPacketData{Type: types.EXECUTE_TX,
@@ -165,7 +172,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 				// Incorrect FromAddress
 				msg = &banktypes.MsgSend{FromAddress: suite.chainB.SenderAccount.GetAddress().String(), ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}
 				// build packet data
-				txBytes, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, msg)
+				txBytes, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg})
 				suite.Require().NoError(err)
 				data := types.InterchainAccountPacketData{Type: types.EXECUTE_TX,
 					Data: txBytes}