fix: share in-flight payload envelope import promise to prevent sync spin loop

[barnabasbusa]

Motivation

While testing glamsterdam-devnet-5 via Kurtosis, lodestar reliably crashed with a JS heap OOM (exit 134) ~60-100s after startup, whenever the CL was following the chain while its EL was still syncing:

<--- Last few GCs --->

[7:0xffffac790000]    65680 ms: Scavenge (interleaved) 8124.2 (8168.0) -> 8101.1 (8172.3) MB, pooled: 0 MB, 14.92 / 0.00 ms  (average mu = 0.254, current mu = 0.204) allocation failure;
[7:0xffffac790000]    66835 ms: Mark-Compact (reduce) 8114.3 (8173.3) -> 8103.7 (8116.8) MB, pooled: 0 MB, 45.22 / 0.09 ms  (+ 1044.7 ms in 209 steps since start of marking, biggest step 6.3 ms, walltime since start of marking 1140 ms) (average mu = 0.257

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
----- Native stack trace -----

 1: 0x73f17c node::OOMErrorHandler(char const*, v8::OOMDetails const&) [node]
 2: 0xbaa8dc  [node]
 3: 0xbaa9b4  [node]
 4: 0xe26c4c  [node]
 5: 0xe26c7c  [node]
 6: 0xe26fd4  [node]
 7: 0xe36784  [node]
 8: 0xe3a3e4  [node]
 9: 0x18226c4  [node]

Debug logs captured the cause: an infinite spin loop in BlockInputSync payload sync, re-processing the same payload root at ~25 iterations per millisecond until the 8GB heap (--max-old-space-size=8192) was exhausted, while starving the event loop (no status logs, dead REST API for the final ~90s):

BlockInputSync.downloadPayload() slot=unknown, root=0x2869…afd0, pendingPayloads=2
BlockInputSync.fetchPayloadInput: successful download … hasPayload=true, hasAllData=true
Processed payload from unknown sync slot=43213, root=0x2869…afd0
Added new payload rootHex to BlockInputSync.pendingPayloads root=0x2869…afd0
No unknown block, process ancestor downloaded blocks pendingBlocks=1, ancestorBlocks=1, processedBlocks=0
(repeats ~25x per ms, cycling through every connected peer, until OOM)

The loop:

1. A pending block's missing dependency is its parent payload; the payload is not in fork choice yet because its real import job is still in flight (e.g. blocked on regen/EL while the EL syncs).
2. advancePendingBlock() re-adds the payload root to pendingPayloads.
3. The scheduler "downloads" it instantly from seenPayloadEnvelopeInputCache.
4. processPayload() → PayloadEnvelopeProcessor.processPayloadEnvelopeJob() sees the input already has an importStatus and returns immediately — silent success — while the actual import has not completed.
5. BlockInputSync logs "Processed payload", deletes the pendingPayloads entry, re-triggers the search → back to step 1. No backoff, no progress check.

Description

Replace the importStatus enum WeakMap in PayloadEnvelopeProcessor with a WeakMap of in-flight import promises. Duplicate callers now await the real import outcome instead of resolving early:

• While the import is in flight, deduped callers wait — BlockInputSync no longer sees a fake success, so the spin loop cannot form.
• On import failure, the rejection propagates to all callers — BlockInputSync.processPayload already has correct bounded handling for PayloadErrors (BLOCK_NOT_IN_FORK_CHOICE → re-enqueue block, engine errors → retry on next scheduler pass), it just never saw the errors before. The failed entry is dropped from the map so a later attempt can retry.
• On success, subsequent calls still resolve immediately without re-running the import (unchanged behavior).

Added unit tests for the processor; the first two fail against the previous implementation.

Note: the bug is currently only reachable on gloas networks (devnets), but the code is live on unstable.

AI disclosure

This PR was diagnosed and authored with AI assistance (Claude Code), reviewed by me.

[gemini-code-assist]

Code Review

This pull request refactors PayloadEnvelopeProcessor to deduplicate payload envelope imports by sharing the in-flight import promise instead of using a status enum. This ensures all callers observe the actual import outcome and prevents duplicate triggers from resolving early. It also adds comprehensive unit tests covering these scenarios. The reviewer suggested a defensive check in the catch block of processPayloadEnvelopeJob to ensure only the matching promise is deleted, preventing potential race conditions if the promise is overwritten.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

To prevent potential race conditions in future refactorings (for example, if a timeout or manual cancellation mechanism is introduced that deletes or overwrites the promise in this.imports), it is safer to defensively check that the promise being deleted is indeed the one that was created by this invocation.

    try {
      await importPromise;
    } catch (e) {
      // Drop the failed import so a later attempt can retry once the failure cause is resolved
      // (e.g. BLOCK_NOT_IN_FORK_CHOICE after the block lands in fork choice)
      if (this.imports.get(payloadInput) === importPromise) {
        this.imports.delete(payloadInput);
      }
      throw e;
    }

[lodekeeper]

Sensible defensive pattern but not a current correctness issue. Walked the call sites: the only imports.set(payloadInput, importPromise) is at line 51 right after jobQueue.push, and the only imports.delete(payloadInput) is the catch at line 58 of the same call that wrote it — so any deletion necessarily targets the same promise this call created. There's no path that overwrites the entry mid-flight (no timeout, no manual cancellation, no concurrent set).

The identity check would only matter if a future refactor introduced one of those overwrite paths, which is exactly the future-proofing case you flagged. Happy to take it as a one-line follow-up or fold it into this PR — your call. Either way, the dedupe semantics are correct as-is. Approved in pullrequestreview-4468399794.

[nflaig]

@lodekeeper can you review this please, is it a different issue from #9489 ?

[lodekeeper]

Different issue from #9489, both are real.

#9489 caps SeenPayloadEnvelopeInput's size so the cache stops growing during stalled finality (insert-time eviction of the lowest-slot entry once over MAX_PAYLOAD_ENVELOPE_INPUT_CACHE_SIZE). The OOM mode there is slow heap growth from cache entries that pruneFinalized / pruneBelowParent never touched.

#9501 fixes a spin loop in PayloadEnvelopeProcessor.processPayloadEnvelopeJob. The previous importStatus enum returned silent success when a duplicate call arrived for an in-flight import. BlockInputSync then logs "Processed payload", deletes the pendingPayloads entry, re-discovers the missing parent payload, re-adds the same root, processes again — at ~25 iterations/ms. Allocation pressure exhausts the heap in 60-100s while the EL is still syncing, while starving the event loop (no status logs, dead REST API).

Different code paths, different root causes, both manifest as OOM:

• #9489: memory leak via unbounded cache — slow growth
• #9501: spin loop via false-success dedupe — fast allocation pressure

The WeakMap<PayloadEnvelopeInput, Promise<void>> is the right shape:

• The waitForSpace() + re-check-after-await pattern (lines 42-48) prevents two callers both pushing the same input even when the first hits queue backpressure.
• Failed imports are dropped from the map so a later call can retry once the root cause (e.g. BLOCK_NOT_IN_FORK_CHOICE) is resolved.
• Successful imports stay in the map so subsequent calls resolve immediately — preserves idempotent semantics for re-imports of the same input.
• WeakMap keying means an entry is freed when the PayloadEnvelopeInput is GC'd — no leak risk from never-deleted resolved promises.

Test coverage is good: the dedupe-doesn't-resolve-early test and the failure-propagation tests would both have failed against the previous implementation. Docstring clearly explains why duplicates must observe the real outcome.

Gemini's defensive identity-check suggestion (discussion_r3388603644) is sensible as future-proofing but not a current correctness issue — the only imports.set runs after the only jobQueue.push and the only imports.delete is in the catch of the same caller, so a deletion can only ever target the same promise it created. I'd accept it as a follow-up tightening or leave as-is. Either way, approving.

[lodekeeper]

Different issue from #9489 — full breakdown + APPROVE in pullrequestreview-4468399794. tldr: #9489 caps cache growth (slow OOM via accumulated entries that pruneFinalized/pruneBelowParent never touched), #9501 fixes a BlockInputSync spin loop where duplicate dedupe was silent-success on in-flight imports (~25 iter/ms allocation pressure). Different files, different code paths, both real.

[lodekeeper]

Re Gemini's defensive imports.get(payloadInput) === importPromise check: addressed in discussion_r3388814416 — sensible future-proofing but not a current correctness issue since the set/delete happen on the same call path with no intervening overwrite. Noted on the consumer-version sunset.

[nflaig]

merged into glamsterdam-devnet-5

[twoeths]

looks good to me

[nflaig]

LGTM, thanks @barnabasbusa

[wemeetagain]

🎉 This PR is included in v1.45.0 🎉