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

fix: set block gas meter on prepare/process proposal #15012

Merged
merged 14 commits into from
Feb 15, 2023
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ clean:
rm -rf \
$(BUILDDIR)/ \
artifacts/ \
tmp-swagger-gen/
tmp-swagger-gen/ \
.testnets

.PHONY: distclean clean

Expand Down
19 changes: 8 additions & 11 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,7 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
WithBlockHeight(req.Header.Height)
}

// add block gas meter
var gasMeter storetypes.GasMeter
if maxGas := app.GetMaximumBlockGas(app.deliverState.ctx); maxGas > 0 {
gasMeter = storetypes.NewGasMeter(maxGas)
} else {
gasMeter = storetypes.NewInfiniteGasMeter()
}

// NOTE: header hash is not set in NewContext, so we manually set it here
gasMeter := app.getBlockGasMeter(app.deliverState.ctx)

app.deliverState.ctx = app.deliverState.ctx.
WithBlockGasMeter(gasMeter).
Expand Down Expand Up @@ -271,13 +263,15 @@ func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) (resp abci.
panic("PrepareProposal called with invalid height")
}

gasMeter := app.getBlockGasMeter(app.prepareProposalState.ctx)
ctx := app.getContextForProposal(app.prepareProposalState.ctx, req.Height)

ctx = ctx.WithVoteInfos(app.voteInfos).
WithBlockHeight(req.Height).
WithBlockTime(req.Time).
WithProposer(req.ProposerAddress).
WithConsensusParams(app.GetConsensusParams(ctx))
WithConsensusParams(app.GetConsensusParams(ctx)).
WithBlockGasMeter(gasMeter)

defer func() {
if err := recover(); err != nil {
Expand All @@ -287,6 +281,7 @@ func (app *BaseApp) PrepareProposal(req abci.RequestPrepareProposal) (resp abci.
"time", req.Time,
"panic", err,
)

resp = abci.ResponsePrepareProposal{Txs: req.Txs}
}
}()
Expand Down Expand Up @@ -315,6 +310,7 @@ func (app *BaseApp) ProcessProposal(req abci.RequestProcessProposal) (resp abci.
panic("app.ProcessProposal is not set")
}

gasMeter := app.getBlockGasMeter(app.processProposalState.ctx)
ctx := app.getContextForProposal(app.processProposalState.ctx, req.Height)

ctx = ctx.
Expand All @@ -323,7 +319,8 @@ func (app *BaseApp) ProcessProposal(req abci.RequestProcessProposal) (resp abci.
WithBlockTime(req.Time).
WithHeaderHash(req.Hash).
WithProposer(req.ProposerAddress).
WithConsensusParams(app.GetConsensusParams(ctx))
WithConsensusParams(app.GetConsensusParams(ctx)).
WithBlockGasMeter(gasMeter)

defer func() {
if err := recover(); err != nil {
Expand Down
26 changes: 19 additions & 7 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,15 +543,26 @@ func (app *BaseApp) getState(mode runTxMode) *state {
switch mode {
case runTxModeDeliver:
return app.deliverState

case runTxPrepareProposal:
return app.prepareProposalState

case runTxProcessProposal:
return app.processProposalState

default:
return app.checkState
}
}

func (app *BaseApp) getBlockGasMeter(ctx sdk.Context) storetypes.GasMeter {
if maxGas := app.GetMaximumBlockGas(ctx); maxGas > 0 {
return storetypes.NewGasMeter(maxGas)
}

return storetypes.NewInfiniteGasMeter()
}

// retrieve the context for the tx w/ txBytes and other memoized values.
func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) sdk.Context {
modeState := app.getState(mode)
Expand Down Expand Up @@ -625,8 +636,10 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re
}()

blockGasConsumed := false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This files contains cosmetic changes only FYI.

// consumeBlockGas makes sure block gas is consumed at most once. It must happen after
// tx processing, and must be executed even if tx processing fails. Hence, we use trick with `defer`

// consumeBlockGas makes sure block gas is consumed at most once. It must
// happen after tx processing, and must be executed even if tx processing
// fails. Hence, it's execution is deferred.
consumeBlockGas := func() {
if !blockGasConsumed {
Comment on lines 636 to 644
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).runTx (baseapp/baseapp.go:604)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).DeliverTx (baseapp/baseapp.go:385)

blockGasConsumed = true
Expand All @@ -639,8 +652,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re
// If BlockGasMeter() panics it will be caught by the above recover and will
// return an error - in any case BlockGasMeter will consume gas past the limit.
//
// NOTE: This must exist in a separate defer function for the above recovery
// to recover from this one.
// NOTE: consumeBlockGas must exist in a separate defer function from the
// general deferred recovery function to recover from consumeBlockGas as it'll
// be executed first (deferred statements are executed as stack).
if mode == runTxModeDeliver {
defer consumeBlockGas()
}
Expand Down Expand Up @@ -718,9 +732,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re
// and we're in DeliverTx. Note, runMsgs will never return a reference to a
// Result if any single message fails or does not have a registered Handler.
result, err = app.runMsgs(runMsgCtx, msgs, mode)

if err == nil {

// Run optional postHandlers.
//
// Note: If the postHandler fails, we also revert the runMsgs state.
Expand Down Expand Up @@ -766,7 +778,6 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s

// NOTE: GasWanted is determined by the AnteHandler and GasUsed by the GasMeter.
for i, msg := range msgs {

if mode != runTxModeDeliver && mode != runTxModeSimulate {
break
}
Expand Down Expand Up @@ -936,6 +947,7 @@ func (app *BaseApp) DefaultProcessProposal() sdk.ProcessProposalHandler {
return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT}
}
}

return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_ACCEPT}
}
}
Expand Down
3 changes: 2 additions & 1 deletion store/types/gas.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ func (g *basicGasMeter) Limit() Gas {

// GasConsumedToLimit returns the gas limit if gas consumed is past the limit,
// otherwise it returns the consumed gas.
// NOTE: This behaviour is only called when recovering from panic when
//
// NOTE: This behavior is only called when recovering from panic when
// BlockGasMeter consumes gas past the limit.
func (g *basicGasMeter) GasConsumedToLimit() Gas {
if g.IsPastLimit() {
Expand Down