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 4 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
8 changes: 8 additions & 0 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,17 @@ func (app *BaseApp) Commit() abci.ResponseCommit {
// Commit. Use the header from this latest block.
app.setState(runTxModeCheck, header)

if app.precommiter != nil {
app.precommiter(app.deliverState.ctx)
}

// empty/reset the deliver state
app.deliverState = nil

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:452)

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

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:426)

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_Commit(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,
})

wasCommiterCalled := false
app.SetCommiter(func(ctx sdk.Context) {
wasCommiterCalled = true
})
app.Seal()

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

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 the Commiter is called with the checkState.
func TestCommiterCalledWithCheckState(t *testing.T) {
t.Parallel()

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

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

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

require.Equal(t, true, wasCommiterCalled)
}

// 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
2 changes: 2 additions & 0 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ type BaseApp struct { //nolint: maligned
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
commiter sdk.Commiter // logic to run during commit
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.
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.SetCommiter(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) SetCommiter(commiter sdk.Commiter) {
if app.sealed {
panic("SetCommiter() on sealed BaseApp")
}

app.commiter = commiter
}

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

app.precommiter = precommiter
}

func (app *BaseApp) SetAnteHandler(ah sdk.AnteHandler) {
if app.sealed {
panic("SetAnteHandler() on sealed BaseApp")
Expand Down
1 change: 1 addition & 0 deletions telemetry/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
const (
MetricKeyBeginBlocker = "begin_blocker"
MetricKeyEndBlocker = "end_blocker"
MetricKeyCommiter = "commiter"
prettymuchbryce marked this conversation as resolved.
Show resolved Hide resolved
MetricLabelNameModule = "module"
)

Expand Down
Loading