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(baseapp): Reset GasMeter before deliverTX #18714

Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

* (baseapp) [#18609](https://github.com/cosmos/cosmos-sdk/issues/18609) Gas Consumption idiosyncracies in BeginBlock / DeliverTx
Eric-Warehime marked this conversation as resolved.
Show resolved Hide resolved
* (client) [#18622](https://github.com/cosmos/cosmos-sdk/pull/18622) Fixed a potential under/overflow from `uint64->int64` when computing gas fees as a LegacyDec.
* (client/keys) [#18562](https://github.com/cosmos/cosmos-sdk/pull/18562) `keys delete` won't terminate when a key is not found.
* (baseapp) [#18383](https://github.com/cosmos/cosmos-sdk/pull/18383) Fixed a data race inside BaseApp.getContext, found by end-to-end (e2e) tests.
Expand Down
5 changes: 5 additions & 0 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,11 @@ func (app *BaseApp) internalFinalizeBlock(ctx context.Context, req *abci.Request

events = append(events, beginBlock.Events...)

// Reset the gas meter so that the AnteHandlers aren't required to
gasMeter = app.getBlockGasMeter(app.finalizeBlockState.ctx)
app.finalizeBlockState.ctx = app.finalizeBlockState.ctx.WithBlockGasMeter(gasMeter)
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

app.checkState.ctx = app.checkState.ctx.WithBlockGasMeter(gasMeter)
Eric-Warehime marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, why are we setting CheckTx state here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call out, there's no need to modify the checkTx state's meter here.


// Iterate over all raw transactions in the proposal and attempt to execute
// them, gathering the execution results.
//
Expand Down
35 changes: 35 additions & 0 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,41 @@ func NewBaseAppSuiteWithSnapshots(t *testing.T, cfg SnapshotsConfig, opts ...fun
return suite
}

func TestAnteHandlerGasMeter(t *testing.T) {
// run BeginBlock and assert that the gas meter passed into the first Txn is zeroed out
anteOpt := func(bapp *baseapp.BaseApp) {
bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) {
gasMeter := ctx.BlockGasMeter()
require.NotNil(t, gasMeter)
require.Equal(t, storetypes.Gas(0), gasMeter.GasConsumed())
return ctx, nil
})
}
// set the beginBlocker to use some gas
beginBlockerOpt := func(bapp *baseapp.BaseApp) {
bapp.SetBeginBlocker(func(ctx sdk.Context) (sdk.BeginBlock, error) {
ctx.BlockGasMeter().ConsumeGas(1, "beginBlocker gas consumption")
return sdk.BeginBlock{}, nil
})
}

suite := NewBaseAppSuite(t, anteOpt, beginBlockerOpt)
_, err := suite.baseApp.InitChain(&abci.RequestInitChain{
ConsensusParams: &cmtproto.ConsensusParams{},
})
require.NoError(t, err)

deliverKey := []byte("deliver-key")
baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), CounterServerImpl{t, capKey1, deliverKey})

tx := newTxCounter(t, suite.txConfig, 0, 0)
txBytes, err := suite.txConfig.TxEncoder()(tx)
require.NoError(t, err)
_, err = suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{Height: 1, Txs: [][]byte{txBytes}})
require.NoError(t, err)

}

func TestLoadVersion(t *testing.T) {
logger := log.NewTestLogger(t)
pruningOpt := baseapp.SetPruning(pruningtypes.NewPruningOptions(pruningtypes.PruningNothing))
Expand Down
Loading