fix: GasMeter reset in AnteHandler EthGasConsumeDecorator (#964)

* Fix GasMeter reset in AnteHandler - EthGasConsumeDecorator

Conforming to the spec:
e7066c4271/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>

CHANGELOG.md

@@ -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
 

app/ante/eth.go

@@ -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)
 
@@ -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
 
@@ -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,
@@ -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)
 }

app/ante/eth_test.go

@@ -205,10 +205,12 @@ 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
@@ -216,32 +218,37 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() {
 	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))
 			},
@@ -250,6 +257,7 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() {
 		{
 			"not enough block gas",
 			tx2,
+			0,
 			func() {
 				vmdb.AddBalance(addr, big.NewInt(1000000))
 
@@ -260,6 +268,7 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() {
 		{
 			"success",
 			tx2,
+			tx2GasLimit,
 			func() {
 				vmdb.AddBalance(addr, big.NewInt(1000000))
 
@@ -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())
 		})
 	}
 }

types/gasmeter.go

@@ -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)
+}

x/evm/keeper/state_transition.go

@@ -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())