diff --git a/crates/blockchain/fork_choice.rs b/crates/blockchain/fork_choice.rs index ff36aa8bbc6..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,11 +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)?; + check_order(&safe_res, &head_res, ForkChoiceElement::Safe)?; } if !finalized_hash.is_zero() && !safe_hash.is_zero() { - check_order(&finalized_res, &safe_res)?; + check_order(&finalized_res, &safe_res, ForkChoiceElement::Finalized)?; } let Some(head) = head_res else { @@ -189,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, )); } @@ -201,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, )); } @@ -261,15 +261,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: 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 +503,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 +772,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, ForkChoiceElement::Safe).unwrap_err(); + assert!(matches!( + err, + InvalidForkChoice::ElementNotFound(ForkChoiceElement::Safe) + )); + + let err = check_order(&None, &present, ForkChoiceElement::Finalized).unwrap_err(); + assert!(matches!( + err, + InvalidForkChoice::ElementNotFound(ForkChoiceElement::Finalized) + )); + } + + #[test] + fn known_blocks_in_the_wrong_order_are_unordered_not_missing() { + let err = check_order( + &Some(header(20)), + &Some(header(10)), + 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)), + ForkChoiceElement::Safe, + ) + .is_ok() + ); + } + } +} diff --git a/crates/networking/p2p/sync.rs b/crates/networking/p2p/sync.rs index ef9f45230b3..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, @@ -296,34 +305,74 @@ 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. + // + // 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; switching 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. @@ -401,6 +450,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}")] @@ -496,6 +547,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 40ad9583fd6..14e88d2a26a 100644 --- a/crates/networking/p2p/sync/full.rs +++ b/crates/networking/p2p/sync/full.rs @@ -406,12 +406,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/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, diff --git a/crates/networking/rpc/engine/fork_choice.rs b/crates/networking/rpc/engine/fork_choice.rs index f225c5e10a9..ceb7196456a 100644 --- a/crates/networking/rpc/engine/fork_choice.rs +++ b/crates/networking/rpc/engine/fork_choice.rs @@ -487,6 +487,13 @@ async fn handle_forkchoice( syncer.sync_to_head(fork_choice_state.head_block_hash); ForkChoiceResponse::from(PayloadStatus::syncing()) } + // 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()));