Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import {ChainForkConfig} from "@lodestar/config";
import {CheckpointWithHex, IForkChoice, ProtoBlock} from "@lodestar/fork-choice";
import {SLOTS_PER_EPOCH} from "@lodestar/params";
import {computeStartSlotAtEpoch} from "@lodestar/state-transition";
import {RootHex} from "@lodestar/types";
import {Logger} from "@lodestar/utils";
import {Metrics} from "../../metrics/metrics.js";
import {MAX_LOOK_AHEAD_EPOCHS} from "../../sync/constants.js";
import {IClock} from "../../util/clock.js";
import {SerializedCache} from "../../util/serializedCache.js";
import {isDaOutOfRange} from "../blocks/blockInput/index.js";
Expand All @@ -24,6 +26,13 @@ export type SeenPayloadEnvelopeInputModules = {
logger?: Logger;
};

// Upper bound on the cache, enforced on every insertion. pruneFinalized / pruneBelowParent are both
// dormant during catch-up (no finality; prepareNextSlot bails once the head is far behind the clock),
// so without an insert-time cap the cache grows unbounded and OOMs the node
// (see https://github.com/ChainSafe/lodestar/issues/9073). Same size as SeenBlockInput's
// MAX_BLOCK_INPUT_CACHE_SIZE.
const MAX_PAYLOAD_ENVELOPE_INPUT_CACHE_SIZE = (MAX_LOOK_AHEAD_EPOCHS + 1) * SLOTS_PER_EPOCH;

/**
* Cache for tracking PayloadEnvelopeInput instances, keyed by beacon block root.
*
Expand Down Expand Up @@ -116,6 +125,7 @@ export class SeenPayloadEnvelopeInput {
root: props.blockRootHex,
daOutOfRange,
});
this.pruneToMaxSize(props.blockRootHex);
return input;
}

Expand All @@ -137,6 +147,7 @@ export class SeenPayloadEnvelopeInput {
root: props.blockRootHex,
daOutOfRange,
});
this.pruneToMaxSize(props.blockRootHex);
return input;
}

Expand Down Expand Up @@ -167,6 +178,28 @@ export class SeenPayloadEnvelopeInput {
}
}

/**
* Enforce the size cap by evicting the lowest-slot entry (oldest / furthest behind the head).
* Called on every insertion so the cache is at most one over the cap. `excludeRoot` is never
* evicted, so an add can't remove itself when it is the lowest slot (e.g. an older block in a reorg).
*/
private pruneToMaxSize(excludeRoot?: RootHex): void {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@lodekeeper can you confirm whether this exclude root is even needed, in what scenario can this be relevant with our current code?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Walked the 5 add / addFromBid call sites for the scenario where the just-inserted entry's slot is the strict minimum in the cache (the only case where excludeRoot actually fires):

Caller Slot of insert Hits excludeRoot?
chain.ts:445 (addFromBid at init) anchor block slot No — cache is empty at init, no eviction can fire
gossipHandlers.ts:191 (gossip block) gossip-window slot Yes, but only when finalized_slot < head - 96 (stalled finality) AND the block is on a non-canonical fork at a slot lower than every cache entry — validateGossipBlock lets that range through
downloadByRange.ts:214 (range sync seed) batch epoch slot Yes for out-of-order batch completion (Codex P1) — but excludeRoot only protects the single entry being added in this call; sibling adds earlier in the same batch can still get evicted by the next add. Partial coverage.
downloadByRoot.ts:119 (parent root sync) parent slot Yes for deep-reorg / fork-recovery where the parent is significantly older than head, edge case
api/impl/beacon/blocks/index.ts:116 (publish API) near-head slot No — validator-published blocks are at or near current slot

So it's reachable but only in three narrow regimes: stalled-finality gossip on a low-slot non-canonical fork, out-of-order range-sync batches, and deep-reorg parent fetches.

One extra wrinkle worth flagging: even when excludeRoot fires, the protection only lasts until the next add() call — that next add doesn't know to exclude the previous entry, so it becomes the new lowest and gets evicted. So excludeRoot is at most a 1-tick reprieve. For the gossip case in particular, the gap between gossipHandlers.ts:191 (block add) and the matching envelope lookup at gossipHandlers.ts:1141 (PAYLOAD_ENVELOPE_INPUT_MISSING throw) can easily span several other gossip adds in a busy network, in which case excludeRoot doesn't actually save the lookup.

Verdict: defensible to keep — costs almost nothing, plugs the immediate self-eviction path that the unit test exercises. But fair to drop too if you'd rather simplify; the OOM scenario this PR addresses (sequential catch-up) never enters the excludeRoot branch since adds are always at the high end of the slot range. If we drop it I'd suggest a brief comment on pruneToMaxSize noting that a single low-slot insert can be self-evicting, so callers shouldn't assume add() always succeeds.

If you want a stronger guarantee for the gossip-block → envelope path specifically, the real fix is either (a) skip-prune on add and prune at a clock-tick boundary, or (b) the local seed-map fallback in cacheByRangeResponses Codex suggested — both follow-up scope.

while (this.payloadInputs.size > MAX_PAYLOAD_ENVELOPE_INPUT_CACHE_SIZE) {
let lowest: PayloadEnvelopeInput | undefined;
for (const input of this.payloadInputs.values()) {
if (input.blockRootHex === excludeRoot) continue;
if (lowest === undefined || input.slot < lowest.slot) lowest = input;
}
if (lowest === undefined) break;
this.evictPayloadInput(lowest);
this.logger?.debug("SeenPayloadEnvelopeInput.pruneToMaxSize evicted entry", {
slot: lowest.slot,
root: lowest.blockRootHex,
maxSize: MAX_PAYLOAD_ENVELOPE_INPUT_CACHE_SIZE,
});
}
}

