Add missing channel upgrade tests (cosmos#5502)

modules/apps/transfer/ibc_module_test.go

@@ -32,6 +32,13 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() {
 		{
 			"success", func() {}, true,
 		},
+		{
+			// connection hops is not used in the transfer application callback,
+			// it is already validated in the core OnChanUpgradeInit.
+			"success: invalid connection hops", func() {
+				path.EndpointA.ConnectionID = "invalid-connection-id"
+			}, true,
+		},
 		{
 			"empty version string", func() {
 				channel.Version = ""

modules/core/04-channel/types/msgs.go

@@ -25,6 +25,10 @@ var (
 	_ sdk.Msg = (*MsgAcknowledgement)(nil)
 	_ sdk.Msg = (*MsgTimeout)(nil)
 	_ sdk.Msg = (*MsgTimeoutOnClose)(nil)
+	_ sdk.Msg = (*MsgChannelUpgradeConfirm)(nil)
+	_ sdk.Msg = (*MsgChannelUpgradeTimeout)(nil)
+	_ sdk.Msg = (*MsgChannelUpgradeCancel)(nil)
+	_ sdk.Msg = (*MsgPruneAcknowledgements)(nil)
 
 	_ sdk.HasValidateBasic = (*MsgChannelOpenInit)(nil)
 	_ sdk.HasValidateBasic = (*MsgChannelOpenTry)(nil)
@@ -40,6 +44,9 @@ var (
 	_ sdk.HasValidateBasic = (*MsgChannelUpgradeTry)(nil)
 	_ sdk.HasValidateBasic = (*MsgChannelUpgradeAck)(nil)
 	_ sdk.HasValidateBasic = (*MsgChannelUpgradeConfirm)(nil)
+	_ sdk.HasValidateBasic = (*MsgChannelUpgradeTimeout)(nil)
+	_ sdk.HasValidateBasic = (*MsgChannelUpgradeCancel)(nil)
+	_ sdk.HasValidateBasic = (*MsgPruneAcknowledgements)(nil)
 )
 
 // NewMsgChannelOpenInit creates a new MsgChannelOpenInit. It sets the counterparty channel

modules/core/04-channel/types/msgs_test.go

@@ -15,6 +15,8 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 
 	ibc "github.com/cosmos/ibc-go/v8/modules/core"
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
@@ -1255,6 +1257,61 @@ func (suite *TypesTestSuite) TestMsgPruneAcknowledgementsValidateBasic() {
 	}
 }
 
+func (suite *TypesTestSuite) TestMsgUpdateParamsValidateBasic() {
+	var msg *types.MsgUpdateParams
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expErr   error
+	}{
+		{
+			"success",
+			func() {},
+			nil,
+		},
+		{
+			"invalid authority",
+			func() {
+				msg.Authority = "invalid-address"
+			},
+			ibcerrors.ErrInvalidAddress,
+		},
+		{
+			"invalid params: non zero height",
+			func() {
+				newHeight := clienttypes.NewHeight(1, 1000)
+				msg = types.NewMsgUpdateChannelParams(authtypes.NewModuleAddress(govtypes.ModuleName).String(), types.NewParams(types.NewTimeout(newHeight, uint64(100000))))
+			},
+			types.ErrInvalidUpgradeTimeout,
+		},
+		{
+			"invalid params: zero timestamp",
+			func() {
+				msg = types.NewMsgUpdateChannelParams(authtypes.NewModuleAddress(govtypes.ModuleName).String(), types.NewParams(types.NewTimeout(clienttypes.ZeroHeight(), uint64(0))))
+			},
+			types.ErrInvalidUpgradeTimeout,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			msg = types.NewMsgUpdateChannelParams(authtypes.NewModuleAddress(govtypes.ModuleName).String(), types.NewParams(types.NewTimeout(clienttypes.ZeroHeight(), uint64(100000))))
+
+			tc.malleate()
+			err := msg.ValidateBasic()
+
+			expPass := tc.expErr == nil
+			if expPass {
+				suite.Require().NoError(err)
+			} else {
+				suite.Require().ErrorIs(err, tc.expErr)
+			}
+		})
+	}
+}
+
 func (suite *TypesTestSuite) TestMsgPruneAcknowledgementsGetSigners() {
 	expSigner, err := sdk.AccAddressFromBech32(addr)
 	suite.Require().NoError(err)

modules/core/04-channel/types/params.go

@@ -1,9 +1,10 @@
 package types
 
 import (
-	"fmt"
 	"time"
 
+	errorsmod "cosmossdk.io/errors"
+
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 )
 
@@ -27,10 +28,10 @@ func DefaultParams() Params {
 // Validate the params.
 func (p Params) Validate() error {
 	if !p.UpgradeTimeout.Height.IsZero() {
-		return fmt.Errorf("upgrade timeout height must be zero. got : %v", p.UpgradeTimeout.Height)
+		return errorsmod.Wrapf(ErrInvalidUpgradeTimeout, "upgrade timeout height must be zero. got : %v", p.UpgradeTimeout.Height)
 	}
 	if p.UpgradeTimeout.Timestamp == 0 {
-		return fmt.Errorf("upgrade timeout timestamp invalid: %v", p.UpgradeTimeout.Timestamp)
+		return errorsmod.Wrapf(ErrInvalidUpgradeTimeout, "upgrade timeout timestamp invalid: %v", p.UpgradeTimeout.Timestamp)
 	}
 	return nil
 }

modules/core/keeper/msg_server_test.go

@@ -2021,53 +2021,51 @@ func (suite *KeeperTestSuite) TestUpdateConnectionParams() {
 func (suite *KeeperTestSuite) TestUpdateChannelParams() {
 	authority := suite.chainA.App.GetIBCKeeper().GetAuthority()
 	testCases := []struct {
-		name    string
-		msg     *channeltypes.MsgUpdateParams
-		expPass bool
+		name     string
+		msg      *channeltypes.MsgUpdateParams
+		expError error
 	}{
 		{
 			"success: valid authority and default params",
 			channeltypes.NewMsgUpdateChannelParams(authority, channeltypes.DefaultParams()),
-			true,
+			nil,
 		},
 		{
 			"failure: malformed authority address",
 			channeltypes.NewMsgUpdateChannelParams(ibctesting.InvalidID, channeltypes.DefaultParams()),
-			false,
+			ibcerrors.ErrUnauthorized,
 		},
 		{
 			"failure: empty authority address",
 			channeltypes.NewMsgUpdateChannelParams("", channeltypes.DefaultParams()),
-			false,
+			ibcerrors.ErrUnauthorized,
 		},
 		{
-			"failure: whitespace authority address",
-			channeltypes.NewMsgUpdateChannelParams("    ", channeltypes.DefaultParams()),
-			false,
+			"failure: empty signer",
+			channeltypes.NewMsgUpdateChannelParams("", channeltypes.DefaultParams()),
+			ibcerrors.ErrUnauthorized,
 		},
 		{
 			"failure: unauthorized authority address",
 			channeltypes.NewMsgUpdateChannelParams(ibctesting.TestAccAddress, channeltypes.DefaultParams()),
-			false,
+			ibcerrors.ErrUnauthorized,
 		},
 	}
 
 	for _, tc := range testCases {
 		tc := tc
 		suite.Run(tc.name, func() {
 			suite.SetupTest()
-
 			resp, err := keeper.Keeper.UpdateChannelParams(*suite.chainA.App.GetIBCKeeper(), suite.chainA.GetContext(), tc.msg)
-
-			if tc.expPass {
+			expPass := tc.expError == nil
+			if expPass {
 				suite.Require().NoError(err)
 				suite.Require().NotNil(resp)
-
 				p := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetParams(suite.chainA.GetContext())
 				suite.Require().Equal(tc.msg.Params, p)
 			} else {
-				suite.Require().Error(err)
 				suite.Require().Nil(resp)
+				suite.Require().ErrorIs(tc.expError, err)
 			}
 		})
 	}