diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f2bc89fd06..53c5a7f07b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,13 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Fixed an intermittent Zakura sync wedge on a node with a single block-sync + peer by allowing four initial `GetBlocks` probes before an unproven peer is + capped. A peer still has to deliver an accepted body to become proven, and the + normal no-progress liveness deadline still disconnects peers that do not serve + blocks. In addition, on regtest the body-sync stall watchdog now falls back to + the legacy downloader after 60s (rather than the mainnet 10 minutes) so a + stalled node recovers within the regtest e2e budget. - Fixed an out-of-memory crash during Zakura block sync when the header chain runs far ahead of the commit tip. The block-sync applying buffer holds decoded block bodies ahead of the in-order committer; its look-ahead budget counted diff --git a/zebra-network/src/zakura/block_sync/config.rs b/zebra-network/src/zakura/block_sync/config.rs index c283b14f050..9bab017fdbf 100644 --- a/zebra-network/src/zakura/block_sync/config.rs +++ b/zebra-network/src/zakura/block_sync/config.rs @@ -83,7 +83,7 @@ pub const DEFAULT_BS_FLOOR_RESCUE_TIMEOUT: Duration = Duration::from_secs(2); /// Request-timeout windows allowed before block-progress liveness disconnects. const BLOCK_PROGRESS_TIMEOUT_REQUESTS: u32 = 4; /// Default `GetBlocks` probes sent to a new peer before it proves block-body progress. -pub const DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS: u32 = 1; +pub const DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS: u32 = 4; /// Maximum `GetBlocks` requests sent to one peer without an accepted block body. pub const DEFAULT_BS_MAX_REQUESTS_WITHOUT_BLOCK_PROGRESS: u32 = 64; /// Default cooldown before a no-progress peer may be admitted again. diff --git a/zebra-network/src/zakura/block_sync/tests.rs b/zebra-network/src/zakura/block_sync/tests.rs index c19999495fa..fe06165858d 100644 --- a/zebra-network/src/zakura/block_sync/tests.rs +++ b/zebra-network/src/zakura/block_sync/tests.rs @@ -640,6 +640,49 @@ fn block_liveness_uses_probe_cap_until_first_accepted_body() { assert_eq!(window.no_progress_request_cap(), 8); } +#[test] +fn block_liveness_default_probe_cap_allows_four_unproven_requests() { + let config = ZakuraBlockSyncConfig::default(); + let timeout = config.effective_liveness_timeout(); + let now = Instant::now(); + let mut window = DownloadWindow::new(&config); + + for request in 1..DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS { + window.outstanding.push(window_request(request)); + window.arm_liveness(now + Duration::from_millis(request.into()), timeout); + assert_eq!(window.requests_without_block_progress, request); + assert!( + window.requests_without_block_progress < window.no_progress_request_cap(), + "the default lets an unproven peer receive another probe" + ); + } + + window + .outstanding + .push(window_request(DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS)); + window.arm_liveness( + now + Duration::from_millis(DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS.into()), + timeout, + ); + assert_eq!( + window.requests_without_block_progress, + DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS + ); + assert!( + window.requests_without_block_progress >= window.no_progress_request_cap(), + "the fourth unproven request spends the default cap" + ); + assert!( + !window.has_block_progress(), + "extra probes must not mark the peer proven" + ); + assert_eq!( + window.no_progress_request_cap(), + DEFAULT_BS_INITIAL_BLOCK_PROBE_REQUESTS, + "still unproven: the peer stays on the initial probe cap" + ); +} + #[test] fn block_liveness_resuming_after_idle_gets_fresh_deadline() { let timeout = ZakuraBlockSyncConfig::default().effective_liveness_timeout(); @@ -3007,7 +3050,15 @@ async fn block_liveness_disconnects_silent_peer_and_traces_reason() { bs_trace::BLOCK_GET_BLOCKS_SENT, &[ ("requests_without_block_progress", TraceValue::U64(1)), - ("no_progress_request_cap", TraceValue::U64(1)), + ("no_progress_request_cap", TraceValue::U64(4)), + ("block_progress_proven", TraceValue::U64(0)), + ], + ); + reader.table("block_sync").assert_row( + bs_trace::BLOCK_GET_BLOCKS_SENT, + &[ + ("requests_without_block_progress", TraceValue::U64(4)), + ("no_progress_request_cap", TraceValue::U64(4)), ("block_progress_proven", TraceValue::U64(0)), ], ); diff --git a/zebrad/src/components/sync.rs b/zebrad/src/components/sync.rs index f0eccdafc93..7fa2e9c647a 100644 --- a/zebrad/src/components/sync.rs +++ b/zebrad/src/components/sync.rs @@ -298,6 +298,18 @@ const GENESIS_TIMEOUT_RETRY: Duration = Duration::from_secs(10); /// syncer simply finds no new blocks and idles. const ZAKURA_BODY_SYNC_STALL_TIMEOUT: Duration = Duration::from_secs(10 * 60); +/// Regtest override for [`ZAKURA_BODY_SYNC_STALL_TIMEOUT`]. +/// +/// The mainnet timeout (10 minutes) is far longer than the Zakura regtest e2e +/// pr-gate's catch-up budget (~3 minutes), so a node that stalls on Zakura body +/// sync in the pr-gate would never reach the fallback before the harness gives +/// up. Regtest produces blocks on demand (not one per ~75s), so a working node +/// still advances its verified tip every poll and never trips this; a genuinely +/// stalled node falls back to legacy `ChainSync` fast enough for the pr-gate to +/// recover within the run. Falling back early is harmless (see +/// [`ZAKURA_BODY_SYNC_STALL_TIMEOUT`]). +const ZAKURA_BODY_SYNC_STALL_TIMEOUT_REGTEST: Duration = Duration::from_secs(60); + /// How often [`ChainSync::bootstrap_genesis_then_pause`] polls the verified tip /// while watching for Zakura body-sync progress. const ZAKURA_BODY_SYNC_STALL_POLL: Duration = Duration::from_secs(10); @@ -962,10 +974,16 @@ where ); // Number of consecutive idle polls (no credited progress) that trip the - // fallback. `as_secs` is non-zero for both constants, so this is >= 1. - let max_idle_polls = (ZAKURA_BODY_SYNC_STALL_TIMEOUT.as_secs() - / ZAKURA_BODY_SYNC_STALL_POLL.as_secs()) - .max(1); + // fallback. Regtest uses a much shorter timeout so the e2e pr-gate can + // exercise/recover through the fallback within its short catch-up budget. + // `as_secs` is non-zero for both constants, so this is >= 1. + let stall_timeout = if self.is_regtest { + ZAKURA_BODY_SYNC_STALL_TIMEOUT_REGTEST + } else { + ZAKURA_BODY_SYNC_STALL_TIMEOUT + }; + let max_idle_polls = + (stall_timeout.as_secs() / ZAKURA_BODY_SYNC_STALL_POLL.as_secs()).max(1); let initial_tip = self.latest_chain_tip.best_tip_height(); let mut tracker = ZakuraStallTracker::new(initial_tip); @@ -989,7 +1007,7 @@ where warn!( verified_tip = ?verified_height, header_tip = ?header_tip_height, - stall = ?ZAKURA_BODY_SYNC_STALL_TIMEOUT, + stall = ?stall_timeout, "Zakura body sync is not closing the gap to the network tip; legacy \ fallback disabled (legacy_p2p is off), continuing to wait for Zakura" ); @@ -999,7 +1017,7 @@ where warn!( verified_tip = ?verified_height, header_tip = ?header_tip_height, - stall = ?ZAKURA_BODY_SYNC_STALL_TIMEOUT, + stall = ?stall_timeout, "Zakura body sync is not closing the gap to the network tip; stopping \ Zakura sync drivers and falling back to legacy ChainSync so legacy peers \ can drive body sync" diff --git a/zebrad/tests/common/configs/v5.0.0-rc.3.toml b/zebrad/tests/common/configs/v5.0.0-rc.3.toml index e5153db1a40..92765445a5b 100644 --- a/zebrad/tests/common/configs/v5.0.0-rc.3.toml +++ b/zebrad/tests/common/configs/v5.0.0-rc.3.toml @@ -113,7 +113,7 @@ bbr_startup_growth_percent = 200 floor_bypass_slots = 2 floor_peer_avoid_cooldown = "8s" floor_rescue_timeout = "2s" -initial_block_probe_requests = 1 +initial_block_probe_requests = 4 initial_inflight_requests = 64 max_blocks_per_response = 1 max_inflight_block_bytes = 6442450944