From b65f3fe070c57cf8841d25f5afe110f5fd951aa3 Mon Sep 17 00:00:00 2001 From: Sishir Giri Date: Tue, 19 Jul 2022 10:19:23 -0700 Subject: [PATCH] feat: Move AppModule.BeginBlock and AppModule.EndBlock to extension interfaces (#12603) ## Description Most modules right now have a no-op for AppModule.BeginBlock and AppModule.EndBlock. We should move these methods off the AppModule interface so we have less deadcode, and instead move them to extension interfaces. 1. I added `BeginBlockAppModule` and `EndBlockAppModule` interface. 2. Remove the dead-code from modules that do no implement them 3. Add type casting in the the module code to use the new interface Closes: #12462 --- ### 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/main/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/main/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/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 + types/module/module.go | 21 ++++++++++++++++++--- types/module/module_test.go | 2 -- x/auth/module.go | 9 --------- x/auth/vesting/module.go | 8 -------- x/authz/module/module.go | 5 ----- x/bank/module.go | 9 --------- x/capability/module.go | 6 ------ x/crisis/module.go | 3 --- x/distribution/module.go | 6 ------ x/evidence/module.go | 6 ------ x/feegrant/module/module.go | 3 --- x/gov/module.go | 3 --- x/group/module/module.go | 2 -- x/mint/module.go | 6 ------ x/nft/module/module.go | 7 ------- x/params/module.go | 8 -------- x/slashing/module.go | 6 ------ x/upgrade/abci_test.go | 2 +- x/upgrade/module.go | 5 ----- 20 files changed, 20 insertions(+), 98 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e6677cd6c08..cf848506e06c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (query) [#12253](https://github.com/cosmos/cosmos-sdk/pull/12253) Add `GenericFilteredPaginate` to the `query` package to improve UX. * (telemetry) [#12405](https://github.com/cosmos/cosmos-sdk/pull/12405) Add _query_ calls metric to telemetry. * (sdk.Coins) [#12627](https://github.com/cosmos/cosmos-sdk/pull/12627) Make a Denoms method on sdk.Coins. +* (upgrade) [#12603](https://github.com/cosmos/cosmos-sdk/pull/12603) feat: Move AppModule.BeginBlock and AppModule.EndBlock to extension interfaces ### Improvements diff --git a/types/module/module.go b/types/module/module.go index 81bd7b246913..bbf4abcadba7 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -169,9 +169,17 @@ type AppModule interface { // introduced by the module. To avoid wrong/empty versions, the initial version // should be set to 1. ConsensusVersion() uint64 +} - // ABCI +// BeginBlockAppModule is an extension interface that contains information about the AppModule and BeginBlock. +type BeginBlockAppModule interface { + AppModule BeginBlock(sdk.Context, abci.RequestBeginBlock) +} + +// EndBlockAppModule is an extension interface that contains information about the AppModule and EndBlock. +type EndBlockAppModule interface { + AppModule EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate } @@ -468,7 +476,10 @@ func (m *Manager) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) abci.R ctx = ctx.WithEventManager(sdk.NewEventManager()) for _, moduleName := range m.OrderBeginBlockers { - m.Modules[moduleName].BeginBlock(ctx, req) + module, ok := m.Modules[moduleName].(BeginBlockAppModule) + if ok { + module.BeginBlock(ctx, req) + } } return abci.ResponseBeginBlock{ @@ -484,7 +495,11 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo validatorUpdates := []abci.ValidatorUpdate{} for _, moduleName := range m.OrderEndBlockers { - moduleValUpdates := m.Modules[moduleName].EndBlock(ctx, req) + module, ok := m.Modules[moduleName].(EndBlockAppModule) + if !ok { + continue + } + moduleValUpdates := module.EndBlock(ctx, req) // use these validator updates if provided, the module manager assumes // only one module will update the validator set diff --git a/types/module/module_test.go b/types/module/module_test.go index 37bbf088cc70..926a4a4ab3f8 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -79,8 +79,6 @@ func TestGenesisOnlyAppModule(t *testing.T) { // no-op goam.RegisterInvariants(mockInvariantRegistry) - goam.BeginBlock(sdk.Context{}, abci.RequestBeginBlock{}) - require.Equal(t, []abci.ValidatorUpdate{}, goam.EndBlock(sdk.Context{}, abci.RequestEndBlock{})) } func TestManagerOrderSetters(t *testing.T) { diff --git a/x/auth/module.go b/x/auth/module.go index 5eb6b0f9e308..52052b362a83 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -171,15 +171,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion } -// BeginBlock returns the begin blocker for the auth module. -func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} - -// EndBlock returns the end blocker for the auth module. It returns no validator -// updates. -func (AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // AppModuleSimulation functions // GenerateGenesisState creates a randomized GenState of the auth module diff --git a/x/auth/vesting/module.go b/x/auth/vesting/module.go index 4484e0a34be2..b893e160c6fa 100644 --- a/x/auth/vesting/module.go +++ b/x/auth/vesting/module.go @@ -116,14 +116,6 @@ func (am AppModule) InitGenesis(_ sdk.Context, _ codec.JSONCodec, _ json.RawMess return []abci.ValidatorUpdate{} } -// BeginBlock performs a no-op. -func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} - -// EndBlock performs a no-op. -func (am AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // ExportGenesis is always empty, as InitGenesis does nothing either. func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONCodec) json.RawMessage { return am.DefaultGenesis(cdc) diff --git a/x/authz/module/module.go b/x/authz/module/module.go index 6f1e2431bde2..29c12c7a5bc3 100644 --- a/x/authz/module/module.go +++ b/x/authz/module/module.go @@ -169,11 +169,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, am.keeper) } -// EndBlock does nothing -func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - func init() { appmodule.Register( &modulev1.Module{}, diff --git a/x/bank/module.go b/x/bank/module.go index 4a31f85cc9e9..1e8b1c094e8d 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -172,15 +172,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 4 } -// BeginBlock performs a no-op. -func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} - -// EndBlock returns the end blocker for the bank module. It returns no validator -// updates. -func (AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // AppModuleSimulation functions // GenerateGenesisState creates a randomized GenState of the bank module. diff --git a/x/capability/module.go b/x/capability/module.go index a4c3f5e818ad..959df84dfc31 100644 --- a/x/capability/module.go +++ b/x/capability/module.go @@ -160,12 +160,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { } } -// EndBlock executes all ABCI EndBlock logic respective to the capability module. It -// returns no validator updates. -func (am AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // GenerateGenesisState creates a randomized GenState of the capability module. func (AppModule) GenerateGenesisState(simState *module.SimulationState) { simulation.RandomizedGenState(simState) diff --git a/x/crisis/module.go b/x/crisis/module.go index ca66890b9c5f..ee3fec22194d 100644 --- a/x/crisis/module.go +++ b/x/crisis/module.go @@ -179,9 +179,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion } -// BeginBlock performs a no-op. -func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} - // EndBlock returns the end blocker for the crisis module. It returns no validator // updates. func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { diff --git a/x/distribution/module.go b/x/distribution/module.go index 2d1f16c0a31b..d9a9b1b813f0 100644 --- a/x/distribution/module.go +++ b/x/distribution/module.go @@ -186,12 +186,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, req, am.keeper) } -// EndBlock returns the end blocker for the distribution module. It returns no validator -// updates. -func (AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // AppModuleSimulation functions // GenerateGenesisState creates a randomized GenState of the distribution module. diff --git a/x/evidence/module.go b/x/evidence/module.go index 1b9759c34a69..009807e88166 100644 --- a/x/evidence/module.go +++ b/x/evidence/module.go @@ -176,12 +176,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, req, am.keeper) } -// EndBlock executes all ABCI EndBlock logic respective to the evidence module. It -// returns no validator updates. -func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // AppModuleSimulation functions // GenerateGenesisState creates a randomized GenState of the evidence module. diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go index 77aa839f49ad..d2b50f410d7e 100644 --- a/x/feegrant/module/module.go +++ b/x/feegrant/module/module.go @@ -182,9 +182,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 2 } -// BeginBlock returns the begin blocker for the feegrant module. -func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} - // EndBlock returns the end blocker for the feegrant module. It returns no validator // updates. func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { diff --git a/x/gov/module.go b/x/gov/module.go index ad54cdd9f696..dd95f7e18896 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -294,9 +294,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 3 } -// BeginBlock performs a no-op. -func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} - // EndBlock returns the end blocker for the gov module. It returns no validator // updates. func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { diff --git a/x/group/module/module.go b/x/group/module/module.go index 3adaff5d0155..915b24970aba 100644 --- a/x/group/module/module.go +++ b/x/group/module/module.go @@ -153,8 +153,6 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } -func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} - // EndBlock implements the group module's EndBlock. func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { EndBlocker(ctx, am.keeper) diff --git a/x/mint/module.go b/x/mint/module.go index 7945b4abee73..dbd91bac8cfd 100644 --- a/x/mint/module.go +++ b/x/mint/module.go @@ -188,12 +188,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { BeginBlocker(ctx, am.keeper, am.inflationCalculator) } -// EndBlock returns the end blocker for the mint module. It returns no validator -// updates. -func (AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // AppModuleSimulation functions // GenerateGenesisState creates a randomized GenState of the mint module. diff --git a/x/nft/module/module.go b/x/nft/module/module.go index 51f08ff123e5..51537ed7379d 100644 --- a/x/nft/module/module.go +++ b/x/nft/module/module.go @@ -157,13 +157,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } -func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {} - -// EndBlock does nothing -func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // ____________________________________________________________________________ // AppModuleSimulation functions diff --git a/x/params/module.go b/x/params/module.go index bcff2504f2cc..7d851f6fec09 100644 --- a/x/params/module.go +++ b/x/params/module.go @@ -149,14 +149,6 @@ func (am AppModule) ExportGenesis(_ sdk.Context, _ codec.JSONCodec) json.RawMess // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } -// BeginBlock performs a no-op. -func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} - -// EndBlock performs a no-op. -func (AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // // New App Wiring Setup // diff --git a/x/slashing/module.go b/x/slashing/module.go index 239465983b1b..773b920f55f9 100644 --- a/x/slashing/module.go +++ b/x/slashing/module.go @@ -184,12 +184,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(ctx, req, am.keeper) } -// EndBlock returns the end blocker for the slashing module. It returns no validator -// updates. -func (AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // _____________________________________________________________________________________ func init() { diff --git a/x/upgrade/abci_test.go b/x/upgrade/abci_test.go index b76185452b09..e7e3b0b6803a 100644 --- a/x/upgrade/abci_test.go +++ b/x/upgrade/abci_test.go @@ -28,7 +28,7 @@ import ( ) type TestSuite struct { - module module.AppModule + module module.BeginBlockAppModule keeper keeper.Keeper querier sdk.Querier handler govtypes.Handler diff --git a/x/upgrade/module.go b/x/upgrade/module.go index 6589b63343b8..011885f0eec9 100644 --- a/x/upgrade/module.go +++ b/x/upgrade/module.go @@ -151,11 +151,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { BeginBlocker(am.keeper, ctx, req) } -// EndBlock does nothing -func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return []abci.ValidatorUpdate{} -} - // // New App Wiring Setup //