Defer state/block pruning until after block cascade completes#240
Defer state/block pruning until after block cascade completes#240pablodeymo merged 2 commits intomainfrom
Conversation
🤖 Kimi Code ReviewReview SummaryThe PR correctly separates heavy pruning from checkpoint updates to prevent state deletion during block cascades. This is a critical fix for consensus correctness. Issues Found
Security & Correctness
Performance
The PR is approved with fixes - the race condition should be addressed before merge. Automated review by Kimi (Moonshot AI) · custom prompt |
🤖 Codex Code Review
Overall, the PR’s core intent (avoiding mid-cascade pruning loops) is sound, and I didn’t spot direct fork-choice/justification/finalization logic regressions in this diff beyond the pruning coverage gap above. Automated review by OpenAI Codex · custom prompt |
🤖 Claude Code ReviewNow I have enough context for a full review. PR Review: Defer state/block pruning until after block cascade completesSummary: This is a targeted, well-diagnosed fix for a real production bug. The root cause analysis in the PR description is excellent, and the change is minimal and focused. The fix correctly identifies that mid-cascade pruning can invalidate state that pending children still need. CorrectnessThe fix is correct. The core issue — The split in
Minor ObservationsTick path no longer triggers state/block pruning.
Observability regression (minor): The old code distinguished "Pruned finalized data" (with Memory growth during large cascades: The PR description acknowledges this — states for the full cascade depth are held until the cascade completes. For the devnet4 scenario (~10 slots), this is negligible. In theory a very deep cascade (thousands of slots, as in the chain walk scenario) could hold thousands of states temporarily. This is bounded and correct, just worth monitoring in practice. TestsThe test updates are correct. The new assertion at SummaryThe change is sound. The fix directly addresses the infinite-loop root cause without unnecessary refactoring. The PR description is thorough and the cross-client comparison adds confidence. The main things worth tracking operationally are temporary memory growth during large cascades and the minor observability loss in the pruning log message. Automated review by Claude (Anthropic) · custom prompt |
Greptile SummaryThis PR fixes a critical infinite re-processing loop observed on devnet4 by deferring heavy state/block pruning ( Key changes:
One gap remains: the Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| crates/blockchain/src/lib.rs | Adds store.prune_old_data() call after the on_block cascade loop — correctly deferring heavy pruning. However, the propose_block → process_block path does not pass through on_block and therefore never triggers prune_old_data(), leaving a pruning gap for locally-produced blocks. |
| crates/storage/src/store.rs | Correctly extracts prune_old_data() from update_checkpoints, leaving only lightweight pruning (live chain, gossip signatures, attestation data) inline. New method is well-documented, uses the same protected roots as the old fallback path, and tests are updated to call it explicitly. |
Sequence Diagram
sequenceDiagram
participant Net as Network/Gossip
participant OB as on_block (BlockChainServer)
participant PoPB as process_or_pend_block
participant PB as process_block
participant SOB as store::on_block (on_block_core)
participant UC as update_checkpoints
participant POD as prune_old_data
Net->>OB: NewBlock message
OB->>OB: push block onto queue
loop cascade — while queue not empty
OB->>PoPB: pop block
PoPB->>PB: parent state exists → process_block
PB->>SOB: store::on_block
SOB->>UC: update_checkpoints
UC-->>UC: write head/justified/finalized metadata
UC-->>UC: prune_live_chain (if finalization advances)
UC-->>UC: prune_gossip_signatures (if finalization advances)
UC-->>UC: prune_attestation_data_by_root (if finalization advances)
Note over UC: NO state/block pruning mid-cascade
SOB-->>PoPB: Ok
PoPB->>OB: collect_pending_children → push to queue
end
OB->>POD: prune_old_data() — runs ONCE after cascade
POD-->>POD: prune_old_states (protected: finalized + justified)
POD-->>POD: prune_old_blocks (protected: finalized + justified)
Note over OB,POD: propose_block path goes directly to process_block,<br/>bypassing on_block — prune_old_data() is never called
Comments Outside Diff (1)
-
crates/blockchain/src/lib.rs, line 278-306 (link)propose_blockbypassesprune_old_data()propose_blockcallsself.process_block(...)directly, which goes throughstore::on_block→on_block_core→update_checkpoints, but does not go throughBlockChainServer::on_block. This meansprune_old_data()is never called for locally-proposed blocks.In normal multi-client devnet conditions this is tolerable because blocks from other validators arrive frequently via gossip and each triggers
BlockChainServer::on_block→prune_old_data(). However, in a degraded-network scenario — e.g. a validator-heavy setup where finalization stalls and the node mostly produces its own blocks without receiving many external ones — storage could grow significantly longer than intended between pruning runs.Consider calling
self.store.prune_old_data()at the end ofpropose_block(or restructuring the call path so it always passes through the outeron_block):fn propose_block(&mut self, slot: u64, validator_id: u64) { // ... existing block build / sign / publish logic ... // Prune here so locally-proposed blocks don't bypass the pruning path. self.store.prune_old_data(); }
Prompt To Fix With AI
This is a comment left during a code review. Path: crates/blockchain/src/lib.rs Line: 278-306 Comment: **`propose_block` bypasses `prune_old_data()`** `propose_block` calls `self.process_block(...)` directly, which goes through `store::on_block` → `on_block_core` → `update_checkpoints`, but does **not** go through `BlockChainServer::on_block`. This means `prune_old_data()` is never called for locally-proposed blocks. In normal multi-client devnet conditions this is tolerable because blocks from other validators arrive frequently via gossip and each triggers `BlockChainServer::on_block` → `prune_old_data()`. However, in a degraded-network scenario — e.g. a validator-heavy setup where finalization stalls and the node mostly produces its own blocks without receiving many external ones — storage could grow significantly longer than intended between pruning runs. Consider calling `self.store.prune_old_data()` at the end of `propose_block` (or restructuring the call path so it always passes through the outer `on_block`): ```rust fn propose_block(&mut self, slot: u64, validator_id: u64) { // ... existing block build / sign / publish logic ... // Prune here so locally-proposed blocks don't bypass the pruning path. self.store.prune_old_data(); } ``` How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: crates/blockchain/src/lib.rs
Line: 278-306
Comment:
**`propose_block` bypasses `prune_old_data()`**
`propose_block` calls `self.process_block(...)` directly, which goes through `store::on_block` → `on_block_core` → `update_checkpoints`, but does **not** go through `BlockChainServer::on_block`. This means `prune_old_data()` is never called for locally-proposed blocks.
In normal multi-client devnet conditions this is tolerable because blocks from other validators arrive frequently via gossip and each triggers `BlockChainServer::on_block` → `prune_old_data()`. However, in a degraded-network scenario — e.g. a validator-heavy setup where finalization stalls and the node mostly produces its own blocks without receiving many external ones — storage could grow significantly longer than intended between pruning runs.
Consider calling `self.store.prune_old_data()` at the end of `propose_block` (or restructuring the call path so it always passes through the outer `on_block`):
```rust
fn propose_block(&mut self, slot: u64, validator_id: u64) {
// ... existing block build / sign / publish logic ...
// Prune here so locally-proposed blocks don't bypass the pruning path.
self.store.prune_old_data();
}
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: a3060ad
pablodeymo
force-pushed
the
prune-at-after-sync
branch
from
a3060ad to
22b13dc
Compare
For locally produced blocks there's no cascade (single block in the queue), so prune_old_data() runs immediately after processing — same timing as the old inline The fix targets the cascade path: when a gossip block triggers pending block resolution, the while loop can process many blocks before exiting. Previously, each |
2 participants
Motivation
During the devnet4 run (2026-03-13), all three ethlambda nodes entered an infinite re-processing loop at slot ~15276, generating ~3.5GB of logs each and consuming 100% CPU for hours.
This PR fixes the root cause by deferring heavy state/block pruning until after a block processing cascade completes, so parent states survive long enough for their children to be processed.
Root Cause
The infinite loop is caused by fallback pruning running inside the block processing cascade, deleting states that pending children still need.
The three interacting mechanisms
1. Asymmetric retention creates a state-header gap
When finalization stalls, fallback pruning keeps only
STATES_TO_KEEP=900states butBLOCKS_TO_KEEP=21600headers. Block headers exist in DB without their states.2. Chain walk reaches protected checkpoints
When a block arrives with a missing parent,
process_or_pend_blockwalks ancestor headers looking for one whose parent has state. Protected checkpoints (justified/finalized) always have state, so the walk can reach blocks thousands of slots behind head.3. Mid-cascade pruning deletes just-computed states
on_block_corecallsupdate_checkpointsafter every block, which runsprune_old_states. States for old slots (far behind head) are immediately deleted — even if they were just computed milliseconds ago by the same cascade.The loop
How it was triggered in devnet4
Solution
Defer heavy pruning (states + blocks) until after the block cascade completes.
Before (pruning runs per-block, mid-cascade)
After (pruning deferred to end of cascade)
Split of
update_checkpointsupdate_checkpoints(per-block)prune_live_chainupdate_checkpoints(per-block)prune_gossip_signaturesupdate_checkpoints(per-block)prune_attestation_data_by_rootupdate_checkpoints(per-block)prune_old_statesprune_old_data(after cascade)prune_old_blocksprune_old_data(after cascade)Why this fixes the loop
With deferred pruning, the devnet4 scenario plays out safely:
collect_pending_children(15276)finds 15278 →has_state(parent=15276)→ TRUE (state still exists)whileloop endsprune_old_data()runs once — deletes old statesCross-client validation
We surveyed how other lean consensus clients handle this (Lighthouse, Zeam, Ream, Qlean, Lantern, Grandine). None of them prune states mid-cascade. Common patterns:
Changes
crates/storage/src/store.rs: Splitupdate_checkpoints— extractprune_old_states/prune_old_blocksinto newprune_old_data()method. Lightweight pruning (live chain, signatures, attestation data) stays inupdate_checkpoints.crates/blockchain/src/lib.rs: Callstore.prune_old_data()once after theon_blockwhile loop completes.fallback_pruning_*tests to callprune_old_data()explicitly.How to Test
make test— all 125 tests pass including 27 fork choice spec tests