From e7916ecdee18aa4af2c6cf17e2c3c095382447e6 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 2 Jan 2024 09:57:21 +0000 Subject: [PATCH 1/7] test: adding test case for invalid connection id --- modules/apps/transfer/ibc_module_test.go | 7 +++++++ 1 file changed, 7 insertions(+) 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 = "" From 2616ac69016dec3d055704c12f09f7dd58df4be8 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 2 Jan 2024 10:00:49 +0000 Subject: [PATCH 2/7] chore: adding additional type assertions --- modules/core/04-channel/types/msgs.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/core/04-channel/types/msgs.go b/modules/core/04-channel/types/msgs.go index 9be788895c9..60b0dcd5fc7 100644 --- a/modules/core/04-channel/types/msgs.go +++ b/modules/core/04-channel/types/msgs.go @@ -40,6 +40,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 From 298c4fdf41c161ba54da059e9046c382aa196e28 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 2 Jan 2024 10:27:34 +0000 Subject: [PATCH 3/7] test: adding tests for msg update params validate basic --- modules/core/04-channel/types/msgs_test.go | 58 ++++++++++++++++++++++ modules/core/04-channel/types/params.go | 8 +-- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/modules/core/04-channel/types/msgs_test.go b/modules/core/04-channel/types/msgs_test.go index 5a7bec0c889..73b4eab1ee0 100644 --- a/modules/core/04-channel/types/msgs_test.go +++ b/modules/core/04-channel/types/msgs_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + dbm "github.com/cosmos/cosmos-db" testifysuite "github.com/stretchr/testify/suite" @@ -15,6 +16,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 +1258,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.ErrInvalidTimeout, + }, + { + "invalid params: zero timestamp", + func() { + msg = types.NewMsgUpdateChannelParams(authtypes.NewModuleAddress(govtypes.ModuleName).String(), types.NewParams(types.NewTimeout(clienttypes.ZeroHeight(), uint64(0)))) + }, + types.ErrInvalidTimeout, + }, + } + + 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..1b58a51c814 100644 --- a/modules/core/04-channel/types/params.go +++ b/modules/core/04-channel/types/params.go @@ -1,9 +1,11 @@ package types import ( - "fmt" "time" + + errorsmod "cosmossdk.io/errors" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" ) @@ -27,10 +29,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(ErrInvalidTimeout, "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(ErrInvalidTimeout, "upgrade timeout timestamp invalid: %v", p.UpgradeTimeout.Timestamp) } return nil } From e078446231253f5c0069ea8ae041252042b6fe27 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 2 Jan 2024 10:37:38 +0000 Subject: [PATCH 4/7] test: adding test for keeper UpdateChannelParams grpc function --- modules/core/keeper/msg_server_test.go | 52 ++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index ee8444514e5..153d4dd20b3 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -2017,6 +2017,58 @@ func (suite *KeeperTestSuite) TestUpdateConnectionParams() { } } +// TestUpdateChannelParams tests the UpdateChannelParams rpc handler +func (suite *KeeperTestSuite) TestUpdateChannelParams() { + signer := suite.chainA.App.GetIBCKeeper().GetAuthority() + testCases := []struct { + name string + msg *channeltypes.MsgUpdateParams + expError error + }{ + { + "success: valid signer and default params", + channeltypes.NewMsgUpdateChannelParams(signer, channeltypes.DefaultParams()), + nil, + }, + { + "failure: malformed signer", + channeltypes.NewMsgUpdateChannelParams(ibctesting.InvalidID, channeltypes.DefaultParams()), + ibcerrors.ErrUnauthorized, + }, + { + "failure: whitespace signer", + channeltypes.NewMsgUpdateChannelParams(" ", channeltypes.DefaultParams()), + ibcerrors.ErrUnauthorized, + }, + { + "failure: empty signer", + channeltypes.NewMsgUpdateChannelParams("", channeltypes.DefaultParams()), + ibcerrors.ErrUnauthorized, + }, + { + "failure: unauthorized signer", + channeltypes.NewMsgUpdateChannelParams(ibctesting.TestAccAddress, channeltypes.DefaultParams()), + ibcerrors.ErrUnauthorized, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() + _, err := keeper.Keeper.UpdateChannelParams(*suite.chainA.App.GetIBCKeeper(), suite.chainA.GetContext(), tc.msg) + expPass := tc.expError == nil + if expPass { + suite.Require().NoError(err) + p := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetParams(suite.chainA.GetContext()) + suite.Require().Equal(tc.msg.Params, p) + } else { + suite.Require().ErrorIs(tc.expError, err) + } + }) + } +} + func (suite *KeeperTestSuite) TestPruneAcknowledgements() { var msg *channeltypes.MsgPruneAcknowledgements From eb13c520e1526dbf715adc061dcd87721f2ce98f Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 2 Jan 2024 11:28:11 +0000 Subject: [PATCH 5/7] chore: run linter --- modules/core/04-channel/types/msgs_test.go | 2 -- modules/core/04-channel/types/params.go | 2 -- 2 files changed, 4 deletions(-) diff --git a/modules/core/04-channel/types/msgs_test.go b/modules/core/04-channel/types/msgs_test.go index 73b4eab1ee0..97d1daf6f7b 100644 --- a/modules/core/04-channel/types/msgs_test.go +++ b/modules/core/04-channel/types/msgs_test.go @@ -3,8 +3,6 @@ package types_test import ( "fmt" "testing" - - dbm "github.com/cosmos/cosmos-db" testifysuite "github.com/stretchr/testify/suite" diff --git a/modules/core/04-channel/types/params.go b/modules/core/04-channel/types/params.go index 1b58a51c814..f72dabcd381 100644 --- a/modules/core/04-channel/types/params.go +++ b/modules/core/04-channel/types/params.go @@ -2,8 +2,6 @@ package types import ( "time" - - errorsmod "cosmossdk.io/errors" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" From 9e1b5225fd910e3d56f66ef4e016bfc436626c2b Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 2 Jan 2024 13:10:55 +0000 Subject: [PATCH 6/7] chore: run linter --- modules/core/04-channel/types/msgs_test.go | 1 + modules/core/04-channel/types/params.go | 1 + 2 files changed, 2 insertions(+) diff --git a/modules/core/04-channel/types/msgs_test.go b/modules/core/04-channel/types/msgs_test.go index 97d1daf6f7b..f673a6c7c35 100644 --- a/modules/core/04-channel/types/msgs_test.go +++ b/modules/core/04-channel/types/msgs_test.go @@ -3,6 +3,7 @@ package types_test import ( "fmt" "testing" + dbm "github.com/cosmos/cosmos-db" testifysuite "github.com/stretchr/testify/suite" diff --git a/modules/core/04-channel/types/params.go b/modules/core/04-channel/types/params.go index f72dabcd381..5415425315c 100644 --- a/modules/core/04-channel/types/params.go +++ b/modules/core/04-channel/types/params.go @@ -2,6 +2,7 @@ package types import ( "time" + errorsmod "cosmossdk.io/errors" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" From 873a24cbc5a96111d11785318fc6787e31150e01 Mon Sep 17 00:00:00 2001 From: chatton Date: Wed, 3 Jan 2024 09:28:58 +0000 Subject: [PATCH 7/7] chore: addressing pr feedback --- modules/core/04-channel/types/msgs.go | 4 ++++ modules/core/04-channel/types/msgs_test.go | 4 ++-- modules/core/04-channel/types/params.go | 4 ++-- modules/core/keeper/msg_server_test.go | 4 +++- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/core/04-channel/types/msgs.go b/modules/core/04-channel/types/msgs.go index 60b0dcd5fc7..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) diff --git a/modules/core/04-channel/types/msgs_test.go b/modules/core/04-channel/types/msgs_test.go index f673a6c7c35..23bf80aa297 100644 --- a/modules/core/04-channel/types/msgs_test.go +++ b/modules/core/04-channel/types/msgs_test.go @@ -1283,14 +1283,14 @@ func (suite *TypesTestSuite) TestMsgUpdateParamsValidateBasic() { newHeight := clienttypes.NewHeight(1, 1000) msg = types.NewMsgUpdateChannelParams(authtypes.NewModuleAddress(govtypes.ModuleName).String(), types.NewParams(types.NewTimeout(newHeight, uint64(100000)))) }, - types.ErrInvalidTimeout, + types.ErrInvalidUpgradeTimeout, }, { "invalid params: zero timestamp", func() { msg = types.NewMsgUpdateChannelParams(authtypes.NewModuleAddress(govtypes.ModuleName).String(), types.NewParams(types.NewTimeout(clienttypes.ZeroHeight(), uint64(0)))) }, - types.ErrInvalidTimeout, + types.ErrInvalidUpgradeTimeout, }, } diff --git a/modules/core/04-channel/types/params.go b/modules/core/04-channel/types/params.go index 5415425315c..688c55dc721 100644 --- a/modules/core/04-channel/types/params.go +++ b/modules/core/04-channel/types/params.go @@ -28,10 +28,10 @@ func DefaultParams() Params { // Validate the params. func (p Params) Validate() error { if !p.UpgradeTimeout.Height.IsZero() { - return errorsmod.Wrapf(ErrInvalidTimeout, "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 errorsmod.Wrapf(ErrInvalidTimeout, "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 153d4dd20b3..0ecab351009 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -2056,13 +2056,15 @@ func (suite *KeeperTestSuite) TestUpdateChannelParams() { tc := tc suite.Run(tc.name, func() { suite.SetupTest() - _, err := keeper.Keeper.UpdateChannelParams(*suite.chainA.App.GetIBCKeeper(), suite.chainA.GetContext(), tc.msg) + resp, err := keeper.Keeper.UpdateChannelParams(*suite.chainA.App.GetIBCKeeper(), suite.chainA.GetContext(), tc.msg) 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().Nil(resp) suite.Require().ErrorIs(tc.expError, err) } })