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.

Sign up for GitHub

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

refactor: Baseapp audit changes #14379

Merged
merged 13 commits into from
Dec 22, 2022
Merged

refactor: Baseapp audit changes #14379

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1e6e8fd
merge utils_test.go and util_test.go
facundomedica Dec 21, 2022
6999e96
fix getState comment
facundomedica Dec 21, 2022
ce138d6
remove parallel block execution comment. in the spec it says it will …
facundomedica Dec 21, 2022
fd0b844
factorize setState functions (from Amaury's notes)
facundomedica Dec 21, 2022
306c596
Revert "merge utils_test.go and util_test.go"
facundomedica Dec 21, 2022
afc2c51
Revert "Revert "merge utils_test.go and util_test.go""
facundomedica Dec 21, 2022
ed5eb8a
Revert "factorize setState functions (from Amaury's notes)"
facundomedica Dec 21, 2022
56e4e6a
Revert "Revert "factorize setState functions (from Amaury's notes)""
facundomedica Dec 21, 2022
c227080
found issue in setState
facundomedica Dec 21, 2022
1ac40fe
Merge branch 'main' into facu/baseapp-audit
alexanderbez Dec 21, 2022
962a584
Merge branch 'main' into facu/baseapp-audit
alexanderbez Dec 21, 2022
2ce1297
Merge branch 'main' into facu/baseapp-audit
facundomedica Dec 21, 2022
8cc6bce
Merge branch 'main' into facu/baseapp-audit
facundomedica Dec 22, 2022
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
18 changes: 9 additions & 9 deletions baseapp/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC
}

// initialize states with a correct header
app.setDeliverState(initHeader)
app.setCheckState(initHeader)
app.setPrepareProposalState(initHeader)
app.setProcessProposalState(initHeader)
app.setState(runTxModeDeliver, initHeader)
app.setState(runTxModeCheck, initHeader)
app.setState(runTxPrepareProposal, initHeader)
app.setState(runTxProcessProposal, initHeader)

// Store the consensus params in the BaseApp's paramstore. Note, this must be
// done after the deliver state and context have been set as it's persisted
Expand Down Expand Up @@ -162,7 +162,7 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
// already be initialized in InitChain. Otherwise app.deliverState will be
// nil, since it is reset on Commit.
if app.deliverState == nil {
app.setDeliverState(req.Header)
app.setState(runTxModeDeliver, req.Header)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
} else {
// In the first block, app.deliverState.ctx will already be initialized
// by InitChain. Context is now updated with Header information.
Expand Down Expand Up @@ -277,7 +277,7 @@ func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) (resp abci.
// that all transactions are valid. If all transactions are valid, then we inform
// Tendermint that the Status is ACCEPT. However, the application is also able
// to implement optimizations such as executing the entire proposed block
// immediately. It may even execute the block in parallel.
// immediately.
//
// If a panic is detected during execution of an application's ProcessProposal
// handler, it will be recovered and we will reject the proposal.
Expand Down Expand Up @@ -423,9 +423,9 @@ func (app *BaseApp) Commit() abci.ResponseCommit {
//
// NOTE: This is safe because Tendermint holds a lock on the mempool for
// Commit. Use the header from this latest block.
app.setCheckState(header)
app.setPrepareProposalState(header)
app.setProcessProposalState(header)
app.setState(runTxModeCheck, header)
app.setState(runTxPrepareProposal, header)
app.setState(runTxProcessProposal, header)

// empty/reset the deliver state
app.deliverState = nil
Expand Down
68 changes: 26 additions & 42 deletions baseapp/baseapp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,11 @@ func (app *BaseApp) Init() error {
emptyHeader := tmproto.Header{}

// needed for the export command which inits from store but never calls initchain
app.setCheckState(emptyHeader)
app.setState(runTxModeCheck, emptyHeader)

// needed for ABCI Replay Blocks mode which calls Prepare/Process proposal (InitChain is not called)
app.setPrepareProposalState(emptyHeader)
app.setProcessProposalState(emptyHeader)
app.setState(runTxPrepareProposal, emptyHeader)
app.setState(runTxProcessProposal, emptyHeader)
app.Seal()

if app.cms == nil {
Expand Down Expand Up @@ -402,49 +402,32 @@ func (app *BaseApp) Seal() { app.sealed = true }
// IsSealed returns true if the BaseApp is sealed and false otherwise.
func (app *BaseApp) IsSealed() bool { return app.sealed }

// setCheckState sets the BaseApp's checkState with a branched multi-store
// (i.e. a CacheMultiStore) and a new Context with the same multi-store branch,
// provided header, and minimum gas prices set. It is set on InitChain and reset
// on Commit.
func (app *BaseApp) setCheckState(header tmproto.Header) {
// setState sets the BaseApp's state for the corresponding mode with a branched
// multi-store (i.e. a CacheMultiStore) and a new Context with the same
// multi-store branch, and provided header.
func (app *BaseApp) setState(mode runTxMode, header tmproto.Header) {
ms := app.cms.CacheMultiStore()
app.checkState = &state{
ms: ms,
ctx: sdk.NewContext(ms, header, true, app.logger).WithMinGasPrices(app.minGasPrices),
}
}

// setDeliverState sets the BaseApp's deliverState with a branched multi-store
// (i.e. a CacheMultiStore) and a new Context with the same multi-store branch,
// and provided header. It is set on InitChain and BeginBlock and set to nil on
// Commit.
func (app *BaseApp) setDeliverState(header tmproto.Header) {
ms := app.cms.CacheMultiStore()
app.deliverState = &state{
ms: ms,
ctx: sdk.NewContext(ms, header, false, app.logger),
}
}

// setPrepareProposalState sets the BaseApp's prepareProposalState with a
// branched multi-store (i.e. a CacheMultiStore) and a new Context with the
// same multi-store branch, and provided header. It is set on InitChain and Commit.
func (app *BaseApp) setPrepareProposalState(header tmproto.Header) {
ms := app.cms.CacheMultiStore()
app.prepareProposalState = &state{
baseState := &state{
ms: ms,
ctx: sdk.NewContext(ms, header, false, app.logger),
}
}

// setProcessProposalState sets the BaseApp's processProposalState with a
// branched multi-store (i.e. a CacheMultiStore) and a new Context with the
// same multi-store branch, and provided header. It is set on InitChain and Commit.
func (app *BaseApp) setProcessProposalState(header tmproto.Header) {
ms := app.cms.CacheMultiStore()
app.processProposalState = &state{
ms: ms,
ctx: sdk.NewContext(ms, header, false, app.logger),
switch mode {
case runTxModeCheck:
// Minimum gas prices are also set. It is set on InitChain and reset on Commit.
baseState.ctx = baseState.ctx.WithIsCheckTx(true).WithMinGasPrices(app.minGasPrices)
app.checkState = baseState
case runTxModeDeliver:
// It is set on InitChain and BeginBlock and set to nil on Commit.
app.deliverState = baseState
case runTxPrepareProposal:
// It is set on InitChain and Commit.
app.prepareProposalState = baseState
case runTxProcessProposal:
// It is set on InitChain and Commit.
app.processProposalState = baseState
default:
panic(fmt.Sprintf("invalid runTxMode for setState: %d", mode))
}
}

Expand Down Expand Up @@ -552,7 +535,8 @@ func validateBasicTxMsgs(msgs []sdk.Msg) error {
}

// Returns the application's deliverState if app is in runTxModeDeliver,
// otherwise it returns the application's checkstate.
// prepareProposalState if app is in runTxPrepareProposal, processProposalState
// if app is in runTxProcessProposal, and checkState otherwise.
func (app *BaseApp) getState(mode runTxMode) *state {
switch mode {
case runTxModeDeliver:
Comment on lines 535 to 542
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.validateBasicTxMsgs (baseapp/baseapp.go:539)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).runTx (baseapp/baseapp.go:618)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).DeliverTx (baseapp/baseapp.go:326)

Expand Down
166 changes: 0 additions & 166 deletions baseapp/util_test.go
View file
Open in desktop

This file was deleted.

Loading