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

feat: upgrade ibc-go from v2 to v4.3.0 (with cosmos-sdk v0.45.12) #630

Merged
merged 14 commits into from
Feb 13, 2023

Conversation

youngjoon-lee
Copy link
Contributor

@youngjoon-lee youngjoon-lee commented Feb 4, 2023

Motivation

Susannah, [Feb 2, 2023 22:12:17]:
Hello, I have queried medibloc and see that you are using v2.0.3 of ibc-go which is in the end of life. This means that patches and bug fixes are no longer back-ported to this version. We strongly encourage you to upgrade your version of ibc to v4.2.0, this is because v3.4.x will be in end of life in the middle of march. The migration documentation to go from v2 to v3 is here https://ibc.cosmos.network/main/migrations/v2-to-v3.html and for v3 to v4, take a look here https://ibc.cosmos.network/main/migrations/v3-to-v4.html

What I did

  • Upgraded ibc-go from v2 to v4
    • Without adopting the following new features:
      • ICS27 Interchain Accounts (aka, ICA)
      • ICS29 Fee Middleware. For more details, see a comment below.
    • This requires us to upgrade wasmd to v0.30.0+ and cosmos-sdk to v0.45.12+. For more details, please see a comment below.
    • There are a lot of changes in app.go. I put reference links as PR comments below.

Tests

  • Tested IBC transfers manually with a Panacea Localnet and the Osmosis Mainnet.
  • Also tested IBC transfers before and after upgrading the chain from v2.0.5 to HEAD.

Future work

@youngjoon-lee youngjoon-lee self-assigned this Feb 4, 2023
@@ -62,7 +62,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
ante.NewSigGasConsumeDecorator(options.AccountKeeper, sigGasConsumer),
ante.NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler),
ante.NewIncrementSequenceDecorator(options.AccountKeeper),
ibcante.NewAnteDecorator(options.IBCChannelkeeper),
ibcante.NewAnteDecorator(options.IBCKeeper),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -492,8 +492,8 @@ func New(

// Create static IBC router, add transfer route, then set and seal it
ibcRouter := porttypes.NewRouter()
ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferModule)
ibcRouter.AddRoute(wasm.ModuleName, wasm.NewIBCHandler(app.wasmKeeper, app.IBCKeeper.ChannelKeeper))
ibcRouter.AddRoute(ibctransfertypes.ModuleName, transfer.NewIBCModule(app.TransferKeeper))
Copy link
Contributor Author

@youngjoon-lee youngjoon-lee Feb 8, 2023

Choose a reason for hiding this comment

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

Regarding the use of transfer.NewIBCModule(...) instead of transferModule (that was created by transfer.NewAppModule(...)), https://ibc.cosmos.network/main/migrations/v2-to-v3.html#ibc-application-callbacks-moved-from-appmodule-to-ibcmodule

@@ -3,72 +3,75 @@ module github.com/medibloc/panacea-core/v2
go 1.19

require (
github.com/CosmWasm/wasmd v0.24.0
github.com/btcsuite/btcd v0.22.1
github.com/CosmWasm/wasmd v0.30.0
Copy link
Contributor Author

@youngjoon-lee youngjoon-lee Feb 8, 2023

Choose a reason for hiding this comment

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

For IBC v4, wasmd must be also upgraded to v0.30.0 that is the first version based on IBC v4. This also requires us to upgrade cosmos-sdk to v0.45.11+.
https://github.com/CosmWasm/wasmd/blob/main/CHANGELOG.md#v0300-2022-12-02

Anyway, we have to upgrade cosmos-sdk to v0.45.12, as ibc-go v4.3.0 requires it.
https://github.com/cosmos/ibc-go/blob/main/RELEASES.md#version-matrix

Comment on lines 408 to 414
// Create Transfer Keepers
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName),
app.IBCKeeper.ChannelKeeper, // may be replaced with middleware such as ics29 fee
app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
app.AccountKeeper, app.BankKeeper, scopedTransferKeeper,
)
Copy link
Contributor Author

@youngjoon-lee youngjoon-lee Feb 8, 2023

Choose a reason for hiding this comment

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

Since IBC v3, IBC middleware has been introduced.

As above, if we don't want to use any middleware for now, we can just pass the app.IBCKeeper.ChannelKeeer as a ICS4Wrapper parameter. It's explained in the IBC v2->v3 migration guide. Also, I referenced the ibc-go simapp.

Later, when we want to use any IBC middleware such as IBC Fee middleware, we can modify this code to pass a IBCFeeKeeper, as ibc-go simapp did.

Still, both Cosmos Hub and Osmosis don't use IBC Fee middleware yet.

ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferModule)
ibcRouter.AddRoute(wasm.ModuleName, wasm.NewIBCHandler(app.wasmKeeper, app.IBCKeeper.ChannelKeeper))
ibcRouter.AddRoute(ibctransfertypes.ModuleName, transfer.NewIBCModule(app.TransferKeeper))
ibcRouter.AddRoute(wasm.ModuleName, wasm.NewIBCHandler(app.wasmKeeper, app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper))
Copy link
Contributor Author

@youngjoon-lee youngjoon-lee Feb 8, 2023

Choose a reason for hiding this comment

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

@youngjoon-lee youngjoon-lee changed the title feat: WIP: upgrade ibc-go from v2 to v4 (with upgrading cosmos-sdk to v0.45.12) feat: WIP: upgrade ibc-go from v2 to v4.3.0 (with upgrading cosmos-sdk to v0.45.12) Feb 10, 2023
@youngjoon-lee youngjoon-lee changed the title feat: WIP: upgrade ibc-go from v2 to v4.3.0 (with upgrading cosmos-sdk to v0.45.12) feat: upgrade ibc-go from v2 to v4.3.0 (with upgrading cosmos-sdk to v0.45.12) Feb 10, 2023
go.mod Outdated Show resolved Hide resolved
@youngjoon-lee youngjoon-lee marked this pull request as ready for review February 10, 2023 07:27
@youngjoon-lee youngjoon-lee changed the title feat: upgrade ibc-go from v2 to v4.3.0 (with upgrading cosmos-sdk to v0.45.12) feat: upgrade ibc-go from v2 to v4.3.0 (with cosmos-sdk v0.45.12) Feb 10, 2023
Copy link
Contributor

@gyuguen gyuguen left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you for your hard working!

It seems to have applied well when I saw the migration documentation

Copy link
Contributor

@0xHansLee 0xHansLee left a comment

Choose a reason for hiding this comment

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

lgtm.

gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
nhooyr.io/websocket v1.8.6 // indirect
)

replace (
github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v0.8.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This replace directive can be deleted as described in the release note of cosmos-sdk v0.45.12. https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.12

Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this redirection anymore 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants