Skip to content

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

Draft
upbqdn wants to merge 2 commits into
mainfrom
fix/find-stall-stale-tip
Draft

fix(network): gate find-response stall tracking on per-peer height#10910
upbqdn wants to merge 2 commits into
mainfrom
fix/find-stall-stale-tip

Conversation

@upbqdn

@upbqdn upbqdn commented Jul 5, 2026

Copy link
Copy Markdown
Member

Motivation

Follow-up to #10732. That PR stops the find-response stall tracker from disconnecting peers at the chain tip, but gates it on ChainTip::is_at_or_near_network_tip, which estimates the distance to the network tip purely from the local tip block's timestamp. When the local tip is stale — mining paused on a test network, or the whole network parked at a shared tip — that wall-clock estimate reports the node as thousands of blocks behind, so the gate re-arms the stall tracker and starts disconnecting healthy at-tip peers again. On a network where every node runs this code, a stall at a shared tip re-arms every node's tracker and they mutually disconnect, which can keep a cohort from catching up to a peer that is ahead.

#11080 tracks the complementary half of this problem (what counts as a stall — measured per-peer contribution instead of response non-emptiness) and is designed to land on top of this PR, which fixes when a peer is eligible for stall tracking.

Solution

Gate the stall tracker per-peer on the peer's advertised handshake height versus our own tip, instead of a single wall-clock estimate. An empty or timed-out find response is only counted against a peer that advertised a height above our tip — a peer that claimed to have blocks and then delivered nothing. A peer at or below our tip legitimately has nothing to send, so its empty responses are never penalised, regardless of how old our tip is.

  • LoadTrackedClient::remote_start_height() exposes the peer's version-handshake height (already stored in connection_info.remote).
  • PeerSet::route_p2c tracks stalls only when remote_start_height() - our_tip > AT_OR_NEAR_TIP_THRESHOLD.

This is robust to a stale local tip, preserves the GHSA-h9hm-m2xj-4rq9 property (a peer that promises height during real sync but only returns empty responses is still dropped after the threshold), and is DoS-resistant: deciding per-peer means a peer lying high about its height can only get itself dropped, never an honest peer — unlike a global "max advertised height" gate. The wire start_height is an arbitrary u32, and Height - Height subtracts exactly in i64, so a hostile extreme value cannot overflow. start_height is captured at handshake and not updated, which only ever makes us stop tracking slightly early near the tip, never falsely disconnect a good peer — including a peer we have since outrun.

Security trade-off (for explicit sign-off)

The per-peer gate narrows the tracker's reach in one attacker-selectable way: a peer that advertises a height at or below our tip is never tracked, so it can serve empty find responses indefinitely without being struck (on main, any peer was tracked whenever the wall-clock estimate said "syncing"). Related: once our tip passes a connection's frozen handshake height, tracking stays off for that connection's lifetime. Both are the deliberate cost of fixing the false-positive direction — the wall-clock gate disconnects every healthy peer on a parked network, which is strictly worse, and an untracked useless peer costs sync only a retry. The residual gap (peers contributing nothing while staying connected) is exactly what #11080's contribution-based accounting is scoped to close on top of this PR.

Interaction with #10952 (zcashd-compat sidecars)

The sidecar exemption from #10952 is preserved and layered with the new gate: track_stalls = is_find_request && !zcashd_compat && peer_claims_ahead(). The exemption still matters under the per-peer gate: a sidecar's handshake height can exceed a fresh local tip before this node loads its state.

Tests

  • find_blocks_stall_tracked_when_syncing: peer advertises a height above our tip → still tracked and disconnected after the threshold (GHSA property preserved), even with the wall-clock estimate set to "at tip".
  • find_blocks_stall_not_tracked_when_at_tip: peer at our tip, with the wall-clock estimate set to 100_000 to simulate a stale/frozen tip → not tracked, peer stays connected. This is the direct regression test for the gap above: under main's wall-clock gate this scenario tracks and disconnects the peer.
  • find_blocks_stall_tracked_when_tip_unknown and find_blocks_stall_count_preserved_across_tip_transition updated to drive the peer's advertised height (new ClientTestHarnessBuilder::with_start_height plus a single-peer discovery helper).
  • find_blocks_stall_not_tracked_for_zcashd_compat (from feat(zebrad): add zcashd-compat mode with a supervised zcashd wallet sidecar #10952) now gives the sidecar a handshake height above the local tip, so it arms the per-peer gate and the sidecar exemption is what the test exercises — it would otherwise pass vacuously under the new gate.
  • New find_blocks_stall_gate_boundary: a peer exactly AT_OR_NEAR_TIP_THRESHOLD above our tip is not tracked; one block further is tracked and disconnected. Mutation-checked: flipping the gate's > to >= fails this test (the rest of the suite passes that mutation silently).

Specifications & References

AI Disclosure

PR Checklist

Copilot AI review requested due to automatic review settings July 5, 2026 00:32
@upbqdn upbqdn self-assigned this Jul 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

upbqdn added 2 commits July 24, 2026 18:09
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants