Skip to content

Commit

Permalink
fix: GasMeter reset in AnteHandler EthGasConsumeDecorator (evmos#964)
Browse files Browse the repository at this point in the history
* Fix GasMeter reset in AnteHandler - EthGasConsumeDecorator

Conforming to the spec:
https://github.com/cosmos/cosmos-sdk/blob/e7066c4271ff3d33dc426dc6313c82a1201ae3c6/docs/basics/gas-fees.md
> Set newCtx.GasMeter to 0, with a limit of GasWanted.
> This step is extremely important, as it not only makes sure the transaction cannot consume infinite gas,
> but also that ctx.GasMeter is reset in-between each DeliverTx
> (ctx is set to newCtx after anteHandler is run, and the anteHandler is run each time DeliverTx is called).

* Compute gasWanted in ante handler based on msg gas

* Tests - check gas meter limit after EthGasConsumeDecorator ante handler runs

* Update CHANGELOG.md

* EthGasConsumeDecorator ante handler resets the gas meter only for CheckTx

* Reset the gas meter in Keeper.EthereumTx to an infinite gas meter

* Fix TestOutOfGasWhenDeployContract error check

* Move gas meter reset to the innermost EthAnteHandle

* add NewInfiniteGasMeterWithLimit for setting the user provided gas limit

Fixes block's consumed gas calculation in the block creation phase.

* Fix lint

* Fix lint

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
  • Loading branch information
2 people authored and yihuang committed Mar 7, 2022
1 parent cc1805a commit 5c0a5dd
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (rpc) [tharsis#933](https://github.com/tharsis/ethermint/pull/933) Fix newPendingTransactions subscription deadlock when a Websocket client exits without unsubscribing and the node errors.
* (rpc) [#955](https://github.com/tharsis/ethermint/pull/955) Fix websocket server push duplicated messages to subscriber.
* (rpc) [#970](https://github.com/tharsis/ethermint/pull/970) Fix unexpected nil reward values on `eth_feeHistory` response
* (ante) [tharsis#964](https://github.com/tharsis/ethermint/pull/964) add NewInfiniteGasMeterWithLimit for storing the user provided gas limit. Fixes block's consumed gas calculation in the block creation phase. (Only fix checkTx mode to avoid breaking consensus).

## [v0.9.0] - 2021-12-01

Expand Down
11 changes: 11 additions & 0 deletions app/ante/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ func NewEthGasConsumeDecorator(
// - transaction's gas limit is lower than the intrinsic gas
// - user doesn't have enough balance to deduct the transaction fees (gas_limit * gas_price)
// - transaction or block gas meter runs out of gas
// - sets the gas meter limit
func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
params := egcd.evmKeeper.GetParams(ctx)

Expand All @@ -169,6 +170,7 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
istanbul := ethCfg.IsIstanbul(blockHeight)
london := ethCfg.IsLondon(blockHeight)
evmDenom := params.EvmDenom
gasWanted := uint64(0)

var events sdk.Events

Expand All @@ -182,6 +184,7 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
if err != nil {
return ctx, sdkerrors.Wrap(err, "failed to unpack tx data")
}
gasWanted += txData.GetGas()

fees, err := egcd.evmKeeper.DeductTxCostsFromUserBalance(
ctx,
Expand Down Expand Up @@ -213,6 +216,14 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
gasPool.ConsumeGas(ctx.GasMeter().GasConsumedToLimit(), "gas pool check")
}

// only fix the checkTx mode to avoid breaking consensus.
if ctx.IsCheckTx() {
// Set ctx.GasMeter with a limit of GasWanted (gasLimit)
gasConsumed := ctx.GasMeter().GasConsumed()
ctx = ctx.WithGasMeter(ethermint.NewInfiniteGasMeterWithLimit(gasWanted))
ctx.GasMeter().ConsumeGas(gasConsumed, "copy gas consumed")
}

// we know that we have enough gas on the pool to cover the intrinsic gas
return next(ctx, tx, simulate)
}
Expand Down
18 changes: 14 additions & 4 deletions app/ante/eth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,43 +205,50 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() {

addr := tests.GenerateAddress()

tx := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 1000, big.NewInt(1), nil, nil, nil, nil)
txGasLimit := uint64(1000)
tx := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), txGasLimit, big.NewInt(1), nil, nil, nil, nil)
tx.From = addr.Hex()

tx2 := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 1000000, big.NewInt(1), nil, nil, nil, &ethtypes.AccessList{{Address: addr, StorageKeys: nil}})
tx2GasLimit := uint64(1000000)
tx2 := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), tx2GasLimit, big.NewInt(1), nil, nil, nil, &ethtypes.AccessList{{Address: addr, StorageKeys: nil}})
tx2.From = addr.Hex()

var vmdb *statedb.StateDB

