ChainSafe / ChainSafe/lodestar

SeenPayloadEnvelopeInput cache missing pruning mechanisms

Open
#9,073 1 comment 0 reactions 1 assignee Claimed by @twoeths View on GitHub
spec-gloas
Dominant language
TypeScript
Stars
1.4k
Forks
483
Avg merge
1d 16h
Merged PRs (30d)
150

Description

## Summary

`SeenPayloadEnvelopeInput` (added in PR #8962) has significantly fewer pruning mechanisms than the equivalent `SeenBlockInput` cache. This means entries can accumulate and leak memory in several scenarios that the block input cache handles correctly.

## Analysis

### Pruning comparison: `SeenBlockInput` vs `SeenPayloadEnvelopeInput`

| Pruning mechanism | `SeenBlockInput` | `SeenPayloadEnvelopeInput` |
|---|---|---|
| After DB persist (`.finally()`) | ✅ `writeBlockInputToDb.ts:133` | ✅ `writePayloadEnvelopeInputToDb.ts:49` |
| On finalization event | ✅ `onFinalized` + `pruneToMaxSize()` | ✅ `onFinalized` (no max-size cap) |
| On gossip validation error (REJECT/IGNORE) | ✅ `gossipHandlers.ts:205` | ❌ Missing |
| On `processBlock` error (gossip path) | ✅ `gossipHandlers.ts:492` | ❌ Missing |
| After range sync batch processing | ✅ `range.ts:223` | ❌ N/A (no range sync for envelopes yet) |
| On unknown block removal (bad chain) | ✅ `unknownBlock.ts:681` | ❌ N/A (no unknown envelope sync yet) |
| `pruneToMaxSize()` cap | ✅ `MAX_BLOCK_INPUT_CACHE_SIZE` | ❌ Missing — no upper bound |

### Specific concerns

#### 1. No `pruneToMaxSize()` — unbounded growth during non-finality

`SeenBlockInput` has `MAX_BLOCK_INPUT_CACHE_SIZE = (MAX_LOOK_AHEAD_EPOCHS + 1) * SLOTS_PER_EPOCH` and calls `pruneToMaxSize()` after every `prune()` and `onFinalized()`. `SeenPayloadEnvelopeInput` has no equivalent — during extended non-finality, entries accumulate with no upper bound besides finalization.

#### 2. No pruning on `importExecutionPayload` failure

When `processExecutionPayload` fails (EL error, invalid signature, state transition error), the `PayloadEnvelopeInput` stays in the cache permanently (until finalization). The gossip handler catches the error but only logs it:

```ts
chain.processExecutionPayload(payloadInput, {validSignature: true}).catch((e) => {
chain.logger.debug("Error processing execution payload from gossip", ...);
// No prune! Entry stays in cache forever
});
```

Compare with `SeenBlockInput` which explicitly prunes on block processing errors:
```ts
chain.seenBlockInputCache.prune(blockInput.blockRootHex);
```

#### 3. `.finally()` prunes even on DB write failure — data loss

`persistPayloadEnvelopeInput` prunes the in-memory cache in `.finally()`, which runs after `.catch()`. If the DB write fails (transient error), the envelope data is evicted from both memory and DB — irrecoverable. This is the same pattern as `persistBlockInput` (existing technical debt), but worth noting as it applies here too.

#### 4. No pruning on gossip validation rejection

If `validateGossipExecutionPayloadEnvelope` passes but `addPayloadEnvelope` throws (e.g., duplicate envelope), the cache entry persists. The block input cache handles analogous cases by pruning on REJECT/IGNORE actions.

## Suggested fixes

1. **Add `pruneToMaxSize()`** with a reasonable constant (e.g., `MAX_PAYLOAD_ENVELOPE_INPUT_CACHE_SIZE`), called after `prune()` and `onFinalized()`
2. **Prune on `processExecutionPayload` failure** in the gossip handler (match block input pattern)
3. **Consider retry-then-prune** for DB write failures instead of unconditional `.finally()` prune (applies to both block and payload envelope caches — tracked separately)

## Related

- #8962 (introduces `SeenPayloadEnvelopeInput`)
- #8915 (block + payload timing issue)
- Existing `persistBlockInput` has the same `.finally()` data-loss pattern (separate issue)

_AI assisted_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.