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: some more v8 migration docs #4728

Merged
merged 22 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
138 changes: 86 additions & 52 deletions docs/docs/05-migrations/11-v7-to-v8.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,66 +44,82 @@ type Keeper struct {

See [this PR](https://github.com/cosmos/ibc-go/pull/4703/files#diff-d18972debee5e64f16e40807b2ae112ddbe609504a93ea5e1c80a5d489c3a08a) for the changes required in `app.go`.

TODO: <https://github.com/cosmos/ibc-go/pull/3505> (extra parameter added to transfer's `GenesisState`)
An extra parameter `totalEscrowed` of type `sdk.Coins` has been added to transfer module's [`NewGenesisState` function](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/modules/apps/transfer/types/genesis.go#L10). This parameter specifies the total amount of tokens that are in the module's escrow accounts.

- You must pass the `authority` to the icahost keeper. ([#3520](https://github.com/cosmos/ibc-go/pull/3520)) See [diff](https://github.com/cosmos/ibc-go/pull/3520/files#diff-d18972debee5e64f16e40807b2ae112ddbe609504a93ea5e1c80a5d489c3a08a).
### Cosmos SDK v0.50 upgrade

Version `v8.0.0` of ibc-go upgrades to Cosmos SDK v0.50. Please follow the [Cosmos SDK v0.50 upgrading guide](https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-rc.0/UPGRADING.md) to account for its API breaking changes.

### Authority

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just a note that the authority should be set in app.go as well for each module?

You must pass the `authority` to the ica/host keeper. ([#3520](https://github.com/cosmos/ibc-go/pull/3520)) See [diff](https://github.com/cosmos/ibc-go/pull/3520/files#diff-d18972debee5e64f16e40807b2ae112ddbe609504a93ea5e1c80a5d489c3a08a).

```diff
// app.go

// ICA Host keeper
app.ICAHostKeeper = icahostkeeper.NewKeeper(
appCodec, keys[icahosttypes.StoreKey], app.GetSubspace(icahosttypes.SubModuleName),
app.IBCFeeKeeper, // use ics29 fee as ics4Wrapper in middleware stack
app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
app.AccountKeeper, scopedICAHostKeeper, app.MsgServiceRouter(),
+ authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
// ICA Host keeper
app.ICAHostKeeper = icahostkeeper.NewKeeper(
appCodec, keys[icahosttypes.StoreKey], app.GetSubspace(icahosttypes.SubModuleName),
app.IBCFeeKeeper, // use ics29 fee as ics4Wrapper in middleware stack
app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
app.AccountKeeper, scopedICAHostKeeper, app.MsgServiceRouter(),
+ authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
```

- You must pass the `authority` to the icacontroller keeper. ([#3590](https://github.com/cosmos/ibc-go/pull/3590)) See [diff](https://github.com/cosmos/ibc-go/pull/3590/files#diff-d18972debee5e64f16e40807b2ae112ddbe609504a93ea5e1c80a5d489c3a08a).
- You must pass the `authority` to the ica/controller keeper. ([#3590](https://github.com/cosmos/ibc-go/pull/3590)) See [diff](https://github.com/cosmos/ibc-go/pull/3590/files#diff-d18972debee5e64f16e40807b2ae112ddbe609504a93ea5e1c80a5d489c3a08a).

```diff
// app.go

// ICA Controller keeper
app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper(
appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName),
app.IBCFeeKeeper, // use ics29 fee as ics4Wrapper in middleware stack
app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
scopedICAControllerKeeper, app.MsgServiceRouter(),
+ authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
// ICA Controller keeper
app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper(
appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName),
app.IBCFeeKeeper, // use ics29 fee as ics4Wrapper in middleware stack
app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
scopedICAControllerKeeper, app.MsgServiceRouter(),
+ authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
```

- You must pass the `authority` to the ibctransfer keeper. ([#3553](https://github.com/cosmos/ibc-go/pull/3553)) See [diff](https://github.com/cosmos/ibc-go/pull/3553/files#diff-d18972debee5e64f16e40807b2ae112ddbe609504a93ea5e1c80a5d489c3a08a).

```diff
// app.go

// Create Transfer Keeper and pass IBCFeeKeeper as expected Channel and PortKeeper
// since fee middleware will wrap the IBCKeeper for underlying application.
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName),
app.IBCFeeKeeper, // ISC4 Wrapper: fee IBC middleware
app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
app.AccountKeeper, app.BankKeeper, scopedTransferKeeper,
+ authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
// Create Transfer Keeper and pass IBCFeeKeeper as expected Channel and PortKeeper
// since fee middleware will wrap the IBCKeeper for underlying application.
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName),
app.IBCFeeKeeper, // ISC4 Wrapper: fee IBC middleware
app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
app.AccountKeeper, app.BankKeeper, scopedTransferKeeper,
+ authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
```

- You should pass the `authority` to the IBC keeper. ([#3640](https://github.com/cosmos/ibc-go/pull/3640) and [#3650](https://github.com/cosmos/ibc-go/pull/3650)) See [diff](https://github.com/cosmos/ibc-go/pull/3640/files#diff-d18972debee5e64f16e40807b2ae112ddbe609504a93ea5e1c80a5d489c3a08a).

```diff
// app.go

// IBC Keepers
app.IBCKeeper = ibckeeper.NewKeeper(
- appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper,
+ appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
// IBC Keepers
app.IBCKeeper = ibckeeper.NewKeeper(
appCodec,
keys[ibcexported.StoreKey],
app.GetSubspace(ibcexported.ModuleName),
app.StakingKeeper,
app.UpgradeKeeper,
scopedIBCKeeper,
+ authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
```

### Testing package

- The function `SetupWithGenesisAccounts` has been removed.
- The function [`RelayPacketWithResults`](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/testing/path.go#L66) has been added. This function returns the result of the packet receive transaction, the acknowledgement written on the receiving chain, an error if a relay step fails or the packet commitment does not exist on either chain.

### Params migration

Params are now self managed in the following submodules:
Expand All @@ -120,22 +136,11 @@ Legacy params subspaces must still be initialised in app.go in order to successf

### Governance V1 migration

Proposals have been migrated to [gov v1 messages](https://docs.cosmos.network/v0.50/modules/gov#messages) ref: [#4620](https://github.com/cosmos/ibc-go/pull/4620). The proposal `ClientUpdateProposal` has been removed and replaced with `MsgRecoverClient` and the proposal `UpgradeProposal` has been removed and replaced with `MsgIBCSoftwareUpgrade`.
Proposals have been migrated to [gov v1 messages](https://docs.cosmos.network/v0.50/modules/gov#messages) ref: [#4620](https://github.com/cosmos/ibc-go/pull/4620). The proposal `ClientUpdateProposal` has been deprecated and [`MsgRecoverClient`](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/proto/ibc/core/client/v1/tx.proto#L121-L134) should be used instead. Likewise, the proposal `UpgradeProposal` has been deprecated and [`MsgIBCSoftwareUpgrade`](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/proto/ibc/core/client/v1/tx.proto#L139-L154) should be used instead. Both proposal will be removed at in the next major release.
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved

Ensure that the correct authority field is provided to the ibc keeper.

Remove legacy proposal registration from app.go ref: [#4602](https://github.com/cosmos/ibc-go/pull/4602).

Remove the 02-client proposal handler from the `govRouter`.

```diff
govRouter := govv1beta1.NewRouter()
govRouter.AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler).
AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)).
- AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper))
```
`MsgRecoverClient` and `MsgIBCSoftwareUpgrade` will only be allowed to be executed if the signer is the authority designated at the time of instantiating the IBC keeper. So please make sure that the correct authority is provided to the IBC keeper.

Remove the UpgradeProposalHandler from the BasicModuleManager
Remove the `UpgradeProposalHandler` and `UpdateClientProposalHandler` from the `BasicModuleManager`:

```diff
app.BasicModuleManager = module.NewBasicManagerFromManager(
Expand All @@ -145,28 +150,57 @@ app.BasicModuleManager = module.NewBasicManagerFromManager(
govtypes.ModuleName: gov.NewAppModuleBasic(
[]govclient.ProposalHandler{
paramsclient.ProposalHandler,
- ibcclientclient.UpdateClientProposalHandler,
- ibcclientclient.UpgradeProposalHandler,
},
),
})
```

Support for in-flight legacy recover client proposals (i.e. `ClientUpdateProposal`) will be made for v8, but chains should use `MsgRecoverClient` only afterwards to avoid in-flight client recovery failing when upgrading to v9. See [this issue](https://github.com/cosmos/ibc-go/issues/4721) for more information.

Please note that ibc-go's offers facilities to test an ibc-go upgrade:
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved

- All e2e tests of the repository can be [run with custom Docker chain images](../../../e2e/README.md#running-tests-with-custom-images).
- An [importable workflow](../../../e2e/README.md#importable-workflow) that can be used from any other repository to test chain upgrades.

### Transfer migration

An automatic migration handler (TODO: add link after <https://github.com/cosmos/ibc-go/pull/3104> is merged) is configured in the transfer module to set the [denomination metadata](https://github.com/cosmos/cosmos-sdk/blob/95178ce036741ae6aa7af131fa9fccf3e13fff7a/proto/cosmos/bank/v1beta1/bank.proto#L96-L125) for the IBC denominations of all vouchers minted by the transfer module.
An [automatic migration handler](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/modules/apps/transfer/module.go#L136) is configured in the transfer module to set the [denomination metadata](https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-rc.0/proto/cosmos/bank/v1beta1/bank.proto#L96-L125) for the IBC denominations of all vouchers minted by the transfer module.

## IBC Apps

TODO:
### ICS20 - Transfer

- The function `IsBound` has been renamed to [`hasCapability`](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/modules/apps/transfer/keeper/keeper.go#L98) and made unexported.

- <https://github.com/cosmos/ibc-go/pull/3303>
- <https://github.com/cosmos/ibc-go/pull/3967>
### ICS27 - Interchain Accounts

- Functions [`SerializeCosmosTx`](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/modules/apps/27-interchain-accounts/types/codec.go#L32) and [`DeserializeCosmosTx`](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/modules/apps/27-interchain-accounts/types/codec.go#L76) now accept an extra parameter `encoding` of type `string` that specifies the format in which the transaction messages are marshaled. Both [protobuf and proto3 JSON formats](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/modules/apps/27-interchain-accounts/types/metadata.go#L14-L17) are supported.
- The function `IsBound` of controller submodule has been renamed to [`hasCapability`](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/modules/apps/27-interchain-accounts/controller/keeper/keeper.go#L112) and made unexported.
- The function `IsBound` of host submodule has been renamed to [`hasCapability`](https://github.com/cosmos/ibc-go/blob/v8.0.0-beta.0/modules/apps/27-interchain-accounts/host/keeper/keeper.go#L95) and made unexported.

## Relayers

- Getter functions in `MsgChannelOpenInitResponse`, `MsgChannelOpenTryResponse`, `MsgTransferResponse`, `MsgRegisterInterchainAccountResponse` and `MsgSendTxResponse` have been removed. The fields can be accessed directly.
- `channeltypes.EventTypeTimeoutPacketOnClose` (where `channeltypes` is an import alias for `"github.com/cosmos/ibc-go/v8/modules/core/04-channel"`) has been removed, since core IBC does not emit any event with this key.
- Attribute with key `counterparty_connection_id` has been removed from event with key `connectiontypes.EventTypeConnectionOpenInit` (where `connectiontypes` is an import alias for `"github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"`) and attribute with key `counterparty_channel_id` has been removed from event with key `channeltypes.EventTypeChannelOpenInit` (where `channeltypes` is an import alias for `"github.com/cosmos/ibc-go/v8/modules/core/04-channel"`) since both (counterparty connection ID and counterparty channel ID) are empty on `ConnectionOpenInit` and `ChannelOpenInit` respectively.
- Attribute with key `counterparty_connection_id` has been removed from event with key `connectiontypes.EventTypeConnectionOpenInit` (where `connectiontypes` is an import alias for `"github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"`) and attribute with key `counterparty_channel_id` has been removed from event with key `channeltypes.EventTypeChannelOpenInit` (where `channeltypes` is an import alias for `"github.com/cosmos/ibc-go/v8/modules/core/04-channel"`) since both (counterparty connection ID and counterparty channel ID) are empty on `ConnectionOpenInit` and `ChannelOpenInit` respectively.
- As part of the migration to [governance V1 messages](#governance-v1-migration) the following changes in events have been made:

```diff
// IBC client events vars
var (
EventTypeCreateClient = "create_client"
EventTypeUpdateClient = "update_client"
EventTypeUpgradeClient = "upgrade_client"
EventTypeSubmitMisbehaviour = "client_misbehaviour"
- EventTypeUpdateClientProposal = "update_client_proposal"
- EventTypeUpgradeClientProposal = "upgrade_client_proposal"
+ EventTypeRecoverClient = "recover_client"
+ EventTypeScheduleIBCSoftwareUpgrade = "schedule_ibc_software_upgrade"
EventTypeUpgradeChain = "upgrade_chain"
)
```
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved

## IBC Light Clients

Expand Down
2 changes: 1 addition & 1 deletion e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ json matrix files under .github/compatibility-test-matrices and is equivalent to

## Importable Workflow

This repository contains an [importable workflow](https://github.com/cosmos/ibc-go/blob/bc963bcfd115a0e06b8196b114496db5ea011247/.github/workflows/e2e-compatibility-workflow-call.yaml) that can be used from any other repository to test chain upgrades. The workflow
This repository contains an [importable workflow](https://github.com/cosmos/ibc-go/blob/185a220244663457372185992cfc85ed9e458bf1/.github/workflows/e2e-compatibility-workflow-call.yaml) that can be used from any other repository to test chain upgrades. The workflow
can be used to test both non-IBC chains, and also IBC-enabled chains.

### Prerequisites
Expand Down
Loading