Skip to content

fix: swallowed oom - #111

Open
Gabriel-Trintinalia wants to merge 4 commits into
Consensys-Incorporated:mainfrom
Gabriel-Trintinalia:fix/swallowed-oom
Open

fix: swallowed oom#111
Gabriel-Trintinalia wants to merge 4 commits into
Consensys-Incorporated:mainfrom
Gabriel-Trintinalia:fix/swallowed-oom

Conversation

@Gabriel-Trintinalia

@Gabriel-Trintinalia Gabriel-Trintinalia commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Split out of #99, which mixed two independent concerns. This is the allocation-failure half; #99 keeps the database-error half. Same principle, different source, reviewable separately.

Problem

Several call sites absorb an allocation failure and carry on with a degraded value. Each turns "out of memory" into "this block computed a different answer" — for a stateless prover the worst possible trade, since a wrong state root is indistinguishable from a consensus disagreement and far harder to diagnose.

site swallowed into count
bytecode/main.zig analyzeLegacy an empty jump table — every JUMPDEST reads invalid 1
context/journal.zig the EIP-7708 transfer log is dropped silently 2
interpreter/opcodes/host_ops.zig LOG reports OOM as out-of-gas 1
precompile/*, crypto/backends/secp256r1.zig OOM reported as PrecompileError.OutOfGas 22

The first is not hypothetical. In for_amsterdam_at_0060M, compute/precompile/identity::test_identity exhausts the ~490 MiB guest heap, then cannot allocate the 63-byte jump-dest bitmap for the EIP-7002 withdrawal contract during the post-block system call. The call halts on invalid_jump, becomes SystemContractCallFailed, and the block is rejected with a wrong root — naming the identity precompile and the 7002 contract, neither of which is at fault. Nothing in the output mentions memory. It took a full day to trace.

Change

These sites cannot propagate — their signatures are fixed by infallible callers. So they record on a channel and the block is rejected, in the same shape as #99's ctx_error. Recording rather than wrapping the allocator keeps the success path free of extra indirection.

Checks live in transitionWithContext, not in its callers: five call sites execute a block, and a caller that forgot would be silently wrong again. It resets on entry, checks at each transaction boundary (so a failed block stops rather than running to completion on degraded values), and gates once more before returning to cover the post-block system calls and BAL/requests hashing.

A boundary check rather than a per-instruction one: the interpreter loop is hot enough that checking there would cost trace cells on every block.

Why not make OOM fatal

Tried and abandoned. The guest has no working abort: @trap() does not halt because ZisK's op_halt() is unimplemented!(), so the panic handler logs, execution resumes, and the block loops printing execution failed: until the 2^36 step cap — a hang instead of a fast failure.

That is a pre-existing hazard for any guest panic (unreachable, out-of-bounds, overflow in ReleaseSafe), not something this PR introduces, and deserves its own issue.

Verification

Two of the seven sites are proven by test (analyzeLegacy, the precompiles); the journal.zig and host_ops.zig sites were found by pattern and are fixed but not reproduced.

🤖 Generated with Claude Code


Note

High Risk
Touches consensus-visible execution paths (bytecode analysis, logs, precompiles) and block transition error handling; incorrect gating could reject valid blocks or still miss silent degradation.

Overview
Swallowed allocation failures on infallible EVM paths no longer produce silently wrong block results. A sticky process-global OOM channel (recordOom / oomSeen / resetOom) on zesu_allocator records failures where callers cannot propagate errors; the block driver rejects the block with OutOfMemory instead of finishing with a wrong state root or logs hash.

Recording sites include legacy jump-table analysis (empty table on alloc failure), EIP-7708 journal logs, LOG topic allocation in the interpreter, and precompile/crypto paths that still return OutOfGas locally but now also call recordOom. Gating lives in transitionWithContext: reset at block start, check at each transaction boundary, and a final check after post-block work.

Tests cover analyzeLegacy and identity precompile OOM recording; build.zig adds the bytecode module to zig build test so those tests actually run. zkVM allocator shims implement the same OOM API.

Reviewed by Cursor Bugbot for commit d83e322. Bugbot is set up for automated code reviews on this repo. Configure here.

Gabriel-Trintinalia and others added 4 commits September 5, 2026 17:38
Three call sites absorbed an allocation failure and carried on with a
degraded value. Each turns "out of memory" into "this block computed a
different answer", which for a stateless prover is the worst possible
trade: a wrong state root is indistinguishable from a consensus
disagreement, and far harder to diagnose.

  bytecode/main.zig  analyzeLegacy returned the code with an EMPTY jump
                     table, so every JUMPDEST reads as invalid and the
                     first JUMP halts with invalid_jump
  journal.zig        the EIP-7708 transfer log was dropped silently,
                     changing logsHash with no error anywhere
  host_ops.zig       LOG topics reported OOM as out_of_gas, so a
                     transaction the reference completes appears to OOG

The first is not hypothetical. In for_amsterdam_at_0060M,
compute/precompile/identity::test_identity exhausts the ~490 MiB guest
heap across its four 60M-gas transactions, then fails to allocate the
63-byte jump-dest bitmap for the EIP-7002 withdrawal contract during the
post-block system call. The call halts on invalid_jump, becomes
SystemContractCallFailed, and the block is rejected with a wrong root —
naming the identity precompile and the 7002 contract, neither of which
is at fault. Nothing in the output mentions memory.

These sites cannot propagate: their signatures are fixed by infallible
callers. So they record into a channel and the block driver rejects the
block, in the same shape as this branch's swallowed-database-error
channel. Recording rather than wrapping the allocator keeps the success
path free of extra indirection.

The channel is part of the zesu_allocator module interface, so all three
implementations provide it. It is sticky and process-global, hence reset
per block: the spec-test runners execute thousands of blocks in one
process and one failure must not condemn the rest. OOM takes precedence
over whatever error it caused downstream, since that error describes the
symptom rather than the cause.

Verified on the fixture above: previously a wrong root with
SystemContractCallFailed, now "execution failed: OutOfMemory". The 10M
and 30M variants of the same test and the mainnet_fusaka_24758573 vector
are unchanged. zig build test: 411/411.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tests only mean anything alongside the build.zig entry: zig collects
tests from the module passed to addTest, not from its imports, so nothing
in src/evm/bytecode/ ran under `zig build test`. Same trap this branch
already documents for context/journal.zig.

Three cases: a forced allocation failure is recorded and still returns the
degraded value; a successful analysis leaves the channel clear (without
which the first test would pass on a channel stuck on); and resetOom
clears, which is what stops one block condemning the next in the
long-lived spec-test runners.

Mutation-checked: dropping the recordOom() call from analyzeLegacy fails
the first test by name.

414/414 across 23 steps, up from 411/21.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every precompile that allocates reports failure as PrecompileError.OutOfGas,
because that is the only failure a precompile can express. That is a
consensus-visible lie: the reference completes the call, so a block the
network accepts is rejected here — and rejected for a fabricated gas
reason, which sends whoever debugs it to the wrong subsystem entirely.

22 sites across identity, hash, blake2, modexp, default_impls and the
secp256r1 backend. They keep returning OutOfGas — locally there is nothing
else to return — but now also record on the allocator's OOM channel, so
the block driver rejects for the real reason. Same shape as the other
swallowed failures on this branch.

Three tests, in a module already covered by `zig build test`:
the failure is recorded while still returning OutOfGas; a successful
allocation leaves the channel clear; and a genuine gas failure (a gas
limit below the base cost, so nothing is ever allocated) does not touch
the channel. That last one pins the distinction this change exists to
make. Mutation-checked: dropping the recordOom() call from identity fails
the first test by name.

417/417, up from 414.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The channel was reset and checked in executeBlockStateless, but five call
sites execute a block and only that one checked — transition.zig:601 runs
one directly, so a swallowed allocation failure there was still silently
wrong. Move the reset and the checks into transitionWithContext so every
caller inherits them.

Also stop at the first transaction boundary rather than running the whole
block on degraded values. A boundary check, not a per-instruction one: the
interpreter loop is hot enough that checking there would cost trace cells on
every block, including the overwhelming majority nowhere near OOM.

A final gate before returning covers the post-block system calls and the
BAL/requests hashing, which run after the last transaction and so are
invisible to the per-transaction check.

Verified on a block that genuinely exhausts the heap (the 60M identity
benchmark, which this branch does not otherwise fix): "execution failed:
OutOfMemory", terminating cleanly. 417/417.

Fatal-on-OOM was tried instead and abandoned: the guest has no working abort.
@trap() does not halt because ZisK's op_halt() is unimplemented, so the panic
handler logs, execution resumes, and the block loops printing "execution
failed:" until the 2^36 step cap — a hang instead of a fast failure. That is
a pre-existing hazard for any guest panic, not something this branch
introduces, and is worth its own issue.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d83e322. Configure here.

topics[2] = std.mem.zeroes([32]u8);
@memcpy(topics[2][12..], &to);
const data = alloc.alloc(u8, 32) catch {
// Dropping the log silently would change logsHash and so the block

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

EIP-7708 topics alloc still swallows OOM

High Severity

addEip7708TransferLog and addEip7708BurnLog record OOM only on the later data alloc. The earlier topics alloc still returns without recordOom(). Topics is the larger allocation, so heap exhaustion typically fails there first, the log is dropped, and the block can complete with a changed logsHash.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d83e322. Configure here.

// transaction the reference completes would appear to OOG.
// Halting is still the only local option, so record it and
// let the block be rejected.
alloc_mod.recordOom();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LOG data copy still reports OOG

High Severity

makeLogFn records OOM on the topics alloc but the following log-data dupe still halts as out-of-gas without recordOom(). LOG0 never takes the topics path, so its only allocation is this unrecorded copy. A transaction the reference completes can appear to OOG and the block can be accepted.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d83e322. Configure here.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant