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

docs: adding migration doc for ibc-go v6 #2417

Merged
merged 30 commits into from
Oct 14, 2022
Merged
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
9fa567e
WIP ics27 v6 migration doc
damiannolan Sep 27, 2022
8935dc6
updating migration doc
damiannolan Sep 28, 2022
1be7d30
updating migration doc
damiannolan Sep 28, 2022
e4a0053
updating migration doc
damiannolan Sep 28, 2022
c847c7c
updating migration doc
damiannolan Sep 28, 2022
417fbe2
updating migration doc
damiannolan Sep 28, 2022
deea020
updating migration doc
damiannolan Sep 28, 2022
1edca61
updating migration doc
damiannolan Sep 28, 2022
b165eee
Merge branch 'main' into damian/2184-migration-docs-v6
damiannolan Sep 28, 2022
25af341
Merge branch 'main' into damian/2184-migration-docs-v6
damiannolan Sep 28, 2022
02815c7
readding migration doc after merge nuked my file -_-
damiannolan Sep 28, 2022
bd85430
updating migration doc with ics27 host params updates
damiannolan Sep 28, 2022
c05eb76
cleanup
damiannolan Sep 28, 2022
3e0e945
adding controller api deprecation to migration doc
damiannolan Sep 30, 2022
54bcdda
updating links
damiannolan Sep 30, 2022
719ba90
updating formatting
damiannolan Sep 30, 2022
2232a43
formatting
damiannolan Sep 30, 2022
9f66eb0
Merge branch 'main' into damian/2184-migration-docs-v6
damiannolan Sep 30, 2022
8666362
moving upgrade handler and migration details to chains section
damiannolan Sep 30, 2022
4d273a5
Merge branch 'damian/2184-migration-docs-v6' of github.com:cosmos/ibc…
damiannolan Sep 30, 2022
ca1bd41
Apply suggestions from code review
damiannolan Oct 4, 2022
f5c883f
mitigate copy pasta by breaking code snippet compilation
damiannolan Oct 4, 2022
fa56276
added note about legacy APIs for packet cbs and ADR 008 ref
damiannolan Oct 4, 2022
202148c
Merge branch 'damian/2184-migration-docs-v6' of github.com:cosmos/ibc…
damiannolan Oct 4, 2022
5fadba1
Merge branch 'main' into damian/2184-migration-docs-v6
damiannolan Oct 11, 2022
5350568
Apply suggestions from code review
damiannolan Oct 11, 2022
3acc779
removing backticks in all refs to ibc-go, controller and host
damiannolan Oct 11, 2022
cc13942
addressing indentation
damiannolan Oct 14, 2022
f8751c2
adding ics29 NewKeeper api breaking changes and removal of ics20 Sen…
damiannolan Oct 14, 2022
ef8f237
Merge branch 'main' into damian/2184-migration-docs-v6
damiannolan Oct 14, 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
132 changes: 131 additions & 1 deletion docs/migrations/v5-to-v6.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

# Migrating from ibc-go v5 to v6

This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG.
Expand All @@ -14,8 +13,131 @@ There are four sections based on the four potential user groups of this document

## Chains

The `ibc-go/v6` release introduces a new set of migrations for `27-interchain-accounts`. Ownership of ICS27 channel capabilities is transferred from ICS27 authentication modules and will now reside with the ICS27 `controller` submodule moving forward.

For chains which contain a custom authentication module using the ICS27 `controller` submodule this requires a migration function to be included in the chain upgrade handler. A subsequent migration handler is run automatically, asserting the ownership of ICS27 channel capabilities has been transferred successfully.
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

For chains which *do not* contain a custom authentication module using the ICS27 `controller` submodule this migration is not required.
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

This migration facilitates the addition of the ICS27 `controller` submodule `MsgServer` which provides a standardised approach to integrating existing forms of authentication such as `x/gov` and `x/group` provided by the Cosmos SDK.

