feat: Move AppModule.BeginBlock and AppModule.EndBlock to extension i…

…nterfaces (backport cosmos#12603) (cosmos#12638) * feat: Move AppModule.BeginBlock and AppModule.EndBlock to extension interfaces (cosmos#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: cosmos#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 b65f3fe) # 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 <[email protected]> Co-authored-by: marbar3778 <[email protected]>
agoric-labs · Sep 28, 2024 · dd94e57 · dd94e57
1 parent c588d13
commit dd94e57
Show file tree

Hide file tree

Showing 15 changed files with 76 additions and 78 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -38,6 +38,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
@@ -213,14 +213,23 @@ func (m *Manager) SetOrderExportGenesis(moduleNames ...string) {
 			return !hasGenesis
 		}
 
-		if _, hasABCIGenesis := module.(HasABCIGenesis); hasABCIGenesis {
-			return !hasABCIGenesis
-		}
+	// ConsensusVersion is a sequence number for state-breaking change of the
+	// module. It should be incremented on each consensus-breaking change
+	// introduced by the module. To avoid wrong/empty versions, the initial version
+	// should be set to 1.
+	ConsensusVersion() uint64
+}
 
-		_, hasGenesis := module.(HasGenesis)
-		return !hasGenesis
-	})
-	m.OrderExportGenesis = moduleNames
+// 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
 }
 
 // SetOrderPreBlockers sets the order of set pre-blocker calls
@@ -697,10 +706,9 @@ func (m *Manager) PreBlock(ctx sdk.Context) error {
 func (m *Manager) BeginBlock(ctx sdk.Context) (sdk.BeginBlock, error) {
 	ctx = ctx.WithEventManager(sdk.NewEventManager())
 	for _, moduleName := range m.OrderBeginBlockers {
-		if module, ok := m.Modules[moduleName].(appmodule.HasBeginBlocker); ok {
-			if err := module.BeginBlock(ctx); err != nil {
-				return sdk.BeginBlock{}, err
-			}
+		module, ok := m.Modules[moduleName].(BeginBlockAppModule)
+		if ok {
+			module.BeginBlock(ctx, req)
 		}
 	}
 
@@ -721,22 +729,11 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo
 	validatorUpdates := []abci.ValidatorUpdate{}
 
 	for _, moduleName := range m.OrderEndBlockers {
-		if module, ok := m.Modules[moduleName].(appmodule.HasEndBlocker); ok {
-			err := module.EndBlock(ctx)
-			if err != nil {
-				return sdk.EndBlock{}, err
-			}
-		} else if module, ok := m.Modules[moduleName].(HasABCIEndBlock); ok {
-			moduleValUpdates, err := module.EndBlock(ctx)
-			if err != nil {
-				return sdk.EndBlock{}, err
-			}
-			// use these validator updates if provided, the module manager assumes
-			// only one module will update the validator set
-			if len(moduleValUpdates) > 0 {
-				if len(validatorUpdates) > 0 {
-					return sdk.EndBlock{}, errors.New("validator EndBlock updates already set by a previous module")
-				}
+		module, ok := m.Modules[moduleName].(EndBlockAppModule)
+		if !ok {
+			continue
+		}
+		moduleValUpdates := module.EndBlock(ctx, req)
 
 				validatorUpdates = append(validatorUpdates, moduleValUpdates...)
 			}

diff --git a/types/module/module_test.go b/types/module/module_test.go
@@ -56,10 +56,8 @@ func TestAssertNoForgottenModules(t *testing.T) {
 		mm.SetOrderExportGenesis("module1")
 	})
 
-	require.Equal(t, []string{"module1", "module3"}, mm.OrderEndBlockers)
-	require.PanicsWithValue(t, "all modules must be defined when setting SetOrderEndBlockers, missing: [module1]", func() {
-		mm.SetOrderEndBlockers("module3")
-	})
+	// no-op
+	goam.RegisterInvariants(mockInvariantRegistry)
 }
 
 func TestManagerOrderSetters(t *testing.T) {

diff --git a/x/auth/module.go b/x/auth/module.go
@@ -170,18 +170,6 @@ func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error {
 		return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx)
 	}
 
-	for _, validator := range validators {
-		if err := validator.ValidateTx(ctx, sdkTx); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
-// ConsensusVersion implements appmodule.HasConsensusVersion
-func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
-
 // 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
@@ -54,15 +54,9 @@ func (am AppModule) InitGenesis(_ sdk.Context, _ codec.JSONCodec, _ json.RawMess
 	return []abci.ValidatorUpdate{}
 }
 
-// RegisterLegacyAminoCodec registers the module's types with the given codec.
-func (AppModule) RegisterLegacyAminoCodec(registrar registry.AminoRegistrar) {
-	types.RegisterLegacyAminoCodec(registrar)
-}
-
-// RegisterInterfaces registers the module's interfaces and implementations with
-// the given interface registry.
-func (AppModule) RegisterInterfaces(registrar registry.InterfaceRegistrar) {
-	types.RegisterInterfaces(registrar)
+// ExportGenesis is always empty, as InitGenesis does nothing either.
+func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONCodec) json.RawMessage {
+	return am.DefaultGenesis(cdc)
 }
 
 // ConsensusVersion implements HasConsensusVersion.

diff --git a/x/authz/module/module.go b/x/authz/module/module.go
@@ -144,10 +144,7 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error)
 // ConsensusVersion implements HasConsensusVersion.
 func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
 
-// BeginBlock returns the begin blocker for the authz module.
-func (am AppModule) BeginBlock(ctx context.Context) error {
-	return BeginBlocker(ctx, am.keeper)
-}
+// ____________________________________________________________________________
 
 // AppModuleSimulation functions
 

diff --git a/x/bank/module.go b/x/bank/module.go
@@ -151,8 +151,8 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error)
 	return am.cdc.MarshalJSON(gs)
 }
 
-// ConsensusVersion implements HasConsensusVersion
-func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
+// ConsensusVersion implements AppModule/ConsensusVersion.
+func (AppModule) ConsensusVersion() uint64 { return 2 }
 
 // AppModuleSimulation functions
 

diff --git a/x/distribution/module.go b/x/distribution/module.go
@@ -170,8 +170,8 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error)
 func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
 
 // BeginBlock returns the begin blocker for the distribution module.
