evm: unit tests for gas refund (#686)

* add TestGetEthIntrinsicGas

* more test cases in TestGetEthIntrinsicGas

* add GasToRefund tests

* add TestRefundGas tests

* remove duplicate leftoverGas check

* made resetGasMeterAndConsumeGas public for testing

* TestResetGasMeterAndConsumeGas tmp

* add mintFeeCollector flag for gas refund tests

* add comment and check

* add TestResetGasMeterAndConsumeGas

* Update x/evm/keeper/state_transition_test.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

x/evm/keeper/keeper_test.go

@@ -14,9 +14,12 @@ import (
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
+	"github.com/cosmos/cosmos-sdk/simapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
+	tmjson "github.com/tendermint/tendermint/libs/json"
 	feemarkettypes "github.com/tharsis/ethermint/x/feemarket/types"
 
 	"github.com/tharsis/ethermint/app"
@@ -31,6 +34,7 @@ import (
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/params"
 
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto/tmhash"
@@ -57,7 +61,8 @@ type KeeperTestSuite struct {
 	appCodec codec.Codec
 	signer   keyring.Signer
 
-	dynamicTxFee bool
+	dynamicTxFee     bool
+	mintFeeCollector bool
 }
 
 /// DoSetupTest setup test environment, it uses`require.TestingT` to support both `testing.T` and `testing.B`.
@@ -86,6 +91,37 @@ func (suite *KeeperTestSuite) DoSetupTest(t require.TestingT) {
 		suite.app = app.Setup(checkTx, nil)
 	}
 
+	if suite.mintFeeCollector {
+		// mint some coin to fee collector
+		coins := sdk.NewCoins(sdk.NewCoin(types.DefaultEVMDenom, sdk.NewInt(int64(params.TxGas)-1)))
+		genesisState := app.ModuleBasics.DefaultGenesis(suite.app.AppCodec())
+		balances := []banktypes.Balance{
+			{
+				Address: suite.app.AccountKeeper.GetModuleAddress(authtypes.FeeCollectorName).String(),
+				Coins:   coins,
+			},
+		}
+		// update total supply
+		bankGenesis := banktypes.NewGenesisState(banktypes.DefaultGenesisState().Params, balances, sdk.NewCoins(sdk.NewCoin(types.DefaultEVMDenom, sdk.NewInt((int64(params.TxGas)-1)))), []banktypes.Metadata{})
+		bz := suite.app.AppCodec().MustMarshalJSON(bankGenesis)
+		require.NotNil(t, bz)
+		genesisState[banktypes.ModuleName] = suite.app.AppCodec().MustMarshalJSON(bankGenesis)
+
+		// we marshal the genesisState of all module to a byte array
+		stateBytes, err := tmjson.MarshalIndent(genesisState, "", " ")
+		require.NoError(t, err)
+
+		//Initialize the chain
+		suite.app.InitChain(
+			abci.RequestInitChain{
+				ChainId:         "ethermint_9000-1",
+				Validators:      []abci.ValidatorUpdate{},
+				ConsensusParams: simapp.DefaultConsensusParams,
+				AppStateBytes:   stateBytes,
+			},
+		)
+	}
+
 	suite.ctx = suite.app.BaseApp.NewContext(checkTx, tmproto.Header{
 		Height:          1,
 		ChainID:         "ethermint_9000-1",

x/evm/keeper/state_transition.go

@@ -244,7 +244,7 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT
 	k.IncreaseTxIndexTransient()
 
 	// update the gas used after refund
-	k.resetGasMeterAndConsumeGas(res.GasUsed)
+	k.ResetGasMeterAndConsumeGas(res.GasUsed)
 	return res, nil
 }
 
@@ -398,6 +398,7 @@ func (k *Keeper) GetEthIntrinsicGas(msg core.Message, cfg *params.ChainConfig, i
 
 // GasToRefund calculates the amount of gas the state machine should refund to the sender. It is
 // capped by the refund quotient value.
+// Note: do not pass 0 to refundQuotient
 func (k *Keeper) GasToRefund(gasConsumed, refundQuotient uint64) uint64 {
 	// Apply refund counter
 	refund := gasConsumed / refundQuotient
@@ -412,6 +413,7 @@ func (k *Keeper) GasToRefund(gasConsumed, refundQuotient uint64) uint64 {
 // consumed in the transaction. Additionally, the function sets the total gas consumed to the value
 // returned by the EVM execution, thus ignoring the previous intrinsic gas consumed during in the
 // AnteHandler.
+// NOTE: DO NOT pass 0 to refundQuotient
 func (k *Keeper) RefundGas(msg core.Message, leftoverGas, refundQuotient uint64) (uint64, error) {
 	// safety check: leftover gas after execution should never exceed the gas limit defined on the message
 	if leftoverGas > msg.Gas() {
@@ -427,14 +429,6 @@ func (k *Keeper) RefundGas(msg core.Message, leftoverGas, refundQuotient uint64)
 	refund := k.GasToRefund(gasConsumed, refundQuotient)
 	leftoverGas += refund
 
-	// safety check: leftover gas after refund should never exceed the gas limit defined on the message
-	if leftoverGas > msg.Gas() {
-		return leftoverGas, stacktrace.Propagate(
-			sdkerrors.Wrapf(types.ErrInconsistentGas, "leftover gas cannot be greater than gas limit (%d > %d)", leftoverGas, msg.Gas()),
-			"failed to update gas consumed after refund of %d gas", refund,
-		)
-	}
-
 	// Return EVM tokens for remaining gas, exchanged at the original rate.
 	remaining := new(big.Int).Mul(new(big.Int).SetUint64(leftoverGas), msg.GasPrice())
 
@@ -461,9 +455,9 @@ func (k *Keeper) RefundGas(msg core.Message, leftoverGas, refundQuotient uint64)
 	return leftoverGas, nil
 }
 
-// resetGasMeterAndConsumeGas reset first the gas meter consumed value to zero and set it back to the new value
+// ResetGasMeterAndConsumeGas reset first the gas meter consumed value to zero and set it back to the new value
 // 'gasUsed'
-func (k *Keeper) resetGasMeterAndConsumeGas(gasUsed uint64) {
+func (k *Keeper) ResetGasMeterAndConsumeGas(gasUsed uint64) {
 	// reset the gas count
 	ctx := k.Ctx()
 	ctx.GasMeter().RefundGas(ctx.GasMeter().GasConsumed(), "reset the gas count")

x/evm/keeper/state_transition_benchmark_test.go

@@ -83,6 +83,8 @@ func newNativeMessage(
 	krSigner keyring.Signer,
 	ethSigner ethtypes.Signer,
 	txType byte,
+	data []byte,
+	accessList ethtypes.AccessList,
 ) (core.Message, error) {
 	msgSigner := ethtypes.MakeSigner(cfg, big.NewInt(blockHeight))
 
@@ -94,12 +96,29 @@ func newNativeMessage(
 	switch txType {
 	case ethtypes.LegacyTxType:
 		templateLegacyTx.Nonce = nonce
+		if data != nil {
+			templateLegacyTx.Data = data
+		}
 		ethTx = ethtypes.NewTx(templateLegacyTx)
 	case ethtypes.AccessListTxType:
 		templateAccessListTx.Nonce = nonce
+		if data != nil {
+			templateAccessListTx.Data = data
+		} else {
+			templateAccessListTx.Data = []byte{}
+		}
+
+		templateAccessListTx.AccessList = accessList
 		ethTx = ethtypes.NewTx(templateAccessListTx)
 	case ethtypes.DynamicFeeTxType:
 		templateDynamicFeeTx.Nonce = nonce
+
+		if data != nil {
+			templateAccessListTx.Data = data
+		} else {
+			templateAccessListTx.Data = []byte{}
+		}
+		templateAccessListTx.AccessList = accessList
 		ethTx = ethtypes.NewTx(templateDynamicFeeTx)
 		baseFee = big.NewInt(3)
 	default:
@@ -224,6 +243,8 @@ func BenchmarkApplyNativeMessage(b *testing.B) {
 			suite.signer,
 			signer,
 			ethtypes.AccessListTxType,
+			nil,
+			nil,
 		)
 		require.NoError(b, err)
 
@@ -257,6 +278,8 @@ func BenchmarkApplyNativeMessageWithLegacyTx(b *testing.B) {
 			suite.signer,
 			signer,
 			ethtypes.LegacyTxType,
+			nil,
+			nil,
 		)
 		require.NoError(b, err)
 
@@ -290,6 +313,8 @@ func BenchmarkApplyNativeMessageWithDynamicFeeTx(b *testing.B) {
 			suite.signer,
 			signer,
 			ethtypes.DynamicFeeTxType,
+			nil,
+			nil,
 		)
 		require.NoError(b, err)