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

PR #10732 stops the find-response stall tracker from disconnecting peers
at the chain tip, but gates it on `is_at_or_near_network_tip`, which
estimates the distance to the network tip from the local tip block's
timestamp. A stale tip — mining paused, or the whole network parked at a
shared tip — makes that estimate report the node as far behind, re-arming
the tracker and disconnecting healthy at-tip peers again.

Gate the tracker per-peer on the peer's advertised handshake height versus
our tip instead: only penalise empty or failed find responses from a peer
that claimed a height above ours. This is robust to a stale tip, preserves
the GHSA-h9hm-m2xj-4rq9 property (a peer that promises height but delivers
nothing is still dropped during real sync), and 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 —
is closed by the contribution-based accounting tracked in #11080.

The zcashd-compat sidecar exemption from #10952 is preserved; its stall
test now arms the per-peer gate so the exemption is what it exercises.

Author: upbqdn

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