Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce PreBlock (backport #17421) #17712

Merged
merged 6 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
16 changes: 16 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,10 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (*abci.Respons
}
}

if err := app.preBlock(); err != nil {
return nil, err
}

Comment on lines 727 to +733
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:660)

beginBlock, err := app.beginBlock(req)
if err != nil {
return nil, err
Expand Down
21 changes: 21 additions & 0 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
8 changes: 8 additions & 0 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
1 change: 1 addition & 0 deletions docs/architecture/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
11 changes: 11 additions & 0 deletions docs/architecture/adr-063-core-module-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions docs/architecture/adr-068-preblock.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion docs/docs/build/building-apps/01-app-go-v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions docs/docs/build/building-apps/03-app-upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading