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: adding assertion of channel capabilities migration #2140

Merged
merged 22 commits into from
Sep 7, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a6575eb
wip initial commit
damiannolan Aug 26, 2022
94091cf
draft migration completed
damiannolan Aug 26, 2022
3c95389
removing unnecessary storekey arg
damiannolan Aug 26, 2022
2bb7c3f
additional cleanup
damiannolan Aug 26, 2022
d546d20
adding updates to migrations and tests additional assertions
damiannolan Aug 26, 2022
a79d10e
updating and moving migrations code
damiannolan Aug 26, 2022
c2da8b2
adding additional checks to tests
damiannolan Aug 26, 2022
6ea2014
cleaning up tests
damiannolan Aug 26, 2022
c981bcc
cleaning up tests
damiannolan Aug 26, 2022
88124f4
updating inline doc comments, checking err return
damiannolan Aug 26, 2022
cf3bfaf
Merge branch 'main' into damian/ics27-chan-capability-migrations
damiannolan Aug 26, 2022
443b548
using InitMemStore in favour of InitializeCapability, adjusting tests
damiannolan Aug 29, 2022
bdec1bc
Merge branch 'damian/ics27-chan-capability-migrations' of github.com:…
damiannolan Aug 29, 2022
5229159
adding assertion of channel capabilities migration
damiannolan Aug 29, 2022
478521e
Merge branch 'main' into damian/ics27-assert-chan-capabilities
damiannolan Sep 1, 2022
bf88e6d
adapting migration code to use get/authenticate capability
damiannolan Sep 1, 2022
8949fa9
adding changelog
damiannolan Sep 1, 2022
0bd27e9
updating tests
damiannolan Sep 1, 2022
dda0398
set middleware enabled for existing channels
damiannolan Sep 1, 2022
49df7dd
assert middleware enabled in tests
damiannolan Sep 6, 2022
4fc9aae
updating changelog with middleware enabled flag
damiannolan Sep 6, 2022
8cd3d49
Merge branch 'main' into damian/ics27-assert-chan-capabilities
damiannolan Sep 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

"github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
host "github.com/cosmos/ibc-go/v5/modules/core/24-host"
)

// Migrator is a struct for handling in-place store migrations.
type Migrator struct {
keeper *Keeper
}

// NewMigrator returns a new Migrator.
func NewMigrator(keeper *Keeper) Migrator {
return Migrator{keeper: keeper}
}

// AssertChannelCapabilityMigrations checks that all channel capabilities generated using the interchain accounts controller port prefix
// are owned by the controller submodule and ibc.
func (m Migrator) AssertChannelCapabilityMigrations(ctx sdk.Context) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm open to suggestions on improving the name of this method.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the name

for _, channel := range m.keeper.GetAllActiveChannels(ctx) {
name := host.ChannelCapabilityPath(channel.PortId, channel.ChannelId)
owners, found := m.keeper.scopedKeeper.GetOwners(ctx, name)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
if !found {
return sdkerrors.Wrapf(capabilitytypes.ErrCapabilityOwnersNotFound, "failed to find capability owners for: %s", name)
}

ibcOwner := capabilitytypes.NewOwner(host.ModuleName, name)
if index, found := owners.Get(ibcOwner); !found && index != 0 {
return sdkerrors.Wrapf(capabilitytypes.ErrCapabilityNotOwned, "expected capability owner: %s", host.ModuleName)
}

controllerOwner := capabilitytypes.NewOwner(types.SubModuleName, name)
if index, found := owners.Get(controllerOwner); !found && index != 1 {
return sdkerrors.Wrapf(capabilitytypes.ErrCapabilityNotOwned, "expected capability owner: %s", types.SubModuleName)
}
}

return nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package keeper_test

import (
"github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/keeper"
ibctesting "github.com/cosmos/ibc-go/v5/testing"
)

