chore: ics27 channel capability migrations (backport #2134) (#2155)

* chore: ics27 channel capability migrations (#2134)

* wip initial commit

* draft migration completed

* removing unnecessary storekey arg

* additional cleanup

* adding updates to migrations and tests additional assertions

* updating and moving migrations code

* adding additional checks to tests

* cleaning up tests

* cleaning up tests

* updating inline doc comments, checking err return

* using InitMemStore in favour of InitializeCapability, adjusting tests

* updating migration code to run against persisted state only, adapting tests

* updating inline comments

* adding changelog entry

(cherry picked from commit 0a8602c)

# Conflicts:
#	CHANGELOG.md

* fix conflicts

* add back changelog entry

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>

CHANGELOG.md

@@ -56,6 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (apps/27-interchain-accounts) [\#2134](https://github.com/cosmos/ibc-go/pull/2134) Adding upgrade handler to ICS27 `controller` submodule for migration of channel capabilities. This upgrade handler migrates ownership of channel capabilities from the underlying application to the ICS27 `controller` submodule.
 * (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.

modules/apps/27-interchain-accounts/controller/migrations/v5/migrations.go

@@ -0,0 +1,58 @@
+package v5
+
+import (
+	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/store/prefix"
+	storetypes "github.com/cosmos/cosmos-sdk/store/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper"
+	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
+
+	"github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
+)
+
+// MigrateICS27ChannelCapability performs a search on a prefix store using the provided store key and module name.
+// It retrieves the associated channel capability index and reassigns ownership to the ICS27 controller submodule.
+func MigrateICS27ChannelCapability(
+	ctx sdk.Context,
+	cdc codec.BinaryCodec,
+	storeKey storetypes.StoreKey,
+	capabilityKeeper *capabilitykeeper.Keeper,
+	module string, // the name of the scoped keeper for the underlying app module
+) error {
+	// construct a prefix store using the x/capability index prefix: index->capability owners
+	prefixStore := prefix.NewStore(ctx.KVStore(storeKey), capabilitytypes.KeyPrefixIndexCapability)
+	iterator := sdk.KVStorePrefixIterator(prefixStore, nil)
+	defer iterator.Close()
+
+	for ; iterator.Valid(); iterator.Next() {
+		// unmarshal the capability index value and set of owners
+		index := capabilitytypes.IndexFromKey(iterator.Key())
+
+		var owners capabilitytypes.CapabilityOwners
+		cdc.MustUnmarshal(iterator.Value(), &owners)
+
+		for _, owner := range owners.GetOwners() {
+			if owner.Module == module {
+				// remove the owner from the set
+				owners.Remove(owner)
+
+				// reassign the owner module to icacontroller
+				owner.Module = types.SubModuleName
+
+				// add the controller submodule to the set of owners
+				if err := owners.Set(owner); err != nil {
+					return err
+				}
+
+				// set the new owners for the current capability index
+				capabilityKeeper.SetOwners(ctx, index, owners)
+			}
+		}
+	}
+
+	// initialise the x/capability memstore
+	capabilityKeeper.InitMemStore(ctx)
+
+	return nil
+}

modules/apps/27-interchain-accounts/controller/migrations/v5/migrations_test.go

@@ -0,0 +1,164 @@
+package v5_test
+
+import (
+	"testing"
+
+	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
+	"github.com/stretchr/testify/suite"
+
+	v5 "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/migrations/v5"
+	"github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
+	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"
+	host "github.com/cosmos/ibc-go/v5/modules/core/24-host"
+	ibctesting "github.com/cosmos/ibc-go/v5/testing"
+	ibcmock "github.com/cosmos/ibc-go/v5/testing/mock"
+)
+
+type MigrationsTestSuite struct {
+	suite.Suite
+
+	chainA *ibctesting.TestChain
+	chainB *ibctesting.TestChain
+
+	coordinator *ibctesting.Coordinator
+	path        *ibctesting.Path
+}
+
+func (suite *MigrationsTestSuite) SetupTest() {
+	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
+
+	suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
+	suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
+
+	suite.path = ibctesting.NewPath(suite.chainA, suite.chainB)
+	suite.path.EndpointA.ChannelConfig.PortID = icatypes.PortID
+	suite.path.EndpointB.ChannelConfig.PortID = icatypes.PortID
+	suite.path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED
+	suite.path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED
+	suite.path.EndpointA.ChannelConfig.Version = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
+	suite.path.EndpointB.ChannelConfig.Version = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
+}
+
+func (suite *MigrationsTestSuite) SetupPath() error {
+	if err := suite.RegisterInterchainAccount(suite.path.EndpointA, ibctesting.TestAccAddress); err != nil {
+		return err
+	}
+
+	if err := suite.path.EndpointB.ChanOpenTry(); err != nil {
+		return err
+	}
+
+	if err := suite.path.EndpointA.ChanOpenAck(); err != nil {
+		return err
+	}
+
+	if err := suite.path.EndpointB.ChanOpenConfirm(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (suite *MigrationsTestSuite) RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) error {
+	portID, err := icatypes.NewControllerPortID(owner)
+	if err != nil {
+		return err
+	}
+
+	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
+
+	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil {
+		return err
+	}
+
+	// commit state changes for proof verification
+	endpoint.Chain.NextBlock()
+
+	// update port/channel ids
+	endpoint.ChannelID = channeltypes.FormatChannelIdentifier(channelSequence)
+	endpoint.ChannelConfig.PortID = portID
+
+	return nil
+}
+
+func TestKeeperTestSuite(t *testing.T) {
+	suite.Run(t, new(MigrationsTestSuite))
+}
+
+func (suite *MigrationsTestSuite) TestMigrateICS27ChannelCapability() {
+	suite.SetupTest()
+	suite.coordinator.SetupConnections(suite.path)
+
+	err := suite.SetupPath()
+	suite.Require().NoError(err)
+
+	// create and claim a new capability with ibc/mock for "channel-1"
+	// note: suite.SetupPath() now claims the chanel capability using icacontroller for "channel-0"
+	capName := host.ChannelCapabilityPath(suite.path.EndpointA.ChannelConfig.PortID, channeltypes.FormatChannelIdentifier(1))
+
+	cap, err := suite.chainA.GetSimApp().ScopedIBCKeeper.NewCapability(suite.chainA.GetContext(), capName)
+	suite.Require().NoError(err)
+
+	err = suite.chainA.GetSimApp().ScopedICAMockKeeper.ClaimCapability(suite.chainA.GetContext(), cap, capName)
+	suite.Require().NoError(err)
+
+	// assert the capability is owned by the mock module
+	cap, found := suite.chainA.GetSimApp().ScopedICAMockKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().NotNil(cap)
+	suite.Require().True(found)
+
+	isAuthenticated := suite.chainA.GetSimApp().ScopedICAMockKeeper.AuthenticateCapability(suite.chainA.GetContext(), cap, capName)
+	suite.Require().True(isAuthenticated)
+
+	cap, found = suite.chainA.GetSimApp().ScopedICAControllerKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().Nil(cap)
+	suite.Require().False(found)
+
+	suite.ResetMemStore() // empty the x/capability in-memory store
+
+	err = v5.MigrateICS27ChannelCapability(
+		suite.chainA.GetContext(),
+		suite.chainA.Codec,
+		suite.chainA.GetSimApp().GetKey(capabilitytypes.StoreKey),
+		suite.chainA.GetSimApp().CapabilityKeeper,
+		ibcmock.ModuleName+types.SubModuleName,
+	)
+
+	suite.Require().NoError(err)
+
+	// assert the capability is now owned by the ICS27 controller submodule
+	cap, found = suite.chainA.GetSimApp().ScopedICAControllerKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().NotNil(cap)
+	suite.Require().True(found)
+
+	isAuthenticated = suite.chainA.GetSimApp().ScopedICAControllerKeeper.AuthenticateCapability(suite.chainA.GetContext(), cap, capName)
+	suite.Require().True(isAuthenticated)
+
+	cap, found = suite.chainA.GetSimApp().ScopedICAMockKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().Nil(cap)
+	suite.Require().False(found)
+
+	// ensure channel capability for "channel-0" is still owned by the controller
+	capName = host.ChannelCapabilityPath(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)
+	cap, found = suite.chainA.GetSimApp().ScopedICAControllerKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().NotNil(cap)
+	suite.Require().True(found)
+
+	isAuthenticated = suite.chainA.GetSimApp().ScopedICAControllerKeeper.AuthenticateCapability(suite.chainA.GetContext(), cap, capName)
+	suite.Require().True(isAuthenticated)
+}
+
+// ResetMemstore removes all existing fwd and rev capability kv pairs and deletes `KeyMemInitialised` from the x/capability memstore.
+// This effectively mocks a new chain binary being started. Migration code is run against persisted state only and allows the memstore to be reinitialised.
+func (suite *MigrationsTestSuite) ResetMemStore() {
+	memStore := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetMemKey(capabilitytypes.MemStoreKey))
+	memStore.Delete(capabilitytypes.KeyMemInitialized)
+
+	iterator := memStore.Iterator(nil, nil)
+	defer iterator.Close()
+
+	for ; iterator.Valid(); iterator.Next() {
+		memStore.Delete(iterator.Key())
+	}
+}