Skip to content

Commit

Permalink
fix: revert tx when block gas limit exceeded (backport: #10770) (#10814)
Browse files Browse the repository at this point in the history
* revert tx when block gas limit exceeded (backport: #10770)

- used a different solution.

changelog

* add unit test

* Update baseapp/baseapp.go

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* simplfy the defer function

Co-authored-by: Robert Zaremba <robert@zaremba.ch>
  • Loading branch information
yihuang and robert-zaremba authored Jan 12, 2022
1 parent a5c60b7 commit ba1e099
Show file tree
Hide file tree
Showing 4 changed files with 223 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (store) [#10218](https://github.com/cosmos/cosmos-sdk/pull/10218) Charge gas even when there are no entries while seeking.
* (store) [#10247](https://github.com/cosmos/cosmos-sdk/pull/10247) Charge gas for the key length in gas meter.
* (x/gov) [\#10740](https://github.com/cosmos/cosmos-sdk/pull/10740) Increase maximum proposal description size from 5k characters to 10k characters.
* [#10814](https://github.com/cosmos/cosmos-sdk/pull/10814) revert tx when block gas limit exceeded.

### API Breaking Changes

Expand Down
34 changes: 18 additions & 16 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,11 +586,6 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re
return gInfo, nil, nil, sdkerrors.Wrap(sdkerrors.ErrOutOfGas, "no block gas left to run tx")
}

var startingGas uint64
if mode == runTxModeDeliver {
startingGas = ctx.BlockGasMeter().GasConsumed()
}

defer func() {
if r := recover(); r != nil {
recoveryMW := newOutOfGasRecoveryMiddleware(gasWanted, ctx, app.runTxRecoveryMiddleware)
Expand All @@ -600,22 +595,26 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re
gInfo = sdk.GasInfo{GasWanted: gasWanted, GasUsed: ctx.GasMeter().GasConsumed()}
}()

blockGasConsumed := false
// consumeBlockGas makes sure block gas is consumed at most once. It must happen after
// tx processing, and must be execute even if tx processing fails. Hence we use trick with `defer`
consumeBlockGas := func() {
if !blockGasConsumed {
blockGasConsumed = true
ctx.BlockGasMeter().ConsumeGas(
ctx.GasMeter().GasConsumedToLimit(), "block gas meter",
)
}
}

// 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.
defer func() {
if mode == runTxModeDeliver {
ctx.BlockGasMeter().ConsumeGas(
ctx.GasMeter().GasConsumedToLimit(), "block gas meter",
)

if ctx.BlockGasMeter().GasConsumed() < startingGas {
panic(sdk.ErrorGasOverflow{Descriptor: "tx gas summation"})
}
}
}()
if mode == runTxModeDeliver {
defer consumeBlockGas()
}

tx, err := app.txDecoder(txBytes)
if err != nil {
Expand Down Expand Up @@ -677,6 +676,9 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte) (gInfo sdk.GasInfo, re
// Result if any single message fails or does not have a registered Handler.
result, err = app.runMsgs(runMsgCtx, msgs, mode)
if err == nil && mode == runTxModeDeliver {
// When block gas exceeds, it'll panic and won't commit the cached store.
consumeBlockGas()

msCache.Write()

if len(anteEvents) > 0 {
Expand Down
200 changes: 200 additions & 0 deletions baseapp/block_gas_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
package baseapp_test

import (
"encoding/json"
"fmt"
"math"
"testing"

"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/log"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
dbm "github.com/tendermint/tm-db"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/tx"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
txtypes "github.com/cosmos/cosmos-sdk/types/tx"
"github.com/cosmos/cosmos-sdk/types/tx/signing"
xauthsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
)

var blockMaxGas = uint64(simapp.DefaultConsensusParams.Block.MaxGas)

func TestBaseApp_BlockGas(t *testing.T) {
testcases := []struct {
name string
gasToConsume uint64 // gas to consume in the msg execution
panicTx bool // panic explicitly in tx execution
expErr bool
}{
{"less than block gas meter", 10, false, false},
{"more than block gas meter", blockMaxGas, false, true},
{"more than block gas meter", uint64(float64(blockMaxGas) * 1.2), false, true},
{"consume MaxUint64", math.MaxUint64, false, true},
{"consume MaxGasWanted", txtypes.MaxGasWanted, false, true},
{"consume block gas when paniced", 10, true, true},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
var app *simapp.SimApp
routerOpt := func(bapp *baseapp.BaseApp) {
route := (&testdata.TestMsg{}).Route()
bapp.Router().AddRoute(sdk.NewRoute(route, func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
_, ok := msg.(*testdata.TestMsg)
if !ok {
return &sdk.Result{}, fmt.Errorf("Wrong Msg type, expected %T, got %T", (*testdata.TestMsg)(nil), msg)
}
ctx.KVStore(app.GetKey(banktypes.ModuleName)).Set([]byte("ok"), []byte("ok"))
ctx.GasMeter().ConsumeGas(tc.gasToConsume, "TestMsg")
if tc.panicTx {
panic("panic in tx execution")
}
return &sdk.Result{}, nil
}))
}
encCfg := simapp.MakeTestEncodingConfig()
encCfg.Amino.RegisterConcrete(&testdata.TestMsg{}, "testdata.TestMsg", nil)
encCfg.InterfaceRegistry.RegisterImplementations((*sdk.Msg)(nil),
&testdata.TestMsg{},
)
app = simapp.NewSimApp(log.NewNopLogger(), dbm.NewMemDB(), nil, true, map[int64]bool{}, "", 0, encCfg, simapp.EmptyAppOptions{}, routerOpt)
genState := simapp.NewDefaultGenesisState(encCfg.Marshaler)
stateBytes, err := json.MarshalIndent(genState, "", " ")
require.NoError(t, err)
app.InitChain(abci.RequestInitChain{
Validators: []abci.ValidatorUpdate{},
ConsensusParams: simapp.DefaultConsensusParams,
AppStateBytes: stateBytes,
})

ctx := app.NewContext(false, tmproto.Header{})

// tx fee
feeCoin := sdk.NewCoin("atom", sdk.NewInt(150))
feeAmount := sdk.NewCoins(feeCoin)

// test account and fund
priv1, _, addr1 := testdata.KeyTestPubAddr()
err = app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, feeAmount)
require.NoError(t, err)
err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr1, feeAmount)
require.NoError(t, err)
require.Equal(t, feeCoin.Amount, app.BankKeeper.GetBalance(ctx, addr1, feeCoin.Denom).Amount)
seq, _ := app.AccountKeeper.GetSequence(ctx, addr1)
require.Equal(t, uint64(0), seq)

// msg and signatures
msg := testdata.NewTestMsg(addr1)

txBuilder := encCfg.TxConfig.NewTxBuilder()
require.NoError(t, txBuilder.SetMsgs(msg))
txBuilder.SetFeeAmount(feeAmount)
txBuilder.SetGasLimit(txtypes.MaxGasWanted) // tx validation checks that gasLimit can't be bigger than this

privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{6}, []uint64{0}
_, txBytes, err := createTestTx(encCfg.TxConfig, txBuilder, privs, accNums, accSeqs, ctx.ChainID())
require.NoError(t, err)

app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: 1}})
rsp := app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes})

// check result
ctx = app.GetContextForDeliverTx(txBytes)
okValue := ctx.KVStore(app.GetKey(banktypes.ModuleName)).Get([]byte("ok"))

if tc.expErr {
if tc.panicTx {
require.Equal(t, sdkerrors.ErrPanic.ABCICode(), rsp.Code)
} else {
require.Equal(t, sdkerrors.ErrOutOfGas.ABCICode(), rsp.Code)
}
require.Empty(t, okValue)
} else {
require.Equal(t, uint32(0), rsp.Code)
require.Equal(t, []byte("ok"), okValue)
}
// check block gas is always consumed
baseGas := uint64(59142) // baseGas is the gas consumed before tx msg
expGasConsumed := addUint64Saturating(tc.gasToConsume, baseGas)
if expGasConsumed > txtypes.MaxGasWanted {
// capped by gasLimit
expGasConsumed = txtypes.MaxGasWanted
}
require.Equal(t, expGasConsumed, ctx.BlockGasMeter().GasConsumed())
// tx fee is always deducted
require.Equal(t, int64(0), app.BankKeeper.GetBalance(ctx, addr1, feeCoin.Denom).Amount.Int64())
// sender's sequence is always increased
seq, err = app.AccountKeeper.GetSequence(ctx, addr1)
require.NoError(t, err)
require.Equal(t, uint64(1), seq)
})
}
}

