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: Add PrepareCheckState and Precommit callbacks #14860

Merged
merged 23 commits into from
Apr 23, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
cda4b81
Add Commit callback
prettymuchbryce Jan 31, 2023
862b4d8
Add tests for Commiter change (#5)
prettymuchbryce Feb 3, 2023
1d72615
add precommit callback (#7)
dydxwill Feb 23, 2023
74ef20a
lint
prettymuchbryce Mar 22, 2023
a491aaf
Rename Commit to PrepareCheckState
prettymuchbryce Mar 24, 2023
d01e7ff
Add documentation to module manager
prettymuchbryce Mar 28, 2023
c32d39a
Merge branch 'main' into prettymuchbryce/add-commiter
prettymuchbryce Mar 28, 2023
ee7016a
Fix implementation and docs
prettymuchbryce Mar 28, 2023
1f05374
Merge branch 'main' into prettymuchbryce/add-commiter
prettymuchbryce Mar 28, 2023
f4f81e2
Add CHANGELOG entry
prettymuchbryce Mar 28, 2023
a557977
Add assertions for Precommit/PrepareCheckState and tests
prettymuchbryce Mar 29, 2023
17ba453
Merge branch 'main' into prettymuchbryce/add-commiter
prettymuchbryce Mar 29, 2023
b7dc52d
Add new callbacks to config
prettymuchbryce Mar 29, 2023
eb40e33
Update mod file with api
prettymuchbryce Mar 29, 2023
ea68fee
Update NewManagerFromMap
prettymuchbryce Mar 30, 2023
caa2cde
Merge branch 'main' into prettymuchbryce/add-commiter
prettymuchbryce Mar 30, 2023
a0616d0
Add api to tests package go.mod
prettymuchbryce Mar 30, 2023
22095a4
Merge branch 'main' into prettymuchbryce/add-commiter
prettymuchbryce Mar 30, 2023
5393018
Add api to tests package go.mod
prettymuchbryce Mar 30, 2023
f20ee96
Set callbacks during Load
prettymuchbryce Apr 5, 2023
cf4674e
Fix comment
prettymuchbryce Apr 5, 2023
d40920f
Merge branch 'main' into prettymuchbryce/add-commiter
prettymuchbryce Apr 5, 2023
bd73f81
Merge branch 'main' into prettymuchbryce/add-commiter
prettymuchbryce Apr 18, 2023
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
10 changes: 9 additions & 1 deletion baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -457,14 +457,18 @@ func (app *BaseApp) Commit() abci.ResponseCommit {
header := app.deliverState.ctx.BlockHeader()
retainHeight := app.GetBlockRetentionHeight(header.Height)

if app.precommiter != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@itsdevbear Was curious if this is where you envisioned this being invoked. At this point, your TransientStore will not have been cleared yet.

Choose a reason for hiding this comment

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

This makes sense to me. If we are passing in the ctx without nuking it, I don't think it really matters where it happens.

app.precommiter(app.deliverState.ctx)
}

Comment on lines 457 to +463
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).Commit (baseapp/abci.go:456)

rms, ok := app.cms.(*rootmulti.Store)
if ok {
rms.SetCommitHeader(header)
}

// Write the DeliverTx state into branched storage and commit the MultiStore.
// The write to the DeliverTx state writes all state transitions to the root
// MultiStore (app.cms) so when Commit() is called is persists those values.
// MultiStore (app.cms) so when Commit() is called it persists those values.
Comment on lines 464 to +471
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).Commit (baseapp/abci.go:456)

app.deliverState.ms.Write()
commitID := app.cms.Commit()

