evm pool memory zeroing

[svlachakis]

Fixes Closes Resolves #

Please choose one of the keywords above to refer to the issue this PR solves followed by the issue number (e.g. Fixes #000). If no issue number, remove the line. Also, remove everything marked optional that is not applicable. Remove this note after reading.

Changes

• List the changes

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Notes on testing

Optional. Remove if not applicable.

Documentation

Requires documentation update

• Yes
• No

If yes, link the PR to the docs update or the issue with the details labeled docs. Remove if not applicable.

Requires explanation in Release Notes

• Yes
• No

If yes, fill in the details here. Remove if not applicable.

Remarks

Optional. Remove if not applicable.

[claude]

Claude finished @svlachakis's task in 6m 6s —— View job

---

Review: EVM pooled memory zeroing (#12145)

• Read the diff
• Analyze correctness (lazy-zeroing frontier invariant)
• Check pool return-dirty implications
• Look for tests / regression coverage
• Post review

---

Summary

This replaces eager buffer zeroing (rent-clean + clear-on-return) with a lazy defined-frontier model: _lastZeroedSize tracks how far the buffer is "defined" (written or zeroed); pooled buffers come back dirty and each read/grow zeroes only the gap it exposes, while contiguous writes skip zeroing the region they fill themselves (writeStart). Net effect is less zeroing on write-heavy paths — a sensible optimization for a hot path.

I traced the frontier invariant across all read / write / grow / dispose / trace paths and it holds. No Critical / High / Medium findings. Highlights of the correctness check:

• Core invariant — every byte < _lastZeroedSize is defined (written or zeroed); every byte ≥ it is dirty. Each write bumps the frontier to its end (if (length > _lastZeroedSize) _lastZeroedSize = length), so no written byte ever sits above the frontier where a later ZeroGapUpTo could wrongly clear it. ✅
• Write paths (TrySaveWord/Byte/Span/byte[]/ZeroPaddedSpan, StoreWord/ByteAfterGas) — each fills exactly [writeStart, length); the pre-write gap [_lastZeroedSize, writeStart) is zeroed via ZeroGapUpTo(Min(length, writeStart)). ZeroPaddedSpan's padding is covered because length includes PaddingLength. ✅
• Read paths (TryLoad*, LoadSpan*AfterGas, Load32BytesAfterGas, Inspect, GetTrace) — writeStart defaults to ulong.MaxValue, so the whole exposed region is zeroed. ✅
• MCOPY (CopyAfterGas) — zeroes up to Max(srcEnd, dstEnd) before the copy so undefined source bytes read as zero; Span.CopyTo handles overlap. ✅
• Gas-only size update — ComputeMemoryExpansionCost → UpdateSize(newSize, rentIfNeeded: false) skips renting/zeroing; the subsequent *AfterGas access does the zeroing. ✅
• Grow — copies the defined prefix, frontier preserved, dirty tail zeroed on next access. Dispose-then-reuse — RentSlow resets _lastZeroedSize = 0 for a fresh buffer, which (as the comment notes) is mandatory because a struct reused after Dispose keeps a stale frontier. _lastZeroedSize is always clamped to ≤ memory.Length, so the preserved-prefix copy on grow is always safe. ✅
• Shared-pool consumers — checked Trie TrackingCappedArrayPool, Discv5 PacketCodec/Packet, and the BLS precompile: all fill before reading, so returning dirty buffers to ArrayPool<byte>.Shared is contract-safe.

Findings (all Low)

#	Severity	Finding
1	Low	_cleanArrays / _cleanArrayCount now hold dirty buffers — the "clean" naming is misleading post-change. (inline)
2	Low	Large buffers returned un-zeroed to the process-wide shared pool — within ArrayPool contract & no consumer assumes zeroed, but removes a defense-in-depth wipe; worth an explicit acknowledgement. (inline)
3	Low	Test coverage. The existing Large_pooled_buffer_is_zeroed_on_reuse covers the large-array path, and GetTrace_memory_should_not_bleed_between_txs covers VM-level reuse, but the write-frontier gap and the small (<64 KiB) ThreadStatic-cache dirty reuse aren't directly unit-tested. A focused test would lock in the new invariant, e.g.: write a byte at a high offset (MSTORE8/TrySaveByte at offset 1000) so a gap forms below the frontier, dispose+reuse the struct on the same thread, then read [0, sizeAboveOffset) and assert all-zero except the written byte.

Process note

The PR description is an unfilled template (no summary, no change type checked, testing/docs sections blank). Please fill it in — the type-of-change checkboxes drive auto-labeling, and given this touches consensus-critical EVM memory zeroing a short rationale + perf note would help reviewers.

ℹ️ I could not run dotnet build / dotnet test in this environment (commands require approval), so the correctness assessment is by code inspection only. The signature changes (UpdateSize, PrepareAccessAfterGas, RentBuffer/ReturnDirty) are all private/internal and consistent at every call site.

Verdict: Algorithm is correct; only Low-severity polish items. Mergeable once findings are acknowledged (or addressed) and the description is completed.

[claude]

Low (naming): After this change these caches hold dirty (un-zeroed) buffers — RentBuffer no longer clears and ReturnDirty no longer wipes. The _cleanArrays / _cleanArrayCount names (and "clean cache" framing) are now misleading and will trip up future readers who rely on the name to reason about zeroing. Consider renaming to _pooledArrays / _pooledArrayCount to match the new dirty-pool semantics.

[claude]

Low (info-hygiene): Large buffers (> MaxCachedArrayLength = 64 KiB) routed through ReturnLarge end up in SafeArrayPool<byte>.Shared (= the process-wide ArrayPool<byte>.Shared, also used by Trie TrackingCappedArrayPool, Discv5 packet codec, and the BLS precompile) for the 64 KiB–1 MiB band. They're now returned un-zeroed, so leftover EVM memory contents (potentially sensitive contract data) flow into the shared pool.

This is within the ArrayPool contract (renters must never assume zeroed buffers — and Shared is already process-global, so it was never a real zeroing guarantee), and I confirmed the other Shared consumers all fill before reading, so there's no correctness regression. Flagging only because it removes a defense-in-depth wipe; worth a one-line acknowledgement that the hygiene loss is intentional.

[github-actions]

EVM Opcode Benchmark Diff

Aggregated runs: base=3, pr=3
Noisy rerun opcodes: MOD

No significant regressions or improvements detected.

[github-actions]

EXPB Benchmark Comparison

Run: View workflow run

superblocks

No metrics were produced for nethermind-flat-superblocks-evm-pool-memory-zeroing-delay0s.

realblocks

No metrics were produced for nethermind-flat-realblocks-evm-pool-memory-zeroing-delay0s.