From 2df95edeccfb9676d1badf1cbd9e0c905386d7f4 Mon Sep 17 00:00:00 2001 From: frisitano Date: Tue, 31 Mar 2026 01:14:18 +0200 Subject: [PATCH 1/3] feat: network sync refactor - min req message size --- beacon_node/lighthouse_network/src/config.rs | 7 - .../lighthouse_network/src/rpc/codec.rs | 6 +- .../lighthouse_network/src/rpc/methods.rs | 82 ++++- .../lighthouse_network/src/rpc/protocol.rs | 3 +- .../network_beacon_processor/rpc_methods.rs | 23 +- beacon_node/network/src/sync/manager.rs | 13 +- .../network/src/sync/network_context.rs | 53 +++- beacon_node/network/src/sync/proof_sync.rs | 286 +++++++++++------- beacon_node/network/src/sync/tests/lookups.rs | 4 +- beacon_node/network/src/sync/tests/range.rs | 280 ++++++++++++----- testing/proof_engine/src/rig.rs | 7 - 11 files changed, 541 insertions(+), 223 deletions(-) diff --git a/beacon_node/lighthouse_network/src/config.rs b/beacon_node/lighthouse_network/src/config.rs index 58b683f2e80..bc0b2ae3669 100644 --- a/beacon_node/lighthouse_network/src/config.rs +++ b/beacon_node/lighthouse_network/src/config.rs @@ -22,7 +22,6 @@ pub const DEFAULT_TCP_PORT: u16 = 9000u16; pub const DEFAULT_DISC_PORT: u16 = 9000u16; pub const DEFAULT_QUIC_PORT: u16 = 9001u16; pub const DEFAULT_IDONTWANT_MESSAGE_SIZE_THRESHOLD: usize = 1000usize; -pub const DEFAULT_PROOF_SYNC_ACTIVATION_SLOTS: u64 = 10; pub struct GossipsubConfigParams { pub message_domain_valid_snappy: [u8; 4], @@ -133,11 +132,6 @@ pub struct Config { /// Proof types supported by this client. pub proof_types: Option>, - /// Number of slot ticks to wait after range sync completes before issuing - /// `ExecutionProofsByRange` requests. Gives the beacon processor time to finish - /// calling `notify_new_payload` for all imported blocks before proofs are requested. - pub proof_sync_activation_slots: u64, - /// Configuration for the outbound rate limiter (requests made by this node). pub outbound_rate_limiter_config: Option, @@ -374,7 +368,6 @@ impl Default for Config { enable_light_client_server: true, enable_execution_proof: false, proof_types: None, - proof_sync_activation_slots: DEFAULT_PROOF_SYNC_ACTIVATION_SLOTS, outbound_rate_limiter_config: None, invalid_block_storage: None, inbound_rate_limiter_config: None, diff --git a/beacon_node/lighthouse_network/src/rpc/codec.rs b/beacon_node/lighthouse_network/src/rpc/codec.rs index c577cee648c..a7c84556836 100644 --- a/beacon_node/lighthouse_network/src/rpc/codec.rs +++ b/beacon_node/lighthouse_network/src/rpc/codec.rs @@ -595,8 +595,12 @@ fn handle_rpc_request( ))) } SupportedProtocol::ExecutionProofsByRangeV1 => { + let max_filters = spec.max_request_blocks(current_fork); Ok(Some(RequestType::ExecutionProofsByRange( - ExecutionProofsByRangeRequest::from_ssz_bytes(decoded_buffer)?, + ExecutionProofsByRangeRequest::from_ssz_bytes_with_max( + decoded_buffer, + max_filters, + )?, ))) } SupportedProtocol::ExecutionProofsByRootV1 => Ok(Some(RequestType::ExecutionProofsByRoot( diff --git a/beacon_node/lighthouse_network/src/rpc/methods.rs b/beacon_node/lighthouse_network/src/rpc/methods.rs index d37330cdc65..b1b72b11def 100644 --- a/beacon_node/lighthouse_network/src/rpc/methods.rs +++ b/beacon_node/lighthouse_network/src/rpc/methods.rs @@ -585,12 +585,22 @@ pub struct ExecutionProofStatus { } /// Request execution proofs for a slot range from a peer. -#[derive(Encode, Decode, Clone, Debug, PartialEq)] +/// +/// `proof_filters` is an optional per-block filter that tells the peer which proof types we still +/// need for specific blocks in the range. Blocks not listed in `proof_filters` will have all known +/// proof types returned; blocks listed will only have the specified types returned. This avoids +/// transferring proof types the requester already holds. +/// +/// Matches the `ExecutionProofsByRange` request type in the EIP-8025 p2p spec. +#[derive(Clone, Debug, PartialEq)] pub struct ExecutionProofsByRangeRequest { /// The starting slot to request execution proofs. pub start_slot: u64, /// The number of slots from the start slot. pub count: u64, + /// Per-block proof-type filters for blocks in the range where only some proof types are needed. + /// Empty list means "return all proof types for every block in the range." + pub proof_filters: RuntimeVariableList, } impl ExecutionProofsByRangeRequest { @@ -601,17 +611,65 @@ impl ExecutionProofsByRangeRequest { .saturating_mul(MaxExecutionProofsPerPayload::to_u64()) } + /// Minimum SSZ encoded byte length: the two fixed `u64` fields plus the 4-byte offset pointer + /// for the variable-length `proof_filters` list. pub fn ssz_min_len() -> usize { - ExecutionProofsByRangeRequest { - start_slot: 0, - count: 0, - } - .as_ssz_bytes() - .len() + // start_slot (8) + count (8) + proof_filters offset (4) + 20 } - pub fn ssz_max_len() -> usize { - Self::ssz_min_len() + /// Maximum SSZ encoded byte length when `proof_filters` holds up to `max_request_blocks` + /// entries. + /// + /// Each `ProofByRootIdentifier` is a variable-length SSZ container: + /// - `block_root`: 32 bytes (fixed) + /// - `proof_types` offset field: 4 bytes (within the container fixed portion) + /// - `proof_types` content: at most `MAX_EXECUTION_PROOFS_PER_PAYLOAD` × 1 byte = 4 bytes + /// + /// A `List` of `max_request_blocks` variable-length items also requires a 4-byte offset table + /// entry per item. + pub fn ssz_max_len(max_request_blocks: usize) -> usize { + const MAX_PROOF_BY_ROOT_IDENTIFIER_BYTES: usize = 32 + 4 + 4; + Self::ssz_min_len() + max_request_blocks * (4 + MAX_PROOF_BY_ROOT_IDENTIFIER_BYTES) + } + + /// Decode from SSZ bytes, supplying a runtime maximum for the `proof_filters` list length. + pub fn from_ssz_bytes_with_max( + bytes: &[u8], + max_filters: usize, + ) -> Result { + let mut builder = ssz::SszDecoderBuilder::new(bytes); + builder.register_type::()?; + builder.register_type::()?; + builder.register_anonymous_variable_length_item()?; + let mut decoder = builder.build()?; + Ok(Self { + start_slot: decoder.decode_next::()?, + count: decoder.decode_next::()?, + proof_filters: decoder.decode_next_with(|slice| { + RuntimeVariableList::from_ssz_bytes(slice, max_filters) + })?, + }) + } +} + +impl ssz::Encode for ExecutionProofsByRangeRequest { + fn is_ssz_fixed_len() -> bool { + false + } + + fn ssz_append(&self, buf: &mut Vec) { + // Fixed portion: start_slot (8) + count (8) + proof_filters offset (4) = 20 bytes. + let num_fixed_bytes = 8 + 8 + ssz::BYTES_PER_LENGTH_OFFSET; + let mut encoder = ssz::SszEncoder::container(buf, num_fixed_bytes); + encoder.append(&self.start_slot); + encoder.append(&self.count); + encoder.append(&self.proof_filters); + encoder.finalize(); + } + + fn ssz_bytes_len(&self) -> usize { + 8 + 8 + ssz::BYTES_PER_LENGTH_OFFSET + self.proof_filters.ssz_bytes_len() } } @@ -619,8 +677,10 @@ impl std::fmt::Display for ExecutionProofsByRangeRequest { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "Request: ExecutionProofsByRange: Start Slot: {}, Count: {}", - self.start_slot, self.count + "Request: ExecutionProofsByRange: Start Slot: {}, Count: {}, Filters: {}", + self.start_slot, + self.count, + self.proof_filters.len() ) } } diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index f4d367a26b3..9b730b3b1b2 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -586,9 +586,10 @@ impl ProtocolId { LightClientUpdatesByRangeRequest::ssz_max_len(), ), Protocol::MetaData => RpcLimits::new(0, 0), // Metadata requests are empty + // EIP-8025 is Fulu+; use the Deneb/post-Deneb max_request_blocks for proof_filters. Protocol::ExecutionProofsByRange => RpcLimits::new( ExecutionProofsByRangeRequest::ssz_min_len(), - ExecutionProofsByRangeRequest::ssz_max_len(), + ExecutionProofsByRangeRequest::ssz_max_len(spec.max_request_blocks(ForkName::Fulu)), ), // ExecutionProofsByRoot request is List[ProofByRootIdentifier, MAX_BLOCKS_BY_ROOT. Protocol::ExecutionProofsByRoot => RpcLimits::new(0, spec.max_blocks_by_root_request), diff --git a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs index a8919942a16..0fe3b86faf8 100644 --- a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs @@ -1326,7 +1326,10 @@ impl NetworkBeaconProcessor { /// Handle an `ExecutionProofsByRange` request from the peer (EIP-8025). /// - /// Streams all `SignedExecutionProof` objects known for the requested slot range. + /// Streams `SignedExecutionProof` objects known for the requested slot range, filtered by + /// `proof_filters` when present. If `proof_filters` is non-empty, blocks listed in it are + /// served only for the proof types specified; blocks absent from `proof_filters` receive all + /// known proof types. This mirrors the `ExecutionProofsByRoot` semantics. pub fn handle_execution_proofs_by_range_request( &self, peer_id: PeerId, @@ -1351,9 +1354,18 @@ impl NetworkBeaconProcessor { %peer_id, start_slot = req.start_slot, count = req.count, + num_filters = req.proof_filters.len(), "Received ExecutionProofsByRange Request" ); + // Build a lookup map: block_root → requested proof types from proof_filters. + // Blocks not listed in proof_filters will have all known proof types served. + let filter_map: std::collections::HashMap<_, _> = req + .proof_filters + .iter() + .map(|id| (id.block_root, &id.proof_types)) + .collect(); + let block_roots = self.get_block_roots_for_slot_range( req.start_slot, req.count, @@ -1362,7 +1374,16 @@ impl NetworkBeaconProcessor { let mut proofs_sent = 0usize; for block_root in block_roots { + let allowed_types = filter_map.get(&block_root); for proof in self.chain.get_execution_proofs_by_block_root(block_root) { + // If this block has a filter entry, skip proof types not requested. + // An absent entry means "return all types" (same as an empty proof_types list). + if let Some(types) = allowed_types + && !types.is_empty() + && !types.contains(&proof.message.proof_type) + { + continue; + } self.send_network_message(NetworkMessage::SendResponse { peer_id, inbound_request_id, diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 79c714255b2..21fe1cc996c 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -353,10 +353,7 @@ impl SyncManager { notified_unknown_roots: LRUTimeCache::new(Duration::from_secs( NOTIFIED_UNKNOWN_ROOT_EXPIRY_SECONDS, )), - proof_sync: ProofSync::new( - beacon_chain.clone(), - network_globals.config.proof_sync_activation_slots, - ), + proof_sync: ProofSync::new(beacon_chain.clone()), } } @@ -424,14 +421,6 @@ impl SyncManager { #[cfg(test)] pub(crate) fn start_proof_sync(&mut self) { self.proof_sync.start(&mut self.network); - // Advance through the Waiting countdown so callers immediately see Syncing state, - // matching pre-Waiting behaviour in unit tests. - while matches!( - self.proof_sync.state(), - super::proof_sync::ProofSyncState::Waiting(_) - ) { - self.proof_sync.poll(&mut self.network); - } } fn network_globals(&self) -> &NetworkGlobals { diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 71d3ad846d3..e0ebcef73e5 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -48,7 +48,7 @@ use requests::{ }; #[cfg(test)] use slot_clock::SlotClock; -use ssz_types::VariableList; +use ssz_types::{RuntimeVariableList, VariableList}; use std::collections::hash_map::Entry; use std::collections::{HashMap, HashSet}; use std::fmt::Debug; @@ -417,17 +417,60 @@ impl SyncNetworkContext { /// Send a `ExecutionProofsByRange` request to the given proof-capable peer. /// + /// `partial_missing` contains `MissingProofInfo` entries for blocks within the requested slot + /// range that already have *some* proof types. An entry is included in the request's + /// `proof_filters` only when the block has partial coverage (i.e. `existing_proof_types` is + /// non-empty), so the peer knows to return only the missing types. Blocks with no existing + /// proofs are not filtered — the peer will return all proof types for them. + /// /// Callers should use `find_best_proof_capable_peer` to select the peer first. pub fn request_execution_proofs_by_range( &mut self, peer_id: PeerId, start_slot: Slot, count: u64, + partial_missing: &[MissingProofInfo], ) -> Result { let id = ExecutionProofsByRangeRequestId { id: self.next_id() }; + + // Build proof_filters: one entry per block in `partial_missing` that still needs a subset + // of proof types. Blocks already fully proven are excluded (wouldn't appear in + // partial_missing); blocks with no proofs at all are also excluded (peer returns all types + // by default for blocks absent from proof_filters). + let max_request_blocks = self + .chain + .spec + .max_request_blocks(self.fork_context.current_fork_name()); + let mut filter_items: Vec = Vec::new(); + for info in partial_missing { + if info.existing_proof_types.is_empty() { + continue; + } + let needed: Vec = self + .proof_types + .iter() + .map(|t| t.to_u8()) + .filter(|t| !info.existing_proof_types.contains(t)) + .collect(); + if needed.is_empty() { + continue; + } + let proof_types = VariableList::new(needed) + .map_err(|e| RpcRequestSendError::InternalError(format!("proof_types: {e:?}")))?; + filter_items.push(ProofByRootIdentifier { + block_root: info.root, + proof_types, + }); + } + let proof_filters = + RuntimeVariableList::new(filter_items, max_request_blocks).map_err(|e| { + RpcRequestSendError::InternalError(format!("proof_filters too long: {e:?}")) + })?; + let request = ExecutionProofsByRangeRequest { start_slot: start_slot.as_u64(), count, + proof_filters, }; self.network_send .send(NetworkMessage::SendRequest { @@ -544,6 +587,14 @@ impl SyncNetworkContext { self.network_globals().local_execution_proof_status() } + /// Number of proof types this node is configured to request. + /// + /// Used by [`ProofSync`] to compute request byte sizes without needing access to the + /// full `ProofTypes` set. + pub fn configured_proof_types_count(&self) -> usize { + self.proof_types.len() + } + /// Returns `true` if the peer has `execution_proof_enabled()` in their ENR. pub fn is_proof_capable_peer(&self, peer_id: &PeerId) -> bool { self.network_globals() diff --git a/beacon_node/network/src/sync/proof_sync.rs b/beacon_node/network/src/sync/proof_sync.rs index cb6eda4ecc3..78d184aed08 100644 --- a/beacon_node/network/src/sync/proof_sync.rs +++ b/beacon_node/network/src/sync/proof_sync.rs @@ -2,8 +2,18 @@ //! //! Defines [`ProofSync`], the subsystem responsible for requesting execution proofs //! that are missing from the local proof engine after block sync completes. It manages -//! peer status tracking, decides between bulk range requests and targeted by-root -//! requests, and coordinates the cooldown period between request batches. +//! peer status tracking and, at each slot tick, consults the proof engine directly to +//! decide the most bandwidth-efficient request strategy: +//! +//! - The SSZ-encoded sizes of an `ExecutionProofsByRange` request (20-byte fixed header +//! plus `proof_filters` for partially-held blocks) and an `ExecutionProofsByRoot` request +//! (one identifier per missing block) are compared over the full set of servable missing +//! proofs. Whichever encoding is smaller is used. +//! - `proof_filters` lets the server skip proof types the requester already holds, so +//! partially-covered blocks do not waste bandwidth even in a range request. +//! +//! The protocol is driven entirely by what the proof engine reports as missing, not by +//! the distance between peer and local verified heads. use super::network_context::{CachedExecutionProofStatus, SyncNetworkContext}; use beacon_chain::{BeaconChain, BeaconChainTypes, WhenSlotSkipped}; @@ -17,11 +27,7 @@ use std::collections::HashMap; use std::sync::Arc; use std::time::Instant; use tracing::{debug, info}; -use types::{EthSpec, Hash256, Slot}; - -/// Default slot gap above which a bulk `ExecutionProofsByRange` request is preferred over -/// individual `ExecutionProofsByRoot` requests. -const DEFAULT_RANGE_REQUEST_THRESHOLD: u64 = 16; +use types::{Hash256, Slot}; /// Tracks the single in-flight `ExecutionProofsByRange` request. /// @@ -43,23 +49,24 @@ pub(crate) struct ByRootRequest { pub enum ProofSyncState { /// Range sync is active; proof sync is paused. Idle, - /// Waiting for the beacon processor to finish importing range sync blocks. - /// The inner value counts down remaining slot ticks before activation. - Waiting(u64), - /// Proof sync is active. Each poll chooses between a range request (large slot gap) - /// or by-root fill requests (small gap) based on current chain state. + /// Proof sync is active. Each poll queries the proof engine for missing proofs and + /// chooses between range or by-root requests based on byte-efficiency. Syncing, } +/// Number of slot ticks to skip after a proof response stream completes before issuing +/// the next request. Gives the beacon processor time to import received proofs so they +/// no longer appear in `missing_execution_proofs()`. +const POST_REQUEST_COOLDOWN_SLOTS: u64 = 1; + /// Proof sync subsystem for EIP-8025. /// -/// Operates as a three-state machine: `Idle` while range sync is active, `Waiting(n)` -/// after range sync completes (counting down n slot ticks to let the beacon processor -/// finish importing blocks), and `Syncing` once active. In `Syncing`, each poll computes -/// the slot gap between the max(finalized epoch, local verified head) and peer verified -/// head to determine the most efficient request strategy. In-flight by-root and range -/// responses are always processed regardless of state transitions — the proofs are valid -/// independent of sync progress. +/// Operates as a two-state machine: `Idle` while range sync is active, `Syncing` once +/// activated. In `Syncing`, each poll queries the proof engine for missing proofs and +/// chooses the most byte-efficient request strategy (range vs by-root). A brief +/// `post_request_cooldown` counter prevents immediate re-requesting after a response +/// stream completes, giving the beacon processor time to import the received proofs. +/// In-flight responses are always processed regardless of state transitions. pub struct ProofSync { chain: Arc>, state: ProofSyncState, @@ -67,15 +74,14 @@ pub struct ProofSync { range_request: Option, /// Tracks the single in-flight `ExecutionProofsByRoot` batch request (ID + serving peer). root_request: Option, - /// Slot gap above which a `ByRange` request is preferred over `ByRoot` fill requests. - range_request_threshold: u64, + /// Slot ticks remaining before the next request may be issued after a response stream + /// completes. Set to `POST_REQUEST_COOLDOWN_SLOTS` on termination, decremented each + /// poll, and blocks new requests until it reaches zero. + post_request_cooldown: u64, /// Cached `ExecutionProofStatus` responses, keyed by peer. peer_statuses: HashMap, /// In-flight `ExecutionProofStatus` request IDs, keyed by peer. status_in_flight: HashMap, - /// Number of slot ticks to wait after `start()` or a range response before issuing - /// the next `ExecutionProofsByRange` request. - activation_slots: u64, /// Suppresses repeated "no proof-capable peer" logs: set when the message is first /// emitted, cleared when a peer becomes available. logged_no_peer: bool, @@ -86,19 +92,15 @@ pub struct ProofSync { impl ProofSync { /// Creates a new `ProofSync` instance in the `Idle` state. - /// - /// `activation_slots` controls how many slot ticks to wait after `start()` or a - /// completed range response before issuing the next request batch. - pub fn new(chain: Arc>, activation_slots: u64) -> Self { + pub fn new(chain: Arc>) -> Self { Self { state: ProofSyncState::Idle, range_request: None, root_request: None, chain, - range_request_threshold: DEFAULT_RANGE_REQUEST_THRESHOLD, + post_request_cooldown: 0, peer_statuses: HashMap::default(), status_in_flight: HashMap::default(), - activation_slots, logged_no_peer: false, #[cfg(test)] test_missing_proofs: None, @@ -121,11 +123,6 @@ impl ProofSync { self.state = state; } - #[cfg(test)] - pub fn set_range_request_threshold(&mut self, threshold: u64) { - self.range_request_threshold = threshold; - } - #[cfg(test)] pub fn by_range_request(&self) -> Option<&ByRangeRequest> { self.range_request.as_ref() @@ -138,16 +135,14 @@ impl ProofSync { /// Called by `SyncManager` when range sync completes. /// - /// Kicks off peer status refreshes and transitions to `Waiting`, which counts down - /// slot ticks before activating. This delay allows the beacon processor to finish - /// importing range sync blocks before proof requests go out. + /// Kicks off peer status refreshes and transitions directly to `Syncing`. The proof + /// engine is the authoritative source of missing proofs — it only reports entries after + /// blocks are imported, so no artificial delay is needed before the first poll. pub fn start(&mut self, cx: &mut SyncNetworkContext) { - info!( - activation_slots = self.activation_slots, - "ProofSync: starting, waiting before activation" - ); + info!("ProofSync: starting"); + self.post_request_cooldown = 0; self.refresh_peer_statuses(cx); - self.state = ProofSyncState::Waiting(self.activation_slots); + self.state = ProofSyncState::Syncing; } /// Called by `SyncManager` when range sync re-enters. @@ -161,70 +156,36 @@ impl ProofSync { /// Drive one polling cycle. /// - /// In `Waiting`, counts down the activation delay. In `Syncing`, computes the slot - /// gap and dispatches either a range request (gap > `range_request_threshold`) or - /// by-root fill requests (gap ≤ threshold). Does nothing if a range request is - /// already in-flight. Peer status refreshes run in the background and do not block - /// request dispatch. + /// In `Syncing`, consults the proof engine for missing proofs and decides the most + /// request-efficient strategy: + /// + /// - Finds the consecutive run of missing slots with the highest byte savings over + /// an equivalent `ExecutionProofsByRoot` request. + /// - If a run's savings are positive it sends `ExecutionProofsByRange` for that run + /// (with `proof_filters` covering partially-held blocks so the peer skips redundant + /// proof types). + /// - Otherwise sends a single `ExecutionProofsByRoot` batch for all servable missing + /// proofs. + /// + /// Does nothing if a range request is already in-flight or a post-request cooldown is + /// active. Peer status refreshes run in the background and do not block request dispatch. pub fn poll(&mut self, cx: &mut SyncNetworkContext) { - match self.state { - ProofSyncState::Idle => return, - ProofSyncState::Waiting(0) => { - info!("ProofSync: activation delay elapsed, transitioning to Syncing"); - self.state = ProofSyncState::Syncing; - } - ProofSyncState::Waiting(ref mut n) => { - *n -= 1; - return; - } - ProofSyncState::Syncing => {} - } - - // If a range request is already in-flight, wait for it to drain. - if self.range_request.is_some() { + if self.state == ProofSyncState::Idle { return; } - // Compute the start slot: the higher of the finalized slot and our own verified proof slot, - // so we don't re-request proofs we've already processed. - let finalized_slot = self - .chain - .canonical_head - .cached_head() - .finalized_checkpoint() - .epoch - .start_slot(T::EthSpec::slots_per_epoch()); - let local_proof_slot = Slot::new(cx.local_execution_proof_status().slot); - let start_slot = finalized_slot.max(local_proof_slot) + 1; - - let Some((peer_id, peer_slot)) = self.best_peer(cx) else { - return; - }; - - let gap = peer_slot - .as_u64() - .checked_add(1) - .and_then(|end| end.checked_sub(start_slot.as_u64())) - .unwrap_or(0); - - if gap > self.range_request_threshold { - match cx.request_execution_proofs_by_range(peer_id, start_slot, gap) { - Ok(id) => { - debug!(%start_slot, %peer_slot, gap, "ProofSync: range request sent"); - self.range_request = Some(ByRangeRequest { id, peer_id }); - } - Err(e) => { - debug!(error = ?e, "ProofSync: range request error"); - } - } + // Drain post-request cooldown before issuing the next request. + if self.post_request_cooldown > 0 { + self.post_request_cooldown -= 1; return; } - // While a by-root batch is already in-flight, wait for it to complete. - if self.root_request.is_some() { + // If a range request is already in-flight, wait for it to drain. + if self.range_request.is_some() { return; } + // Ask the proof engine what it still needs — this is the authoritative source. #[cfg(not(test))] let missing = self.chain.missing_execution_proofs(); #[cfg(test)] @@ -233,8 +194,16 @@ impl ProofSync { .clone() .unwrap_or_else(|| self.chain.missing_execution_proofs()); - // Collect all eligible roots into one batch, skipping slots ahead of the best peer. - let batch: Vec = missing + if missing.is_empty() { + return; + } + + let Some((peer_id, peer_slot)) = self.best_peer(cx) else { + return; + }; + + // Keep only entries the best peer can serve; sort by slot for run analysis. + let mut servable: Vec = missing .into_iter() .filter(|info| { if peer_slot < info.slot { @@ -251,15 +220,62 @@ impl ProofSync { }) .collect(); - if batch.is_empty() { + if servable.is_empty() { return; } - match cx.request_execution_proofs_by_root(peer_id, &batch) { + servable.sort_unstable_by_key(|m| m.slot); + + let num_types = cx.configured_proof_types_count(); + + // Partially-held entries must appear in proof_filters so the peer skips + // proof types the requester already has. + let partial: Vec = servable + .iter() + .filter(|m| !m.existing_proof_types.is_empty()) + .cloned() + .collect(); + + let range_bytes = by_range_request_size(&partial, num_types); + let root_bytes = by_root_request_size(&servable, num_types); + + // A single range request covers all servable slots with a fixed 20-byte header + // plus filters only for partial entries. If that is cheaper than naming every + // block individually in a root request, use range; otherwise use root. + if range_bytes < root_bytes { + let start_slot = servable[0].slot; + // count spans from first to last missing slot inclusive. + let count = (servable.last().expect("non-empty").slot - start_slot).as_u64() + 1; + + match cx.request_execution_proofs_by_range(peer_id, start_slot, count, &partial) { + Ok(id) => { + debug!( + start_slot = %start_slot, + count, + num_filters = partial.len(), + range_bytes, + root_bytes, + "ProofSync: range request sent" + ); + self.range_request = Some(ByRangeRequest { id, peer_id }); + } + Err(e) => { + debug!(error = ?e, "ProofSync: range request error"); + } + } + return; + } + + // Root request is smaller — name every block and proof type still needed. + if self.root_request.is_some() { + return; + } + + match cx.request_execution_proofs_by_root(peer_id, &servable) { Ok(id) => { debug!( - num_roots = batch.len(), - "ProofSync: requesting missing proofs batch" + num_roots = servable.len(), + root_bytes, range_bytes, "ProofSync: by-root batch sent" ); self.root_request = Some(ByRootRequest { id, peer_id }); } @@ -271,13 +287,13 @@ impl ProofSync { /// Called when an `ExecutionProofsByRange` RPC stream terminates (response `None`). /// - /// Transitions back to `Waiting` to give the proof engine time to process the - /// received proofs before the next request is issued. + /// Clears the in-flight request and starts a brief cooldown so the beacon processor + /// has time to import the received proofs before the next request is issued. pub fn on_range_request_terminated(&mut self, id: &ExecutionProofsByRangeRequestId) { if self.range_request.as_ref().map(|r| &r.id) == Some(id) { - info!("ProofSync: range stream complete, cooling down before next request"); + debug!("ProofSync: range stream complete"); self.range_request = None; - self.state = ProofSyncState::Waiting(self.activation_slots); + self.post_request_cooldown = POST_REQUEST_COOLDOWN_SLOTS; } } @@ -303,11 +319,12 @@ impl ProofSync { /// Called when an `ExecutionProofsByRoot` RPC stream terminates (response `None`). /// - /// Clears the in-flight root request. The proof engine decides whether the received - /// proofs satisfy the request; this just frees the slot for the next batch. + /// Clears the in-flight root request and starts a brief cooldown so the beacon + /// processor has time to import the received proofs before the next request is issued. pub fn on_root_request_terminated(&mut self, id: &ExecutionProofsByRootRequestId) { if self.root_request.as_ref().map(|r| &r.id) == Some(id) { self.root_request = None; + self.post_request_cooldown = POST_REQUEST_COOLDOWN_SLOTS; } } @@ -487,3 +504,54 @@ impl ProofSync { result } } + +// ── Request-size helpers ────────────────────────────────────────────────────── + +/// SSZ encoded byte cost of one `ProofByRootIdentifier` entry when it appears inside a +/// variable-length list. +/// +/// Layout: +/// - 4 bytes — list offset-table entry (one u32 pointer per variable-length item) +/// - 32 bytes — `block_root: Hash256` (fixed field within the container) +/// - 4 bytes — SSZ offset for `proof_types` (variable field pointer within the container) +/// - `needed` × 1 byte — the proof type values themselves (`ProofType` encodes as one `u8`) +/// +/// `needed` = `num_configured_types` − `existing_proof_types.len()`, i.e. the types still +/// outstanding for this block. +fn per_identifier_ssz_bytes(info: &MissingProofInfo, num_configured_types: usize) -> usize { + let needed = num_configured_types.saturating_sub(info.existing_proof_types.len()); + 4 + 32 + 4 + needed +} + +/// Byte size of an `ExecutionProofsByRoot` request encoding `missing` identifiers. +/// +/// The request body is `List[ProofByRootIdentifier, ...]`; each entry pays the full +/// per-identifier cost regardless of whether it is fully or partially missing. +pub(crate) fn by_root_request_size( + missing: &[MissingProofInfo], + num_configured_types: usize, +) -> usize { + missing + .iter() + .map(|m| per_identifier_ssz_bytes(m, num_configured_types)) + .sum() +} + +/// Byte size of an `ExecutionProofsByRange` request. +/// +/// Layout: +/// - 20 bytes — fixed header: `start_slot` (8) + `count` (8) + `proof_filters` offset (4) +/// - `proof_filters` — only entries where `existing_proof_types` is non-empty; fully-missing +/// blocks are absent (the peer returns all proof types for them by default). +/// +/// `partial_missing` must contain only entries with non-empty `existing_proof_types`. +pub(crate) fn by_range_request_size( + partial_missing: &[MissingProofInfo], + num_configured_types: usize, +) -> usize { + let filter_bytes: usize = partial_missing + .iter() + .map(|m| per_identifier_ssz_bytes(m, num_configured_types)) + .sum(); + 20 + filter_bytes +} diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 8b264419685..24c75f6d993 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -111,7 +111,7 @@ impl TestRig { init_tracing(); let network_globals = beacon_processor.network_globals.clone(); - let mut sync_manager = SyncManager::new( + let sync_manager = SyncManager::new( chain, network_tx, beacon_processor.into(), @@ -120,8 +120,6 @@ impl TestRig { fork_context, ProofTypes::default(), ); - // In tests any non-zero gap triggers a range request, keeping slot advancement minimal. - sync_manager.proof_sync_mut().set_range_request_threshold(0); TestRig { beacon_processor_rx, diff --git a/beacon_node/network/src/sync/tests/range.rs b/beacon_node/network/src/sync/tests/range.rs index 3372ce98765..f555b1cade3 100644 --- a/beacon_node/network/src/sync/tests/range.rs +++ b/beacon_node/network/src/sync/tests/range.rs @@ -340,12 +340,21 @@ impl TestRig { fn find_execution_proofs_by_range_request_with_slot( &mut self, ) -> ((ExecutionProofsByRangeRequestId, PeerId), Slot) { + let ((id, peer_id), start_slot, _count) = + self.find_execution_proofs_by_range_request_params(); + ((id, peer_id), start_slot) + } + + /// Assert an `ExecutionProofsByRange` RPC was sent; returns `((id, peer_id), start_slot, count)`. + fn find_execution_proofs_by_range_request_params( + &mut self, + ) -> ((ExecutionProofsByRangeRequestId, PeerId), Slot, u64) { self.pop_received_network_event(|ev| match ev { NetworkMessage::SendRequest { peer_id, request: RequestType::ExecutionProofsByRange(req), app_request_id: AppRequestId::Sync(SyncRequestId::ExecutionProofsByRange(id)), - } => Some(((*id, *peer_id), Slot::new(req.start_slot))), + } => Some(((*id, *peer_id), Slot::new(req.start_slot), req.count)), _ => None, }) .unwrap_or_else(|e| panic!("Expected ExecutionProofsByRange request: {e:?}")) @@ -752,13 +761,19 @@ fn finalized_sync_not_enough_custody_peers_on_start() { // --- ProofSync state-machine tests --- // These tests exercise the `ProofSync` state machine directly, covering its full lifecycle: -// Idle → Syncing (range request for large gaps, by-root fill for small gaps), +// Idle → Syncing (range request when a consecutive run of fully-missing blocks saves bytes, +// by-root fill when all missing blocks are only partially missing). // pause/resume semantics, concurrency cap, in-flight deduplication, and response forwarding. // -// In tests, range_request_threshold = 0, so any non-zero slot gap triggers a range request. -// At genesis (slot 0, gap = 0) the poll goes directly to by-root fill requests. +// Range vs root decisions are driven by `test_missing_proofs` (injected into proof_sync) and +// a direct size comparison: by_range_request_size vs by_root_request_size over all servable +// missing proofs. Fully-missing blocks always prefer range; all-partial sets always prefer root. -/// Build a `MissingProofInfo` with a fresh random root for test seeding. +/// Build a fully-missing `MissingProofInfo` (no proof types held yet). +/// +/// Because no existing types are held, the size comparison always prefers a range request: +/// the 20-byte range header is smaller than the 40-byte per-identifier cost when all types +/// are needed. fn missing_proof(root: Hash256) -> MissingProofInfo { MissingProofInfo { root, @@ -767,6 +782,20 @@ fn missing_proof(root: Hash256) -> MissingProofInfo { } } +/// Build a partially-missing `MissingProofInfo` (one proof type already held). +/// +/// Because the entry is partial, it must appear in `proof_filters` if included in a range +/// request — making the range request larger than the equivalent by-root identifier. +/// The size comparison therefore never prefers range for these entries; `poll()` falls back +/// to `ExecutionProofsByRoot`. +fn partial_missing_proof(root: Hash256) -> MissingProofInfo { + MissingProofInfo { + root, + existing_proof_types: vec![0u8], // one type already held; still needs the rest + slot: Default::default(), + } +} + /// Build a minimal `SignedExecutionProof` suitable for RPC response messages. fn make_signed_proof() -> Arc { Arc::new(types::SignedExecutionProof { @@ -786,7 +815,7 @@ fn test_proof_sync_starts_in_idle() { } /// Test 2: After `start()`, the next `poll()` sends an `ExecutionProofsByRange` RPC. -/// (slot gap = 1 > range_request_threshold = 0 in tests → range request). +/// A fully-missing block is cheaper to request via range (20 bytes) than by-root (40+ bytes). #[test] fn test_proof_sync_pending_range_issues_request_on_poll() { let mut rig = TestRig::test_setup(); @@ -799,6 +828,10 @@ fn test_proof_sync_pending_range_issues_request_on_poll() { ProofSyncState::Syncing ); + // Inject a fully-missing proof — range request is always cheaper. + rig.sync_manager.proof_sync_mut().test_missing_proofs = + Some(vec![missing_proof(Hash256::random())]); + rig.sync_manager.poll_proof_sync(); let _ = rig.find_execution_proofs_by_range_request(); assert!( @@ -815,6 +848,9 @@ fn test_proof_sync_no_peer_stays_pending() { rig.harness.advance_slot(); rig.sync_manager.start_proof_sync(); + // Inject a fully-missing proof so the size comparison prefers range when a peer is found. + rig.sync_manager.proof_sync_mut().test_missing_proofs = + Some(vec![missing_proof(Hash256::random())]); rig.sync_manager.poll_proof_sync(); rig.expect_no_execution_proof_range_request(); assert_eq!( @@ -838,6 +874,8 @@ fn test_proof_sync_in_flight_poll_is_noop() { rig.harness.advance_slot(); rig.sync_manager.start_proof_sync(); + rig.sync_manager.proof_sync_mut().test_missing_proofs = + Some(vec![missing_proof(Hash256::random())]); rig.sync_manager.poll_proof_sync(); let _ = rig.find_execution_proofs_by_range_request(); rig.drain_execution_proof_status_requests(); @@ -849,33 +887,35 @@ fn test_proof_sync_in_flight_poll_is_noop() { assert!(rig.sync_manager.proof_sync().by_range_request().is_some()); } -/// Test 5: Stream termination with the correct ID clears the in-flight range request. -/// The next poll will then issue by-root fill requests (gap is now 0 at genesis). +/// Test 5: Stream termination with the correct ID clears the in-flight range request and +/// starts a post-request cooldown; state stays `Syncing`. #[test] -fn test_proof_sync_range_termination_enters_fill_mode() { +fn test_proof_sync_range_termination_clears_request() { let mut rig = TestRig::test_setup(); let _proof_peer = rig.new_proof_peer_with_status(1); rig.harness.advance_slot(); rig.sync_manager.start_proof_sync(); + rig.sync_manager.proof_sync_mut().test_missing_proofs = + Some(vec![missing_proof(Hash256::random())]); rig.sync_manager.poll_proof_sync(); let (req_id, peer_id) = rig.find_execution_proofs_by_range_request(); assert!(rig.sync_manager.proof_sync().by_range_request().is_some()); rig.terminate_execution_proofs_by_range(req_id, peer_id); - // Termination transitions to Waiting to give the proof engine time to process - // received proofs before the next range request is issued. - assert!( - matches!( - rig.sync_manager.proof_sync().state(), - ProofSyncState::Waiting(_) - ), - "Range termination should enter Waiting state" + assert_eq!( + rig.sync_manager.proof_sync().state(), + ProofSyncState::Syncing, + "State should remain Syncing after range termination" ); assert!( rig.sync_manager.proof_sync().by_range_request().is_none(), "Range request should be cleared after stream termination" ); + // The cooldown blocks a new request for one slot tick. + rig.sync_manager.poll_proof_sync(); + rig.expect_no_execution_proof_range_request(); + assert!(rig.sync_manager.proof_sync().by_root_request().is_none()); } /// Test 6: Stream termination with a wrong ID is ignored; the range request stays in-flight. @@ -886,6 +926,8 @@ fn test_proof_sync_wrong_id_termination_ignored() { rig.harness.advance_slot(); rig.sync_manager.start_proof_sync(); + rig.sync_manager.proof_sync_mut().test_missing_proofs = + Some(vec![missing_proof(Hash256::random())]); rig.sync_manager.poll_proof_sync(); let (_req_id, peer_id) = rig.find_execution_proofs_by_range_request(); assert!(rig.sync_manager.proof_sync().by_range_request().is_some()); @@ -918,8 +960,8 @@ fn test_proof_sync_fill_mode_no_missing_proofs() { ); } -/// Test 8: With seeded missing proofs, `poll()` sends all roots in a single batched -/// `ExecutionProofsByRoot` request. +/// Test 8: With partially-missing proofs, `poll()` sends all roots in a single batched +/// `ExecutionProofsByRoot` request (partial entries make range more expensive than root). #[test] fn test_proof_sync_fill_mode_issues_by_root_requests() { let mut rig = TestRig::test_setup(); @@ -929,8 +971,8 @@ fn test_proof_sync_fill_mode_issues_by_root_requests() { rig.drain_execution_proof_status_requests(); let missing = vec![ - missing_proof(Hash256::random()), - missing_proof(Hash256::random()), + partial_missing_proof(Hash256::random()), + partial_missing_proof(Hash256::random()), ]; rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(missing); rig.sync_manager.poll_proof_sync(); @@ -951,8 +993,10 @@ fn test_proof_sync_fill_mode_batches_all_roots() { rig.sync_manager.start_proof_sync(); rig.drain_execution_proof_status_requests(); - // Seed 6 distinct missing proofs; all go into one batch request. - let missing: Vec = (0..6).map(|_| missing_proof(Hash256::random())).collect(); + // Seed 6 distinct partial proofs; all go into one by-root batch request. + let missing: Vec = (0..6) + .map(|_| partial_missing_proof(Hash256::random())) + .collect(); rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(missing); rig.sync_manager.poll_proof_sync(); @@ -975,8 +1019,8 @@ fn test_proof_sync_fill_mode_skips_while_batch_in_flight() { rig.drain_execution_proof_status_requests(); let missing = vec![ - missing_proof(Hash256::random()), - missing_proof(Hash256::random()), + partial_missing_proof(Hash256::random()), + partial_missing_proof(Hash256::random()), ]; rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(missing); @@ -1004,8 +1048,8 @@ fn test_proof_sync_fill_mode_no_peer_breaks() { .set_state(ProofSyncState::Syncing); let missing = vec![ - missing_proof(Hash256::random()), - missing_proof(Hash256::random()), + partial_missing_proof(Hash256::random()), + partial_missing_proof(Hash256::random()), ]; rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(missing); rig.sync_manager.poll_proof_sync(); @@ -1026,7 +1070,7 @@ fn test_proof_sync_on_request_terminated_clears_in_flight() { rig.sync_manager.start_proof_sync(); rig.drain_execution_proof_status_requests(); - let missing = vec![missing_proof(Hash256::random())]; + let missing = vec![partial_missing_proof(Hash256::random())]; rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(missing); rig.sync_manager.poll_proof_sync(); @@ -1050,10 +1094,10 @@ fn test_proof_sync_pause_resets_to_idle() { rig.sync_manager.start_proof_sync(); rig.drain_execution_proof_status_requests(); - // Seed some missing proofs; poll sends one batch request. + // Seed some partial proofs; poll sends one by-root batch request. let missing = vec![ - missing_proof(Hash256::random()), - missing_proof(Hash256::random()), + partial_missing_proof(Hash256::random()), + partial_missing_proof(Hash256::random()), ]; rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(missing); rig.sync_manager.poll_proof_sync(); @@ -1080,6 +1124,10 @@ fn test_proof_sync_re_enter_range_resets_then_restarts() { let _proof_peer = rig.new_proof_peer_with_status(1); rig.harness.advance_slot(); + // Inject a fully-missing proof so the size comparison prefers range. + rig.sync_manager.proof_sync_mut().test_missing_proofs = + Some(vec![missing_proof(Hash256::random())]); + // First range request cycle. rig.sync_manager.start_proof_sync(); rig.sync_manager.poll_proof_sync(); @@ -1098,29 +1146,31 @@ fn test_proof_sync_re_enter_range_resets_then_restarts() { ProofSyncState::Syncing ); - // New poll sends a fresh range request (slot gap still > 0). + // New poll sends a fresh range request (same missing proof data still injected). rig.sync_manager.poll_proof_sync(); let _ = rig.find_execution_proofs_by_range_request(); assert!(rig.sync_manager.proof_sync().by_range_request().is_some()); } -/// Test 15: At genesis (slot gap = 0 ≤ range_request_threshold = 0), no range request -/// is issued — `poll()` goes directly to the by-root fill path. +/// Test 15: With no missing proofs reported by the proof engine, no request of any kind +/// is emitted and the state stays `Syncing`. #[test] -fn test_proof_sync_count_zero_skips_to_fill() { +fn test_proof_sync_no_missing_proofs_no_request() { let mut rig = TestRig::test_setup(); let _proof_peer = rig.new_proof_peer_with_status(0); rig.sync_manager.start_proof_sync(); rig.drain_execution_proof_status_requests(); + // test_missing_proofs = None → chain.missing_execution_proofs() returns empty. rig.sync_manager.poll_proof_sync(); - rig.expect_no_execution_proof_range_request(); + rig.expect_empty_network(); assert_eq!( rig.sync_manager.proof_sync().state(), ProofSyncState::Syncing ); assert!(rig.sync_manager.proof_sync().by_range_request().is_none()); + assert!(rig.sync_manager.proof_sync().by_root_request().is_none()); } /// Test 16: A proof arriving on an `ExecutionProofsByRange` stream must be forwarded @@ -1132,6 +1182,8 @@ fn test_proof_sync_range_response_forwarded_to_processor() { rig.harness.advance_slot(); rig.sync_manager.start_proof_sync(); + rig.sync_manager.proof_sync_mut().test_missing_proofs = + Some(vec![missing_proof(Hash256::random())]); rig.sync_manager.poll_proof_sync(); let (req_id, peer_id) = rig.find_execution_proofs_by_range_request(); assert!(rig.sync_manager.proof_sync().by_range_request().is_some()); @@ -1159,11 +1211,11 @@ fn test_proof_sync_root_response_forwarded_to_processor() { let mut rig = TestRig::test_setup(); let _proof_peer = rig.new_proof_peer_with_status(0); - // At genesis (gap = 0) poll goes directly to by-root fill. + // Partial missing proofs are cheaper via by-root than range. rig.sync_manager.start_proof_sync(); rig.drain_execution_proof_status_requests(); - let missing = vec![missing_proof(Hash256::random())]; + let missing = vec![partial_missing_proof(Hash256::random())]; rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(missing); rig.sync_manager.poll_proof_sync(); @@ -1383,7 +1435,7 @@ fn test_inbound_status_populates_cache() { /// Test 22: `local_execution_proof_status` can be set and read back via `network_globals`. /// /// This verifies the `NetworkGlobals` getter/setter used by the proof-verified callback path -/// (in `gossip_methods.rs`) and consumed by `ProofSync.poll()`. +/// in `gossip_methods.rs`. #[test] fn test_local_execution_proof_status_read_write() { let rig = TestRig::test_setup(); @@ -1405,58 +1457,146 @@ fn test_local_execution_proof_status_read_write() { assert_eq!(updated.block_root, block_root); } -/// Test 23: `ProofSync.poll()` uses `local_execution_proof_status` as the lower bound -/// for `start_slot`, so proofs already verified locally are never re-requested. -/// -/// Setup: peer announces slot 10, local proof status is at slot 7. -/// Expected: range request start_slot = max(finalized_slot, local_proof_slot) + 1 = 8. +/// Test 23: For a single fully-missing proof, the range request covers exactly that slot +/// (start_slot = proof slot, count = 1). #[test] -fn test_proof_sync_start_slot_respects_local_proof_status() { +fn test_proof_sync_range_covers_single_missing_slot() { let mut rig = TestRig::test_setup(); - - // Peer has proofs up to slot 10. let _proof_peer = rig.new_proof_peer_with_status(10); rig.harness.advance_slot(); - // Simulate that we have already verified proofs up to slot 7 locally. - rig.network_globals - .set_local_execution_proof_status(ExecutionProofStatus { - slot: 7, - block_root: Hash256::repeat_byte(0xcc), - }); + rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(vec![MissingProofInfo { + root: Hash256::random(), + existing_proof_types: vec![], + slot: Slot::new(5), + }]); rig.sync_manager.start_proof_sync(); rig.sync_manager.poll_proof_sync(); - let ((_req_id, _peer_id), start_slot) = rig.find_execution_proofs_by_range_request_with_slot(); - - // start_slot must be at least local_proof_slot + 1 = 8. - assert!( - start_slot.as_u64() >= 8, - "start_slot {start_slot} should be >= 8 (local_proof_slot + 1)" + let (_, start_slot, count) = rig.find_execution_proofs_by_range_request_params(); + assert_eq!( + start_slot.as_u64(), + 5, + "start_slot should be the proof's slot" ); + assert_eq!(count, 1, "count should be 1 for a single missing slot"); } -/// Test 24: When `local_execution_proof_status` is updated to a slot beyond the peer's -/// announced slot, `ProofSync.poll()` computes a zero gap and issues no range request. +/// Test 24: When all missing proofs have slots beyond the peer's announced slot, no request +/// is issued (the peer cannot serve them yet). #[test] -fn test_proof_sync_no_request_when_local_status_ahead_of_peer() { +fn test_proof_sync_no_request_when_missing_slot_ahead_of_peer() { let mut rig = TestRig::test_setup(); // Peer only has proofs up to slot 5. let _proof_peer = rig.new_proof_peer_with_status(5); - rig.harness.advance_slot(); - // Local proof status is already at slot 5 (equal to peer) — gap = 0. - rig.network_globals - .set_local_execution_proof_status(ExecutionProofStatus { - slot: 5, - block_root: Hash256::repeat_byte(0xdd), - }); + // Inject a proof at slot 6 — beyond what the peer can serve. + rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(vec![MissingProofInfo { + root: Hash256::random(), + existing_proof_types: vec![], + slot: Slot::new(6), + }]); rig.sync_manager.start_proof_sync(); + rig.drain_execution_proof_status_requests(); rig.sync_manager.poll_proof_sync(); - // No range request should be emitted since start_slot (6) > peer_slot (5). + // No request should be emitted since the only missing slot is beyond peer_slot = 5. rig.expect_no_execution_proof_range_request(); + rig.expect_empty_network(); +} + +/// Test 25: Non-consecutive fully-missing slots are covered by a single range request whose +/// `count` spans from the first slot to the last (inclusive), bridging any gaps. +/// +/// By-range request size = 20 bytes (no partial filters). +/// By-root request size = 44 + 44 = 88 bytes. +/// Range wins; count = last_slot − first_slot + 1. +#[test] +fn test_proof_sync_range_spans_non_consecutive_slots() { + let mut rig = TestRig::test_setup(); + let _proof_peer = rig.new_proof_peer_with_status(20); + rig.harness.advance_slot(); + + // Two fully-missing proofs at slots 5 and 10 (not consecutive). + rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(vec![ + MissingProofInfo { + root: Hash256::random(), + existing_proof_types: vec![], + slot: Slot::new(5), + }, + MissingProofInfo { + root: Hash256::random(), + existing_proof_types: vec![], + slot: Slot::new(10), + }, + ]); + + rig.sync_manager.start_proof_sync(); + rig.sync_manager.poll_proof_sync(); + + let (_, start_slot, count) = rig.find_execution_proofs_by_range_request_params(); + assert_eq!( + start_slot.as_u64(), + 5, + "range should start at the earliest missing slot" + ); + assert_eq!( + count, 6, + "count should span from slot 5 to slot 10 inclusive (10 - 5 + 1 = 6)" + ); +} + +/// Test 26: When all missing proofs are partially held (some proof types already present), +/// every block must appear in `proof_filters` — making the range request larger than root. +/// The size comparison picks root even when slots are consecutive. +/// +/// By-range size = 20 + 43 + 43 = 106 bytes (both entries in proof_filters). +/// By-root size = 43 + 43 = 86 bytes. +/// Root wins. +/// +/// A mixed set (one fully-missing + one partial) is also tested: range wins there because +/// the fully-missing entry is free in range (not in proof_filters). +/// By-range = 20 + 43 = 63 bytes, by-root = 44 + 43 = 87 bytes → range cheaper. +#[test] +fn test_proof_sync_range_vs_root_size_decision() { + // ── All partial → root chosen ────────────────────────────────────────────── + { + let mut rig = TestRig::test_setup(); + let _proof_peer = rig.new_proof_peer_with_status(10); + rig.harness.advance_slot(); + + rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(vec![ + partial_missing_proof(Hash256::random()), + partial_missing_proof(Hash256::random()), + ]); + rig.sync_manager.start_proof_sync(); + rig.drain_execution_proof_status_requests(); + rig.sync_manager.poll_proof_sync(); + + rig.expect_no_execution_proof_range_request(); + let _ = rig.find_execution_proofs_by_root_request(); + } + + // ── One fully-missing + one partial → range chosen ───────────────────────── + { + let mut rig = TestRig::test_setup(); + let _proof_peer = rig.new_proof_peer_with_status(10); + rig.harness.advance_slot(); + + rig.sync_manager.proof_sync_mut().test_missing_proofs = Some(vec![ + // fully missing at slot 0 — free in range, costs 44 in root + missing_proof(Hash256::random()), + // partial at slot 0 — costs 43 in both range (as filter) and root + partial_missing_proof(Hash256::random()), + ]); + rig.sync_manager.start_proof_sync(); + rig.sync_manager.poll_proof_sync(); + + // range_bytes = 20 + 43 = 63 < root_bytes = 44 + 43 = 87 + let (_, _, count) = rig.find_execution_proofs_by_range_request_params(); + assert_eq!(count, 1, "both entries are at slot 0; count = 1"); + } } diff --git a/testing/proof_engine/src/rig.rs b/testing/proof_engine/src/rig.rs index 74a6f12c6e1..f4fa74d9f3e 100644 --- a/testing/proof_engine/src/rig.rs +++ b/testing/proof_engine/src/rig.rs @@ -228,11 +228,4 @@ fn base_builder() -> TestNetworkFixtureBuilder { delayed_nodes: 0, genesis_delay: 40, }) - // Disable the activation delay so proof sync fires immediately after the node - // becomes head-synced. The default (10 slots) is intended for production to let - // the beacon processor drain, but in CI each slot can take several seconds, - // causing the countdown to exhaust the test timeout. - .map_client_config(|config| { - config.network.proof_sync_activation_slots = 0; - }) } From 0dda7bd91e61346e4d0949cdf7735b9296d9cbc8 Mon Sep 17 00:00:00 2001 From: frisitano Date: Tue, 31 Mar 2026 19:52:49 +0200 Subject: [PATCH 2/3] feat: update proof rpc req type --- .../execution_layer/src/eip8025/state.rs | 30 +++++----- .../network_beacon_processor/rpc_methods.rs | 16 +++--- .../network/src/sync/network_context.rs | 31 ++++++----- beacon_node/network/src/sync/proof_sync.rs | 55 ++++++++++++------- 4 files changed, 78 insertions(+), 54 deletions(-) diff --git a/beacon_node/execution_layer/src/eip8025/state.rs b/beacon_node/execution_layer/src/eip8025/state.rs index 4f090c17dd4..e2bab4487e3 100644 --- a/beacon_node/execution_layer/src/eip8025/state.rs +++ b/beacon_node/execution_layer/src/eip8025/state.rs @@ -51,13 +51,17 @@ impl State { Self::default() } - /// Return buffer entries that do not yet have sufficient proofs for promotion, - /// restricted to those on the ancestor path required to satisfy `latest_fcs`. + /// Return all buffer entries on the ancestor path required to satisfy `latest_fcs`, + /// including entries that already have sufficient proofs. + /// + /// Complete entries are returned so the sync layer can include them as skip-filters in + /// `ExecutionProofsByRange` requests, telling the serving peer not to re-send proofs + /// for blocks the requester already holds. Callers should inspect `existing_proof_types` + /// against the configured proof type set to determine which entries are still missing. /// /// If `latest_fcs` is unset there is no pending fork-choice update to satisfy, so /// nothing is returned. Otherwise the buffer is walked backwards from - /// `latest_fcs.head_block_hash`; entries that lack sufficient proofs are collected - /// until a block is not found in the buffer (reached the tree or an unseen block). + /// `latest_fcs.head_block_hash` until a block is not found in the buffer. pub fn missing_proofs(&self) -> Vec { let Some(latest_fcs) = &self.latest_fcs else { return vec![]; @@ -71,22 +75,20 @@ impl State { .map(|p| (p.metadata.block_hash, p)) .collect(); - // Walk backwards from the FCS head through buffer entries, collecting - // those that still lack sufficient proofs. Stop when a block is not in - // the buffer (reached the tree or an unseen block). + // Walk backwards from the FCS head through buffer entries, collecting all + // entries (missing and complete). Stop when a block is not in the buffer + // (reached the tree or an unseen block). let mut result = Vec::new(); let mut current = latest_fcs.head_block_hash; loop { let Some(req) = buffer_by_block_hash.get(¤t) else { break; }; - if req.proofs.len() < self.min_required_proofs { - result.push(MissingProofInfo { - root: req.metadata.request_root, - existing_proof_types: req.proofs.iter().map(|p| p.message.proof_type).collect(), - slot: Default::default(), // populated by BeaconChain::missing_execution_proofs() - }); - } + result.push(MissingProofInfo { + root: req.metadata.request_root, + existing_proof_types: req.proofs.iter().map(|p| p.message.proof_type).collect(), + slot: Default::default(), // populated by BeaconChain::missing_execution_proofs() + }); current = req.metadata.parent_hash; } diff --git a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs index 0fe3b86faf8..1758f7f5bd5 100644 --- a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs @@ -1327,9 +1327,10 @@ impl NetworkBeaconProcessor { /// Handle an `ExecutionProofsByRange` request from the peer (EIP-8025). /// /// Streams `SignedExecutionProof` objects known for the requested slot range, filtered by - /// `proof_filters` when present. If `proof_filters` is non-empty, blocks listed in it are - /// served only for the proof types specified; blocks absent from `proof_filters` receive all - /// known proof types. This mirrors the `ExecutionProofsByRoot` semantics. + /// `proof_filters` when present. For blocks listed in `proof_filters`: + /// - a non-empty `proof_types` list → serve only those types + /// - an empty `proof_types` list → skip the block entirely (requester already has all proofs) + /// Blocks absent from `proof_filters` receive all known proof types. pub fn handle_execution_proofs_by_range_request( &self, peer_id: PeerId, @@ -1376,11 +1377,12 @@ impl NetworkBeaconProcessor { for block_root in block_roots { let allowed_types = filter_map.get(&block_root); for proof in self.chain.get_execution_proofs_by_block_root(block_root) { - // If this block has a filter entry, skip proof types not requested. - // An absent entry means "return all types" (same as an empty proof_types list). + // If this block has a filter entry: + // - empty proof_types → skip the block entirely (requester already complete) + // - non-empty → serve only the listed types + // An absent entry means "return all types". if let Some(types) = allowed_types - && !types.is_empty() - && !types.contains(&proof.message.proof_type) + && (types.is_empty() || !types.contains(&proof.message.proof_type)) { continue; } diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index e0ebcef73e5..6ee866b37e1 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -417,11 +417,14 @@ impl SyncNetworkContext { /// Send a `ExecutionProofsByRange` request to the given proof-capable peer. /// - /// `partial_missing` contains `MissingProofInfo` entries for blocks within the requested slot - /// range that already have *some* proof types. An entry is included in the request's - /// `proof_filters` only when the block has partial coverage (i.e. `existing_proof_types` is - /// non-empty), so the peer knows to return only the missing types. Blocks with no existing - /// proofs are not filtered — the peer will return all proof types for them. + /// `filter_entries` contains `MissingProofInfo` entries for blocks within the requested slot + /// range that should appear in `proof_filters`: + /// - entries with non-empty `existing_proof_types` → peer returns only the missing types + /// - entries with all types already present (`needed` is empty) → peer skips the block + /// entirely (requester already holds all proofs for it) + /// + /// Blocks with no existing proofs at all are excluded from `filter_entries`; the peer + /// will return all known proof types for them by default. /// /// Callers should use `find_best_proof_capable_peer` to select the peer first. pub fn request_execution_proofs_by_range( @@ -429,21 +432,22 @@ impl SyncNetworkContext { peer_id: PeerId, start_slot: Slot, count: u64, - partial_missing: &[MissingProofInfo], + filter_entries: &[MissingProofInfo], ) -> Result { let id = ExecutionProofsByRangeRequestId { id: self.next_id() }; - // Build proof_filters: one entry per block in `partial_missing` that still needs a subset - // of proof types. Blocks already fully proven are excluded (wouldn't appear in - // partial_missing); blocks with no proofs at all are also excluded (peer returns all types - // by default for blocks absent from proof_filters). + // Build proof_filters from filter_entries: + // - partial blocks: proof_types = types still needed (non-empty) + // - complete blocks: proof_types = [] → peer skips the block entirely + // - fully-missing blocks (existing empty): excluded — peer returns all types by default let max_request_blocks = self .chain .spec .max_request_blocks(self.fork_context.current_fork_name()); let mut filter_items: Vec = Vec::new(); - for info in partial_missing { + for info in filter_entries { if info.existing_proof_types.is_empty() { + // Fully missing: no filter entry; peer returns all proof types by default. continue; } let needed: Vec = self @@ -452,9 +456,8 @@ impl SyncNetworkContext { .map(|t| t.to_u8()) .filter(|t| !info.existing_proof_types.contains(t)) .collect(); - if needed.is_empty() { - continue; - } + // needed may be empty for complete blocks — that is intentional. + // An empty proof_types list tells the peer to skip this block entirely. let proof_types = VariableList::new(needed) .map_err(|e| RpcRequestSendError::InternalError(format!("proof_types: {e:?}")))?; filter_items.push(ProofByRootIdentifier { diff --git a/beacon_node/network/src/sync/proof_sync.rs b/beacon_node/network/src/sync/proof_sync.rs index 78d184aed08..f08021a978d 100644 --- a/beacon_node/network/src/sync/proof_sync.rs +++ b/beacon_node/network/src/sync/proof_sync.rs @@ -228,31 +228,47 @@ impl ProofSync { let num_types = cx.configured_proof_types_count(); - // Partially-held entries must appear in proof_filters so the peer skips - // proof types the requester already has. - let partial: Vec = servable + // Partition into blocks still needing at least one proof type and blocks + // that are already complete in the proof engine buffer. + let (actually_missing, complete_in_window): (Vec, Vec) = + servable + .into_iter() + .partition(|m| m.existing_proof_types.len() < num_types); + + if actually_missing.is_empty() { + return; + } + + // Build filter_entries for the range request: + // - partial entries (some types present, some missing) → peer returns only needed types + // - complete entries → peer skips the block entirely (empty proof_types in filter) + // Fully-missing entries are excluded from the filter; the peer returns all types for them. + let filter_entries: Vec = actually_missing .iter() .filter(|m| !m.existing_proof_types.is_empty()) .cloned() + .chain(complete_in_window) .collect(); - let range_bytes = by_range_request_size(&partial, num_types); - let root_bytes = by_root_request_size(&servable, num_types); + let range_bytes = by_range_request_size(&filter_entries, num_types); + let root_bytes = by_root_request_size(&actually_missing, num_types); - // A single range request covers all servable slots with a fixed 20-byte header - // plus filters only for partial entries. If that is cheaper than naming every - // block individually in a root request, use range; otherwise use root. + // A single range request covers all servable slots with a fixed 20-byte header plus + // proof_filters for partial and complete blocks. If cheaper than naming every block + // individually in a root request, use range; otherwise use root. if range_bytes < root_bytes { - let start_slot = servable[0].slot; + let start_slot = actually_missing[0].slot; // count spans from first to last missing slot inclusive. - let count = (servable.last().expect("non-empty").slot - start_slot).as_u64() + 1; + let count = + (actually_missing.last().expect("non-empty").slot - start_slot).as_u64() + 1; - match cx.request_execution_proofs_by_range(peer_id, start_slot, count, &partial) { + match cx.request_execution_proofs_by_range(peer_id, start_slot, count, &filter_entries) + { Ok(id) => { debug!( start_slot = %start_slot, count, - num_filters = partial.len(), + num_filters = filter_entries.len(), range_bytes, root_bytes, "ProofSync: range request sent" @@ -271,10 +287,10 @@ impl ProofSync { return; } - match cx.request_execution_proofs_by_root(peer_id, &servable) { + match cx.request_execution_proofs_by_root(peer_id, &actually_missing) { Ok(id) => { debug!( - num_roots = servable.len(), + num_roots = actually_missing.len(), root_bytes, range_bytes, "ProofSync: by-root batch sent" ); self.root_request = Some(ByRootRequest { id, peer_id }); @@ -541,15 +557,16 @@ pub(crate) fn by_root_request_size( /// /// Layout: /// - 20 bytes — fixed header: `start_slot` (8) + `count` (8) + `proof_filters` offset (4) -/// - `proof_filters` — only entries where `existing_proof_types` is non-empty; fully-missing -/// blocks are absent (the peer returns all proof types for them by default). +/// - `proof_filters` — partial entries (some types held, some needed) and complete entries +/// (empty `proof_types` list tells the peer to skip the block). Fully-missing blocks are +/// absent; the peer returns all proof types for them by default. /// -/// `partial_missing` must contain only entries with non-empty `existing_proof_types`. +/// `filter_entries` should contain partial and complete entries only (not fully-missing ones). pub(crate) fn by_range_request_size( - partial_missing: &[MissingProofInfo], + filter_entries: &[MissingProofInfo], num_configured_types: usize, ) -> usize { - let filter_bytes: usize = partial_missing + let filter_bytes: usize = filter_entries .iter() .map(|m| per_identifier_ssz_bytes(m, num_configured_types)) .sum(); From 31a429531baaa1b2430c01c0178fd41d03bc5c0a Mon Sep 17 00:00:00 2001 From: frisitano Date: Tue, 31 Mar 2026 20:28:54 +0200 Subject: [PATCH 3/3] refactor: encapsulate fork state --- beacon_node/lighthouse_network/src/rpc/protocol.rs | 5 ++--- .../src/network_beacon_processor/rpc_methods.rs | 1 + consensus/types/src/core/chain_spec.rs | 12 ++++++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/beacon_node/lighthouse_network/src/rpc/protocol.rs b/beacon_node/lighthouse_network/src/rpc/protocol.rs index 9b730b3b1b2..d5435b1721c 100644 --- a/beacon_node/lighthouse_network/src/rpc/protocol.rs +++ b/beacon_node/lighthouse_network/src/rpc/protocol.rs @@ -585,11 +585,10 @@ impl ProtocolId { LightClientUpdatesByRangeRequest::ssz_min_len(), LightClientUpdatesByRangeRequest::ssz_max_len(), ), - Protocol::MetaData => RpcLimits::new(0, 0), // Metadata requests are empty - // EIP-8025 is Fulu+; use the Deneb/post-Deneb max_request_blocks for proof_filters. + Protocol::MetaData => RpcLimits::new(0, 0), Protocol::ExecutionProofsByRange => RpcLimits::new( ExecutionProofsByRangeRequest::ssz_min_len(), - ExecutionProofsByRangeRequest::ssz_max_len(spec.max_request_blocks(ForkName::Fulu)), + ExecutionProofsByRangeRequest::ssz_max_len(spec.max_request_blocks_upper_bound()), ), // ExecutionProofsByRoot request is List[ProofByRootIdentifier, MAX_BLOCKS_BY_ROOT. Protocol::ExecutionProofsByRoot => RpcLimits::new(0, spec.max_blocks_by_root_request), diff --git a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs index 1758f7f5bd5..b882dffb9b8 100644 --- a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs @@ -1330,6 +1330,7 @@ impl NetworkBeaconProcessor { /// `proof_filters` when present. For blocks listed in `proof_filters`: /// - a non-empty `proof_types` list → serve only those types /// - an empty `proof_types` list → skip the block entirely (requester already has all proofs) + /// /// Blocks absent from `proof_filters` receive all known proof types. pub fn handle_execution_proofs_by_range_request( &self, diff --git a/consensus/types/src/core/chain_spec.rs b/consensus/types/src/core/chain_spec.rs index d3f585199c1..2cb24b3347b 100644 --- a/consensus/types/src/core/chain_spec.rs +++ b/consensus/types/src/core/chain_spec.rs @@ -700,6 +700,18 @@ impl ChainSpec { } } + /// Returns the highest possible value for max_request_blocks based on enabled forks. + /// + /// This is useful for upper bounds where no specific fork context is available, such as + /// computing SSZ size limits for RPC protocols that activate at Deneb or later. + pub fn max_request_blocks_upper_bound(&self) -> usize { + if self.deneb_fork_epoch.is_some() { + self.max_request_blocks_deneb as usize + } else { + self.max_request_blocks as usize + } + } + /// Returns the highest possible value for max_request_blobs based on enabled forks. /// /// This is useful for upper bounds in testing.