Skip to content

Commit

Permalink
chore: use connection ID in active channel store keys (#807)
Browse files Browse the repository at this point in the history
* adding connection id to active channel genesis protobuf type

* adding connection id to active channel store key

* updating protodoc

* fixing doc typo

* updating godocs to include connection ID

* readding active channel check in SendTx
  • Loading branch information
damiannolan authored Jan 28, 2022
1 parent 7298d1d commit cb8f19c
Show file tree
Hide file tree
Showing 21 changed files with 190 additions and 118 deletions.
3 changes: 2 additions & 1 deletion docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,12 @@ An InterchainAccount is defined as a BaseAccount & the address of the account ow
<a name="ibc.applications.interchain_accounts.v1.ActiveChannel"></a>

### ActiveChannel
ActiveChannel contains a pairing of port ID and channel ID for an active interchain accounts channel
ActiveChannel contains a connection ID, port ID and associated active channel ID


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `connection_id` | [string](#string) | | |
| `port_id` | [string](#string) | | |
| `channel_id` | [string](#string) | | |

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount() {
portID, err := icatypes.NewControllerPortID(TestOwnerAddress)
suite.Require().NoError(err)

suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), portID, path.EndpointA.ChannelID)
suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, portID, path.EndpointA.ChannelID)

counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
channel := channeltypes.Channel{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, state icatypes.ControllerGenesi
}

for _, ch := range state.ActiveChannels {
keeper.SetActiveChannelID(ctx, ch.PortId, ch.ChannelId)
keeper.SetActiveChannelID(ctx, ch.ConnectionId, ch.PortId, ch.ChannelId)
}

for _, acc := range state.InterchainAccounts {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
genesisState := icatypes.ControllerGenesisState{
ActiveChannels: []icatypes.ActiveChannel{
{
PortId: TestPortID,
ChannelId: ibctesting.FirstChannelID,
ConnectionId: ibctesting.FirstConnectionID,
PortId: TestPortID,
ChannelId: ibctesting.FirstChannelID,
},
},
InterchainAccounts: []icatypes.RegisteredInterchainAccount{
Expand All @@ -29,7 +30,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() {

keeper.InitGenesis(suite.chainA.GetContext(), suite.chainA.GetSimApp().ICAControllerKeeper, genesisState)

channelID, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainA.GetContext(), TestPortID)
channelID, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, TestPortID)
suite.Require().True(found)
suite.Require().Equal(ibctesting.FirstChannelID, channelID)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (k Keeper) OnChanOpenInit(
return err
}

activeChannelID, found := k.GetOpenActiveChannel(ctx, portID)
activeChannelID, found := k.GetOpenActiveChannel(ctx, connectionHops[0], portID)
if found {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "existing active channel %s for portID %s", activeChannelID, portID)
}
Expand Down Expand Up @@ -92,7 +92,7 @@ func (k Keeper) OnChanOpenAck(
return sdkerrors.Wrap(icatypes.ErrInvalidAccountAddress, "interchain account address cannot be empty")
}

k.SetActiveChannelID(ctx, portID, channelID)
k.SetActiveChannelID(ctx, metadata.ControllerConnectionId, portID, channelID)
k.SetInterchainAccountAddress(ctx, metadata.ControllerConnectionId, portID, metadata.Address)

return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
{
"channel is already active",
func() {
suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)

counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
channel := channeltypes.Channel{
Expand Down Expand Up @@ -279,7 +279,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() {
if tc.expPass {
suite.Require().NoError(err)

activeChannelID, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)
activeChannelID, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID)
suite.Require().True(found)

suite.Require().Equal(path.EndpointA.ChannelID, activeChannelID)
Expand Down Expand Up @@ -326,7 +326,7 @@ func (suite *KeeperTestSuite) TestOnChanCloseConfirm() {
err = suite.chainB.GetSimApp().ICAControllerKeeper.OnChanCloseConfirm(suite.chainB.GetContext(),
path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)

activeChannelID, found := suite.chainB.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
activeChannelID, found := suite.chainB.GetSimApp().ICAControllerKeeper.GetActiveChannelID(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointB.ChannelConfig.PortID)

if tc.expPass {
suite.Require().NoError(err)
Expand Down
31 changes: 16 additions & 15 deletions modules/apps/27-interchain-accounts/controller/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ 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
func (k Keeper) GetActiveChannelID(ctx sdk.Context, portID string) (string, bool) {
// GetActiveChannelID retrieves the active channelID from the store, keyed by the provided connectionID and portID
func (k Keeper) GetActiveChannelID(ctx sdk.Context, connectionID, portID string) (string, bool) {
store := ctx.KVStore(k.storeKey)
key := icatypes.KeyActiveChannel(portID)
key := icatypes.KeyActiveChannel(connectionID, portID)

if !store.Has(key) {
return "", false
Expand All @@ -114,9 +114,9 @@ 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)
// GetOpenActiveChannel retrieves the active channelID from the store, keyed by the provided connectionID and portID & checks if the channel in question is in state OPEN
func (k Keeper) GetOpenActiveChannel(ctx sdk.Context, connectionID, portID string) (string, bool) {
channelID, found := k.GetActiveChannelID(ctx, connectionID, portID)
if !found {
return "", false
}
Expand All @@ -130,7 +130,7 @@ func (k Keeper) GetOpenActiveChannel(ctx sdk.Context, portID string) (string, bo
return "", false
}

// GetAllActiveChannels returns a list of all active interchain accounts controller channels and their associated port identifiers
// GetAllActiveChannels returns a list of all active interchain accounts controller channels and their associated connection and port identifiers
func (k Keeper) GetAllActiveChannels(ctx sdk.Context) []icatypes.ActiveChannel {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, []byte(icatypes.ActiveChannelKeyPrefix))
Expand All @@ -141,8 +141,9 @@ func (k Keeper) GetAllActiveChannels(ctx sdk.Context) []icatypes.ActiveChannel {
keySplit := strings.Split(string(iterator.Key()), "/")

ch := icatypes.ActiveChannel{
PortId: keySplit[1],
ChannelId: string(iterator.Value()),
ConnectionId: keySplit[1],
PortId: keySplit[2],
ChannelId: string(iterator.Value()),
}

activeChannels = append(activeChannels, ch)
Expand All @@ -151,15 +152,15 @@ func (k Keeper) GetAllActiveChannels(ctx sdk.Context) []icatypes.ActiveChannel {
return activeChannels
}

// SetActiveChannelID stores the active channelID, keyed by the provided portID
func (k Keeper) SetActiveChannelID(ctx sdk.Context, portID, channelID string) {
// SetActiveChannelID stores the active channelID, keyed by the provided connectionID and portID
func (k Keeper) SetActiveChannelID(ctx sdk.Context, connectionID, portID, channelID string) {
store := ctx.KVStore(k.storeKey)
store.Set(icatypes.KeyActiveChannel(portID), []byte(channelID))
store.Set(icatypes.KeyActiveChannel(connectionID, portID), []byte(channelID))
}

// 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)
// IsActiveChannel returns true if there exists an active channel for the provided connectionID and portID, otherwise false
func (k Keeper) IsActiveChannel(ctx sdk.Context, connectionID, portID string) bool {
_, ok := k.GetActiveChannelID(ctx, connectionID, portID)
return ok
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,18 @@ func (suite *KeeperTestSuite) TestGetAllActiveChannels() {
err := SetupICAPath(path, TestOwnerAddress)
suite.Require().NoError(err)

suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), expectedPortID, expectedChannelID)
suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, expectedPortID, expectedChannelID)

expectedChannels := []icatypes.ActiveChannel{
{
PortId: TestPortID,
ChannelId: path.EndpointA.ChannelID,
ConnectionId: ibctesting.FirstConnectionID,
PortId: TestPortID,
ChannelId: path.EndpointA.ChannelID,
},
{
PortId: expectedPortID,
ChannelId: expectedChannelID,
ConnectionId: ibctesting.FirstConnectionID,
PortId: expectedPortID,
ChannelId: expectedChannelID,
},
}

Expand Down Expand Up @@ -240,7 +242,7 @@ func (suite *KeeperTestSuite) TestIsActiveChannel() {
suite.Require().NoError(err)
portID := path.EndpointA.ChannelConfig.PortID

isActive := suite.chainA.GetSimApp().ICAControllerKeeper.IsActiveChannel(suite.chainA.GetContext(), portID)
isActive := suite.chainA.GetSimApp().ICAControllerKeeper.IsActiveChannel(suite.chainA.GetContext(), ibctesting.FirstConnectionID, portID)
suite.Require().Equal(isActive, true)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ import (
// If the base application has the capability to send on the provided portID. An appropriate
// absolute timeoutTimestamp must be provided. If the packet is timed out, the channel will be closed.
// In the case of channel closure, a new channel may be reopened to reconnect to the host chain.
func (k Keeper) SendTx(ctx sdk.Context, chanCap *capabilitytypes.Capability, portID string, icaPacketData icatypes.InterchainAccountPacketData, timeoutTimestamp uint64) (uint64, error) {
// Check for the active channel
activeChannelID, found := k.GetActiveChannelID(ctx, portID)
func (k Keeper) SendTx(ctx sdk.Context, chanCap *capabilitytypes.Capability, connectionID, portID string, icaPacketData icatypes.InterchainAccountPacketData, timeoutTimestamp uint64) (uint64, error) {
activeChannelID, found := k.GetActiveChannelID(ctx, connectionID, portID)
if !found {
return 0, sdkerrors.Wrapf(icatypes.ErrActiveChannelNotFound, "failed to retrieve active channel for port %s", portID)
return 0, sdkerrors.Wrapf(icatypes.ErrActiveChannelNotFound, "failed to retrieve active channel on connection %s for port %s", connectionID, portID)
}

sourceChannelEnd, found := k.channelKeeper.GetChannel(ctx, portID, activeChannelID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (suite *KeeperTestSuite) TestSendTx() {
{
"channel does not exist",
func() {
suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, "channel-100")
suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, "channel-100")
},
false,
},
Expand Down Expand Up @@ -160,7 +160,7 @@ func (suite *KeeperTestSuite) TestSendTx() {

tc.malleate() // malleate mutates test data

_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), chanCap, path.EndpointA.ChannelConfig.PortID, packetData, timeoutTimestamp)
_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), chanCap, ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, packetData, timeoutTimestamp)

if tc.expPass {
suite.Require().NoError(err)
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/27-interchain-accounts/host/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose()
chanCap, ok := suite.chainA.GetSimApp().ScopedICAMockKeeper.GetCapability(path.EndpointA.Chain.GetContext(), host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
suite.Require().True(ok)

_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), chanCap, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), chanCap, ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
suite.Require().NoError(err)
path.EndpointB.UpdateClient()

Expand Down Expand Up @@ -673,7 +673,7 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose()
chanCap, ok = suite.chainA.GetSimApp().ScopedICAMockKeeper.GetCapability(path.EndpointA.Chain.GetContext(), host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
suite.Require().True(ok)

_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), chanCap, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), chanCap, ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
suite.Require().NoError(err)
path.EndpointB.UpdateClient()

Expand Down
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/host/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, state icatypes.HostGenesisState
}

for _, ch := range state.ActiveChannels {
keeper.SetActiveChannelID(ctx, ch.PortId, ch.ChannelId)
keeper.SetActiveChannelID(ctx, ch.ConnectionId, ch.PortId, ch.ChannelId)
}

for _, acc := range state.InterchainAccounts {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
genesisState := icatypes.HostGenesisState{
ActiveChannels: []icatypes.ActiveChannel{
{
PortId: TestPortID,
ChannelId: ibctesting.FirstChannelID,
ConnectionId: ibctesting.FirstConnectionID,
PortId: TestPortID,
ChannelId: ibctesting.FirstChannelID,
},
},
InterchainAccounts: []icatypes.RegisteredInterchainAccount{
Expand All @@ -29,7 +30,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() {

keeper.InitGenesis(suite.chainA.GetContext(), suite.chainA.GetSimApp().ICAHostKeeper, genesisState)

channelID, found := suite.chainA.GetSimApp().ICAHostKeeper.GetActiveChannelID(suite.chainA.GetContext(), TestPortID)
channelID, found := suite.chainA.GetSimApp().ICAHostKeeper.GetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, TestPortID)
suite.Require().True(found)
suite.Require().Equal(ibctesting.FirstChannelID, channelID)

Expand Down
6 changes: 5 additions & 1 deletion modules/apps/27-interchain-accounts/host/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,12 @@ func (k Keeper) OnChanOpenConfirm(
portID,
channelID string,
) error {
channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID)
if !found {
return sdkerrors.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)
}

k.SetActiveChannelID(ctx, portID, channelID)
k.SetActiveChannelID(ctx, channel.ConnectionHops[0], portID, channelID)

return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ func (suite *KeeperTestSuite) TestOnChanOpenConfirm() {
{
"success", func() {}, true,
},
{
"channel not found",
func() {
path.EndpointB.ChannelID = "invalid-channel-id"
path.EndpointB.ChannelConfig.PortID = "invalid-port-id"
},
false,
},
}

for _, tc := range testCases {
Expand All @@ -199,7 +207,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenConfirm() {
tc.malleate() // malleate mutates test data

err = suite.chainB.GetSimApp().ICAHostKeeper.OnChanOpenConfirm(suite.chainB.GetContext(),
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)

if tc.expPass {
suite.Require().NoError(err)
Expand Down
25 changes: 13 additions & 12 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ 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
func (k Keeper) GetActiveChannelID(ctx sdk.Context, portID string) (string, bool) {
// GetActiveChannelID retrieves the active channelID from the store keyed by the provided connectionID and portID
func (k Keeper) GetActiveChannelID(ctx sdk.Context, connectionID, portID string) (string, bool) {
store := ctx.KVStore(k.storeKey)
key := icatypes.KeyActiveChannel(portID)
key := icatypes.KeyActiveChannel(connectionID, portID)

if !store.Has(key) {
return "", false
Expand All @@ -102,7 +102,7 @@ func (k Keeper) GetActiveChannelID(ctx sdk.Context, portID string) (string, bool
return string(store.Get(key)), true
}

// GetAllActiveChannels returns a list of all active interchain accounts host channels and their associated port identifiers
// GetAllActiveChannels returns a list of all active interchain accounts host channels and their associated connection and port identifiers
func (k Keeper) GetAllActiveChannels(ctx sdk.Context) []icatypes.ActiveChannel {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, []byte(icatypes.ActiveChannelKeyPrefix))
Expand All @@ -113,8 +113,9 @@ func (k Keeper) GetAllActiveChannels(ctx sdk.Context) []icatypes.ActiveChannel {
keySplit := strings.Split(string(iterator.Key()), "/")

ch := icatypes.ActiveChannel{
PortId: keySplit[1],
ChannelId: string(iterator.Value()),
ConnectionId: keySplit[1],
PortId: keySplit[2],
ChannelId: string(iterator.Value()),
}

activeChannels = append(activeChannels, ch)
Expand All @@ -123,15 +124,15 @@ func (k Keeper) GetAllActiveChannels(ctx sdk.Context) []icatypes.ActiveChannel {
return activeChannels
}

// SetActiveChannelID stores the active channelID, keyed by the provided portID
func (k Keeper) SetActiveChannelID(ctx sdk.Context, portID, channelID string) {
// SetActiveChannelID stores the active channelID, keyed by the provided connectionID and portID
func (k Keeper) SetActiveChannelID(ctx sdk.Context, connectionID, portID, channelID string) {
store := ctx.KVStore(k.storeKey)
store.Set(icatypes.KeyActiveChannel(portID), []byte(channelID))
store.Set(icatypes.KeyActiveChannel(connectionID, portID), []byte(channelID))
}

// 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)
// IsActiveChannel returns true if there exists an active channel for the provided connectionID and portID, otherwise false
func (k Keeper) IsActiveChannel(ctx sdk.Context, connectionID, portID string) bool {
_, ok := k.GetActiveChannelID(ctx, connectionID, portID)
return ok
}

Expand Down
Loading

0 comments on commit cb8f19c

Please sign in to comment.