Problem: nft module store is not initialized after upgrade #552
Conversation
Solution: - call SetStoreLoader to initialise nft module store
Codecov Report
@@ Coverage Diff @@
## master #552 +/- ##
==========================================
+ Coverage 17.94% 20.76% +2.82%
==========================================
Files 69 69
Lines 8994 9135 +141
==========================================
+ Hits 1614 1897 +283
+ Misses 6869 6686 -183
- Partials 511 552 +41
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk() | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| if upgradeInfo.Name == planName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) { | ||
| storeUpgrades := storetypes.StoreUpgrades{ | ||
| Added: []string{nfttypes.StoreKey}, | ||
| Renamed: []storetypes.StoreRename{}, | ||
| Deleted: []string{}, | ||
| } | ||
|
|
||
| // configure store loader that checks if version == upgradeHeight and applies store upgrades | ||
| app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades)) |
There was a problem hiding this comment.
should this just be called once / inside SetUpgradeHandler ?
There was a problem hiding this comment.
I think the override only works before storage constructed, so it need to happen pretty early. while upgrade handler is called in abci event.
There was a problem hiding this comment.
I get the example code from the godoc of upgrade module: https://pkg.go.dev/github.com/cosmos/[email protected]/x/upgrade#hdr-Performing_Upgrades
There was a problem hiding this comment.
https://github.com/cosmos/cosmos-sdk/blob/master/x/upgrade/types/storeloader.go#L13
And before apply the upgrade, it'll check the height again, so it should be safe here.
tomtau
requested review from
allthatjazzleo,
devashishdxt,
JayT106 and
leejw51crypto
| @@ -414,6 +415,27 @@ func New( | |||
| ) | |||
| app.SetEndBlocker(app.EndBlocker) | |||
|
|
|||
| planName := "v2.0.0" | |||
There was a problem hiding this comment.
can this fetched from other place?
because two places hard-coded
There was a problem hiding this comment.
I think the plan name only specified once here.
| panic(err) | ||
| } | ||
|
|
||
| if upgradeInfo.Name == planName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) { |
There was a problem hiding this comment.
could add some explain for IsSkipHeight,
such as height > upgradeInfo.Height?
There was a problem hiding this comment.
in latest planName is different from "2.0.0"?
and not fall into this step?
There was a problem hiding this comment.
cosmos-sdk has a feature that you can skip upgrades in command line:
$ chain-maind start --help
...
--unsafe-skip-upgrades ints Skip a set of upgrade heights to continue the old binary
Solution:
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)