From f167b17dc2309bde7910d6037695a59193d61712 Mon Sep 17 00:00:00 2001 From: ilitteri Date: Fri, 21 Aug 2026 17:34:47 -0300 Subject: [PATCH 1/3] Recover instead of stalling when the head's post-state is unreachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a deep reorg unwinds past the window of retained state, the canonical chain and the state history can end up on different branches, leaving no canonical block whose post-state we can still read. Full sync then walked to genesis, found no stateful parent, and returned Ok to pause "until a reconcilable forkchoice head arrives" — a head that can never arrive, because the state is gone rather than merely unreferenced. Meanwhile every forkchoiceUpdated whose finalized block we had not downloaded was rejected with -38002 through an arm that, unlike Syncing and StateNotReachable, never starts a sync, so the missing blocks were never fetched and the node followed the chain no further. Report the exhausted full-sync walk as SyncError::StateUnrecoverable and escalate to snap sync, which is the only in-protocol way to obtain state we do not hold. Treat a safe/finalized block we simply do not have as missing data: start a sync and answer SYNCING, keeping -38002 for elements we do hold but that are ordered wrongly or sit on a disjoint branch. Also report the forkchoice element that is actually absent instead of always naming the finalized one, and correct the comment at the deep-reorg cache-edge bail, which claimed the shallow path should already have succeeded when that path having failed is precisely how execution reaches it. --- crates/blockchain/fork_choice.rs | 78 +++++++++++++++++++-- crates/networking/p2p/sync.rs | 70 +++++++++++++----- crates/networking/p2p/sync/full.rs | 12 ++-- crates/networking/rpc/engine/fork_choice.rs | 14 +++- 4 files changed, 143 insertions(+), 31 deletions(-) diff --git a/crates/blockchain/fork_choice.rs b/crates/blockchain/fork_choice.rs index ff36aa8bbc6..bd2e452781d 100644 --- a/crates/blockchain/fork_choice.rs +++ b/crates/blockchain/fork_choice.rs @@ -99,11 +99,15 @@ pub async fn apply_fork_choice( let head_res = store.get_block_header_by_hash(head_hash)?; if !safe_hash.is_zero() { - check_order(&safe_res, &head_res)?; + check_order(&safe_res, &head_res, error::ForkChoiceElement::Safe)?; } if !finalized_hash.is_zero() && !safe_hash.is_zero() { - check_order(&finalized_res, &safe_res)?; + check_order( + &finalized_res, + &safe_res, + error::ForkChoiceElement::Finalized, + )?; } let Some(head) = head_res else { @@ -261,15 +265,17 @@ pub async fn apply_fork_choice( } // Checks that block 1 is prior to block 2 and that if the second is present, the first one is too. +// `missing` names the element `block_1` was looked up as, so an absent block is reported as the +// element the caller actually asked for. Hardcoding one element here mislabels every other call +// site, which sends whoever reads the log looking at the wrong forkchoice field. fn check_order( block_1: &Option, block_2: &Option, + missing: error::ForkChoiceElement, ) -> Result<(), InvalidForkChoice> { // We don't need to perform the check if the hashes are null match (block_1, block_2) { - (None, Some(_)) => Err(InvalidForkChoice::ElementNotFound( - error::ForkChoiceElement::Finalized, - )), + (None, Some(_)) => Err(InvalidForkChoice::ElementNotFound(missing)), (Some(b1), Some(b2)) => { if b1.number > b2.number { Err(InvalidForkChoice::Unordered) @@ -501,8 +507,12 @@ async fn reorg_apply_deep( .ok_or(InvalidForkChoice::StateNotReachable)?; let to_block = pivot_number.saturating_add(1); if edge < to_block { - // Pivot is above the cache edge ; `apply_fork_choice` should have - // succeeded as a shallow reorg. Bail. + // The overlay range [pivot+1, edge] is empty: no committed block sits above the + // pivot, so no overlay can make the head's state readable again. We only reach here + // after the shallow path already failed with an unreachable head state, so that + // state is genuinely absent locally — never committed, and its in-memory layers + // dropped — rather than merely shadowed by newer commits. Nothing local recovers + // it, so report it and let the caller sync. warn!( edge, to_block, "deep-reorg path entered but pivot is above cache edge" @@ -766,3 +776,57 @@ fn map_chain_error_for_fcu(err: ChainError, last_valid_hash: H256) -> InvalidFor | ChainError::UnknownPayload => InvalidForkChoice::StateNotReachable, } } + +#[cfg(test)] +mod tests { + use super::*; + + fn header(number: u64) -> BlockHeader { + BlockHeader { + number, + ..Default::default() + } + } + + #[test] + fn missing_element_is_reported_as_the_element_looked_up() { + let present = Some(header(10)); + + let err = check_order(&None, &present, error::ForkChoiceElement::Safe).unwrap_err(); + assert!(matches!( + err, + InvalidForkChoice::ElementNotFound(error::ForkChoiceElement::Safe) + )); + + let err = check_order(&None, &present, error::ForkChoiceElement::Finalized).unwrap_err(); + assert!(matches!( + err, + InvalidForkChoice::ElementNotFound(error::ForkChoiceElement::Finalized) + )); + } + + #[test] + fn known_blocks_in_the_wrong_order_are_unordered_not_missing() { + let err = check_order( + &Some(header(20)), + &Some(header(10)), + error::ForkChoiceElement::Safe, + ) + .unwrap_err(); + assert!(matches!(err, InvalidForkChoice::Unordered)); + } + + #[test] + fn known_blocks_in_order_pass() { + for (lower, upper) in [(10, 20), (10, 10)] { + assert!( + check_order( + &Some(header(lower)), + &Some(header(upper)), + error::ForkChoiceElement::Safe, + ) + .is_ok() + ); + } + } +} diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 68e6bc7f8df..c57b040d9e9 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -289,34 +289,59 @@ impl Syncer { ) .await; } - METRICS.enable().await; - // We validate that we have the folders that are being used empty, as we currently assume - // they are. If they are not empty we empty the folder - delete_leaves_folder(&self.datadir); - let sync_cycle_result = snap_sync::sync_cycle_snap( - &mut self.peers, - self.blockchain.clone(), - &self.snap_enabled, - sync_head, - store, - &self.datadir, - &self.diagnostics, - ) - .await; - METRICS.disable().await; - sync_cycle_result + self.run_snap_cycle(sync_head, store).await } else { - full::sync_cycle_full( + let result = full::sync_cycle_full( &mut self.peers, self.blockchain.clone(), self.cancel_token.clone(), sync_head, - store, + store.clone(), &self.diagnostics, ) - .await + .await; + if !matches!(result, Err(SyncError::StateUnrecoverable)) { + return result; + } + // Full sync walked all the way to genesis without finding a single block whose + // post-state we can still read, so it has no base to execute from. This happens + // when the layered store drops the state of every canonical block we hold — + // typically after a deep reorg unwinds past the retained window, leaving the + // canonical chain and the state history pointing at different branches. + // + // Full sync cannot dig itself out: it needs a stateful parent and there is none, + // so every later cycle repeats the same walk and pauses again while the chain + // moves on — the node goes quiet indefinitely. Snap sync is the only in-protocol + // way to obtain state we do not have, so escalate rather than stall until an + // operator notices and runs `ethrex removedb`. + warn!( + %sync_head, + "Full sync has no reachable state to resume from; escalating to snap sync" + ); + self.snap_enabled.store(true, Ordering::Relaxed); + self.run_snap_cycle(sync_head, store).await } } + + /// Runs one snap-sync cycle, enabling snap metrics for its duration. + async fn run_snap_cycle(&mut self, sync_head: H256, store: Store) -> Result<(), SyncError> { + METRICS.enable().await; + // We validate that we have the folders that are being used empty, as we currently assume + // they are. If they are not empty we empty the folder + delete_leaves_folder(&self.datadir); + let sync_cycle_result = snap_sync::sync_cycle_snap( + &mut self.peers, + self.blockchain.clone(), + &self.snap_enabled, + sync_head, + store, + &self.datadir, + &self.diagnostics, + ) + .await; + METRICS.disable().await; + sync_cycle_result + } } /// Number of attempts to fetch the sync head's header for the snap-vs-full pre-check. @@ -394,6 +419,8 @@ pub enum SyncError { CorruptDB, #[error("Failed to fetch latest canonical block, unable to sync")] NoLatestCanonical, + #[error("No block with a reachable post-state to resume full sync from, down to genesis")] + StateUnrecoverable, #[error("Range received is invalid")] InvalidRangeReceived, #[error("Failed to fetch block number for head {0}")] @@ -489,6 +516,11 @@ impl SyncError { | SyncError::BlockNumber(_) | SyncError::NoBlocks | SyncError::NoBlockHeaders => true, + // `sync_cycle` escalates this to snap sync before it can reach the + // classifier, so reaching here means the escalation itself failed to run. + // Retry rather than exit: killing the process does not restore the missing + // state, and the restart path refuses to boot without it. + SyncError::StateUnrecoverable => true, // PeerHandler handled above by delegation SyncError::PeerHandler(_) => unreachable!(), } diff --git a/crates/networking/p2p/sync/full.rs b/crates/networking/p2p/sync/full.rs index 34eee99117e..db1185e1370 100644 --- a/crates/networking/p2p/sync/full.rs +++ b/crates/networking/p2p/sync/full.rs @@ -365,12 +365,16 @@ pub async fn sync_cycle_full( local_head, "Full sync cannot resume: post-state for block {resume_parent_number} is absent \ (pruned from the layered store, or never executed). The consensus sync head does \ - not reconcile to a block whose state we retain; pausing until a reconcilable \ - forkchoice head arrives. If this persists with no state above genesis, the datadir \ - needs a fresh resync (ethrex removedb)." + not reconcile to a block whose state we retain, so there is nothing to execute \ + on top of; handing over to snap sync to rebuild state from a recent pivot." ); store.clear_fullsync_headers().await?; - return Ok(()); + // Returning Ok here would pause the cycle and wait for a forkchoice head that + // reconciles to state we retain. When the walk has already bottomed out at + // genesis no such head exists — the state is gone, not merely unreferenced — + // so the wait never ends and the node stops following the chain entirely. + // Report it so the caller can escalate to snap sync. + return Err(SyncError::StateUnrecoverable); } // If we are resuming at or below the canonical head, the canonical chain extends // past the executed-state head: an FCU canonicalized blocks before their state diff --git a/crates/networking/rpc/engine/fork_choice.rs b/crates/networking/rpc/engine/fork_choice.rs index 7427130b985..97483561a71 100644 --- a/crates/networking/rpc/engine/fork_choice.rs +++ b/crates/networking/rpc/engine/fork_choice.rs @@ -382,10 +382,22 @@ async fn handle_forkchoice( syncer.sync_to_head(fork_choice_state.head_block_hash); ForkChoiceResponse::from(PayloadStatus::syncing()) } - InvalidForkChoice::Disconnected(_, _) | InvalidForkChoice::ElementNotFound(_) => { + InvalidForkChoice::Disconnected(_, _) => { warn!("Invalid fork choice state. Reason: {:?}", forkchoice_error); return Err(RpcErr::InvalidForkChoiceState(forkchoice_error.to_string())); } + InvalidForkChoice::ElementNotFound(_) => { + // A safe/finalized block we simply do not have yet is missing data, not an + // inconsistent forkchoice: -38002 is for elements we DO hold but that are + // ordered wrongly (`Unordered`) or sit on a disjoint branch (`Disconnected`). + // Reporting the hard error here wedges a node that has fallen behind, because + // unlike `Syncing`/`StateNotReachable` this arm never starts a sync: the very + // blocks we are missing are never fetched, so every later FCU fails + // identically and the node stays stuck for as long as the CL keeps asking. + warn!("Fork choice element not found, syncing. Reason: {forkchoice_error:?}"); + syncer.sync_to_head(fork_choice_state.head_block_hash); + ForkChoiceResponse::from(PayloadStatus::syncing()) + } InvalidForkChoice::TooDeepReorg { .. } => { warn!("Rejecting fork choice update. Reason: {forkchoice_error}"); return Err(RpcErr::TooDeepReorg(forkchoice_error.to_string())); From 8b21ed62a6a2d1c9812b382460fb1b3ecf920ce3 Mon Sep 17 00:00:00 2001 From: ilitteri Date: Fri, 28 Aug 2026 18:44:38 -0300 Subject: [PATCH 2/3] fix(l1): keep -38002 for an unknown safe/finalized forkchoice element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An earlier revision of this PR mapped ElementNotFound to a SYNCING response and started a sync, to avoid wedging a node that has fallen behind. But ElementNotFound only fires when the block the missing element is compared against (the head, or the safe block) is already present — the ordering checks run before the head-absent syncing path — so the node is not behind on it. An unknown safe/finalized hash against a known head is the -38002 case the engine spec mandates, which the Hive 'Unknown SafeBlockHash' and 'Unknown FinalizedBlockHash' Cancun tests enforce. The genuine fell-behind wedge this PR targets surfaces as the head-absent Syncing path or StateNotReachable, which are unaffected. The check_order change that names the actually-missing element in the error is kept. --- crates/networking/rpc/engine/fork_choice.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/crates/networking/rpc/engine/fork_choice.rs b/crates/networking/rpc/engine/fork_choice.rs index 97483561a71..9e85b718da2 100644 --- a/crates/networking/rpc/engine/fork_choice.rs +++ b/crates/networking/rpc/engine/fork_choice.rs @@ -382,22 +382,17 @@ async fn handle_forkchoice( syncer.sync_to_head(fork_choice_state.head_block_hash); ForkChoiceResponse::from(PayloadStatus::syncing()) } - InvalidForkChoice::Disconnected(_, _) => { + // A missing safe/finalized element only reaches this arm when the block it + // is compared against (the head, or the safe block) IS present — the checks + // run before the head-absent syncing path. So the node is not behind on that + // block; an unknown safe/finalized hash against a known head is exactly the + // `-38002` case the engine spec mandates (execution-apis, "Unknown + // SafeBlockHash"/"Unknown FinalizedBlockHash"). The genuine fell-behind wedge + // surfaces as the head-absent `Syncing` path or `StateNotReachable`, not here. + InvalidForkChoice::Disconnected(_, _) | InvalidForkChoice::ElementNotFound(_) => { warn!("Invalid fork choice state. Reason: {:?}", forkchoice_error); return Err(RpcErr::InvalidForkChoiceState(forkchoice_error.to_string())); } - InvalidForkChoice::ElementNotFound(_) => { - // A safe/finalized block we simply do not have yet is missing data, not an - // inconsistent forkchoice: -38002 is for elements we DO hold but that are - // ordered wrongly (`Unordered`) or sit on a disjoint branch (`Disconnected`). - // Reporting the hard error here wedges a node that has fallen behind, because - // unlike `Syncing`/`StateNotReachable` this arm never starts a sync: the very - // blocks we are missing are never fetched, so every later FCU fails - // identically and the node stays stuck for as long as the CL keeps asking. - warn!("Fork choice element not found, syncing. Reason: {forkchoice_error:?}"); - syncer.sync_to_head(fork_choice_state.head_block_hash); - ForkChoiceResponse::from(PayloadStatus::syncing()) - } InvalidForkChoice::TooDeepReorg { .. } => { warn!("Rejecting fork choice update. Reason: {forkchoice_error}"); return Err(RpcErr::TooDeepReorg(forkchoice_error.to_string())); From c25c435f16a3b67730d76255036b259a8d0c9468 Mon Sep 17 00:00:00 2001 From: ilitteri Date: Mon, 31 Aug 2026 01:06:15 -0300 Subject: [PATCH 3/3] fix(l1): don't escalate an explicit --syncmode full node to snap, address review Review feedback on the unreachable-state recovery: - Gate the snap-sync escalation on the configured mode. At the recovery point snap_enabled is false both for an explicit --syncmode full node and for a snap-default node that auto-switched to full after initial sync; escalating the former silently runs a snap cycle that wipes the leaves folder, discarding state the operator chose to keep. Thread the configured intent (snap_permitted) down to the Syncer and, when snap is not permitted, surface the unrecoverable state for operator action instead of switching. A snap-default node still escalates as before. - Import ForkChoiceElement instead of spelling error::ForkChoiceElement at every call site (per review). --- crates/blockchain/fork_choice.rs | 32 ++++++++++++--------------- crates/networking/p2p/sync.rs | 30 ++++++++++++++++++++++--- crates/networking/p2p/sync_manager.rs | 7 +++++- 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/crates/blockchain/fork_choice.rs b/crates/blockchain/fork_choice.rs index bd2e452781d..0d210b84343 100644 --- a/crates/blockchain/fork_choice.rs +++ b/crates/blockchain/fork_choice.rs @@ -9,7 +9,7 @@ use tracing::{error, info, warn}; use crate::{ Blockchain, - error::{self, ChainError, InvalidForkChoice}, + error::{ChainError, ForkChoiceElement, InvalidForkChoice}, is_canonical, }; @@ -99,15 +99,11 @@ pub async fn apply_fork_choice( let head_res = store.get_block_header_by_hash(head_hash)?; if !safe_hash.is_zero() { - check_order(&safe_res, &head_res, error::ForkChoiceElement::Safe)?; + check_order(&safe_res, &head_res, ForkChoiceElement::Safe)?; } if !finalized_hash.is_zero() && !safe_hash.is_zero() { - check_order( - &finalized_res, - &safe_res, - error::ForkChoiceElement::Finalized, - )?; + check_order(&finalized_res, &safe_res, ForkChoiceElement::Finalized)?; } let Some(head) = head_res else { @@ -193,8 +189,8 @@ pub async fn apply_fork_choice( || new_canonical_blocks.contains(&(finalized.number, finalized_hash))) { return Err(InvalidForkChoice::Disconnected( - error::ForkChoiceElement::Head, - error::ForkChoiceElement::Finalized, + ForkChoiceElement::Head, + ForkChoiceElement::Finalized, )); } @@ -205,8 +201,8 @@ pub async fn apply_fork_choice( || new_canonical_blocks.contains(&(safe.number, safe_hash))) { return Err(InvalidForkChoice::Disconnected( - error::ForkChoiceElement::Head, - error::ForkChoiceElement::Safe, + ForkChoiceElement::Head, + ForkChoiceElement::Safe, )); } @@ -271,7 +267,7 @@ pub async fn apply_fork_choice( fn check_order( block_1: &Option, block_2: &Option, - missing: error::ForkChoiceElement, + missing: ForkChoiceElement, ) -> Result<(), InvalidForkChoice> { // We don't need to perform the check if the hashes are null match (block_1, block_2) { @@ -792,16 +788,16 @@ mod tests { fn missing_element_is_reported_as_the_element_looked_up() { let present = Some(header(10)); - let err = check_order(&None, &present, error::ForkChoiceElement::Safe).unwrap_err(); + let err = check_order(&None, &present, ForkChoiceElement::Safe).unwrap_err(); assert!(matches!( err, - InvalidForkChoice::ElementNotFound(error::ForkChoiceElement::Safe) + InvalidForkChoice::ElementNotFound(ForkChoiceElement::Safe) )); - let err = check_order(&None, &present, error::ForkChoiceElement::Finalized).unwrap_err(); + let err = check_order(&None, &present, ForkChoiceElement::Finalized).unwrap_err(); assert!(matches!( err, - InvalidForkChoice::ElementNotFound(error::ForkChoiceElement::Finalized) + InvalidForkChoice::ElementNotFound(ForkChoiceElement::Finalized) )); } @@ -810,7 +806,7 @@ mod tests { let err = check_order( &Some(header(20)), &Some(header(10)), - error::ForkChoiceElement::Safe, + ForkChoiceElement::Safe, ) .unwrap_err(); assert!(matches!(err, InvalidForkChoice::Unordered)); @@ -823,7 +819,7 @@ mod tests { check_order( &Some(header(lower)), &Some(header(upper)), - error::ForkChoiceElement::Safe, + ForkChoiceElement::Safe, ) .is_ok() ); diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index 35abb998652..7fc891ec2dc 100644 --- a/crates/networking/p2p/sync.rs +++ b/crates/networking/p2p/sync.rs @@ -168,6 +168,13 @@ pub struct Syncer { /// This is also held by the SyncManager allowing it to track the latest syncmode, without modifying it /// No outside process should modify this value, only being modified by the sync cycle snap_enabled: Arc, + /// Whether the node was configured to allow snap sync (`--syncmode snap`, the default). + /// A snap-default node flips `snap_enabled` to full once it has synced state, so at + /// recovery time `snap_enabled` alone cannot tell an explicitly full-sync node from a + /// snap-default one that switched. This records the configured intent so the + /// unreachable-state recovery only escalates to snap when the operator did not opt out + /// of it — escalating a `--syncmode full` node would wipe state it was told to keep. + snap_permitted: bool, peers: PeerHandler, // Used for cancelling long-living tasks upon shutdown cancel_token: CancellationToken, @@ -182,6 +189,7 @@ impl Syncer { pub fn new( peers: PeerHandler, snap_enabled: Arc, + snap_permitted: bool, cancel_token: CancellationToken, blockchain: Arc, datadir: PathBuf, @@ -189,6 +197,7 @@ impl Syncer { ) -> Self { Self { snap_enabled, + snap_permitted, peers, cancel_token, blockchain, @@ -319,11 +328,26 @@ impl Syncer { // Full sync cannot dig itself out: it needs a stateful parent and there is none, // so every later cycle repeats the same walk and pauses again while the chain // moves on — the node goes quiet indefinitely. Snap sync is the only in-protocol - // way to obtain state we do not have, so escalate rather than stall until an - // operator notices and runs `ethrex removedb`. + // way to obtain state we do not have. + // + // Only escalate when snap sync is permitted (`--syncmode snap`, the default). A + // node explicitly run with `--syncmode full` opted out of snap, and the snap + // cycle wipes the leaves folder to re-heal state from a pivot — silently doing + // that would discard data the operator chose to keep. For those nodes, surface + // the unrecoverable state so the operator can act (e.g. `ethrex removedb`) + // rather than trading a stall for data loss. + if !self.snap_permitted { + warn!( + %sync_head, + "Full sync has no reachable state to resume from, and snap sync is disabled \ + (--syncmode full). Cannot recover in-protocol without discarding retained \ + state; operator intervention required (e.g. `ethrex removedb`)." + ); + return result; + } warn!( %sync_head, - "Full sync has no reachable state to resume from; escalating to snap sync" + "Full sync has no reachable state to resume from; switching to snap sync" ); self.snap_enabled.store(true, Ordering::Relaxed); self.run_snap_cycle(sync_head, store).await diff --git a/crates/networking/p2p/sync_manager.rs b/crates/networking/p2p/sync_manager.rs index aff1cf450e0..d692c79c305 100644 --- a/crates/networking/p2p/sync_manager.rs +++ b/crates/networking/p2p/sync_manager.rs @@ -68,7 +68,11 @@ impl SyncManager { backfill_config: BackfillConfig, tracker: TaskTracker, ) -> Self { - let snap_enabled = Arc::new(AtomicBool::new(matches!(sync_mode, SyncMode::Snap))); + // Whether snap sync is permitted at all, captured from the configured mode before + // the auto-switch below can flip `snap_enabled` to full. The unreachable-state + // recovery uses this to avoid escalating a `--syncmode full` node into snap. + let snap_permitted = matches!(sync_mode, SyncMode::Snap); + let snap_enabled = Arc::new(AtomicBool::new(snap_permitted)); // Clone the shared handles the optional backfill task needs before // `peer_handler`/`cancel_token`/`blockchain` are moved into the Syncer below. @@ -113,6 +117,7 @@ impl SyncManager { let syncer = Arc::new(Mutex::new(Syncer::new( peer_handler, snap_enabled.clone(), + snap_permitted, cancel_token, blockchain, datadir,