From d3a8e1e9535e4d980723fbabdc274cd55cf965ae Mon Sep 17 00:00:00 2001 From: Amaury <1293565+amaurym@users.noreply.github.com> Date: Fri, 10 Dec 2021 18:35:31 +0100 Subject: [PATCH] fix: Panic if SetOrder* functions forgot modules (#10711) ## Description fixes: #10708 Panic at startup if chain devs forgot to add some modules in SetOrder* functions. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- CHANGELOG.md | 1 + simapp/app.go | 17 ++++++++++++++++- types/module/module.go | 17 +++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da992685c6..9bf2e26dac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#10486](https://github.com/cosmos/cosmos-sdk/pull/10486) store/cachekv's `Store.Write` conservatively looks up keys, but also uses the [map clearing idiom](https://bencher.orijtech.com/perfclinic/mapclearing/) to reduce the RAM usage, CPU time usage, and garbage collection pressure from clearing maps, instead of allocating new maps. * (types) [\#10630](https://github.com/cosmos/cosmos-sdk/pull/10630) Add an `Events` field to the `TxResponse` type that captures _all_ events emitted by a transaction, unlike `Logs` which only contains events emitted during message execution. * (deps) [\#10706](https://github.com/cosmos/cosmos-sdk/issues/10706) Bump rosetta-sdk-go to v0.7.2 and rosetta-cli to v0.7.3 +* (module) [\#10711](https://github.com/cosmos/cosmos-sdk/pull/10711) Panic at startup if the app developer forgot to add modules in the `SetOrder{BeginBlocker, EndBlocker, InitGenesis, ExportGenesis}` functions. This means that all modules, even those who have empty implementations for those methods, need to be added to `SetOrder*`. ### API Breaking Changes diff --git a/simapp/app.go b/simapp/app.go index afbb63c3a6..90a8112861 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -38,6 +38,7 @@ import ( authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/cosmos/cosmos-sdk/x/auth/vesting" + vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" "github.com/cosmos/cosmos-sdk/x/authz" authzkeeper "github.com/cosmos/cosmos-sdk/x/authz/keeper" authzmodule "github.com/cosmos/cosmos-sdk/x/authz/module" @@ -358,8 +359,20 @@ func NewSimApp( app.mm.SetOrderBeginBlockers( upgradetypes.ModuleName, capabilitytypes.ModuleName, minttypes.ModuleName, distrtypes.ModuleName, slashingtypes.ModuleName, evidencetypes.ModuleName, stakingtypes.ModuleName, + authtypes.ModuleName, banktypes.ModuleName, govtypes.ModuleName, crisistypes.ModuleName, genutiltypes.ModuleName, + authz.ModuleName, feegrant.ModuleName, nft.ModuleName, + paramstypes.ModuleName, vestingtypes.ModuleName, + group.ModuleName, + ) + app.mm.SetOrderEndBlockers( + crisistypes.ModuleName, govtypes.ModuleName, stakingtypes.ModuleName, + capabilitytypes.ModuleName, authtypes.ModuleName, banktypes.ModuleName, distrtypes.ModuleName, + slashingtypes.ModuleName, minttypes.ModuleName, + genutiltypes.ModuleName, evidencetypes.ModuleName, authz.ModuleName, + feegrant.ModuleName, nft.ModuleName, + paramstypes.ModuleName, upgradetypes.ModuleName, vestingtypes.ModuleName, + group.ModuleName, ) - app.mm.SetOrderEndBlockers(crisistypes.ModuleName, govtypes.ModuleName, stakingtypes.ModuleName) // NOTE: The genutils module must occur after staking so that pools are // properly initialized with tokens from genesis accounts. @@ -371,6 +384,8 @@ func NewSimApp( slashingtypes.ModuleName, govtypes.ModuleName, minttypes.ModuleName, crisistypes.ModuleName, genutiltypes.ModuleName, evidencetypes.ModuleName, authz.ModuleName, group.ModuleName, feegrant.ModuleName, nft.ModuleName, + paramstypes.ModuleName, upgradetypes.ModuleName, vestingtypes.ModuleName, + group.ModuleName, ) app.mm.RegisterInvariants(&app.CrisisKeeper) diff --git a/types/module/module.go b/types/module/module.go index 6e1012c0ac..ae836509fd 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -253,21 +253,25 @@ func NewManager(modules ...AppModule) *Manager { // SetOrderInitGenesis sets the order of init genesis calls func (m *Manager) SetOrderInitGenesis(moduleNames ...string) { + m.checkForgottenModules("SetOrderInitGenesis", moduleNames) m.OrderInitGenesis = moduleNames } // SetOrderExportGenesis sets the order of export genesis calls func (m *Manager) SetOrderExportGenesis(moduleNames ...string) { + m.checkForgottenModules("SetOrderExportGenesis", moduleNames) m.OrderExportGenesis = moduleNames } // SetOrderBeginBlockers sets the order of set begin-blocker calls func (m *Manager) SetOrderBeginBlockers(moduleNames ...string) { + m.checkForgottenModules("SetOrderBeginBlockers", moduleNames) m.OrderBeginBlockers = moduleNames } // SetOrderEndBlockers sets the order of set end-blocker calls func (m *Manager) SetOrderEndBlockers(moduleNames ...string) { + m.checkForgottenModules("SetOrderEndBlockers", moduleNames) m.OrderEndBlockers = moduleNames } @@ -341,6 +345,19 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) map[string return genesisData } +// checkForgottenModules checks that we didn't forget any modules in the +// SetOrder* functions. +func (m *Manager) checkForgottenModules(setOrderFnName string, moduleNames []string) { + setOrderMap := map[string]struct{}{} + for _, m := range moduleNames { + setOrderMap[m] = struct{}{} + } + + if len(setOrderMap) != len(m.Modules) { + panic(fmt.Sprintf("got %d modules in the module manager, but %d modules in %s", len(m.Modules), len(setOrderMap), setOrderFnName)) + } +} + // MigrationHandler is the migration function that each module registers. type MigrationHandler func(sdk.Context) error