Skip to content

Commit

Permalink
chore: adapt ics27 controller claiming of channel capability (backport
Browse files Browse the repository at this point in the history
…#2146) (#2150)

* chore: adapt ics27 controller claiming of channel capability (#2146)

* [WIP] adding current code, test failures due to removal of claim capaiblities in mock IBCApp

* [WIP] removing changes to fee tests

* [WIP] removing whitespaces

* adding nil check on chan caps in mock ibc module

* adding changelog entry

Co-authored-by: colin axnér <[email protected]>
(cherry picked from commit 56f9da2)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/controller/ibc_middleware.go

* resolving conflicts

* Update CHANGELOG.md

Co-authored-by: Damian Nolan <[email protected]>
  • Loading branch information
mergify[bot] and damiannolan authored Aug 31, 2022
1 parent ac9e0d7 commit 7b5c6ad
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (apps/27-interchain-accounts) [\#2133](https://github.com/cosmos/ibc-go/pull/2133) Generates genesis protos in a separate directory to avoid circular import errors. The protobuf package name has changed for the genesis types.
* (apps/27-interchain-accounts) [\#2102](https://github.com/cosmos/ibc-go/pull/2102) ICS27 controller middleware now supports a nil underlying application. This allows chains to make use of interchain accounts with existing auth mechanisms such as x/group and x/gov.
* (linting) [\#1418](https://github.com/cosmos/ibc-go/pull/1418) Fix linting errors, resulting compatiblity with go1.18 linting style, golangci-lint 1.46.2 and the revivie linter. This caused breaking changes in core/04-channel, core/ante, and the testing library.
* (apps/27-interchain-accounts) [\#2146](https://github.com/cosmos/ibc-go/pull/2146) ICS27 controller now claims the channel capability passed via ibc core, and passes `nil` to the underlying app callback. The channel capability arg in `SendTx` is now ignored and looked up internally.

### Features

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v5/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v5/modules/core/24-host"
ibcexported "github.com/cosmos/ibc-go/v5/modules/core/exported"
)

Expand Down Expand Up @@ -55,11 +56,15 @@ func (im IBCMiddleware) OnChanOpenInit(
return "", err
}

if err := im.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
return "", err
}

// call underlying app's OnChanOpenInit callback with the passed in version
// the version returned is discarded as the ica-auth module does not have permission to edit the version string.
// ics27 will always return the version string containing the Metadata struct which is created during the `RegisterInterchainAccount` call.
if im.app != nil {
if _, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil {
if _, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, nil, counterparty, version); err != nil {
return "", err
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,20 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
{
"success", func() {}, true,
},
{
"ICA auth module does not claim channel capability", func() {
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string,
portID, channelID string, chanCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty, version string,
) (string, error) {
if chanCap != nil {
return "", fmt.Errorf("channel capability should be nil")
}

return version, nil
}
}, true,
},
{
"ICA auth module modification of channel version is ignored", func() {
// NOTE: explicitly modify the channel version via the auth module callback,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import (
icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types"
clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v5/modules/core/24-host"
)

// SendTx takes pre-built packet data containing messages to be executed on the host chain from an authentication module and attempts to send the packet.
// The packet sequence for the outgoing packet is returned as a result.
// 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, connectionID, portID string, icaPacketData icatypes.InterchainAccountPacketData, timeoutTimestamp uint64) (uint64, error) {
func (k Keeper) SendTx(ctx sdk.Context, _ *capabilitytypes.Capability, connectionID, portID string, icaPacketData icatypes.InterchainAccountPacketData, timeoutTimestamp uint64) (uint64, error) {
activeChannelID, found := k.GetOpenActiveChannel(ctx, connectionID, portID)
if !found {
return 0, sdkerrors.Wrapf(icatypes.ErrActiveChannelNotFound, "failed to retrieve active channel on connection %s for port %s", connectionID, portID)
Expand All @@ -29,6 +30,11 @@ func (k Keeper) SendTx(ctx sdk.Context, chanCap *capabilitytypes.Capability, con
destinationPort := sourceChannelEnd.GetCounterparty().GetPortID()
destinationChannel := sourceChannelEnd.GetCounterparty().GetChannelID()

chanCap, found := k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(portID, activeChannelID))
if !found {
return 0, sdkerrors.Wrapf(capabilitytypes.ErrCapabilityNotFound, "failed to find capability: %s", host.ChannelCapabilityPath(portID, activeChannelID))
}

if uint64(ctx.BlockTime().UnixNano()) >= timeoutTimestamp {
return 0, icatypes.ErrInvalidTimeoutTimestamp
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,17 @@ package keeper_test
import (
sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types"
clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v5/modules/core/24-host"
ibctesting "github.com/cosmos/ibc-go/v5/testing"
)

func (suite *KeeperTestSuite) TestSendTx() {
var (
path *ibctesting.Path
packetData icatypes.InterchainAccountPacketData
chanCap *capabilitytypes.Capability
timeoutTimestamp uint64
)

Expand Down Expand Up @@ -119,13 +116,6 @@ func (suite *KeeperTestSuite) TestSendTx() {
},
false,
},
{
"invalid channel capability provided",
func() {
chanCap = nil
},
false,
},
{
"timeout timestamp is not in the future",
func() {
Expand Down Expand Up @@ -165,13 +155,9 @@ func (suite *KeeperTestSuite) TestSendTx() {
err := SetupICAPath(path, TestOwnerAddress)
suite.Require().NoError(err)

var ok bool
chanCap, ok = suite.chainA.GetSimApp().ScopedICAMockKeeper.GetCapability(path.EndpointA.Chain.GetContext(), host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
suite.Require().True(ok)

tc.malleate() // malleate mutates test data

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

if tc.expPass {
suite.Require().NoError(err)
Expand Down
11 changes: 2 additions & 9 deletions modules/apps/27-interchain-accounts/host/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,10 +638,7 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose()
params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)

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, ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), nil, ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
suite.Require().NoError(err)
path.EndpointB.UpdateClient()

Expand All @@ -668,11 +665,7 @@ func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose()
path.EndpointB.ChannelID = ""
suite.coordinator.CreateChannels(path)

// try to control the interchain account again
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, ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
_, err = suite.chainA.GetSimApp().ICAControllerKeeper.SendTx(suite.chainA.GetContext(), nil, ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, icaPacketData, ^uint64(0))
suite.Require().NoError(err)
path.EndpointB.UpdateClient()

Expand Down
16 changes: 10 additions & 6 deletions testing/mock/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ func (im IBCModule) OnChanOpenInit(
return im.IBCApp.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version)
}

// Claim channel capability passed back by IBC module
if err := im.IBCApp.ScopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
return "", err
if chanCap != nil {
// Claim channel capability passed back by IBC module
if err := im.IBCApp.ScopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
return "", err
}
}

return version, nil
Expand All @@ -59,9 +61,11 @@ func (im IBCModule) OnChanOpenTry(
return im.IBCApp.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, counterpartyVersion)
}

// Claim channel capability passed back by IBC module
if err := im.IBCApp.ScopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
return "", err
if chanCap != nil {
// Claim channel capability passed back by IBC module
if err := im.IBCApp.ScopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
return "", err
}
}

return Version, nil
Expand Down

0 comments on commit 7b5c6ad

Please sign in to comment.