private evictPayloadInput(payloadInput: PayloadEnvelopeInput): void {
this.serializedCache.delete(payloadInput.getSerializedCacheKeys());
this.payloadInputs.delete(payloadInput.blockRootHex);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {beforeEach, describe, expect, it, vi} from "vitest";
import {ExecutionStatus, IForkChoice, PayloadStatus, ProtoBlock} from "@lodestar/fork-choice";
import {testLogger} from "@lodestar/logger/test-utils";
import {ForkName} from "@lodestar/params";
import {ForkName, SLOTS_PER_EPOCH} from "@lodestar/params";
import {DataAvailabilityStatus} from "@lodestar/state-transition";
import {RootHex} from "@lodestar/types";
import {ChainEventEmitter} from "../../../../src/chain/emitter.js";
import {SeenPayloadEnvelopeInput} from "../../../../src/chain/seenCache/seenPayloadEnvelopeInput.js";
import {MAX_LOOK_AHEAD_EPOCHS} from "../../../../src/sync/constants.js";
import {SerializedCache} from "../../../../src/util/serializedCache.js";
import {getMockedClock} from "../../../mocks/clock.js";
import {config, generateBlock} from "../../../utils/blocksAndData.js";
Expand Down Expand Up @@ -96,6 +97,43 @@ describe("SeenPayloadEnvelopeInput", () => {
expect(cache.get(rootHex)).toBeDefined();
});

it("pruneToMaxSize bounds the cache on insertion, evicting the lowest-slot entries", () => {
// pruneFinalized / pruneBelowParent never run here, so the insertion-time cap is the only bound
const maxSize = (MAX_LOOK_AHEAD_EPOCHS + 1) * SLOTS_PER_EPOCH;
const overflow = 5;

const rootHexBySlot = new Map<number, string>();
for (let slot = 1; slot <= maxSize + overflow; slot++) {
rootHexBySlot.set(slot, addPayloadInput(slot));
}

// never grows past the cap despite no finality-based pruning
expect(cache.size()).toBe(maxSize);

// the oldest (lowest-slot) entries are evicted ...
for (let slot = 1; slot <= overflow; slot++) {
expect(cache.get(rootHexBySlot.get(slot) as string)).toBeUndefined();
}
// ... and the most recent window is retained
for (let slot = overflow + 1; slot <= maxSize + overflow; slot++) {
expect(cache.get(rootHexBySlot.get(slot) as string)).toBeDefined();
}
});

it("excludes the just-inserted entry from pruning even when it is the lowest slot", () => {
const maxSize = (MAX_LOOK_AHEAD_EPOCHS + 1) * SLOTS_PER_EPOCH;
const prevLowest = addPayloadInput(101);
for (let slot = 102; slot < 101 + maxSize; slot++) addPayloadInput(slot);
expect(cache.size()).toBe(maxSize);

// adding an older block (lowest slot) while at cap must not evict itself; the previous lowest
// (slot 101) is evicted instead. Guards the reorg / older-fork case.
const older = addPayloadInput(1);
expect(cache.size()).toBe(maxSize);
expect(cache.get(older)).toBeDefined();
expect(cache.get(prevLowest)).toBeUndefined();
});

it("add returns the existing entry on duplicate root", () => {
const {block, rootHex} = generateBlock({forkName: ForkName.gloas, slot: 1});
const props = {
Expand Down
Loading