diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ef18c0dc0a..1678b579626 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index cd09386f1e5..2da54dd3eda 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -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" ) @@ -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 } } diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index f9c44a7cefe..6a4680a4dfc 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -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, diff --git a/modules/apps/27-interchain-accounts/controller/keeper/relay.go b/modules/apps/27-interchain-accounts/controller/keeper/relay.go index 57d92ebef99..fc8bc4ea005 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/relay.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/relay.go @@ -8,6 +8,7 @@ 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. @@ -15,7 +16,7 @@ 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, 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) @@ -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 } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go b/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go index 96ce41b3e23..58afd075bd4 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/relay_test.go @@ -3,12 +3,10 @@ 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" ) @@ -16,7 +14,6 @@ func (suite *KeeperTestSuite) TestSendTx() { var ( path *ibctesting.Path packetData icatypes.InterchainAccountPacketData - chanCap *capabilitytypes.Capability timeoutTimestamp uint64 ) @@ -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() { @@ -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) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 5de70cc3fab..0dd7c71f8ed 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -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() @@ -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() diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index 7687be61a09..f94e6a90361 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -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 @@ -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