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.

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

chore: loosen assertions in SetOrderBeginBlockers() and SetOrderEndBlockers() #14415

Merged
merged 12 commits into from
Dec 29, 2022
24 changes: 13 additions & 11 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,19 +416,21 @@ func NewSimApp(
// NOTE: staking module is required if HistoricalEntries param > 0
// NOTE: capability module's beginblocker must come before any modules using capabilities (e.g. IBC)
app.ModuleManager.SetOrderBeginBlockers(
0Tech marked this conversation as resolved.
Show resolved Hide resolved
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, group.ModuleName,
paramstypes.ModuleName, vestingtypes.ModuleName, consensusparamtypes.ModuleName,
upgradetypes.ModuleName,
capabilitytypes.ModuleName,
minttypes.ModuleName,
distrtypes.ModuleName,
slashingtypes.ModuleName,
evidencetypes.ModuleName,
stakingtypes.ModuleName,
authz.ModuleName,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the main reason of the PR is to make the list clean, I removed module names which does not have BeginBlock().
But, it's not mandatory.

)
app.ModuleManager.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, group.ModuleName,
paramstypes.ModuleName, upgradetypes.ModuleName, vestingtypes.ModuleName, consensusparamtypes.ModuleName,
crisistypes.ModuleName,
govtypes.ModuleName,
stakingtypes.ModuleName,
feegrant.ModuleName,
group.ModuleName,
)

// NOTE: The genutils module must occur after staking so that pools are
Expand Down
28 changes: 21 additions & 7 deletions types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,32 +293,42 @@ func NewManagerFromMap(moduleMap map[string]appmodule.AppModule) *Manager {

// SetOrderInitGenesis sets the order of init genesis calls
func (m *Manager) SetOrderInitGenesis(moduleNames ...string) {
m.assertNoForgottenModules("SetOrderInitGenesis", moduleNames)
m.assertNoForgottenModules("SetOrderInitGenesis", moduleNames, nil)
m.OrderInitGenesis = moduleNames
}

// SetOrderExportGenesis sets the order of export genesis calls
func (m *Manager) SetOrderExportGenesis(moduleNames ...string) {
m.assertNoForgottenModules("SetOrderExportGenesis", moduleNames)
m.assertNoForgottenModules("SetOrderExportGenesis", moduleNames, nil)
m.OrderExportGenesis = moduleNames
}

// SetOrderBeginBlockers sets the order of set begin-blocker calls
func (m *Manager) SetOrderBeginBlockers(moduleNames ...string) {
m.assertNoForgottenModules("SetOrderBeginBlockers", moduleNames)
m.assertNoForgottenModules("SetOrderBeginBlockers", moduleNames,
func(moduleName string) bool {
module := m.Modules[moduleName]
_, hasBeginBlock := module.(BeginBlockAppModule)
return !hasBeginBlock
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The assertion passes if the module does not have BeginBlock().

})
m.OrderBeginBlockers = moduleNames
}

// SetOrderEndBlockers sets the order of set end-blocker calls
func (m *Manager) SetOrderEndBlockers(moduleNames ...string) {
m.assertNoForgottenModules("SetOrderEndBlockers", moduleNames)
m.assertNoForgottenModules("SetOrderEndBlockers", moduleNames,
func(moduleName string) bool {
module := m.Modules[moduleName]
_, hasEndBlock := module.(EndBlockAppModule)
return !hasEndBlock
})
m.OrderEndBlockers = moduleNames
}

// SetOrderMigrations sets the order of migrations to be run. If not set
// then migrations will be run with an order defined in `DefaultMigrationsOrder`.
func (m *Manager) SetOrderMigrations(moduleNames ...string) {
m.assertNoForgottenModules("SetOrderMigrations", moduleNames)
m.assertNoForgottenModules("SetOrderMigrations", moduleNames, nil)
m.OrderMigrations = moduleNames
}

Expand Down Expand Up @@ -429,21 +439,25 @@ func (m *Manager) checkModulesExists(moduleName []string) error {

// assertNoForgottenModules checks that we didn't forget any modules in the
// SetOrder* functions.
0Tech marked this conversation as resolved.
Show resolved Hide resolved
func (m *Manager) assertNoForgottenModules(setOrderFnName string, moduleNames []string) {
func (m *Manager) assertNoForgottenModules(setOrderFnName string, moduleNames []string, pass func(string) bool) {
ms := make(map[string]bool)
for _, m := range moduleNames {
ms[m] = true
}
var missing []string
for m := range m.Modules {
if pass != nil && pass(m) {
continue
}

Fixed Show fixed Hide fixed
if !ms[m] {
missing = append(missing, m)
}
}
if len(missing) != 0 {
sort.Strings(missing)
panic(fmt.Sprintf(
"%s: all modules must be defined when setting %s, missing: %v", setOrderFnName, setOrderFnName, missing))
"all modules must be defined when setting %s, missing: %v", setOrderFnName, missing))
}
}

Expand Down
11 changes: 7 additions & 4 deletions types/module/module_int_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,19 @@ func (s TestSuite) TestAssertNoForgottenModules() { //nolint:govet
name string
positive bool
modules []string
pass func(string) bool
}{
{"same modules", true, []string{"a", "b"}},
{"more modules", true, []string{"a", "b", "c"}},
{"less modules", false, []string{"a"}, nil},
{"same modules", true, []string{"a", "b"}, nil},
{"more modules", true, []string{"a", "b", "c"}, nil},
{"pass module b", true, []string{"a"}, func(moduleName string) bool { return moduleName == "b" }},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The relevant unit test case.

}

for _, tc := range tcs {
if tc.positive {
m.assertNoForgottenModules("x", tc.modules)
m.assertNoForgottenModules("x", tc.modules, tc.pass)
} else {
s.Panics(func() { m.assertNoForgottenModules("x", tc.modules) })
s.Panics(func() { m.assertNoForgottenModules("x", tc.modules, tc.pass) })
}
}
}
Expand Down