From ae595ba92c4f8a627d909ec4f27fbbc140f61f3f Mon Sep 17 00:00:00 2001 From: Nilesh Gupta Date: Wed, 26 Aug 2026 08:10:44 +0530 Subject: [PATCH] fix: honour the gasCap argument in CallEVMWithData The parameter was accepted and ignored, so callers sandboxing EVM work got a 25M gas message regardless. Also runs the timeout callback on cachedCtx, as the ack path already does. --- CHANGELOG.md | 6 ++ .../integration/x/vm/test_call_evm_gascap.go | 85 +++++++++++++++++++ x/ibc/callbacks/keeper/keeper.go | 8 +- x/vm/keeper/call_evm.go | 20 ++++- 4 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 tests/integration/x/vm/test_call_evm_gascap.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 4064e1bbc..e04d8ae97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ ### BUG FIXES +- Honour the `gasCap` argument in `CallEVMWithData`. The parameter was accepted and then ignored - + `GasLimit` was hardcoded to `config.DefaultGasCap` - so callers passing a cap to sandbox EVM work + (the IBC callback keeper passes its remaining Cosmos gas) got a message that could burn up to 25M + gas regardless. `DefaultGasCap` remains the ceiling, so a caller can only narrow the limit. This + matches `DerivedEVMCallWithData` and the shape upstream cosmos/evm settled on. Also routes the + timeout callback's EVM call through `cachedCtx`, as the acknowledgement path already does. - Run the gov precompile's `getTallyResult` against a throwaway cache. The SDK's `TallyResult` query delegates to `Keeper.Tally`, which deletes every vote it counts; that is safe on the SDK's own query paths because a gRPC query runs against a context that is never committed, but precompiles diff --git a/tests/integration/x/vm/test_call_evm_gascap.go b/tests/integration/x/vm/test_call_evm_gascap.go new file mode 100644 index 000000000..16e9c9358 --- /dev/null +++ b/tests/integration/x/vm/test_call_evm_gascap.go @@ -0,0 +1,85 @@ +package vm + +import ( + "math/big" + + "github.com/ethereum/go-ethereum/common" + + "github.com/cosmos/evm/contracts" + "github.com/cosmos/evm/server/config" + testconstants "github.com/cosmos/evm/testutil/constants" + utiltx "github.com/cosmos/evm/testutil/tx" + "github.com/cosmos/evm/x/erc20/types" +) + +// TestCallEVMWithDataHonoursGasCap is the guard for F-2026-18818. +// +// CallEVMWithData accepted a gasCap and then hardcoded GasLimit to +// config.DefaultGasCap, so the parameter did nothing. Callers passing one - the +// IBC callback keeper passes its remaining Cosmos gas, intending to sandbox the +// EVM work - got an EVM message that could burn up to 25M gas regardless. +// +// The assertion that catches this is the middle case: a cap below what the call +// costs has to actually stop the call. Asserting only that a large cap succeeds +// would pass just as happily against the ignored parameter. +func (s *KeeperTestSuite) TestCallEVMWithDataHonoursGasCap() { + erc20 := contracts.ERC20MinterBurnerDecimalsContract.ABI + contractAddr := common.HexToAddress(testconstants.WEVMOSContractMainnet) + + data, err := erc20.Pack("balanceOf", utiltx.GenerateAddress()) + s.Require().NoError(err) + + callWithCap := func(gasCap *big.Int) (uint64, error) { + s.SetupTest() + res, err := s.Network.App.GetEVMKeeper().CallEVMWithData( + s.Network.GetContext(), types.ModuleAddress, &contractAddr, data, false, gasCap, + ) + if res == nil { + return 0, err + } + return res.GasUsed, err + } + + // Baseline: no cap. Establishes what the call actually costs, so the + // too-small case below is derived rather than guessed. + baseline, err := callWithCap(nil) + s.Require().NoError(err, "an uncapped call must still work") + s.Require().Positive(baseline) + + // A cap below the call's own cost must fail it. This is the case that fails + // when gasCap is ignored. + _, err = callWithCap(new(big.Int).SetUint64(baseline - 1)) + s.Require().Error(err, "a gasCap below the call's cost must stop the call") + + // A modest cap that comfortably covers the call succeeds, while staying two + // orders of magnitude below DefaultGasCap - so success here is the supplied + // cap being honoured, not the default ceiling doing the work. + // + // Note GasUsed is not itself a viable limit. It is the net, post-refund + // figure (~24k for this call), whereas the execution needs materially more + // than that to be *given* up front - a cap of 2x GasUsed still reverts. Do + // not "tighten" this to baseline or a small multiple of it. + room := new(big.Int).SetUint64(baseline * 10) + used, err := callWithCap(room) + s.Require().NoError(err, "a gasCap with headroom over the call's cost must succeed") + s.Require().Less(room.Uint64(), config.DefaultGasCap, + "the headroom cap must stay below DefaultGasCap or this proves nothing") + s.Require().Positive(used) + + // DefaultGasCap remains the ceiling: a caller may narrow the limit, never + // widen it. + huge := new(big.Int).Mul(new(big.Int).SetUint64(config.DefaultGasCap), big.NewInt(10)) + used, err = callWithCap(huge) + s.Require().NoError(err) + s.Require().LessOrEqual(used, config.DefaultGasCap, + "a gasCap above DefaultGasCap must not raise the EVM gas limit") + + // A non-positive or absurdly wide cap falls back to the default ceiling + // rather than truncating to something meaningless. + _, err = callWithCap(big.NewInt(0)) + s.Require().NoError(err, "a zero gasCap must fall back to DefaultGasCap, not to a zero limit") + + overUint64 := new(big.Int).Lsh(big.NewInt(1), 70) + _, err = callWithCap(overUint64) + s.Require().NoError(err, "a gasCap wider than uint64 must fall back to DefaultGasCap") +} diff --git a/x/ibc/callbacks/keeper/keeper.go b/x/ibc/callbacks/keeper/keeper.go index d12603e26..c6d63760f 100644 --- a/x/ibc/callbacks/keeper/keeper.go +++ b/x/ibc/callbacks/keeper/keeper.go @@ -421,14 +421,18 @@ func (k ContractKeeper) IBCOnTimeoutPacketCallback( return err } - res, err := k.evmKeeper.CallEVM(ctx, *abi, sender, contractAddr, true, math.NewIntFromUint64(cachedCtx.GasMeter().GasRemaining()).BigInt(), "onPacketTimeout", + // NOTE: use the cached ctx for the EVM calls, as the acknowledgement path + // does. Running on the parent ctx committed the callback's EVM state + // regardless of the outcome and made writeFn() below a no-op, while the gas + // cap was still being read off cachedCtx (F-2026-18818). + res, err := k.evmKeeper.CallEVM(cachedCtx, *abi, sender, contractAddr, true, math.NewIntFromUint64(cachedCtx.GasMeter().GasRemaining()).BigInt(), "onPacketTimeout", packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence(), packet.GetData()) if err != nil { return errorsmod.Wrapf(types.ErrCallbackFailed, "EVM returned error: %s", err.Error()) } // Consume the actual gas used on the original callback context. - ctx.GasMeter().ConsumeGas(res.GasUsed, "callback onPacketAcknowledgement") + ctx.GasMeter().ConsumeGas(res.GasUsed, "callback onPacketTimeout") if ctx.GasMeter().IsOutOfGas() { return errorsmod.Wrapf(types.ErrCallbackFailed, "out of gas") } diff --git a/x/vm/keeper/call_evm.go b/x/vm/keeper/call_evm.go index 3d8668bff..878990a02 100644 --- a/x/vm/keeper/call_evm.go +++ b/x/vm/keeper/call_evm.go @@ -59,12 +59,30 @@ func (k Keeper) CallEVMWithData( return nil, err } + // Honour the caller's gasCap. Callers that pass one (the IBC callback keeper + // passes the remaining Cosmos gas) are trying to sandbox the EVM work; before + // F-2026-18818 this parameter was accepted and silently ignored, so the EVM + // always ran to DefaultGasCap and could burn far more gas than the caller had + // budgeted. DefaultGasCap stays the ceiling, so a caller can only ever narrow + // the limit, never widen it. Mirrors DerivedEVMCallWithData, and matches the + // shape upstream cosmos/evm settled on. + gasLimit := config.DefaultGasCap + if gasCap != nil && gasCap.Sign() > 0 { + // A gasCap wider than uint64 cannot be a meaningful limit; fall back to + // the default ceiling rather than truncating it. + if gasCap.BitLen() <= 64 { + if provided := gasCap.Uint64(); provided < gasLimit { + gasLimit = provided + } + } + } + msg := core.Message{ From: from, To: contract, Nonce: nonce, Value: big.NewInt(0), - GasLimit: config.DefaultGasCap, + GasLimit: gasLimit, GasPrice: big.NewInt(0), GasTipCap: big.NewInt(0), GasFeeCap: big.NewInt(0),