Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Fix GasMeter reset in AnteHandler - EthGasConsumeDecorator #964

Merged
merged 15 commits into from
Mar 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

* (rpc) [\#529](https://github.com/tharsis/ethermint/pull/975) Fix unexpected `nil` values for `reward`, returned by `EffectiveGasTipValue(blockBaseFee)` in the `eth_feeHistory` RPC method.
* (evm) [\#529](https://github.com/tharsis/ethermint/issues/529) Add support return value on trace tx response.
* (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.
* (rpc) [tharsis#975](https://github.com/tharsis/ethermint/pull/975) Fix unexpected `nil` values for `reward`, returned by `EffectiveGasTipValue(blockBaseFee)` in the `eth_feeHistory` RPC method.
* (rpc) [tharsis#970] (https://github.com/tharsis/ethermint/pull/970) Fix unexpected nil reward values on `eth_feeHistory` response
* (evm) [tharsis#529](https://github.com/tharsis/ethermint/issues/529) support return value on trace tx response.

## Improvements

Expand Down
8 changes: 8 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,11 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
gasPool.ConsumeGas(ctx.GasMeter().GasConsumedToLimit(), "gas pool check")
}

// 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()), cfg.ChainConfig.MergeForkBlock != nil); rules.IsBerlin {
stateDB.PrepareAccessList(msg.From(), msg.To(), vm.ActivePrecompiles(rules), msg.AccessList())
Expand Down