fix: evm fees refund

CHANGELOG.md

@@ -16,6 +16,7 @@ Follow the [migration document](docs/migrations/v0.5.x_to_v0.6.0.md) for upgrade
 
 ### BUG FIXES
 
+- Fix EVM fee-abstraction gas refund to compute the refund against the full transaction gas (`gasUsed + leftoverGas`) instead of `gasUsed`, bounding the refund by the paid fee and preventing the fee collector from being drained by high gas limit transactions.
 - [\#15](https://github.com/KiiChain/evm/pull/15) Fix distribution precompile 32-byte withdraw address inflating native supply by skipping non-20-byte accounts when mirroring balance changes to the StateDB.
 
 ## v0.5.1

tests/integration/x/vm/test_gas.go

@@ -193,7 +193,7 @@ func (suite *KeeperTestSuite) TestGasRefundGas() {
 				ctx,
 				*coreMsg,
 				tc.leftoverGas,
-				DefaultCoreMsgGasUsage,
+				DefaultCoreMsgGasUsage-tc.leftoverGas,
 				baseDenom,
 			)
 
@@ -214,3 +214,66 @@ func (suite *KeeperTestSuite) TestGasRefundGas() {
 		})
 	}
 }
+
+// TestGasRefundGasNoOverRefund guards against the fee_collector drain where the fee-abstraction
+// refund used the consumed gas instead of the gas limit as the denominator. With a gas limit far
+// greater than the gas used, the refund must never exceed the value of the unused gas.
+func (suite *KeeperTestSuite) TestGasRefundGasNoOverRefund() {
+	baseDenom := types.GetEVMCoinDenom()
+
+	feeAddress := authtypes.NewModuleAddress(authtypes.FeeCollectorName)
+	balances := []banktypes.Balance{
+		{
+			Address: feeAddress.String(),
+			Coins:   sdk.NewCoins(sdk.NewCoin(baseDenom, sdkmath.NewInt(6e18))),
+		},
+	}
+	bankGenesis := banktypes.DefaultGenesisState()
+	bankGenesis.Balances = balances
+	customGenesis := network.CustomGenesisState{}
+	customGenesis[banktypes.ModuleName] = bankGenesis
+
+	keyring := testkeyring.New(2)
+	unitNetwork := network.NewUnitTestNetwork(
+		suite.Create,
+		network.WithPreFundedAccounts(keyring.GetAllAccAddrs()...),
+		network.WithCustomGenesis(customGenesis),
+	)
+	grpcHandler := grpc.NewIntegrationHandler(unitNetwork)
+	txFactory := factory.New(unitNetwork, grpcHandler)
+
+	sender := keyring.GetKey(0)
+	recipient := keyring.GetAddr(1)
+
+	const gasLimit = uint64(100000)
+	const gasUsed = uint64(21000)
+	const leftoverGas = gasLimit - gasUsed
+
+	coreMsg, err := txFactory.GenerateGethCoreMsg(
+		sender.Priv,
+		types.EvmTxArgs{
+			To:       &recipient,
+			Amount:   big.NewInt(100),
+			GasPrice: big.NewInt(DefaultGasPrice),
+			GasLimit: gasLimit,
+		},
+	)
+	suite.Require().NoError(err)
+
+	ctx, _ := unitNetwork.GetContext().CacheContext()
+
+	paidFee := sdkmath.NewInt(int64(gasLimit) * DefaultGasPrice)
+	ctx = ctx.WithValue(keeper.ContextPaidFeesKey{}, sdk.NewCoins(sdk.NewCoin(baseDenom, paidFee)))
+
+	initialFeeCollector := unitNetwork.App.GetBankKeeper().GetBalance(ctx, feeAddress, baseDenom).Amount
+
+	err = unitNetwork.App.GetEVMKeeper().RefundGas(ctx, *coreMsg, leftoverGas, gasUsed, baseDenom)
+	suite.Require().NoError(err)
+
+	finalFeeCollector := unitNetwork.App.GetBankKeeper().GetBalance(ctx, feeAddress, baseDenom).Amount
+	refunded := initialFeeCollector.Sub(finalFeeCollector)
+
+	expectedRefund := sdkmath.NewInt(int64(leftoverGas) * DefaultGasPrice)
+	suite.Require().Equal(expectedRefund, refunded)
+	suite.Require().True(refunded.LTE(paidFee))
+}

x/vm/keeper/gas.go

@@ -31,11 +31,10 @@ func (k *Keeper) GetEthIntrinsicGas(ctx sdk.Context, msg core.Message, cfg *para
 		homestead, istanbul, shanghai)
 }
 
-// RefundGas transfers the leftover gas to the sender of the message, capped to half of the total gas
-// 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.
-func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64, gasUsed uint64, denom string) error {
+// RefundGas transfers the value of the leftover gas to the sender of the message. The refund is
+// bounded by the paid fee: it is computed against the full transaction gas (gasUsed + leftoverGas),
+// so the sender can never receive more than what was charged upfront.
+func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas, gasUsed uint64, denom string) error {
 	// Return EVM tokens for remaining gas, exchanged at the original rate.
 	remaining := new(big.Int).Mul(new(big.Int).SetUint64(leftoverGas), msg.GasPrice)
 
@@ -67,12 +66,9 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64
 				denom = paidCoin.Denom
 				amount := paidCoin.Amount.BigInt()
 
-				// Calculate the amount to refund
-				// This is calculated as:
-				// remaining = amount * leftoverGas / gasUsed
 				remaining = new(big.Int).Div(
 					new(big.Int).Mul(amount, new(big.Int).SetUint64(leftoverGas)),
-					new(big.Int).SetUint64(gasUsed),
+					new(big.Int).SetUint64(msg.GasLimit),
 				)
 			}
 		}