From cb8b9940490a74919a6377271690d0f10cb0bd8a Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Sat, 27 Jun 2026 02:57:12 -0500 Subject: [PATCH 1/6] perf(network): retain raw block bodies in reorder backlog --- .../src/zakura/block_sync/peer_routine.rs | 56 +++++++----- .../src/zakura/block_sync/reorder.rs | 87 +++++++++++++++++-- .../src/zakura/block_sync/sequencer.rs | 29 ++++++- .../src/zakura/block_sync/sequencer_task.rs | 7 +- zebra-network/src/zakura/block_sync/tests.rs | 64 +++++++++++++- zebra-network/src/zakura/block_sync/wire.rs | 24 ++++- 6 files changed, 227 insertions(+), 40 deletions(-) diff --git a/zebra-network/src/zakura/block_sync/peer_routine.rs b/zebra-network/src/zakura/block_sync/peer_routine.rs index 248ced17b34..a80996baf20 100644 --- a/zebra-network/src/zakura/block_sync/peer_routine.rs +++ b/zebra-network/src/zakura/block_sync/peer_routine.rs @@ -37,6 +37,7 @@ use super::{ reactor::{ block_sync_message_label, bs_insert_height, bs_insert_peer, bs_insert_u64, tolerated_bytes, }, + reorder::BufferedBlockBody, request::{BlockRangeRequest, ExpectedBlock}, sequencer_task::{SequencedBody, SequencerView}, state::{DownloadWindow, OutstandingBlockRange, ReceivedBlockTracker, ThroughputMeter}, @@ -332,25 +333,26 @@ impl PeerRoutine { }; // Measured here, on the per-peer task, so the body size never has to be // recomputed by re-serializing the block on another thread (A1). - let msg = match BlockSyncMessage::decode_frame(frame) { - Ok(msg) => msg, - Err(error) => { - // A malformed frame is `MalformedMessage` misbehavior AND a fatal - // protocol reject for the whole connection (matches the previous - // `run_peer` decode-error path). Report via the shared channel, - // then reject; the report is best-effort and never blocks. - let protocol_error = - std::io::Error::new(std::io::ErrorKind::InvalidData, error.to_string()); - tracing::debug!(peer = ?self.peer, ?error, "malformed Zakura block-sync frame"); - let _ = self - .routine_to_reactor - .try_send(RoutineToReactor::Misbehavior { - peer: self.peer.clone(), - reason: BlockSyncMisbehavior::MalformedMessage, - }); - return Err(SinkReject::protocol(protocol_error)); - } - }; + let (msg, raw_block_payload) = + match BlockSyncMessage::decode_frame_with_raw_block_payload(frame) { + Ok(decoded) => decoded, + Err(error) => { + // A malformed frame is `MalformedMessage` misbehavior AND a fatal + // protocol reject for the whole connection (matches the previous + // `run_peer` decode-error path). Report via the shared channel, + // then reject; the report is best-effort and never blocks. + let protocol_error = + std::io::Error::new(std::io::ErrorKind::InvalidData, error.to_string()); + tracing::debug!(peer = ?self.peer, ?error, "malformed Zakura block-sync frame"); + let _ = self + .routine_to_reactor + .try_send(RoutineToReactor::Misbehavior { + peer: self.peer.clone(), + reason: BlockSyncMisbehavior::MalformedMessage, + }); + return Err(SinkReject::protocol(protocol_error)); + } + }; let body_wire_bytes = msg.block_body_wire_bytes(frame_payload_bytes); self.trace_message_received(&msg); @@ -372,7 +374,8 @@ impl PeerRoutine { } BlockSyncMessage::Block(block) => { self.trace_wake("own_body"); - self.handle_body(block, body_wire_bytes, body_permit).await; + self.handle_body(block, body_wire_bytes, body_permit, raw_block_payload) + .await; } BlockSyncMessage::BlocksDone { start_height, @@ -798,6 +801,7 @@ impl PeerRoutine { block: Arc, body_wire_bytes: Option, body_permit: Option>, + raw_block_payload: Option>, ) { let hash = block.hash(); let Some(height) = block.coinbase_height() else { @@ -818,6 +822,7 @@ impl PeerRoutine { block.clone(), body_wire_bytes, body_permit, + raw_block_payload.clone(), ) .await { @@ -911,7 +916,8 @@ impl PeerRoutine { // the routine: a slow verifier blocks the task draining input, the bounded // input channel fills, and this routine blocks here — backpressure // isolated to this peer (the per-peer routines throughput win). - self.forward_body_to_sequencer(height, hash, block, serialized_bytes, body_permit) + let body = BufferedBlockBody::from_decoded_block(block, raw_block_payload); + self.forward_body_to_sequencer(height, hash, body, serialized_bytes, body_permit) .await; // This body opened only this peer's slots; the want-work loop runs at the // top of the next iteration. @@ -939,7 +945,7 @@ impl PeerRoutine { &self, height: block::Height, hash: block::Hash, - block: Arc, + body: BufferedBlockBody, serialized_bytes: u64, body_permit: Option>, ) { @@ -948,7 +954,7 @@ impl PeerRoutine { let body = SequencedBody { height, hash, - block, + body, bytes: serialized_bytes, peer: self.peer.clone(), received_at, @@ -984,6 +990,7 @@ impl PeerRoutine { block: Arc, body_wire_bytes: Option, body_permit: Option>, + raw_block_payload: Option>, ) -> bool { if self.work.hash_for_height(height) != Some(hash) { return false; @@ -1033,7 +1040,8 @@ impl PeerRoutine { // later duplicate. let _ = self.work.take_in_range(height, height, 1); - self.forward_body_to_sequencer(height, hash, block, serialized_bytes, body_permit) + let body = BufferedBlockBody::from_decoded_block(block, raw_block_payload); + self.forward_body_to_sequencer(height, hash, body, serialized_bytes, body_permit) .await; true } diff --git a/zebra-network/src/zakura/block_sync/reorder.rs b/zebra-network/src/zakura/block_sync/reorder.rs index 5b3e78ed4a3..eefe9f94315 100644 --- a/zebra-network/src/zakura/block_sync/reorder.rs +++ b/zebra-network/src/zakura/block_sync/reorder.rs @@ -1,4 +1,4 @@ -use super::{state::*, *}; +use super::{state::*, wire::BLOCK_SYNC_MESSAGE_TYPE_BYTES, *}; #[derive(Clone, Debug)] pub(crate) struct ReorderBuffer { @@ -31,9 +31,7 @@ impl ReorderBuffer { } pub(super) fn hash(&self, height: block::Height) -> Option { - self.blocks - .get(&height) - .map(|buffered| buffered.block.hash()) + self.blocks.get(&height).map(|buffered| buffered.hash) } /// Buffer a received body that already owns its `bytes` reservation. @@ -42,12 +40,32 @@ impl ReorderBuffer { /// shrank that reservation to `bytes` on receipt, so the reorder buffer takes /// ownership of the existing reservation without touching the budget and can /// never fail on budget. A `Duplicate` height is left to the caller to release. + #[cfg(test)] pub(super) fn insert( &mut self, height: block::Height, block: Arc, bytes: u64, source_peer: ZakuraPeerId, + ) -> ReorderInsertResult { + self.insert_body( + height, + block.hash(), + BufferedBlockBody::Decoded(block), + bytes, + source_peer, + ) + } + + /// Buffer a received body, keeping raw block bytes when the peer routine can + /// provide them so non-contiguous backlog does not retain decoded blocks. + pub(super) fn insert_body( + &mut self, + height: block::Height, + hash: block::Hash, + body: BufferedBlockBody, + bytes: u64, + source_peer: ZakuraPeerId, ) -> ReorderInsertResult { if self.blocks.contains_key(&height) { return ReorderInsertResult::Duplicate; @@ -56,7 +74,8 @@ impl ReorderBuffer { self.blocks.insert( height, BufferedBlock { - block, + hash, + body, bytes, source_peer, }, @@ -77,7 +96,12 @@ impl ReorderBuffer { while let Some(buffered) = self.blocks.remove(&next) { self.buffered_bytes = self.buffered_bytes.saturating_sub(buffered.bytes); - released.push((next, buffered.block, buffered.bytes, buffered.source_peer)); + released.push(( + next, + buffered.body.into_block(), + buffered.bytes, + buffered.source_peer, + )); let Some(after) = next_height(next) else { break; }; @@ -138,9 +162,58 @@ pub(super) enum ReorderInsertResult { #[derive(Clone, Debug)] struct BufferedBlock { - block: Arc, + hash: block::Hash, + body: BufferedBlockBody, bytes: u64, /// The peer that delivered this body, so an apply rejection can be attributed /// back to it for misbehavior scoring. source_peer: ZakuraPeerId, } + +#[derive(Clone, Debug)] +pub(super) enum BufferedBlockBody { + RawFramePayload(Arc<[u8]>), + Decoded(Arc), + DecodedWithRawFramePayload { + block: Arc, + raw_frame_payload: Arc<[u8]>, + }, +} + +impl BufferedBlockBody { + pub(super) fn from_decoded_block( + block: Arc, + raw_frame_payload: Option>, + ) -> Self { + match raw_frame_payload { + Some(raw_frame_payload) => BufferedBlockBody::DecodedWithRawFramePayload { + block, + raw_frame_payload, + }, + None => BufferedBlockBody::Decoded(block), + } + } + + pub(super) fn retain_for_backlog(self) -> Self { + match self { + BufferedBlockBody::DecodedWithRawFramePayload { + raw_frame_payload, .. + } => BufferedBlockBody::RawFramePayload(raw_frame_payload), + body => body, + } + } + + fn into_block(self) -> Arc { + match self { + BufferedBlockBody::Decoded(block) => block, + BufferedBlockBody::DecodedWithRawFramePayload { block, .. } => block, + BufferedBlockBody::RawFramePayload(payload) => { + let mut reader = Cursor::new(&payload[BLOCK_SYNC_MESSAGE_TYPE_BYTES..]); + Arc::new( + block::Block::zcash_deserialize(&mut reader) + .expect("raw block bytes deserialize because the peer routine decoded them before buffering"), + ) + } + } + } +} diff --git a/zebra-network/src/zakura/block_sync/sequencer.rs b/zebra-network/src/zakura/block_sync/sequencer.rs index df966c039ce..626f09f8267 100644 --- a/zebra-network/src/zakura/block_sync/sequencer.rs +++ b/zebra-network/src/zakura/block_sync/sequencer.rs @@ -206,6 +206,7 @@ impl Sequencer { /// Offer a received body to the commit pipeline. Runs the redundancy checks /// and (when not redundant) buffers it in the reorder buffer, which takes /// ownership of the body's existing `bytes` reservation. + #[cfg(test)] pub(super) fn accept_body( &mut self, height: block::Height, @@ -213,6 +214,23 @@ impl Sequencer { block: Arc, bytes: u64, source_peer: ZakuraPeerId, + ) -> AcceptOutcome { + self.accept_buffered_body( + height, + hash, + BufferedBlockBody::Decoded(block), + bytes, + source_peer, + ) + } + + pub(super) fn accept_buffered_body( + &mut self, + height: block::Height, + hash: block::Hash, + body: BufferedBlockBody, + bytes: u64, + source_peer: ZakuraPeerId, ) -> AcceptOutcome { if height <= self.body_download_floor || self.reorder.contains(height) @@ -224,7 +242,16 @@ impl Sequencer { }; } - match self.reorder.insert(height, block, bytes, source_peer) { + let body = if next_height(self.body_download_floor) == Some(height) { + body + } else { + body.retain_for_backlog() + }; + + match self + .reorder + .insert_body(height, hash, body, bytes, source_peer) + { ReorderInsertResult::Inserted => AcceptOutcome::Buffered { covered: height }, ReorderInsertResult::Duplicate => AcceptOutcome::Redundant { release_bytes: bytes, diff --git a/zebra-network/src/zakura/block_sync/sequencer_task.rs b/zebra-network/src/zakura/block_sync/sequencer_task.rs index 82116c07afd..7682bbd5ef5 100644 --- a/zebra-network/src/zakura/block_sync/sequencer_task.rs +++ b/zebra-network/src/zakura/block_sync/sequencer_task.rs @@ -17,6 +17,7 @@ use super::{ events::*, reactor::{bs_insert_height, bs_insert_u64}, + reorder::BufferedBlockBody, sequencer::*, state::*, work_queue::WorkQueue, @@ -31,7 +32,7 @@ use super::{ pub(super) struct SequencedBody { pub(super) height: block::Height, pub(super) hash: block::Hash, - pub(super) block: Arc, + pub(super) body: BufferedBlockBody, pub(super) bytes: u64, pub(super) peer: ZakuraPeerId, pub(super) received_at: Instant, @@ -264,10 +265,10 @@ impl SequencerTask { /// `Redundant`, then drain ready prefix into applying and submit. async fn handle_accept_body(&mut self, body: SequencedBody) { let queued_elapsed = body.received_at.elapsed(); - let outcome = match self.sequencer.accept_body( + let outcome = match self.sequencer.accept_buffered_body( body.height, body.hash, - body.block, + body.body, body.bytes, body.peer, ) { diff --git a/zebra-network/src/zakura/block_sync/tests.rs b/zebra-network/src/zakura/block_sync/tests.rs index 19be01bd776..7258c3187e9 100644 --- a/zebra-network/src/zakura/block_sync/tests.rs +++ b/zebra-network/src/zakura/block_sync/tests.rs @@ -41,6 +41,14 @@ fn mainnet_blocks_1_to_3() -> Vec> { ] } +fn raw_block_payload(block: &Arc) -> Arc<[u8]> { + let frame = BlockSyncMessage::Block(block.clone()) + .encode_frame() + .expect("test block frame encodes"); + + Arc::from(frame.payload.into_boxed_slice()) +} + fn forked_block(block: &Arc, nonce_tag: u8) -> Arc { let mut fork = block.as_ref().clone(); let mut header = *fork.header; @@ -1941,6 +1949,51 @@ fn sequencer_accept_body_buffers_then_reports_duplicate() { ); } +#[test] +fn sequencer_retains_raw_bytes_for_non_contiguous_backlog() { + let mut seq = test_sequencer(0, 4); + let blocks = mainnet_blocks_1_to_3(); + let block1 = blocks[0].clone(); + let block2 = blocks[1].clone(); + let distinguishable_decoded_block2 = forked_block(&block2, 99); + + assert_ne!(distinguishable_decoded_block2.hash(), block2.hash()); + assert_eq!( + distinguishable_decoded_block2.coinbase_height(), + block2.coinbase_height() + ); + + let block2_body = BufferedBlockBody::from_decoded_block( + distinguishable_decoded_block2.clone(), + Some(raw_block_payload(&block2)), + ); + + assert_eq!( + seq.accept_buffered_body(block::Height(2), block2.hash(), block2_body, 200, peer(0)), + AcceptOutcome::Buffered { + covered: block::Height(2) + } + ); + assert!(seq.drain_ready_into_applying().is_empty()); + assert!(seq.reorder_contains(block::Height(2))); + + assert_eq!( + seq.accept_body(block::Height(1), block1.hash(), block1, 100, peer(0)), + AcceptOutcome::Buffered { + covered: block::Height(1) + } + ); + assert_eq!( + seq.drain_ready_into_applying(), + vec![block::Height(1), block::Height(2)] + ); + assert_eq!(seq.applying_hash(block::Height(2)), Some(block2.hash())); + assert_ne!( + seq.applying_hash(block::Height(2)), + Some(distinguishable_decoded_block2.hash()) + ); +} + #[test] fn sequencer_accept_body_rejects_at_or_below_floor() { let mut seq = test_sequencer(5, 4); @@ -3799,7 +3852,8 @@ async fn reactor_ignores_unmatched_body_for_currently_needed_height() { #[tokio::test] async fn reactor_accepts_unmatched_body_for_queued_height() { let blocks = mainnet_blocks_1_to_3(); - let config = immediate_body_download_config(); + let mut config = immediate_body_download_config(); + config.max_inflight_block_bytes = u64::from(block_size(&blocks[0])); let (_tip_tx, tip_rx) = watch::channel((block::Height(1), blocks[0].hash())); let startup = BlockSyncStartup::new( BlockSyncFrontiers { @@ -5485,10 +5539,14 @@ async fn checkpoint_hole_disconnect_retries_first_missing_height_with_fresh_peer } }); - let mut requests = Vec::new(); + let mut requests: Vec<(block::Height, u32)> = Vec::new(); let mut submitted = std::collections::HashSet::new(); let primed = tokio::time::timeout(Duration::from_secs(40), async { - while requests.len() < 4 || !prefix.is_subset(&submitted) { + while !prefix.is_subset(&submitted) + || !requests.iter().any(|(start, count)| { + *start <= block::Height(HOLE_START) && start.0.saturating_add(*count) > HOLE_END + }) + { // The old peer's `GetBlocks` arrive on its own real outbound (reading // that stream proves they targeted it); needed-block queries and // submissions come over the action channel. diff --git a/zebra-network/src/zakura/block_sync/wire.rs b/zebra-network/src/zakura/block_sync/wire.rs index 2be2eb458e8..3a49676a2b4 100644 --- a/zebra-network/src/zakura/block_sync/wire.rs +++ b/zebra-network/src/zakura/block_sync/wire.rs @@ -172,19 +172,39 @@ impl BlockSyncMessage { /// Decode this message from a Zakura frame after checking flags and type agreement. pub fn decode_frame(frame: Frame) -> Result { + Self::decode_frame_with_raw_block_payload(frame).map(|(message, _)| message) + } + + /// Decode this message and return the raw frame payload for block bodies. + /// + /// The returned raw payload includes the stream-6 message type byte. Keeping + /// the whole payload lets the decoder move the frame allocation into the + /// reorder buffer without copying; consumers skip + /// [`BLOCK_SYNC_MESSAGE_TYPE_BYTES`] before deserializing the block body. + pub(super) fn decode_frame_with_raw_block_payload( + frame: Frame, + ) -> Result<(Self, Option>), BlockSyncWireError> { if frame.flags != 0 { return Err(BlockSyncWireError::UnsupportedFlags(frame.flags)); } - let message = Self::decode(&frame.payload)?; let frame_message_type = u8::try_from(frame.message_type) .map_err(|_| BlockSyncWireError::UnknownFrameMessageType(frame.message_type))?; + + let (message, raw_block_payload) = if frame_message_type == MSG_BS_BLOCK { + let raw_block_payload = Arc::<[u8]>::from(frame.payload.into_boxed_slice()); + let message = Self::decode(&raw_block_payload)?; + (message, Some(raw_block_payload)) + } else { + (Self::decode(&frame.payload)?, None) + }; + if frame_message_type != message.message_type() { return Err(BlockSyncWireError::MismatchedFrameMessageType { frame: frame.message_type, payload: message.message_type(), }); } - Ok(message) + Ok((message, raw_block_payload)) } /// Exact serialized length of a `Block` body, derived from the frame payload From 00fe3fdcdd69c5f52205c013a62e9dcb2ddffbdc Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 26 Jun 2026 20:11:37 -0500 Subject: [PATCH 2/6] fix(network): prioritize block sync floor requests --- .../src/zakura/block_sync/reorder.rs | 7 ++ .../src/zakura/block_sync/sequencer.rs | 5 + .../src/zakura/block_sync/sequencer_task.rs | 102 ++++++++++++++++-- zebra-network/src/zakura/block_sync/tests.rs | 63 +++++++++++ 4 files changed, 167 insertions(+), 10 deletions(-) diff --git a/zebra-network/src/zakura/block_sync/reorder.rs b/zebra-network/src/zakura/block_sync/reorder.rs index eefe9f94315..18162d4dc94 100644 --- a/zebra-network/src/zakura/block_sync/reorder.rs +++ b/zebra-network/src/zakura/block_sync/reorder.rs @@ -22,6 +22,13 @@ impl ReorderBuffer { self.blocks.len() } + /// Highest buffered height, if any. The shed-for-floor-starvation path drops + /// this (the body furthest from the committed floor) to free budget for a + /// lower, commit-unblocking request. + pub(super) fn max_height(&self) -> Option { + self.blocks.keys().next_back().copied() + } + pub(super) fn contains(&self, height: block::Height) -> bool { self.blocks.contains_key(&height) } diff --git a/zebra-network/src/zakura/block_sync/sequencer.rs b/zebra-network/src/zakura/block_sync/sequencer.rs index 626f09f8267..cdf0aaf4740 100644 --- a/zebra-network/src/zakura/block_sync/sequencer.rs +++ b/zebra-network/src/zakura/block_sync/sequencer.rs @@ -133,6 +133,11 @@ impl Sequencer { self.reorder.buffered_bytes() } + /// Highest buffered reorder height, for shed-for-floor-starvation. + pub(super) fn reorder_max_height(&self) -> Option { + self.reorder.max_height() + } + pub(super) fn unsubmitted_applying_count(&self) -> usize { self.applying .values() diff --git a/zebra-network/src/zakura/block_sync/sequencer_task.rs b/zebra-network/src/zakura/block_sync/sequencer_task.rs index 7682bbd5ef5..07cdead9d45 100644 --- a/zebra-network/src/zakura/block_sync/sequencer_task.rs +++ b/zebra-network/src/zakura/block_sync/sequencer_task.rs @@ -24,6 +24,53 @@ use super::{ *, }; +/// How often the Sequencer task checks whether the byte budget is starving the +/// commit-unblocking (lowest pending) height and sheds the speculative top of the +/// reorder buffer to fund it. Bounds the recovery latency when no bodies are +/// flowing to trigger the inline check (e.g. once outstanding requests drain). +const FLOOR_STARVATION_SHED_INTERVAL: Duration = Duration::from_millis(500); + +/// Favor the lowest re-requestable height over the speculative high tail. +/// +/// While the byte budget cannot fund even one worst-case request yet the lowest +/// needed height (the commit-unblocking floor gap) is pending *below* the highest +/// buffered body, drop that top body: release its bytes to the budget and return +/// its height to `pending` (it was held, hence in `work.in_flight` per the +/// `held ⟺ in_flight` invariant) for later re-fetch. Because another top can +/// always be shed, a low retry never blocks on budget — the floor can never wedge +/// behind a full buffer — and under a stall the speculative tail is shed and the +/// chain fills bottom-up, which also bounds the reorder backlog. Returns whether +/// it shed anything. +pub(super) fn shed_top_for_floor_starvation( + budget: &mut ByteBudget, + work: &WorkQueue, + sequencer: &mut Sequencer, +) -> bool { + let worst = super::config::BS_PER_BLOCK_WORST_CASE_BYTES; + let mut shed_any = false; + while budget.available() < worst { + let Some(lowest_pending) = work.min_pending() else { + break; + }; + let Some(top) = sequencer.reorder_max_height() else { + break; + }; + // Only shed a body that sits above a starved lower height: we trade a + // far-from-floor body for the ability to fetch a nearer, higher-value one. + if lowest_pending >= top { + break; + } + let freed = sequencer.drop_reorder_from(top); + if freed == 0 { + break; + } + budget.release(freed); + work.return_items([top]); + shed_any = true; + } + shed_any +} + /// A received body a peer routine matched (or accepted unmatched) and forwards /// to the commit pipeline. This is the only bounded Sequencer input: a slow /// verifier can backpressure body intake, but must not block apply/frontier @@ -186,25 +233,60 @@ impl SequencerTask { } pub(super) async fn run(mut self) { + // Periodic shed backstop: catches budget starvation of the floor even when + // no bodies/control events are arriving to trigger the inline checks. + let mut shed_tick = tokio::time::interval(FLOOR_STARVATION_SHED_INTERVAL); + shed_tick.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay); + // Track input closure explicitly: the always-ready shed timer means the + // `select!` never falls through to an `else`, so shut down only once both + // input channels have closed. + let mut control_open = true; + let mut body_open = true; loop { + if !control_open && !body_open { + break; + } tokio::select! { biased; - Some(input) = self.control_input_rx.recv() => { - let needs_reaction = self.handle_control_input(input).await; - if needs_reaction { - self.reaction_epoch = self.reaction_epoch.saturating_add(1); + input = self.control_input_rx.recv(), if control_open => { + match input { + Some(input) => { + let needs_reaction = self.handle_control_input(input).await; + if needs_reaction { + self.reaction_epoch = self.reaction_epoch.saturating_add(1); + } + self.publish_view(); + } + None => control_open = false, } - self.publish_view(); } - Some(body) = self.body_input_rx.recv() => { - self.release_body_input_bytes(body.bytes); - self.handle_accept_body(body).await; - self.publish_view(); + body = self.body_input_rx.recv(), if body_open => { + match body { + Some(body) => { + self.release_body_input_bytes(body.bytes); + self.handle_accept_body(body).await; + shed_top_for_floor_starvation( + &mut self.budget, + &self.work, + &mut self.sequencer, + ); + self.publish_view(); + } + None => body_open = false, + } } - else => break, + _ = shed_tick.tick() => { + if shed_top_for_floor_starvation( + &mut self.budget, + &self.work, + &mut self.sequencer, + ) { + self.publish_view(); + } + } } } } diff --git a/zebra-network/src/zakura/block_sync/tests.rs b/zebra-network/src/zakura/block_sync/tests.rs index 7258c3187e9..c817f54cd65 100644 --- a/zebra-network/src/zakura/block_sync/tests.rs +++ b/zebra-network/src/zakura/block_sync/tests.rs @@ -1921,6 +1921,69 @@ fn reorder_drains_only_contiguous_prefix_without_releasing_budget() { assert_eq!(budget.reserved(), 0); } +#[test] +fn shed_top_for_floor_starvation_funds_lowest_pending_by_dropping_top() { + let worst = BS_PER_BLOCK_WORST_CASE_BYTES; + // Budget holds exactly two worst-case blocks, both consumed by buffered bodies + // (heights 5 and 6). Height 1 — the commit-unblocking floor gap — is pending + // and unfunded: this is the "download ran ahead, budget full, low height + // needs (re-)requesting" shape that wedged sync. + let mut budget = ByteBudget::new(2 * worst); + let work = work_queue_with( + 0, + [ + needed(1, BlockSizeEstimate::Unknown), + needed(5, BlockSizeEstimate::Unknown), + needed(6, BlockSizeEstimate::Unknown), + ], + ); + let mut seq = test_sequencer(0, 100); + let block = mainnet_block(&BLOCK_MAINNET_1_BYTES); + + // Heights 5 and 6 are taken (pending -> in_flight) and buffered, each holding a + // worst-case block of budget; height 1 stays pending and unfunded. + work.take_in_range(block::Height(5), block::Height(6), 2); + for height in [5u32, 6] { + assert!(budget.try_reserve(worst)); + seq.accept_body( + block::Height(height), + block::Hash([height as u8; 32]), + block.clone(), + worst, + peer(0), + ); + } + assert_eq!(budget.available(), 0, "budget saturated by buffered bodies"); + assert!(work.pending_contains(block::Height(1))); + + // Shedding drops the top buffered body (6) — the one furthest from the floor — + // releasing its budget and returning its height to `pending` for later + // re-fetch, so the lower floor-gap request can now be funded. Without this the + // budget stays full and height 1 can never be requested (the wedge). + let shed = super::sequencer_task::shed_top_for_floor_starvation(&mut budget, &work, &mut seq); + assert!(shed, "the top buffered body is shed"); + assert!( + budget.available() >= worst, + "freed budget can now fund the floor-gap request" + ); + assert!( + !seq.reorder_contains(block::Height(6)), + "the top body is evicted" + ); + assert!( + seq.reorder_contains(block::Height(5)), + "the lower buffered body is kept" + ); + assert!( + work.pending_contains(block::Height(1)), + "the floor gap is still pending and now fundable" + ); + assert!( + work.pending_contains(block::Height(6)), + "the evicted height is returned to pending for re-fetch" + ); +} + // ---- Sequencer commit pipeline ---- fn test_sequencer(verified_tip: u32, submitted_apply_limit: usize) -> Sequencer { From a7e79f34622c5955b47c6d7543b0383fbae465a6 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 26 Jun 2026 20:18:08 -0500 Subject: [PATCH 3/6] fix(network): add block sync congestion control --- zebra-network/src/config.rs | 4 + zebra-network/src/config/tests/vectors.rs | 18 + .../src/zakura/block_sync/admission.rs | 87 ++ zebra-network/src/zakura/block_sync/config.rs | 102 +- zebra-network/src/zakura/block_sync/mod.rs | 3 +- .../src/zakura/block_sync/peer_registry.rs | 103 +- .../src/zakura/block_sync/peer_routine.rs | 429 +++++++-- .../src/zakura/block_sync/reactor.rs | 99 +- .../src/zakura/block_sync/request.rs | 6 +- .../src/zakura/block_sync/sequencer_task.rs | 107 ++- .../src/zakura/block_sync/service.rs | 1 + zebra-network/src/zakura/block_sync/state.rs | 244 ++++- zebra-network/src/zakura/block_sync/tests.rs | 907 ++++++++++++++++-- .../src/zakura/block_sync/work_queue.rs | 258 ++++- zebra-network/src/zakura/trace.rs | 2 + zebra-network/src/zakura/transport/guard.rs | 69 +- zebra-state/src/request.rs | 4 +- 17 files changed, 2114 insertions(+), 329 deletions(-) create mode 100644 zebra-network/src/zakura/block_sync/admission.rs diff --git a/zebra-network/src/config.rs b/zebra-network/src/config.rs index 6d82ed5c89c..e49bfe6049b 100644 --- a/zebra-network/src/config.rs +++ b/zebra-network/src/config.rs @@ -1050,6 +1050,10 @@ impl<'de> Deserialize<'de> for Config { non_zero_config_field.filter(|config_value| config_value > &0).unwrap_or(default_config_value) }); + zakura.block_sync.validate().map_err(|error| { + de::Error::custom(format!("invalid zakura.block_sync config: {error}")) + })?; + Ok(Config { listen_addr: canonical_socket_addr(listen_addr), external_addr: external_socket_addr, diff --git a/zebra-network/src/config/tests/vectors.rs b/zebra-network/src/config/tests/vectors.rs index 397def3b05a..c52b69955d0 100644 --- a/zebra-network/src/config/tests/vectors.rs +++ b/zebra-network/src/config/tests/vectors.rs @@ -228,6 +228,24 @@ fn p2p_v2_unknown_future_config_fields_are_rejected() { ); } +#[test] +fn p2p_v2_block_sync_config_validation_rejects_degenerate_values() { + let _init_guard = zebra_test::init(); + + let err = toml::from_str::( + r#" + [zakura.block_sync] + max_inflight_block_bytes = 0 + "#, + ) + .expect_err("top-level network config must validate Zakura block-sync settings"); + + assert!( + err.to_string().contains("invalid zakura.block_sync config"), + "unexpected block-sync config validation error: {err}", + ); +} + #[test] fn p2p_v2_config_roundtrip_keeps_dconfig_zakura_fields() { let _init_guard = zebra_test::init(); diff --git a/zebra-network/src/zakura/block_sync/admission.rs b/zebra-network/src/zakura/block_sync/admission.rs new file mode 100644 index 00000000000..b93ed10e9be --- /dev/null +++ b/zebra-network/src/zakura/block_sync/admission.rs @@ -0,0 +1,87 @@ +use zebra_chain::block; + +use super::{config::ZakuraBlockSyncConfig, state::next_height}; + +/// Pure inputs for deciding whether a block request may consume budget. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub(super) struct AdmissionSnapshot { + pub(super) download_floor: block::Height, + pub(super) reorder_buffered_bytes: u64, + pub(super) reorder_buffered_blocks: u64, + pub(super) applying_buffered_bytes: u64, + pub(super) applying_buffered_blocks: u64, + pub(super) sequencer_input_queued_bytes: u64, + pub(super) reserved_above_floor_bytes: u64, + pub(super) reserved_above_floor_blocks: u64, + pub(super) budget_available: u64, +} + +/// Whether a request is rescuing the current floor or speculating above it. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub(super) enum RequestPriority { + Floor, + AboveFloor, +} + +/// Admission result for one candidate request. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub(super) struct AdmissionDecision { + pub(super) priority: RequestPriority, + pub(super) max_request_bytes: u64, +} + +pub(super) fn floor_rescue_high(download_floor: block::Height) -> block::Height { + next_height(download_floor).unwrap_or(download_floor) +} + +pub(super) fn request_priority( + download_floor: block::Height, + start_height: block::Height, +) -> RequestPriority { + if start_height <= floor_rescue_high(download_floor) { + RequestPriority::Floor + } else { + RequestPriority::AboveFloor + } +} + +pub(super) fn admission_decision( + config: &ZakuraBlockSyncConfig, + snapshot: AdmissionSnapshot, + start_height: block::Height, + response_byte_cap: u64, +) -> Option { + let priority = request_priority(snapshot.download_floor, start_height); + let max_request_bytes = match priority { + RequestPriority::Floor => snapshot.budget_available.min(response_byte_cap), + RequestPriority::AboveFloor => { + let held_bytes = snapshot + .reorder_buffered_bytes + .saturating_add(snapshot.applying_buffered_bytes) + .saturating_add(snapshot.sequencer_input_queued_bytes) + .saturating_add(snapshot.reserved_above_floor_bytes); + let held_blocks = snapshot + .reorder_buffered_blocks + .saturating_add(snapshot.applying_buffered_blocks) + .saturating_add(snapshot.reserved_above_floor_blocks); + if held_bytes >= config.effective_max_reorder_lookahead_bytes() + || held_blocks >= u64::from(config.max_reorder_lookahead_blocks) + { + return None; + } + + let remaining_lookahead_bytes = config + .effective_max_reorder_lookahead_bytes() + .saturating_sub(held_bytes); + snapshot + .budget_available + .min(remaining_lookahead_bytes) + .min(response_byte_cap) + } + }; + + (max_request_bytes > 0).then_some(AdmissionDecision { + priority, + max_request_bytes, + }) +} diff --git a/zebra-network/src/zakura/block_sync/config.rs b/zebra-network/src/zakura/block_sync/config.rs index 53c5f13e608..a947ccada19 100644 --- a/zebra-network/src/zakura/block_sync/config.rs +++ b/zebra-network/src/zakura/block_sync/config.rs @@ -5,14 +5,29 @@ use super::{error::*, wire::*, *}; /// Keep block-body ranges narrow so a missing response only holds one height at /// the body-download floor. pub const DEFAULT_BS_BLOCKS_PER_RESPONSE: u32 = 1; -/// Initial number of in-flight block requests advertised per peer. +/// Default advertised hard cap on concurrent in-flight block requests per peer. /// -/// Outbound scheduling starts at this window and adjusts per peer based on -/// request timeouts, while peer advertisements can still allow growth up to -/// [`MAX_BS_INFLIGHT_REQUESTS`]. -pub const DEFAULT_BS_MAX_INFLIGHT: u32 = 2_048; +/// This is the ceiling the adaptive per-peer window grows *toward*, **not** the +/// opening window. Scheduling starts at [`DEFAULT_BS_INITIAL_INFLIGHT`] and ramps +/// up to the peer-advertised value (this default, clamped to +/// [`MAX_BS_INFLIGHT_REQUESTS`]) only after sustained error-free responses (see +/// the streak-gated cubic ramp on `DownloadWindow`). In a homogeneous fleet this +/// is the per-peer concurrency ceiling every peer offers. +pub const DEFAULT_BS_MAX_INFLIGHT: u32 = 32000; +/// Initial per-peer outbound request window (slow-start point). +/// +/// The adaptive window starts here and grows toward the peer-advertised hard cap +/// on successful responses, rather than opening at the full `max_inflight`. This +/// keeps the opening burst modest so a peer is not flooded before its latency is +/// known. +pub const DEFAULT_BS_INITIAL_INFLIGHT: u32 = 64; /// Maximum peer-advertised in-flight request count accepted by this node. -pub const MAX_BS_INFLIGHT_REQUESTS: u32 = 240_000; +/// +/// This is the hard ceiling the default advertisement ([`DEFAULT_BS_MAX_INFLIGHT`] +/// = 32,000) is clamped to, and also the per-peer outstanding-request safety bound +/// (`EFFECTIVE_BS_OUTBOUND_INFLIGHT_PER_PEER`). It bounds how many concurrent +/// requests a remote peer can make us hold against it, so it doubles as a DoS bound. +pub const MAX_BS_INFLIGHT_REQUESTS: u32 = 32_768; /// Default total response byte target advertised per range response. pub const DEFAULT_BS_MAX_RESPONSE_BYTES: u32 = 32 * 1024 * 1024; /// Default global byte budget reserved for later block-download scheduling. @@ -26,6 +41,16 @@ pub const DEFAULT_BS_MAX_INFLIGHT_BLOCK_BYTES: u64 = 2 * 1024 * 1024 * 1024; /// at decode (`MAX_BS_MESSAGE_BYTES > MAX_BLOCK_BYTES`), so the actual size can /// never exceed this worst case and the shrink is always non-negative. pub const BS_PER_BLOCK_WORST_CASE_BYTES: u64 = block::MAX_BLOCK_BYTES; +/// Default byte cap for speculative reorder look-ahead above the download floor. +/// +/// The default leaves one advertised response worth of headroom below the global +/// byte budget. The synchronous floor-pop path is the funding guarantee when +/// that headroom has been consumed by races or changed configuration. +pub const DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BYTES: u64 = + // `DEFAULT_BS_MAX_RESPONSE_BYTES` is a `u32`, so widening to `u64` is lossless. + DEFAULT_BS_MAX_INFLIGHT_BLOCK_BYTES - DEFAULT_BS_MAX_RESPONSE_BYTES as u64; +/// Default block-count cap for speculative reorder look-ahead bookkeeping. +pub const DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BLOCKS: u32 = 4096; /// Default maximum submitted block applies awaiting verifier completion. /// /// The checkpoint verifier resolves a checkpoint window only after the whole @@ -33,7 +58,11 @@ pub const BS_PER_BLOCK_WORST_CASE_BYTES: u64 = block::MAX_BLOCK_BYTES; pub const DEFAULT_BS_MAX_SUBMITTED_BLOCK_APPLIES: usize = zebra_chain::parameters::checkpoint::constants::MAX_CHECKPOINT_HEIGHT_GAP; /// Default block-sync request timeout. -pub const DEFAULT_BS_REQUEST_TIMEOUT: Duration = Duration::from_secs(10); +pub const DEFAULT_BS_REQUEST_TIMEOUT: Duration = Duration::from_secs(8); +/// Default central floor-watchdog cadence. +pub const DEFAULT_BS_FLOOR_WATCHDOG_TICK: Duration = Duration::from_secs(1); +/// Default hard floor-peer avoid cooldown after a watchdog cancellation. +pub const DEFAULT_BS_FLOOR_PEER_AVOID_COOLDOWN: Duration = DEFAULT_BS_REQUEST_TIMEOUT; /// Default block-sync status refresh interval reserved for later advertisement. pub const DEFAULT_BS_STATUS_REFRESH_INTERVAL: Duration = Duration::from_secs(30); /// Default tolerated size-hint deviation percentage reserved for later soft scoring. @@ -119,10 +148,23 @@ pub struct ZakuraBlockSyncConfig { pub max_blocks_per_response: u32, /// Maximum concurrent `GetBlocks` requests this node advertises per peer. pub max_inflight_requests: u32, + /// Initial per-peer outbound request window (slow-start point); grows toward + /// the advertised hard cap on success. Clamped to `[1, max_inflight_requests]`. + pub initial_inflight_requests: u32, /// Maximum total response bytes this node advertises per `GetBlocks` response. pub max_response_bytes: u32, /// Maximum estimated bytes reserved for in-flight and buffered block bodies. pub max_inflight_block_bytes: u64, + /// Maximum speculative body bytes held above the download floor. + pub max_reorder_lookahead_bytes: u64, + /// Maximum speculative body heights tracked above the download floor. + pub max_reorder_lookahead_blocks: u32, + /// Cadence for the central floor watchdog that rescues expired floor claims. + #[serde(with = "humantime_serde")] + pub floor_watchdog_tick: Duration, + /// How long to avoid reassigning an expired floor height to the same peer. + #[serde(with = "humantime_serde")] + pub floor_peer_avoid_cooldown: Duration, /// Maximum block bodies submitted to the verifier before completed applies /// release more submission slots. pub max_submitted_block_applies: usize, @@ -154,8 +196,13 @@ impl Default for ZakuraBlockSyncConfig { replace_legacy_syncer: false, max_blocks_per_response: DEFAULT_BS_BLOCKS_PER_RESPONSE, max_inflight_requests: DEFAULT_BS_MAX_INFLIGHT, + initial_inflight_requests: DEFAULT_BS_INITIAL_INFLIGHT, max_response_bytes: DEFAULT_BS_MAX_RESPONSE_BYTES, max_inflight_block_bytes: DEFAULT_BS_MAX_INFLIGHT_BLOCK_BYTES, + max_reorder_lookahead_bytes: DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BYTES, + max_reorder_lookahead_blocks: DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BLOCKS, + floor_watchdog_tick: DEFAULT_BS_FLOOR_WATCHDOG_TICK, + floor_peer_avoid_cooldown: DEFAULT_BS_FLOOR_PEER_AVOID_COOLDOWN, max_submitted_block_applies: DEFAULT_BS_MAX_SUBMITTED_BLOCK_APPLIES, request_timeout: DEFAULT_BS_REQUEST_TIMEOUT, status_refresh_interval: DEFAULT_BS_STATUS_REFRESH_INTERVAL, @@ -187,6 +234,47 @@ impl ZakuraBlockSyncConfig { self.max_submitted_block_applies.max(1) } + /// Return the speculative look-ahead byte cap clamped to the global budget. + pub fn effective_max_reorder_lookahead_bytes(&self) -> u64 { + self.max_reorder_lookahead_bytes + .min(self.max_inflight_block_bytes) + } + + /// Return the watchdog tick clamped to a positive duration no larger than the request timeout. + pub fn effective_floor_watchdog_tick(&self) -> Duration { + self.floor_watchdog_tick + .min(self.request_timeout) + .max(Duration::from_millis(1)) + } + + /// Return the floor avoid cooldown clamped to a positive duration. + pub fn effective_floor_peer_avoid_cooldown(&self) -> Duration { + self.floor_peer_avoid_cooldown.max(Duration::from_millis(1)) + } + + /// Return the largest byte reservation a single floor request can need. + pub fn floor_request_byte_reservation(&self) -> u64 { + let fanout = u64::try_from(self.fanout.max(1)).unwrap_or(u64::MAX); + let worst_case_blocks = u64::from(self.advertised_max_blocks_per_response()) + .saturating_mul(BS_PER_BLOCK_WORST_CASE_BYTES) + .saturating_mul(fanout); + u64::from(self.advertised_max_response_bytes()).max(worst_case_blocks) + } + + /// Validate production-safety bounds after deserialization. + pub fn validate(&self) -> Result<(), &'static str> { + if self.max_inflight_block_bytes == 0 { + return Err("max_inflight_block_bytes must be greater than zero"); + } + if self.max_reorder_lookahead_blocks == 0 { + return Err("max_reorder_lookahead_blocks must be greater than zero"); + } + if self.max_inflight_block_bytes <= self.floor_request_byte_reservation() { + return Err("max_inflight_block_bytes must exceed one floor request"); + } + Ok(()) + } + /// Build the inert local status used before the block-sync reactor is wired. pub fn initial_status(&self) -> BlockSyncStatus { BlockSyncStatus { diff --git a/zebra-network/src/zakura/block_sync/mod.rs b/zebra-network/src/zakura/block_sync/mod.rs index 3c6eb07bb27..3b3dabcd0d3 100644 --- a/zebra-network/src/zakura/block_sync/mod.rs +++ b/zebra-network/src/zakura/block_sync/mod.rs @@ -15,7 +15,7 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use serde::{Deserialize, Serialize}; use thiserror::Error; use tokio::{ - sync::{mpsc, watch}, + sync::{mpsc, oneshot, watch}, task::JoinHandle, time, }; @@ -30,6 +30,7 @@ use super::{ Frame, ServicePeerDirection, ServicePeerLimits, ZakuraPeerId, ZakuraTrace, }; +mod admission; mod config; mod error; mod events; diff --git a/zebra-network/src/zakura/block_sync/peer_registry.rs b/zebra-network/src/zakura/block_sync/peer_registry.rs index 52caa4b28b5..cc79e576fee 100644 --- a/zebra-network/src/zakura/block_sync/peer_registry.rs +++ b/zebra-network/src/zakura/block_sync/peer_registry.rs @@ -22,6 +22,7 @@ use std::{ collections::{BTreeMap, HashMap}, sync::Mutex as StdMutex, + time::Instant, }; use zebra_chain::block; @@ -48,12 +49,14 @@ pub(super) struct Entry { /// producer filter's `!has_outstanding_request` home, now routine-owned and /// independent of `work.in_flight`, so it structurally closes the /// reject-rollback window. - pub(super) outstanding: BTreeMap, + pub(super) outstanding: BTreeMap, /// Routine-published slot diagnostics (trace only): the per-peer download /// window state the reactor used to read off `PeerBlockState` for the periodic /// `BLOCK_SYNC_STATE` row. Updated whenever the routine issues/finishes/times /// out a request. pub(super) slots: SlotDiagnostics, + /// Heights this peer may not re-take until the given instant. + pub(super) retry_avoid: BTreeMap, /// Monotonic generation bumped each time a routine is (re)spawned for this /// peer. A cancelled routine's async `Drop` only clears outstanding when the /// generation still matches, so an old Drop racing a reset respawn cannot wipe @@ -77,6 +80,7 @@ impl Entry { max_response_bytes: config.advertised_max_response_bytes(), outstanding: BTreeMap::new(), slots: SlotDiagnostics::default(), + retry_avoid: BTreeMap::new(), generation, } } @@ -93,6 +97,23 @@ pub(super) struct SlotDiagnostics { pub(super) outstanding_requests: usize, } +/// Published metadata for one unreceived outstanding height. +#[derive(Copy, Clone, Debug)] +pub(super) struct OutstandingMeta { + pub(super) hash: block::Hash, + pub(super) estimated_bytes: u64, + pub(super) queued_at: Instant, + pub(super) deadline: Instant, +} + +/// Reactor-visible claim that can be force-cancelled by the floor watchdog. +#[derive(Clone, Debug)] +pub(super) struct OutstandingClaim { + pub(super) peer: ZakuraPeerId, + pub(super) height: block::Height, + pub(super) meta: OutstandingMeta, +} + /// The shared per-peer fact table. `Arc`-wrapped at the construction site so the /// reactor and every routine share one table. #[derive(Debug)] @@ -144,6 +165,7 @@ impl PeerRegistry { .and_modify(|entry| { entry.direction = direction; entry.outstanding.clear(); + entry.retry_avoid.clear(); entry.generation = generation; }) .or_insert_with(|| Entry::new(direction, config, generation)); @@ -187,7 +209,7 @@ impl PeerRegistry { &self, peer: &ZakuraPeerId, generation: u64, - outstanding: BTreeMap, + outstanding: BTreeMap, ) { let mut peers = self.lock(); if let Some(entry) = peers.get_mut(peer) { @@ -257,9 +279,12 @@ impl PeerRegistry { /// `ignore_unmatched_active` fallthrough). pub(super) fn has_outstanding_request(&self, height: block::Height, hash: block::Hash) -> bool { let peers = self.lock(); - peers - .values() - .any(|entry| entry.outstanding.get(&height) == Some(&hash)) + peers.values().any(|entry| { + entry + .outstanding + .get(&height) + .is_some_and(|meta| meta.hash == hash) + }) } /// Whether any connected peer has an outstanding request covering `height` @@ -273,6 +298,18 @@ impl PeerRegistry { .any(|entry| entry.outstanding.contains_key(&height)) } + /// Whether this exact peer still owns an outstanding claim for `height`. + pub(super) fn peer_has_outstanding_height( + &self, + peer: &ZakuraPeerId, + height: block::Height, + ) -> bool { + let peers = self.lock(); + peers + .get(peer) + .is_some_and(|entry| entry.outstanding.contains_key(&height)) + } + /// Total unreceived in-flight heights summed across peers — *per request*, /// never per body (an `outstanding` entry is one requested height). Feeds the /// producer's low-water refill gate. @@ -307,7 +344,7 @@ impl PeerRegistry { entry .outstanding .get(&height) - .is_some_and(|expected| *expected != hash) + .is_some_and(|expected| expected.hash != hash) }) } @@ -388,6 +425,60 @@ impl PeerRegistry { } (servable, outstanding) } + + /// Snapshot all peer claims for one height. + pub(super) fn outstanding_claims_at(&self, height: block::Height) -> Vec { + let peers = self.lock(); + peers + .iter() + .filter_map(|(peer, entry)| { + entry.outstanding.get(&height).map(|meta| OutstandingClaim { + peer: peer.clone(), + height, + meta: *meta, + }) + }) + .collect() + } + + /// Remove a published outstanding claim for `height` from `peer`. + pub(super) fn clear_outstanding_height(&self, peer: &ZakuraPeerId, height: block::Height) { + let mut peers = self.lock(); + if let Some(entry) = peers.get_mut(peer) { + entry.outstanding.remove(&height); + } + } + + /// Hard-exclude this peer from re-taking `height` until `until`. + pub(super) fn avoid_height_until( + &self, + peer: &ZakuraPeerId, + height: block::Height, + until: Instant, + ) { + let mut peers = self.lock(); + if let Some(entry) = peers.get_mut(peer) { + entry.retry_avoid.insert(height, until); + } + } + + /// Whether this peer is still hard-excluded from `height`. + pub(super) fn is_avoiding_height( + &self, + peer: &ZakuraPeerId, + height: block::Height, + now: Instant, + ) -> bool { + let mut peers = self.lock(); + let Some(entry) = peers.get_mut(peer) else { + return false; + }; + entry.retry_avoid.retain(|_, until| *until > now); + entry + .retry_avoid + .get(&height) + .is_some_and(|until| *until > now) + } } /// Aggregated slot diagnostics across peers for the periodic trace row. diff --git a/zebra-network/src/zakura/block_sync/peer_routine.rs b/zebra-network/src/zakura/block_sync/peer_routine.rs index a80996baf20..c0e07e72300 100644 --- a/zebra-network/src/zakura/block_sync/peer_routine.rs +++ b/zebra-network/src/zakura/block_sync/peer_routine.rs @@ -12,9 +12,9 @@ //! //! The one throughput-critical effect: the matched-body `sequencer_input.send(..).await` //! runs in this per-peer task, so a slow verifier (Sequencer backpressure) -//! stalls only one routine, not the whole fleet. The download decision gates **only** on the byte budget + per-peer -//! slots: `take_in_range(servable_low, servable_high, n)` uses `servable_high` -//! as the *only* upper bound. +//! stalls only one routine, not the whole fleet. The download decision gates only +//! on the byte budget + per-peer slots: `take_in_range(servable_low, +//! servable_high, n)` uses `servable_high` as the upper bound. //! //! The download logic is **moved, not rewritten** from the previous reactor: the //! want-work fill loop ports `fill_peer`, the matched-body tail ports @@ -26,12 +26,12 @@ use std::collections::BTreeMap; -use tokio::sync::{futures::Notified, mpsc, watch}; +use tokio::sync::{futures::Notified, mpsc, oneshot, watch}; use tokio_util::sync::CancellationToken; use super::events::RoutineToReactor; use super::{ - config::BS_PER_BLOCK_WORST_CASE_BYTES, + admission::{admission_decision, floor_rescue_high, AdmissionSnapshot, RequestPriority}, peer_registry::{hard_outbound_capacity, PeerRegistry}, pipe::block_sync_guard, reactor::{ @@ -39,8 +39,11 @@ use super::{ }, reorder::BufferedBlockBody, request::{BlockRangeRequest, ExpectedBlock}, - sequencer_task::{SequencedBody, SequencerView}, - state::{DownloadWindow, OutstandingBlockRange, ReceivedBlockTracker, ThroughputMeter}, + sequencer_task::{SequencedBody, SequencerControlInput, SequencerView}, + state::{ + next_height, DownloadWindow, OutstandingBlockRange, ReceivedBlockTracker, ThroughputMeter, + TimeoutBackoffOutcome, + }, work_queue::{WorkItem, WorkQueue}, BlockSyncAction, BlockSyncMessage, BlockSyncMisbehavior, BlockSyncPeerSession, BlockSyncStatus, ZakuraBlockSyncConfig, ZakuraPeerId, ZakuraTrace, MSG_BS_BLOCK, @@ -142,6 +145,7 @@ pub(super) struct PeerRoutine { received_throughput: Arc>, sequencer_input: mpsc::Sender, sequencer_input_bytes: Arc, + sequencer_control: mpsc::UnboundedSender, actions: mpsc::Sender, /// Shared routine→reactor channel for serving / status-advertise / re-query / /// serving-misbehavior. `try_send` (bounded, never-wedging) so a busy reactor @@ -176,6 +180,7 @@ impl PeerRoutine { received_throughput: Arc>, sequencer_input: mpsc::Sender, sequencer_input_bytes: Arc, + sequencer_control: mpsc::UnboundedSender, actions: mpsc::Sender, routine_to_reactor: mpsc::Sender, view: watch::Receiver, @@ -217,6 +222,7 @@ impl PeerRoutine { received_throughput, sequencer_input, sequencer_input_bytes, + sequencer_control, actions, routine_to_reactor, view, @@ -286,9 +292,7 @@ impl PeerRoutine { Err(_) => return Ok(()), } } - _ = &mut timeout => { - self.expire_due_timeouts(Instant::now()); - } + _ = &mut timeout => self.expire_due_timeouts(Instant::now())?, _ = &mut capacity => { self.trace_wake("budget_capacity"); } @@ -477,8 +481,8 @@ impl PeerRoutine { // `reset_above`) and release their reservations exactly once. let outstanding = std::mem::take(&mut self.window.outstanding); for outstanding in outstanding { - self.budget.release(outstanding.reserved_bytes()); - self.work.return_items(unreceived_heights(&outstanding)); + let released = self.return_unreceived_to_queue(&outstanding); + self.budget.release(released); } self.retry_avoid.clear(); // Clear our (now-empty) registry outstanding and refresh slot diagnostics. @@ -520,10 +524,9 @@ impl PeerRoutine { /// Ported from the reactor's `fill_peer`; the only changes are where the /// per-peer state lives (now routine-local / the registry) and the handles. /// - /// There is no floor gate: downloads are governed solely by the byte budget - /// and per-peer slots — never floor-distance / near-tip lag. + /// There is no floor gate: downloads are governed by the byte budget and + /// per-peer slots, never floor-distance / near-tip lag. async fn try_fill(&mut self) { - let worst = BS_PER_BLOCK_WORST_CASE_BYTES; // Reconcile the adaptive window's hard cap with the peer's currently // advertised `max_inflight_requests` (it may have grown/shrunk via a // `Status`; `handle_status` set `window.max_inflight_requests`). Mirrors @@ -559,35 +562,98 @@ impl PeerRoutine { .unwrap_or(usize::MAX); let (servable_low, servable_high) = (self.servable_low, self.servable_high); - // Compute this chunk's byte ceiling BEFORE taking any work. The - // count cap is still bounded by what the global budget can reserve at - // worst case, so advertised sizes never weaken the existing - // pre-send reservation. The estimate cap only decides how many of the - // already-affordable heights to pack into this one request. - let available_bytes = self.budget.available(); - let max_count = available_bytes - .checked_div(worst) - .map(|count| { - usize::try_from(count) - .unwrap_or(usize::MAX) - .min(local_peer_count_cap) - }) - .unwrap_or(local_peer_count_cap); - if max_count == 0 { - break; + let max_count = local_peer_count_cap; + let response_byte_cap = u64::from(self.max_response_bytes.max(1)); + + let view = *self.view.borrow(); + let floor_high = floor_rescue_high(view.download_floor); + let mut request_priority = RequestPriority::Floor; + let (reserved_above_floor_bytes, reserved_above_floor_blocks) = + self.work.reserved_above(view.download_floor); + let mut items = if servable_low <= floor_high + && self + .work + .first_pending_in_range(servable_low, servable_high.min(floor_high)) + .is_some() + { + let floor_available = self.budget.available().min(response_byte_cap); + if floor_available == 0 { + Vec::new() + } else { + let floor_take_high = match next_height(floor_high).and_then(|tail_start| { + admission_decision( + &self.config, + AdmissionSnapshot { + download_floor: view.download_floor, + reorder_buffered_bytes: view.reorder_buffered_bytes, + reorder_buffered_blocks: view.reorder_len, + applying_buffered_bytes: view.applying_buffered_bytes, + applying_buffered_blocks: view.applying_len, + sequencer_input_queued_bytes: self + .sequencer_input_bytes + .load(std::sync::atomic::Ordering::Relaxed), + reserved_above_floor_bytes, + reserved_above_floor_blocks, + budget_available: self.budget.available(), + }, + tail_start, + response_byte_cap, + ) + }) { + Some(_) => servable_high, + None => servable_high.min(floor_high), + }; + self.work.take_in_range_budgeted( + servable_low, + floor_take_high, + max_count, + floor_available, + ) + } + } else { + Vec::new() + }; + + if items.is_empty() { + let Some(start_height) = self + .work + .first_pending_in_range(servable_low, servable_high) + else { + break; + }; + let Some(decision) = admission_decision( + &self.config, + AdmissionSnapshot { + download_floor: view.download_floor, + reorder_buffered_bytes: view.reorder_buffered_bytes, + reorder_buffered_blocks: view.reorder_len, + applying_buffered_bytes: view.applying_buffered_bytes, + applying_buffered_blocks: view.applying_len, + sequencer_input_queued_bytes: self + .sequencer_input_bytes + .load(std::sync::atomic::Ordering::Relaxed), + reserved_above_floor_bytes, + reserved_above_floor_blocks, + budget_available: self.budget.available(), + }, + start_height, + response_byte_cap, + ) else { + metrics::gauge!("sync.block.backlog.at_cap").set(1.0); + break; + }; + + if decision.priority == RequestPriority::AboveFloor { + metrics::gauge!("sync.block.backlog.at_cap").set(0.0); + request_priority = RequestPriority::AboveFloor; + items = self.work.take_in_range_budgeted( + servable_low, + servable_high, + max_count, + decision.max_request_bytes, + ); + } } - let max_estimated_bytes = - available_bytes.min(u64::from(self.max_response_bytes.max(1))); - - // Take work in this peer's servable range. `servable_high` is NOT - // clamped to the floor: a peer fetches as far ahead of the committed - // floor as its servable range and the byte budget allow. - let mut items = self.work.take_in_range_budgeted( - servable_low, - servable_high, - max_count, - max_estimated_bytes, - ); if items.is_empty() { break; } @@ -600,10 +666,11 @@ impl PeerRoutine { // self-wake into a take/return spin. If the whole chunk is still // avoided, break — the routine wakes to retry when the avoid window // expires (see `earliest_deadline_sleep`). - if !self.retry_avoid.is_empty() { - let keep_from = items - .iter() - .position(|(height, _)| !self.retry_avoid.contains_key(height)); + { + let keep_from = items.iter().position(|(height, _)| { + !self.retry_avoid.contains_key(height) + && !self.registry.is_avoiding_height(&self.peer, *height, now) + }); match keep_from { Some(0) => {} Some(index) => { @@ -619,21 +686,40 @@ impl PeerRoutine { } self.trace_work_taken(servable_low, servable_high, items.len()); - // `take_in_range_budgeted` already honoured the byte-capped count; - // every taken item has worst-case reservation capacity. + // Reserve the summed per-block size estimate for this request (not + // worst case), so the budget admits far more typically-small bodies. + // `take_in_range_budgeted` already bounded the summed estimate to the + // response-byte cap. let kept_count = items.len(); - let reserved_bytes = worst.saturating_mul(kept_count as u64); - if !self.budget.try_reserve(reserved_bytes) { + let reserved_bytes = items.iter().fold(0u64, |acc, (_, item)| { + acc.saturating_add(item.estimated_bytes) + }); + if !self + .reserve_request_budget(request_priority, reserved_bytes) + .await + { self.return_taken_items(&items); break; } + let marked = self + .work + .mark_reserved(items.iter().map(|(height, _)| *height)); + if marked != reserved_bytes { + self.budget.release(reserved_bytes); + let _ = self + .work + .release_and_return_items(items.iter().map(|(height, _)| *height)); + break; + } let count = match u32::try_from(kept_count) { Ok(count) => count, Err(_) => { - self.budget.release(reserved_bytes); - self.return_taken_items(&items); + let released = self + .work + .release_and_return_items(items.iter().map(|(height, _)| *height)); + self.budget.release(released); break; } }; @@ -641,8 +727,9 @@ impl PeerRoutine { start_height: items[0].0, count, anchor_hash: items[0].1.hash, - // The reserved worst-case total (released on a send failure - // below); distinct from the size estimates in `expected_blocks`. + // The summed size-estimate reservation for this request (released + // on a send failure below); equals the sum of the per-height + // `expected_blocks` estimates. estimated_bytes: reserved_bytes, expected_blocks: items .iter() @@ -666,9 +753,11 @@ impl PeerRoutine { ?error, "failed to queue Zakura block-sync GetBlocks" ); - self.budget.release(request.estimated_bytes); // Nothing was received, so return every taken height to the queue. - self.return_taken_items(&items); + let released = self + .work + .release_and_return_items(items.iter().map(|(height, _)| *height)); + self.budget.release(released); if matches!(error, OrderedSendError::Full) { break; } @@ -705,6 +794,39 @@ impl PeerRoutine { } } + async fn reserve_request_budget( + &mut self, + priority: RequestPriority, + reserved_bytes: u64, + ) -> bool { + if priority == RequestPriority::AboveFloor { + return self.budget.try_reserve(reserved_bytes); + } + + loop { + if self.budget.try_reserve(reserved_bytes) { + return true; + } + + let (reply, funded) = oneshot::channel(); + if self + .sequencer_control + .send(SequencerControlInput::FundFloorReservation { + needed_bytes: reserved_bytes, + reply, + }) + .is_err() + { + return false; + } + + match funded.await { + Ok(true) => continue, + Ok(false) | Err(_) => return false, + } + } + } + /// Refill low-water mark in blocks (ported from the reactor's /// `refill_low_water_blocks`, but for a single peer's caps). fn refill_low_water_blocks(&self) -> usize { @@ -737,50 +859,62 @@ impl PeerRoutine { // ===================== own-timeout arm (ports `expire_due_timeouts`) ===== - fn expire_due_timeouts(&mut self, now: Instant) { + fn expire_due_timeouts(&mut self, now: Instant) -> Result<(), SinkReject> { let mut timed_out = Vec::new(); let mut index = 0; while index < self.window.outstanding.len() { if self.window.outstanding[index].deadline <= now { - self.window.reduce_outbound_window_after_timeout(); timed_out.push(self.window.outstanding.remove(index)); } else { index += 1; } } if timed_out.is_empty() { - return; + return Ok(()); } + let backoff = self.window.reduce_outbound_window_after_timeout(); for outstanding in &timed_out { - self.budget.release(outstanding.reserved_bytes()); // Return only the unreceived heights — received ones are buffered (in // `in_flight` until committed); re-queuing them would re-fetch a body // we already hold (the WorkQueue single-owner invariant forbids it). - self.return_unreceived_to_queue(outstanding); + let released = self.return_unreceived_to_queue(outstanding); + self.budget.release(released); } // Bias away from immediately re-grabbing the heights this peer just timed // out, so another peer can contest them (the peer-local timeout bias). let timed_out_heights: Vec<_> = timed_out.iter().flat_map(unreceived_heights).collect(); self.note_retry_avoid(timed_out_heights); self.publish_outstanding(); + if backoff == TimeoutBackoffOutcome::DisconnectPeer { + tracing::debug!( + peer = ?self.peer, + "disconnecting Zakura block-sync peer after repeated timeouts at minimum window" + ); + return Err(SinkReject::protocol( + "block-sync peer repeatedly timed out at minimum window", + )); + } + Ok(()) } /// Drop this routine's outstanding requests whose whole range is at or below - /// the committed floor: their bodies are committed and no longer needed, so - /// release the worst-case reservation still held for any unreceived heights + /// the download floor: their bodies have entered the commit pipeline or have + /// already been verified, so + /// release the size-estimate reservation still held for any unreceived heights /// and free the slot. No heights return to the queue (they are committed, /// below the floor, GC'd from the WorkQueue). A partially-committed request /// (suffix still above the floor) is left so its remaining bodies keep their /// reservation and arrive on the same request. fn gc_committed_outstanding(&mut self) { - let floor = self.committed_floor(); + let floor = self.download_floor(); let mut released = 0u64; let mut removed = false; let mut index = 0; while index < self.window.outstanding.len() { if self.window.outstanding[index].request.end_height() <= floor { let outstanding = self.window.outstanding.remove(index); - released = released.saturating_add(outstanding.reserved_bytes()); + released = released + .saturating_add(self.work.release_heights(unreceived_heights(&outstanding))); removed = true; } else { index += 1; @@ -851,6 +985,18 @@ impl PeerRoutine { .await; return; } + if !self + .registry + .peer_has_outstanding_height(&self.peer, height) + { + tracing::debug!( + peer = ?self.peer, + ?height, + "ignoring late block-sync body for a claim cancelled by the floor watchdog" + ); + self.finish_outstanding_at(index, Disposition::RetryMissing); + return; + } let estimated_bytes = outstanding.estimated_bytes_for_height(height).unwrap_or(0); let request_start_height = outstanding.request.start_height; let request_range_count = outstanding.request.count; @@ -879,17 +1025,33 @@ impl PeerRoutine { { self.report_misbehavior(BlockSyncMisbehavior::SizeMismatch) .await; + self.finish_outstanding_at(index, Disposition::RetryOriginal); + return; } metrics::counter!("sync.block.body.received").increment(1); self.record_received(serialized_bytes); - // The block reserved `BS_PER_BLOCK_WORST_CASE_BYTES` at send time; shrink - // to the actual size (the reservation only ever decreases, so the budget - // can never reject a valid body). `mark_received` then stops - // `reserved_bytes()` counting this height; the only bytes still held are - // the `serialized_bytes` carried into the reorder buffer. - self.budget - .shrink(BS_PER_BLOCK_WORST_CASE_BYTES, serialized_bytes); + // The block reserved its size estimate at send time; settle to the actual + // size. When the body is no larger than its estimate this frees the + // slack; when it is larger (a stale/under-advertised hint) this charges + // the overshoot so held bodies are never under-counted. + // `mark_received` then stops `reserved_bytes()` counting this height; the + // only bytes still held are the `serialized_bytes` carried into the reorder + // buffer. + let Some(delta) = self + .work + .settle_active_reserved_height(height, serialized_bytes) + else { + tracing::debug!( + peer = ?self.peer, + ?height, + serialized_bytes, + "ignoring late block-sync body for a request already released" + ); + self.finish_outstanding_at(index, Disposition::RetryMissing); + return; + }; + self.apply_budget_delta(delta); self.trace_body_received( height, serialized_bytes, @@ -925,12 +1087,12 @@ impl PeerRoutine { // ===================== unmatched fallthroughs (ported) ================== - /// Whether a response for `height` is stale (already committed or held). The + /// Whether a response for `height` is stale (already downloaded or held). The /// held-height portion is recovered through the WorkQueue's `in_flight` - /// (every buffered/applying height stays claimed until the floor commits past - /// it). Reads `committed_floor` from the view. + /// (every buffered/applying height stays claimed until the download floor + /// passes it). Reads `download_floor` from the view. fn is_stale_response_height(&self, height: block::Height) -> bool { - height <= self.committed_floor() || self.work.in_flight_contains(height) + height <= self.download_floor() || self.work.in_flight_contains(height) } async fn ignore_stale_response(&mut self, height: block::Height, response_kind: &str) -> bool { @@ -1021,10 +1183,50 @@ impl PeerRoutine { self.record_received(serialized_bytes); self.trace_body_received(height, serialized_bytes, None, None, None); + let view = *self.view.borrow(); + let (reserved_above_floor_bytes, reserved_above_floor_blocks) = + self.work.reserved_above(view.download_floor); + let Some(decision) = admission_decision( + &self.config, + AdmissionSnapshot { + download_floor: view.download_floor, + reorder_buffered_bytes: view.reorder_buffered_bytes, + reorder_buffered_blocks: view.reorder_len, + applying_buffered_bytes: view.applying_buffered_bytes, + applying_buffered_blocks: view.applying_len, + sequencer_input_queued_bytes: self + .sequencer_input_bytes + .load(std::sync::atomic::Ordering::Relaxed), + reserved_above_floor_bytes, + reserved_above_floor_blocks, + budget_available: self.budget.available(), + }, + height, + serialized_bytes, + ) else { + tracing::debug!( + peer = ?self.peer, + ?height, + serialized_bytes, + "not buffering unmatched queued block-sync body at look-ahead cap" + ); + return true; + }; + if decision.max_request_bytes < serialized_bytes { + tracing::debug!( + peer = ?self.peer, + ?height, + serialized_bytes, + admitted_bytes = decision.max_request_bytes, + "not buffering unmatched queued block-sync body; insufficient admitted budget" + ); + return true; + } + // This queued height owns no prior reservation: reserve its actual size // before buffering. If the budget is genuinely full of other legitimate // bodies, skip buffering (the height stays queued for retry with its own - // worst-case reservation, so no valid body is lost overall). + // size-estimate reservation, so no valid body is lost overall). if !self.budget.try_reserve(serialized_bytes) { tracing::debug!( peer = ?self.peer, @@ -1039,6 +1241,8 @@ impl PeerRoutine { // already `in_flight` the take is a no-op and the Sequencer drops the // later duplicate. let _ = self.work.take_in_range(height, height, 1); + let old_charge = self.work.mark_held_direct(height, serialized_bytes); + self.budget.release(old_charge); let body = BufferedBlockBody::from_decoded_block(block, raw_block_payload); self.forward_body_to_sequencer(height, hash, body, serialized_bytes, body_permit) @@ -1099,7 +1303,7 @@ impl PeerRoutine { } /// An unmatched response for a height the peer *claims to serve* - /// (`committed_floor < height <= servable_high`) that no other fallthrough + /// (`download_floor < height <= servable_high`) that no other fallthrough /// claimed. The common cause is an honest, in-flight body/terminator for a /// height we requested before a destructive reset (reorg) then dropped from /// our `outstanding` and from `work` (`reset_above`), or one that simply @@ -1112,8 +1316,7 @@ impl PeerRoutine { /// window — restore the no-churn property by dropping the response quietly. A /// response *outside* the peer's advertised range is still scored. fn ignore_servable_range_response(&self, height: block::Height, response_kind: &str) -> bool { - if !self.received_status || height <= self.committed_floor() || height > self.servable_high - { + if !self.received_status || height <= self.download_floor() || height > self.servable_high { return false; } metrics::counter!("sync.block.response.unmatched_servable_ignored").increment(1); @@ -1178,14 +1381,16 @@ impl PeerRoutine { /// response can still match after the floor moved through its prefix; mark /// the stale prefix satisfied and retry only the remaining suffix. fn stale_adjusted_disposition(&mut self, index: usize, current: Disposition) -> Disposition { - let tip = self.committed_floor(); + let tip = self.download_floor(); let Some(outstanding) = self.window.outstanding.get_mut(index) else { return current; }; if outstanding.request.start_height > tip { return current; } - let released_bytes = outstanding.mark_received_through(tip); + let released_heights: Vec<_> = outstanding_unreceived_through(outstanding, tip).collect(); + let _ = outstanding.mark_received_through(tip); + let released_bytes = self.work.release_heights(released_heights); self.budget.release(released_bytes); if outstanding.is_complete() { Disposition::Satisfied @@ -1205,7 +1410,8 @@ impl PeerRoutine { } fn finish_detached(&mut self, outstanding: OutstandingBlockRange, disposition: Disposition) { - self.budget.release(outstanding.reserved_bytes()); + let released = self.work.release_heights(unreceived_heights(&outstanding)); + self.budget.release(released); match disposition { Disposition::Satisfied => { // Every requested height was received and buffered; nothing @@ -1216,7 +1422,8 @@ impl PeerRoutine { // be re-fetched, so both retry dispositions return only the unreceived // heights to `pending`. `return_items` is idempotent. Disposition::RetryOriginal | Disposition::RetryMissing => { - self.return_unreceived_to_queue(&outstanding); + let released = self.return_unreceived_to_queue(&outstanding); + self.budget.release(released); // This peer just failed these heights (RangeUnavailable / short // BlocksDone): bias away from re-grabbing them so another peer // contests the range first (and so the routine cannot self-wake @@ -1227,11 +1434,22 @@ impl PeerRoutine { self.publish_outstanding(); } - fn return_unreceived_to_queue(&self, outstanding: &OutstandingBlockRange) { - self.work.return_items(unreceived_heights(outstanding)); + fn return_unreceived_to_queue(&self, outstanding: &OutstandingBlockRange) -> u64 { + self.work + .release_and_return_items(unreceived_heights(outstanding)) + } + + fn apply_budget_delta(&mut self, delta: i128) { + if delta > 0 { + self.budget + .charge(u64::try_from(delta).expect("positive budget delta fits in u64")); + } else if delta < 0 { + self.budget + .release(u64::try_from(-delta).expect("negative budget delta fits in u64")); + } } - /// Publish this peer's current *unreceived* in-flight height→hash set to the + /// Publish this peer's current *unreceived* in-flight height metadata to the /// registry, so the producer's `!has_outstanding_request` filter and the /// low-water `total_unreceived` gate read the same per-request-granularity /// count the previous reactor used (`expected_blocks.len() − received.len()`). @@ -1239,11 +1457,20 @@ impl PeerRoutine { /// `work.in_flight` instead — the producer's `!in_flight_contains` clause /// already keeps them out of `pending`. fn publish_outstanding(&self) { - let mut map: BTreeMap = BTreeMap::new(); + let mut map: BTreeMap = + BTreeMap::new(); for outstanding in &self.window.outstanding { for expected in &outstanding.request.expected_blocks { if !outstanding.has_received(expected.height) { - map.insert(expected.height, expected.hash); + map.insert( + expected.height, + super::peer_registry::OutstandingMeta { + hash: expected.hash, + estimated_bytes: expected.estimated_bytes, + queued_at: outstanding.queued_at, + deadline: outstanding.deadline, + }, + ); } } } @@ -1284,8 +1511,8 @@ impl PeerRoutine { // ===================== view reads ====================================== - fn committed_floor(&self) -> block::Height { - self.view.borrow().floor + fn download_floor(&self) -> block::Height { + self.view.borrow().download_floor } fn record_received(&self, bytes: u64) { @@ -1471,6 +1698,20 @@ fn unreceived_heights( .map(|expected| expected.height) } +fn outstanding_unreceived_through( + outstanding: &OutstandingBlockRange, + tip: block::Height, +) -> impl Iterator + '_ { + outstanding + .request + .expected_blocks + .iter() + .filter(move |expected| { + expected.height <= tip && !outstanding.has_received(expected.height) + }) + .map(|expected| expected.height) +} + impl Drop for PeerRoutine { /// disconnect-mid-fetch correctness: on every exit path /// (cancel/panic/normal) return this routine's unreceived outstanding heights @@ -1487,8 +1728,7 @@ impl Drop for PeerRoutine { /// admission-reject); see `handle_peer_disconnected`. fn drop(&mut self) { for outstanding in self.window.outstanding.drain(..) { - self.budget.release(outstanding.reserved_bytes()); - self.work.return_items( + let released = self.work.release_and_return_items( outstanding .request .expected_blocks @@ -1496,6 +1736,7 @@ impl Drop for PeerRoutine { .filter(|expected| !outstanding.has_received(expected.height)) .map(|expected| expected.height), ); + self.budget.release(released); } self.registry.clear_outstanding(&self.peer, self.generation); } diff --git a/zebra-network/src/zakura/block_sync/reactor.rs b/zebra-network/src/zakura/block_sync/reactor.rs index 47d856d7036..1d74831c386 100644 --- a/zebra-network/src/zakura/block_sync/reactor.rs +++ b/zebra-network/src/zakura/block_sync/reactor.rs @@ -21,7 +21,6 @@ const BS_ACTION_SPARE_POOL: usize = 128; /// request; the routine never blocks on it (the only blocking routine send is the /// Sequencer `AcceptBody`), so a full channel just defers an idempotent ping. const ROUTINE_TO_REACTOR_DEPTH: usize = 1024; - #[derive(Copy, Clone, Debug, Eq, PartialEq)] struct FloorGapDiagnostics { height: block::Height, @@ -134,6 +133,7 @@ pub fn spawn_block_sync_reactor( received_throughput: state.received_throughput.clone(), sequencer_input: sequencer_input_tx.clone(), sequencer_input_bytes: sequencer_input_bytes.clone(), + sequencer_control: sequencer_control_tx.clone(), actions: actions_tx.clone(), routine_to_reactor: routine_to_reactor_tx, view: sequencer_view_rx.clone(), @@ -150,7 +150,7 @@ pub fn spawn_block_sync_reactor( }; let reactor = BlockSyncReactor { verified_block_tip: startup.frontiers.verified_block_tip, - committed_floor: startup.frontiers.verified_block_tip, + request_floor: startup.frontiers.verified_block_tip, pending_needed_query: None, last_reset_epoch: 0, last_reaction_epoch: 0, @@ -219,10 +219,10 @@ pub(super) struct BlockSyncReactor { /// Reactor-side mirror of the Sequencer's verified tip (it no longer lives /// in `state`). Updated from the committed view; initialized from startup. verified_block_tip: block::Height, - /// Reactor-side mirror of the Sequencer's body-download floor. Used ONLY for - /// the producer query lower bound, candidate prune, and stale-prefix trim — - /// never as a fetch decision. - committed_floor: block::Height, + /// Reactor-side scheduler/query lower bound. It follows the Sequencer's + /// download floor, but it is not verified state and must not be used for + /// serving/status advertisement. + request_floor: block::Height, /// `(verified_tip, best_header_tip, best_header_hash)` for a dispatched /// `QueryNeededBlocks` action whose `NeededBlocks` response has not come /// back yet. @@ -256,6 +256,8 @@ impl BlockSyncReactor { .status_refresh_interval .max(Duration::from_millis(1)), ); + let mut floor_watchdog_ticks = + time::interval(self.startup.config.effective_floor_watchdog_tick()); self.query_needed_blocks().await; self.publish_metrics(); @@ -343,7 +345,47 @@ impl BlockSyncReactor { self.trace_sync_state(); } _ = status_ticks.tick() => self.flush_status_refresh().await, + _ = floor_watchdog_ticks.tick() => { + self.run_floor_watchdog(Instant::now()); + self.publish_metrics(); + } + } + } + } + + fn run_floor_watchdog(&mut self, now: Instant) { + let Some(height) = next_height(self.request_floor) else { + return; + }; + let (servable_peers, _) = self.registry.floor_gap_servable(height); + let claims = self.registry.outstanding_claims_at(height); + for claim in claims { + if claim.meta.deadline > now { + continue; + } + + self.registry + .clear_outstanding_height(&claim.peer, claim.height); + if servable_peers > 2 { + self.registry.avoid_height_until( + &claim.peer, + claim.height, + now + self.startup.config.effective_floor_peer_avoid_cooldown(), + ); } + let released = self + .state + .work + .release_reserved_and_return_items([claim.height]); + self.state.budget.release(released); + metrics::counter!("sync.block.floor_watchdog.cancelled").increment(1); + tracing::debug!( + peer = ?claim.peer, + height = ?claim.height, + estimated_bytes = claim.meta.estimated_bytes, + released, + "force-cancelled expired Zakura block-sync floor request" + ); } } @@ -635,10 +677,10 @@ impl BlockSyncReactor { /// floor advances. The producer (`handle_needed_blocks`) only ever *grows* /// `needed_heights`; this prunes the heights the floor passed so the candidate /// gap clears promptly without waiting for the next `NeededBlocks` snapshot. - /// Reads the `committed_floor` mirror (the Sequencer's floor now lives on the - /// task); this is a GC/candidate use, never a fetch throttle. + /// Reads the `request_floor` mirror (the Sequencer's download floor now lives + /// on the task); this is a GC/candidate use, never a fetch throttle. fn prune_needed_below_floor(&mut self) { - let floor = self.committed_floor; + let floor = self.request_floor; let before = self.state.needed_heights.len(); self.state.needed_heights.retain(|height| *height > floor); if self.state.needed_heights.len() != before { @@ -718,7 +760,7 @@ impl BlockSyncReactor { self.last_view = view; self.state.finalized_height = self.state.finalized_height.max(view.finalized); self.verified_block_tip = view.verified_tip; - self.committed_floor = view.floor; + self.request_floor = view.download_floor; self.state.verified_block_hash = view.verified_hash; self.state.servable_high = view.verified_tip; self.state.servable_hash = view.verified_hash; @@ -783,7 +825,7 @@ impl BlockSyncReactor { // or `reset_above` (reset). So a height above the committed floor that is // not in `in_flight` is genuinely missing and re-queuable; one that is // in `in_flight` is already claimed and must not be re-issued. The - // `committed_floor` mirror is the producer's lower bound only. + // `request_floor` mirror is the producer's lower bound only. // // `!has_outstanding_request` is kept (the registry's per-peer outstanding): // the `in_flight ⟺ outstanding` half of the invariant breaks transiently @@ -798,7 +840,7 @@ impl BlockSyncReactor { let blocks: Vec<_> = blocks .into_iter() .filter(|block| { - block.height > self.committed_floor + block.height > self.request_floor && !self.state.work.in_flight_contains(block.height) && !self .registry @@ -1094,7 +1136,7 @@ impl BlockSyncReactor { if !self.startup.state_queries_enabled { return false; } - if self.committed_floor >= self.state.best_header_tip { + if self.request_floor >= self.state.best_header_tip { self.pending_needed_query = None; return true; } @@ -1110,7 +1152,7 @@ impl BlockSyncReactor { return true; } let query = ( - self.committed_floor, + self.request_floor, self.state.best_header_tip, self.state.best_header_hash, ); @@ -1118,7 +1160,7 @@ impl BlockSyncReactor { return true; } let dispatched = self.dispatch_action(BlockSyncAction::QueryNeededBlocks { - verified_block_tip: self.committed_floor, + verified_block_tip: self.request_floor, best_header_tip: self.state.best_header_tip, }); if dispatched { @@ -1425,7 +1467,8 @@ impl BlockSyncReactor { .map(|meter| (meter.bytes_per_sec(), meter.blocks_per_sec())) .unwrap_or((0, 0)); self.emit_trace(bs_trace::BLOCK_SYNC_STATE, |row| { - bs_insert_height(row, bs_trace::BODY_DOWNLOAD_FLOOR, view.floor); + bs_insert_height(row, bs_trace::REQUEST_FLOOR, self.request_floor); + bs_insert_height(row, bs_trace::BODY_DOWNLOAD_FLOOR, view.download_floor); bs_insert_height(row, bs_trace::VERIFIED_BLOCK_TIP, view.verified_tip); bs_insert_height(row, bs_trace::BEST_HEADER_TIP, self.state.best_header_tip); bs_insert_u64(row, bs_trace::BODY_LAG, u64::from(self.body_lag())); @@ -1780,21 +1823,23 @@ impl BlockSyncReactor { }); } - fn floor_gap_diagnostics(&self, _now: Instant) -> Option { - let height = next_height(self.committed_floor)?; + fn floor_gap_diagnostics(&self, now: Instant) -> Option { + let height = next_height(self.request_floor)?; if height > self.state.best_header_tip { return None; } - // Servable / outstanding peer counts come from the registry (the routines - // mirror their outstanding heights there). The per-request deadline ages - // now live in the routines and are no longer reactor-visible; this trace - // field drops the `oldest/next deadline ms` breakdown (per-peer routines) — the periodic - // `BLOCK_SYNC_STATE` row still carries the slot/budget signals. let (servable_peers, outstanding_peers) = self.registry.floor_gap_servable(height); + let claims = self.registry.outstanding_claims_at(height); let available_peers = 0usize; - let oldest_outstanding_ms = None; - let next_deadline_ms = None; + let oldest_outstanding_ms = claims + .iter() + .map(|claim| elapsed_ms_u64(now.saturating_duration_since(claim.meta.queued_at))) + .max(); + let next_deadline_ms = claims + .iter() + .map(|claim| elapsed_ms_u64(claim.meta.deadline.saturating_duration_since(now))) + .min(); // Sequencer task: the Sequencer's per-height `applying`/`submitted_apply`/`reorder` // membership is no longer reactor-visible (it lives on the task). A height @@ -2161,6 +2206,10 @@ fn bs_insert_str( row.insert(key.to_string(), serde_json::Value::from(value.to_string())); } +fn elapsed_ms_u64(duration: Duration) -> u64 { + u64::try_from(duration.as_millis()).unwrap_or(u64::MAX) +} + pub(super) fn tolerated_bytes(reserved_bytes: u64, tolerance_percent: u32) -> u64 { reserved_bytes.saturating_mul(u64::from(tolerance_percent.max(100))) / 100 } diff --git a/zebra-network/src/zakura/block_sync/request.rs b/zebra-network/src/zakura/block_sync/request.rs index dd699a74a5e..3ab52a4c7b2 100644 --- a/zebra-network/src/zakura/block_sync/request.rs +++ b/zebra-network/src/zakura/block_sync/request.rs @@ -16,7 +16,7 @@ pub enum BlockSizeEstimate { Confirmed(u32), /// Untrusted advertised size hint from header sync. Advertised(u32), - /// No size hint is known; use the EWMA fallback. + /// No size hint is known; reserve the per-block worst case. Unknown, } @@ -28,8 +28,8 @@ pub(super) struct BlockRangeRequest { pub(super) start_height: block::Height, pub(super) count: u32, pub(super) anchor_hash: block::Hash, - /// The reserved worst-case byte total for this request (released on - /// timeout/disconnect/send-failure). Distinct from the per-height size + /// The reserved byte total for this request (released on + /// timeout/disconnect/send-failure). Equal to the sum of the per-height size /// estimates in `expected_blocks`. pub(super) estimated_bytes: u64, pub(super) expected_blocks: Vec, diff --git a/zebra-network/src/zakura/block_sync/sequencer_task.rs b/zebra-network/src/zakura/block_sync/sequencer_task.rs index 07cdead9d45..09f12cb0809 100644 --- a/zebra-network/src/zakura/block_sync/sequencer_task.rs +++ b/zebra-network/src/zakura/block_sync/sequencer_task.rs @@ -30,47 +30,67 @@ use super::{ /// flowing to trigger the inline check (e.g. once outstanding requests drain). const FLOOR_STARVATION_SHED_INTERVAL: Duration = Duration::from_millis(500); -/// Favor the lowest re-requestable height over the speculative high tail. +/// Favor the lowest needed height over the speculative high tail. /// /// While the byte budget cannot fund even one worst-case request yet the lowest -/// needed height (the commit-unblocking floor gap) is pending *below* the highest -/// buffered body, drop that top body: release its bytes to the budget and return -/// its height to `pending` (it was held, hence in `work.in_flight` per the -/// `held ⟺ in_flight` invariant) for later re-fetch. Because another top can -/// always be shed, a low retry never blocks on budget — the floor can never wedge -/// behind a full buffer — and under a stall the speculative tail is shed and the -/// chain fills bottom-up, which also bounds the reorder backlog. Returns whether -/// it shed anything. -pub(super) fn shed_top_for_floor_starvation( +/// needed height (pending or outstanding) sits *below* the highest buffered body, +/// drop that top body: release its bytes to the budget and return its height to +/// `pending` (it was held, hence in `work.in_flight` per the `held ⟺ in_flight` +/// invariant) for later re-fetch. Because another top can always be shed, a low +/// retry never blocks on budget; the floor can never wedge behind a full buffer, +/// and under a stall the speculative tail is shed and the chain fills bottom-up, +/// which also bounds the reorder backlog. Returns whether it shed +/// anything. +pub(super) fn shed_top_until_available( budget: &mut ByteBudget, work: &WorkQueue, sequencer: &mut Sequencer, + target_available: u64, ) -> bool { - let worst = super::config::BS_PER_BLOCK_WORST_CASE_BYTES; let mut shed_any = false; - while budget.available() < worst { - let Some(lowest_pending) = work.min_pending() else { - break; + while budget.available() < target_available { + let lowest_needed = match (work.min_pending(), work.min_in_flight()) { + (Some(pending), Some(in_flight)) => pending.min(in_flight), + (Some(pending), None) => pending, + (None, Some(in_flight)) => in_flight, + (None, None) => break, }; let Some(top) = sequencer.reorder_max_height() else { break; }; // Only shed a body that sits above a starved lower height: we trade a // far-from-floor body for the ability to fetch a nearer, higher-value one. - if lowest_pending >= top { + if lowest_needed >= top { break; } let freed = sequencer.drop_reorder_from(top); if freed == 0 { break; } - budget.release(freed); - work.return_items([top]); + let released = work.release_and_return_items([top]); + debug_assert!( + released == 0 || released == freed, + "shed reorder release must match the per-height budget ledger when present" + ); + budget.release(if released == 0 { freed } else { released }); shed_any = true; } shed_any } +pub(super) fn shed_top_for_floor_starvation( + budget: &mut ByteBudget, + work: &WorkQueue, + sequencer: &mut Sequencer, +) -> bool { + shed_top_until_available( + budget, + work, + sequencer, + super::config::BS_PER_BLOCK_WORST_CASE_BYTES, + ) +} + /// A received body a peer routine matched (or accepted unmatched) and forwards /// to the commit pipeline. This is the only bounded Sequencer input: a slow /// verifier can backpressure body intake, but must not block apply/frontier @@ -91,7 +111,7 @@ pub(super) struct SequencedBody { /// budget and verifier slots, and frontier/reset events can release or discard /// stale body work. They are locally generated and tiny, so they use a separate /// unbounded channel and are prioritized by the Sequencer task. -#[derive(Clone, Debug)] +#[derive(Debug)] pub(super) enum SequencerControlInput { /// A verified-tip advance (frontier growth/commit). FrontierAdvance { @@ -120,6 +140,12 @@ pub(super) enum SequencerControlInput { result: BlockApplyResult, local_frontier: Option, }, + /// Synchronously pop the speculative high tail until a floor request can + /// reserve `needed_bytes`, then wake the requester to retry the reservation. + FundFloorReservation { + needed_bytes: u64, + reply: oneshot::Sender, + }, } /// The committed view the reactor reacts to. A `watch` (latest-wins) send never @@ -129,7 +155,7 @@ pub(super) enum SequencerControlInput { pub(super) struct SequencerView { pub(super) verified_tip: block::Height, pub(super) verified_hash: block::Hash, - pub(super) floor: block::Height, + pub(super) download_floor: block::Height, pub(super) finalized: block::Height, /// Increments only when the task performs a destructive `reset_to`, so the /// reactor distinguishes an advance (drop outstanding *through* tip) from a @@ -157,7 +183,7 @@ pub(super) fn initial_view(frontiers: BlockSyncFrontiers) -> SequencerView { SequencerView { verified_tip: frontiers.verified_block_tip, verified_hash: frontiers.verified_block_hash, - floor: frontiers.verified_block_tip, + download_floor: frontiers.verified_block_tip, finalized: frontiers.finalized_height, reset_epoch: 0, reaction_epoch: 0, @@ -331,6 +357,19 @@ impl SequencerTask { self.handle_apply_finished(token, height, hash, result, local_frontier) .await } + SequencerControlInput::FundFloorReservation { + needed_bytes, + reply, + } => { + let shed = shed_top_until_available( + &mut self.budget, + &self.work, + &mut self.sequencer, + needed_bytes, + ); + let _ = reply.send(self.budget.available() >= needed_bytes); + shed + } } } @@ -388,7 +427,8 @@ impl SequencerTask { .advance_verified_tip(frontiers.verified_block_tip, release_applied); self.budget.release(advance.release_bytes); if advance.changed { - self.work.advance_floor(frontiers.verified_block_tip); + let released = self.work.advance_floor(frontiers.verified_block_tip); + self.budget.release(released); self.release_contiguous_blocks().await; } } @@ -466,7 +506,8 @@ impl SequencerTask { // Drop every download work item above the reset target (their buffers // were cleared by `reset_to`); the reactor's `query_needed_blocks` // re-fills. - self.work.reset_above(self.sequencer.floor()); + let released = self.work.reset_above(self.sequencer.floor()); + self.budget.release(released); // A destructive reset: bump the epoch so the reactor drops *all* // outstanding requests (not just those through the tip). self.reset_epoch = self.reset_epoch.saturating_add(1); @@ -542,7 +583,8 @@ impl SequencerTask { let released = self.sequencer.release_applying_blocks_from(height); self.budget.release(released); self.sequencer.reset_floor_below(height); - self.work.reset_above(self.sequencer.floor()); + let released = self.work.reset_above(self.sequencer.floor()); + self.budget.release(released); let dropped = self.sequencer.drop_reorder_from(height); self.budget.release(dropped); // A `Rejected` result means consensus found the body invalid. @@ -710,17 +752,30 @@ impl SequencerTask { fn publish_view(&mut self) { self.committed_throughput.sample(Instant::now()); + let reorder_buffered_bytes = self.sequencer.reorder_buffered_bytes(); + let applying_buffered_bytes = self.sequencer.applying_buffered_bytes(); + let body_input_bytes = self + .body_input_bytes + .load(std::sync::atomic::Ordering::Relaxed); + let expected_budget = self + .work + .reserved_bytes() + .saturating_add(reorder_buffered_bytes) + .saturating_add(applying_buffered_bytes) + .saturating_add(body_input_bytes); + self.budget + .audit(expected_budget, "block-sync sequencer view"); let _ = self.view_tx.send_replace(SequencerView { verified_tip: self.sequencer.verified_tip(), verified_hash: self.verified_block_hash, - floor: self.sequencer.floor(), + download_floor: self.sequencer.floor(), finalized: self.finalized_height, reset_epoch: self.reset_epoch, reaction_epoch: self.reaction_epoch, reorder_len: self.sequencer.reorder_len() as u64, applying_len: self.sequencer.applying_len() as u64, - reorder_buffered_bytes: self.sequencer.reorder_buffered_bytes(), - applying_buffered_bytes: self.sequencer.applying_buffered_bytes(), + reorder_buffered_bytes, + applying_buffered_bytes, unsubmitted_applying_count: self.sequencer.unsubmitted_applying_count() as u64, submitted_applying_count: self.sequencer.submitted_applying_count() as u64, submitted_applying_bytes: self.sequencer.submitted_applying_bytes(), diff --git a/zebra-network/src/zakura/block_sync/service.rs b/zebra-network/src/zakura/block_sync/service.rs index 8b4aaa78050..3506a6bd9dd 100644 --- a/zebra-network/src/zakura/block_sync/service.rs +++ b/zebra-network/src/zakura/block_sync/service.rs @@ -453,6 +453,7 @@ impl Service for BlockSyncService { wiring.received_throughput, wiring.sequencer_input, wiring.sequencer_input_bytes, + wiring.sequencer_control, wiring.actions, wiring.routine_to_reactor, wiring.view, diff --git a/zebra-network/src/zakura/block_sync/state.rs b/zebra-network/src/zakura/block_sync/state.rs index 31ccefe91d8..1da717b1cc6 100644 --- a/zebra-network/src/zakura/block_sync/state.rs +++ b/zebra-network/src/zakura/block_sync/state.rs @@ -11,10 +11,29 @@ use crate::zakura::{ /// [`MAX_BS_INFLIGHT_REQUESTS`]). // `MAX_BS_INFLIGHT_REQUESTS` is a `u32`, which fits in `usize` on supported targets. pub(super) const EFFECTIVE_BS_OUTBOUND_INFLIGHT_PER_PEER: usize = MAX_BS_INFLIGHT_REQUESTS as usize; -/// Minimum additive growth after a successful response. -const MIN_OUTBOUND_WINDOW_SUCCESS_GROWTH: usize = 64; -/// Fractional additive-increase divisor after a successful response. -const OUTBOUND_WINDOW_SUCCESS_GROWTH_DIVISOR: usize = 8; +/// Consecutive error-free responses that make up one window-growth epoch. +/// +/// The adaptive window holds flat for a full epoch of successes before each +/// growth step, so it never opens faster than the peer has demonstrably served. +const OUTBOUND_WINDOW_GROWTH_EPOCH_SUCCESSES: usize = 16; +/// Cubic coefficient for the streak-gated window ramp. +/// +/// Target window = `growth_base + COEFF * epoch^3`, capped at the hard cap, where +/// `epoch` counts completed [`OUTBOUND_WINDOW_GROWTH_EPOCH_SUCCESSES`]-success +/// runs since the last timeout. Growth is gentle for the first few epochs and +/// accelerates the longer the peer serves without an error, then resets on the +/// next timeout. With the default base of 64 and a 16-success epoch, the window +/// ramps cubically toward the peer's advertised hard cap (locally clamped to +/// [`MAX_BS_INFLIGHT_REQUESTS`] = 32,768; the default advertisement is 32,000), +/// reaching the default 32,000 ceiling after 16 epochs (256 consecutive +/// error-free successes). +const OUTBOUND_WINDOW_GROWTH_CUBIC_COEFF: usize = 8; +/// Cubic coefficient for the streak-gated timeout backoff. +const OUTBOUND_WINDOW_REDUCTION_CUBIC_COEFF: usize = OUTBOUND_WINDOW_GROWTH_CUBIC_COEFF; +/// Consecutive timeout batches that make up one window-reduction epoch. +const OUTBOUND_WINDOW_REDUCTION_EPOCH_TIMEOUTS: usize = OUTBOUND_WINDOW_GROWTH_EPOCH_SUCCESSES; +/// Timeouts tolerated after the adaptive window has already reached its floor. +const OUTBOUND_WINDOW_FLOOR_TIMEOUTS_BEFORE_DISCONNECT: usize = 3; /// Cached chain frontiers used by the block-sync reactor. #[derive(Copy, Clone, Debug, Eq, PartialEq)] @@ -151,6 +170,8 @@ pub(super) struct RoutineWiring { pub(super) received_throughput: Arc>, pub(super) sequencer_input: mpsc::Sender, pub(super) sequencer_input_bytes: Arc, + pub(super) sequencer_control: + mpsc::UnboundedSender, pub(super) actions: mpsc::Sender, pub(super) routine_to_reactor: mpsc::Sender, pub(super) view: watch::Receiver, @@ -308,17 +329,55 @@ pub(super) struct DownloadWindow { pub(super) outbound_request_window: usize, pub(super) timeout_recovery_slots: usize, pub(super) outstanding: Vec, + /// Completed error-free responses since the last timeout-driven reduction. + /// Drives the streak-gated cubic ramp in + /// [`increase_outbound_window_after_success`](Self::increase_outbound_window_after_success). + consecutive_successes: usize, + /// Consecutive timeout batches since the last successful response. + /// + /// Drives the same streak-gated cubic shape as successful response growth, + /// but downward. The window holds flat for a full timeout epoch, then lowers + /// faster as the consecutive timeout streak grows. + consecutive_timeouts: usize, + /// Window value at the last reduction — the floor the cubic ramp grows back + /// up from, so probing resumes from the post-backoff window rather than the + /// original slow-start point. + growth_base: usize, + /// Window value when the current timeout streak began. + reduction_base: usize, + /// Consecutive timeout batches observed while the window was already at the + /// minimum. Once this crosses the threshold, the caller should disconnect the + /// peer rather than retrying indefinitely at one request. + floor_timeouts: usize, +} + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub(super) enum TimeoutBackoffOutcome { + KeepPeer, + DisconnectPeer, } impl DownloadWindow { pub(super) fn new(config: &ZakuraBlockSyncConfig) -> Self { let max_inflight_requests = config.advertised_max_inflight_requests(); + // Slow-start: open at the configured initial window (clamped to the + // advertised hard cap) and grow toward the cap on success, rather than + // opening at the full `max_inflight`. + let initial_window = config + .initial_inflight_requests + .clamp(1, max_inflight_requests); + let initial_window = usize::try_from(initial_window) + .expect("u32 initial inflight requests fits in usize on supported targets"); Self { max_inflight_requests, - outbound_request_window: usize::try_from(max_inflight_requests) - .expect("u32 max inflight requests fits in usize on supported targets"), + outbound_request_window: initial_window, timeout_recovery_slots: 0, outstanding: Vec::new(), + consecutive_successes: 0, + consecutive_timeouts: 0, + growth_base: initial_window, + reduction_base: initial_window, + floor_timeouts: 0, } } @@ -334,27 +393,74 @@ impl DownloadWindow { .min(hard_capacity.saturating_sub(self.outstanding.len())) } - pub(super) fn reduce_outbound_window_after_timeout(&mut self) { - self.outbound_request_window = self.outbound_request_window.saturating_div(2).max(1); + pub(super) fn reduce_outbound_window_after_timeout(&mut self) -> TimeoutBackoffOutcome { + if self.consecutive_timeouts == 0 { + self.reduction_base = self.outbound_request_window; + } + self.consecutive_timeouts = self.consecutive_timeouts.saturating_add(1); + + let was_at_floor = self.outbound_request_window == 1; + let epoch = self.consecutive_timeouts / OUTBOUND_WINDOW_REDUCTION_EPOCH_TIMEOUTS; + if epoch > 0 { + let cubic_reduction = epoch + .saturating_mul(epoch) + .saturating_mul(epoch) + .saturating_mul(OUTBOUND_WINDOW_REDUCTION_CUBIC_COEFF); + let target = self.reduction_base.saturating_sub(cubic_reduction).max(1); + // Reduction only: never grow the window on a timeout. + self.outbound_request_window = self.outbound_request_window.min(target).max(1); + } + + // A timeout ends the current success streak; the cubic ramp restarts from + // the reduced window, so growth probes back up from the post-backoff floor. + self.consecutive_successes = 0; + self.growth_base = self.outbound_request_window; self.timeout_recovery_slots = self .timeout_recovery_slots .saturating_add(1) .min(self.hard_outbound_capacity()); + + if was_at_floor { + self.floor_timeouts = self.floor_timeouts.saturating_add(1); + } else { + self.floor_timeouts = 0; + } + + if self.floor_timeouts >= OUTBOUND_WINDOW_FLOOR_TIMEOUTS_BEFORE_DISCONNECT { + TimeoutBackoffOutcome::DisconnectPeer + } else { + TimeoutBackoffOutcome::KeepPeer + } } + /// Grow the adaptive window on a successful response using a streak-gated + /// cubic ramp: hold the window flat until a full epoch of consecutive + /// successes, then raise it to `growth_base + COEFF * epoch^3` (capped at the + /// hard cap). The step grows cubically with the no-error streak, so the window + /// opens gently at first and accelerates the longer the peer serves without a + /// timeout. Any timeout resets the streak and the base (see + /// [`reduce_outbound_window_after_timeout`](Self::reduce_outbound_window_after_timeout)). pub(super) fn increase_outbound_window_after_success(&mut self) { + self.consecutive_timeouts = 0; + self.reduction_base = self.outbound_request_window; + self.floor_timeouts = 0; + self.consecutive_successes = self.consecutive_successes.saturating_add(1); let max_window = self.hard_outbound_capacity(); - if self.outbound_request_window < max_window { - let growth = self - .outbound_request_window - .checked_div(OUTBOUND_WINDOW_SUCCESS_GROWTH_DIVISOR) - .unwrap_or(0) - .max(MIN_OUTBOUND_WINDOW_SUCCESS_GROWTH); - self.outbound_request_window = self - .outbound_request_window - .saturating_add(growth) - .min(max_window); + if self.outbound_request_window >= max_window { + return; + } + let epoch = self.consecutive_successes / OUTBOUND_WINDOW_GROWTH_EPOCH_SUCCESSES; + if epoch == 0 { + // Still inside the first flat epoch: do not open the window yet. + return; } + let cubic = epoch + .saturating_mul(epoch) + .saturating_mul(epoch) + .saturating_mul(OUTBOUND_WINDOW_GROWTH_CUBIC_COEFF); + let target = self.growth_base.saturating_add(cubic).min(max_window); + // Growth only: never shrink the window on a success. + self.outbound_request_window = self.outbound_request_window.max(target); } pub(super) fn record_outbound_request_scheduled(&mut self) { @@ -458,19 +564,20 @@ pub(super) struct OutstandingBlockRange { } impl OutstandingBlockRange { - /// Worst-case bytes still reserved for this request: the per-block worst case - /// for every requested height not yet received. The reservation for a request - /// only ever shrinks, so releasing this (on timeout/disconnect/short response) - /// never over-releases bytes that were already handed to the reorder buffer. + /// Bytes still reserved for this request: the sum of the per-height size + /// estimates for every requested height not yet received. Each received body + /// shrinks its estimate toward the actual size, so releasing this (on + /// timeout/disconnect/short response) never over-releases bytes already handed + /// to the reorder buffer. + #[cfg(test)] pub(super) fn reserved_bytes(&self) -> u64 { - let outstanding = self - .request + self.request .expected_blocks - .len() - .saturating_sub(self.received.len()); - // `outstanding` is a count bounded by `MAX_BS_BLOCKS_PER_REQUEST`, so the - // product cannot overflow `u64`; `saturating_mul` is belt-and-suspenders. - BS_PER_BLOCK_WORST_CASE_BYTES.saturating_mul(outstanding as u64) + .iter() + .filter(|expected| !self.has_received(expected.height)) + .fold(0u64, |acc, expected| { + acc.saturating_add(expected.estimated_bytes) + }) } pub(super) fn estimated_bytes_for_height(&self, height: block::Height) -> Option { @@ -490,11 +597,10 @@ impl OutstandingBlockRange { } /// Mark every requested height at or below `tip` as received and return the - /// worst-case bytes that those newly-received heights had reserved, so the - /// caller releases exactly the reservation those heights still held. + /// sum of the per-height size estimates those newly-received heights still + /// held, so the caller releases exactly the reservation those heights held. pub(super) fn mark_received_through(&mut self, tip: block::Height) -> u64 { - let newly_received = self - .request + self.request .expected_blocks .iter() .filter(|expected| { @@ -504,9 +610,9 @@ impl OutstandingBlockRange { .offset_for_height(expected.height) .is_some_and(|offset| self.received.insert_offset(offset)) }) - .count(); - // Bounded by `MAX_BS_BLOCKS_PER_REQUEST`; cannot overflow `u64`. - BS_PER_BLOCK_WORST_CASE_BYTES.saturating_mul(newly_received as u64) + .fold(0u64, |acc, expected| { + acc.saturating_add(expected.estimated_bytes) + }) } pub(super) fn is_complete(&self) -> bool { @@ -514,6 +620,72 @@ impl OutstandingBlockRange { } } +/// Pure per-height byte-accounting state. +/// +/// The shared [`ByteBudget`] is just the atomic sink. This ledger owns the +/// lifecycle arithmetic for one requested height: +/// `Reserved(estimate) -> Held(actual) -> Released`. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub(super) enum BlockBudgetLedger { + Reserved(u64), + Held(u64), + Released, +} + +impl BlockBudgetLedger { + pub(super) fn reserved(estimate: u64) -> Self { + Self::Reserved(estimate) + } + + pub(super) fn current_charge(self) -> u64 { + match self { + Self::Reserved(bytes) | Self::Held(bytes) => bytes, + Self::Released => 0, + } + } + + pub(super) fn release_reserved(&mut self) -> u64 { + let released = match *self { + Self::Reserved(bytes) => bytes, + Self::Held(_) | Self::Released => 0, + }; + *self = Self::Released; + released + } + + pub(super) fn reserved_charge(self) -> u64 { + match self { + Self::Reserved(bytes) => bytes, + Self::Held(_) | Self::Released => 0, + } + } + + pub(super) fn is_reserved(self) -> bool { + matches!(self, Self::Reserved(_)) + } + + /// Move a reserved height to held bytes and return the signed budget delta. + /// + /// Positive means charge more bytes; negative means release bytes. + pub(super) fn settle(&mut self, actual: u64) -> i128 { + match *self { + Self::Reserved(reserved) => { + *self = Self::Held(actual); + i128::from(actual) - i128::from(reserved) + } + Self::Released => 0, + Self::Held(_) => 0, + } + } + + /// Release the current charge exactly once. + pub(super) fn release(&mut self) -> u64 { + let charge = self.current_charge(); + *self = Self::Released; + charge + } +} + #[derive(Clone, Debug, Default)] pub(super) struct ReceivedBlockTracker { bits: u128, diff --git a/zebra-network/src/zakura/block_sync/tests.rs b/zebra-network/src/zakura/block_sync/tests.rs index c817f54cd65..e9d72358379 100644 --- a/zebra-network/src/zakura/block_sync/tests.rs +++ b/zebra-network/src/zakura/block_sync/tests.rs @@ -1,9 +1,14 @@ use std::{collections::HashMap, future}; +use proptest::{prop_assert, prop_assert_eq}; + use super::*; use super::{ config::{ - BS_PER_BLOCK_WORST_CASE_BYTES, DEFAULT_BS_FANOUT, DEFAULT_BS_MAX_SUBMITTED_BLOCK_APPLIES, + BS_PER_BLOCK_WORST_CASE_BYTES, DEFAULT_BS_FANOUT, DEFAULT_BS_FLOOR_PEER_AVOID_COOLDOWN, + DEFAULT_BS_FLOOR_WATCHDOG_TICK, DEFAULT_BS_MAX_INFLIGHT_BLOCK_BYTES, + DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BLOCKS, DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BYTES, + DEFAULT_BS_MAX_RESPONSE_BYTES, DEFAULT_BS_MAX_SUBMITTED_BLOCK_APPLIES, DEFAULT_BS_REQUEST_TIMEOUT, MAX_BS_INFLIGHT_REQUESTS, MAX_BS_RESPONSE_BYTES, }, reactor::node_id_from_block_peer_id, @@ -437,7 +442,7 @@ fn window_request(height: u32) -> OutstandingBlockRange { } #[test] -fn peer_outbound_request_window_halves_on_timeout_and_grows_on_success() { +fn peer_outbound_request_window_backs_off_and_grows_with_streaks() { let mut window = download_window(); let max_inflight = usize::try_from(MAX_BS_INFLIGHT_REQUESTS).expect("test max inflight fits usize"); @@ -446,19 +451,79 @@ fn peer_outbound_request_window_halves_on_timeout_and_grows_on_success() { window.outstanding.push(window_request(1)); assert_eq!(window.available_slots(), max_inflight - 1); - window.reduce_outbound_window_after_timeout(); - assert_eq!(window.outbound_request_window, max_inflight / 2); + for _ in 0..15 { + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + } + assert_eq!( + window.outbound_request_window, max_inflight, + "window holds flat within the first timeout epoch" + ); + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + assert_eq!(window.outbound_request_window, max_inflight - 8); for _ in 0..16 { - window.reduce_outbound_window_after_timeout(); + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + } + assert_eq!(window.outbound_request_window, max_inflight - 64); + + for _ in 0..(16 * 16) { + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + if window.outbound_request_window == 1 { + break; + } } assert_eq!(window.outbound_request_window, 1); assert_eq!(window.available_slots(), window.timeout_recovery_slots); + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::DisconnectPeer + ); + // Streak-gated cubic ramp: the window holds flat for a full epoch of + // consecutive successes, then steps up. From the reduced base of 1, the first + // epoch (16 successes) raises it to base + COEFF * 1^3 = 1 + 8 = 9. window.outstanding.clear(); window.timeout_recovery_slots = 0; + for _ in 0..15 { + window.increase_outbound_window_after_success(); + } + assert_eq!( + window.outbound_request_window, 1, + "window holds flat within the first success epoch" + ); window.increase_outbound_window_after_success(); - assert_eq!(window.outbound_request_window, 65); + assert_eq!( + window.outbound_request_window, 9, + "the first full success epoch steps the window up by COEFF * 1^3" + ); + // A second epoch accelerates cubically: base + COEFF * 2^3 = 1 + 64 = 65. + for _ in 0..16 { + window.increase_outbound_window_after_success(); + } + assert_eq!( + window.outbound_request_window, 65, + "the second epoch grows cubically faster than the first" + ); window.outbound_request_window = max_inflight; window.increase_outbound_window_after_success(); @@ -477,9 +542,14 @@ fn peer_timeout_recovery_slot_replaces_timed_out_request_above_reduced_window() assert_eq!(window.available_slots(), 0); - window.reduce_outbound_window_after_timeout(); - assert_eq!(window.outbound_request_window, 4); - assert_eq!(window.timeout_recovery_slots, 1); + for _ in 0..16 { + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + } + assert_eq!(window.outbound_request_window, 1); + assert_eq!(window.timeout_recovery_slots, 8); assert_eq!( window.available_slots(), 0, @@ -494,7 +564,7 @@ fn peer_timeout_recovery_slot_replaces_timed_out_request_above_reduced_window() ); window.record_outbound_request_scheduled(); - assert_eq!(window.timeout_recovery_slots, 0); + assert_eq!(window.timeout_recovery_slots, 7); window.outstanding.push(window_request(9)); assert_eq!( window.available_slots(), @@ -604,7 +674,7 @@ fn work_queue_with( items: impl IntoIterator, ) -> super::work_queue::WorkQueue { let queue = super::work_queue::WorkQueue::new(block::Height(floor)); - queue.set_estimator_for_tests(750, 1); + queue.set_estimate_floor_for_tests(1); queue.extend(items); queue } @@ -621,7 +691,41 @@ fn block_meta(block: &Arc) -> BlockSyncBlockMeta { fn block_sync_config_defaults_and_round_trips() { let default = ZakuraBlockSyncConfig::default(); assert_eq!(default.max_blocks_per_response, 1); - assert_eq!(default.max_inflight_requests, 2_048); + assert_eq!(default.max_inflight_requests, 32000); + assert_eq!( + default.max_inflight_block_bytes, + DEFAULT_BS_MAX_INFLIGHT_BLOCK_BYTES + ); + assert_eq!( + default.max_reorder_lookahead_bytes, + DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BYTES + ); + assert_eq!( + default.max_reorder_lookahead_blocks, + DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BLOCKS + ); + assert_eq!(default.floor_watchdog_tick, DEFAULT_BS_FLOOR_WATCHDOG_TICK); + assert_eq!( + default.floor_peer_avoid_cooldown, + DEFAULT_BS_FLOOR_PEER_AVOID_COOLDOWN + ); + assert_eq!( + default.effective_max_reorder_lookahead_bytes(), + DEFAULT_BS_MAX_REORDER_LOOKAHEAD_BYTES + ); + assert_eq!( + default.floor_request_byte_reservation(), + u64::from(DEFAULT_BS_MAX_RESPONSE_BYTES) + ); + assert_eq!( + default.effective_floor_watchdog_tick(), + DEFAULT_BS_FLOOR_WATCHDOG_TICK + ); + assert_eq!( + default.effective_floor_peer_avoid_cooldown(), + DEFAULT_BS_FLOOR_PEER_AVOID_COOLDOWN + ); + assert!(default.validate().is_ok()); assert_eq!( default.max_submitted_block_applies, DEFAULT_BS_MAX_SUBMITTED_BLOCK_APPLIES @@ -644,6 +748,27 @@ fn block_sync_config_defaults_and_round_trips() { assert_eq!(config.zakura.block_sync.max_submitted_block_applies, 9); } +#[test] +fn config_validate_rejects_degenerate_values() { + let mut config = ZakuraBlockSyncConfig { + max_inflight_block_bytes: 0, + ..ZakuraBlockSyncConfig::default() + }; + assert!(config.validate().is_err()); + + config = ZakuraBlockSyncConfig { + max_reorder_lookahead_blocks: 0, + ..ZakuraBlockSyncConfig::default() + }; + assert!(config.validate().is_err()); + + config = ZakuraBlockSyncConfig { + max_inflight_block_bytes: u64::from(DEFAULT_BS_MAX_RESPONSE_BYTES), + ..ZakuraBlockSyncConfig::default() + }; + assert!(config.validate().is_err()); +} + #[test] fn codec_round_trips_every_message_variant() { round_trip(BlockSyncMessage::Status(status())); @@ -1062,6 +1187,272 @@ fn work_queue_budgeted_take_preserves_estimates_through_take_and_return() { assert_eq!(retaken[0].1.estimated_bytes, 12_345); } +#[test] +fn admission_blocks_above_floor_at_cap_but_keeps_floor_fundable() { + let config = ZakuraBlockSyncConfig { + max_inflight_block_bytes: 40_000_000, + max_reorder_lookahead_bytes: 500, + max_reorder_lookahead_blocks: 4, + ..ZakuraBlockSyncConfig::default() + }; + let snapshot = super::admission::AdmissionSnapshot { + download_floor: block::Height(10), + reorder_buffered_bytes: 500, + reorder_buffered_blocks: 1, + applying_buffered_bytes: 0, + applying_buffered_blocks: 0, + sequencer_input_queued_bytes: 0, + reserved_above_floor_bytes: 0, + reserved_above_floor_blocks: 0, + budget_available: 40_000_000, + }; + + let floor = super::admission::admission_decision(&config, snapshot, block::Height(11), 1_000) + .expect("floor rescue remains admitted at the look-ahead cap"); + assert_eq!(floor.priority, super::admission::RequestPriority::Floor); + assert_eq!(floor.max_request_bytes, 1_000); + + assert_eq!( + super::admission::admission_decision(&config, snapshot, block::Height(12), 1_000), + None, + "above-floor work stops at the look-ahead cap" + ); + + let under_cap = super::admission::AdmissionSnapshot { + reorder_buffered_bytes: 100, + budget_available: 40_000_000, + ..snapshot + }; + let above = + super::admission::admission_decision(&config, under_cap, block::Height(12), u64::MAX) + .expect("above-floor work is admitted below the cap"); + assert_eq!( + above.priority, + super::admission::RequestPriority::AboveFloor + ); + assert_eq!(above.max_request_bytes, 400); +} + +#[test] +fn admission_counts_inflight_to_sequencer_bytes() { + let config = ZakuraBlockSyncConfig { + max_inflight_block_bytes: 64_000_000, + max_reorder_lookahead_bytes: 1_000, + max_reorder_lookahead_blocks: 10, + ..ZakuraBlockSyncConfig::default() + }; + let snapshot = super::admission::AdmissionSnapshot { + download_floor: block::Height(10), + reorder_buffered_bytes: 200, + reorder_buffered_blocks: 1, + applying_buffered_bytes: 200, + applying_buffered_blocks: 1, + sequencer_input_queued_bytes: 600, + reserved_above_floor_bytes: 0, + reserved_above_floor_blocks: 0, + budget_available: 64_000_000, + }; + + assert_eq!( + super::admission::admission_decision(&config, snapshot, block::Height(12), 1_000), + None, + "above-floor admission includes bytes already queued to the sequencer" + ); +} + +#[test] +fn total_resident_plateaus_under_commit_stall() { + let config = ZakuraBlockSyncConfig { + max_inflight_block_bytes: 64_000_000, + max_reorder_lookahead_bytes: 1_000, + max_reorder_lookahead_blocks: 10, + ..ZakuraBlockSyncConfig::default() + }; + let snapshot = super::admission::AdmissionSnapshot { + download_floor: block::Height(10), + reorder_buffered_bytes: 300, + reorder_buffered_blocks: 1, + applying_buffered_bytes: 700, + applying_buffered_blocks: 1, + sequencer_input_queued_bytes: 0, + reserved_above_floor_bytes: 0, + reserved_above_floor_blocks: 0, + budget_available: 64_000_000, + }; + + assert_eq!( + super::admission::admission_decision(&config, snapshot, block::Height(12), 1_000), + None, + "above-floor admission includes applying bytes held during a commit stall" + ); +} + +#[test] +fn floor_priority_request_does_not_buffer_above_floor_past_cap() { + let config = ZakuraBlockSyncConfig { + max_inflight_block_bytes: 64_000_000, + max_reorder_lookahead_bytes: 1_000, + max_reorder_lookahead_blocks: 10, + ..ZakuraBlockSyncConfig::default() + }; + let capped = super::admission::AdmissionSnapshot { + download_floor: block::Height(10), + reorder_buffered_bytes: 1_000, + reorder_buffered_blocks: 1, + applying_buffered_bytes: 0, + applying_buffered_blocks: 0, + sequencer_input_queued_bytes: 0, + reserved_above_floor_bytes: 0, + reserved_above_floor_blocks: 0, + budget_available: 64_000_000, + }; + + assert_eq!( + super::admission::admission_decision(&config, capped, block::Height(12), 1_000), + None, + "the above-floor tail of a floor-starting request is refused at the cap" + ); + assert_eq!( + super::admission::admission_decision(&config, capped, block::Height(11), 1_000) + .expect("floor height remains fundable") + .priority, + super::admission::RequestPriority::Floor + ); +} + +#[test] +fn work_queue_force_cancel_and_owner_timeout_release_once() { + let queue = work_queue_with(0, [needed(1, BlockSizeEstimate::Advertised(100))]); + let mut budget = ByteBudget::new(1_000); + let taken = queue.take_in_range(block::Height(1), block::Height(1), 1); + assert_eq!(taken.len(), 1); + assert!(budget.try_reserve(100)); + assert_eq!(queue.mark_reserved([block::Height(1)]), 100); + assert_eq!(budget.reserved(), 100); + + let watchdog_released = queue.release_and_return_items([block::Height(1)]); + budget.release(watchdog_released); + assert_eq!(watchdog_released, 100); + assert_eq!(budget.reserved(), 0); + assert!(queue.pending_contains(block::Height(1))); + + let late_owner_released = queue.release_and_return_items([block::Height(1)]); + budget.release(late_owner_released); + assert_eq!(late_owner_released, 0); + assert_eq!(budget.reserved(), 0); + assert!(queue.pending_contains(block::Height(1))); +} + +#[test] +fn watchdog_after_held_settle_releases_once() { + let queue = work_queue_with(0, [needed(1, BlockSizeEstimate::Advertised(100))]); + let mut budget = ByteBudget::new(1_000); + let taken = queue.take_in_range(block::Height(1), block::Height(1), 1); + assert_eq!(taken.len(), 1); + assert!(budget.try_reserve(100)); + assert_eq!(queue.mark_reserved([block::Height(1)]), 100); + + let delta = queue + .settle_active_reserved_height(block::Height(1), 80) + .expect("active reserved height settles"); + assert_eq!(delta, -20); + budget.release(20); + assert_eq!(budget.reserved(), 80); + + let watchdog_released = queue.release_reserved_and_return_items([block::Height(1)]); + budget.release(watchdog_released); + assert_eq!( + watchdog_released, 0, + "watchdog must not release a held body owned by the sequencer handoff" + ); + assert!(queue.in_flight_contains(block::Height(1))); + assert_eq!(budget.reserved(), 80); + + budget.release(80); + assert_eq!(queue.advance_floor(block::Height(1)), 0); + assert_eq!(budget.reserved(), 0); +} + +#[test] +fn late_delivery_after_watchdog_cancellation_does_not_resurrect_released_claim() { + let queue = work_queue_with(0, [needed(1, BlockSizeEstimate::Advertised(100))]); + let mut budget = ByteBudget::new(1_000); + let taken = queue.take_in_range(block::Height(1), block::Height(1), 1); + assert_eq!(taken.len(), 1); + assert!(budget.try_reserve(100)); + assert_eq!(queue.mark_reserved([block::Height(1)]), 100); + + let watchdog_released = queue.release_reserved_and_return_items([block::Height(1)]); + budget.release(watchdog_released); + assert_eq!(budget.reserved(), 0); + assert!(queue.pending_contains(block::Height(1))); + + assert_eq!( + queue.settle_active_reserved_height(block::Height(1), 80), + None, + "a late body cannot settle a claim the watchdog already released" + ); + assert_eq!(budget.reserved(), 0); +} + +#[test] +fn mark_held_direct_does_not_orphan_a_charge() { + let queue = work_queue_with(0, [needed(1, BlockSizeEstimate::Advertised(100))]); + let mut budget = ByteBudget::new(1_000); + let taken = queue.take_in_range(block::Height(1), block::Height(1), 1); + assert_eq!(taken.len(), 1); + assert!(budget.try_reserve(100)); + assert_eq!(queue.mark_reserved([block::Height(1)]), 100); + + assert!(budget.try_reserve(80)); + let old_charge = queue.mark_held_direct(block::Height(1), 80); + budget.release(old_charge); + assert_eq!(old_charge, 100); + assert_eq!(budget.reserved(), 80); + + let released = queue.release_heights([block::Height(1)]); + budget.release(released); + assert_eq!(released, 80); + assert_eq!(budget.reserved(), 0); +} + +#[test] +fn release_reserved_mixed_reserved_held_conserves_budget() { + let queue = work_queue_with( + 0, + [ + needed(1, BlockSizeEstimate::Advertised(100)), + needed(2, BlockSizeEstimate::Advertised(100)), + ], + ); + let mut budget = ByteBudget::new(1_000); + let taken = queue.take_in_range(block::Height(1), block::Height(2), 2); + assert_eq!(taken.len(), 2); + assert!(budget.try_reserve(200)); + assert_eq!( + queue.mark_reserved([block::Height(1), block::Height(2)]), + 200 + ); + + let delta = queue + .settle_active_reserved_height(block::Height(1), 80) + .expect("active height settles"); + assert_eq!(delta, -20); + budget.release(20); + assert_eq!(budget.reserved(), 180); + + let released = queue.advance_floor(block::Height(2)); + budget.release(released); + assert_eq!( + released, 100, + "WorkQueue releases only the still-reserved height; held bytes are released by Sequencer" + ); + assert_eq!(budget.reserved(), 80); + + budget.release(80); + assert_eq!(budget.reserved(), 0); +} + #[test] fn work_queue_take_does_not_clamp_high_to_floor() { // The committed floor is NOT an upper bound on a take: a peer fetches as far @@ -1731,9 +2122,9 @@ async fn reactor_timeout_backoff_is_local_and_healthy_peer_keeps_filling() { #[test] fn work_queue_estimate_clamps_hint_between_floor_and_max_block_bytes() { - use super::work_queue::{DEFAULT_BS_EWMA_SEED_BYTES, DEFAULT_BS_SIZE_FLOOR_BYTES}; + use super::work_queue::DEFAULT_BS_SIZE_FLOOR_BYTES; - // Default estimator: Unknown -> EWMA seed; tiny hints clamp up to the floor; + // Default estimator: Unknown -> worst case; tiny hints clamp up to the floor; // huge hints clamp down to MAX_BLOCK_BYTES; ordinary hints pass through. let queue = super::work_queue::WorkQueue::new(block::Height(0)); queue.extend([ @@ -1750,15 +2141,14 @@ fn work_queue_estimate_clamps_hint_between_floor_and_max_block_bytes() { .1 .estimated_bytes }; - assert_eq!(item(1), DEFAULT_BS_EWMA_SEED_BYTES); + assert_eq!(item(1), block::MAX_BLOCK_BYTES); assert_eq!(item(2), DEFAULT_BS_SIZE_FLOOR_BYTES); assert_eq!(item(3), 12_345); assert_eq!(item(4), block::MAX_BLOCK_BYTES); - // The test estimator override changes the EWMA seed and floor (floor < ewma - // so the two clamp endpoints stay distinguishable). + // The test estimator override changes the floor clamp. let tuned = super::work_queue::WorkQueue::new(block::Height(0)); - tuned.set_estimator_for_tests(750, 100); + tuned.set_estimate_floor_for_tests(100); tuned.extend([ needed(10, BlockSizeEstimate::Unknown), needed(11, BlockSizeEstimate::Advertised(50)), // below the tuned floor @@ -1770,7 +2160,7 @@ fn work_queue_estimate_clamps_hint_between_floor_and_max_block_bytes() { .unwrap() .1 .estimated_bytes, - 750 + block::MAX_BLOCK_BYTES ); assert_eq!( tuned @@ -1984,6 +2374,160 @@ fn shed_top_for_floor_starvation_funds_lowest_pending_by_dropping_top() { ); } +#[test] +fn shed_top_for_floor_starvation_funds_outstanding_floor_by_dropping_top() { + let worst = BS_PER_BLOCK_WORST_CASE_BYTES; + let mut budget = ByteBudget::new(2 * worst); + let work = work_queue_with( + 0, + [ + needed(1, BlockSizeEstimate::Unknown), + needed(5, BlockSizeEstimate::Unknown), + needed(6, BlockSizeEstimate::Unknown), + ], + ); + let mut seq = test_sequencer(0, 100); + let block = mainnet_block(&BLOCK_MAINNET_1_BYTES); + + // Height 1 is outstanding on a slow peer, so it is not pending. The top + // reorder bodies saturate the budget and must still be shed to make the + // floor watchdog's re-request fundable. + work.take_in_range(block::Height(1), block::Height(1), 1); + for height in [5u32, 6] { + work.take_in_range(block::Height(height), block::Height(height), 1); + assert!(budget.try_reserve(worst)); + seq.accept_body( + block::Height(height), + block::Hash([height as u8; 32]), + block.clone(), + worst, + peer(0), + ); + } + assert_eq!(budget.available(), 0, "budget saturated by buffered bodies"); + assert!( + !work.pending_contains(block::Height(1)), + "the floor gap is outstanding, not pending" + ); + + let shed = super::sequencer_task::shed_top_for_floor_starvation(&mut budget, &work, &mut seq); + assert!( + shed, + "the top buffered body is shed even while the floor gap is outstanding" + ); + assert!( + budget.available() >= worst, + "freed budget can now fund the watchdog floor re-request" + ); + assert!( + !seq.reorder_contains(block::Height(6)), + "the top body is evicted" + ); + assert!( + seq.reorder_contains(block::Height(5)), + "the lower buffered body is kept" + ); + assert!( + work.pending_contains(block::Height(6)), + "the evicted height is returned to pending for re-fetch" + ); +} + +#[test] +fn shed_top_until_available_self_funds_floor_reservation() { + let worst = BS_PER_BLOCK_WORST_CASE_BYTES; + let mut budget = ByteBudget::new(3 * worst); + let work = work_queue_with( + 0, + [ + needed(1, BlockSizeEstimate::Unknown), + needed(8, BlockSizeEstimate::Unknown), + needed(9, BlockSizeEstimate::Unknown), + needed(10, BlockSizeEstimate::Unknown), + ], + ); + let mut seq = test_sequencer(0, 100); + let block = mainnet_block(&BLOCK_MAINNET_1_BYTES); + + // The floor has already been taken for a request whose reservation lost a + // budget race. The speculative bodies saturate the budget, so the floor + // reservation path must pop enough high-tail bodies and retry. + work.take_in_range(block::Height(1), block::Height(1), 1); + for height in [8u32, 9, 10] { + work.take_in_range(block::Height(height), block::Height(height), 1); + assert!(budget.try_reserve(worst)); + seq.accept_body( + block::Height(height), + block::Hash([height as u8; 32]), + block.clone(), + worst, + peer(0), + ); + } + assert_eq!(budget.available(), 0); + + let shed = + super::sequencer_task::shed_top_until_available(&mut budget, &work, &mut seq, 2 * worst); + assert!(shed, "the high tail is popped to fund the floor request"); + assert!( + budget.available() >= 2 * worst, + "the pop frees the requested floor bytes" + ); + assert!( + !seq.reorder_contains(block::Height(10)) && !seq.reorder_contains(block::Height(9)), + "the furthest buffered bodies are evicted first" + ); + assert!( + seq.reorder_contains(block::Height(8)), + "nearer buffered body is kept once the request is fundable" + ); +} + +#[test] +fn window_reduction_uses_consecutive_timeout_streak() { + let mut window = download_window(); + window.max_inflight_requests = 256; + window.outbound_request_window = 256; + + for _ in 0..15 { + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + } + assert_eq!(window.outbound_request_window, 256); + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + assert_eq!(window.outbound_request_window, 248); + + for _ in 0..16 { + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + } + assert_eq!(window.outbound_request_window, 192); + + // A successful response resets the timeout streak, so the next timeout starts + // a fresh cubic backoff from the current window instead of continuing the old + // streak. + window.increase_outbound_window_after_success(); + for _ in 0..15 { + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + } + assert_eq!(window.outbound_request_window, 192); + assert_eq!( + window.reduce_outbound_window_after_timeout(), + TimeoutBackoffOutcome::KeepPeer + ); + assert_eq!(window.outbound_request_window, 184); +} + // ---- Sequencer commit pipeline ---- fn test_sequencer(verified_tip: u32, submitted_apply_limit: usize) -> Sequencer { @@ -2277,15 +2821,17 @@ fn reorder_fuzzes_arrival_order_as_parent_first() { /// Build an outstanding three-block range whose worst-case reservation is already /// held against `budget`, mirroring what the scheduler does at send time. +/// Per-height size-estimate reservation used by the budget-accounting tests. +const THREE_BLOCK_ESTIMATE: u64 = 1_000; + fn outstanding_three_block_range(budget: &mut ByteBudget) -> OutstandingBlockRange { - let worst = BS_PER_BLOCK_WORST_CASE_BYTES; let request = BlockRangeRequest { start_height: block::Height(1), count: 3, anchor_hash: block::Hash([1; 32]), - // Worst-case reservation: three blocks each reserve one worst-case share. - estimated_bytes: worst * 3, - // Size hints below the worst case; the reservation does not depend on them. + // Size-estimate reservation: each block reserves its size hint, so the + // request reserves the sum of the per-height estimates below. + estimated_bytes: THREE_BLOCK_ESTIMATE * 3, expected_blocks: vec![ ExpectedBlock { height: block::Height(1), @@ -2313,15 +2859,156 @@ fn outstanding_three_block_range(budget: &mut ByteBudget) -> OutstandingBlockRan } } -/// The global reservation must never exceed the budget and must monotonically -/// shrink over a block's lifetime across the download -> buffer -> apply -> commit -/// path, and across timeout/duplicate/short-response paths. This is the budget -/// half of the worst-case lossless scheme: a block reserves worst case at send, -/// only ever shrinks toward its actual serialized size, and is never re-reserved. +#[test] +fn block_budget_ledger_settles_and_releases_current_charge() { + let mut under = BlockBudgetLedger::reserved(1_000); + assert_eq!(under.current_charge(), 1_000); + assert_eq!(under.settle(700), -300); + assert_eq!(under.current_charge(), 700); + assert_eq!(under.release(), 700); + assert_eq!(under.release(), 0); + + let mut equal = BlockBudgetLedger::reserved(1_000); + assert_eq!(equal.settle(1_000), 0); + assert_eq!(equal.release(), 1_000); + + let mut over = BlockBudgetLedger::reserved(1_000); + assert_eq!(over.settle(1_300), 300); + assert_eq!(over.current_charge(), 1_300); + assert_eq!(over.release(), 1_300); + + let mut released = BlockBudgetLedger::Released; + assert_eq!(released.settle(900), 0); + assert_eq!(released.current_charge(), 0); + assert_eq!(released.release(), 0); +} + +#[test] +fn budget_audit_catches_injected_drift() { + let mut budget = ByteBudget::new(1_000); + assert!(budget.try_reserve(400)); + assert!(budget.audit(400, "test matching audit")); + assert!( + !budget.audit(300, "test injected drift"), + "audit reports mismatched derived accounting" + ); + budget.release(400); + assert!(budget.audit(0, "test released audit")); +} + +proptest::proptest! { + #![proptest_config(proptest::test_runner::Config::with_cases(256))] + + #[test] + fn admission_decision_respects_lookahead_bounds( + reorder_bytes in 0u64..2_000, + applying_bytes in 0u64..2_000, + input_bytes in 0u64..2_000, + reserved_bytes in 0u64..2_000, + reorder_blocks in 0u64..20, + applying_blocks in 0u64..20, + reserved_blocks in 0u64..20, + ) { + let config = ZakuraBlockSyncConfig { + max_inflight_block_bytes: 64_000_000, + max_reorder_lookahead_bytes: 1_000, + max_reorder_lookahead_blocks: 10, + ..ZakuraBlockSyncConfig::default() + }; + let snapshot = super::admission::AdmissionSnapshot { + download_floor: block::Height(10), + reorder_buffered_bytes: reorder_bytes, + reorder_buffered_blocks: reorder_blocks, + applying_buffered_bytes: applying_bytes, + applying_buffered_blocks: applying_blocks, + sequencer_input_queued_bytes: input_bytes, + reserved_above_floor_bytes: reserved_bytes, + reserved_above_floor_blocks: reserved_blocks, + budget_available: 64_000_000, + }; + let held_bytes = reorder_bytes + .saturating_add(applying_bytes) + .saturating_add(input_bytes) + .saturating_add(reserved_bytes); + let held_blocks = reorder_blocks + .saturating_add(applying_blocks) + .saturating_add(reserved_blocks); + let above = super::admission::admission_decision( + &config, + snapshot, + block::Height(12), + 1_000, + ); + if held_bytes >= config.effective_max_reorder_lookahead_bytes() + || held_blocks >= u64::from(config.max_reorder_lookahead_blocks) + { + prop_assert_eq!(above, None); + } else { + prop_assert!(above.is_some()); + } + + let floor = super::admission::admission_decision( + &config, + snapshot, + block::Height(11), + 1_000, + ); + prop_assert_eq!( + floor.expect("floor remains admitted while budget is available").priority, + super::admission::RequestPriority::Floor + ); + } +} + +proptest::proptest! { + #![proptest_config(proptest::test_runner::Config::with_cases(256))] + + #[test] + fn block_budget_ledger_mirrors_byte_budget( + estimate in 1u64..1_000_000, + actual in 1u64..1_000_000, + release_before_receipt in proptest::bool::ANY, + ) { + let mut ledger = BlockBudgetLedger::reserved(estimate); + let mut budget = ByteBudget::new(u64::MAX); + prop_assert!(budget.try_reserve(estimate)); + + if release_before_receipt { + let released = ledger.release(); + budget.release(released); + prop_assert_eq!(budget.reserved(), 0); + let delta = ledger.settle(actual); + prop_assert_eq!(delta, 0); + prop_assert_eq!(budget.reserved(), 0); + prop_assert_eq!(ledger.current_charge(), 0); + let released = ledger.release(); + budget.release(released); + prop_assert_eq!(budget.reserved(), 0); + } else { + let delta = ledger.settle(actual); + if delta >= 0 { + budget.charge(u64::try_from(delta).expect("positive test delta fits in u64")); + } else { + budget.release(u64::try_from(-delta).expect("negative test delta fits in u64")); + } + prop_assert_eq!(ledger.current_charge(), actual); + prop_assert_eq!(budget.reserved(), actual); + + let released = ledger.release(); + budget.release(released); + prop_assert_eq!(budget.reserved(), 0); + prop_assert_eq!(ledger.current_charge(), 0); + } + } +} + +/// The global reservation settles to the current held body bytes across the +/// download -> buffer -> apply -> commit path, and releases exactly once across +/// timeout/duplicate/short-response paths. #[test] fn budget_reservation_never_exceeds_max_and_only_shrinks_per_block() { - let worst = BS_PER_BLOCK_WORST_CASE_BYTES; - let max = worst * 3; + let estimate = THREE_BLOCK_ESTIMATE; + let max = estimate * 3; // Happy path: download -> shrink-on-receipt -> buffer -> apply -> commit. { @@ -2332,27 +3019,28 @@ fn budget_reservation_never_exceeds_max_and_only_shrinks_per_block() { assert!(budget.reserved() <= budget.max_bytes_for_test()); let block = mainnet_block(&BLOCK_MAINNET_1_BYTES); - // Receive each height: release `worst - actual`, keep `actual` reserved, + // Receive each height: release `estimate - actual`, keep `actual` reserved, // and hand `actual` to the reorder buffer without re-reserving. + let actuals = [700u64, 800, 900]; // each < its estimate, varies per block for (index, height) in [block::Height(1), block::Height(2), block::Height(3)] .into_iter() .enumerate() { let before = budget.reserved(); - let actual = 1_000u64 + index as u64; // < worst, varies per block - budget.release(worst.saturating_sub(actual)); + let actual = actuals[index]; + budget.settle(estimate, actual); outstanding.mark_received(height); assert_eq!( reorder.insert(height, block.clone(), actual, peer(0)), ReorderInsertResult::Inserted ); - // Per-block reservation only shrank (worst -> actual), never grew. + // Per-block reservation only shrank (estimate -> actual), never grew. assert!(budget.reserved() <= before); assert!(budget.reserved() <= budget.max_bytes_for_test()); } assert!(outstanding.is_complete()); assert_eq!(outstanding.reserved_bytes(), 0); - assert_eq!(budget.reserved(), 1_000 + 1_001 + 1_002); + assert_eq!(budget.reserved(), 700 + 800 + 900); // Commit: draining to apply carries the actual bytes; the apply finish // releases them. @@ -2362,27 +3050,28 @@ fn budget_reservation_never_exceeds_max_and_only_shrinks_per_block() { applied_bytes += bytes; floor = block::Height(floor.0 + 1); } - assert_eq!(applied_bytes, 1_000 + 1_001 + 1_002); + assert_eq!(applied_bytes, 700 + 800 + 900); budget.release(applied_bytes); assert_eq!(budget.reserved(), 0); } // Timeout / short-response path: heights that never buffer release exactly - // their worst-case share, with no leak and no double-release. + // their size-estimate share, with no leak and no double-release. { let mut budget = ByteBudget::new(max); let mut outstanding = outstanding_three_block_range(&mut budget); - assert_eq!(budget.reserved(), worst * 3); - // A short response delivers only height 1; release its worst-case share. - budget.release(worst.saturating_sub(1_000)); + assert_eq!(budget.reserved(), estimate * 3); + // A short response delivers only height 1; release its estimate's slack. + let actual = 700u64; + budget.settle(estimate, actual); outstanding.mark_received(block::Height(1)); - // The remaining two unreceived heights still reserve worst case each. - assert_eq!(outstanding.reserved_bytes(), worst * 2); + // The remaining two unreceived heights still reserve their estimate each. + assert_eq!(outstanding.reserved_bytes(), estimate * 2); assert!(budget.reserved() <= budget.max_bytes_for_test()); - // On timeout the outstanding range releases its still-reserved worst case. + // On timeout the outstanding range releases its still-reserved estimate. budget.release(outstanding.reserved_bytes()); // Plus the actual bytes held for the one received-but-not-buffered height. - budget.release(1_000); + budget.release(actual); assert_eq!(budget.reserved(), 0); } @@ -2411,23 +3100,21 @@ fn budget_reservation_never_exceeds_max_and_only_shrinks_per_block() { } /// A body whose actual serialized size exceeds its advertised size hint is still -/// accepted and buffered: worst case (not the hint) was reserved up front, so the -/// shrink-on-receipt cannot fail. Under the old release-then-reserve scheme this -/// path could re-reserve more than the released estimate and drop a valid body. +/// accepted and buffered, and the byte budget charges the overshoot so it cannot +/// issue more work while under-counting held bodies. #[test] -fn underestimated_body_is_buffered_without_budget_drop() { - let worst = BS_PER_BLOCK_WORST_CASE_BYTES; - // Budget holds exactly one worst-case share, so a hint-sized re-reservation - // would have had no headroom for an underestimated body. - let mut budget = ByteBudget::new(worst); +fn underestimated_body_is_buffered_and_charges_budget_delta() { + let hint = 1_000u64; + // Budget holds several hint-sized shares: enough to reserve this body's hint. + let mut budget = ByteBudget::new(hint * 4); let mut reorder = ReorderBuffer::new(); - let hint = 1_000u64; let request = BlockRangeRequest { start_height: block::Height(1), count: 1, anchor_hash: block::Hash([1; 32]), - estimated_bytes: worst, + // Size-estimate reservation: the per-height hint, not worst case. + estimated_bytes: hint, expected_blocks: vec![ExpectedBlock { height: block::Height(1), hash: block::Hash([1; 32]), @@ -2441,27 +3128,33 @@ fn underestimated_body_is_buffered_without_budget_drop() { deadline: Instant::now(), received: ReceivedBlockTracker::default(), }; - assert_eq!(budget.reserved(), worst); + assert_eq!(budget.reserved(), hint); // The body's actual serialized size is far larger than the hint (but still - // <= MAX_BLOCK_BYTES, the per-block worst case). + // <= MAX_BLOCK_BYTES). let actual = hint * 50; - assert!(actual < worst); + assert!(actual < BS_PER_BLOCK_WORST_CASE_BYTES); assert!(actual > hint); - // Receipt: shrink toward the actual size and hand it to the reorder buffer - // without re-reserving. The shrink is non-negative because actual <= worst. - budget.release(worst.saturating_sub(actual)); + // Receipt: settle toward the actual size. Because actual exceeds the reserved + // hint, the budget charges the delta even though the body is already admitted. + budget.settle(hint, actual); outstanding.mark_received(block::Height(1)); let block = mainnet_block(&BLOCK_MAINNET_1_BYTES); assert_eq!( reorder.insert(block::Height(1), block, actual, peer(0)), ReorderInsertResult::Inserted, - "an underestimated body must still buffer; worst case was reserved up front" + "an underestimated body must still buffer; a received body is never dropped" ); assert_eq!(reorder.buffered_bytes(), actual); assert_eq!(budget.reserved(), actual); - assert!(budget.reserved() <= budget.max_bytes_for_test()); + assert_eq!(budget.available(), 0); + assert!( + !budget.try_reserve(hint), + "charging the underestimated delta closes the budget gate" + ); + budget.release(reorder.drop_through(block::Height(1))); + assert_eq!(budget.reserved(), 0); } #[test] @@ -2993,7 +3686,9 @@ async fn reactor_keeps_submitted_body_budget_until_apply_finishes() { let blocks = mainnet_blocks_1_to_3(); let block1_size = block_size(&blocks[0]); let mut config = immediate_body_download_config(); - config.max_inflight_block_bytes = BS_PER_BLOCK_WORST_CASE_BYTES; + // One block's size hint of budget: size-based reservation now means one + // advertised body fills the budget, throttling to one in-flight request. + config.max_inflight_block_bytes = u64::from(block1_size); let (tip_tx, tip_rx) = watch::channel((block::Height(0), block::Hash([0; 32]))); let startup = BlockSyncStartup::new( @@ -3135,7 +3830,7 @@ async fn reactor_keeps_submitted_body_budget_until_apply_finishes() { reactor_task.abort(); } -/// Pins the Sequencer task producer-filter substitution `height > committed_floor && +/// Pins the Sequencer task producer-filter substitution `height > request_floor && /// !work.in_flight_contains(height)`. A height that has been received and is held /// in the commit pipeline (buffered / applying / submitted) was taken into the /// WorkQueue's `in_flight` at issuance and stays there until it commits, so a @@ -3573,7 +4268,9 @@ async fn reactor_keeps_applying_body_after_non_advancing_duplicate_result() { let blocks = mainnet_blocks_1_to_3(); let block1_size = block_size(&blocks[0]); let mut config = immediate_body_download_config(); - config.max_inflight_block_bytes = BS_PER_BLOCK_WORST_CASE_BYTES; + // One block's size hint of budget: size-based reservation now means one + // advertised body fills the budget, throttling to one in-flight request. + config.max_inflight_block_bytes = u64::from(block1_size); let (_tip_tx, tip_rx) = watch::channel((block::Height(2), blocks[1].hash())); let startup = BlockSyncStartup::new( @@ -3941,8 +4638,20 @@ async fn reactor_accepts_unmatched_body_for_queued_height() { ) .await; + // Queue height 1 with a deliberately oversized advertised hint so its send-time + // reservation (the estimate) exceeds the one-actual-block budget: no GetBlocks + // can issue, leaving the height queued without an outstanding request. The body, + // arriving unsolicited, reserves only its small actual size and is still + // buffered. Under the old worst-case reservation this gap was implicit; with + // size-based reservation the oversized hint recreates it. handle - .send(BlockSyncEvent::NeededBlocks(vec![block_meta(&blocks[0])])) + .send(BlockSyncEvent::NeededBlocks(vec![BlockSyncBlockMeta { + height: block::Height(1), + hash: blocks[0].hash(), + size: BlockSizeEstimate::Advertised( + u32::try_from(BS_PER_BLOCK_WORST_CASE_BYTES).expect("worst case fits u32"), + ), + }])) .await .expect("needed metadata queues"); @@ -4583,17 +5292,14 @@ async fn routine_disconnect_returns_outstanding_and_releases_budget() { } #[tokio::test] -async fn reactor_reserves_worst_case_per_block_not_size_hint() { - // Ports the deleted scheduler unit test - // `scheduler_reserves_worst_case_regardless_of_size_hints` to the post-WorkQueue - // issuance path (`fill_peer`): a request reserves `BS_PER_BLOCK_WORST_CASE_BYTES` - // per block — never the smaller advertised size hint. - // The global byte budget = 3 worst-case blocks, so the budget is the binding - // constraint. The peer advertises a generous slot/response/block-count budget and - // tiny 1 KiB size hints, so the only thing that can bound the request to 3 blocks - // is the worst-case-per-block reservation reaching the budget. If the reservation - // honored the 1 KiB hints, the full 16-block range would fit under the budget and - // the request would be 16 blocks. +async fn reactor_reserves_size_hint_per_block_not_worst_case() { + // The issuance path (`try_fill`) reserves the advertised size hint per block, + // NOT `BS_PER_BLOCK_WORST_CASE_BYTES`. The global byte budget = 3 worst-case + // blocks; the peer advertises a generous slot/response/block-count budget and + // tiny 1 KiB size hints. Under the old worst-case reservation the budget would + // bound the request to 3 blocks; because the reservation now honors the 1 KiB + // hints, the full 16-block range fits under the budget and the request is the + // whole 16-block range. let budget_blocks = 3u32; let config = ZakuraBlockSyncConfig { max_inflight_block_bytes: u64::from(budget_blocks) * BS_PER_BLOCK_WORST_CASE_BYTES, @@ -4652,10 +5358,10 @@ async fn reactor_reserves_worst_case_per_block_not_size_hint() { let (_start_height, count) = wait_for_outbound_getblocks(&mut outbound).await; assert_eq!( - count, budget_blocks, - "the request must be bounded to {budget_blocks} worst-case blocks by the global \ - byte budget, proving the reservation is worst-case-per-block and not the 1 KiB \ - advertised size hint", + count, 16, + "the request must cover the full 16-block range: the 1 KiB size hints (not a \ + worst-case-per-block reservation) drive the budget, so all 16 fit under a \ + {budget_blocks}-worst-case-block budget", ); reactor_task.abort(); @@ -4725,7 +5431,10 @@ async fn reactor_packs_small_estimates_under_peer_response_byte_cap() { } #[tokio::test] -async fn reactor_tiny_estimates_do_not_exceed_one_worst_case_budget_block() { +async fn reactor_tiny_estimates_pack_into_one_worst_case_budget_block() { + // A global budget of one worst-case block (2 MB) now holds many tiny-hint + // bodies, because each reserves its size hint (clamped to the 1 KiB floor), + // not a worst-case share. All 4 advertised heights fit in one request. let config = ZakuraBlockSyncConfig { max_inflight_block_bytes: BS_PER_BLOCK_WORST_CASE_BYTES, max_blocks_per_response: 4, @@ -4777,8 +5486,9 @@ async fn reactor_tiny_estimates_do_not_exceed_one_worst_case_budget_block() { let (start_height, count) = wait_for_outbound_getblocks(&mut outbound).await; assert_eq!(start_height, block::Height(1)); assert_eq!( - count, 1, - "only one worst-case block of global budget is available, regardless of tiny estimates", + count, 4, + "all 4 tiny-hint heights pack into one worst-case-block budget, because each \ + reserves its 1 KiB-floored size hint rather than a worst-case share", ); reactor_task.abort(); @@ -9496,7 +10206,7 @@ async fn reactor_publishes_block_sync_candidate_gap() { } #[tokio::test] -async fn reactor_reports_size_mismatch_softly_and_still_submits_valid_block() { +async fn oversize_body_policy_reports_size_mismatch_and_retries_without_buffering() { let mut config = ZakuraBlockSyncConfig { size_deviation_tolerance: 100, ..immediate_body_download_config() @@ -9580,25 +10290,32 @@ async fn reactor_reports_size_mismatch_softly_and_still_submits_valid_block() { .await .expect("block queues"); - let mut saw_size_mismatch = false; - let mut saw_submit = false; - while !(saw_size_mismatch && saw_submit) { + loop { match next_action(&mut actions).await { BlockSyncAction::Misbehavior { reason, .. } => { assert_eq!(reason, BlockSyncMisbehavior::SizeMismatch); - saw_size_mismatch = true; - } - BlockSyncAction::SubmitBlock { - block: submitted, .. - } => { - assert_eq!(submitted.hash(), block.hash()); - saw_submit = true; + break; } BlockSyncAction::QueryNeededBlocks { .. } => {} action => panic!("unexpected action during size mismatch test: {action:?}"), } } + let no_submit = tokio::time::timeout(Duration::from_millis(200), async { + while let Some(action) = actions.recv().await { + if matches!(action, BlockSyncAction::SubmitBlock { .. }) { + return false; + } + } + true + }) + .await + .unwrap_or(true); + assert!( + no_submit, + "oversize body is not submitted after SizeMismatch" + ); + reactor_task.abort(); } diff --git a/zebra-network/src/zakura/block_sync/work_queue.rs b/zebra-network/src/zakura/block_sync/work_queue.rs index 186228d996b..a76c72db242 100644 --- a/zebra-network/src/zakura/block_sync/work_queue.rs +++ b/zebra-network/src/zakura/block_sync/work_queue.rs @@ -19,19 +19,15 @@ //! splices held **never across `.await`** (the anti-block rule). `estimated_bytes` //! on a [`WorkItem`] is the block's size *estimate* (not its worst-case //! reservation); it exists only to carry the `SizeMismatch` tolerance check -//! through to the reactor's receive path. +//! through to the reactor's receive path and request budget. use std::sync::Mutex as StdMutex; use tokio::sync::Notify; use zebra_chain::block; -use super::request::BlockSizeEstimate; +use super::{request::BlockSizeEstimate, state::BlockBudgetLedger}; -/// EWMA seed used as the fallback body-size estimate when a height has no size -/// hint. Moved here from the old scheduler; the estimate only feeds the -/// `SizeMismatch` tolerance check, never the byte reservation. -pub(super) const DEFAULT_BS_EWMA_SEED_BYTES: u64 = 256 * 1024; /// Lower clamp on a body-size estimate. pub(super) const DEFAULT_BS_SIZE_FLOOR_BYTES: u64 = 1024; @@ -40,9 +36,17 @@ pub(super) const DEFAULT_BS_SIZE_FLOOR_BYTES: u64 = 1024; pub(super) struct WorkItem { /// Expected hash of the block at this height (drives the response match). pub(super) hash: block::Hash, - /// The block's *size estimate* (not its worst-case byte reservation). Only - /// used to feed the receive-path `SizeMismatch` tolerance check. + /// The block's size estimate. Used for request budget reservation and the + /// receive-path `SizeMismatch` tolerance check. pub(super) estimated_bytes: u64, + /// Current byte-budget charge owned by this height. + /// + /// Pending items normally have `Released`; issued-but-unreceived items have + /// `Reserved(estimate)`; received bodies held by the commit pipeline have + /// `Held(actual)`. All terminal release paths go through this ledger so a + /// stale local owner cannot release a charge that another path already + /// returned. + pub(super) budget: BlockBudgetLedger, } #[derive(Debug)] @@ -50,31 +54,24 @@ struct WorkQueueInner { pending: std::collections::BTreeMap, in_flight: std::collections::BTreeMap, floor: block::Height, - /// EWMA fallback estimate (overridable for tests). - ewma_fallback_bytes: u64, /// Floor clamp for size estimates (overridable for tests). floor_estimate_bytes: u64, } impl WorkQueueInner { fn estimate_bytes(&self, estimate: BlockSizeEstimate) -> u64 { - estimate_bytes_with( - estimate, - self.ewma_fallback_bytes, - self.floor_estimate_bytes, - ) + estimate_bytes_with(estimate, self.floor_estimate_bytes) } } /// Compute a clamped body-size estimate from a [`BlockSizeEstimate`] hint. /// -/// `Confirmed`/`Advertised` use the hinted size; `Unknown` uses the EWMA seed. -/// The result is clamped to `[floor, MAX_BLOCK_BYTES]`. Preserved verbatim from -/// the old scheduler's `estimate_bytes`. -fn estimate_bytes_with(estimate: BlockSizeEstimate, ewma: u64, floor: u64) -> u64 { +/// `Confirmed`/`Advertised` use the hinted size; `Unknown` reserves the +/// per-block worst case. The result is clamped to `[floor, MAX_BLOCK_BYTES]`. +fn estimate_bytes_with(estimate: BlockSizeEstimate, floor: u64) -> u64 { let hinted = match estimate { BlockSizeEstimate::Confirmed(size) | BlockSizeEstimate::Advertised(size) => u64::from(size), - BlockSizeEstimate::Unknown => ewma, + BlockSizeEstimate::Unknown => block::MAX_BLOCK_BYTES, }; hinted.max(floor).min(block::MAX_BLOCK_BYTES) } @@ -93,7 +90,6 @@ impl WorkQueue { pending: std::collections::BTreeMap::new(), in_flight: std::collections::BTreeMap::new(), floor, - ewma_fallback_bytes: DEFAULT_BS_EWMA_SEED_BYTES, floor_estimate_bytes: DEFAULT_BS_SIZE_FLOOR_BYTES, }), available: Notify::new(), @@ -101,9 +97,8 @@ impl WorkQueue { } #[cfg(test)] - pub(super) fn set_estimator_for_tests(&self, ewma: u64, floor: u64) { + pub(super) fn set_estimate_floor_for_tests(&self, floor: u64) { let mut inner = self.lock(); - inner.ewma_fallback_bytes = ewma.max(1); inner.floor_estimate_bytes = floor.max(1); } @@ -137,6 +132,7 @@ impl WorkQueue { WorkItem { hash, estimated_bytes, + budget: BlockBudgetLedger::Released, }, ); inserted += 1; @@ -194,8 +190,7 @@ impl WorkQueue { /// `low..=high` from `pending` to `in_flight`, also stopping before the /// sum of stored size estimates would exceed `max_estimated_bytes`. /// - /// The estimate cap is scheduler input hygiene only. Callers must still - /// reserve their existing worst-case bytes before sending the request. To + /// The estimate cap bounds the request's summed byte reservation. To /// guarantee progress, the first eligible item is always taken when /// `max_count > 0`, even if its estimate alone exceeds the cap. pub(super) fn take_in_range_budgeted( @@ -245,6 +240,7 @@ impl WorkQueue { /// Move each given height `in_flight → pending`, preserving its stored /// [`WorkItem`]. Heights not currently `in_flight` are skipped (idempotent). /// Wakes waiters if anything moved. + #[cfg(test)] pub(super) fn return_items(&self, heights: impl IntoIterator) { let mut moved = false; { @@ -278,25 +274,185 @@ impl WorkQueue { } } + /// Mark already-taken heights as owning an estimated byte reservation. + /// + /// Returns the sum marked. The caller must have already admitted the same + /// byte total through [`ByteBudget`](crate::zakura::transport::guard::ByteBudget). + pub(super) fn mark_reserved(&self, heights: impl IntoIterator) -> u64 { + let mut marked = 0u64; + let mut inner = self.lock(); + for height in heights { + let Some(item) = inner.in_flight.get_mut(&height) else { + continue; + }; + if item.budget.current_charge() != 0 { + continue; + } + item.budget = BlockBudgetLedger::reserved(item.estimated_bytes); + marked = marked.saturating_add(item.estimated_bytes); + } + marked + } + + /// Settle a body only if this height still owns an active request reservation. + /// + /// Returns `None` when a central watchdog or local timeout already released + /// and returned the height. Late bodies from that superseded claim must not + /// resurrect a second charge. + pub(super) fn settle_active_reserved_height( + &self, + height: block::Height, + actual: u64, + ) -> Option { + let mut inner = self.lock(); + let item = inner.in_flight.get_mut(&height)?; + item.budget + .is_reserved() + .then(|| item.budget.settle(actual)) + } + + /// Mark a height as directly held after the caller admitted `actual` bytes. + /// + /// Used for unmatched queued bodies, which did not have a prior request + /// estimate reservation. + pub(super) fn mark_held_direct(&self, height: block::Height, actual: u64) -> u64 { + let mut inner = self.lock(); + if let Some(item) = inner.in_flight.get_mut(&height) { + let previous_charge = item.budget.release(); + item.budget = BlockBudgetLedger::Held(actual); + return previous_charge; + } + if let Some(mut item) = inner.pending.remove(&height) { + let previous_charge = item.budget.release(); + item.budget = BlockBudgetLedger::Held(actual); + inner.in_flight.insert(height, item); + return previous_charge; + } + 0 + } + + /// Release any live charges for `heights`, exactly once. + pub(super) fn release_heights(&self, heights: impl IntoIterator) -> u64 { + let mut released = 0u64; + let mut inner = self.lock(); + for height in heights { + if let Some(item) = inner.in_flight.get_mut(&height) { + released = released.saturating_add(item.budget.release()); + } else if let Some(item) = inner.pending.get_mut(&height) { + released = released.saturating_add(item.budget.release()); + } + } + released + } + + /// Release and return `in_flight` heights to `pending`. + pub(super) fn release_and_return_items( + &self, + heights: impl IntoIterator, + ) -> u64 { + let mut moved = false; + let mut released = 0u64; + { + let mut inner = self.lock(); + for height in heights { + if let Some(mut item) = inner.in_flight.remove(&height) { + released = released.saturating_add(item.budget.release()); + inner.pending.insert(height, item); + moved = true; + } + } + } + if moved { + self.available.notify_waiters(); + } + released + } + + /// Release and return only still-reserved `in_flight` heights to `pending`. + /// + /// A height that has already settled to `Held(actual)` is owned by the body + /// handoff / Sequencer path. A central watchdog may clear stale peer claims, + /// but it must not release or requeue those bytes. + pub(super) fn release_reserved_and_return_items( + &self, + heights: impl IntoIterator, + ) -> u64 { + let mut moved = false; + let mut released = 0u64; + { + let mut inner = self.lock(); + for height in heights { + let Some(item) = inner.in_flight.get(&height) else { + continue; + }; + if !item.budget.is_reserved() { + continue; + } + let mut item = inner + .in_flight + .remove(&height) + .expect("reserved item exists because it was just checked"); + released = released.saturating_add(item.budget.release()); + inner.pending.insert(height, item); + moved = true; + } + } + if moved { + self.available.notify_waiters(); + } + released + } + /// Garbage-collect committed heights: raise the floor to `max(self.floor, - /// floor)` and drop every `pending`/`in_flight` entry `<= floor`. Does **not** throttle - /// how far above the floor peers may fetch. - pub(super) fn advance_floor(&self, floor: block::Height) { + /// floor)` and drop every `pending`/`in_flight` entry `<= floor`. + /// + /// Returns request-estimate bytes that were still reserved for unreceived + /// heights. Held body bytes are cleared from the ledger here but are not + /// returned: the Sequencer releases those actual body bytes when it drops + /// reorder/applying state. + pub(super) fn advance_floor(&self, floor: block::Height) -> u64 { let mut inner = self.lock(); inner.floor = inner.floor.max(floor); let floor = inner.floor; + let mut released = 0u64; + for item in inner.pending.range_mut(..=floor).map(|(_, item)| item) { + released = released.saturating_add(item.budget.release_reserved()); + } + for item in inner.in_flight.range_mut(..=floor).map(|(_, item)| item) { + released = released.saturating_add(item.budget.release_reserved()); + } inner.pending.retain(|height, _| *height > floor); inner.in_flight.retain(|height, _| *height > floor); + released } /// Frontier reset: pin the floor and drop every `pending`/`in_flight` entry /// `> floor` (their buffers were dropped; the producer re-fills via the next /// query). - pub(super) fn reset_above(&self, floor: block::Height) { + /// + /// Returns request-estimate bytes still reserved for unreceived heights, as + /// in [`advance_floor`](Self::advance_floor). + pub(super) fn reset_above(&self, floor: block::Height) -> u64 { let mut inner = self.lock(); inner.floor = floor; + let mut released = 0u64; + for item in inner + .pending + .range_mut((std::ops::Bound::Excluded(floor), std::ops::Bound::Unbounded)) + .map(|(_, item)| item) + { + released = released.saturating_add(item.budget.release_reserved()); + } + for item in inner + .in_flight + .range_mut((std::ops::Bound::Excluded(floor), std::ops::Bound::Unbounded)) + .map(|(_, item)| item) + { + released = released.saturating_add(item.budget.release_reserved()); + } inner.pending.retain(|height, _| *height <= floor); inner.in_flight.retain(|height, _| *height <= floor); + released } /// The "work added" notifier (per-peer routines wake source). @@ -315,6 +471,31 @@ impl WorkQueue { self.lock().in_flight.len() } + pub(super) fn reserved_above(&self, floor: block::Height) -> (u64, u64) { + let inner = self.lock(); + inner + .in_flight + .range((std::ops::Bound::Excluded(floor), std::ops::Bound::Unbounded)) + .fold((0u64, 0u64), |(bytes, count), (_, item)| { + let charge = item.budget.reserved_charge(); + if charge == 0 { + (bytes, count) + } else { + (bytes.saturating_add(charge), count.saturating_add(1)) + } + }) + } + + pub(super) fn reserved_bytes(&self) -> u64 { + let inner = self.lock(); + inner + .pending + .values() + .chain(inner.in_flight.values()) + .map(|item| item.budget.reserved_charge()) + .fold(0u64, u64::saturating_add) + } + /// Number of contiguous runs across `pending` (the old `queue_len` meaning: /// one queued range per maximal contiguous run of heights). pub(super) fn pending_run_count(&self) -> usize { @@ -336,6 +517,25 @@ impl WorkQueue { self.lock().pending.keys().next().copied() } + pub(super) fn min_in_flight(&self) -> Option { + self.lock().in_flight.keys().next().copied() + } + + pub(super) fn first_pending_in_range( + &self, + low: block::Height, + high: block::Height, + ) -> Option { + if low > high { + return None; + } + self.lock() + .pending + .range(low..=high) + .next() + .map(|(height, _)| *height) + } + pub(super) fn max_in_flight(&self) -> Option { self.lock().in_flight.keys().next_back().copied() } diff --git a/zebra-network/src/zakura/trace.rs b/zebra-network/src/zakura/trace.rs index 53da3a15f47..afd6904eb76 100644 --- a/zebra-network/src/zakura/trace.rs +++ b/zebra-network/src/zakura/trace.rs @@ -135,6 +135,8 @@ pub mod block_sync_trace { pub const APPLY_TOKEN: &str = "apply_token"; /// Bounded reason field. pub const REASON: &str = "reason"; + /// Scheduler/query lower bound for block body requests. + pub const REQUEST_FLOOR: &str = "request_floor"; /// Highest contiguous body height already submitted for apply. pub const BODY_DOWNLOAD_FLOOR: &str = "body_download_floor"; /// First height not yet in the contiguous body-download floor. diff --git a/zebra-network/src/zakura/transport/guard.rs b/zebra-network/src/zakura/transport/guard.rs index c267f0111b2..4f82ef42bf2 100644 --- a/zebra-network/src/zakura/transport/guard.rs +++ b/zebra-network/src/zakura/transport/guard.rs @@ -114,11 +114,53 @@ impl ByteBudget { self.inner.capacity.notify_waiters(); } - /// Shrink a reservation from `from` to `to` bytes, releasing the difference. - /// Used when a received body's worst-case reservation is replaced by its - /// actual (smaller) size; a no-op when `to >= from`. - pub(crate) fn shrink(&mut self, from: u64, to: u64) { - self.release(from.saturating_sub(to)); + /// Add `bytes` to the shared counter without applying the admission gate. + /// + /// Used when a body was already admitted based on an estimate and its actual + /// serialized size is larger than that estimate. The request cannot be + /// rejected at this point, so the budget must record the overshoot and let + /// later releases drain it. + pub(crate) fn charge(&mut self, bytes: u64) { + if bytes == 0 { + return; + } + self.inner + .reserved_bytes + .fetch_update(Ordering::AcqRel, Ordering::Acquire, |reserved| { + Some(reserved.saturating_add(bytes)) + }) + .ok(); + } + + /// Settle an estimated reservation to the actual bytes now held. + /// + /// If `actual` is smaller, this releases slack. If it is larger, this charges + /// the overshoot so held bodies are never under-counted. + #[cfg(test)] + pub(crate) fn settle(&mut self, reserved: u64, actual: u64) { + if actual > reserved { + self.charge(actual - reserved); + } else { + self.release(reserved - actual); + } + } + + /// Audit the shared counter against an externally-derived expected value. + /// + /// Returns `true` when the budget matches. + pub(crate) fn audit(&self, expected: u64, context: &'static str) -> bool { + let actual = self.reserved(); + let ok = actual == expected; + if !ok { + metrics::counter!("sync.block.budget.audit_drift").increment(1); + tracing::warn!( + actual, + expected, + context, + "Zakura block-sync byte-budget audit drift" + ); + } + ok } /// Subscribe to capacity-freed notifications. A consumer blocked on a full @@ -299,6 +341,23 @@ mod tests { assert_eq!(budget.reserved(), 0); } + #[test] + fn byte_budget_settles_estimates_to_actuals() { + let mut budget = ByteBudget::new(1_000); + assert!(budget.try_reserve(300)); + budget.settle(300, 200); + assert_eq!(budget.reserved(), 200); + + assert!(budget.try_reserve(300)); + budget.settle(300, 300); + assert_eq!(budget.reserved(), 500); + + assert!(budget.try_reserve(300)); + budget.settle(300, 450); + assert_eq!(budget.reserved(), 950); + assert_eq!(budget.available(), 50); + } + #[test] fn admit_rejects_disallowed_type() { let mut guard = SessionGuard::new(ALLOWED, 1_024, None); diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 5e01d6185e5..66415aa6857 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -1496,8 +1496,8 @@ pub enum ReadRequest { /// Returns scheduling-only body-size hints for a contiguous height range. /// - /// Only confirmed committed block sizes are returned. Unknown sizes are - /// returned as `None`. + /// Confirmed committed block sizes are preferred over advertised header + /// hints. Unknown sizes are returned as `None`. BlockSizeHints { /// First height to read. from: block::Height, From 8b47dd39cf5e15f65dfd8fd59becfc839e236619 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 26 Jun 2026 20:22:37 -0500 Subject: [PATCH 4/6] fix(network): tune block sync throughput defaults --- zebra-network/src/zakura/block_sync/config.rs | 2 +- zebra-network/src/zakura/handler.rs | 18 +- zebrad/tests/common/configs/v5.0.0-rc.4.toml | 168 ++++++++++++++++++ 3 files changed, 178 insertions(+), 10 deletions(-) create mode 100644 zebrad/tests/common/configs/v5.0.0-rc.4.toml diff --git a/zebra-network/src/zakura/block_sync/config.rs b/zebra-network/src/zakura/block_sync/config.rs index a947ccada19..dc594fa9468 100644 --- a/zebra-network/src/zakura/block_sync/config.rs +++ b/zebra-network/src/zakura/block_sync/config.rs @@ -31,7 +31,7 @@ pub const MAX_BS_INFLIGHT_REQUESTS: u32 = 32_768; /// Default total response byte target advertised per range response. pub const DEFAULT_BS_MAX_RESPONSE_BYTES: u32 = 32 * 1024 * 1024; /// Default global byte budget reserved for later block-download scheduling. -pub const DEFAULT_BS_MAX_INFLIGHT_BLOCK_BYTES: u64 = 2 * 1024 * 1024 * 1024; +pub const DEFAULT_BS_MAX_INFLIGHT_BLOCK_BYTES: u64 = 6 * 1024 * 1024 * 1024; /// Worst-case serialized bytes reserved per requested block body. /// /// Block-sync reserves this much per requested block at send time and only ever diff --git a/zebra-network/src/zakura/handler.rs b/zebra-network/src/zakura/handler.rs index 5e4b000cb4c..acb10793499 100644 --- a/zebra-network/src/zakura/handler.rs +++ b/zebra-network/src/zakura/handler.rs @@ -67,12 +67,12 @@ use crate::{ }; use crate::{BoxError, Config, MAX_TX_INV_IN_SENT_MESSAGE}; -/// Conservative default for total Zakura connections when P2P v2 is enabled. -pub const DEFAULT_ZAKURA_MAX_CONNECTIONS: usize = 32; -/// Conservative default for simultaneous Zakura control handshakes. -pub const DEFAULT_ZAKURA_MAX_PENDING_HANDSHAKES: usize = 8; -/// Conservative default for stream-open churn per connection. -pub const DEFAULT_ZAKURA_STREAM_OPEN_RATE_PER_SECOND: u32 = 16; +/// Default total Zakura connections when P2P v2 is enabled. +pub const DEFAULT_ZAKURA_MAX_CONNECTIONS: usize = 256; +/// Default simultaneous Zakura control handshakes. +pub const DEFAULT_ZAKURA_MAX_PENDING_HANDSHAKES: usize = 32; +/// Default stream-open churn per connection. +pub const DEFAULT_ZAKURA_STREAM_OPEN_RATE_PER_SECOND: u32 = 32; /// Per-kind inbound message rate per connection. /// /// This is a generous universal cap: block-sync legitimately delivers @@ -111,11 +111,11 @@ pub const DEFAULT_ZAKURA_KEEP_ALIVE_INTERVAL: Duration = Duration::from_secs(10) /// reclaim the slot without disturbing active transfers. pub const ZAKURA_DUPLICATE_EVICT_MIN_AGE: Duration = Duration::from_secs(300); /// QUIC stream receive window used by Zakura endpoints. -pub const DEFAULT_ZAKURA_STREAM_RECEIVE_WINDOW: u32 = 3 * 1024 * 1024; +pub const DEFAULT_ZAKURA_STREAM_RECEIVE_WINDOW: u32 = 32 * 1024 * 1024; /// QUIC connection receive window used by Zakura endpoints. -pub const DEFAULT_ZAKURA_RECEIVE_WINDOW: u32 = 16 * 1024 * 1024; +pub const DEFAULT_ZAKURA_RECEIVE_WINDOW: u32 = 32 * 1024 * 1024; /// QUIC send window used by Zakura endpoints. -pub const DEFAULT_ZAKURA_SEND_WINDOW: u64 = 16 * 1024 * 1024; +pub const DEFAULT_ZAKURA_SEND_WINDOW: u64 = 32 * 1024 * 1024; /// Initial backoff before re-dialing a configured Zakura bootstrap peer. pub const DEFAULT_ZAKURA_REDIAL_INITIAL_BACKOFF: Duration = Duration::from_secs(1); /// Maximum backoff between re-dials of a configured Zakura bootstrap peer. diff --git a/zebrad/tests/common/configs/v5.0.0-rc.4.toml b/zebrad/tests/common/configs/v5.0.0-rc.4.toml new file mode 100644 index 00000000000..5586c3af45d --- /dev/null +++ b/zebrad/tests/common/configs/v5.0.0-rc.4.toml @@ -0,0 +1,168 @@ +# Default configuration for zebrad. +# +# This file can be used as a skeleton for custom configs. +# +# Unspecified fields use default values. Optional fields are Some(field) if the +# field is present and None if it is absent. +# +# This file is generated as an example using zebrad's current defaults. +# You should set only the config options you want to keep, and delete the rest. +# Only a subset of fields are present in the skeleton, since optional values +# whose default is None are omitted. +# +# The config format (including a complete list of sections and fields) is +# documented here: +# https://docs.rs/zebrad/latest/zebrad/config/struct.ZebradConfig.html +# +# CONFIGURATION SOURCES (in order of precedence, highest to lowest): +# +# 1. Environment variables with ZEBRA_ prefix (highest precedence) +# - Format: ZEBRA_SECTION__KEY (double underscore for nested keys) +# - Examples: +# - ZEBRA_NETWORK__NETWORK=Testnet +# - ZEBRA_RPC__LISTEN_ADDR=127.0.0.1:8232 +# - ZEBRA_STATE__CACHE_DIR=/path/to/cache +# - ZEBRA_TRACING__FILTER=debug +# - ZEBRA_METRICS__ENDPOINT_ADDR=0.0.0.0:9999 +# +# 2. Configuration file (TOML format) +# - At the path specified via -c flag, e.g. `zebrad -c myconfig.toml start`, or +# - At the default path in the user's preference directory (platform-dependent, see below) +# +# 3. Hard-coded defaults (lowest precedence) +# +# The user's preference directory and the default path to the `zebrad` config are platform dependent, +# based on `dirs::preference_dir`, see https://docs.rs/dirs/latest/dirs/fn.preference_dir.html : +# +# | Platform | Value | Example | +# | -------- | ------------------------------------- | ---------------------------------------------- | +# | Linux | `$XDG_CONFIG_HOME` or `$HOME/.config` | `/home/alice/.config/zebrad.toml` | +# | macOS | `$HOME/Library/Preferences` | `/Users/Alice/Library/Preferences/zebrad.toml` | +# | Windows | `{FOLDERID_RoamingAppData}` | `C:\Users\Alice\AppData\Local\zebrad.toml` | + +[consensus] +checkpoint_sync = true + +[health] +enforce_on_test_networks = false +min_connected_peers = 1 +ready_max_blocks_behind = 2 +ready_max_tip_age = "5m" + +[mempool] +eviction_memory_time = "1h" +max_datacarrier_bytes = 83 +tx_cost_limit = 80000000 + +[metrics] + +[mining] +internal_miner = false + +[network] +cache_dir = true +crawl_new_peer_interval = "1m 1s" +initial_mainnet_peers = [ + "dnsseed.str4d.xyz:8233", + "dnsseed.z.cash:8233", + "mainnet.seeder.shieldedinfra.net:8233", + "mainnet.seeder.zfnd.org:8233", +] +initial_testnet_peers = [ + "dnsseed.testnet.z.cash:18233", + "testnet.seeder.zfnd.org:18233", +] +legacy_p2p = true +listen_addr = "[::]:8233" +max_connections_per_ip = 1 +network = "Mainnet" +peerset_initial_target_size = 100 +v2_p2p = true + +[network.zakura] +bootstrap_peers = [] +listen_addr = "0.0.0.0:8234" +max_connections = 256 +max_pending_handshakes = 32 +message_rate_per_second = 2048 +stream_open_rate_per_second = 32 + +[network.zakura.block_sync] +fanout = 1 +floor_peer_avoid_cooldown = "8s" +floor_watchdog_tick = "1s" +initial_inflight_requests = 64 +max_blocks_per_response = 1 +max_inflight_block_bytes = 6442450944 +max_inflight_requests = 32000 +max_reorder_lookahead_blocks = 4096 +max_reorder_lookahead_bytes = 6408896512 +max_response_bytes = 33554432 +max_submitted_block_applies = 400 +request_timeout = "8s" +size_deviation_tolerance = 200 +status_refresh_interval = "30s" + +[network.zakura.block_sync.peer_limits] +inbound_queue_depth = 128 +max_inbound_peers = 256 +max_outbound_peers = 256 +max_pending_escalations = 32 +outbound_queue_depth = 128 + +[network.zakura.header_sync] +accept_new_blocks = true +max_headers_per_response = 1000 +max_inflight_requests = 10 +status_refresh_interval = "30s" + +[network.zakura.header_sync.peer_limits] +inbound_queue_depth = 128 +max_inbound_peers = 256 +max_outbound_peers = 256 +max_pending_escalations = 32 +outbound_queue_depth = 128 + +[rpc] +cookie_dir = "cache_dir" +cookie_file_name = ".cookie" +debug_force_finished_sync = false +enable_cookie_auth = true +max_response_body_size = 52428800 +parallel_cpu_threads = 0 + +[state] +cache_dir = "cache_dir" +debug_skip_non_finalized_state_backup_task = false +delete_old_database = true +ephemeral = false +should_backup_non_finalized_state = true +storage_mode = "archive" + +[sync] +checkpoint_verify_concurrency_limit = 1000 +download_concurrency_limit = 100 +full_verify_concurrency_limit = 20 +parallel_cpu_threads = 0 +zakura_block_apply_concurrency_limit = 32 + +[tracing] +buffer_limit = 128000 +force_use_color = false +use_color = true +use_journald = false + +[zcashd_compat] +cookie_dir = "cache_dir" +cookie_file_name = ".zcashd-compat.cookie" +enable_cookie_auth = true +enabled = false +manage_zcashd = false +restart_backoff = "2s" +restart_backoff_max = "5m" +restart_reset_after = "1h" +shutdown_grace_period = "5m" +startup_delay = "1s" +unsafe_allow_remote_http = false +zcashd_extra_args = [] +zcashd_source = "path" From cc9ea76d9f17d419cc13e1b6287b51cdb0769cf9 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Sat, 27 Jun 2026 11:47:45 -0500 Subject: [PATCH 5/6] fix(zebrad): restore disable_vct_fast_sync in v5.0.0-rc.4.toml golden config The throughput-defaults rc.4 golden config dropped the [consensus] disable_vct_fast_sync field, so the generated default config no longer matched any stored config and last_config_is_stored (config_tests) failed once the build break was fixed and the test could run. --- zebrad/tests/common/configs/v5.0.0-rc.4.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/zebrad/tests/common/configs/v5.0.0-rc.4.toml b/zebrad/tests/common/configs/v5.0.0-rc.4.toml index 5586c3af45d..fabd8e9bae5 100644 --- a/zebrad/tests/common/configs/v5.0.0-rc.4.toml +++ b/zebrad/tests/common/configs/v5.0.0-rc.4.toml @@ -42,6 +42,7 @@ [consensus] checkpoint_sync = true +disable_vct_fast_sync = false [health] enforce_on_test_networks = false From 7db61d72a5b65c54bdfc29a10b258eca6b1504c1 Mon Sep 17 00:00:00 2001 From: roman Date: Sat, 27 Jun 2026 14:21:59 -0600 Subject: [PATCH 6/6] merge conflicts --- zebra-network/src/zakura/block_sync/config.rs | 3 +++ .../src/zakura/block_sync/peer_routine.rs | 14 +++++++------- zebra-network/src/zakura/block_sync/tests.rs | 6 ++++++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/zebra-network/src/zakura/block_sync/config.rs b/zebra-network/src/zakura/block_sync/config.rs index dc594fa9468..92674d11f4c 100644 --- a/zebra-network/src/zakura/block_sync/config.rs +++ b/zebra-network/src/zakura/block_sync/config.rs @@ -266,6 +266,9 @@ impl ZakuraBlockSyncConfig { if self.max_inflight_block_bytes == 0 { return Err("max_inflight_block_bytes must be greater than zero"); } + if self.max_reorder_lookahead_bytes == 0 { + return Err("max_reorder_lookahead_bytes must be greater than zero"); + } if self.max_reorder_lookahead_blocks == 0 { return Err("max_reorder_lookahead_blocks must be greater than zero"); } diff --git a/zebra-network/src/zakura/block_sync/peer_routine.rs b/zebra-network/src/zakura/block_sync/peer_routine.rs index 63dd8841d8f..f4ec415eec7 100644 --- a/zebra-network/src/zakura/block_sync/peer_routine.rs +++ b/zebra-network/src/zakura/block_sync/peer_routine.rs @@ -1186,17 +1186,17 @@ impl PeerRoutine { self.record_received(serialized_bytes); self.trace_body_received(height, serialized_bytes, None, None, None); - let view = *self.sequencer_view.borrow(); + let sequencer_view = *self.sequencer_view.borrow(); let (reserved_above_floor_bytes, reserved_above_floor_blocks) = - self.work.reserved_above(view.download_floor); + self.work.reserved_above(sequencer_view.download_floor); let Some(decision) = admission_decision( &self.config, AdmissionSnapshot { - download_floor: view.download_floor, - reorder_buffered_bytes: view.reorder_buffered_bytes, - reorder_buffered_blocks: view.reorder_len, - applying_buffered_bytes: view.applying_buffered_bytes, - applying_buffered_blocks: view.applying_len, + download_floor: sequencer_view.download_floor, + reorder_buffered_bytes: sequencer_view.reorder_buffered_bytes, + reorder_buffered_blocks: sequencer_view.reorder_len, + applying_buffered_bytes: sequencer_view.applying_buffered_bytes, + applying_buffered_blocks: sequencer_view.applying_len, sequencer_input_queued_bytes: self .sequencer_input_bytes .load(std::sync::atomic::Ordering::Relaxed), diff --git a/zebra-network/src/zakura/block_sync/tests.rs b/zebra-network/src/zakura/block_sync/tests.rs index a8670a224ea..30010a56642 100644 --- a/zebra-network/src/zakura/block_sync/tests.rs +++ b/zebra-network/src/zakura/block_sync/tests.rs @@ -756,6 +756,12 @@ fn config_validate_rejects_degenerate_values() { }; assert!(config.validate().is_err()); + config = ZakuraBlockSyncConfig { + max_reorder_lookahead_bytes: 0, + ..ZakuraBlockSyncConfig::default() + }; + assert!(config.validate().is_err()); + config = ZakuraBlockSyncConfig { max_reorder_lookahead_blocks: 0, ..ZakuraBlockSyncConfig::default()