feat: introduce PreBlock (backport #17421) (#17712)

Co-authored-by: mmsqe <mavis@crypto.com> Co-authored-by: Julien Robert <julien@rbrt.fr>
cosmos · Sep 13, 2023 · 4c083c6 · 4c083c6
1 parent 92d1d9a
commit 4c083c6
Show file tree

Hide file tree

Showing 41 changed files with 476 additions and 94 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Features
 
+* (baseapp & types) [#17712](https://github.com/cosmos/cosmos-sdk/pull/17712) 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. Additionally it can be used for vote extensions.
 * (x/bank) [#14224](https://github.com/cosmos/cosmos-sdk/pull/14224) Allow injection of restrictions on transfers using `AppendSendRestriction` or `PrependSendRestriction`.
 * (genutil) [#17571](https://github.com/cosmos/cosmos-sdk/pull/17571) Allow creation of `AppGenesis` without a file lookup.
 

diff --git a/UPGRADING.md b/UPGRADING.md
@@ -67,6 +67,22 @@ 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.
 
+#### Set PreBlocker
+
+**Users using `depinject` / app v2 do not need any changes, this is abstracted for them.**
+
+```diff
++ app.SetPreBlocker(app.PreBlocker)
+```
+
+```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
 
 The log section of `abci.TxResult` is not populated in the case of successful

diff --git a/baseapp/abci.go b/baseapp/abci.go
@@ -727,6 +727,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

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
@@ -75,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
@@ -664,6 +665,26 @@ 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

diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go
@@ -422,6 +422,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)
 	})

diff --git a/baseapp/options.go b/baseapp/options.go
@@ -158,6 +158,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")

diff --git a/docs/architecture/README.md b/docs/architecture/README.md
@@ -92,3 +92,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)
diff --git a/docs/architecture/adr-063-core-module-api.md b/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

diff --git a/docs/architecture/adr-068-preblock.md b/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
diff --git a/docs/docs/build/building-apps/01-app-go-v2.md b/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

diff --git a/docs/docs/build/building-apps/03-app-upgrade.md b/docs/docs/build/building-apps/03-app-upgrade.md
@@ -50,6 +50,24 @@ 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 PreBlocker
+
+:::tip
+Users using `depinject` / app v2 do not need any changes, this is abstracted for them.
+:::
+
+Call `SetPreBlocker` to run `PreBlock`:
+
+```go
+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
 
 Setup an upgrade Keeper for the app and then define a `BeginBlocker` that calls the upgrade