From 8288315a11b74bae0763530a8bb30a6c935d3e83 Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa Date: Wed, 16 Jul 2025 18:28:11 -0300 Subject: [PATCH 1/6] feat: update the gas refundGas function to handle paied fees from the context --- x/vm/keeper/gas.go | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/x/vm/keeper/gas.go b/x/vm/keeper/gas.go index b8dc6bc5..c14ee65a 100644 --- a/x/vm/keeper/gas.go +++ b/x/vm/keeper/gas.go @@ -15,6 +15,9 @@ import ( "github.com/cosmos/evm/x/vm/types" ) +// ContextPaidFeesKey is a key used to store the paid fee in the context +type ContextPaidFeesKey struct{} + // GetEthIntrinsicGas returns the intrinsic gas cost for the transaction func (k *Keeper) GetEthIntrinsicGas(ctx sdk.Context, msg core.Message, cfg *params.ChainConfig, isContractCreation bool) (uint64, error) { height := big.NewInt(ctx.BlockHeight()) @@ -37,17 +40,44 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64 // negative refund errors return errorsmod.Wrapf(types.ErrInvalidRefund, "refunded amount value cannot be negative %d", remaining.Int64()) case 1: - // positive amount refund + // Attempt to extract the paid coin from the context + // This is used when fee abstraction is applied into the fee payment + // If no value is found under the context, the original denom is used + if val := ctx.Value(ContextPaidFeesKey{}); val != nil { + // We check if a coin exists under the value and if its not empty + if paidCoins, ok := val.(sdk.Coins); ok && !paidCoins.IsZero() { + // We know that only a single coin is used for EVM payments + if len(paidCoins) != 1 { + // This should never happen, but if it does, we return an error + return errorsmod.Wrapf(types.ErrInvalidRefund, "expected a single coin for EVM refunds, got %d", len(paidCoins)) + } + paidCoin := paidCoins[0] + + // Extract the coin information + 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(msg.Gas()), + ) + } + } + + // Positive amount refund refundedCoins := sdk.Coins{sdk.NewCoin(denom, sdkmath.NewIntFromBigInt(remaining))} - // refund to sender from the fee collector module account, which is the escrow account in charge of collecting tx fees + // Refund to sender from the fee collector module account, which is the escrow account in charge of collecting tx fees err := k.bankWrapper.SendCoinsFromModuleToAccount(ctx, authtypes.FeeCollectorName, msg.From().Bytes(), refundedCoins) if err != nil { err = errorsmod.Wrapf(errortypes.ErrInsufficientFunds, "fee collector account failed to refund fees: %s", err.Error()) return errorsmod.Wrapf(err, "failed to refund %d leftover gas (%s)", leftoverGas, refundedCoins.String()) } default: - // no refund, consume gas and update the tx gas meter + // No refund, consume gas and update the tx gas meter } return nil From e5f134ffbfe1fa766275d5b07db26a35dd39bd1d Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa Date: Wed, 16 Jul 2025 18:28:45 -0300 Subject: [PATCH 2/6] test: add tests to the new logic for gasRefunds --- x/vm/keeper/gas_test.go | 205 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 x/vm/keeper/gas_test.go diff --git a/x/vm/keeper/gas_test.go b/x/vm/keeper/gas_test.go new file mode 100644 index 00000000..30f7724f --- /dev/null +++ b/x/vm/keeper/gas_test.go @@ -0,0 +1,205 @@ +package keeper_test + +import ( + "math/big" + + sdkmath "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + "github.com/cosmos/evm/testutil/integration/os/factory" + "github.com/cosmos/evm/testutil/integration/os/grpc" + testkeyring "github.com/cosmos/evm/testutil/integration/os/keyring" + erc20mocks "github.com/cosmos/evm/x/erc20/types/mocks" + "github.com/cosmos/evm/x/vm/keeper" + "github.com/cosmos/evm/x/vm/types" + "go.uber.org/mock/gomock" +) + +const ( + DefaultCoreMsgGasUsage = 21000 + DefaultGasPrice = 120000 +) + +// TestGasRefundGas tests the refund gas exclusively without going though the state transition +// The gas part on the name refers to the file name to not generate a duplicated test name +func (suite *KeeperTestSuite) TestGasRefundGas() { + // Create a txFactory + grpcHandler := grpc.NewIntegrationHandler(suite.network) + txFactory := factory.New(suite.network, grpcHandler) + + // Create a core message to use for the test + keyring := testkeyring.New(2) + sender := keyring.GetKey(0) + recipient := keyring.GetAddr(1) + coreMsg, err := txFactory.GenerateGethCoreMsg( + sender.Priv, + types.EvmTxArgs{ + To: &recipient, + Amount: big.NewInt(100), + GasPrice: big.NewInt(120000), + }, + ) + suite.Require().NoError(err) + + // Produce all the test cases + testCases := []struct { + name string + leftoverGas uint64 // The coreMsg always uses 21000 gas limit + malleate func(sdk.Context) sdk.Context + expectedRefund sdk.Coins + errContains string + }{ + { + name: "Refund the full value as no gas was used", + leftoverGas: DefaultCoreMsgGasUsage, + expectedRefund: sdk.NewCoins( + sdk.NewCoin(suite.network.GetBaseDenom(), sdkmath.NewInt(DefaultCoreMsgGasUsage*DefaultGasPrice)), + ), + }, + { + name: "Refund half the value as half gas was used", + leftoverGas: DefaultCoreMsgGasUsage / 2, + expectedRefund: sdk.NewCoins( + sdk.NewCoin(suite.network.GetBaseDenom(), sdkmath.NewInt((DefaultCoreMsgGasUsage*DefaultGasPrice)/2)), + ), + }, + { + name: "No refund as no gas was left over used", + leftoverGas: 0, + expectedRefund: sdk.NewCoins( + sdk.NewCoin(suite.network.GetBaseDenom(), sdkmath.NewInt(0)), + ), + }, + { + name: "Refund with context fees, refunding the full value", + leftoverGas: DefaultCoreMsgGasUsage, + malleate: func(ctx sdk.Context) sdk.Context { + // Set the fee abstraction paid fee key with a single coin + return ctx.WithValue( + keeper.ContextPaidFeesKey{}, + sdk.NewCoins( + sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)), + ), + ) + }, + expectedRefund: sdk.NewCoins( + sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)), + ), + }, + { + name: "Refund with context fees, refunding the half the value", + leftoverGas: DefaultCoreMsgGasUsage / 2, + malleate: func(ctx sdk.Context) sdk.Context { + // Set the fee abstraction paid fee key with a single coin + return ctx.WithValue( + keeper.ContextPaidFeesKey{}, + sdk.NewCoins( + sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)), + ), + ) + }, + expectedRefund: sdk.NewCoins( + sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000/2)), + ), + }, + { + name: "Refund with context fees, no refund", + leftoverGas: 0, + malleate: func(ctx sdk.Context) sdk.Context { + // Set the fee abstraction paid fee key with a single coin + return ctx.WithValue( + keeper.ContextPaidFeesKey{}, + sdk.NewCoins( + sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)), + ), + ) + }, + expectedRefund: sdk.NewCoins( + sdk.NewCoin("acoin", sdkmath.NewInt(0)), + ), + }, + { + name: "Error - More than one coin being passed", + leftoverGas: DefaultCoreMsgGasUsage, + malleate: func(ctx sdk.Context) sdk.Context { + // Set the fee abstraction paid fee key with a single coin + return ctx.WithValue( + keeper.ContextPaidFeesKey{}, + sdk.NewCoins( + sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)), + sdk.NewCoin("atwo", sdkmath.NewInt(750_000_000)), + ), + ) + }, + expectedRefund: sdk.NewCoins( + sdk.NewCoin("acoin", sdkmath.NewInt(0)), // We say as zero to skip the mock bank check + ), + errContains: "expected a single coin for EVM refunds, got 2", + }, + } + + // Iterate though the test cases + for _, tc := range testCases { + suite.Run(tc.name, func() { + // Generate a cached context to not leak data between tests + ctx, _ := suite.network.GetContext().CacheContext() + + // Create a new controller for the mock + ctrl := gomock.NewController(suite.T()) + defer ctrl.Finish() + + // Apply the malleate function to the context + if tc.malleate != nil { + ctx = tc.malleate(ctx) + } + + // Create a new mock bank keeper + mockBankKeeper := erc20mocks.NewMockBankKeeper(ctrl) + + // Apply the expect, but only if expected refund is not zero + if !tc.expectedRefund.IsZero() { + mockBankKeeper.EXPECT().SendCoinsFromModuleToAccount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func(ctx sdk.Context, senderModule string, recipient sdk.AccAddress, coins sdk.Coins) error { + if !coins.Equal(tc.expectedRefund) { + suite.T().Errorf("expected %s, got %s", tc.expectedRefund, coins) + } + + return nil + }) + } + + // Initialize a new EVM keeper with the mock bank keeper + // We need to redo this every time, since we will apply the mocked bank keeper at this step + evmKeeper := keeper.NewKeeper( + suite.network.App.AppCodec(), + suite.network.App.GetKey(types.StoreKey), + suite.network.App.GetTKey(types.StoreKey), + authtypes.NewModuleAddress(govtypes.ModuleName), + suite.network.App.AccountKeeper, + mockBankKeeper, + suite.network.App.StakingKeeper, + suite.network.App.FeeMarketKeeper, + suite.network.App.Erc20Keeper, + "", + suite.network.App.GetSubspace(types.ModuleName), + ) + + // Call the msg, not further checks are needed, all balance checks are done in the mock + err := evmKeeper.RefundGas( + ctx, + coreMsg, + tc.leftoverGas, + suite.network.GetBaseDenom(), + ) + + // Check the error + if tc.errContains != "" { + suite.Require().ErrorContains(err, tc.errContains, "RefundGas should return an error") + } else { + suite.Require().NoError(err, "RefundGas should not return an error") + } + }) + } + +} From 102da88df601e5beecffcf7a31d896bf959f2e10 Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa <68653689+jhelison@users.noreply.github.com> Date: Wed, 16 Jul 2025 18:36:13 -0300 Subject: [PATCH 3/6] refactor: apply typo fix Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- x/vm/keeper/gas.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/vm/keeper/gas.go b/x/vm/keeper/gas.go index c14ee65a..57ec8e12 100644 --- a/x/vm/keeper/gas.go +++ b/x/vm/keeper/gas.go @@ -44,7 +44,7 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64 // This is used when fee abstraction is applied into the fee payment // If no value is found under the context, the original denom is used if val := ctx.Value(ContextPaidFeesKey{}); val != nil { - // We check if a coin exists under the value and if its not empty + // We check if a coin exists under the value and if it's not empty if paidCoins, ok := val.(sdk.Coins); ok && !paidCoins.IsZero() { // We know that only a single coin is used for EVM payments if len(paidCoins) != 1 { From bdff4ca94c2c477809ffaec69812f48dbc95d108 Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa Date: Wed, 16 Jul 2025 18:37:43 -0300 Subject: [PATCH 4/6] 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 57ec8e12..f7de7f02 100644 --- a/x/vm/keeper/gas.go +++ b/x/vm/keeper/gas.go @@ -35,6 +35,12 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64 // Return EVM tokens for remaining gas, exchanged at the original rate. remaining := new(big.Int).Mul(new(big.Int).SetUint64(leftoverGas), msg.GasPrice()) + // 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 f0609d9c06e69c83ea9264975c74dd61907e05c5 Mon Sep 17 00:00:00 2001 From: Alex | Interchain Labs Date: Thu, 14 Aug 2025 14:42:52 -0400 Subject: [PATCH 5/6] retract (#465) --- go.mod | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go.mod b/go.mod index f88c0a37..116d4501 100644 --- a/go.mod +++ b/go.mod @@ -279,3 +279,5 @@ replace ( // replace broken goleveldb github.com/syndtr/goleveldb => github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 ) + +retract v0.4.0 \ No newline at end of file From 32bfe20e9bc915b42423acd4472de37ccf3f51e3 Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa Date: Mon, 15 Sep 2025 17:32:40 -0300 Subject: [PATCH 6/6] test: move gas refund tests to test/integration --- .../integration/x/vm/test_gas.go | 41 ++++++++++--------- x/vm/keeper/gas.go | 4 +- 2 files changed, 23 insertions(+), 22 deletions(-) rename x/vm/keeper/gas_test.go => tests/integration/x/vm/test_gas.go (84%) diff --git a/x/vm/keeper/gas_test.go b/tests/integration/x/vm/test_gas.go similarity index 84% rename from x/vm/keeper/gas_test.go rename to tests/integration/x/vm/test_gas.go index 30f7724f..bce4c7d2 100644 --- a/x/vm/keeper/gas_test.go +++ b/tests/integration/x/vm/test_gas.go @@ -1,4 +1,4 @@ -package keeper_test +package vm import ( "math/big" @@ -7,9 +7,9 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" - "github.com/cosmos/evm/testutil/integration/os/factory" - "github.com/cosmos/evm/testutil/integration/os/grpc" - testkeyring "github.com/cosmos/evm/testutil/integration/os/keyring" + "github.com/cosmos/evm/testutil/integration/evm/factory" + "github.com/cosmos/evm/testutil/integration/evm/grpc" + testkeyring "github.com/cosmos/evm/testutil/keyring" erc20mocks "github.com/cosmos/evm/x/erc20/types/mocks" "github.com/cosmos/evm/x/vm/keeper" "github.com/cosmos/evm/x/vm/types" @@ -25,8 +25,8 @@ const ( // The gas part on the name refers to the file name to not generate a duplicated test name func (suite *KeeperTestSuite) TestGasRefundGas() { // Create a txFactory - grpcHandler := grpc.NewIntegrationHandler(suite.network) - txFactory := factory.New(suite.network, grpcHandler) + grpcHandler := grpc.NewIntegrationHandler(suite.Network) + txFactory := factory.New(suite.Network, grpcHandler) // Create a core message to use for the test keyring := testkeyring.New(2) @@ -54,21 +54,21 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { name: "Refund the full value as no gas was used", leftoverGas: DefaultCoreMsgGasUsage, expectedRefund: sdk.NewCoins( - sdk.NewCoin(suite.network.GetBaseDenom(), sdkmath.NewInt(DefaultCoreMsgGasUsage*DefaultGasPrice)), + sdk.NewCoin(suite.Network.GetBaseDenom(), sdkmath.NewInt(DefaultCoreMsgGasUsage*DefaultGasPrice)), ), }, { name: "Refund half the value as half gas was used", leftoverGas: DefaultCoreMsgGasUsage / 2, expectedRefund: sdk.NewCoins( - sdk.NewCoin(suite.network.GetBaseDenom(), sdkmath.NewInt((DefaultCoreMsgGasUsage*DefaultGasPrice)/2)), + sdk.NewCoin(suite.Network.GetBaseDenom(), sdkmath.NewInt((DefaultCoreMsgGasUsage*DefaultGasPrice)/2)), ), }, { name: "No refund as no gas was left over used", leftoverGas: 0, expectedRefund: sdk.NewCoins( - sdk.NewCoin(suite.network.GetBaseDenom(), sdkmath.NewInt(0)), + sdk.NewCoin(suite.Network.GetBaseDenom(), sdkmath.NewInt(0)), ), }, { @@ -143,7 +143,7 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { for _, tc := range testCases { suite.Run(tc.name, func() { // Generate a cached context to not leak data between tests - ctx, _ := suite.network.GetContext().CacheContext() + ctx, _ := suite.Network.GetContext().CacheContext() // Create a new controller for the mock ctrl := gomock.NewController(suite.T()) @@ -172,25 +172,26 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { // Initialize a new EVM keeper with the mock bank keeper // We need to redo this every time, since we will apply the mocked bank keeper at this step evmKeeper := keeper.NewKeeper( - suite.network.App.AppCodec(), - suite.network.App.GetKey(types.StoreKey), - suite.network.App.GetTKey(types.StoreKey), + suite.Network.App.AppCodec(), + suite.Network.App.GetKey(types.StoreKey), + suite.Network.App.GetKey(types.StoreKey), + suite.Network.App.GetEVMKeeper().KVStoreKeys(), authtypes.NewModuleAddress(govtypes.ModuleName), - suite.network.App.AccountKeeper, + suite.Network.App.GetAccountKeeper(), mockBankKeeper, - suite.network.App.StakingKeeper, - suite.network.App.FeeMarketKeeper, - suite.network.App.Erc20Keeper, + suite.Network.App.GetStakingKeeper(), + suite.Network.App.GetFeeMarketKeeper(), + suite.Network.App.GetConsensusParamsKeeper(), + suite.Network.App.GetErc20Keeper(), "", - suite.network.App.GetSubspace(types.ModuleName), ) // Call the msg, not further checks are needed, all balance checks are done in the mock err := evmKeeper.RefundGas( ctx, - coreMsg, + *coreMsg, tc.leftoverGas, - suite.network.GetBaseDenom(), + suite.Network.GetBaseDenom(), ) // Check the error diff --git a/x/vm/keeper/gas.go b/x/vm/keeper/gas.go index 1ae23e86..be7c565f 100644 --- a/x/vm/keeper/gas.go +++ b/x/vm/keeper/gas.go @@ -40,7 +40,7 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64 remaining := new(big.Int).Mul(new(big.Int).SetUint64(leftoverGas), msg.GasPrice) // Check if gas is zero - if msg.Gas() == 0 { + if msg.GasLimit == 0 { // If gas is zero, we cannot refund anything, so we return early return nil } @@ -72,7 +72,7 @@ func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64 // remaining = amount * leftoverGas / gasUsed remaining = new(big.Int).Div( new(big.Int).Mul(amount, new(big.Int).SetUint64(leftoverGas)), - new(big.Int).SetUint64(msg.Gas()), + new(big.Int).SetUint64(msg.GasLimit), ) } }