testCases := []struct {
name string
tx sdk.Tx
gasLimit uint64
malleate func()
expPass bool
expPanic bool
}{
{"invalid transaction type", &invalidTx{}, func() {}, false, false},
{"invalid transaction type", &invalidTx{}, 0, func() {}, false, false},
{
"sender not found",
evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 1000, big.NewInt(1), nil, nil, nil, nil),
0,
func() {},
false, false,
},
{
"gas limit too low",
tx,
0,
func() {},
false, false,
},
{
"not enough balance for fees",
tx2,
0,
func() {},
false, false,
},
{
"not enough tx gas",
tx2,
0,
func() {
vmdb.AddBalance(addr, big.NewInt(1000000))
},
Expand All @@ -250,6 +257,7 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() {
{
"not enough block gas",
tx2,
0,
func() {
vmdb.AddBalance(addr, big.NewInt(1000000))

Expand All @@ -260,6 +268,7 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() {
{
"success",
tx2,
tx2GasLimit,
func() {
vmdb.AddBalance(addr, big.NewInt(1000000))

Expand All @@ -282,12 +291,13 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() {
return
}

_, err := dec.AnteHandle(suite.ctx.WithIsCheckTx(true), tc.tx, false, nextFn)
ctx, err := dec.AnteHandle(suite.ctx.WithIsCheckTx(true).WithGasMeter(sdk.NewInfiniteGasMeter()), tc.tx, false, nextFn)
if tc.expPass {
suite.Require().NoError(err)
} else {
suite.Require().Error(err)
}
suite.Require().Equal(tc.gasLimit, ctx.GasMeter().Limit())
})
}
}
Expand Down
91 changes: 91 additions & 0 deletions types/gasmeter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package types

import (
fmt "fmt"
math "math"

sdk "github.com/cosmos/cosmos-sdk/types"
)

// ErrorNegativeGasConsumed defines an error thrown when the amount of gas refunded results in a
// negative gas consumed amount.
// Copied from cosmos-sdk
type ErrorNegativeGasConsumed struct {
Descriptor string
}

// ErrorGasOverflow defines an error thrown when an action results gas consumption
// unsigned integer overflow.
type ErrorGasOverflow struct {
Descriptor string
}

type infiniteGasMeterWithLimit struct {
consumed sdk.Gas
limit sdk.Gas
}

// NewInfiniteGasMeterWithLimit returns a reference to a new infiniteGasMeter.
func NewInfiniteGasMeterWithLimit(limit sdk.Gas) sdk.GasMeter {
return &infiniteGasMeterWithLimit{
consumed: 0,
limit: limit,
}
}

func (g *infiniteGasMeterWithLimit) GasConsumed() sdk.Gas {
return g.consumed
}

func (g *infiniteGasMeterWithLimit) GasConsumedToLimit() sdk.Gas {
return g.consumed
}

func (g *infiniteGasMeterWithLimit) Limit() sdk.Gas {
return g.limit
}

// addUint64Overflow performs the addition operation on two uint64 integers and
// returns a boolean on whether or not the result overflows.
func addUint64Overflow(a, b uint64) (uint64, bool) {
if math.MaxUint64-a < b {
return 0, true
}

return a + b, false
}

func (g *infiniteGasMeterWithLimit) ConsumeGas(amount sdk.Gas, descriptor string) {
var overflow bool
// TODO: Should we set the consumed field after overflow checking?
g.consumed, overflow = addUint64Overflow(g.consumed, amount)
if overflow {
panic(ErrorGasOverflow{descriptor})
}
}

// RefundGas will deduct the given amount from the gas consumed. If the amount is greater than the
// gas consumed, the function will panic.
//
// Use case: This functionality enables refunding gas to the trasaction or block gas pools so that
// EVM-compatible chains can fully support the go-ethereum StateDb interface.
// See https://github.com/cosmos/cosmos-sdk/pull/9403 for reference.
func (g *infiniteGasMeterWithLimit) RefundGas(amount sdk.Gas, descriptor string) {
if g.consumed < amount {
panic(ErrorNegativeGasConsumed{Descriptor: descriptor})
}

g.consumed -= amount
}

func (g *infiniteGasMeterWithLimit) IsPastLimit() bool {
return false
}

func (g *infiniteGasMeterWithLimit) IsOutOfGas() bool {
return false
}

func (g *infiniteGasMeterWithLimit) String() string {
return fmt.Sprintf("InfiniteGasMeter:\n consumed: %d", g.consumed)
}
2 changes: 1 addition & 1 deletion x/evm/keeper/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ func (k *Keeper) ApplyMessageWithConfig(ctx sdk.Context, msg core.Message, trace
}
leftoverGas := msg.Gas() - intrinsicGas

// access list preparaion is moved from ante handler to here, because it's needed when `ApplyMessage` is called
// access list preparation is moved from ante handler to here, because it's needed when `ApplyMessage` is called
// under contexts where ante handlers are not run, for example `eth_call` and `eth_estimateGas`.
if rules := cfg.ChainConfig.Rules(big.NewInt(ctx.BlockHeight())); rules.IsBerlin {
stateDB.PrepareAccessList(msg.From(), msg.To(), vm.ActivePrecompiles(rules), msg.AccessList())
Expand Down

0 comments on commit 5c0a5dd

Please sign in to comment.