fix(network): gate find-response stall tracking on per-peer height

CHANGELOG.md

@@ -54,6 +54,12 @@ and this project adheres to [Semantic Versioning](https://semver.org).
   never selected as a reconnection candidate. Previously an entry on a different port could survive
   the ban and occupy the first candidate slot until the node restarted
   ([#11134](https://github.com/ZcashFoundation/zebra/issues/11134)).
+- Empty `FindBlocks` and `FindHeaders` responses are now only counted against peers
+  that advertised a height above the local tip in their handshake, instead of being
+  counted whenever a wall-clock estimate reports the node as far from the network
+  tip. Peers at or below the local tip are no longer disconnected when the local
+  tip is stale (mining paused, or the whole network parked at a shared tip)
+  ([#10910](https://github.com/ZcashFoundation/zebra/pull/10910)).
 
 ### Security
 

zebra-network/CHANGELOG.md

@@ -33,6 +33,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   never returns an address whose IP is banned. Previously an entry for the banned IP on another
   port could survive the ban and stay at the front of the reconnection order for the lifetime of
   the process ([#11134](https://github.com/ZcashFoundation/zebra/issues/11134)).
+- `FindBlocks`/`FindHeaders` stall tracking is now gated per peer on the height the peer
+  advertised in its version handshake instead of a wall-clock estimate of the node's distance to
+  the network tip, so peers at or below a stale local tip are no longer disconnected
+  ([#10910](https://github.com/ZcashFoundation/zebra/pull/10910)).
 
 ### Security
 

zebra-network/src/peer/client/tests.rs

@@ -56,6 +56,7 @@ impl ClientTestHarness {
     pub fn build() -> ClientTestHarnessBuilder {
         ClientTestHarnessBuilder {
             version: None,
+            start_height: None,
             connection_task: None,
             heartbeat_task: None,
             connected_addr: None,
@@ -253,6 +254,7 @@ pub struct ClientTestHarnessBuilder<C = future::Ready<()>, H = future::Ready<()>
     heartbeat_task: Option<H>,
     version: Option<Version>,
     connected_addr: Option<ConnectedAddr>,
+    start_height: Option<Height>,
 }
 
 impl<C, H> ClientTestHarnessBuilder<C, H>
@@ -272,6 +274,12 @@ where
         self
     }
 
+    /// Configure the mocked handshake start height reported by the peer.
+    pub fn with_start_height(mut self, start_height: Height) -> Self {
+        self.start_height = Some(start_height);
+        self
+    }
+
     /// Configure the mock connection task future to use.
     pub fn with_connection_task<NewC>(
         self,
@@ -282,6 +290,7 @@ where
             heartbeat_task: self.heartbeat_task,
             version: self.version,
             connected_addr: self.connected_addr,
+            start_height: self.start_height,
         }
     }
 
@@ -295,6 +304,7 @@ where
             heartbeat_task: Some(heartbeat_task),
             version: self.version,
             connected_addr: self.connected_addr,
+            start_height: self.start_height,
         }
     }
 
@@ -329,7 +339,7 @@ where
             ),
             nonce: Nonce::default(),
             user_agent: "client test harness".to_string(),
-            start_height: Height(0),
+            start_height: self.start_height.unwrap_or(Height(0)),
             relay: true,
         };
 

zebra-network/src/peer/load_tracked_client.rs

@@ -12,6 +12,8 @@ use tower::{
     Service,
 };
 
+use zebra_chain::block;
+
 use crate::{
     constants::{EWMA_DECAY_TIME_NANOS, EWMA_DEFAULT_RTT},
     peer::{Client, ConnectedAddr, ConnectionInfo},
@@ -65,6 +67,17 @@ impl LoadTrackedClient {
                 if canonical_socket_addr(addr.remove_socket_addr_privacy()).ip() == expected_ip
         )
     }
+
+    /// Retrieve the block height the peer reported in its version handshake.
+    ///
+    /// This is the peer's best height at the time it connected. It is not updated as the peer
+    /// syncs, so it under-reports a peer that advanced after handshake. For stall detection this
+    /// errs toward not tracking: it can stop tracking a peer early (missing a stall), but never
+    /// falsely tracks a peer that is not ahead of us (see
+    /// [`PeerSet::route_p2c`](crate::peer_set::PeerSet::route_p2c)).
+    pub fn remote_start_height(&self) -> block::Height {
+        self.connection_info.remote.start_height
+    }
 }
 
 impl<Request> Service<Request> for LoadTrackedClient

zebra-network/src/peer_set/set.rs

@@ -125,7 +125,11 @@ use tower::{
     Service,
 };
 
-use zebra_chain::{chain_tip::ChainTip, parameters::Network};
+use zebra_chain::{
+    block,
+    chain_tip::{ChainTip, AT_OR_NEAR_TIP_THRESHOLD},
+    parameters::Network,
+};
 
 use crate::{
     address_book::AddressMetrics,
@@ -1046,16 +1050,31 @@ where
                 &req,
                 Request::FindBlocks { .. } | Request::FindHeaders { .. }
             );
-            let is_syncing = || {
-                !self
+            // Only penalise empty or failed find responses from a peer that advertised a height
+            // meaningfully above our own tip. A peer at or below our tip legitimately has nothing
+            // beyond it to send, so its empty responses are expected — including when our own tip
+            // is stale (mining paused, or the whole network parked at a shared tip), where
+            // extrapolating the distance to the network tip from wall-clock time (as
+            // `ChainTip::is_at_or_near_network_tip` does) would wrongly report us as still syncing
+            // and re-arm the stall tracker. Deciding per-peer, rather than from a single global
+            // tip estimate, also means a peer lying high about its height can only get itself
+            // dropped, never an honest peer. The trade-off: a peer claiming a height at or below
+            // our tip is never tracked, so its empty responses go unpunished; #11080 tracks
+            // measuring per-peer contribution instead, which closes that gap.
+            let peer_claims_ahead = || {
+                let our_tip = self
                     .minimum_peer_version
                     .chain_tip()
-                    .is_at_or_near_network_tip(&self.network)
+                    .best_tip_height()
+                    .unwrap_or(block::Height(0));
+                svc.remote_start_height() - our_tip > AT_OR_NEAR_TIP_THRESHOLD
             };
-            // zcashd-compat sidecars are exempt: they sync *from* this node,
-            // so they can legitimately trail it without being stalled peers.
-            let track_stalls =
-                is_find_request && !self.zcashd_compat_peer_keys.contains(&p2c_key) && is_syncing();
+            // zcashd-compat sidecars are exempt: they sync *from* this node, so they can
+            // legitimately return nothing — including when their handshake height exceeds a fresh
+            // local tip before this node loads its state.
+            let track_stalls = is_find_request
+                && !self.zcashd_compat_peer_keys.contains(&p2c_key)
+                && peer_claims_ahead();
 
             let fut = svc.call(req);
             self.push_unready(p2c_key, svc);

zebra-network/src/peer_set/set/tests.rs

@@ -110,6 +110,31 @@ impl PeerVersions {
     }
 }
 
+/// Build a single mock peer with the given protocol version and advertised handshake start
+/// height, exposed as a [`Discover`]-compatible stream (like
+/// [`PeerVersions::mock_peer_discovery`]).
+///
+/// The stall-tracker tests use this to control the peer's reported height relative to the local
+/// tip, which is what the `route_p2c` stall gate keys on.
+fn mock_peer_discovery_with_start_height(
+    version: Version,
+    start_height: block::Height,
+) -> (
+    impl Stream<Item = Result<Change<PeerSocketAddr, LoadTrackedClient>, BoxError>>,
+    Vec<ClientTestHarness>,
+) {
+    let (client, harness) = ClientTestHarness::build()
+        .with_version(version)
+        .with_start_height(start_height)
+        .finish();
+
+    let peer_address: PeerSocketAddr = SocketAddr::new([127, 0, 0, 1].into(), 1).into();
+    let discovered_peers =
+        stream::iter([Ok(Change::Insert(peer_address, client.into()))]).chain(stream::pending());
+
+    (discovered_peers, vec![harness])
+}
+
 /// A helper builder type for creating test [`PeerSet`] instances.
 ///
 /// This helps to reduce repeated boilerplate code. Fields that are not set are configured to use