From eedb0cbe120d9da2e0d3b5d5b5be9d5a3a2f95be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 1 Dec 2021 11:34:58 +0100 Subject: [PATCH] Disable usage of controller and host submodules based on on-chain params (#575) * add usage of enabling/disabling controller and host submodules Adds if statement checks in controller/host ibc_module.go. Adds tests for each added if statements. Tests not added for controller ack/timeout since tests do not exist for those functions yet. * Update modules/apps/27-interchain-accounts/controller/ibc_module_test.go --- .../controller/ibc_module.go | 17 +++++++++++++++++ .../controller/ibc_module_test.go | 17 ++++++++++++++--- .../controller/keeper/params.go | 7 ++++--- .../controller/types/errors.go | 10 ++++++++++ .../27-interchain-accounts/host/ibc_module.go | 13 +++++++++++++ .../host/ibc_module_test.go | 16 ++++++++++++++++ .../host/keeper/params.go | 7 ++++--- .../27-interchain-accounts/host/types/errors.go | 10 ++++++++++ 8 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 modules/apps/27-interchain-accounts/controller/types/errors.go create mode 100644 modules/apps/27-interchain-accounts/host/types/errors.go diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module.go b/modules/apps/27-interchain-accounts/controller/ibc_module.go index 74d9d0800c1..897191eca08 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_module.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_module.go @@ -6,6 +6,7 @@ import ( capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/controller/keeper" + "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/controller/types" icatypes "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types" channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v2/modules/core/05-port/types" @@ -42,6 +43,10 @@ func (im IBCModule) OnChanOpenInit( counterparty channeltypes.Counterparty, version string, ) error { + if !im.keeper.IsControllerEnabled(ctx) { + return types.ErrControllerSubModuleDisabled + } + if err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil { return err } @@ -78,6 +83,10 @@ func (im IBCModule) OnChanOpenAck( channelID string, counterpartyVersion string, ) error { + if !im.keeper.IsControllerEnabled(ctx) { + return types.ErrControllerSubModuleDisabled + } + if err := im.keeper.OnChanOpenAck(ctx, portID, channelID, counterpartyVersion); err != nil { return err } @@ -130,6 +139,10 @@ func (im IBCModule) OnAcknowledgementPacket( acknowledgement []byte, relayer sdk.AccAddress, ) error { + if !im.keeper.IsControllerEnabled(ctx) { + return types.ErrControllerSubModuleDisabled + } + // call underlying app's OnAcknowledgementPacket callback. return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) } @@ -140,6 +153,10 @@ func (im IBCModule) OnTimeoutPacket( packet channeltypes.Packet, relayer sdk.AccAddress, ) error { + if !im.keeper.IsControllerEnabled(ctx) { + return types.ErrControllerSubModuleDisabled + } + if err := im.keeper.OnTimeoutPacket(ctx, packet); err != nil { return err } diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go b/modules/apps/27-interchain-accounts/controller/ibc_module_test.go index cf01f0c5f29..f1aa84be591 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_module_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/tendermint/tendermint/crypto" + "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/controller/types" icatypes "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types" clienttypes "github.com/cosmos/ibc-go/v2/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types" @@ -117,6 +118,11 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { { "success", func() {}, true, }, + { + "controller submodule disabled", func() { + suite.chainA.GetSimApp().ICAControllerKeeper.SetParams(suite.chainA.GetContext(), types.NewParams(false)) + }, false, + }, { "ICA OnChanOpenInit fails - UNORDERED channel", func() { channel.Ordering = channeltypes.UNORDERED @@ -251,6 +257,11 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenAck() { { "success", func() {}, true, }, + { + "controller submodule disabled", func() { + suite.chainA.GetSimApp().ICAControllerKeeper.SetParams(suite.chainA.GetContext(), types.NewParams(false)) + }, false, + }, { "ICA OnChanOpenACK fails - invalid version", func() { path.EndpointB.ChannelConfig.Version = "invalid|version" @@ -460,15 +471,15 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { packet := channeltypes.NewPacket( []byte("empty packet data"), suite.chainA.SenderAccount.GetSequence(), - path.EndpointB.ChannelConfig.PortID, - path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, + path.EndpointB.ChannelConfig.PortID, + path.EndpointB.ChannelID, clienttypes.NewHeight(0, 100), 0, ) - ack := cbs.OnRecvPacket(suite.chainA.GetContext(), packet, TestAccAddress) + ack := cbs.OnRecvPacket(suite.chainB.GetContext(), packet, TestAccAddress) suite.Require().Equal(tc.expPass, ack.Success()) }) } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/params.go b/modules/apps/27-interchain-accounts/controller/keeper/params.go index 5169d51e6a9..55e15e26d97 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/params.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/params.go @@ -6,8 +6,9 @@ import ( "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/controller/types" ) -// GetControllerEnabled retrieves the host enabled boolean from the paramstore -func (k Keeper) GetControllerEnabled(ctx sdk.Context) bool { +// IsControllerEnabled retrieves the host enabled boolean from the paramstore. +// True is returned if the controller submodule is enabled. +func (k Keeper) IsControllerEnabled(ctx sdk.Context) bool { var res bool k.paramSpace.Get(ctx, types.KeyControllerEnabled, &res) return res @@ -15,7 +16,7 @@ func (k Keeper) GetControllerEnabled(ctx sdk.Context) bool { // GetParams returns the total set of the host submodule parameters. func (k Keeper) GetParams(ctx sdk.Context) types.Params { - return types.NewParams(k.GetControllerEnabled(ctx)) + return types.NewParams(k.IsControllerEnabled(ctx)) } // SetParams sets the total set of the host submodule parameters. diff --git a/modules/apps/27-interchain-accounts/controller/types/errors.go b/modules/apps/27-interchain-accounts/controller/types/errors.go new file mode 100644 index 00000000000..efb316097e4 --- /dev/null +++ b/modules/apps/27-interchain-accounts/controller/types/errors.go @@ -0,0 +1,10 @@ +package types + +import ( + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +// ICA Controller sentinel errors +var ( + ErrControllerSubModuleDisabled = sdkerrors.Register(ModuleName, 2, "controller submodule is disabled") +) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index 3e28117ba22..b8dbc15d42e 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -6,6 +6,7 @@ import ( capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/host/keeper" + "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types" channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types" ibcexported "github.com/cosmos/ibc-go/v2/modules/core/exported" @@ -49,6 +50,10 @@ func (im IBCModule) OnChanOpenTry( version, counterpartyVersion string, ) error { + if !im.keeper.IsHostEnabled(ctx) { + return types.ErrHostSubModuleDisabled + } + return im.keeper.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version, counterpartyVersion) } @@ -68,6 +73,10 @@ func (im IBCModule) OnChanOpenConfirm( portID, channelID string, ) error { + if !im.keeper.IsHostEnabled(ctx) { + return types.ErrHostSubModuleDisabled + } + return im.keeper.OnChanOpenConfirm(ctx, portID, channelID) } @@ -96,6 +105,10 @@ func (im IBCModule) OnRecvPacket( packet channeltypes.Packet, _ sdk.AccAddress, ) ibcexported.Acknowledgement { + if !im.keeper.IsHostEnabled(ctx) { + return channeltypes.NewErrorAcknowledgement(types.ErrHostSubModuleDisabled.Error()) + } + ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)}) if err := im.keeper.OnRecvPacket(ctx, packet); err != nil { diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index aa99ac811f5..2a75c71ce0e 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/tendermint/tendermint/crypto" + "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types" clienttypes "github.com/cosmos/ibc-go/v2/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types" @@ -136,6 +137,11 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { { "success", func() {}, true, }, + { + "host submodule disabled", func() { + suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false)) + }, false, + }, { "success: ICA auth module callback returns error", func() { // mock module callback should not be called on host side @@ -252,6 +258,11 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenConfirm() { { "success", func() {}, true, }, + { + "host submodule disabled", func() { + suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false)) + }, false, + }, { "success: ICA auth module callback returns error", func() { // mock module callback should not be called on host side @@ -386,6 +397,11 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() { { "success", func() {}, true, }, + { + "host submodule disabled", func() { + suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), types.NewParams(false)) + }, false, + }, { "success with ICA auth module callback failure", func() { suite.chainB.GetSimApp().ICAAuthModule.IBCApp.OnRecvPacket = func( diff --git a/modules/apps/27-interchain-accounts/host/keeper/params.go b/modules/apps/27-interchain-accounts/host/keeper/params.go index c5198283b53..7b91c1bf110 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/params.go +++ b/modules/apps/27-interchain-accounts/host/keeper/params.go @@ -6,8 +6,9 @@ import ( "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/host/types" ) -// GetHostEnabled retrieves the host enabled boolean from the paramstore -func (k Keeper) GetHostEnabled(ctx sdk.Context) bool { +// IsHostEnabled retrieves the host enabled boolean from the paramstore. +// True is returned if the host submodule is enabled. +func (k Keeper) IsHostEnabled(ctx sdk.Context) bool { var res bool k.paramSpace.Get(ctx, types.KeyHostEnabled, &res) return res @@ -15,7 +16,7 @@ func (k Keeper) GetHostEnabled(ctx sdk.Context) bool { // GetParams returns the total set of the host submodule parameters. func (k Keeper) GetParams(ctx sdk.Context) types.Params { - return types.NewParams(k.GetHostEnabled(ctx)) + return types.NewParams(k.IsHostEnabled(ctx)) } // SetParams sets the total set of the host submodule parameters. diff --git a/modules/apps/27-interchain-accounts/host/types/errors.go b/modules/apps/27-interchain-accounts/host/types/errors.go new file mode 100644 index 00000000000..b188ac027c7 --- /dev/null +++ b/modules/apps/27-interchain-accounts/host/types/errors.go @@ -0,0 +1,10 @@ +package types + +import ( + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +// ICA Host sentinel errors +var ( + ErrHostSubModuleDisabled = sdkerrors.Register(ModuleName, 2, "host submodule is disabled") +)