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.

Sign up for GitHub

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

Make module.Manager.RunMigrations handle new modules correctly #8988

Merged
merged 4 commits into from
Mar 25, 2021
Merged

Make module.Manager.RunMigrations handle new modules correctly #8988

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
240d7af
Make module.Manager.RunMigrations handle module additions correctly
aaronc Mar 25, 2021
1459145
fix if
aaronc Mar 25, 2021
fac9078
Merge branch 'master' into aaronc/fix-migrations-from-zero
Mar 25, 2021
631e807
Merge branch 'master' into aaronc/fix-migrations-from-zero
aaronc Mar 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions types/module/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,19 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version

updatedVM := make(VersionMap)
for moduleName, module := range m.Modules {
err := c.runModuleMigrations(ctx, moduleName, fromVM[moduleName], module.ConsensusVersion())
updatedVM[moduleName] = module.ConsensusVersion()
if err != nil {
return nil, err
fromVersion := fromVM[moduleName]
toVersion := module.ConsensusVersion()

// only run migrations when the from version is > 0
// from version will be 0 when a new module is added and migrations shouldn't be run in this case
if fromVersion > 0 {
Copy link
Contributor

@amaury1093 amaury1093 Mar 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'm thinking about the hub right now. It has fromVersion == 0 for all modules.

I agree with skipping & calling InitGenesis #8989 (with an empty genesis) for new modules with fromVersion == 0. However, for the hub with pre-existing modules with fromversion == 0, we should do something too.

The easiest way I can think of is add a hardcoded map of old module versions inside the hub's UpgradeHandler, something like this

Copy link
Member Author

@aaronc aaronc Mar 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yep I see no other choice if this is the first upgrade. We need to make sure this is well documented.

err := c.runModuleMigrations(ctx, moduleName, fromVersion, toVersion)
if err != nil {
return nil, err
}
}

updatedVM[moduleName] = toVersion
}

return updatedVM, nil
Expand Down