feat: introduce PreBlock (#17421)

CHANGELOG.md

@@ -57,7 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
-* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372) Add `MigrationModuleManager` to handle migration of upgrade module before other modules, ensuring access to the updated context with consensus parameters within the same block that executes the migration.
+* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583), [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372), [#17421](https://github.com/cosmos/cosmos-sdk/pull/17421) Introduce `PreBlock`, which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics.
 * (baseapp) [#17518](https://github.com/cosmos/cosmos-sdk/pull/17518) Utilizing voting power from vote extensions (CometBFT) instead of the current bonded tokens (x/staking) to determine if a set of vote extensions are valid.
 * (config) [#17649](https://github.com/cosmos/cosmos-sdk/pull/17649) Fix `mempool.max-txs` configuration is invalid in `app.config`.
 * (mempool) [#17668](https://github.com/cosmos/cosmos-sdk/pull/17668) Fix: `PriorityNonceIterator.Next()` nil pointer ref for min priority at the end of iteration.

UPGRADING.md

@@ -102,15 +102,21 @@ allows an application to define handlers for these methods via `ExtendVoteHandle
 and `VerifyVoteExtensionHandler` respectively. Please see [here](https://docs.cosmos.network/v0.50/building-apps/vote-extensions)
 for more info.
 
-#### Upgrade
+#### Set PreBlocker
 
 **Users using `depinject` / app v2 do not need any changes, this is abstracted for them.**
 
 ```diff
-+ app.BaseApp.SetMigrationModuleManager(app.ModuleManager)
++ app.SetPreBlocker(app.PreBlocker)
 ```
 
-BaseApp added `SetMigrationModuleManager` for apps to set their ModuleManager which implements `RunMigrationBeginBlock`. This is essential for BaseApp to run `BeginBlock` of upgrade module and inject `ConsensusParams` to context for `beginBlocker` during `beginBlock`.
+```diff
++func (app *SimApp) PreBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (sdk.ResponsePreBlock, error) {
++	return app.ModuleManager.PreBlock(ctx, req)
++}
+```
+
+BaseApp added `SetPreBlocker` for apps. This is essential for BaseApp to run `PreBlock` which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics.
 
 #### Events
 

baseapp/abci.go

@@ -736,6 +736,10 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons
 		}
 	}
 
+	if err := app.preBlock(); err != nil {
+		return nil, err
+	}
+
 	beginBlock, err := app.beginBlock(req)
 	if err != nil {
 		return nil, err

baseapp/baseapp.go

@@ -43,12 +43,6 @@ type (
 	StoreLoader func(ms storetypes.CommitMultiStore) error
 )
 
-// MigrationModuleManager is the interface that a migration module manager should implement to handle
-// the execution of migration logic during the beginning of a block.
-type MigrationModuleManager interface {
-	RunMigrationBeginBlock(ctx sdk.Context) (bool, error)
-}
-
 const (
 	execModeCheck           execMode = iota // Check a transaction
 	execModeReCheck                         // Recheck a (pending) transaction after a commit
@@ -81,6 +75,7 @@ type BaseApp struct {
 	postHandler sdk.PostHandler // post handler, optional, e.g. for tips
 
 	initChainer          sdk.InitChainer                // ABCI InitChain handler
+	preBlocker           sdk.PreBlocker                 // logic to run before BeginBlocker
 	beginBlocker         sdk.BeginBlocker               // (legacy ABCI) BeginBlock handler
 	endBlocker           sdk.EndBlocker                 // (legacy ABCI) EndBlock handler
 	processProposal      sdk.ProcessProposalHandler     // ABCI ProcessProposal handler
@@ -98,9 +93,6 @@ type BaseApp struct {
 	// manages snapshots, i.e. dumps of app state at certain intervals
 	snapshotManager *snapshots.Manager
 
-	// manages migrate module
-	migrationModuleManager MigrationModuleManager
-
 	// volatile states:
 	//
 	// - checkState is set on InitChain and reset on Commit
@@ -283,11 +275,6 @@ func (app *BaseApp) SetMsgServiceRouter(msgServiceRouter *MsgServiceRouter) {
 	app.msgServiceRouter = msgServiceRouter
 }
 
-// SetMigrationModuleManager sets the MigrationModuleManager of a BaseApp.
-func (app *BaseApp) SetMigrationModuleManager(migrationModuleManager MigrationModuleManager) {
-	app.migrationModuleManager = migrationModuleManager
-}
-
 // MountStores mounts all IAVL or DB stores to the provided keys in the BaseApp
 // multistore.
 func (app *BaseApp) MountStores(keys ...storetypes.StoreKey) {
@@ -682,29 +669,34 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (sdk.Context
 	return ctx.WithMultiStore(msCache), msCache
 }
 
+func (app *BaseApp) preBlock() error {
+	if app.preBlocker != nil {
+		ctx := app.finalizeBlockState.ctx
+		rsp, err := app.preBlocker(ctx)
+		if err != nil {
+			return err
+		}
+		// rsp.ConsensusParamsChanged is true from preBlocker means ConsensusParams in store get changed
+		// write the consensus parameters in store to context
+		if rsp.ConsensusParamsChanged {
+			ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx))
+			// GasMeter must be set after we get a context with updated consensus params.
+			gasMeter := app.getBlockGasMeter(ctx)
+			ctx = ctx.WithBlockGasMeter(gasMeter)
+			app.finalizeBlockState.ctx = ctx
+		}
+	}
+	return nil
+}
+
 func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) (sdk.BeginBlock, error) {
 	var (
 		resp sdk.BeginBlock
 		err  error
 	)
 
 	if app.beginBlocker != nil {
-		ctx := app.finalizeBlockState.ctx
-		if app.migrationModuleManager != nil {
-			if success, err := app.migrationModuleManager.RunMigrationBeginBlock(ctx); success {
-				cp := ctx.ConsensusParams()
-				// Manager skips this step if Block is non-nil since upgrade module is expected to set this params
-				// and consensus parameters should not be overwritten.
-				if cp.Block == nil {
-					if cp = app.GetConsensusParams(ctx); cp.Block != nil {
-						ctx = ctx.WithConsensusParams(cp)
-					}
-				}
-			} else if err != nil {
-				return sdk.BeginBlock{}, err
-			}
-		}
-		resp, err = app.beginBlocker(ctx)
+		resp, err = app.beginBlocker(app.finalizeBlockState.ctx)
 		if err != nil {
 			return resp, err
 		}

baseapp/baseapp_test.go

@@ -427,6 +427,9 @@ func TestBaseAppOptionSeal(t *testing.T) {
 	require.Panics(t, func() {
 		suite.baseApp.SetInitChainer(nil)
 	})
+	require.Panics(t, func() {
+		suite.baseApp.SetPreBlocker(nil)
+	})
 	require.Panics(t, func() {
 		suite.baseApp.SetBeginBlocker(nil)
 	})

baseapp/options.go

@@ -176,6 +176,14 @@ func (app *BaseApp) SetInitChainer(initChainer sdk.InitChainer) {
 	app.initChainer = initChainer
 }
 
+func (app *BaseApp) SetPreBlocker(preBlocker sdk.PreBlocker) {
+	if app.sealed {
+		panic("SetPreBlocker() on sealed BaseApp")
+	}
+
+	app.preBlocker = preBlocker
+}
+
 func (app *BaseApp) SetBeginBlocker(beginBlocker sdk.BeginBlocker) {
 	if app.sealed {
 		panic("SetBeginBlocker() on sealed BaseApp")

docs/architecture/README.md

@@ -93,3 +93,4 @@ When writing ADRs, follow the same best practices for writing RFCs. When writing
 * [ADR 044: Guidelines for Updating Protobuf Definitions](./adr-044-protobuf-updates-guidelines.md)
 * [ADR 047: Extend Upgrade Plan](./adr-047-extend-upgrade-plan.md)
 * [ADR 053: Go Module Refactoring](./adr-053-go-module-refactoring.md)
+* [ADR 068: Preblock](./adr-068-preblock.md)

docs/architecture/adr-063-core-module-api.md

@@ -280,6 +280,17 @@ type HasGenesis interface {
 }
 ```
 
+#### Pre Blockers
+
+Modules that have functionality that runs before BeginBlock and should implement the has `HasPreBlocker` interfaces:
+
+```go
+type HasPreBlocker interface {
+  AppModule
+  PreBlock(context.Context) error
+}
+```
+
 #### Begin and End Blockers
 
 Modules that have functionality that runs before transactions (begin blockers) or after transactions

docs/architecture/adr-068-preblock.md

@@ -0,0 +1,60 @@
+# ADR 068: Preblock
+
+## Changelog
+
+* Sept 13, 2023: Initial Draft
+
+## Status
+
+DRAFT
+
+## Abstract
+
+Introduce `PreBlock`, which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics.
+
+## Context
+
+When upgrading to sdk 0.47, the storage format for consensus parameters changed, but in the migration block, `ctx.ConsensusParams()` is always `nil`, because it fails to load the old format using new code, it's supposed to be migrated by the `x/upgrade` module first, but unfortunately, the migration happens in `BeginBlocker` handler, which runs after the `ctx` is initialized.
+When we try to solve this, we find the `x/upgrade` module can't modify the context to make the consensus parameters visible for the other modules, the context is passed by value, and sdk team want to keep it that way, that's good for isolations between modules.
+
+## Alternatives
+
+The first alternative solution introduced a `MigrateModuleManager`, which only includes the `x/upgrade` module right now, and baseapp will run their `BeginBlocker`s before the other modules, and reload context's consensus parameters in between.
+
+## Decision
+
+Suggested this new lifecycle method.
+
+### `PreBlocker`
+
+There are two semantics around the new lifecycle method:
+
+- It runs before the `BeginBlocker` of all modules
+- It can modify consensus parameters in storage, and signal the caller through the return value.
+
+When it returns `ConsensusParamsChanged=true`, the caller must refresh the consensus parameter in the finalize context:
+```
+app.finalizeBlockState.ctx = app.finalizeBlockState.ctx.WithConsensusParams(app.GetConsensusParams())
+```
+
+The new ctx must be passed to all the other lifecycle methods.
+
+
+## Consequences
+
+### Backwards Compatibility
+
+### Positive
+
+### Negative
+
+### Neutral
+
+## Further Discussions
+
+## Test Cases
+
+## References
+* [1] https://github.com/cosmos/cosmos-sdk/issues/16494
+* [2] https://github.com/cosmos/cosmos-sdk/pull/16583
+* [3] https://github.com/cosmos/cosmos-sdk/pull/17421

docs/docs/build/building-apps/01-app-go-v2.md

@@ -37,7 +37,7 @@ The `app_config.go` file is the single place to configure all modules parameters
     https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/simapp/app_config.go#L103-L167
     ```
 
-3. Configure the modules defined in the `BeginBlocker` and `EndBlocker` and the `tx` module:
+3. Configure the modules defined in the `PreBlocker`, `BeginBlocker` and `EndBlocker` and the `tx` module:
 
     ```go reference
     https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/simapp/app_config.go#L112-L129

docs/docs/build/building-apps/03-app-upgrade.md

@@ -50,16 +50,22 @@ the rest of the block as normal. Once 2/3 of the voting power has upgraded, the
 resume the consensus mechanism. If the majority of operators add a custom `do-upgrade` script, this should
 be a matter of minutes and not even require them to be awake at that time.
 
-## Set Migration Module Manager
+## Set PreBlocker
 
 :::tip
 Users using `depinject` / app v2 do not need any changes, this is abstracted for them.
 :::
 
-After app initiation, call `SetMigrationModuleManager` with ModuleManager to give BaseApp access to `RunMigrationBeginBlock`:
+Call `SetPreBlocker` to run `PreBlock`:
 
 ```go
-app.BaseApp.SetMigrationModuleManager(app.ModuleManager)
+app.SetPreBlocker(app.PreBlocker)
+```
+
+```go
+func (app *SimApp) PreBlocker(ctx sdk.Context, req abci.RequestBeginBlock) (sdk.ResponsePreBlock, error) {
+	return app.ModuleManager.PreBlock(ctx, req)
+}
 ```
 
 ## Integrating With An App