For more information please refer to [ADR 009](https://github.com/cosmos/ibc-go/blob/main/docs/architecture/adr-009-v6-ics27-msgserver.md).
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

#### Upgrade proposal
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

Please refer to [PR #2383](https://github.com/cosmos/ibc-go/pull/2383) for integrating the ICS27 channel capability migration logic or follow the steps outlined below:

1. Add the upgrade migration logic to chain distribution. This may be, for example, maintained under a package `app/upgrades/v6`.

```go
package v6

import (
"github.com/cosmos/cosmos-sdk/codec"
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"

v6 "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/migrations/v6"
)

const (
UpgradeName = "v6"
)

func CreateUpgradeHandler(
mm *module.Manager,
configurator module.Configurator,
cdc codec.BinaryCodec,
capabilityStoreKey *storetypes.KVStoreKey,
capabilityKeeper *capabilitykeeper.Keeper,
moduleName string,
) upgradetypes.UpgradeHandler {
return func(ctx sdk.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
if err := v6.MigrateICS27ChannelCapability(ctx, cdc, capabilityStoreKey, capabilityKeeper, moduleName); err != nil {
return nil, err
}

return mm.RunMigrations(ctx, configurator, vm)
}
}
```

2. Set the upgrade handler in `app.go`. The `moduleName` parameter refers to the authentication module's `ScopedKeeper` name. This is the name provided upon instantiation in `app.go` via the [`x/capability` keeper `ScopeToModule(moduleName string)`](https://github.com/cosmos/cosmos-sdk/blob/v0.46.1/x/capability/keeper/keeper.go#L70) method. [See here for an example in `simapp`](https://github.com/cosmos/ibc-go/blob/v5.0.0-rc2/testing/simapp/app.go#L304).
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

```go
app.UpgradeKeeper.SetUpgradeHandler(
v6.UpgradeName,
v6.CreateUpgradeHandler(
app.mm,
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
app.configurator,
app.appCodec,
app.keys[capabilitytypes.ModuleName],
app.CapabilityKeeper,
>>>> moduleName <<<<,
),
)
```

## IBC Apps

### ICS27 - Interchain Accounts

#### Controller APIs

In previous releases of `ibc-go`, chain developers integrating the ICS27 interchain accounts `controller` functionality were expected to create a custom `Base Application` referred to as an authentication module, see: [Building an authentication module](https://github.com/cosmos/ibc-go/blob/v5.0.0/docs/apps/interchain-accounts/auth-modules.md).
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

The `Base Application` was intended to be composed with the ICS27 `controller` submodule `Keeper` and faciliate many forms of message authentication depending on a chain's particular use case.

The `controller` submodule exposes two functions:

- [`RegisterInterchainAccount`](https://github.com/cosmos/ibc-go/blob/v5.0.0/modules/apps/27-interchain-accounts/controller/keeper/account.go#L19)
- [`SendTx`](https://github.com/cosmos/ibc-go/blob/v5.0.0/modules/apps/27-interchain-accounts/controller/keeper/relay.go#L18)

These functions have been deprecated in favour of the new `controller` submodule `MsgServer` and will be removed in later releases.
Both APIs remain functional and maintain backwards compatibility in `ibc-go/v6`, however consumers of these APIs are now recommended to follow the message passing paradigm outlined in Cosmos SDK [ADR 031](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-031-msg-service.md) and [ADR 033](https://github.com/cosmos/cosmos-sdk/blob/main/docs/architecture/adr-033-protobuf-inter-module-comm.md). This is facilitated by the Cosmos SDK [`MsgServiceRouter`](https://github.com/cosmos/cosmos-sdk/blob/main/baseapp/msg_service_router.go#L17) and chain developers creating custom application logic can now omit the ICS27 `ControllerKeeper` from their module and instead depend on message routing.
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

Legacy APIs are still required if application developers wish to consume IBC packet callbacks and react upon packet acknowledgements. In future this will be replaced by IBC Actor Callbacks, see [ADR 008](https://github.com/cosmos/ibc-go/pull/1976).
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to get it merged, so that we can use a relative link...


For more information see the new [ICS27 integration documentation](TODO: add link to new docs).
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'll put together some new docs for message routing examples for MsgRegisterInterchainAccount and MsgSendTx.


#### Host params

The `27-interchain-accounts/host` submodule default params have been updated to include the `AllowAllHostMsgs` wildcard `*`.
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
This enables execution of any `sdk.Msg` type for ICS27 registered on the host chain `InterfaceRegistry`.

```diff
// AllowAllHostMsgs holds the string key that allows all message types on interchain accounts host module
const AllowAllHostMsgs = "*"

...

// DefaultParams is the default parameter configuration for the host submodule
func DefaultParams() Params {
- return NewParams(DefaultHostEnabled, nil)
+ return NewParams(DefaultHostEnabled, []string{AllowAllHostMsgs})
}
```

#### API breaking changes

The `27-interchain-accounts` genesis types have been moved to their own package: `modules/apps/27-interchain-acccounts/genesis/types`.
This change facilitates the addition of the ICS27 `controller` submodule `Msg` server and avoids cyclic imports. This should have minimal disruption to chain developers integrating `27-interchain-accounts`.
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

The ICS27 `host` submodule `NewKeeper` function in `modules/apps/27-interchain-acccounts/host/keeper` now includes an additional parameter of type `ICS4Wrapper`.
This provides the `host` submodule with the ability to correctly unwrap channel versions in the event of a channel reopening handshake.

```diff
func NewKeeper(
cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace,
- channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
+ ics4Wrapper icatypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
accountKeeper icatypes.AccountKeeper, scopedKeeper icatypes.ScopedKeeper, msgRouter icatypes.MessageRouter,
) Keeper
```

### ICS04 - `SendPacket` API change

The `SendPacket` API has been simplified:
Expand All @@ -38,3 +160,11 @@ The `SendPacket` API has been simplified:
Callers no longer need to pass in a pre-constructed packet.
The destination port/channel identifiers and the packet sequence will be determined by core IBC.
`SendPacket` will return the packet sequence.

## Relayers

- No relevant changes were made in this release.

## IBC Light Clients

- No relevant changes were made in this release.