Problem: Interchain accounts are not enabled in chain-main
#732
Conversation
Solution: Enable intterchain accounts in `app.go` and add an upgrade handler
devashishdxt
requested review from
tomtau,
calvinaco and
yihuang
as code owners
Codecov Report
@@ Coverage Diff @@
## master #732 +/- ##
==========================================
+ Coverage 20.60% 21.03% +0.43%
==========================================
Files 76 89 +13
Lines 9989 10226 +237
==========================================
+ Hits 2058 2151 +93
- Misses 7465 7599 +134
- Partials 466 476 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
app/app.go
Outdated
| "/cosmos.authz.v1beta1.MsgExec", | ||
| "/cosmos.authz.v1beta1.MsgGrant", | ||
| "/cosmos.authz.v1beta1.MsgRevoke", |
There was a problem hiding this comment.
maybe remove authz and feegrant "/cosmos.staking.v1beta1.MsgCreateValidator", "/cosmos.vesting.v1beta1.MsgCreateVestingAccount", and "/cosmos.staking.v1beta1.MsgEditValidator"....
they seem a bit esoteric for interchain accounts?
and maybe add Nft transfer? it could be potentially interesting e.g. if a Nft is owned by an interchain account that corresponds to a smart contract on Cronos?
app/app.go
Outdated
| @@ -478,7 +575,9 @@ func New( | |||
| } | |||
|
|
|||
| if upgradeInfo.Name == planName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) { | |||
| storeUpgrades := storetypes.StoreUpgrades{} | |||
| storeUpgrades := storetypes.StoreUpgrades{ | |||
| Added: []string{icacontrollertypes.StoreKey, icahosttypes.StoreKey}, | |||
There was a problem hiding this comment.
the ica auth module doesn't need to be added?
There was a problem hiding this comment.
ica auth module does not access the store. Anyway, I've added it to the store upgrades just to be safe.
There was a problem hiding this comment.
and it doesn't have any params etc., so no need to initialize anything in the upgrade handler?
or should that 5minute timeout be a param?
There was a problem hiding this comment.
It doesn't have any params right now. But, it's a good idea to move default timeout duration to params. I'll do that in future PR.
There was a problem hiding this comment.
Added an option here: #731 (comment)
There was a problem hiding this comment.
@calvinaco mentioned it may also be good to add minting/burning NFTs or perhaps denom issue?
Ok. I'll keep #726 open until we've reached a conclusion on this. |
Solution: Enable interchain accounts in
app.goand add an upgrade handler