Expand Down Expand Up @@ -497,6 +501,10 @@ func (app *BaseApp) Commit() abci.ResponseCommit {
// empty/reset the deliver state
app.deliverState = nil

if app.prepareCheckStater != nil {
app.prepareCheckStater(app.checkState.ctx)
}

var halt bool

switch {
Expand Down
97 changes: 97 additions & 0 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,60 @@ func TestABCI_EndBlock(t *testing.T) {
require.Equal(t, cp.Block.MaxGas, res.ConsensusParamUpdates.Block.MaxGas)
}

func TestBaseApp_PrepareCheckState(t *testing.T) {
db := dbm.NewMemDB()
name := t.Name()
logger := log.NewTestLogger(t)

cp := &cmtproto.ConsensusParams{
Block: &cmtproto.BlockParams{
MaxGas: 5000000,
},
}

app := baseapp.NewBaseApp(name, logger, db, nil)
app.SetParamStore(&paramStore{db: dbm.NewMemDB()})
app.InitChain(abci.RequestInitChain{
ConsensusParams: cp,
})

wasPrepareCheckStateCalled := false
app.SetPrepareCheckState(func(ctx sdk.Context) {
wasPrepareCheckStateCalled = true
})
app.Seal()

app.Commit()
require.Equal(t, true, wasPrepareCheckStateCalled)
}

func TestBaseApp_Precommit(t *testing.T) {
db := dbm.NewMemDB()
name := t.Name()
logger := log.NewTestLogger(t)

cp := &cmtproto.ConsensusParams{
Block: &cmtproto.BlockParams{
MaxGas: 5000000,
},
}

app := baseapp.NewBaseApp(name, logger, db, nil)
app.SetParamStore(&paramStore{db: dbm.NewMemDB()})
app.InitChain(abci.RequestInitChain{
ConsensusParams: cp,
})

wasPrecommiterCalled := false
app.SetPrecommiter(func(ctx sdk.Context) {
wasPrecommiterCalled = true
})
app.Seal()

app.Commit()
require.Equal(t, true, wasPrecommiterCalled)
}

func TestABCI_CheckTx(t *testing.T) {
// This ante handler reads the key and checks that the value matches the
// current counter. This ensures changes to the KVStore persist across
Expand Down Expand Up @@ -1322,6 +1376,49 @@ func TestABCI_GetBlockRetentionHeight(t *testing.T) {
}
}

// Verifies that PrepareCheckState is called with the checkState.
func TestPrepareCheckStateCalledWithCheckState(t *testing.T) {
t.Parallel()

logger := log.NewTestLogger(t)
db := dbm.NewMemDB()
name := t.Name()
app := baseapp.NewBaseApp(name, logger, db, nil)

wasPrepareCheckStateCalled := false
app.SetPrepareCheckState(func(ctx sdk.Context) {
require.Equal(t, true, ctx.IsCheckTx())
wasPrepareCheckStateCalled = true
})

app.BeginBlock(abci.RequestBeginBlock{Header: cmtproto.Header{Height: 1}})
app.Commit()

require.Equal(t, true, wasPrepareCheckStateCalled)
}

// Verifies that the Precommiter is called with the deliverState.
func TestPrecommiterCalledWithDeliverState(t *testing.T) {
t.Parallel()

logger := log.NewTestLogger(t)
db := dbm.NewMemDB()
name := t.Name()
app := baseapp.NewBaseApp(name, logger, db, nil)

wasPrecommiterCalled := false
app.SetPrecommiter(func(ctx sdk.Context) {
require.Equal(t, false, ctx.IsCheckTx())
require.Equal(t, false, ctx.IsReCheckTx())
wasPrecommiterCalled = true
})

app.BeginBlock(abci.RequestBeginBlock{Header: cmtproto.Header{Height: 1}})
app.Commit()

require.Equal(t, true, wasPrecommiterCalled)
}

func TestABCI_Proposal_HappyPath(t *testing.T) {
anteKey := []byte("ante-key")
pool := mempool.NewSenderNonceMempool()
Expand Down
24 changes: 13 additions & 11 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,19 @@ type BaseApp struct { //nolint: maligned
txDecoder sdk.TxDecoder // unmarshal []byte into sdk.Tx
txEncoder sdk.TxEncoder // marshal sdk.Tx into []byte

mempool mempool.Mempool // application side mempool
anteHandler sdk.AnteHandler // ante handler for fee and auth
postHandler sdk.PostHandler // post handler, optional, e.g. for tips
initChainer sdk.InitChainer // initialize state with validators and state blob
beginBlocker sdk.BeginBlocker // logic to run before any txs
processProposal sdk.ProcessProposalHandler // the handler which runs on ABCI ProcessProposal
prepareProposal sdk.PrepareProposalHandler // the handler which runs on ABCI PrepareProposal
endBlocker sdk.EndBlocker // logic to run after all txs, and to determine valset changes
addrPeerFilter sdk.PeerFilter // filter peers by address and port
idPeerFilter sdk.PeerFilter // filter peers by node ID
fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed.
mempool mempool.Mempool // application side mempool
anteHandler sdk.AnteHandler // ante handler for fee and auth
postHandler sdk.PostHandler // post handler, optional, e.g. for tips
initChainer sdk.InitChainer // initialize state with validators and state blob
beginBlocker sdk.BeginBlocker // logic to run before any txs
processProposal sdk.ProcessProposalHandler // the handler which runs on ABCI ProcessProposal
prepareProposal sdk.PrepareProposalHandler // the handler which runs on ABCI PrepareProposal
endBlocker sdk.EndBlocker // logic to run after all txs, and to determine valset changes
prepareCheckStater sdk.PrepareCheckStater // logic to run during commit using the checkState
precommiter sdk.Precommiter // logic to run during commit using the deliverState
addrPeerFilter sdk.PeerFilter // filter peers by address and port
idPeerFilter sdk.PeerFilter // filter peers by node ID
fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed.

// manages snapshots, i.e. dumps of app state at certain intervals
snapshotManager *snapshots.Manager
Expand Down
6 changes: 6 additions & 0 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,12 @@ func TestBaseAppOptionSeal(t *testing.T) {
require.Panics(t, func() {
suite.baseApp.SetEndBlocker(nil)
})
require.Panics(t, func() {
suite.baseApp.SetPrepareCheckState(nil)
})
require.Panics(t, func() {
suite.baseApp.SetPrecommiter(nil)
})
require.Panics(t, func() {
suite.baseApp.SetAnteHandler(nil)
})
Expand Down
16 changes: 16 additions & 0 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,22 @@ func (app *BaseApp) SetEndBlocker(endBlocker sdk.EndBlocker) {
app.endBlocker = endBlocker
}

func (app *BaseApp) SetPrepareCheckState(prepareCheckStater sdk.PrepareCheckStater) {
if app.sealed {
panic("SetPrepareCheckState() on sealed BaseApp")
}

app.prepareCheckStater = prepareCheckStater
}

func (app *BaseApp) SetPrecommiter(precommiter sdk.Precommiter) {
if app.sealed {
panic("SetPrecommiter() on sealed BaseApp")
}

app.precommiter = precommiter
}

prettymuchbryce marked this conversation as resolved.
Show resolved Hide resolved
func (app *BaseApp) SetAnteHandler(ah sdk.AnteHandler) {
if app.sealed {
panic("SetAnteHandler() on sealed BaseApp")
Expand Down
18 changes: 18 additions & 0 deletions docs/docs/building-modules/01-module-manager.md
prettymuchbryce marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ The above interfaces are mostly embedding smaller interfaces (extension interfac
* [`HasConsensusVersion`](#hasconsensusversion): The extension interface for declaring a module consensus version.
* [`BeginBlockAppModule`](#beginblockappmodule): The extension interface that contains information about the `AppModule` and `BeginBlock`.
* [`EndBlockAppModule`](#endblockappmodule): The extension interface that contains information about the `AppModule` and `EndBlock`.
* [`PrecommitAppModule`](#precommitappmodule): The extension interface that contains information about the `AppModule` and `Precommit`.
* [`PrepareCheckStateAppModule`](#preparecheckstateappmodule): The extension interface that contains information about the `AppModule` and `PrepareCheckState`.

The `AppModuleBasic` interface exists to define independent methods of the module, i.e. those that do not depend on other modules in the application. This allows for the construction of the basic application structure early in the application definition, generally in the `init()` function of the [main application file](../basics/00-app-anatomy.md#core-application-file).

Expand Down Expand Up @@ -167,6 +169,18 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/types/module/module.go#L20

* `EndBlock(sdk.Context, abci.RequestEndBlock)`: This method gives module developers the option to implement logic that is automatically triggered at the end of each block. This is also where the module can inform the underlying consensus engine of validator set changes (e.g. the `staking` module). Implement empty if no logic needs to be triggered at the end of each block for this module.

### `PrecommitAppModule`

The `PrecommitAppModule` is an extension interface from `AppModule`. All modules that have a `Precommit` method implement this interface.

* `Precommit(sdk.Context)`: This method gives module developers the option to implement logic that is automatically triggered during [`Commit'](../core/00-baseapp.md#commit) of each block using the [`deliverState`](../core/00-baseapp.md#state-updates) of the block to be committed. Implement empty if no logic needs to be triggered during `Commit` of each block for this module.

### `PrepareCheckStateAppModule`

The `PrepareCheckStateAppModule` is an extension interface from `AppModule`. All modules that have a `PrepareCheckState` method implement this interface.

* `PrepareCheckState(sdk.Context)`: This method gives module developers the option to implement logic that is automatically triggered during [`Commit'](../core/00-baseapp.md#commit) of each block using the [`checkState`](../core/00-baseapp.md#state-updates) of the next block. Implement empty if no logic needs to be triggered during `Commit` of each block for this module.

### Implementing the Application Module Interfaces

Typically, the various application module interfaces are implemented in a file called `module.go`, located in the module's folder (e.g. `./x/module/module.go`).
Expand Down Expand Up @@ -228,6 +242,8 @@ The module manager is used throughout the application whenever an action on a co
* `SetOrderExportGenesis(moduleNames ...string)`: Sets the order in which the [`ExportGenesis`](./08-genesis.md#exportgenesis) function of each module will be called in case of an export. This function is generally called from the application's main [constructor function](../basics/00-app-anatomy.md#constructor-function).
* `SetOrderBeginBlockers(moduleNames ...string)`: Sets the order in which the `BeginBlock()` function of each module will be called at the beginning of each block. This function is generally called from the application's main [constructor function](../basics/00-app-anatomy.md#constructor-function).
* `SetOrderEndBlockers(moduleNames ...string)`: Sets the order in which the `EndBlock()` function of each module will be called at the end of each block. This function is generally called from the application's main [constructor function](../basics/00-app-anatomy.md#constructor-function).
* `SetOrderPrecommiters(moduleNames ...string)`: Sets the order in which the `Precommit()` function of each module will be called during commit of each block. This function is generally called from the application's main [constructor function](../basics/00-app-anatomy.md#constructor-function).
* `SetOrderPrepareCheckStaters(moduleNames ...string)`: Sets the order in which the `PrepareCheckState()` function of each module will be called during commit of each block. This function is generally called from the application's main [constructor function](../basics/00-app-anatomy.md#constructor-function).
* `SetOrderMigrations(moduleNames ...string)`: Sets the order of migrations to be run. If not set then migrations will be run with an order defined in `DefaultMigrationsOrder`.
* `RegisterInvariants(ir sdk.InvariantRegistry)`: Registers the [invariants](./07-invariants.md) of module implementing the `HasInvariants` interface.
* `RegisterRoutes(router sdk.Router, queryRouter sdk.QueryRouter, legacyQuerierCdc *codec.LegacyAmino)`: Registers legacy [`Msg`](./02-messages-and-queries.md#messages) and [`querier`](./04-query-services.md#legacy-queriers) routes.
Expand All @@ -237,6 +253,8 @@ The module manager is used throughout the application whenever an action on a co
* `ExportGenesisForModules(ctx sdk.Context, cdc codec.JSONCodec, modulesToExport []string)`: Behaves the same as `ExportGenesis`, except takes a list of modules to export.
* `BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock)`: At the beginning of each block, this function is called from [`BaseApp`](../core/00-baseapp.md#beginblock) and, in turn, calls the [`BeginBlock`](./05-beginblock-endblock.md) function of each modules implementing the `BeginBlockAppModule` interface, in the order defined in `OrderBeginBlockers`. It creates a child [context](../core/02-context.md) with an event manager to aggregate [events](../core/08-events.md) emitted from all modules. The function returns an `abci.ResponseBeginBlock` which contains the aforementioned events.
* `EndBlock(ctx sdk.Context, req abci.RequestEndBlock)`: At the end of each block, this function is called from [`BaseApp`](../core/00-baseapp.md#endblock) and, in turn, calls the [`EndBlock`](./05-beginblock-endblock.md) function of each modules implementing the `EndBlockAppModule` interface, in the order defined in `OrderEndBlockers`. It creates a child [context](../core/02-context.md) with an event manager to aggregate [events](../core/08-events.md) emitted from all modules. The function returns an `abci.ResponseEndBlock` which contains the aforementioned events, as well as validator set updates (if any).
* `Precommit(ctx sdk.Context)`: During [`Commit`](../core/00-baseapp.md#commit), this function is called from `BaseApp` immediately before the [`deliverState`](../core/00-baseapp.md#state-updates) is written to the underlying [`rootMultiStore`](../core/04-store.md#commitmultistore) and, in turn calls the `Precommit` function of each modules implementing the `PrecommitAppModule` interface, in the order defined in `OrderPrecommiters`. It creates a child [context](../core/02-context.md) where the underlying `CacheMultiStore` is that of the newly committed block's [`deliverState`](../core/00-baseapp.md#state-updates).
* `PrepareCheckState(ctx sdk.Context)`: During [`Commit`](../core/00-baseapp.md#commit), this function is called from `BaseApp` immediately after the [`deliverState`](../core/00-baseapp.md#state-updates) is written to the underlying [`rootMultiStore`](../core/04-store.md#commitmultistore) and, in turn calls the `PrepareCheckState` function of each module implementing the `PrepareCheckStateAppModule` interface, in the order defined in `OrderPrepareCheckStaters`. It creates a child [context](../core/02-context.md) where the underlying `CacheMultiStore` is that of the next block's [`checkState`](../core/00-baseapp.md#state-updates). Writes to this state will be present in the [`checkState`](../core/00-baseapp.md#state-updates) of the next block, and therefore this method can be used to prepare the `checkState` for the next block.

Here's an example of a concrete integration within an `simapp`:

Expand Down
8 changes: 5 additions & 3 deletions telemetry/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (

// Common metric key constants
const (
MetricKeyBeginBlocker = "begin_blocker"
MetricKeyEndBlocker = "end_blocker"
MetricLabelNameModule = "module"
MetricKeyBeginBlocker = "begin_blocker"
MetricKeyEndBlocker = "end_blocker"
MetricKeyPrepareCheckStater = "prepare_check_stater"
MetricKeyPrecommiter = "precommiter"
MetricLabelNameModule = "module"
)

// NewLabel creates a new instance of Label with name and value
Expand Down
Loading