From 704b246edb0dd0bb03b0215e46be0307193d1d3a Mon Sep 17 00:00:00 2001 From: Nilesh Gupta Date: Wed, 26 Aug 2026 06:14:30 +0530 Subject: [PATCH] fix: charge parent gas meter for failed EVM calls (F-2026-18824) CallEVMWithData and DerivedEVMCallWithData returned on res.Failed() before reaching ctx.GasMeter().ConsumeGas(res.GasUsed), so a reverting or deliberately out-of-gas internal call was free at the Cosmos meter. DerivedEVMCallWithData also clamps a caller-supplied gasLimit to config.DefaultGasCap: on an out-of-gas halt res.GasUsed equals the cap, so without the clamp the caller would choose how much gas the enclosing Cosmos tx is forced to consume and could panic it with OutOfGas. --- CHANGELOG.md | 9 + tests/integration/x/vm/test_call_evm.go | 54 ++++++ tests/integration/x/vm/test_derived_call.go | 188 ++++++++++++++++++++ x/vm/keeper/call_evm.go | 30 +++- 4 files changed, 276 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4064e1bbc..6fa9bc933 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,15 @@ the executor with its signature never checked, letting a victim-signed transaction be replayed as the victim. `VerifySender` is now required before `ApplyTransaction`, so `From` is always the ECDSA-recovered signer regardless of how the message arrived. +- Charge the parent Cosmos gas meter for failed EVM calls in `CallEVMWithData` and + `DerivedEVMCallWithData`. Both functions returned on `res.Failed()` before reaching + `ctx.GasMeter().ConsumeGas(res.GasUsed)`, so a reverting — or deliberately out-of-gas — + internal call performed real EVM work that cost the enclosing Cosmos transaction nothing + beyond the incidental KV-store gas. `DerivedEVMCallWithData` additionally clamps a + caller-supplied `gasLimit` to `config.DefaultGasCap`; on an out-of-gas halt `res.GasUsed` + equals the gas cap, so without the clamp the caller would pick how much gas the enclosing + transaction is forced to consume and could panic it with `OutOfGas`. `CallEVMWithData` + needs no clamp — its message gas limit is already the hardcoded `config.DefaultGasCap`. - [\#690](https://github.com/cosmos/evm/pull/690) Fix Ledger hardware wallet support for coin type 60. - [\#769](https://github.com/cosmos/evm/pull/769) Fix erc20 ibc middleware to not to validate sender address format. - [\#790](https://github.com/cosmos/evm/pull/790) fix panic in historical query due to missing EvmCoinInfo. diff --git a/tests/integration/x/vm/test_call_evm.go b/tests/integration/x/vm/test_call_evm.go index 013b200f2..787ef3dd3 100644 --- a/tests/integration/x/vm/test_call_evm.go +++ b/tests/integration/x/vm/test_call_evm.go @@ -10,6 +10,8 @@ import ( utiltx "github.com/cosmos/evm/testutil/tx" "github.com/cosmos/evm/x/erc20/types" evmtypes "github.com/cosmos/evm/x/vm/types" + + storetypes "cosmossdk.io/store/types" ) func (s *KeeperTestSuite) TestCallEVM() { @@ -148,3 +150,55 @@ func (s *KeeperTestSuite) TestCallEVMWithData() { }) } } + +// TestCallEVMWithDataFailedCallChargesParentGas is the CallEVMWithData half of +// F-2026-18824. Like DerivedEVMCallWithData, this function returned on +// res.Failed() before reaching ctx.GasMeter().ConsumeGas, so a reverting call +// performed real EVM work for free at the Cosmos meter. Unlike the derived path +// it needs no clamp: msg.GasLimit here is the hardcoded config.DefaultGasCap, so +// res.GasUsed is already bounded and the caller cannot inflate it. +func (s *KeeperTestSuite) TestCallEVMWithDataFailedCallChargesParentGas() { + s.SetupTest() + + from := s.Keyring.GetAddr(0) + ctx := s.Network.GetContext().WithGasMeter(storetypes.NewGasMeter(parentGasMeterLimit)) + keeper := s.Network.App.GetEVMKeeper() + + burner := s.deployRawCode(ctx, gasBurnerRevertCode) + gasBefore := ctx.GasMeter().GasConsumed() + + res, err := keeper.CallEVMWithData(ctx, from, &burner, nil, true, nil) + s.Require().Error(err, "the burner reverts, so the call must surface an error") + s.Require().NotNil(res) + s.Require().True(res.Failed()) + s.Require().Greater(res.GasUsed, uint64(2_000_000)) + + charged := ctx.GasMeter().GasConsumed() - gasBefore + s.Require().GreaterOrEqual(charged, res.GasUsed, + "a failed CallEVMWithData must charge its EVM gas to the parent Cosmos meter (F-2026-18824)") + s.Require().LessOrEqual(charged, res.GasUsed+1_000_000, + "res.GasUsed must be charged once, not doubled") +} + +// TestCallEVMWithDataSuccessChargesGasUsed pins the CallEVMWithData happy path: +// res.GasUsed is charged exactly once, unchanged by the failure-path fix. +func (s *KeeperTestSuite) TestCallEVMWithDataSuccessChargesGasUsed() { + s.SetupTest() + + from := s.Keyring.GetAddr(0) + ctx := s.Network.GetContext().WithGasMeter(storetypes.NewGasMeter(parentGasMeterLimit)) + keeper := s.Network.App.GetEVMKeeper() + + burner := s.deployRawCode(ctx, gasBurnerSuccessCode) + gasBefore := ctx.GasMeter().GasConsumed() + + res, err := keeper.CallEVMWithData(ctx, from, &burner, nil, true, nil) + s.Require().NoError(err) + s.Require().False(res.Failed()) + s.Require().Greater(res.GasUsed, uint64(2_000_000)) + + charged := ctx.GasMeter().GasConsumed() - gasBefore + s.Require().GreaterOrEqual(charged, res.GasUsed) + s.Require().LessOrEqual(charged, res.GasUsed+1_000_000, + "res.GasUsed must be charged exactly once on the success path") +} diff --git a/tests/integration/x/vm/test_derived_call.go b/tests/integration/x/vm/test_derived_call.go index b1d73f80a..5cacb679b 100644 --- a/tests/integration/x/vm/test_derived_call.go +++ b/tests/integration/x/vm/test_derived_call.go @@ -5,14 +5,21 @@ import ( "strconv" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/holiman/uint256" abcitypes "github.com/cometbft/cometbft/abci/types" "github.com/cosmos/evm/contracts" + "github.com/cosmos/evm/server/config" testconstants "github.com/cosmos/evm/testutil/constants" "github.com/cosmos/evm/testutil/integration/evm/utils" + utiltx "github.com/cosmos/evm/testutil/tx" + "github.com/cosmos/evm/x/vm/statedb" evmtypes "github.com/cosmos/evm/x/vm/types" + storetypes "cosmossdk.io/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -189,3 +196,184 @@ func (s *KeeperTestSuite) TestDerivedEVMCallWithDataRevertDoesNotMutateBloom() { s.Require().Equal(bloomBefore, keeper.GetBlockBloomTransient(ctx).Bytes(), "a reverted derived execution must not mutate the block bloom (F-2026-17738)") } + +// --------------------------------------------------------------------------- +// F-2026-18824: failed EVM calls must charge the parent Cosmos gas meter +// --------------------------------------------------------------------------- + +// gasBurnerRevertCode expands memory to 1 MiB and then REVERTs: +// +// PUSH1 0x00 // MSTORE value +// PUSH3 0x100000 // MSTORE offset -> forces a 1 MiB memory expansion +// MSTORE +// PUSH1 0x00 +// PUSH1 0x00 +// REVERT +// +// The memory expansion costs ~2.2M gas, deterministically, and REVERT hands the +// rest back — so res.GasUsed lands far above the incidental KV-store gas the +// call also draws, which is what makes it a usable signal in these tests. +var gasBurnerRevertCode = []byte{0x60, 0x00, 0x62, 0x10, 0x00, 0x00, 0x52, 0x60, 0x00, 0x60, 0x00, 0xfd} + +// gasBurnerSuccessCode is gasBurnerRevertCode with the trailing REVERT swapped +// for STOP: the same ~2.2M of memory expansion, but a successful execution. +var gasBurnerSuccessCode = []byte{0x60, 0x00, 0x62, 0x10, 0x00, 0x00, 0x52, 0x00} + +// invalidOpcodeCode is a single INVALID opcode. An exceptional halt burns every +// unit of gas handed to the frame, so res.GasUsed comes back equal to the gas +// cap — the same shape as running out of gas, without waiting for a loop to +// actually consume tens of millions of gas. +var invalidOpcodeCode = []byte{0xfe} + +// parentGasMeterLimit bounds the enclosing "Cosmos tx" in these tests. It sits +// above config.DefaultGasCap (25M) so a correctly clamped failure fits, and far +// below an unclamped caller-supplied limit so an unclamped charge panics. +const parentGasMeterLimit = 60_000_000 + +// deployRawCode installs raw runtime bytecode at a fresh address. +func (s *KeeperTestSuite) deployRawCode(ctx sdk.Context, code []byte) common.Address { + addr := utiltx.GenerateAddress() + codeHash := crypto.Keccak256Hash(code) + + k := s.Network.App.GetEVMKeeper() + k.SetCode(ctx, codeHash.Bytes(), code) + s.Require().NoError(k.SetAccount(ctx, addr, statedb.Account{ + Nonce: 1, + Balance: new(uint256.Int), + CodeHash: codeHash.Bytes(), + })) + return addr +} + +// TestDerivedEVMCallWithDataFailedCallChargesParentGas is the regression test for +// F-2026-18824. A reverting derived call used to return before the parent +// GasMeter().ConsumeGas, so the EVM work it had just performed was free at the +// Cosmos meter — only the incidental KV-store gas leaked through. The call now +// charges res.GasUsed on the failure path exactly as it does on success. +func (s *KeeperTestSuite) TestDerivedEVMCallWithDataFailedCallChargesParentGas() { + s.SetupTest() + + from := s.Keyring.GetAddr(0) + ctx := s.Network.GetContext().WithGasMeter(storetypes.NewGasMeter(parentGasMeterLimit)) + keeper := s.Network.App.GetEVMKeeper() + + burner := s.deployRawCode(ctx, gasBurnerRevertCode) + gasBefore := ctx.GasMeter().GasConsumed() + + res, err := keeper.DerivedEVMCallWithData( + ctx, from, &burner, nil, + true, // commit + false, // gasless + false, // isModuleSender + big.NewInt(0), big.NewInt(5_000_000), nil, + ) + s.Require().Error(err, "the burner reverts, so the call must surface an error") + s.Require().NotNil(res) + s.Require().True(res.Failed()) + s.Require().Greater(res.GasUsed, uint64(2_000_000), + "the burner must actually burn multi-million gas for this test to mean anything") + + charged := ctx.GasMeter().GasConsumed() - gasBefore + s.Require().GreaterOrEqual(charged, res.GasUsed, + "a failed derived call must charge its EVM gas to the parent Cosmos meter (F-2026-18824)") + s.Require().LessOrEqual(charged, res.GasUsed+1_000_000, + "res.GasUsed must be charged once, not doubled") +} + +// TestDerivedEVMCallWithDataGaslessFailureChargesParentGas covers the same +// failure path with gasless=true. The flag only zeroes the TxGasUsed event +// attribute; it never exempted the parent meter, and must not become an exemption +// now that failures are charged. +func (s *KeeperTestSuite) TestDerivedEVMCallWithDataGaslessFailureChargesParentGas() { + s.SetupTest() + + from := s.Keyring.GetAddr(0) + ctx := s.Network.GetContext().WithGasMeter(storetypes.NewGasMeter(parentGasMeterLimit)) + keeper := s.Network.App.GetEVMKeeper() + + burner := s.deployRawCode(ctx, gasBurnerRevertCode) + gasBefore := ctx.GasMeter().GasConsumed() + + res, err := keeper.DerivedEVMCallWithData( + ctx, from, &burner, nil, + true, // commit + true, // gasless + false, + big.NewInt(0), big.NewInt(5_000_000), nil, + ) + s.Require().Error(err) + s.Require().True(res.Failed()) + s.Require().GreaterOrEqual(ctx.GasMeter().GasConsumed()-gasBefore, res.GasUsed, + "gasless only zeroes the reported event attribute, not the parent meter") +} + +// TestDerivedEVMCallWithDataClampsCallerGasLimit is the safety half of +// F-2026-18824. Charging res.GasUsed on failure is only sound because the +// effective gas cap is clamped to config.DefaultGasCap: on an exceptional halt +// res.GasUsed equals the cap, so an unclamped caller-supplied gasLimit would let +// the caller choose how much gas the enclosing Cosmos tx is forced to consume and +// panic it with OutOfGas — which, on the MsgVoteInbound path, means a lost +// validator vote. Here the caller asks for 10x DefaultGasCap and the call halts +// on an INVALID opcode; the parent meter must absorb at most the clamped cap. +func (s *KeeperTestSuite) TestDerivedEVMCallWithDataClampsCallerGasLimit() { + s.SetupTest() + + from := s.Keyring.GetAddr(0) + ctx := s.Network.GetContext().WithGasMeter(storetypes.NewGasMeter(parentGasMeterLimit)) + keeper := s.Network.App.GetEVMKeeper() + + halter := s.deployRawCode(ctx, invalidOpcodeCode) + gasBefore := ctx.GasMeter().GasConsumed() + + //nolint:gosec // test-only constant, no overflow + callerGasLimit := big.NewInt(int64(config.DefaultGasCap) * 10) + + var res *evmtypes.MsgEthereumTxResponse + var err error + s.Require().NotPanics(func() { + res, err = keeper.DerivedEVMCallWithData( + ctx, from, &halter, nil, + true, false, false, + big.NewInt(0), callerGasLimit, nil, + ) + }, "a caller-supplied gasLimit above DefaultGasCap must not be able to panic the parent gas meter") + + s.Require().Error(err) + s.Require().True(res.Failed()) + s.Require().Equal(config.DefaultGasCap, res.GasUsed, + "an exceptional halt burns the whole cap, and the cap must be the clamped DefaultGasCap") + + charged := ctx.GasMeter().GasConsumed() - gasBefore + s.Require().GreaterOrEqual(charged, config.DefaultGasCap, + "the clamped cap is still charged to the parent meter") + s.Require().LessOrEqual(charged, config.DefaultGasCap+1_000_000, + "the parent meter must never be charged beyond the clamped cap") +} + +// TestDerivedEVMCallWithDataSuccessChargesGasUsed pins the happy path: a +// successful derived call charges res.GasUsed once, unchanged by the failure-path +// fix. +func (s *KeeperTestSuite) TestDerivedEVMCallWithDataSuccessChargesGasUsed() { + s.SetupTest() + + from := s.Keyring.GetAddr(0) + ctx := s.Network.GetContext().WithGasMeter(storetypes.NewGasMeter(parentGasMeterLimit)) + keeper := s.Network.App.GetEVMKeeper() + + burner := s.deployRawCode(ctx, gasBurnerSuccessCode) + gasBefore := ctx.GasMeter().GasConsumed() + + res, err := keeper.DerivedEVMCallWithData( + ctx, from, &burner, nil, + true, false, false, + big.NewInt(0), big.NewInt(5_000_000), nil, + ) + s.Require().NoError(err) + s.Require().False(res.Failed()) + s.Require().Greater(res.GasUsed, uint64(2_000_000)) + + charged := ctx.GasMeter().GasConsumed() - gasBefore + s.Require().GreaterOrEqual(charged, res.GasUsed) + s.Require().LessOrEqual(charged, res.GasUsed+1_000_000, + "res.GasUsed must be charged exactly once on the success path") +} diff --git a/x/vm/keeper/call_evm.go b/x/vm/keeper/call_evm.go index 3d8668bff..de9507c6b 100644 --- a/x/vm/keeper/call_evm.go +++ b/x/vm/keeper/call_evm.go @@ -72,10 +72,9 @@ func (k Keeper) CallEVMWithData( AccessList: ethtypes.AccessList{}, } - // Use a cache context so that a reverting EVM call does not corrupt the - // parent gas meter. On success we commit the cache and charge the actual - // gas used; on revert we discard the cache and leave the parent meter - // untouched (matching DerivedEVMCallWithData semantics). + // Use a cache context so that a reverting EVM call does not commit its state + // changes. The cache shares the parent gas meter, so gas is metered against + // the caller either way; only the store writes are discarded on failure. tmpCtx, commitState := ctx.CacheContext() res, err := k.ApplyMessage(tmpCtx, msg, nil, commit, true) if err != nil { @@ -83,6 +82,13 @@ func (k Keeper) CallEVMWithData( } if res.Failed() { + // A failed execution still burned real EVM work, so charge it to the parent + // Cosmos gas meter exactly like a successful one. Skipping this made a + // revert (or a deliberate out-of-gas) a free way to consume block compute. + // msg.GasLimit above is the hardcoded config.DefaultGasCap, so res.GasUsed + // — which equals the gas limit on out-of-gas — is bounded by that cap and + // cannot be inflated by the caller. + ctx.GasMeter().ConsumeGas(res.GasUsed, "apply evm message (failed)") return res, errorsmod.Wrap(types.ErrVMExecution, res.VmError) } @@ -191,7 +197,15 @@ func (k Keeper) DerivedEVMCallWithData( gasCap = gasRes.Gas } if gasLimit != nil { - gasCap = gasLimit.Uint64() + // Clamp the caller-supplied limit to DefaultGasCap. The failure path below + // charges res.GasUsed to the parent Cosmos gas meter, and on out-of-gas + // res.GasUsed equals this cap — so an unclamped caller value would let the + // caller choose how much gas the enclosing Cosmos tx is forced to consume, + // and panic it with OutOfGas. DefaultGasCap is already the ceiling on every + // other path into this function: the estimator above runs with + // GasCap: config.DefaultGasCap, and the remaining branch uses the cap + // directly. The clamp therefore only ever narrows an outlier. + gasCap = min(gasLimit.Uint64(), config.DefaultGasCap) } msg := core.Message{ @@ -323,6 +337,12 @@ func (k Keeper) DerivedEVMCallWithData( } if res.Failed() { + // A failed execution still burned real EVM work, so charge it to the parent + // Cosmos gas meter exactly like a successful one. Skipping this made a + // revert (or a deliberate out-of-gas) a free way to consume block compute, + // and left the `gasless` flag zeroing only the reported event attribute. + // res.GasUsed is bounded by gasCap, clamped to config.DefaultGasCap above. + ctx.GasMeter().ConsumeGas(res.GasUsed, "apply evm message (failed)") return res, errorsmod.Wrapf(types.ErrVMExecution, "%s: ret 0x%x", res.VmError, res.Ret) }