func (suite *KeeperTestSuite) TestAssertChannelCapabilityMigrations() {
testCases := []struct {
name string
malleate func()
expPass bool
}{
{
"success",
func() {},
true,
},
}

for _, tc := range testCases {
suite.Run(tc.name, func() {
suite.SetupTest()

path := NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)

err := SetupICAPath(path, ibctesting.TestAccAddress)
suite.Require().NoError(err)

tc.malleate()

migrator := keeper.NewMigrator(&suite.chainA.GetSimApp().ICAControllerKeeper)
err = migrator.MigrateCapabilitiesConfirm(suite.chainA.GetContext())
suite.Require().NoError(err)
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package v5

import (
"fmt"

"github.com/cosmos/cosmos-sdk/store/prefix"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
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"
icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types"
host "github.com/cosmos/ibc-go/v5/modules/core/24-host"
)

// 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,
memStoreKey storetypes.StoreKey,
capabilityKeeper *capabilitykeeper.Keeper,
module string,
) error {
// construct a prefix store using the x/capability reverse lookup key: {module}/rev/{name} -> index
keyPrefix := capabilitytypes.RevCapabilityKey(module, fmt.Sprintf("%s/%s/%s", host.KeyChannelCapabilityPrefix, host.KeyPortPrefix, icatypes.PortPrefix))
prefixStore := prefix.NewStore(ctx.KVStore(memStoreKey), keyPrefix)

iterator := sdk.KVStorePrefixIterator(prefixStore, nil)
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
// unmarshal the capability index value and retrieve the set of owners
index := sdk.BigEndianToUint64(iterator.Value())
owners, found := capabilityKeeper.GetOwners(ctx, index)
if !found {
return sdkerrors.Wrapf(capabilitytypes.ErrCapabilityOwnersNotFound, "no owners found for capability at index: %d", index)
}

// reconstruct the capability name using the prefixes and iterator key
name := fmt.Sprintf("%s/%s/%s%s", host.KeyChannelCapabilityPrefix, host.KeyPortPrefix, icatypes.PortPrefix, string(iterator.Key()))
prevOwner := capabilitytypes.NewOwner(module, name)
newOwner := capabilitytypes.NewOwner(types.SubModuleName, name)

// remove the existing module owner
owners.Remove(prevOwner)
prefixStore.Delete(iterator.Key())

// add the controller submodule to the set of owners
if err := owners.Set(newOwner); err != nil {
return err
}

// set the new owners for the appropriate capability index
capabilityKeeper.SetOwners(ctx, index, owners)

// initialise the x/capability memstore
capabilityKeeper.InitMemStore(ctx)
}

return nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
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)

capName := host.ChannelCapabilityPath(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)

// 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)

// manually delete the `KeyMemInitialised` from the x/capability memstore
// this allows capabilityKeeper.InitMemStore(ctx) to re-initialise capabilities
memStore := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetMemKey(capabilitytypes.MemStoreKey))
memStore.Delete(capabilitytypes.KeyMemInitialized)

err = v5.MigrateICS27ChannelCapability(
suite.chainA.GetContext(),
suite.chainA.GetSimApp().GetMemKey(capabilitytypes.MemStoreKey),
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)
}
7 changes: 6 additions & 1 deletion modules/apps/27-interchain-accounts/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,11 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
controllertypes.RegisterMsgServer(cfg.MsgServer(), am.controllerKeeper)
controllertypes.RegisterQueryServer(cfg.QueryServer(), am.controllerKeeper)
hosttypes.RegisterQueryServer(cfg.QueryServer(), am.hostKeeper)

m := controllerkeeper.NewMigrator(am.controllerKeeper)
if err := cfg.RegisterMigration(types.ModuleName, 1, m.AssertChannelCapabilityMigrations); err != nil {
panic(fmt.Sprintf("failed to migrate interchainaccounts app from version 1 to 2: %v", err))
}
}

// InitGenesis performs genesis initialization for the interchain accounts module.
Expand Down Expand Up @@ -187,7 +192,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw
}

// ConsensusVersion implements AppModule/ConsensusVersion.
func (AppModule) ConsensusVersion() uint64 { return 1 }
func (AppModule) ConsensusVersion() uint64 { return 2 }

// BeginBlock implements the AppModule interface
func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,5 @@ type ScopedKeeper interface {
AuthenticateCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) bool
LookupModules(ctx sdk.Context, name string) ([]string, *capabilitytypes.Capability, error)
ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) error
GetOwners(ctx sdk.Context, name string) (*capabilitytypes.CapabilityOwners, bool)
}