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: ics27 channel capability migrations #2134

Merged
merged 24 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
24 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
e11be4c
updating migration code to run against persisted state only, adapting…
damiannolan Aug 30, 2022
b5ad7aa
Merge branch 'main' into damian/ics27-chan-capability-migrations
damiannolan Aug 30, 2022
8571ecc
updating inline comments
damiannolan Aug 30, 2022
bf8457b
adding changelog entry
damiannolan Aug 30, 2022
5840e16
Merge branch 'main' into damian/ics27-chan-capability-migrations
damiannolan Aug 30, 2022
96b733b
Merge branch 'main' into damian/ics27-chan-capability-migrations
damiannolan Aug 31, 2022
738135d
Merge branch 'damian/ics27-chan-capability-migrations' of github.com:…
damiannolan Aug 31, 2022
a1e34b9
adding inline comment re. module name
damiannolan Aug 31, 2022
8fba15f
updating tests
damiannolan Aug 31, 2022
dc258a2
assert channel-0 ownership by controller
damiannolan Aug 31, 2022
1cee4fc
Merge branch 'main' into damian/ics27-chan-capability-migrations
damiannolan Aug 31, 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,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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth linking to an issue describing why the migration is necessary 🤔

func MigrateICS27ChannelCapability(
ctx sdk.Context,
memStoreKey storetypes.StoreKey,
capabilityKeeper *capabilitykeeper.Keeper,
module string,
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
) 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))
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
prefixStore := prefix.NewStore(ctx.KVStore(memStoreKey), keyPrefix)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

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