feat(transfer): add validation for forwarding info in msg transfer va…

…lidate basic (#6583)
cosmos · Jun 13, 2024 · ba7c4a8 · ba7c4a8
1 parent ae2046a
commit ba7c4a8
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 36 deletions.
diff --git a/modules/apps/transfer/types/msgs.go b/modules/apps/transfer/types/msgs.go
@@ -102,6 +102,17 @@ func (msg MsgTransfer) ValidateBasic() error {
 		return errorsmod.Wrapf(ErrInvalidMemo, "memo must not exceed %d bytes", MaximumMemoLength)
 	}
 
+	if msg.ForwardingPath != nil {
+		if err := msg.ForwardingPath.Validate(); err != nil {
+			return err
+		}
+
+		// We cannot have non-empty memo and non-empty forwarding path hops at the same time.
+		if len(msg.ForwardingPath.Hops) > 0 && msg.Memo != "" {
+			return errorsmod.Wrapf(ErrInvalidMemo, "memo must be empty if forwarding path hops is not empty: %s, %s", msg.Memo, msg.ForwardingPath.Hops)
+		}
+	}
+
 	for _, coin := range msg.GetCoins() {
 		if err := validateIBCCoin(coin); err != nil {
 			return errorsmod.Wrapf(ibcerrors.ErrInvalidCoins, "%s: %s", err.Error(), coin.String())

diff --git a/modules/apps/transfer/types/msgs_test.go b/modules/apps/transfer/types/msgs_test.go
@@ -79,19 +79,24 @@ func TestMsgTransferValidation(t *testing.T) {
 		{"multidenom: zero coins", types.NewMsgTransfer(validPort, validChannel, zeroCoins, sender, receiver, timeoutHeight, 0, "", nil), ibcerrors.ErrInvalidCoins},
 		{"multidenom: too many coins", types.NewMsgTransfer(validPort, validChannel, make([]sdk.Coin, types.MaximumTokensLength+1), sender, receiver, timeoutHeight, 0, "", nil), ibcerrors.ErrInvalidCoins},
 		{"multidenom: both token and tokens are set", &types.MsgTransfer{validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, "", coins, nil}, ibcerrors.ErrInvalidCoins},
+		{"memo must be empty if forwarding path hops is not empty", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "memo", types.NewForwardingInfo("", validHop)), types.ErrInvalidMemo},
+		{"invalid forwarding info port", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "", types.NewForwardingInfo("", types.Hop{PortId: invalidPort, ChannelId: validChannel})), host.ErrInvalidID},
+		{"invalid forwarding info channel", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "", types.NewForwardingInfo("", types.Hop{PortId: validPort, ChannelId: invalidChannel})), host.ErrInvalidID},
+		{"invalid forwarding info too many hops", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "", types.NewForwardingInfo("", generateHops(types.MaximumNumberOfForwardingHops+1)...)), types.ErrInvalidForwardingInfo},
+		{"invalid forwarding info too long memo", types.NewMsgTransfer(validPort, validChannel, coins, sender, receiver, timeoutHeight, 0, "", types.NewForwardingInfo(ibctesting.GenerateString(types.MaximumMemoLength+1), validHop)), types.ErrInvalidMemo},
 	}
 
 	for _, tc := range testCases {
-		tc := tc
-
-		err := tc.msg.ValidateBasic()
-
-		expPass := tc.expError == nil
-		if expPass {
-			require.NoError(t, err)
-		} else {
-			require.ErrorIs(t, err, tc.expError)
-		}
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.msg.ValidateBasic()
+
+			expPass := tc.expError == nil
+			if expPass {
+				require.NoError(t, err)
+			} else {
+				require.ErrorIs(t, err, tc.expError)
+			}
+		})
 	}
 }
 
@@ -119,16 +124,16 @@ func TestMsgUpdateParamsValidateBasic(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		tc := tc
-
-		err := tc.msg.ValidateBasic()
-
-		expPass := tc.expError == nil
-		if expPass {
-			require.NoError(t, err)
-		} else {
-			require.ErrorIs(t, err, tc.expError)
-		}
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.msg.ValidateBasic()
+
+			expPass := tc.expError == nil
+			if expPass {
+				require.NoError(t, err)
+			} else {
+				require.ErrorIs(t, err, tc.expError)
+			}
+		})
 	}
 }
 
@@ -144,21 +149,21 @@ func TestMsgUpdateParamsGetSigners(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		tc := tc
-
-		msg := types.MsgUpdateParams{
-			Signer: tc.address.String(),
-			Params: types.DefaultParams(),
-		}
-
-		encodingCfg := moduletestutil.MakeTestEncodingConfig(transfer.AppModuleBasic{})
-		signers, _, err := encodingCfg.Codec.GetMsgV1Signers(&msg)
-
-		if tc.expPass {
-			require.NoError(t, err)
-			require.Equal(t, tc.address.Bytes(), signers[0])
-		} else {
-			require.Error(t, err)
-		}
+		t.Run(tc.name, func(t *testing.T) {
+			msg := types.MsgUpdateParams{
+				Signer: tc.address.String(),
+				Params: types.DefaultParams(),
+			}
+
+			encodingCfg := moduletestutil.MakeTestEncodingConfig(transfer.AppModuleBasic{})
+			signers, _, err := encodingCfg.Codec.GetMsgV1Signers(&msg)
+
+			if tc.expPass {
+				require.NoError(t, err)
+				require.Equal(t, tc.address.Bytes(), signers[0])
+			} else {
+				require.Error(t, err)
+			}
+		})
 	}
 }