Skip to content

Commit

Permalink
refactor: no longer removing active channel mapping on close channel (#…
Browse files Browse the repository at this point in the history 
…730)

* refactor: no longer remove active channel mapping on close chan

* refactor: adding HasActiveChannel helper fn & refactor active channel test cases

* fix: update helper fn

* refactor: remove DeleteChannelId

* refactor: err on GetActiveChannel & use TestVersion var in use testcase

* chore: comments

* nit: TestVersion var

* nits: fn name + comment
  • Loading branch information
@seantking
seantking authored Jan 14, 2022
1 parent e28b6d1 commit 7415da7
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 41 deletions.
8 changes: 0 additions & 8 deletions modules/apps/27-interchain-accounts/controller/ibc_module_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
13 changes: 12 additions & 1 deletion modules/apps/27-interchain-accounts/controller/keeper/account_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
},
Expand Down
4 changes: 1 addition & 3 deletions modules/apps/27-interchain-accounts/controller/keeper/handshake.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -105,8 +105,6 @@ func (k Keeper) OnChanCloseConfirm(
channelID string,
) error {

k.DeleteActiveChannelID(ctx, portID)

return nil
}

Expand Down
10 changes: 10 additions & 0 deletions modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
25 changes: 18 additions & 7 deletions modules/apps/27-interchain-accounts/controller/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions modules/apps/27-interchain-accounts/controller/keeper/relay.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 0 additions & 4 deletions modules/apps/27-interchain-accounts/controller/keeper/relay_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 0 additions & 4 deletions modules/apps/27-interchain-accounts/host/ibc_module_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 0 additions & 2 deletions modules/apps/27-interchain-accounts/host/keeper/handshake.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ func (k Keeper) OnChanCloseConfirm(
channelID string,
) error {

k.DeleteActiveChannelID(ctx, portID)

return nil
}

Expand Down
4 changes: 0 additions & 4 deletions modules/apps/27-interchain-accounts/host/keeper/handshake_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 0 additions & 6 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 7415da7

Please sign in to comment.