From 155d9ecd6e6b0464ea938ac79371c1bc2ec18fbd Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 19 Jul 2022 20:28:47 +0200 Subject: [PATCH] feat: Move AppModule.BeginBlock and AppModule.EndBlock to extension interfaces (backport #12603) (#12638) * 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) (cherry picked from commit b65f3fe070c57cf8841d25f5afe110f5fd951aa3) # Conflicts: # CHANGELOG.md # x/authz/module/module.go # x/group/module/module.go # x/nft/module/module.go # x/params/module.go # x/slashing/module.go # x/upgrade/module.go * remove conflicts * remove conflicts * remove unused modules Co-authored-by: Sishir Giri Co-authored-by: marbar3778 --- CHANGELOG.md | 3 +++ 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/mint/module.go | 6 ------ x/params/module.go | 8 -------- x/slashing/module.go | 6 ------ x/upgrade/abci_test.go | 2 +- x/upgrade/module.go | 5 ----- 18 files changed, 22 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22c1de01c866..8606402db8e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### Features + +* (upgrade) [#12603](https://github.com/cosmos/cosmos-sdk/pull/12603) feat: Move AppModule.BeginBlock and AppModule.EndBlock to extension interfaces ### Bug Fixes * (x/mint) [#12384](https://github.com/cosmos/cosmos-sdk/pull/12384) Ensure `GoalBonded` must be positive when performing `x/mint` parameter validation. diff --git a/types/module/module.go b/types/module/module.go index 62c9725283ce..e674f90d8197 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -178,9 +178,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 } @@ -475,7 +483,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{ @@ -491,7 +502,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 e7356d62b6d0..7e880b4b1003 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -82,8 +82,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 ca2be016761e..ef13f74a590f 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -155,15 +155,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 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 25820b11bf66..78296fdd6fe0 100644 --- a/x/auth/vesting/module.go +++ b/x/auth/vesting/module.go @@ -115,14 +115,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 211b814aa6d7..2c652e1d8fe7 100644 --- a/x/authz/module/module.go +++ b/x/authz/module/module.go @@ -155,11 +155,6 @@ 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/bank/module.go b/x/bank/module.go index c43251acc253..8035240795b9 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -159,15 +159,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 2 } -// 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 65d7a92f7c65..d448f0db4180 100644 --- a/x/capability/module.go +++ b/x/capability/module.go @@ -149,12 +149,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { am.keeper.InitMemStore(ctx) } -// 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 0f75edfcf886..445ef6860432 100644 --- a/x/crisis/module.go +++ b/x/crisis/module.go @@ -159,9 +159,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } -// 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 257747f36df6..bb958b2c68a8 100644 --- a/x/distribution/module.go +++ b/x/distribution/module.go @@ -169,12 +169,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 cfcf5a35d353..f7ce284b17b5 100644 --- a/x/evidence/module.go +++ b/x/evidence/module.go @@ -183,12 +183,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 c9a7555506ad..421ecfca186b 100644 --- a/x/feegrant/module/module.go +++ b/x/feegrant/module/module.go @@ -173,9 +173,6 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { return 1 } -// 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 (AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { diff --git a/x/gov/module.go b/x/gov/module.go index a090b2c90da2..6f60030772e9 100644 --- a/x/gov/module.go +++ b/x/gov/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 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/mint/module.go b/x/mint/module.go index ab03da9b6a37..30575c978072 100644 --- a/x/mint/module.go +++ b/x/mint/module.go @@ -151,12 +151,6 @@ func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { BeginBlocker(ctx, am.keeper) } -// 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/params/module.go b/x/params/module.go index 616c7e28a1c6..cadd2183a588 100644 --- a/x/params/module.go +++ b/x/params/module.go @@ -139,11 +139,3 @@ 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{} -} diff --git a/x/slashing/module.go b/x/slashing/module.go index 5b4412fbd12f..22c57dddf39c 100644 --- a/x/slashing/module.go +++ b/x/slashing/module.go @@ -167,12 +167,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{} -} - // AppModuleSimulation functions // GenerateGenesisState creates a randomized GenState of the slashing module. diff --git a/x/upgrade/abci_test.go b/x/upgrade/abci_test.go index b38b6e21d425..cbda3e429b90 100644 --- a/x/upgrade/abci_test.go +++ b/x/upgrade/abci_test.go @@ -27,7 +27,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 b80b56f92a1f..cb1dc05c689a 100644 --- a/x/upgrade/module.go +++ b/x/upgrade/module.go @@ -129,8 +129,3 @@ func (AppModule) ConsensusVersion() uint64 { return 1 } 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{} -}