-func (am AppModule) BeginBlock(ctx context.Context) error {
-	return am.keeper.BeginBlocker(ctx)
+func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
+	BeginBlocker(ctx, req, am.keeper)
 }
 
 // AppModuleSimulation functions

diff --git a/x/evidence/module.go b/x/evidence/module.go
@@ -137,8 +137,8 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error)
 func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
 
 // BeginBlock executes all ABCI BeginBlock logic respective to the evidence module.
-func (am AppModule) BeginBlock(ctx context.Context) error {
-	return am.keeper.BeginBlocker(ctx, am.cometService)
+func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
+	BeginBlocker(ctx, req, am.keeper)
 }
 
 // AppModuleSimulation functions

diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go
@@ -137,7 +137,29 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error)
 		return nil, err
 	}
 
-	return am.cdc.MarshalJSON(gs)
+	return cdc.MustMarshalJSON(gs)
+}
+
+// ConsensusVersion implements AppModule/ConsensusVersion.
+func (AppModule) ConsensusVersion() uint64 { return 1 }
+
+// EndBlock returns the end blocker for the feegrant 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 feegrant module.
+func (AppModule) GenerateGenesisState(simState *module.SimulationState) {
+	simulation.RandomizedGenState(simState)
+}
+
+// ProposalContents returns all the feegrant content functions used to
+// simulate governance proposals.
+func (AppModule) ProposalContents(simState module.SimulationState) []simtypes.WeightedProposalContent {
+	return nil
 }
 
 // ConsensusVersion implements HasConsensusVersion

diff --git a/x/gov/module.go b/x/gov/module.go
@@ -198,9 +198,11 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error)
 // ConsensusVersion implements HasConsensusVersion
 func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
 
-// EndBlock returns the end blocker for the gov module.
-func (am AppModule) EndBlock(ctx context.Context) error {
-	return am.keeper.EndBlocker(ctx)
+// 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 {
+	EndBlocker(ctx, am.keeper)
+	return []abci.ValidatorUpdate{}
 }
 
 // AppModuleSimulation functions

diff --git a/x/mint/module.go b/x/mint/module.go
@@ -143,8 +143,8 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error)
 func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
 
 // BeginBlock returns the begin blocker for the mint module.
-func (am AppModule) BeginBlock(ctx context.Context) error {
-	return am.keeper.BeginBlocker(ctx)
+func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) {
+	BeginBlocker(ctx, am.keeper)
 }
 
 // AppModuleSimulation functions

diff --git a/x/params/module.go b/x/params/module.go
@@ -76,8 +76,5 @@ func (am AppModule) RegisterServices(registrar grpc.ServiceRegistrar) error {
 	return nil
 }
 
-// RegisterStoreDecoder doesn't register any type.
-func (AppModule) RegisterStoreDecoder(sdr simtypes.StoreDecoderRegistry) {}
-
-// ConsensusVersion implements HasConsensusVersion
-func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
+// ConsensusVersion implements AppModule/ConsensusVersion.
+func (AppModule) ConsensusVersion() uint64 { return 1 }
diff --git a/x/slashing/module.go b/x/slashing/module.go
@@ -172,8 +172,8 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error)
 func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
 
 // BeginBlock returns the begin blocker for the slashing module.
-func (am AppModule) BeginBlock(ctx context.Context) error {
-	return BeginBlocker(ctx, am.keeper, am.cometService)
+func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
+	BeginBlocker(ctx, req, am.keeper)
 }
 
 // AppModuleSimulation functions

diff --git a/x/upgrade/module.go b/x/upgrade/module.go
@@ -143,7 +143,7 @@ func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion }
 
 // PreBlock calls the upgrade module hooks
 //
-// CONTRACT: this is called *before* all other modules' BeginBlock functions
-func (am AppModule) PreBlock(ctx context.Context) error {
-	return am.keeper.PreBlocker(ctx)
+// CONTRACT: this is registered in BeginBlocker *before* all other modules' BeginBlock functions
+func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
+	BeginBlocker(am.keeper, ctx, req)
 }