-
Notifications
You must be signed in to change notification settings - Fork 625
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: denom traces migration handler (#1680)
* update code & test * register migrator service (cherry picked from commit be5ccf3) # Conflicts: # CHANGELOG.md # docs/migrations/support-denoms-with-slashes.md # docs/migrations/v3-to-v4.md # modules/apps/transfer/types/trace.go
- Loading branch information
1 parent
d85319b
commit fc5f607
Showing
8 changed files
with
391 additions
and
40 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
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
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,128 @@ | ||
| # Migrating from ibc-go v3 to v4 | ||
|
|
||
| This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG. | ||
| Any changes that must be done by a user of ibc-go should be documented here. | ||
|
|
||
| There are four sections based on the four potential user groups of this document: | ||
| - Chains | ||
| - IBC Apps | ||
| - Relayers | ||
| - IBC Light Clients | ||
|
|
||
| **Note:** ibc-go supports golang semantic versioning and therefore all imports must be updated to bump the version number on major releases. | ||
| ```go | ||
| github.com/cosmos/ibc-go/v3 -> github.com/cosmos/ibc-go/v4 | ||
| ``` | ||
|
|
||
| No genesis or in-place migrations required when upgrading from v1 or v2 of ibc-go. | ||
|
|
||
| ## Chains | ||
|
|
||
| ### Migration to fix support for base denoms with slashes | ||
|
|
||
| As part of [v1.5.0](https://github.com/cosmos/ibc-go/releases/tag/v1.5.0), [v2.3.0](https://github.com/cosmos/ibc-go/releases/tag/v2.3.0) and [v3.1.0](https://github.com/cosmos/ibc-go/releases/tag/v3.1.0) some [migration handler code sample was documented](https://github.com/cosmos/ibc-go/blob/main/docs/migrations/support-denoms-with-slashes.md#upgrade-proposal) that needs to run in order to correct the trace information of coins transferred using ICS20 whose base denom contains slashes. | ||
|
|
||
| Based on feedback from the community we add now an improved solution to run the same migration that does not require copying a large piece of code over from the migration document, but instead requires only adding a one-line upgrade handler. | ||
|
|
||
| If the chain will migrate to supporting base denoms with slashes, it must set the appropriate params during the execution of the upgrade handler in `app.go`: | ||
| ```go | ||
| app.UpgradeKeeper.SetUpgradeHandler("MigrateTraces", | ||
| func(ctx sdk.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { | ||
| // transfer module consensus version has been bumped to 2 | ||
| return app.mm.RunMigrations(ctx, app.configurator, fromVM) | ||
| }) | ||
|
|
||
| ``` | ||
|
|
||
| If a chain receives coins of a base denom with slashes before it upgrades to supporting it, the receive may pass however the trace information will be incorrect. | ||
|
|
||
| E.g. If a base denom of `testcoin/testcoin/testcoin` is sent to a chain that does not support slashes in the base denom, the receive will be successful. However, the trace information stored on the receiving chain will be: `Trace: "transfer/{channel-id}/testcoin/testcoin", BaseDenom: "testcoin"`. | ||
|
|
||
| This incorrect trace information must be corrected when the chain does upgrade to fully supporting denominations with slashes. | ||
|
|
||
| ## IBC Apps | ||
|
|
||
| ### ICS03 - Connection | ||
|
|
||
| Crossing hellos have been removed from 03-connection handshake negotiation. | ||
| `PreviousConnectionId` in `MsgConnectionOpenTry` has been deprecated and is no longer used by core IBC. | ||
|
|
||
| ### ICS04 - Channel | ||
|
|
||
| The `WriteAcknowledgement` API now takes the `exported.Acknowledgement` type instead of passing in the acknowledgement byte array directly. | ||
| This is an API breaking change and as such IBC application developers will have to update any calls to `WriteAcknowledgement`. | ||
|
|
||
| The `OnChanOpenInit` application callback has been modified. | ||
| The return signature now includes the application version as detailed in the latest IBC [spec changes](https://github.com/cosmos/ibc/pull/629). | ||
|
|
||
| The `NewErrorAcknowledgement` method signature has changed. | ||
| It now accepts an `error` rather than a `string`. This was done in order to prevent accidental state changes. | ||
| All error acknowledgements now contain a deterministic ABCI code and error message. It is the responsibility of the application developer to emit error details in events. | ||
|
|
||
| Crossing hellos have been removed from 04-channel handshake negotiation. | ||
| IBC Applications no longer need to account from already claimed capabilities in the `OnChanOpenTry` callback. The capability provided by core IBC must be able to be claimed with error. | ||
| `PreviousChannelId` in `MsgChannelOpenTry` has been deprecated and is no longer used by core IBC. | ||
|
|
||
| ### ICS27 - Interchain Accounts | ||
|
|
||
| The `RegisterInterchainAccount` API has been modified to include an additional `version` argument. This change has been made in order to support ICS29 fee middleware, for relayer incentivization of ICS27 packets. | ||
| Consumers of the `RegisterInterchainAccount` are now expected to build the appropriate JSON encoded version string themselves and pass it accordingly. | ||
| This should be constructed within the interchain accounts authentication module which leverages the APIs exposed via the interchain accounts `controllerKeeper`. If an empty string is passed in the `version` argument, then the version will be initialized to a default value in the `OnChanOpenInit` callback of the controller's handler, so that channel handshake can proceed. | ||
|
|
||
| The following code snippet illustrates how to construct an appropriate interchain accounts `Metadata` and encode it as a JSON bytestring: | ||
|
|
||
| ```go | ||
| icaMetadata := icatypes.Metadata{ | ||
| Version: icatypes.Version, | ||
| ControllerConnectionId: controllerConnectionID, | ||
| HostConnectionId: hostConnectionID, | ||
| Encoding: icatypes.EncodingProtobuf, | ||
| TxType: icatypes.TxTypeSDKMultiMsg, | ||
| } | ||
|
|
||
| appVersion, err := icatypes.ModuleCdc.MarshalJSON(&icaMetadata) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err := k.icaControllerKeeper.RegisterInterchainAccount(ctx, msg.ConnectionId, msg.Owner, string(appVersion)); err != nil { | ||
| return err | ||
| } | ||
| ``` | ||
|
|
||
| Similarly, if the application stack is configured to route through ICS29 fee middleware and a fee enabled channel is desired, construct the appropriate ICS29 `Metadata` type: | ||
|
|
||
| ```go | ||
| icaMetadata := icatypes.Metadata{ | ||
| Version: icatypes.Version, | ||
| ControllerConnectionId: controllerConnectionID, | ||
| HostConnectionId: hostConnectionID, | ||
| Encoding: icatypes.EncodingProtobuf, | ||
| TxType: icatypes.TxTypeSDKMultiMsg, | ||
| } | ||
|
|
||
| appVersion, err := icatypes.ModuleCdc.MarshalJSON(&icaMetadata) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| feeMetadata := feetypes.Metadata{ | ||
| AppVersion: string(appVersion), | ||
| FeeVersion: feetypes.Version, | ||
| } | ||
|
|
||
| feeEnabledVersion, err := feetypes.ModuleCdc.MarshalJSON(&feeMetadata) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err := k.icaControllerKeeper.RegisterInterchainAccount(ctx, msg.ConnectionId, msg.Owner, string(feeEnabledVersion)); err != nil { | ||
| return err | ||
| } | ||
| ``` | ||
|
|
||
| ## Relayers | ||
|
|
||
| When using the `DenomTrace` gRPC, the full IBC denomination with the `ibc/` prefix may now be passed in. | ||
|
|
||
| Crossing hellos are no longer supported by core IBC for 03-connection and 04-channel. The handshake should be completed in the logical 4 step process (INIT, TRY, ACK, CONFIRM). |
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,59 @@ | ||
| package keeper | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
|
||
| "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types" | ||
| ) | ||
|
|
||
| // 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} | ||
| } | ||
|
|
||
| // MigrateTraces migrates the DenomTraces to the correct format, accounting for slashes in the BaseDenom. | ||
| func (m Migrator) MigrateTraces(ctx sdk.Context) error { | ||
|
|
||
| // list of traces that must replace the old traces in store | ||
| var newTraces []types.DenomTrace | ||
| m.keeper.IterateDenomTraces(ctx, | ||
| func(dt types.DenomTrace) (stop bool) { | ||
| // check if the new way of splitting FullDenom | ||
| // is the same as the current DenomTrace. | ||
| // If it isn't then store the new DenomTrace in the list of new traces. | ||
| newTrace := types.ParseDenomTrace(dt.GetFullDenomPath()) | ||
| err := newTrace.Validate() | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| if dt.IBCDenom() != newTrace.IBCDenom() { | ||
| // The new form of parsing will result in a token denomination change. | ||
| // A bank migration is required. A panic should occur to prevent the | ||
| // chain from using corrupted state. | ||
| panic(fmt.Sprintf("migration will result in corrupted state. Previous IBC token (%s) requires a bank migration. Expected denom trace (%s)", dt, newTrace)) | ||
| } | ||
|
|
||
| if !equalTraces(newTrace, dt) { | ||
| newTraces = append(newTraces, newTrace) | ||
| } | ||
| return false | ||
| }) | ||
|
|
||
| // replace the outdated traces with the new trace information | ||
| for _, nt := range newTraces { | ||
| m.keeper.SetDenomTrace(ctx, nt) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func equalTraces(dtA, dtB types.DenomTrace) bool { | ||
| return dtA.BaseDenom == dtB.BaseDenom && dtA.Path == dtB.Path | ||
| } |
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,123 @@ | ||
| package keeper_test | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| transferkeeper "github.com/cosmos/ibc-go/v4/modules/apps/transfer/keeper" | ||
| transfertypes "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types" | ||
| ) | ||
|
|
||
| func (suite *KeeperTestSuite) TestMigratorMigrateTraces() { | ||
|
|
||
| testCases := []struct { | ||
| msg string | ||
| malleate func() | ||
| expectedTraces transfertypes.Traces | ||
| }{ | ||
|
|
||
| { | ||
| "success: two slashes in base denom", | ||
| func() { | ||
| suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace( | ||
| suite.chainA.GetContext(), | ||
| transfertypes.DenomTrace{ | ||
| BaseDenom: "pool/1", Path: "transfer/channel-0/gamm", | ||
| }) | ||
| }, | ||
| transfertypes.Traces{ | ||
| { | ||
| BaseDenom: "gamm/pool/1", Path: "transfer/channel-0", | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| "success: one slash in base denom", | ||
| func() { | ||
| suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace( | ||
| suite.chainA.GetContext(), | ||
| transfertypes.DenomTrace{ | ||
| BaseDenom: "0x85bcBCd7e79Ec36f4fBBDc54F90C643d921151AA", Path: "transfer/channel-149/erc", | ||
| }) | ||
| }, | ||
| transfertypes.Traces{ | ||
| { | ||
| BaseDenom: "erc/0x85bcBCd7e79Ec36f4fBBDc54F90C643d921151AA", Path: "transfer/channel-149", | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| "success: multiple slashes in a row in base denom", | ||
| func() { | ||
| suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace( | ||
| suite.chainA.GetContext(), | ||
| transfertypes.DenomTrace{ | ||
| BaseDenom: "1", Path: "transfer/channel-5/gamm//pool", | ||
| }) | ||
| }, | ||
| transfertypes.Traces{ | ||
| { | ||
| BaseDenom: "gamm//pool/1", Path: "transfer/channel-5", | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| "success: multihop base denom", | ||
| func() { | ||
| suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace( | ||
| suite.chainA.GetContext(), | ||
| transfertypes.DenomTrace{ | ||
| BaseDenom: "transfer/channel-1/uatom", Path: "transfer/channel-0", | ||
| }) | ||
| }, | ||
| transfertypes.Traces{ | ||
| { | ||
| BaseDenom: "uatom", Path: "transfer/channel-0/transfer/channel-1", | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| "success: non-standard port", | ||
| func() { | ||
| suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace( | ||
| suite.chainA.GetContext(), | ||
| transfertypes.DenomTrace{ | ||
| BaseDenom: "customport/channel-7/uatom", Path: "transfer/channel-0/transfer/channel-1", | ||
| }) | ||
| }, | ||
| transfertypes.Traces{ | ||
| { | ||
| BaseDenom: "uatom", Path: "transfer/channel-0/transfer/channel-1/customport/channel-7", | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| suite.Run(fmt.Sprintf("case %s", tc.msg), func() { | ||
| suite.SetupTest() // reset | ||
|
|
||
| tc.malleate() // explicitly set up denom traces | ||
|
|
||
| migrator := transferkeeper.NewMigrator(suite.chainA.GetSimApp().TransferKeeper) | ||
| err := migrator.MigrateTraces(suite.chainA.GetContext()) | ||
| suite.Require().NoError(err) | ||
|
|
||
| traces := suite.chainA.GetSimApp().TransferKeeper.GetAllDenomTraces(suite.chainA.GetContext()) | ||
| suite.Require().Equal(tc.expectedTraces, traces) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func (suite *KeeperTestSuite) TestMigratorMigrateTracesCorruptionDetection() { | ||
| // IBCDenom() previously would return "customport/channel-0/uatom", but now should return ibc/{hash} | ||
| corruptedDenomTrace := transfertypes.DenomTrace{ | ||
| BaseDenom: "customport/channel-0/uatom", | ||
| Path: "", | ||
| } | ||
| suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace(suite.chainA.GetContext(), corruptedDenomTrace) | ||
|
|
||
| migrator := transferkeeper.NewMigrator(suite.chainA.GetSimApp().TransferKeeper) | ||
| suite.Panics(func() { | ||
| migrator.MigrateTraces(suite.chainA.GetContext()) | ||
| }) | ||
| } |
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
Oops, something went wrong.