Skip to content

Commit

Permalink
chore: use connection ID in interchain account store keys (cosmos#791)
Browse files Browse the repository at this point in the history
* updating protos to include connection id

* updating interchain account store key to include conn id

* updating arg names

* updating proto comments

* cleanup

* rename connID -> connectionID in AuthenticateTx arg

* updating to use controller connection id in controller store key

* chore: use connection ID in active channel store keys (cosmos#807)

* 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 54dc848 commit f393893
Show file tree
Hide file tree
Showing 25 changed files with 330 additions and 188 deletions.
6 changes: 4 additions & 2 deletions 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 Expand Up @@ -379,11 +380,12 @@ HostGenesisState defines the interchain accounts host genesis state
<a name="ibc.applications.interchain_accounts.v1.RegisteredInterchainAccount"></a>

### RegisteredInterchainAccount
RegisteredInterchainAccount contains a pairing of controller port ID and associated interchain account address
RegisteredInterchainAccount contains a connection ID, port ID and associated interchain account address


| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `connection_id` | [string](#string) | | |
| `port_id` | [string](#string) | | |
| `account_address` | [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,11 +21,11 @@ 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 {
keeper.SetInterchainAccountAddress(ctx, acc.PortId, acc.AccountAddress)
keeper.SetInterchainAccountAddress(ctx, acc.ConnectionId, acc.PortId, acc.AccountAddress)
}

keeper.SetParams(ctx, state.Params)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ 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{
{
ConnectionId: ibctesting.FirstConnectionID,
PortId: TestPortID,
AccountAddress: TestAccAddress.String(),
},
Expand All @@ -28,11 +30,11 @@ 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)

accountAdrr, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), TestPortID)
accountAdrr, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), ibctesting.FirstConnectionID, TestPortID)
suite.Require().True(found)
suite.Require().Equal(TestAccAddress.String(), accountAdrr)

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,8 +92,8 @@ func (k Keeper) OnChanOpenAck(
return sdkerrors.Wrap(icatypes.ErrInvalidAccountAddress, "interchain account address cannot be empty")
}

k.SetActiveChannelID(ctx, portID, channelID)
k.SetInterchainAccountAddress(ctx, portID, metadata.Address)
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,12 +279,12 @@ 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)

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

suite.Require().Equal(metadata.Address, interchainAccAddress)
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
48 changes: 25 additions & 23 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,22 +152,22 @@ 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
}

// GetInterchainAccountAddress retrieves the InterchainAccount address from the store keyed by the provided portID
func (k Keeper) GetInterchainAccountAddress(ctx sdk.Context, portID string) (string, bool) {
// GetInterchainAccountAddress retrieves the InterchainAccount address from the store associated with the provided connectionID and portID
func (k Keeper) GetInterchainAccountAddress(ctx sdk.Context, connectionID, portID string) (string, bool) {
store := ctx.KVStore(k.storeKey)
key := icatypes.KeyOwnerAccount(portID)
key := icatypes.KeyOwnerAccount(connectionID, portID)

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

// GetAllInterchainAccounts returns a list of all registered interchain account addresses and their associated controller port identifiers
// GetAllInterchainAccounts returns a list of all registered interchain account addresses and their associated connection and controller port identifiers
func (k Keeper) GetAllInterchainAccounts(ctx sdk.Context) []icatypes.RegisteredInterchainAccount {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, []byte(icatypes.OwnerKeyPrefix))
Expand All @@ -185,7 +186,8 @@ func (k Keeper) GetAllInterchainAccounts(ctx sdk.Context) []icatypes.RegisteredI
keySplit := strings.Split(string(iterator.Key()), "/")

acc := icatypes.RegisteredInterchainAccount{
PortId: keySplit[1],
ConnectionId: keySplit[1],
PortId: keySplit[2],
AccountAddress: string(iterator.Value()),
}

Expand All @@ -195,8 +197,8 @@ func (k Keeper) GetAllInterchainAccounts(ctx sdk.Context) []icatypes.RegisteredI
return interchainAccounts
}

// SetInterchainAccountAddress stores the InterchainAccount address, keyed by the associated portID
func (k Keeper) SetInterchainAccountAddress(ctx sdk.Context, portID string, address string) {
// SetInterchainAccountAddress stores the InterchainAccount address, keyed by the associated connectionID and portID
func (k Keeper) SetInterchainAccountAddress(ctx sdk.Context, connectionID, portID, address string) {
store := ctx.KVStore(k.storeKey)
store.Set(icatypes.KeyOwnerAccount(portID), []byte(address))
store.Set(icatypes.KeyOwnerAccount(connectionID, portID), []byte(address))
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,11 @@ func (suite *KeeperTestSuite) TestGetInterchainAccountAddress() {
counterpartyPortID := path.EndpointA.ChannelConfig.PortID
expectedAddr := authtypes.NewBaseAccountWithAddress(icatypes.GenerateAddress(suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(icatypes.ModuleName), counterpartyPortID)).GetAddress()

retrievedAddr, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), counterpartyPortID)
retrievedAddr, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), ibctesting.FirstConnectionID, counterpartyPortID)
suite.Require().True(found)
suite.Require().Equal(expectedAddr.String(), retrievedAddr)

retrievedAddr, found = suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), "invalid port")
retrievedAddr, found = suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), "invalid conn", "invalid port")
suite.Require().False(found)
suite.Require().Empty(retrievedAddr)
}
Expand All @@ -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 All @@ -209,14 +211,16 @@ func (suite *KeeperTestSuite) TestGetAllInterchainAccounts() {
err := SetupICAPath(path, TestOwnerAddress)
suite.Require().NoError(err)

suite.chainA.GetSimApp().ICAControllerKeeper.SetInterchainAccountAddress(suite.chainA.GetContext(), expectedPortID, expectedAccAddr)
suite.chainA.GetSimApp().ICAControllerKeeper.SetInterchainAccountAddress(suite.chainA.GetContext(), ibctesting.FirstConnectionID, expectedPortID, expectedAccAddr)

expectedAccounts := []icatypes.RegisteredInterchainAccount{
{
ConnectionId: ibctesting.FirstConnectionID,
PortId: TestPortID,
AccountAddress: TestAccAddress.String(),
},
{
ConnectionId: ibctesting.FirstConnectionID,
PortId: expectedPortID,
AccountAddress: expectedAccAddr,
},
Expand All @@ -238,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 All @@ -248,9 +252,9 @@ func (suite *KeeperTestSuite) TestSetInterchainAccountAddress() {
expectedPortID string = "test-port"
)

suite.chainA.GetSimApp().ICAControllerKeeper.SetInterchainAccountAddress(suite.chainA.GetContext(), expectedPortID, expectedAccAddr)
suite.chainA.GetSimApp().ICAControllerKeeper.SetInterchainAccountAddress(suite.chainA.GetContext(), ibctesting.FirstConnectionID, expectedPortID, expectedAccAddr)

retrievedAddr, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), expectedPortID)
retrievedAddr, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), ibctesting.FirstConnectionID, expectedPortID)
suite.Require().True(found)
suite.Require().Equal(expectedAccAddr, retrievedAddr)
}
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
Loading

0 comments on commit f393893

Please sign in to comment.