fix(precompile): free precompile output instead of retaining it for t… - #110
Open
Gabriel-Trintinalia wants to merge 1 commit into
Open
Conversation
…he block The interpreter stores return data as a borrowed slice (ReturnDataImpl.data), so a precompile's heap-allocated output had no owner and was never freed. A 30M-gas identity benchmark allocated 1,773 buffers totalling 300 MiB and released none of them; at 60M the guest heap is exhausted and the block cannot be executed at all. The failure is unrecognisable as memory pressure. Out of heap, analyzeLegacy falls back to an empty jump table, so the EIP-7002 withdrawal contract has no valid JUMPDESTs, its post-block system call halts on invalid_jump, and the block is rejected as SystemContractCallFailed with a wrong root — naming the identity precompile and the 7002 contract, neither of which is at fault. Hold at most one output at a time and free the previous one when the next arrives. That is sound because a frame can only read its return data between its own calls, and any subsequent call — precompile or sub-frame — overwrites it, so no live pointer is stranded. Ownership is one module-wide rule rather than a per-precompile flag: a non-empty output is always heap-allocated and the caller frees it, empty outputs are static and exempt by their length. Only the KZG return value needed changing (it was the one non-empty static); the other 23 precompiles are untouched. An earlier version carried an `owned: bool` with newOwned/newBorrowed constructors and reclassified all 24 sites — twice the diff, and marginally slower for the per-call branch. Copying the output into a caller-owned buffer was also measured and rejected: it costs +76% trace cells at 60M, because moving bytes is expensive in a zkVM even where it would be free natively. Heap on the identity benchmark: 446 MiB -> 3 MiB at 30M; 60M goes from unexecutable to validating. Real blocks are unaffected either way (24 MiB before and after) since they make few precompile calls. Verified: 394/394 unit tests; Amsterdam 0010M 2458/2458 validated at +0.013% cells; mainnet 500-block 500/500 validated at +0.001%. PRECOMPILES cells are byte-identical on both — nothing about precompile execution changed, only what happens to the output afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The interpreter stores return data as a borrowed slice (
ReturnDataImpl.data), so a precompile's heap-allocated output has no owner and is never freed. A 30M-gas identity benchmark allocates 1,773 buffers totalling 300 MiB and releases none. At 60M the ~490 MiB guest heap is exhausted and the block cannot execute.The resulting failure names the wrong thing entirely. Out of heap,
analyzeLegacyfalls back to an empty jump table, so the EIP-7002 withdrawal contract has no validJUMPDESTs, its post-block system call halts oninvalid_jump, and the block is rejected asSystemContractCallFailedwith a wrong root — blaming the identity precompile and the 7002 contract, neither of which is at fault. Nothing in the output mentions memory.Change
Hold at most one output at a time; free the previous when the next arrives. Sound because a frame can only read its return data between its own calls, and any subsequent call — precompile or sub-frame — overwrites it, so no live pointer is stranded.
Ownership is one module-wide rule, not a per-precompile flag:
Only the KZG return value needed changing (the one non-empty static). The other 23 precompiles are untouched.
Results
PRECOMPILEScells are byte-identical on both suites — nothing about precompile execution changed, only what happens to the output afterwards.Alternatives measured and rejected
owned: boolflag withnewOwned/newBorrowed, reclassifying all 24 sites: twice the diff (10 files, +87/−27 vs 5 files, +43/−2) and marginally slower for the per-call branch.Known gap
A future precompile returning a non-empty static would violate the rule and corrupt the heap, and unlike the flag version the compiler cannot catch it. A debug assertion (allocator bounds check) would close this if wanted.
Not included
The heap exhaustion is also swallowed elsewhere —
analyzeLegacy, the EIP-7708 transfer log,LOGtopics, and 22 precompile sites reporting OOM asOutOfGas. That reporting problem is #99's subject and is being handled there.🤖 Generated with Claude Code
Note
Medium Risk
Touches interpreter/host precompile dispatch and allocator ownership; incorrect free/double-free would corrupt memory, but the single-slot model matches EVM return-data visibility and tests report identical precompile traces.
Overview
Fixes a memory leak where heap-allocated precompile return data was never freed because the interpreter only borrows return-data slices. Heavy identity-precompile workloads could retain hundreds of MiB and OOM the zkVM guest heap, surfacing as misleading block failures elsewhere.
Hostnow keeps at most one outstanding precompile buffer inlast_output, frees the prior buffer before each new precompile call, and documents a module-wide rule inPrecompileOutput: non-emptybytesare always heap-owned and freed when no longer observable; empty outputs stay static.Frame.executeandMainnetHandler.executeFramecalldefer host.releaseOutput()so the last buffer is released when execution ends.The KZG point-evaluation precompile is updated to heap-duplicate its fixed 64-byte return value so it follows the same ownership rule as other non-empty outputs. Precompile semantics and trace cells are unchanged—only lifecycle of output buffers.
Reviewed by Cursor Bugbot for commit 334a941. Bugbot is set up for automated code reviews on this repo. Configure here.