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 68e534cda74..605fa9c6415 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_module_test.go @@ -432,12 +432,8 @@ func (suite *InterchainAccountsTestSuite) TestOnChanCloseConfirm() { err = cbs.OnChanCloseConfirm( suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - activeChannelID, found := suite.chainA.GetSimApp().ICAHostKeeper.GetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) - if tc.expPass { suite.Require().NoError(err) - suite.Require().False(found) - suite.Require().Empty(activeChannelID) } else { suite.Require().Error(err) } @@ -628,12 +624,8 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() { err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil) - activeChannelID, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) - if tc.expPass { suite.Require().NoError(err) - suite.Require().Empty(activeChannelID) - suite.Require().False(found) } else { suite.Require().Error(err) } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/account_test.go b/modules/apps/27-interchain-accounts/controller/keeper/account_test.go index 967f7511aa2..fa0da9558f7 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/account_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/account_test.go @@ -2,6 +2,7 @@ package keeper_test import ( icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types" + channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/v3/testing" ) @@ -35,12 +36,22 @@ func (suite *KeeperTestSuite) TestInitInterchainAccount() { false, }, { - "MsgChanOpenInit fails - channel is already active", + "MsgChanOpenInit fails - channel is already active & in state OPEN", func() { portID, err := icatypes.NewControllerPortID(TestOwnerAddress) suite.Require().NoError(err) suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), portID, path.EndpointA.ChannelID) + + counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) + channel := channeltypes.Channel{ + State: channeltypes.OPEN, + Ordering: channeltypes.ORDERED, + Counterparty: counterparty, + ConnectionHops: []string{path.EndpointA.ConnectionID}, + Version: TestVersion, + } + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainA.GetContext(), portID, path.EndpointA.ChannelID, channel) }, false, }, diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index 1dedbc228c4..e6c6f85366a 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go @@ -55,7 +55,7 @@ func (k Keeper) OnChanOpenInit( return sdkerrors.Wrapf(icatypes.ErrInvalidVersion, "expected %s, got %s", icatypes.Version, metadata.Version) } - activeChannelID, found := k.GetActiveChannelID(ctx, portID) + activeChannelID, found := k.GetOpenActiveChannel(ctx, portID) if found { return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "existing active channel %s for portID %s", activeChannelID, portID) } @@ -105,8 +105,6 @@ func (k Keeper) OnChanCloseConfirm( channelID string, ) error { - k.DeleteActiveChannelID(ctx, portID) - return nil } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index 9e902c41084..d1d9847cde5 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -111,6 +111,16 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { "channel is already active", func() { suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + + counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) + channel := channeltypes.Channel{ + State: channeltypes.OPEN, + Ordering: channeltypes.ORDERED, + Counterparty: counterparty, + ConnectionHops: []string{path.EndpointA.ConnectionID}, + Version: TestVersion, + } + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, channel) }, false, }, diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go index 855b2a2de5d..afc1a2eda5d 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go @@ -14,6 +14,7 @@ import ( "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/types" icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types" + channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v3/modules/core/24-host" ) @@ -101,7 +102,7 @@ func (k Keeper) ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability return k.scopedKeeper.ClaimCapability(ctx, cap, name) } -// GetActiveChannelID retrieves the active channelID from the store keyed by the provided portID +// GetActiveChannelID retrieves the active channelID from the store, keyed by the provided portID func (k Keeper) GetActiveChannelID(ctx sdk.Context, portID string) (string, bool) { store := ctx.KVStore(k.storeKey) key := icatypes.KeyActiveChannel(portID) @@ -113,6 +114,22 @@ func (k Keeper) GetActiveChannelID(ctx sdk.Context, portID string) (string, bool return string(store.Get(key)), true } +// GetOpenActiveChannel retrieves the active channelID from the store, keyed by the provided portID & checks if the channel in question is in state OPEN +func (k Keeper) GetOpenActiveChannel(ctx sdk.Context, portID string) (string, bool) { + channelID, found := k.GetActiveChannelID(ctx, portID) + if !found { + return "", false + } + + channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID) + + if found && channel.State == channeltypes.OPEN { + return channelID, true + } + + return "", false +} + // GetAllActiveChannels returns a list of all active interchain accounts controller channels and their associated port identifiers func (k Keeper) GetAllActiveChannels(ctx sdk.Context) []icatypes.ActiveChannel { store := ctx.KVStore(k.storeKey) @@ -140,12 +157,6 @@ func (k Keeper) SetActiveChannelID(ctx sdk.Context, portID, channelID string) { store.Set(icatypes.KeyActiveChannel(portID), []byte(channelID)) } -// DeleteActiveChannelID removes the active channel keyed by the provided portID stored in state -func (k Keeper) DeleteActiveChannelID(ctx sdk.Context, portID string) { - store := ctx.KVStore(k.storeKey) - store.Delete(icatypes.KeyActiveChannel(portID)) -} - // IsActiveChannel returns true if there exists an active channel for the provided portID, otherwise false func (k Keeper) IsActiveChannel(ctx sdk.Context, portID string) bool { _, ok := k.GetActiveChannelID(ctx, portID) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/relay.go b/modules/apps/27-interchain-accounts/controller/keeper/relay.go index 1381e158bd2..e49f2e83c0e 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/relay.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/relay.go @@ -78,7 +78,5 @@ func (k Keeper) createOutgoingPacket( // OnTimeoutPacket removes the active channel associated with the provided packet, the underlying channel end is closed // due to the semantics of ORDERED channels func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) error { - k.DeleteActiveChannelID(ctx, packet.SourcePort) - return nil } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go b/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go index 4601e16ff48..bc25210b08a 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go @@ -213,12 +213,8 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() { err = suite.chainA.GetSimApp().ICAControllerKeeper.OnTimeoutPacket(suite.chainA.GetContext(), packet) - activeChannelID, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) - if tc.expPass { suite.Require().NoError(err) - suite.Require().Empty(activeChannelID) - suite.Require().False(found) } else { suite.Require().Error(err) } 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 0556f5b4100..28b7b9864fd 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -381,12 +381,8 @@ func (suite *InterchainAccountsTestSuite) TestOnChanCloseConfirm() { err = cbs.OnChanCloseConfirm( suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) - activeChannelID, found := suite.chainB.GetSimApp().ICAHostKeeper.GetActiveChannelID(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID) - if tc.expPass { suite.Require().NoError(err) - suite.Require().False(found) - suite.Require().Empty(activeChannelID) } else { suite.Require().Error(err) } diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go index 5d95f605076..8e0501cd165 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go @@ -91,8 +91,6 @@ func (k Keeper) OnChanCloseConfirm( channelID string, ) error { - k.DeleteActiveChannelID(ctx, portID) - return nil } diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go index 52ca9dfb319..833ca32f54f 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go @@ -242,12 +242,8 @@ func (suite *KeeperTestSuite) TestOnChanCloseConfirm() { err = suite.chainB.GetSimApp().ICAHostKeeper.OnChanCloseConfirm(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) - activeChannelID, found := suite.chainB.GetSimApp().ICAHostKeeper.GetActiveChannelID(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID) - if tc.expPass { suite.Require().NoError(err) - suite.Require().False(found) - suite.Require().Empty(activeChannelID) } else { suite.Require().Error(err) } diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper.go b/modules/apps/27-interchain-accounts/host/keeper/keeper.go index 5d8569a6e1a..bf76bce737d 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/host/keeper/keeper.go @@ -129,12 +129,6 @@ func (k Keeper) SetActiveChannelID(ctx sdk.Context, portID, channelID string) { store.Set(icatypes.KeyActiveChannel(portID), []byte(channelID)) } -// DeleteActiveChannelID removes the active channel keyed by the provided portID stored in state -func (k Keeper) DeleteActiveChannelID(ctx sdk.Context, portID string) { - store := ctx.KVStore(k.storeKey) - store.Delete(icatypes.KeyActiveChannel(portID)) -} - // IsActiveChannel returns true if there exists an active channel for the provided portID, otherwise false func (k Keeper) IsActiveChannel(ctx sdk.Context, portID string) bool { _, ok := k.GetActiveChannelID(ctx, portID)