From 09a681f939b8e04be7f4398dbcacd0f730f984d2 Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa Date: Wed, 16 Jul 2025 18:28:11 -0300 Subject: [PATCH 01/17] 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 ab5dcf08..c829def2 100644 --- a/x/vm/keeper/gas.go +++ b/x/vm/keeper/gas.go @@ -16,6 +16,9 @@ import ( authtypes "github.com/cosmos/cosmos-sdk/x/auth/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, @@ -41,17 +44,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.GasLimit), + ) + } + } + + // 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 beedbe8201d6e494b063945bd4bad96904866466 Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa Date: Wed, 16 Jul 2025 18:28:45 -0300 Subject: [PATCH 02/17] 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 bbbef89366dc49a8d1d3afacb029828bbbf48238 Mon Sep 17 00:00:00 2001 From: Jhelison Uchoa Date: Wed, 16 Jul 2025 18:37:43 -0300 Subject: [PATCH 03/17] 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 c829def2..d25ca213 100644 --- a/x/vm/keeper/gas.go +++ b/x/vm/keeper/gas.go @@ -39,6 +39,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.GasLimit == 0 { + // If gas is zero, we cannot refund anything, so we return early + return nil + } + switch remaining.Sign() { case -1: // negative refund errors From dac5b6cace4956ee8ded8b956cfb3df33eff1c42 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 04/17] 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 d25ca213..621b2ed1 100644 --- a/x/vm/keeper/gas.go +++ b/x/vm/keeper/gas.go @@ -54,7 +54,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 5ff443d1595dcae14cbdaa5151d8629ccc7fd743 Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Thu, 18 Dec 2025 12:55:45 -0300 Subject: [PATCH 05/17] feat: fix unconsistent states --- .../integration/x/vm/test_gas.go | 71 ++++++------------- .../integration/x/vm/test_state_transition.go | 1 + x/vm/keeper/gas.go | 6 +- x/vm/keeper/state_transition.go | 2 +- 4 files changed, 26 insertions(+), 54 deletions(-) rename x/vm/keeper/gas_test.go => tests/integration/x/vm/test_gas.go (65%) diff --git a/x/vm/keeper/gas_test.go b/tests/integration/x/vm/test_gas.go similarity index 65% rename from x/vm/keeper/gas_test.go rename to tests/integration/x/vm/test_gas.go index 30f7724f..dbe2e7e3 100644 --- a/x/vm/keeper/gas_test.go +++ b/tests/integration/x/vm/test_gas.go @@ -1,19 +1,16 @@ -package keeper_test +package vm 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/testutil/integration/evm/factory" + "github.com/cosmos/evm/testutil/integration/evm/grpc" + testkeyring "github.com/cosmos/evm/testutil/keyring" "github.com/cosmos/evm/x/vm/keeper" "github.com/cosmos/evm/x/vm/types" - "go.uber.org/mock/gomock" + "github.com/ethereum/go-ethereum/params" ) const ( @@ -25,8 +22,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 +51,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,54 +140,28 @@ 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() - - // Create a new controller for the mock - ctrl := gomock.NewController(suite.T()) - defer ctrl.Finish() + ctx, _ := suite.Network.GetContext().CacheContext() // 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) - } + vmdb := suite.Network.GetStateDB() + vmdb.AddRefund(params.TxGas) - return nil - }) + if tc.leftoverGas > DefaultCoreMsgGasUsage { + return } - // 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), - ) + gasUsed := DefaultCoreMsgGasUsage - tc.leftoverGas - // Call the msg, not further checks are needed, all balance checks are done in the mock - err := evmKeeper.RefundGas( - ctx, - coreMsg, + err = suite.Network.App.GetEVMKeeper().RefundGas( + suite.Network.GetContext(), + *coreMsg, tc.leftoverGas, - suite.network.GetBaseDenom(), + gasUsed, + suite.Network.GetBaseDenom(), ) // Check the error diff --git a/tests/integration/x/vm/test_state_transition.go b/tests/integration/x/vm/test_state_transition.go index 1847c902..587dc292 100644 --- a/tests/integration/x/vm/test_state_transition.go +++ b/tests/integration/x/vm/test_state_transition.go @@ -489,6 +489,7 @@ func (s *KeeperTestSuite) TestRefundGas() { unitNetwork.GetContext(), *coreMsg, refund, + gasUsed, unitNetwork.GetBaseDenom(), ) diff --git a/x/vm/keeper/gas.go b/x/vm/keeper/gas.go index 621b2ed1..c7bdd5f4 100644 --- a/x/vm/keeper/gas.go +++ b/x/vm/keeper/gas.go @@ -35,12 +35,12 @@ func (k *Keeper) GetEthIntrinsicGas(ctx sdk.Context, msg core.Message, cfg *para // 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, denom string) error { +func (k *Keeper) RefundGas(ctx sdk.Context, msg core.Message, leftoverGas uint64, 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) // Check if gas is zero - if msg.GasLimit == 0 { + if gasUsed == 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.GasLimit), + new(big.Int).SetUint64(gasUsed), ) } } diff --git a/x/vm/keeper/state_transition.go b/x/vm/keeper/state_transition.go index c9e7df65..a5c64c55 100644 --- a/x/vm/keeper/state_transition.go +++ b/x/vm/keeper/state_transition.go @@ -312,7 +312,7 @@ func (k *Keeper) ApplyTransaction(ctx sdk.Context, tx *ethtypes.Transaction) (*t if msg.GasLimit > res.GasUsed { remainingGas = msg.GasLimit - res.GasUsed } - if err = k.RefundGas(ctx, *msg, remainingGas, types.GetEVMCoinDenom()); err != nil { + if err = k.RefundGas(ctx, *msg, remainingGas, res.GasUsed, types.GetEVMCoinDenom()); err != nil { return nil, errorsmod.Wrapf(err, "failed to refund gas leftover gas to sender %s", msg.From) } From 46f3b4ce72d55bf26c5cfd2bbcdecace8964f5e9 Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 22 Dec 2025 11:06:10 -0300 Subject: [PATCH 06/17] fix: use correct network type for test --- tests/integration/x/vm/test_gas.go | 53 +++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/tests/integration/x/vm/test_gas.go b/tests/integration/x/vm/test_gas.go index dbe2e7e3..a1ea69f3 100644 --- a/tests/integration/x/vm/test_gas.go +++ b/tests/integration/x/vm/test_gas.go @@ -5,8 +5,11 @@ import ( sdkmath "cosmossdk.io/math" 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" "github.com/cosmos/evm/testutil/integration/evm/factory" "github.com/cosmos/evm/testutil/integration/evm/grpc" + "github.com/cosmos/evm/testutil/integration/evm/network" testkeyring "github.com/cosmos/evm/testutil/keyring" "github.com/cosmos/evm/x/vm/keeper" "github.com/cosmos/evm/x/vm/types" @@ -21,12 +24,38 @@ const ( // 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() { + // FeeCollector account is pre-funded with enough tokens + // for refund to work + // NOTE: everything should happen within the same block for + // feecollector account to remain funded + baseDenom := types.GetEVMCoinDenom() + + coins := sdk.NewCoins(sdk.NewCoin( + baseDenom, + sdkmath.NewInt(6e18), + )) + balances := []banktypes.Balance{ + { + Address: authtypes.NewModuleAddress(authtypes.FeeCollectorName).String(), + Coins: coins, + }, + } + bankGenesis := banktypes.DefaultGenesisState() + bankGenesis.Balances = balances + customGenesis := network.CustomGenesisState{} + customGenesis[banktypes.ModuleName] = bankGenesis + // Create a txFactory - grpcHandler := grpc.NewIntegrationHandler(suite.Network) - txFactory := factory.New(suite.Network, grpcHandler) + 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) // 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( @@ -34,7 +63,7 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { types.EvmTxArgs{ To: &recipient, Amount: big.NewInt(100), - GasPrice: big.NewInt(120000), + GasPrice: big.NewInt(DefaultGasPrice), }, ) suite.Require().NoError(err) @@ -51,21 +80,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(baseDenom, 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(baseDenom, 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(baseDenom, sdkmath.NewInt(0)), ), }, { @@ -140,14 +169,14 @@ 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, _ := unitNetwork.GetContext().CacheContext() // Apply the malleate function to the context if tc.malleate != nil { ctx = tc.malleate(ctx) } - vmdb := suite.Network.GetStateDB() + vmdb := unitNetwork.GetStateDB() vmdb.AddRefund(params.TxGas) if tc.leftoverGas > DefaultCoreMsgGasUsage { @@ -156,12 +185,12 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { gasUsed := DefaultCoreMsgGasUsage - tc.leftoverGas - err = suite.Network.App.GetEVMKeeper().RefundGas( - suite.Network.GetContext(), + err = unitNetwork.App.GetEVMKeeper().RefundGas( + unitNetwork.GetContext(), *coreMsg, tc.leftoverGas, gasUsed, - suite.Network.GetBaseDenom(), + baseDenom, ) // Check the error From adada32e690dd0fa9c1e80c21c009970bbc6fb1b Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 22 Dec 2025 11:25:10 -0300 Subject: [PATCH 07/17] feat: use correct malleate and unify testdenom --- tests/integration/x/vm/test_gas.go | 39 ++++++++++++++++++------------ 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/tests/integration/x/vm/test_gas.go b/tests/integration/x/vm/test_gas.go index a1ea69f3..2a57a038 100644 --- a/tests/integration/x/vm/test_gas.go +++ b/tests/integration/x/vm/test_gas.go @@ -19,6 +19,7 @@ import ( const ( DefaultCoreMsgGasUsage = 21000 DefaultGasPrice = 120000 + TestDenom = "acoin" ) // TestGasRefundGas tests the refund gas exclusively without going though the state transition @@ -30,10 +31,16 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { // feecollector account to remain funded baseDenom := types.GetEVMCoinDenom() - coins := sdk.NewCoins(sdk.NewCoin( - baseDenom, - sdkmath.NewInt(6e18), - )) + coins := sdk.NewCoins( + sdk.NewCoin( + baseDenom, + sdkmath.NewInt(6e18), + ), + sdk.NewCoin( + TestDenom, + sdkmath.NewInt(6e18), + ), + ) balances := []banktypes.Balance{ { Address: authtypes.NewModuleAddress(authtypes.FeeCollectorName).String(), @@ -98,19 +105,19 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { ), }, { - name: "Refund with context fees, refunding the full value", - leftoverGas: DefaultCoreMsgGasUsage, + name: "Refund with context fees, refunding the almost full value", + leftoverGas: DefaultCoreMsgGasUsage - 1, 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(TestDenom, sdkmath.NewInt(750_000_000)), ), ) }, expectedRefund: sdk.NewCoins( - sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)), + sdk.NewCoin(TestDenom, sdkmath.NewInt(750_000_000)), ), }, { @@ -121,28 +128,28 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { return ctx.WithValue( keeper.ContextPaidFeesKey{}, sdk.NewCoins( - sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)), + sdk.NewCoin(TestDenom, sdkmath.NewInt(750_000_000)), ), ) }, expectedRefund: sdk.NewCoins( - sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000/2)), + sdk.NewCoin(TestDenom, sdkmath.NewInt(750_000_000/2)), ), }, { name: "Refund with context fees, no refund", - leftoverGas: 0, + leftoverGas: 1, 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(TestDenom, sdkmath.NewInt(750_000_000)), ), ) }, expectedRefund: sdk.NewCoins( - sdk.NewCoin("acoin", sdkmath.NewInt(0)), + sdk.NewCoin(TestDenom, sdkmath.NewInt(0)), ), }, { @@ -153,13 +160,13 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { return ctx.WithValue( keeper.ContextPaidFeesKey{}, sdk.NewCoins( - sdk.NewCoin("acoin", sdkmath.NewInt(750_000_000)), + sdk.NewCoin(TestDenom, 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 + sdk.NewCoin(TestDenom, sdkmath.NewInt(0)), // We say as zero to skip the mock bank check ), errContains: "expected a single coin for EVM refunds, got 2", }, @@ -186,7 +193,7 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { gasUsed := DefaultCoreMsgGasUsage - tc.leftoverGas err = unitNetwork.App.GetEVMKeeper().RefundGas( - unitNetwork.GetContext(), + ctx, *coreMsg, tc.leftoverGas, gasUsed, From 70088e5f045bedebd380fd341e6473c5ab2df9ea Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 22 Dec 2025 11:26:35 -0300 Subject: [PATCH 08/17] fix: correct double coin case --- tests/integration/x/vm/test_gas.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/x/vm/test_gas.go b/tests/integration/x/vm/test_gas.go index 2a57a038..6a50cab2 100644 --- a/tests/integration/x/vm/test_gas.go +++ b/tests/integration/x/vm/test_gas.go @@ -154,7 +154,7 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { }, { name: "Error - More than one coin being passed", - leftoverGas: DefaultCoreMsgGasUsage, + leftoverGas: DefaultCoreMsgGasUsage - 1, // Using some leftover so the refund doesn't short circuit malleate: func(ctx sdk.Context) sdk.Context { // Set the fee abstraction paid fee key with a single coin return ctx.WithValue( From 6c808b1bc8b281fd1f077613431a24b97899890f Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 22 Dec 2025 15:47:47 -0300 Subject: [PATCH 09/17] feat: use correct gas used --- tests/integration/x/vm/test_gas.go | 39 +++++++++++++++++++----------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/tests/integration/x/vm/test_gas.go b/tests/integration/x/vm/test_gas.go index 6a50cab2..1213bec0 100644 --- a/tests/integration/x/vm/test_gas.go +++ b/tests/integration/x/vm/test_gas.go @@ -3,17 +3,20 @@ package vm import ( "math/big" - sdkmath "cosmossdk.io/math" - 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" + "github.com/ethereum/go-ethereum/params" + "github.com/cosmos/evm/testutil/integration/evm/factory" "github.com/cosmos/evm/testutil/integration/evm/grpc" "github.com/cosmos/evm/testutil/integration/evm/network" testkeyring "github.com/cosmos/evm/testutil/keyring" "github.com/cosmos/evm/x/vm/keeper" "github.com/cosmos/evm/x/vm/types" - "github.com/ethereum/go-ethereum/params" + + sdkmath "cosmossdk.io/math" + + 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" ) const ( @@ -41,9 +44,11 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { sdkmath.NewInt(6e18), ), ) + feeAddress := authtypes.NewModuleAddress(authtypes.FeeCollectorName) + balances := []banktypes.Balance{ { - Address: authtypes.NewModuleAddress(authtypes.FeeCollectorName).String(), + Address: feeAddress.String(), Coins: coins, }, } @@ -105,8 +110,8 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { ), }, { - name: "Refund with context fees, refunding the almost full value", - leftoverGas: DefaultCoreMsgGasUsage - 1, + 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( @@ -165,9 +170,6 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { ), ) }, - expectedRefund: sdk.NewCoins( - sdk.NewCoin(TestDenom, sdkmath.NewInt(0)), // We say as zero to skip the mock bank check - ), errContains: "expected a single coin for EVM refunds, got 2", }, } @@ -190,13 +192,15 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { return } - gasUsed := DefaultCoreMsgGasUsage - tc.leftoverGas + initialBalances := unitNetwork.App.GetBankKeeper().GetAllBalances(ctx, feeAddress) + + // gasUsed := DefaultCoreMsgGasUsage - tc.leftoverGas err = unitNetwork.App.GetEVMKeeper().RefundGas( ctx, *coreMsg, tc.leftoverGas, - gasUsed, + DefaultCoreMsgGasUsage, baseDenom, ) @@ -206,7 +210,14 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { } else { suite.Require().NoError(err, "RefundGas should not return an error") } + + // Check the balance change + if !tc.expectedRefund.Empty() { + diff := initialBalances.Sub(unitNetwork.App.GetBankKeeper().GetAllBalances(ctx, feeAddress)...) + for _, coin := range tc.expectedRefund { + suite.Require().Equal(coin.Amount, diff.AmountOf(coin.Denom)) + } + } }) } - } From 04a2921cff75df1cc48b3f45730181bc05b0ac72 Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 22 Dec 2025 15:49:39 -0300 Subject: [PATCH 10/17] chore: remove commented code --- tests/integration/x/vm/test_gas.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration/x/vm/test_gas.go b/tests/integration/x/vm/test_gas.go index 1213bec0..d3b99cc9 100644 --- a/tests/integration/x/vm/test_gas.go +++ b/tests/integration/x/vm/test_gas.go @@ -194,8 +194,6 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { initialBalances := unitNetwork.App.GetBankKeeper().GetAllBalances(ctx, feeAddress) - // gasUsed := DefaultCoreMsgGasUsage - tc.leftoverGas - err = unitNetwork.App.GetEVMKeeper().RefundGas( ctx, *coreMsg, From df73f060fc1c0ef6424bec5936dab87c58a87a8c Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 22 Dec 2025 16:17:08 -0300 Subject: [PATCH 11/17] fix: revert uneeded changes --- tests/integration/x/vm/test_gas.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/integration/x/vm/test_gas.go b/tests/integration/x/vm/test_gas.go index d3b99cc9..156455f9 100644 --- a/tests/integration/x/vm/test_gas.go +++ b/tests/integration/x/vm/test_gas.go @@ -3,8 +3,6 @@ package vm import ( "math/big" - "github.com/ethereum/go-ethereum/params" - "github.com/cosmos/evm/testutil/integration/evm/factory" "github.com/cosmos/evm/testutil/integration/evm/grpc" "github.com/cosmos/evm/testutil/integration/evm/network" @@ -143,7 +141,7 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { }, { name: "Refund with context fees, no refund", - leftoverGas: 1, + leftoverGas: 0, malleate: func(ctx sdk.Context) sdk.Context { // Set the fee abstraction paid fee key with a single coin return ctx.WithValue( @@ -159,7 +157,7 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { }, { name: "Error - More than one coin being passed", - leftoverGas: DefaultCoreMsgGasUsage - 1, // Using some leftover so the refund doesn't short circuit + leftoverGas: DefaultCoreMsgGasUsage, malleate: func(ctx sdk.Context) sdk.Context { // Set the fee abstraction paid fee key with a single coin return ctx.WithValue( @@ -185,8 +183,8 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { ctx = tc.malleate(ctx) } - vmdb := unitNetwork.GetStateDB() - vmdb.AddRefund(params.TxGas) + // vmdb := unitNetwork.GetStateDB() + // vmdb.AddRefund(DefaultCoreMsgGasUsage) if tc.leftoverGas > DefaultCoreMsgGasUsage { return From 63cf7fbc1145682bad55c094186a684df2616f55 Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 22 Dec 2025 16:20:26 -0300 Subject: [PATCH 12/17] fix: remove uneeded comment --- tests/integration/x/vm/test_gas.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/integration/x/vm/test_gas.go b/tests/integration/x/vm/test_gas.go index 156455f9..a3e56362 100644 --- a/tests/integration/x/vm/test_gas.go +++ b/tests/integration/x/vm/test_gas.go @@ -183,9 +183,6 @@ func (suite *KeeperTestSuite) TestGasRefundGas() { ctx = tc.malleate(ctx) } - // vmdb := unitNetwork.GetStateDB() - // vmdb.AddRefund(DefaultCoreMsgGasUsage) - if tc.leftoverGas > DefaultCoreMsgGasUsage { return } From 4dfb97e18937deba4efa58d73545da9f36d0dc3c Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 2 Feb 2026 10:51:45 -0300 Subject: [PATCH 13/17] feat: fork mempool broadcast fix --- evmd/app.go | 5 -- evmd/mempool.go | 1 - mempool/check_tx.go | 7 +++ mempool/mempool.go | 65 ++++++++++++++++--------- server/start.go | 16 +++--- tests/integration/mempool/test_setup.go | 4 ++ 6 files changed, 61 insertions(+), 37 deletions(-) diff --git a/evmd/app.go b/evmd/app.go index 43dfe3e7..b65a1546 100644 --- a/evmd/app.go +++ b/evmd/app.go @@ -157,7 +157,6 @@ type EVMD struct { appCodec codec.Codec interfaceRegistry types.InterfaceRegistry txConfig client.TxConfig - clientCtx client.Context pendingTxListeners []evmante.PendingTxListener @@ -1072,10 +1071,6 @@ func (app *EVMD) GetTxConfig() client.TxConfig { return app.txConfig } -func (app *EVMD) SetClientCtx(clientCtx client.Context) { // TODO:VLAD - Remove this if possible - app.clientCtx = clientCtx -} - // Close unsubscribes from the CometBFT event bus (if set) and closes the mempool and underlying BaseApp. func (app *EVMD) Close() error { var err error diff --git a/evmd/mempool.go b/evmd/mempool.go index 7ed654b7..f1e28c0f 100644 --- a/evmd/mempool.go +++ b/evmd/mempool.go @@ -38,7 +38,6 @@ func (app *EVMD) configureEVMMempool(appOpts servertypes.AppOptions, logger log. app.EVMKeeper, app.FeeMarketKeeper, app.txConfig, - app.clientCtx, mempoolConfig, cosmosPoolMaxTx, ) diff --git a/mempool/check_tx.go b/mempool/check_tx.go index f42a024f..33eeb12a 100644 --- a/mempool/check_tx.go +++ b/mempool/check_tx.go @@ -7,6 +7,7 @@ import ( "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/evm/mempool/txpool" ) // NewCheckTxHandler creates a CheckTx handler that integrates with the EVM mempool for transaction validation. @@ -28,6 +29,12 @@ func NewCheckTxHandler(mempool *ExperimentalEVMMempool) types.CheckTxHandler { // anything else, return regular error return sdkerrors.ResponseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, false), nil } + // If its already known, this can mean the the tx was promoted from nonce gap to valid + // and by allowing ErrAlreadyKnown to be silent, we allow re-gossiping of such txs + // this also covers the case of re-submission of the same tx enforcing overpricing for replacement + if errors.Is(err, txpool.ErrAlreadyKnown) { + return sdkerrors.ResponseCheckTxWithEvents(nil, gInfo.GasWanted, gInfo.GasUsed, anteEvents, false), nil + } return &abci.ResponseCheckTx{ GasWanted: int64(gInfo.GasWanted), // #nosec G115 -- this is copied from the Cosmos SDK diff --git a/mempool/mempool.go b/mempool/mempool.go index 532631ed..b85bb59c 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -28,6 +28,11 @@ import ( var _ sdkmempool.ExtMempool = &ExperimentalEVMMempool{} +// AllowUnsafeSyncInsert indicates whether to perform synchronous inserts into the mempool +// for testing purposes. When true, Insert will block until the transaction is fully processed. +// This should be used only in tests to ensure deterministic behavior +var AllowUnsafeSyncInsert = false + const ( // SubscriberName is the name of the event bus subscriber for the EVM mempool SubscriberName = "evm" @@ -53,6 +58,7 @@ type ( logger log.Logger txConfig client.TxConfig blockchain *Blockchain + clientCtx client.Context blockGasLimit uint64 // Block gas limit from consensus parameters minTip *uint256.Int @@ -88,7 +94,6 @@ func NewExperimentalEVMMempool( vmKeeper VMKeeperI, feeMarketKeeper FeeMarketKeeperI, txConfig client.TxConfig, - clientCtx client.Context, config *EVMMempoolConfig, cosmosPoolMaxTx int, ) *ExperimentalEVMMempool { @@ -121,19 +126,6 @@ func NewExperimentalEVMMempool( legacyPool := legacypool.New(legacyConfig, blockchain) - // Set up broadcast function using clientCtx - if config.BroadCastTxFn != nil { - legacyPool.BroadcastTxFn = config.BroadCastTxFn - } else { - // Create default broadcast function using clientCtx. - // The EVM mempool will broadcast transactions when it promotes them - // from queued into pending, noting their readiness to be executed. - legacyPool.BroadcastTxFn = func(txs []*ethtypes.Transaction) error { - logger.Debug("broadcasting EVM transactions", "tx_count", len(txs)) - return broadcastEVMTransactions(clientCtx, txConfig, txs) - } - } - txPool, err := txpool.New(uint64(0), blockchain, []txpool.SubPool{legacyPool}) if err != nil { panic(err) @@ -179,6 +171,7 @@ func NewExperimentalEVMMempool( cosmosPoolConfig.MaxTx = cosmosPoolMaxTx cosmosPool = sdkmempool.NewPriorityMempool(*cosmosPoolConfig) + // Create the evmMempool evmMempool := &ExperimentalEVMMempool{ vmKeeper: vmKeeper, txPool: txPool, @@ -192,6 +185,16 @@ func NewExperimentalEVMMempool( anteHandler: config.AnteHandler, } + // Set up broadcast function + if config.BroadCastTxFn != nil { + legacyPool.BroadcastTxFn = config.BroadCastTxFn + } else { + // Create default broadcast function using clientCtx. + // The EVM mempool will broadcast transactions when it promotes them + // from queued into pending, noting their readiness to be executed. + legacyPool.BroadcastTxFn = evmMempool.defaultBroadcastTxFn + } + vmKeeper.SetEvmMempool(evmMempool) return evmMempool @@ -209,6 +212,11 @@ func (m *ExperimentalEVMMempool) GetTxPool() *txpool.TxPool { return m.txPool } +// SetClientCtx sets the client context provider for broadcasting transactions +func (m *ExperimentalEVMMempool) SetClientCtx(clientCtx client.Context) { + m.clientCtx = clientCtx +} + // Insert adds a transaction to the appropriate mempool (EVM or Cosmos). // EVM transactions are routed to the EVM transaction pool, while all other // transactions are inserted into the Cosmos sdkmempool. The method assumes @@ -227,7 +235,7 @@ func (m *ExperimentalEVMMempool) Insert(goCtx context.Context, tx sdk.Tx) error hash := ethMsg.Hash() m.logger.Debug("inserting EVM transaction", "tx_hash", hash) ethTxs := []*ethtypes.Transaction{ethMsg.AsTransaction()} - errs := m.txPool.Add(ethTxs, true) + errs := m.txPool.Add(ethTxs, AllowUnsafeSyncInsert) if len(errs) > 0 && errs[0] != nil { m.logger.Error("failed to insert EVM transaction", "error", errs[0], "tx_hash", hash) return errs[0] @@ -477,20 +485,33 @@ func (m *ExperimentalEVMMempool) getIterators(goCtx context.Context, i [][]byte) return orderedEVMPendingTxes, cosmosPendingTxes } +// defaultBroadcastTxFn is the default function for broadcasting EVM transactions +// using the configured client context +func (m *ExperimentalEVMMempool) defaultBroadcastTxFn(txs []*ethtypes.Transaction) error { + m.logger.Debug("broadcasting EVM transactions", "tx_count", len(txs)) + + // Apply the broadcast EVM transactions using the client context + return broadcastEVMTransactions(m.clientCtx, txs) +} + // broadcastEVMTransactions converts Ethereum transactions to Cosmos SDK format and broadcasts them. // This function wraps EVM transactions in MsgEthereumTx messages and submits them to the network // using the provided client context. It handles encoding and error reporting for each transaction. -func broadcastEVMTransactions(clientCtx client.Context, txConfig client.TxConfig, ethTxs []*ethtypes.Transaction) error { +func broadcastEVMTransactions(clientCtx client.Context, ethTxs []*ethtypes.Transaction) error { for _, ethTx := range ethTxs { msg := &evmtypes.MsgEthereumTx{} - msg.FromEthereumTx(ethTx) - txBuilder := txConfig.NewTxBuilder() - if err := txBuilder.SetMsgs(msg); err != nil { - return fmt.Errorf("failed to set msg in tx builder: %w", err) + ethSigner := ethtypes.LatestSigner(evmtypes.GetEthChainConfig()) + if err := msg.FromSignedEthereumTx(ethTx, ethSigner); err != nil { + return fmt.Errorf("failed to convert ethereum transaction: %w", err) + } + + cosmosTx, err := msg.BuildTx(clientCtx.TxConfig.NewTxBuilder(), evmtypes.GetEVMCoinDenom()) + if err != nil { + return fmt.Errorf("failed to build cosmos tx: %w", err) } - txBytes, err := txConfig.TxEncoder()(txBuilder.GetTx()) + txBytes, err := clientCtx.TxConfig.TxEncoder()(cosmosTx) if err != nil { return fmt.Errorf("failed to encode transaction: %w", err) } @@ -499,7 +520,7 @@ func broadcastEVMTransactions(clientCtx client.Context, txConfig client.TxConfig if err != nil { return fmt.Errorf("failed to broadcast transaction %s: %w", ethTx.Hash().Hex(), err) } - if res.Code != 0 { + if res.Code != 0 && res.Code != 19 && res.RawLog != "already known" { return fmt.Errorf("transaction %s rejected by mempool: code=%d, log=%s", ethTx.Hash().Hex(), res.Code, res.RawLog) } } diff --git a/server/start.go b/server/start.go index 72577885..670cb65c 100644 --- a/server/start.go +++ b/server/start.go @@ -61,7 +61,6 @@ type Application interface { types.Application AppWithPendingTxStream GetMempool() sdkmempool.ExtMempool - SetClientCtx(clientCtx client.Context) } // AppCreator is a function that allows us to lazily initialize an application implementing with AppWithPendingTxStream. @@ -137,7 +136,7 @@ which accepts a path for the resulting pprof file. if !withbft { serverCtx.Logger.Info("starting ABCI without CometBFT") return wrapCPUProfile(serverCtx, func() error { - return startStandAlone(serverCtx, clientCtx, opts) + return startStandAlone(serverCtx, opts) }) } @@ -247,7 +246,7 @@ which accepts a path for the resulting pprof file. // Parameters: // - svrCtx: The context object that holds server configurations, logger, and other stateful information. // - opts: Options for starting the server, including functions for creating the application and opening the database. -func startStandAlone(svrCtx *server.Context, clientCtx client.Context, opts StartOptions) error { +func startStandAlone(svrCtx *server.Context, opts StartOptions) error { addr := svrCtx.Viper.GetString(srvflags.Address) transport := svrCtx.Viper.GetString(srvflags.Transport) home := svrCtx.Viper.GetString(flags.FlagHome) @@ -278,11 +277,6 @@ func startStandAlone(svrCtx *server.Context, clientCtx client.Context, opts Star svrCtx.Logger.Error("close application failed", "error", err.Error()) } }() - evmApp, ok := app.(Application) - if !ok { - svrCtx.Logger.Error("failed to get server config", "error", err.Error()) - } - evmApp.SetClientCtx(clientCtx) config, err := cosmosevmserverconfig.GetConfig(svrCtx.Viper) if err != nil { @@ -401,7 +395,6 @@ func startInProcess(svrCtx *server.Context, clientCtx client.Context, opts Start if !ok { svrCtx.Logger.Error("failed to get server config", "error", err.Error()) } - evmApp.SetClientCtx(clientCtx) nodeKey, err := p2p.LoadOrGenNodeKey(cfg.NodeKeyFile()) if err != nil { @@ -463,6 +456,11 @@ func startInProcess(svrCtx *server.Context, clientCtx client.Context, opts Start app.RegisterTxService(clientCtx) app.RegisterTendermintService(clientCtx) app.RegisterNodeService(clientCtx, config.Config) + + // Set the clientCtx into the mempool + if m, ok := evmApp.GetMempool().(*evmmempool.ExperimentalEVMMempool); ok && m != nil { + m.SetClientCtx(clientCtx) + } } metrics, err := startTelemetry(config) diff --git a/tests/integration/mempool/test_setup.go b/tests/integration/mempool/test_setup.go index ae80aaee..d1e3b418 100644 --- a/tests/integration/mempool/test_setup.go +++ b/tests/integration/mempool/test_setup.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/suite" + evmmempool "github.com/cosmos/evm/mempool" testconstants "github.com/cosmos/evm/testutil/constants" "github.com/cosmos/evm/testutil/integration/evm/factory" "github.com/cosmos/evm/testutil/integration/evm/grpc" @@ -80,6 +81,9 @@ func (s *IntegrationTestSuite) SetupTestWithChainID(chainID testconstants.ChainI initialCount := mempool.CountTx() s.Require().Equal(0, initialCount, "mempool should be empty initially") + // Enforces deterministic mempool state for tests + evmmempool.AllowUnsafeSyncInsert = true + s.network = nw s.factory = tf } From f03b3bf9cb7ebb4602cfe627b6619edf1cfb3963 Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 2 Feb 2026 11:53:53 -0300 Subject: [PATCH 14/17] fix: fork duplicate broadcast fix --- mempool/mempool.go | 11 + rpc/backend/call_tx.go | 7 + tests/systemtests/main_test.go | 2 +- tests/systemtests/mempool/test_broadcast.go | 403 +++++++++++++++++++ tests/systemtests/mempool/test_exceptions.go | 79 ---- tests/systemtests/suite/test_suite.go | 77 ++++ 6 files changed, 499 insertions(+), 80 deletions(-) create mode 100644 tests/systemtests/mempool/test_broadcast.go diff --git a/mempool/mempool.go b/mempool/mempool.go index b85bb59c..19e7ca52 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -6,6 +6,7 @@ import ( "fmt" "sync" + "github.com/ethereum/go-ethereum/common" ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/holiman/uint256" @@ -424,6 +425,16 @@ func (m *ExperimentalEVMMempool) HasEventBus() bool { return m.eventBus != nil } +// Has returns true if the transaction with the given hash is already in the mempool. +// This checks tx pool for EVM transactions, which iterates through all pools (currently only legacypool) +func (m *ExperimentalEVMMempool) Has(hash common.Hash) bool { + m.mtx.Lock() + defer m.mtx.Unlock() + + // Check the tx pool + return m.txPool.Has(hash) +} + // Close unsubscribes from the CometBFT event bus and shuts down the mempool. func (m *ExperimentalEVMMempool) Close() error { var errs []error diff --git a/rpc/backend/call_tx.go b/rpc/backend/call_tx.go index 217830ff..0425f27c 100644 --- a/rpc/backend/call_tx.go +++ b/rpc/backend/call_tx.go @@ -17,6 +17,7 @@ import ( "google.golang.org/grpc/status" "github.com/cosmos/evm/mempool" + "github.com/cosmos/evm/mempool/txpool" rpctypes "github.com/cosmos/evm/rpc/types" evmtypes "github.com/cosmos/evm/x/vm/types" @@ -148,6 +149,12 @@ func (b *Backend) SendRawTransaction(data hexutil.Bytes) (common.Hash, error) { txHash := ethereumTx.AsTransaction().Hash() + // Check if transaction is already in the mempool before broadcasting + // This is important for user-submitted transactions via JSON-RPC to provide proper error feedback + if b.Mempool != nil && b.Mempool.Has(txHash) { + return txHash, txpool.ErrAlreadyKnown + } + syncCtx := b.ClientCtx.WithBroadcastMode(flags.BroadcastSync) rsp, err := syncCtx.BroadcastTx(txBytes) if rsp != nil && rsp.Code != 0 { diff --git a/tests/systemtests/main_test.go b/tests/systemtests/main_test.go index caabc9e3..9f3aaa36 100644 --- a/tests/systemtests/main_test.go +++ b/tests/systemtests/main_test.go @@ -33,7 +33,7 @@ func TestTxsReplacement(t *testing.T) { } func TestExceptions(t *testing.T) { - mempool.TestTxRebroadcasting(t) + mempool.TestRunTxBroadcasting(t) mempool.TestMinimumGasPricesZero(t) } diff --git a/tests/systemtests/mempool/test_broadcast.go b/tests/systemtests/mempool/test_broadcast.go new file mode 100644 index 00000000..87582809 --- /dev/null +++ b/tests/systemtests/mempool/test_broadcast.go @@ -0,0 +1,403 @@ +package mempool + +import ( + "context" + "fmt" + "slices" + "testing" + "time" + + "github.com/test-go/testify/require" + + "github.com/cosmos/evm/tests/systemtests/suite" +) + +// TestRunTxBroadcasting tests transaction broadcasting and duplicate handling in a multi-node network. +// +// This test verifies two critical aspects of transaction propagation: +// +// 1. Mempool Broadcasting: Transactions submitted to one node are gossiped to all other nodes +// via the mempool gossip protocol BEFORE blocks are produced. +// +// 2. Duplicate Detection: The RPC layer properly rejects duplicate transactions submitted +// by users via JSON-RPC (returning txpool.ErrAlreadyKnown), while internal gossip remains silent. +// +// The test uses a 5-second consensus timeout to create a larger window for verifying that +// transactions appear in other nodes' mempools before blocks are committed. +func TestRunTxBroadcasting(t *testing.T) { + testCases := []struct { + name string + actions []func(TestSuite) + }{ + { + // Scenario 1: Basic Broadcasting and Transaction Promotion + // + // This scenario verifies that: + // 1. Transactions are gossiped to all nodes BEFORE blocks are committed + // 2. Queued transactions (with nonce gaps) are NOT gossiped + // 3. When gaps are filled, queued txs are promoted and then gossiped + // + // Broadcasting Flow: + // User -> JSON-RPC -> Mempool (pending) -> Gossip to peers + // When nonce gap filled: Queued -> Promoted to pending -> Gossiped + name: "tx broadcast to other nodes %s", + actions: []func(TestSuite){ + func(s TestSuite) { + // Step 1: Send tx with nonce 0 to node0 + // Expected: tx is added to node0's pending pool (nonce is correct) + + // Send transaction to node0 + tx1, err := s.SendTx(t, s.Node(0), "acc0", 0, s.GetTxGasPrice(s.BaseFee()), nil) + require.NoError(t, err, "failed to send tx to node0") + + // Step 2: Verify tx appears in nodes 1, 2, 3 mempools within 3 seconds + // Expected: tx is gossiped to all nodes BEFORE any block is committed (5s timeout) + // This proves mempool gossip works, not just block propagation + maxWaitTime := 3 * time.Second + checkInterval := 100 * time.Millisecond + + for _, nodeIdx := range []int{1, 2, 3} { + func(nodeIdx int) { + nodeID := s.Node(nodeIdx) + found := false + + timeoutCtx, cancel := context.WithTimeout(context.Background(), maxWaitTime) + defer cancel() + + ticker := time.NewTicker(checkInterval) + defer ticker.Stop() + + for !found { + select { + case <-timeoutCtx.Done(): + require.FailNow(t, fmt.Sprintf( + "transaction %s was not broadcast to %s within %s - mempool gossip may not be working", + tx1.TxHash, nodeID, maxWaitTime, + )) + case <-ticker.C: + pendingTxs, _, err := s.TxPoolContent(nodeID, suite.TxTypeEVM) + if err != nil { + // Retry on error + continue + } + + if slices.Contains(pendingTxs, tx1.TxHash) { + t.Logf("✓ Transaction %s successfully broadcast to %s", tx1.TxHash, nodeID) + found = true + } + } + } + }(nodeIdx) + } + + // Now set expected state and let the transaction commit normally + s.SetExpPendingTxs(tx1) + }, + func(s TestSuite) { + // Step 3: Send tx with nonce 2 to node1 (creating a nonce gap) + // Current nonce is 1 (after previous tx), so nonce 2 creates a gap + // Expected: tx is added to node1's QUEUED pool (not pending due to gap) + + // Send tx with nonce +2 to node1 (creating a gap since current nonce is 1) + tx3, err := s.SendTx(t, s.Node(1), "acc0", 2, s.GetTxGasPrice(s.BaseFee()), nil) + require.NoError(t, err, "failed to send tx with nonce 2") + + // Step 4: Verify tx is in node1's QUEUED pool (not pending) + // Queued txs cannot execute until the nonce gap is filled + maxWaitTime := 2 * time.Second + checkInterval := 100 * time.Millisecond + + timeoutCtx, cancel := context.WithTimeout(context.Background(), maxWaitTime) + defer cancel() + + ticker := time.NewTicker(checkInterval) + defer ticker.Stop() + + queuedOnNode1 := false + for !queuedOnNode1 { + select { + case <-timeoutCtx.Done(): + require.FailNow(t, fmt.Sprintf( + "transaction %s was not queued on node1 within %s", + tx3.TxHash, maxWaitTime, + )) + case <-ticker.C: + _, queuedTxs, err := s.TxPoolContent(s.Node(1), suite.TxTypeEVM) + if err != nil { + continue + } + + if slices.Contains(queuedTxs, tx3.TxHash) { + t.Logf("✓ Transaction %s is queued on node1 (as expected due to nonce gap)", tx3.TxHash) + queuedOnNode1 = true + } + } + } + + // Step 5: Verify queued tx is NOT gossiped to other nodes + // Queued txs should stay local until they become pending (executable) + // This prevents network spam from non-executable transactions + time.Sleep(1 * time.Second) // Give some time for any potential gossip + + for _, nodeIdx := range []int{0, 2, 3} { + nodeID := s.Node(nodeIdx) + pendingTxs, queuedTxs, err := s.TxPoolContent(nodeID, suite.TxTypeEVM) + require.NoError(t, err, "failed to get txpool content from %s", nodeID) + + require.NotContains(t, pendingTxs, tx3.TxHash, + "queued transaction should not be in pending pool of %s", nodeID) + require.NotContains(t, queuedTxs, tx3.TxHash, + "queued transaction should not be broadcast to %s", nodeID) + } + + // Step 6: Send tx with nonce +1 to node2 (filling the gap) + // Expected: tx is added to node2's pending pool and gossiped + tx2, err := s.SendTx(t, s.Node(2), "acc0", 1, s.GetTxGasPrice(s.BaseFee()), nil) + require.NoError(t, err, "failed to send tx with nonce 1") + + // Step 7: Verify BOTH txs appear in all nodes' pending pools + // - tx2 (nonce=1) should be gossiped immediately (it's pending) + // - tx3 (nonce=2) should be promoted from queued to pending on node1 + // - Promoted tx3 should then be gossiped to all other nodes + // This proves queued txs get rebroadcast when promoted + maxWaitTime = 3 * time.Second + ticker2 := time.NewTicker(checkInterval) + defer ticker2.Stop() + + for _, nodeIdx := range []int{0, 1, 3} { + func(nodeIdx int) { + nodeID := s.Node(nodeIdx) + foundTx2 := false + foundTx3 := false + + timeoutCtx2, cancel2 := context.WithTimeout(context.Background(), maxWaitTime) + defer cancel2() + + for !foundTx2 || !foundTx3 { + select { + case <-timeoutCtx2.Done(): + if !foundTx2 { + require.FailNow(t, fmt.Sprintf( + "transaction %s was not broadcast to %s within %s", + tx2.TxHash, nodeID, maxWaitTime, + )) + } + if !foundTx3 { + require.FailNow(t, fmt.Sprintf( + "transaction %s (promoted from queued) was not broadcast to %s within %s", + tx3.TxHash, nodeID, maxWaitTime, + )) + } + case <-ticker2.C: + pendingTxs, _, err := s.TxPoolContent(nodeID, suite.TxTypeEVM) + if err != nil { + continue + } + + if !foundTx2 && slices.Contains(pendingTxs, tx2.TxHash) { + t.Logf("✓ Transaction %s broadcast to %s", tx2.TxHash, nodeID) + foundTx2 = true + } + + if !foundTx3 && slices.Contains(pendingTxs, tx3.TxHash) { + t.Logf("✓ Transaction %s (promoted) broadcast to %s", tx3.TxHash, nodeID) + foundTx3 = true + } + } + } + }(nodeIdx) + } + + s.SetExpPendingTxs(tx2, tx3) + }, + }, + }, + { + // Scenario 2: Duplicate Detection on Same Node + // + // This scenario verifies that when a user submits the same transaction twice + // to the same node via JSON-RPC, the second submission returns an error. + // + // Error Handling: + // RPC Layer: Checks mempool.Has(txHash) -> returns txpool.ErrAlreadyKnown + // Users get immediate error feedback (not silent failure) + name: "duplicate tx rejected on same node %s", + actions: []func(TestSuite){ + func(s TestSuite) { + // Step 1: Send tx with the current nonce to node0 + // Expected: tx is accepted and added to pending pool + + // Send transaction to node0 + tx1, err := s.SendTx(t, s.Node(0), "acc0", 0, s.GetTxGasPrice(s.BaseFee()), nil) + require.NoError(t, err, "failed to send tx to node0") + + // Step 2: Verify tx is in node0's pending pool + // Poll for the transaction to appear (it should be fast, but we need to wait for async processing) + maxWaitTime := 3 * time.Second + checkInterval := 100 * time.Millisecond + + timeoutCtx, cancel := context.WithTimeout(context.Background(), maxWaitTime) + defer cancel() + + ticker := time.NewTicker(checkInterval) + defer ticker.Stop() + + found := false + for !found { + select { + case <-timeoutCtx.Done(): + require.FailNow(t, fmt.Sprintf( + "transaction %s was not found in node0's pending pool within %s", + tx1.TxHash, maxWaitTime, + )) + case <-ticker.C: + pendingTxs, _, err := s.TxPoolContent(s.Node(0), suite.TxTypeEVM) + if err != nil { + continue + } + if slices.Contains(pendingTxs, tx1.TxHash) { + found = true + } + } + } + + // Step 3: Send the SAME transaction again to node0 via JSON-RPC + // Expected: Error returned (txpool.ErrAlreadyKnown) + // Users must receive error feedback for duplicate submissions + _, err = s.SendTx(t, s.Node(0), "acc0", 0, s.GetTxGasPrice(s.BaseFee()), nil) + require.Error(t, err, "duplicate tx via JSON-RPC must return error") + require.Contains(t, err.Error(), "already known", "error should indicate transaction is already known") + + t.Logf("✓ Duplicate transaction correctly rejected with 'already known' error") + + s.SetExpPendingTxs(tx1) + }, + }, + }, + { + // Scenario 3: Duplicate Detection After Gossip + // + // This scenario verifies that even when a node receives a transaction via + // internal gossip (not user submission), attempting to submit that same + // transaction again via JSON-RPC returns an error. + // + // Flow: + // 1. User submits tx to node0 -> added to mempool -> gossiped to node1 + // 2. User tries to submit same tx to node1 via JSON-RPC + // 3. RPC layer detects duplicate (mempool.Has) and returns error + // + // This ensures duplicate detection works regardless of how the node + // originally received the transaction (user submission vs gossip). + name: "duplicate tx rejected after gossip %s", + actions: []func(TestSuite){ + func(s TestSuite) { + // Step 1: Send tx with the current nonce to node0 + // Expected: tx is accepted, added to pending, and gossiped + + // Send transaction to node0 + tx1, err := s.SendTx(t, s.Node(0), "acc0", 0, s.GetTxGasPrice(s.BaseFee()), nil) + require.NoError(t, err, "failed to send tx to node0") + + // Step 2: Wait for tx to be gossiped to node1 + // Expected: tx appears in node1's pending pool within 3 seconds + maxWaitTime := 3 * time.Second + checkInterval := 100 * time.Millisecond + + timeoutCtx, cancel := context.WithTimeout(context.Background(), maxWaitTime) + defer cancel() + + ticker := time.NewTicker(checkInterval) + defer ticker.Stop() + + found := false + for !found { + select { + case <-timeoutCtx.Done(): + require.FailNow(t, fmt.Sprintf( + "transaction %s was not broadcast to node1 within %s", + tx1.TxHash, maxWaitTime, + )) + case <-ticker.C: + pendingTxs, _, err := s.TxPoolContent(s.Node(1), suite.TxTypeEVM) + if err != nil { + continue + } + if slices.Contains(pendingTxs, tx1.TxHash) { + t.Logf("✓ Transaction %s broadcast to node1 via gossip", tx1.TxHash) + found = true + } + } + } + + // Step 3: Try to send the SAME transaction to node1 via JSON-RPC + // Expected: Error returned (txpool.ErrAlreadyKnown) + // Even though node1 received it via gossip (not user submission), + // the RPC layer should still detect and reject the duplicate + _, err = s.SendTx(t, s.Node(1), "acc0", 0, s.GetTxGasPrice(s.BaseFee()), nil) + require.Error(t, err, "duplicate tx via JSON-RPC should return error even after gossip") + require.Contains(t, err.Error(), "already known", "error should indicate transaction is already known") + + t.Logf("✓ JSON-RPC correctly rejects duplicate that node already has from gossip") + + s.SetExpPendingTxs(tx1) + }, + }, + }, + } + + testOptions := []*suite.TestOptions{ + { + Description: "EVM LegacyTx", + TxType: suite.TxTypeEVM, + IsDynamicFeeTx: false, + }, + { + Description: "EVM DynamicFeeTx", + TxType: suite.TxTypeEVM, + IsDynamicFeeTx: true, + }, + } + + s := suite.NewSystemTestSuite(t) + + // First, setup the chain with default configuration + s.SetupTest(t) + + // Now modify the consensus timeout to slow down block production + // This gives us time to verify broadcasting happens before blocks are committed + s.ModifyConsensusTimeout(t, "5s") + + for _, to := range testOptions { + s.SetOptions(to) + for _, tc := range testCases { + testName := fmt.Sprintf(tc.name, to.Description) + t.Run(testName, func(t *testing.T) { + // Await a block before starting the test case (ensures clean state) + s.AwaitNBlocks(t, 1) + + // Capture the initial block height - no blocks should be produced during the test case + initialHeight := s.GetCurrentBlockHeight(t, "node0") + t.Logf("Test case starting at block height %d", initialHeight) + + // Execute all test actions (broadcasting, mempool checks, etc.) + for _, action := range tc.actions { + action(s) + // NOTE: We don't call AfterEachAction here because we're manually + // checking the mempool state in the action functions + } + + // Verify no blocks were produced during the test case + // All broadcasting and mempool checks should happen within a single block period + currentHeight := s.GetCurrentBlockHeight(t, "node0") + require.Equal(t, initialHeight, currentHeight, + "No blocks should be produced during test case execution - expected height %d but got %d", + initialHeight, currentHeight) + t.Logf("✓ Test case completed at same block height %d (no blocks produced)", currentHeight) + + // Now await a block to allow transactions to commit + s.AwaitNBlocks(t, 1) + t.Logf("Awaited block for transaction commits") + }) + } + } +} diff --git a/tests/systemtests/mempool/test_exceptions.go b/tests/systemtests/mempool/test_exceptions.go index 3f38bba6..5f3e024c 100644 --- a/tests/systemtests/mempool/test_exceptions.go +++ b/tests/systemtests/mempool/test_exceptions.go @@ -8,85 +8,6 @@ import ( "github.com/test-go/testify/require" ) -func TestTxRebroadcasting(t *testing.T) { - testCases := []struct { - name string - actions []func(s TestSuite) - }{ - { - name: "ordering of pending txs %s", - actions: []func(s TestSuite){ - func(s TestSuite) { - tx1, err := s.SendTx(t, s.Node(0), "acc0", 0, s.GetTxGasPrice(s.BaseFee()), nil) - require.NoError(t, err, "failed to send tx") - - tx2, err := s.SendTx(t, s.Node(1), "acc0", 1, s.GetTxGasPrice(s.BaseFee()), nil) - require.NoError(t, err, "failed to send tx") - - tx3, err := s.SendTx(t, s.Node(2), "acc0", 2, s.GetTxGasPrice(s.BaseFee()), nil) - require.NoError(t, err, "failed to send tx") - - // Skip tx4 with nonce 3 - - tx5, err := s.SendTx(t, s.Node(3), "acc0", 4, s.GetTxGasPrice(s.BaseFee()), nil) - require.NoError(t, err, "failed to send tx") - - tx6, err := s.SendTx(t, s.Node(0), "acc0", 5, s.GetTxGasPrice(s.BaseFee()), nil) - require.NoError(t, err, "failed to send tx") - - // At AfterEachAction hook, we will check expected queued txs are not broadcasted. - s.SetExpPendingTxs(tx1, tx2, tx3) - s.SetExpQueuedTxs(tx5, tx6) - }, - func(s TestSuite) { - // Wait for 3 blocks. - // It is because tx1, tx2, tx3 are sent to different nodes, tx3 needs maximum 3 blocks to be committed. - // e.g. node3 is 1st proposer -> tx3 will tale 1 block to be committed. - // e.g. node3 is 3rd proposer -> tx3 will take 3 blocks to be committed. - s.AwaitNBlocks(t, 3) - - // current nonce is 3. - // so, we should set nonce idx to 0. - nonce3Idx := uint64(0) - - tx4, err := s.SendTx(t, s.Node(2), "acc0", nonce3Idx, s.GetTxGasPrice(s.BaseFee()), nil) - require.NoError(t, err, "failed to send tx") - - // At AfterEachAction hook, we will check expected pending txs are broadcasted. - s.SetExpPendingTxs(tx4) - s.PromoteExpTxs(2) - }, - }, - }, - } - - testOptions := []*suite.TestOptions{ - { - Description: "EVM LegacyTx", - TxType: suite.TxTypeEVM, - IsDynamicFeeTx: false, - }, - } - - s := suite.NewSystemTestSuite(t) - s.SetupTest(t) - - for _, to := range testOptions { - s.SetOptions(to) - for _, tc := range testCases { - testName := fmt.Sprintf(tc.name, to.Description) - t.Run(testName, func(t *testing.T) { - s.BeforeEachCase(t) - for _, action := range tc.actions { - action(s) - s.AfterEachAction(t) - } - s.AfterEachCase(t) - }) - } - } -} - func TestMinimumGasPricesZero(t *testing.T) { testCases := []struct { name string diff --git a/tests/systemtests/suite/test_suite.go b/tests/systemtests/suite/test_suite.go index 345c8522..35013f0f 100644 --- a/tests/systemtests/suite/test_suite.go +++ b/tests/systemtests/suite/test_suite.go @@ -1,11 +1,16 @@ package suite import ( + "fmt" "math/big" + "os" + "path/filepath" "testing" "time" "github.com/cosmos/evm/tests/systemtests/clients" + "github.com/creachadair/tomledit" + "github.com/creachadair/tomledit/parser" "github.com/stretchr/testify/require" "cosmossdk.io/systemtests" @@ -56,6 +61,15 @@ func (s *SystemTestSuite) SetupTest(t *testing.T, nodeStartArgs ...string) { s.AwaitNBlocks(t, 2) } +// GetCurrentBlockHeight returns the current block height from the specified node +func (s *SystemTestSuite) GetCurrentBlockHeight(t *testing.T, nodeID string) uint64 { + t.Helper() + ctx, cli, _ := s.EthClient.Setup(nodeID, "acc0") + blockNumber, err := cli.BlockNumber(ctx) + require.NoError(t, err, "failed to get block number from %s", nodeID) + return blockNumber +} + // BeforeEach resets the expected mempool state and retrieves the current base fee before each test case func (s *SystemTestSuite) BeforeEachCase(t *testing.T) { // Reset expected pending/queued transactions @@ -116,3 +130,66 @@ func (s *SystemTestSuite) AfterEachCase(t *testing.T) { // Wait for block commit s.AwaitNBlocks(t, 1) } + +// ModifyConsensusTimeout modifies the consensus timeout_commit in the config.toml +// for all nodes and restarts the chain with the new configuration. +func (s *SystemTestSuite) ModifyConsensusTimeout(t *testing.T, timeout string, nodeStartArgs ...string) { + t.Helper() + + // Stop the chain if running + if s.ChainStarted { + s.ResetChain(t) + } + + // Modify config.toml for each node + for i := 0; i < s.NodesCount(); i++ { + nodeDir := s.NodeDir(i) + configPath := filepath.Join(nodeDir, "config", "config.toml") + + err := editToml(configPath, func(doc *tomledit.Document) { + setValue(doc, timeout, "consensus", "timeout_commit") + }) + require.NoError(t, err, "failed to modify config.toml for node %d", i) + } + + // Restart the chain with modified config + s.StartChain(t, nodeStartArgs...) + s.AwaitNBlocks(t, 2) +} + +// editToml is a helper to edit TOML files +func editToml(filename string, f func(doc *tomledit.Document)) error { + tomlFile, err := os.OpenFile(filename, os.O_RDWR, 0o600) + if err != nil { + return fmt.Errorf("failed to open file: %w", err) + } + defer tomlFile.Close() + + doc, err := tomledit.Parse(tomlFile) + if err != nil { + return fmt.Errorf("failed to parse toml: %w", err) + } + + f(doc) + + if _, err := tomlFile.Seek(0, 0); err != nil { + return fmt.Errorf("failed to seek: %w", err) + } + if err := tomlFile.Truncate(0); err != nil { + return fmt.Errorf("failed to truncate: %w", err) + } + if err := tomledit.Format(tomlFile, doc); err != nil { + return fmt.Errorf("failed to format: %w", err) + } + + return nil +} + +// setValue sets a value in a TOML document +func setValue(doc *tomledit.Document, newVal string, xpath ...string) { + e := doc.First(xpath...) + if e == nil { + panic(fmt.Sprintf("not found: %v", xpath)) + } + e.Value = parser.MustValue(fmt.Sprintf("%q", newVal)) +} From 4b70cb19512402a55eea9a25b7893386b79db607 Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 2 Feb 2026 16:21:01 -0300 Subject: [PATCH 15/17] fix: ledger nano x on linus --- wallets/usbwallet/hub.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/wallets/usbwallet/hub.go b/wallets/usbwallet/hub.go index c9c6f112..f464ca67 100644 --- a/wallets/usbwallet/hub.go +++ b/wallets/usbwallet/hub.go @@ -152,7 +152,11 @@ func (hub *Hub) refreshWallets() { for _, info := range infos { for _, id := range hub.productIDs { // Windows and macOS use UsageID matching, Linux uses Interface matching - if info.ProductID == id && (info.UsagePage == hub.usageID || info.Interface == hub.endpointID) { + // Ledger product IDs use MMII format where MM is device model and II is interface flags. + // Match either exact legacy IDs (0x0001, 0x0004, etc.) or device model prefix for + // WebUSB/app-specific IDs (e.g., 0x4011 matches 0x4000 prefix for Nano X with Ethereum app). + modelMatch := id >= 0x1000 && (info.ProductID>>8 == id>>8) + if (info.ProductID == id || modelMatch) && (info.UsagePage == hub.usageID || info.Interface == hub.endpointID) { devices = append(devices, info) break } From 6c071362c8bec32b3533b9cc798a7ff302ec852d Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Mon, 2 Feb 2026 16:21:23 -0300 Subject: [PATCH 16/17] fix encoding using default chain id --- ethereum/eip712/encoding.go | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index c4cfa989..df9aae99 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -3,6 +3,7 @@ package eip712 import ( "errors" "fmt" + "strconv" apitypes "github.com/ethereum/go-ethereum/signer/core/apitypes" @@ -101,8 +102,17 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { return apitypes.TypedData{}, err } + // Extract the chain ID from the sign doc itself for EIP-712 domain + // This ensures we use the same chain ID that was used during signing + chainID := eip155ChainID + if aminoDoc.ChainID != "" { + if parsedChainID, err := parseChainID(aminoDoc.ChainID); err == nil { + chainID = parsedChainID + } + } + typedData, err := WrapTxToTypedData( - eip155ChainID, + chainID, signDocBytes, ) if err != nil { @@ -176,8 +186,17 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { body.Memo, ) + // Extract the chain ID from the sign doc itself for EIP-712 domain + // This ensures we use the same chain ID that was used during signing + chainID := eip155ChainID + if signDoc.ChainId != "" { + if parsedChainID, err := parseChainID(signDoc.ChainId); err == nil { + chainID = parsedChainID + } + } + typedData, err := WrapTxToTypedData( - eip155ChainID, + chainID, signBytes, ) if err != nil { @@ -227,3 +246,9 @@ func validatePayloadMessages(msgs []sdk.Msg) error { return nil } + +// parseChainID attempts to parse the chain ID string as a uint64. +// The chain ID in the sign doc should be the EIP-155 chain ID. +func parseChainID(chainIDStr string) (uint64, error) { + return strconv.ParseUint(chainIDStr, 10, 64) +} From 329ec938e0268410f5c0a6c7a7748ff548b3c94c Mon Sep 17 00:00:00 2001 From: Thales Zirbel Date: Wed, 4 Feb 2026 09:13:55 -0300 Subject: [PATCH 17/17] fix: correctly ident error check and return --- mempool/check_tx.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/mempool/check_tx.go b/mempool/check_tx.go index 33eeb12a..0700a212 100644 --- a/mempool/check_tx.go +++ b/mempool/check_tx.go @@ -26,15 +26,16 @@ func NewCheckTxHandler(mempool *ExperimentalEVMMempool) types.CheckTxHandler { return sdkerrors.ResponseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, false), nil } } + // If its already known, this can mean the the tx was promoted from nonce gap to valid + // and by allowing ErrAlreadyKnown to be silent, we allow re-gossiping of such txs + // this also covers the case of re-submission of the same tx enforcing overpricing for replacement + if errors.Is(err, txpool.ErrAlreadyKnown) { + return sdkerrors.ResponseCheckTxWithEvents(nil, gInfo.GasWanted, gInfo.GasUsed, anteEvents, false), nil + } + // anything else, return regular error return sdkerrors.ResponseCheckTxWithEvents(err, gInfo.GasWanted, gInfo.GasUsed, anteEvents, false), nil } - // If its already known, this can mean the the tx was promoted from nonce gap to valid - // and by allowing ErrAlreadyKnown to be silent, we allow re-gossiping of such txs - // this also covers the case of re-submission of the same tx enforcing overpricing for replacement - if errors.Is(err, txpool.ErrAlreadyKnown) { - return sdkerrors.ResponseCheckTxWithEvents(nil, gInfo.GasWanted, gInfo.GasUsed, anteEvents, false), nil - } return &abci.ResponseCheckTx{ GasWanted: int64(gInfo.GasWanted), // #nosec G115 -- this is copied from the Cosmos SDK