-
Notifications
You must be signed in to change notification settings - Fork 579
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
1 parent
cc16ae4
commit f7d4256
Showing
3 changed files
with
283 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
58 changes: 58 additions & 0 deletions
58
modules/apps/27-interchain-accounts/controller/migrations/v5/migrations.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } |
164 changes: 164 additions & 0 deletions
164
modules/apps/27-interchain-accounts/controller/migrations/v5/migrations_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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()) | ||
| } | ||
| } |