diff --git a/beacon_node/network/src/sync/block_lookups/mod.rs b/beacon_node/network/src/sync/block_lookups/mod.rs index d403382e9e..ef9807f037 100644 --- a/beacon_node/network/src/sync/block_lookups/mod.rs +++ b/beacon_node/network/src/sync/block_lookups/mod.rs @@ -114,6 +114,8 @@ pub(crate) struct BlockLookupSummary { pub block_root: Hash256, /// List of peers that claim to have imported this set of block components. pub peers: Vec, + /// Whether the lookup expects some future event to make progress. + pub is_awaiting_event: bool, } impl BlockLookups { @@ -150,6 +152,7 @@ impl BlockLookups { id: *id, block_root: l.block_root(), peers: l.all_peers(), + is_awaiting_event: l.is_awaiting_event(), }) .collect() } diff --git a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs index 346594c2f5..dbf3604cf0 100644 --- a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs +++ b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs @@ -355,12 +355,16 @@ impl SingleBlockLookup { self.awaiting_parent.is_some() || self.block_request.state.is_awaiting_event() || match &self.data_request { - DataRequest::WaitingForBlock => true, + // Not awaiting an event itself; it's blocked on the block request, already covered + // by the `block_request` term above. Returning `true` kept a peerless lookup parked + // in `AwaitingDownload` from being pruned, so it got stuck. + DataRequest::WaitingForBlock => false, DataRequest::Request { state, .. } => state.is_awaiting_event(), DataRequest::NoData => false, } || match &self.payload_request { - PayloadRequest::WaitingForBlock => true, + // See `data_request` above: not awaiting an event itself, the block request covers it. + PayloadRequest::WaitingForBlock => false, PayloadRequest::Request { state, .. } => state.is_awaiting_event(), PayloadRequest::PreGloas => false, } diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index d84596cf3c..835b7546b3 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -2407,6 +2407,21 @@ async fn peer_disconnected_then_rpc_error(depth: usize) { r.assert_single_lookups_count(1); } +#[tokio::test] +/// A lookup that loses its only peer while still waiting to download the block must not report +/// itself as awaiting an event, else `drop_lookups_without_peers` skips it and it gets stuck. +/// Regression for the "Notify the devs a sync lookup is stuck" report. +async fn peerless_lookup_awaiting_download_is_not_awaiting_event() { + let mut r = TestRig::default(); + r.build_chain_and_trigger_last_block(1).await; + r.disconnect_all_peers(); + r.simulate(SimulateConfig::new().return_rpc_error(RPCError::Disconnected)) + .await; + let lookup = &r.active_single_lookups()[0]; + assert_eq!(lookup.peers.len(), 0); + assert!(!lookup.is_awaiting_event); +} + #[tokio::test] /// Assert that when creating multiple lookups their parent-child relation is discovered and we add /// peers recursively from child to parent.