-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Fix upgrade store loader #7817
Fix upgrade store loader #7817
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7817 +/- ##
==========================================
- Coverage 54.17% 54.17% -0.01%
==========================================
Files 612 612
Lines 38999 39000 +1
==========================================
Hits 21129 21129
- Misses 15697 15698 +1
Partials 2173 2173 |
@@ -10,9 +10,9 @@ import ( | |||
// pattern. This is useful for custom upgrade loading logic. | |||
func UpgradeStoreLoader(upgradeHeight int64, storeUpgrades *store.StoreUpgrades) baseapp.StoreLoader { | |||
return func(ms sdk.CommitMultiStore) error { | |||
if upgradeHeight == ms.LastCommitID().Version { | |||
if upgradeHeight == ms.LastCommitID().Version+1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We stop the node process on beginblock(upgradeHeight)
, the block is not commited, so it should be LastCommitID().Version+1
.
There was a very similar pr for launchpad, was that not also on master? If this is needed in addition to the launchpad fix, please backport this to the launchpad branch |
Is the launchpad branch launchpad/backports? This branch does not seem to support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK, thanks!
I just have 2 small questions, will merge right after they are answered.
@ethanfrey Are you talking about #7531? That got backported correctly from master. I think @chengwenxi is right though, I can't see StoreUpgrades in launchpad either. |
* cherry pick 2ea9cf4 * move sm2 from tendermint to sdk * fix bug * register sm2 * fix bug * fix test error * fix bug * fix bug * cherry-pick from cosmos#7817 * update tendermint version Co-authored-by: chengwenxi <[email protected]>
* cherry pick 2ea9cf4 * move sm2 from tendermint to sdk * fix bug * register sm2 * fix bug * fix test error * fix bug * fix bug * cherry-pick from #7817 * update tendermint version Co-authored-by: chengwenxi <[email protected]>
Description
The
rootmultiStore.lastCommitInfo
is empty if it has not been loaded yet, so therootmultiStore.LastCommitID().version
is 0,storeUpgrades
will be executed only whenupgradeHeight == 0
.If the latest version is returned when
rootmultiStore
is not loaded, it works.closes: #6346
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes