Initial ICA integration #837

ethanfrey · 2022-05-04T09:45:28Z

Based on gaiad integration (for host) and referencing #793 for the ice controller setup.

Closes #806

Replaces #793

This enables ICA on the default wasmd application. However, it is very unclear how to add proper icaController support.

TODO:

figure out proper icaAuth setup, or remove icaController support (see https://github.com/cosmos/gaia/blob/ica-acct-auth/docs/hub-tutorials/modules/mauth/mauth-module-tutorial.md)

Please see #839 for updates to use icaAuth

ethanfrey · 2022-05-04T09:46:22Z

app/app.go


-	// icaControllerIBCModule := icacontroller.NewIBCModule(app.icaControllerKeeper, icaAuthModule)
-	// icaHostIBCModule := icahost.NewIBCModule(app.icaHostKeeper)
+	// TODO: clarify how to build icaAuthModule or remove icaController support?


I am a bit lost here.
Can anyone show me an example of non-mock authorization for an icaController?

Otherwise, I would just make this a host... I want to integrate tried-and-true ibc code, not experiment here.

Just host makes sense for me, in this PR

codecov · 2022-05-04T09:54:04Z

Codecov Report

Merging #837 (38932b7) into main (7b78b7e) will increase coverage by 0.14%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #837      +/-   ##
==========================================
+ Coverage   59.63%   59.77%   +0.14%     
==========================================
  Files          52       52              
  Lines        5948     5969      +21     
==========================================
+ Hits         3547     3568      +21     
  Misses       2139     2139              
  Partials      262      262

Impacted Files	Coverage Δ
app/app.go	`88.30% <100.00%> (+0.59%)`	⬆️

ethanfrey · 2022-05-04T11:29:01Z

Happy for a review here.

I could also merge as is (with icaController not working) for the demo of ica integration, and add #839 later, to allow icaController once the downstream dependencies get worked out.

alpe

Looks complete. I have added a lot of comments about the ICA controller setup.
If you want to be strict on the host role, you can remove some setup

alpe · 2022-05-04T14:58:17Z

app/app.go

-		// TODO: ICA
-		// icatypes.ModuleName: nil,
-		wasm.ModuleName: {authtypes.Burner},
+		icatypes.ModuleName:            nil,


app/app.go

alpe · 2022-05-04T15:02:14Z

app/app.go

@@ -290,7 +300,7 @@ func NewWasmApp(
 		minttypes.StoreKey, distrtypes.StoreKey, slashingtypes.StoreKey,
 		govtypes.StoreKey, paramstypes.StoreKey, ibchost.StoreKey, upgradetypes.StoreKey,
 		evidencetypes.StoreKey, ibctransfertypes.StoreKey, capabilitytypes.StoreKey,
-		feegrant.StoreKey, authzkeeper.StoreKey, wasm.StoreKey,
+		feegrant.StoreKey, authzkeeper.StoreKey, wasm.StoreKey, icahosttypes.StoreKey, icacontrollertypes.StoreKey,


icacontrollertypes not required for host role

alpe · 2022-05-04T15:02:33Z

app/app.go

+	ica "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts"
+	icacontroller "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller"
+	icacontrollerkeeper "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/keeper"
+	icacontrollertypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/types"


icacontrollertypes is not required for host role

alpe · 2022-05-04T15:02:48Z

app/app.go

@@ -74,6 +74,14 @@ import (
 	upgradeclient "github.com/cosmos/cosmos-sdk/x/upgrade/client"
 	upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
+	ica "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts"
+	icacontroller "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller"


icacontroller is not required for the host role

alpe · 2022-05-04T15:09:53Z

app/app.go

+	// See https://github.com/cosmos/ibc-go/blob/v3.0.0/docs/apps/interchain-accounts/integration.md
+	// "Note: No `icaauth` exists, this must be substituted with an actual Interchain Accounts authentication module"
+	var icaAuthModule porttypes.IBCModule
+	icaControllerIBCModule := icacontroller.NewIBCModule(app.icaControllerKeeper, icaAuthModule)


let's not have ica controller setup here

alpe · 2022-05-04T15:10:48Z

app/app.go

@@ -508,11 +529,11 @@ func NewWasmApp(
 	}
 	ibcRouter.
 		AddRoute(wasm.ModuleName, wasm.NewIBCHandler(app.wasmKeeper, app.ibcKeeper.ChannelKeeper)).
-		AddRoute(ibctransfertypes.ModuleName, transferIBCModule)
+		AddRoute(ibctransfertypes.ModuleName, transferIBCModule).
+		AddRoute(icacontrollertypes.SubModuleName, icaControllerIBCModule).


remove this line for the just host role only

alpe · 2022-05-04T15:15:56Z

app/app.go

@@ -718,6 +743,8 @@ func NewWasmApp(
 	app.scopedIBCKeeper = scopedIBCKeeper
 	app.scopedTransferKeeper = scopedTransferKeeper
 	app.scopedWasmKeeper = scopedWasmKeeper
+	app.scopedICAHostKeeper = scopedICAHostKeeper
+	app.scopedICAControllerKeeper = scopedICAControllerKeeper


no controller needed

alpe · 2022-05-04T15:16:20Z

app/app.go

@@ -865,6 +892,8 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
 	paramsKeeper.Subspace(crisistypes.ModuleName)
 	paramsKeeper.Subspace(ibctransfertypes.ModuleName)
 	paramsKeeper.Subspace(ibchost.ModuleName)
+	paramsKeeper.Subspace(icahosttypes.SubModuleName)
+	paramsKeeper.Subspace(icacontrollertypes.SubModuleName)


no controller needed

alpe · 2022-05-04T15:22:20Z

app/app.go

-		AddRoute(ibctransfertypes.ModuleName, transferIBCModule)
+		AddRoute(ibctransfertypes.ModuleName, transferIBCModule).
+		AddRoute(icacontrollertypes.SubModuleName, icaControllerIBCModule).
+		AddRoute(icahosttypes.SubModuleName, icaHostIBCModule)


ethanfrey · 2022-05-04T19:56:02Z

Removed all controller code in 38932b7 as we only use host mode here

Added it back in #839 as 9b8fb98 which can be used as a good start for any real controller+auth integration

Reference: CosmWasm/wasmd#837

Reference: CosmWasm/wasmd#837 https://ibc.cosmos.network/main/migrations/v2-to-v3.html#upgrade-proposal

Reference: CosmWasm/wasmd#837 https://ibc.cosmos.network/main/migrations/v2-to-v3.html#upgrade-proposal https://github.com/cosmos/gaia/blob/6742b7158a9b7abf28e3615143620e759cb8307d/app/upgrades/v7/upgrades.go#L15 https://github.com/cosmos/gaia/blob/a4e9d1171d6ab82c95e96bd65c7366d6dee87a49/app/upgrades/v8/upgrades.go#L119 fix

Reference: CosmWasm/wasmd#837 https://ibc.cosmos.network/main/migrations/v2-to-v3.html#upgrade-proposal https://github.com/cosmos/gaia/blob/6742b7158a9b7abf28e3615143620e759cb8307d/app/upgrades/v7/upgrades.go#L15 https://github.com/cosmos/gaia/blob/a4e9d1171d6ab82c95e96bd65c7366d6dee87a49/app/upgrades/v8/upgrades.go#L119

Initial ICA integration, missing auth setup

4fb675e

ethanfrey commented May 4, 2022

View reviewed changes

ethanfrey added this to the v0.27.0 milestone May 4, 2022

ethanfrey mentioned this pull request May 4, 2022

interchain accounts #793

Closed

ethanfrey changed the title ~~Initial ICA integration, missing auth setup~~ Initial ICA integration May 4, 2022

ethanfrey marked this pull request as ready for review May 4, 2022 11:28

ethanfrey requested a review from alpe as a code owner May 4, 2022 11:28

alpe approved these changes May 4, 2022

View reviewed changes

Remove controller, host-only mode

38932b7

ethanfrey merged commit bef8a31 into main May 4, 2022

youngjoon-lee added a commit to medibloc/panacea-core that referenced this pull request Feb 4, 2023

fead: initial ICA integration

fb53438

Reference: CosmWasm/wasmd#837

youngjoon-lee added a commit to medibloc/panacea-core that referenced this pull request Feb 4, 2023

fead: initial ICA integration

6e10226

Reference: CosmWasm/wasmd#837 https://ibc.cosmos.network/main/migrations/v2-to-v3.html#upgrade-proposal

youngjoon-lee added a commit to medibloc/panacea-core that referenced this pull request Feb 4, 2023

feat: initial ICA integration

e4f64f5

Reference: CosmWasm/wasmd#837 https://ibc.cosmos.network/main/migrations/v2-to-v3.html#upgrade-proposal

alpe deleted the interchain-accounts branch May 10, 2023 08:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial ICA integration #837

Initial ICA integration #837

ethanfrey commented May 4, 2022 •

edited

Loading

ethanfrey May 4, 2022

alpe May 4, 2022

codecov bot commented May 4, 2022 •

edited

Loading

ethanfrey commented May 4, 2022

alpe left a comment

alpe May 4, 2022

alpe May 4, 2022

alpe May 4, 2022

alpe May 4, 2022

alpe May 4, 2022

alpe May 4, 2022

alpe May 4, 2022

alpe May 4, 2022

alpe May 4, 2022

ethanfrey commented May 4, 2022

Initial ICA integration #837

Initial ICA integration #837

Conversation

ethanfrey commented May 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 4, 2022 • edited Loading

Codecov Report

ethanfrey commented May 4, 2022

alpe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanfrey commented May 4, 2022

ethanfrey commented May 4, 2022 •

edited

Loading

codecov bot commented May 4, 2022 •

edited

Loading