Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: use connection ID in active channel store keys #807

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some places we use the terminology connectionID & others connection sequence. When should we use one or the other? cc @AdityaSripal @colin-axner

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anywhere the connection ID is used in the string format "connection-{sequence}" eg. "connection-0", then connectionID is used, otherwise connectionSeq when it's just the parsed uint64 sequence value associated with the connection 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)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
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