diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index 6d7df22781a..d383533004f 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/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 = "" diff --git a/modules/core/04-channel/types/msgs.go b/modules/core/04-channel/types/msgs.go index 9be788895c9..c1d708b480e 100644 --- a/modules/core/04-channel/types/msgs.go +++ b/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 diff --git a/modules/core/04-channel/types/msgs_test.go b/modules/core/04-channel/types/msgs_test.go index 5a7bec0c889..23bf80aa297 100644 --- a/modules/core/04-channel/types/msgs_test.go +++ b/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) diff --git a/modules/core/04-channel/types/params.go b/modules/core/04-channel/types/params.go index cf44ae307c0..688c55dc721 100644 --- a/modules/core/04-channel/types/params.go +++ b/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 } diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 01cc0ba4665..25b0fb9729f 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -2021,34 +2021,34 @@ 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, }, } @@ -2056,18 +2056,16 @@ func (suite *KeeperTestSuite) TestUpdateChannelParams() { 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) } }) }