You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
// The module manager assumes only one module will update the
// validator set, and it can't be a new module.
iflen(moduleValUpdates) >0 {
returnnil, errorsmod.Wrapf(sdkerrors.ErrLogic, "validator InitGenesis update is already set by another module")
}
}
However, this is not always desired as the chain might be migrating state from old module to a new module. The call to InitGenesis() erases any migrated parameters and state, leading to problems that are hard to identify and debug.
It should also be possible to call RunMigrations() prior to performing any inter-module migrations in the upgrade handler to avoid having RunMigrations() overwrite the inter-module state changes.
However, both approaches not solve the major problem around UX and the difficulty with identifying and debugging it. There should be a guard against missing this low-level but important detail.
Proposal
We should make it so that chain developers must specifically identify for which new modules to run InitGenesis() during the upgrade. If a new module is added without an explicit setting to disable or enable running InitGenesis() during the next upgrade, the binary should not start and spit out a descriptive error message about where to configure this.
The text was updated successfully, but these errors were encountered:
Summary
Currently, the default behaviour for any new module is to run
InitGenesis()
inRunMigrations()
. This is called from the upgrade handler:cosmos-sdk/types/module/module.go
Lines 620 to 629 in 9569b34
However, this is not always desired as the chain might be migrating state from old module to a new module. The call to
InitGenesis()
erases any migrated parameters and state, leading to problems that are hard to identify and debug.Problem Definition
In Osmosis, we've run into this issue on 2 different occasions. In both cases, this problem ended up being non-trivial to debug as we simply observed missing parameters after e2e testing our upgrades and no additional context. We have come up with a workaround:
https://github.com/osmosis-labs/osmosis/blob/b49e35e180fdffa8dfe2730f2105803b9045bf89/app/upgrades/v15/upgrades.go#L41-L44
It should also be possible to call
RunMigrations()
prior to performing any inter-module migrations in the upgrade handler to avoid havingRunMigrations()
overwrite the inter-module state changes.However, both approaches not solve the major problem around UX and the difficulty with identifying and debugging it. There should be a guard against missing this low-level but important detail.
Proposal
We should make it so that chain developers must specifically identify for which new modules to run
InitGenesis()
during the upgrade. If a new module is added without an explicit setting to disable or enable runningInitGenesis()
during the next upgrade, the binary should not start and spit out a descriptive error message about where to configure this.The text was updated successfully, but these errors were encountered: