fix: make gas refunds error free and panic safe (#96)

* make gas refunds error free and panic safe

* add test

* add logger message

app/app.go

@@ -367,7 +367,7 @@ func (app *MinitiaApp) SetCheckTx(handler blockchecktx.CheckTx) {
 
 func (app *MinitiaApp) setPostHandler() {
 	app.SetPostHandler(posthandler.NewPostHandler(
-		app.AccountKeeper,
+		app.Logger(),
 		app.EVMKeeper,
 	))
 }

app/posthandler/gasrefund.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	errorsmod "cosmossdk.io/errors"
+	"cosmossdk.io/log"
 	"cosmossdk.io/math"
 	storetypes "cosmossdk.io/store/types"
 
@@ -17,12 +18,14 @@ import (
 var _ sdk.PostDecorator = &GasRefundDecorator{}
 
 type GasRefundDecorator struct {
-	ek EVMKeeper
+	logger log.Logger
+	ek     EVMKeeper
 }
 
-func NewGasRefundDecorator(ek EVMKeeper) sdk.PostDecorator {
+func NewGasRefundDecorator(logger log.Logger, ek EVMKeeper) sdk.PostDecorator {
+	logger = logger.With("module", "gas_refund")
 	return &GasRefundDecorator{
-		ek,
+		logger, ek,
 	}
 }
 
@@ -70,20 +73,40 @@ func (erd *GasRefundDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simulate,
 			sdk.NewAttribute(AttributeKeyCoins, coinsRefund.String()),
 		))
 
-		// TODO - should we charge gas for refund?
-		//
-		// for now, we use infinite gas meter to prevent out of gas error or inconsistency between
-		// used gas and refunded gas.
-		feeCollectorAddr := authtypes.NewModuleAddress(authtypes.FeeCollectorName)
-		err = erd.ek.ERC20Keeper().SendCoins(ctx.WithGasMeter(storetypes.NewInfiniteGasMeter()), feeCollectorAddr, feePayer, coinsRefund)
-		if err != nil {
-			return ctx, err
-		}
+		// conduct gas refund
+		erd.safeRefund(ctx, feePayer, coinsRefund)
 	}
 
 	return next(ctx, tx, simulate, success)
 }
 
+func (erd *GasRefundDecorator) safeRefund(ctx sdk.Context, feePayer sdk.AccAddress, coinsRefund sdk.Coins) {
+	defer func() {
+		if r := recover(); r != nil {
+			switch r := r.(type) {
+			case storetypes.ErrorOutOfGas:
+				erd.logger.Error("failed to refund gas", "err", r, "feePayer", feePayer, "refudnAmount", coinsRefund)
+			default:
+				panic(r)
+			}
+		}
+	}()
+
+	// prepare context for refund operation
+	const gasLimitForRefund = 1_000_000
+	cacheCtx, commit := ctx.CacheContext()
+	cacheCtx = cacheCtx.WithGasMeter(storetypes.NewGasMeter(gasLimitForRefund))
+
+	feeCollectorAddr := authtypes.NewModuleAddress(authtypes.FeeCollectorName)
+	err := erd.ek.ERC20Keeper().SendCoins(cacheCtx, feeCollectorAddr, feePayer, coinsRefund)
+	if err != nil {
+		erd.logger.Error("failed to refund gas", "err", err)
+		return
+	}
+
+	commit()
+}
+
 const (
 	EventTypeGasRefund = "gas_refund"
 	AttributeKeyGas    = "gas"

app/posthandler/gasrefund_test.go

@@ -0,0 +1,126 @@
+package posthandler_test
+
+import (
+	"bytes"
+	"crypto/ecdsa"
+	"crypto/rand"
+	"math/big"
+
+	"cosmossdk.io/math"
+	storetypes "cosmossdk.io/store/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+
+	"github.com/ethereum/go-ethereum/common"
+	coretypes "github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/crypto"
+
+	"github.com/initia-labs/initia/crypto/ethsecp256k1"
+	"github.com/initia-labs/minievm/app/posthandler"
+	evmante "github.com/initia-labs/minievm/x/evm/ante"
+	"github.com/initia-labs/minievm/x/evm/contracts/erc20_factory"
+	"github.com/initia-labs/minievm/x/evm/keeper"
+	"github.com/initia-labs/minievm/x/evm/types"
+)
+
+func (suite *PostHandlerTestSuite) Test_NotSpendingGasForTxWithFeeDenom() {
+	suite.SetupTest() // setup
+	suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder()
+
+	gasRefundPostHandler := posthandler.NewGasRefundDecorator(suite.app.Logger(), suite.app.EVMKeeper)
+
+	params, err := suite.app.EVMKeeper.Params.Get(suite.ctx)
+	suite.NoError(err)
+
+	// create fee token
+	decimals := uint8(18)
+	feeCollectorAddr := authtypes.NewModuleAddress(authtypes.FeeCollectorName)
+	suite.app.EVMKeeper.InitializeWithDecimals(suite.ctx, decimals)
+	err = suite.app.EVMKeeper.ERC20Keeper().CreateERC20(suite.ctx, params.FeeDenom, decimals)
+	suite.NoError(err)
+
+	// mint fee token to fee collector
+	gasPrice := math.NewInt(1_000_000_000)
+	gasLimit := uint64(1_000_000)
+	paidFeeAmount := sdk.NewCoins(sdk.NewCoin(params.FeeDenom, gasPrice.Mul(math.NewIntFromUint64(gasLimit))))
+	err = suite.app.EVMKeeper.ERC20Keeper().MintCoins(suite.ctx, feeCollectorAddr, paidFeeAmount)
+	suite.NoError(err)
+
+	feeAmount := new(big.Int).Mul(
+		big.NewInt(int64(gasLimit)),
+		new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(decimals)-8), nil), // gas price is 1e-8
+	)
+
+	ethFactoryAddr, err := suite.app.EVMKeeper.GetERC20FactoryAddr(suite.ctx)
+	suite.NoError(err)
+
+	abi, err := erc20_factory.Erc20FactoryMetaData.GetAbi()
+	suite.NoError(err)
+
+	inputBz, err := abi.Pack("createERC20", "bar", "bar", uint8(6))
+	suite.NoError(err)
+
+	gasFeeCap := types.ToEthersUint(decimals, feeAmount)
+	gasFeeCap = gasFeeCap.Quo(gasFeeCap, new(big.Int).SetUint64(gasLimit))
+	value := types.ToEthersUint(decimals, big.NewInt(100))
+
+	ethChainID := types.ConvertCosmosChainIDToEthereumChainID(suite.ctx.ChainID())
+	ethTx := coretypes.NewTx(&coretypes.DynamicFeeTx{
+		ChainID:   types.ConvertCosmosChainIDToEthereumChainID(suite.ctx.ChainID()),
+		Nonce:     100,
+		GasTipCap: big.NewInt(100),
+		GasFeeCap: gasFeeCap,
+		Gas:       gasLimit,
+		To:        &ethFactoryAddr,
+		Data:      inputBz,
+		Value:     value,
+		AccessList: coretypes.AccessList{
+			coretypes.AccessTuple{Address: ethFactoryAddr,
+				StorageKeys: []common.Hash{
+					common.HexToHash("0x0000000000000000000000000000000000000000000000000000000000000000"),
+					common.HexToHash("0x0000000000000000000000000000000000000000000000000000000000000001"),
+					common.HexToHash("0x0000000000000000000000000000000000000000000000000000000000000002"),
+				}},
+		},
+	})
+
+	randBytes := make([]byte, 64)
+	_, err = rand.Read(randBytes)
+	suite.NoError(err)
+	reader := bytes.NewReader(randBytes)
+	privKey, err := ecdsa.GenerateKey(crypto.S256(), reader)
+	suite.NoError(err)
+	signer := coretypes.LatestSignerForChainID(ethChainID)
+	signedTx, err := coretypes.SignTx(ethTx, signer, privKey)
+	suite.NoError(err)
+
+	// Compute sender address
+	cosmosKey := ethsecp256k1.PrivKey{
+		Key: crypto.FromECDSA(privKey),
+	}
+	addrBz := cosmosKey.PubKey().Address()
+
+	// Convert to cosmos tx
+	sdkTx, err := keeper.NewTxUtils(suite.app.EVMKeeper).ConvertEthereumTxToCosmosTx(suite.ctx, signedTx)
+	suite.NoError(err)
+
+	// Spend half of the gas
+	gasMeter := storetypes.NewGasMeter(gasLimit)
+	gasMeter.ConsumeGas(gasLimit/2-2216 /* 2216 is extra gas for refunds */, "test")
+	gasPrices := sdk.DecCoins{sdk.NewDecCoin(params.FeeDenom, gasPrice)}
+
+	ctx := sdk.UnwrapSDKContext(suite.ctx).WithValue(evmante.ContextKeyGasPrices, gasPrices)
+	ctx = ctx.WithGasMeter(gasMeter).WithExecMode(sdk.ExecModeFinalize)
+	ctx, err = gasRefundPostHandler.PostHandle(ctx, sdkTx, false, true, func(ctx sdk.Context, tx sdk.Tx, simulate, success bool) (newCtx sdk.Context, err error) {
+		return ctx, nil
+	})
+	suite.NoError(err)
+
+	gasRefundRatio := params.GasRefundRatio
+	sender := sdk.AccAddress(addrBz.Bytes())
+
+	// Check the gas refund
+	amount := suite.app.BankKeeper.GetBalance(ctx, sender, params.FeeDenom)
+	refunds, _ := gasPrices.MulDec(gasRefundRatio.MulInt64(int64(gasLimit / 2))).TruncateDecimal()
+	suite.Equal(amount, refunds[0])
+}

app/posthandler/posthandler.go

@@ -3,20 +3,20 @@ package posthandler
 import (
 	"context"
 
+	"cosmossdk.io/log"
 	"cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
 
 	evmtypes "github.com/initia-labs/minievm/x/evm/types"
 )
 
 // NewPostHandler returns a new sdk.PostHandler that is composed of the sdk.ChainPostDecorators
 func NewPostHandler(
-	ak authante.AccountKeeper,
+	logger log.Logger,
 	ek EVMKeeper,
 ) sdk.PostHandler {
 	return sdk.ChainPostDecorators(
-		NewGasRefundDecorator(ek),
+		NewGasRefundDecorator(logger, ek),
 	)
 }
 

app/posthandler/posthandler_test.go

@@ -0,0 +1,136 @@
+package posthandler_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+
+	tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
+
+	"cosmossdk.io/log"
+	dbm "github.com/cosmos/cosmos-db"
+	"github.com/cosmos/cosmos-sdk/client"
+	"github.com/cosmos/cosmos-sdk/client/flags"
+	"github.com/cosmos/cosmos-sdk/client/tx"
+	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
+	"github.com/cosmos/cosmos-sdk/server"
+	simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
+	"github.com/cosmos/cosmos-sdk/testutil/testdata"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/tx/signing"
+	authsign "github.com/cosmos/cosmos-sdk/x/auth/signing"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	simcli "github.com/cosmos/cosmos-sdk/x/simulation/client/cli"
+
+	minievmapp "github.com/initia-labs/minievm/app"
+	evmconfig "github.com/initia-labs/minievm/x/evm/config"
+	evmtypes "github.com/initia-labs/minievm/x/evm/types"
+)
+
+const feeDenom = "feetoken"
+
+// PostHandlerTestSuite is a test suite to be used with ante handler tests.
+type PostHandlerTestSuite struct {
+	suite.Suite
+
+	app       *minievmapp.MinitiaApp
+	ctx       sdk.Context
+	clientCtx client.Context
+	txBuilder client.TxBuilder
+}
+
+// returns context and app with params set on account keeper
+func (suite *PostHandlerTestSuite) createTestApp(tempDir string) (*minievmapp.MinitiaApp, sdk.Context) {
+	appOptions := make(simtestutil.AppOptionsMap, 0)
+	appOptions[flags.FlagHome] = tempDir
+	appOptions[server.FlagInvCheckPeriod] = simcli.FlagPeriodValue
+
+	app := minievmapp.NewMinitiaApp(
+		log.NewNopLogger(), dbm.NewMemDB(), dbm.NewMemDB(), dbm.NewMemDB(), nil, true, evmconfig.DefaultEVMConfig(), appOptions,
+	)
+	ctx := app.BaseApp.NewUncachedContext(false, tmproto.Header{})
+	err := app.AccountKeeper.Params.Set(ctx, authtypes.DefaultParams())
+	suite.NoError(err)
+
+	params := evmtypes.DefaultParams()
+	params.FeeDenom = feeDenom
+	err = app.EVMKeeper.Params.Set(ctx, params)
+	suite.NoError(err)
+
+	return app, ctx
+}
+
+// SetupTest setups a new test, with new app, context, and anteHandler.
+func (suite *PostHandlerTestSuite) SetupTest() {
+	tempDir := suite.T().TempDir()
+	suite.app, suite.ctx = suite.createTestApp(tempDir)
+	suite.ctx = suite.ctx.WithBlockHeight(1)
+
+	// Set up TxConfig.
+	encodingConfig := minievmapp.MakeEncodingConfig()
+
+	// We're using TestMsg encoding in some tests, so register it here.
+	encodingConfig.Amino.RegisterConcrete(&testdata.TestMsg{}, "testdata.TestMsg", nil)
+	testdata.RegisterInterfaces(encodingConfig.InterfaceRegistry)
+
+	suite.clientCtx = client.Context{}.
+		WithTxConfig(encodingConfig.TxConfig)
+}
+
+// CreateTestTx is a helper function to create a tx given multiple inputs.
+func (suite *PostHandlerTestSuite) CreateTestTx(privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (authsign.Tx, error) {
+	defaultSignMode, err := authsign.APISignModeToInternal(suite.clientCtx.TxConfig.SignModeHandler().DefaultMode())
+	suite.NoError(err)
+
+	// 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:  defaultSignMode,
+				Signature: nil,
+			},
+			Sequence: accSeqs[i],
+		}
+
+		sigsV2 = append(sigsV2, sigV2)
+	}
+	err = suite.txBuilder.SetSignatures(sigsV2...)
+	if err != nil {
+		return nil, err
+	}
+
+	// Second round: all signer infos are set, so each signer can sign.
+	sigsV2 = []signing.SignatureV2{}
+	for i, priv := range privs {
+		signerData := authsign.SignerData{
+			Address:       sdk.AccAddress(priv.PubKey().Address()).String(),
+			ChainID:       chainID,
+			AccountNumber: accNums[i],
+			Sequence:      accSeqs[i],
+			PubKey:        priv.PubKey(),
+		}
+		sigV2, err := tx.SignWithPrivKey(
+			context.TODO(), defaultSignMode, signerData,
+			suite.txBuilder, priv, suite.clientCtx.TxConfig, accSeqs[i])
+		if err != nil {
+			return nil, err
+		}
+
+		sigsV2 = append(sigsV2, sigV2)
+	}
+	err = suite.txBuilder.SetSignatures(sigsV2...)
+	if err != nil {
+		return nil, err
+	}
+
+	return suite.txBuilder.GetTx(), nil
+}
+
+func TestPostHandlerTestSuite(t *testing.T) {
+	suite.Run(t, new(PostHandlerTestSuite))
+}

x/evm/keeper/txutils.go

@@ -225,7 +225,7 @@ func (u *TxUtils) ConvertCosmosTxToEthereumTx(ctx context.Context, sdkTx sdk.Tx)
 		return nil, nil, err
 	}
 
-	if len(fees) > 0 && fees[0].Denom != params.FeeDenom {
+	if !(len(fees) == 0 || (len(fees) == 1 && fees[0].Denom == params.FeeDenom)) {
 		return nil, nil, nil
 	}
 
@@ -403,7 +403,7 @@ func (u *TxUtils) IsEthereumTx(ctx context.Context, sdkTx sdk.Tx) (bool, error)
 		return false, err
 	}
 
-	if len(fees) > 0 && fees[0].Denom != params.FeeDenom {
+	if !(len(fees) == 0 || (len(fees) == 1 && fees[0].Denom == params.FeeDenom)) {
 		return false, nil
 	}