Skip to content

core/vm: rework gas measurement for call variants#33648

Merged
lightclient merged 2 commits into
ethereum:masterfrom
rjl493456442:call-gas-revamp-2
Mar 19, 2026
Merged

core/vm: rework gas measurement for call variants#33648
lightclient merged 2 commits into
ethereum:masterfrom
rjl493456442:call-gas-revamp-2

Conversation

@jwasinger
Copy link
Copy Markdown
Contributor

simplified version of #33380

@jwasinger
Copy link
Copy Markdown
Contributor Author

There's a lint error, but I don't have perms to push to your branch @rjl493456442 . I'm going to apply this PR on top of the bal changes.

@rjl493456442
Copy link
Copy Markdown
Member

Feel free to reown it!

@rjl493456442
Copy link
Copy Markdown
Member

I would like to revert the changes to EIP7702 wrapper for now, if all clients agree the current spec is the best solution, we can add it back!

sssalrawahi9-coder

This comment was marked as spam.

Comment thread core/vm/operations_acl.go
// - transfer value
// - memory expansion
// - create new account
intrinsicCost, err := intrinsicFunc(evm, contract, stack, mem, memorySize)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I had with this was that we are computing the intrinsic cost (and with that the readOnly guard) after we potentially added the address to the access list. This could lead to an address ending up in the BAL that was called through a call that should've been stopped by the readOnly guard. I guess our old behavior did the same thing. So the PR is okay, its just something we need to refactor for BALs anyway

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah just realized that jared has reordered the clauses to move the readOnly guard first. It kinda duplicates it, but it fixes the issue

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jared modified it a few weeks ago, the gas calculation should be terminated immediately if the readOnly is applied and state mutation occurs.

Copy link
Copy Markdown
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

Copy link
Copy Markdown
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think the "intrinsic" suffix is a bit overloaded with tx intrinsic gas. We can address it later though.

LGTM

@lightclient lightclient added this to the 1.17.2 milestone Mar 19, 2026
@lightclient lightclient merged commit fd85963 into ethereum:master Mar 19, 2026
7 of 8 checks passed
@wanyvic
Copy link
Copy Markdown

wanyvic commented Apr 29, 2026

Issue: duplicated "out of gas" error string in call traces

Summary

Commit fd85963
("core/vm: rework gas measurement for call variants", PR #33648)
changed the CALL gas calculation flow and introduced an observable trace error
string regression.

The semantic VM error is still out-of-gas, but the public trace error string
changed from:

out of gas

to:

out of gas: out of gas

This is visible in debug_traceTransaction and also propagates to persisted
internal transaction traces, because tracers store or return err.Error()
directly.

Observed Behavior

Example transaction:

0xf536a76b7617fe85eaebe807020adeb1d28c6b752472c1cba9ffd3ad99b16181

Observed trace path:

result.calls[0].calls[0].error

Old behavior, v1.16.8 / old CALL gas flow:

"out of gas"

New behavior, v1.17.2 / new CALL gas flow:

"out of gas: out of gas"

Root Cause

Before fd85963, CALL gas calculation returned the full dynamic gas cost to
the interpreter. If the contract did not have enough gas, the interpreter
returned the bare ErrOutOfGas:

if contract.Gas < dynamicCost {
    return nil, ErrOutOfGas
}

Therefore tracers exposed:

out of gas

After fd85963, CALL gas calculation was split into intrinsic/stateless and
stateful parts. The new gasCallIntrinsic path performs an early gas check:

if contract.Gas < gas {
    return 0, ErrOutOfGas
}

That ErrOutOfGas is returned from operation.dynamicGas(...). The interpreter
then wraps all dynamic gas errors here:

if err != nil {
    return nil, fmt.Errorf("%w: %v", ErrOutOfGas, err)
}

Since err is already ErrOutOfGas, the final error string becomes:

out of gas: out of gas

Impact

The EVM semantics are unchanged, but the public trace error string changes.
This breaks downstream compatibility checks and reconciliation jobs that expect
the historical trace error string:

out of gas

The difference affects:

- debug_traceTransaction callTracer output

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants