From 2f0b96acd55501c7704ab9789507119fed2a62be Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa Date: Wed, 16 Jul 2025 18:37:43 -0300 Subject: [PATCH 1/2] feat: add check if gas is zero --- x/vm/keeper/gas.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x/vm/keeper/gas.go b/x/vm/keeper/gas.go index c7bdd5f..4666dbb 100644 --- a/x/vm/keeper/gas.go +++ b/x/vm/keeper/gas.go @@ -45,6 +45,12 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64 return nil } + // Check if gas is zero + if msg.Gas() == 0 { + // If gas is zero, we cannot refund anything, so we return early + return nil + } + switch remaining.Sign() { case -1: // negative refund errors From 902990888a95d668cc5b462b85cfc5cc9d698e96 Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa Date: Mon, 15 Sep 2025 17:32:40 -0300 Subject: [PATCH 2/2] test: move gas refund tests to test/integration --- CHANGELOG.md | 2 + tests/integration/x/vm/test_gas.go | 65 +++++++++++++++++++++++++++++- x/vm/keeper/gas.go | 20 +++------ 3 files changed, 71 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56954ae..a652af1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ 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. + ## v0.5.1 ### DEPENDENCIES diff --git a/tests/integration/x/vm/test_gas.go b/tests/integration/x/vm/test_gas.go index a3e5636..4f45c1b 100644 --- a/tests/integration/x/vm/test_gas.go +++ b/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)) +} diff --git a/x/vm/keeper/gas.go b/x/vm/keeper/gas.go index 4666dbb..50ef0d7 100644 --- a/x/vm/keeper/gas.go +++ b/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) @@ -45,12 +44,6 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64 return nil } - // Check if gas is zero - if msg.Gas() == 0 { - // If gas is zero, we cannot refund anything, so we return early - return nil - } - switch remaining.Sign() { case -1: // negative refund errors @@ -73,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), ) } }