func createTestTx(txConfig client.TxConfig, txBuilder client.TxBuilder, privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.Tx, []byte, error) {
// First round: we gather all the signer infos. We use the "set empty
// signature" hack to do that.
var sigsV2 []signing.SignatureV2
for i, priv := range privs {
sigV2 := signing.SignatureV2{
PubKey: priv.PubKey(),
Data: &signing.SingleSignatureData{
SignMode: txConfig.SignModeHandler().DefaultMode(),
Signature: nil,
},
Sequence: accSeqs[i],
}

sigsV2 = append(sigsV2, sigV2)
}
err := txBuilder.SetSignatures(sigsV2...)
if err != nil {
return nil, nil, err
}

// Second round: all signer infos are set, so each signer can sign.
sigsV2 = []signing.SignatureV2{}
for i, priv := range privs {
signerData := xauthsigning.SignerData{
ChainID: chainID,
AccountNumber: accNums[i],
Sequence: accSeqs[i],
}
sigV2, err := tx.SignWithPrivKey(
txConfig.SignModeHandler().DefaultMode(), signerData,
txBuilder, priv, txConfig, accSeqs[i])
if err != nil {
return nil, nil, err
}

sigsV2 = append(sigsV2, sigV2)
}
err = txBuilder.SetSignatures(sigsV2...)
if err != nil {
return nil, nil, err
}

txBytes, err := txConfig.TxEncoder()(txBuilder.GetTx())
if err != nil {
return nil, nil, err
}

return txBuilder.GetTx(), txBytes, nil
}

func addUint64Saturating(a, b uint64) uint64 {
if math.MaxUint64-a < b {
return math.MaxUint64
}

return a + b
}
4 changes: 4 additions & 0 deletions baseapp/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,7 @@ func (app *BaseApp) NewContext(isCheckTx bool, header tmproto.Header) sdk.Contex
func (app *BaseApp) NewUncachedContext(isCheckTx bool, header tmproto.Header) sdk.Context {
return sdk.NewContext(app.cms, header, isCheckTx, app.logger)
}

func (app *BaseApp) GetContextForDeliverTx(txBytes []byte) sdk.Context {
return app.getContextForTx(runTxModeDeliver, txBytes)
}

0 comments on commit ba1e099

Please sign in to comment.