Skip to content

fix(network): ignore stale Zakura peer teardowns#486

Closed
p0mvn wants to merge 2 commits into
split/pr458-header-root-validationfrom
split/pr458-duplicate-peer-teardown
Closed

fix(network): ignore stale Zakura peer teardowns#486
p0mvn wants to merge 2 commits into
split/pr458-header-root-validationfrom
split/pr458-duplicate-peer-teardown

Conversation

@p0mvn

@p0mvn p0mvn commented Jul 6, 2026

Copy link
Copy Markdown

Summary

  • Carry connection registration ids into Zakura service sessions.
  • Ignore stale disconnect events from displaced duplicate connections so they cannot deregister the newer live peer.

Test plan

  • cargo fmt --all -- --check
  • cargo test -p zebra-network deliver_rootless_solicited_headers_does_not_disconnect_the_peer

Stack

Depends on the header-root validation PR.

AI disclosure

Used Cursor to split PR #458 into stacked branches and prepare this PR.

p0mvn added 2 commits July 6, 2026 05:34
Carry connection registration ids into service sessions so a displaced duplicate connection cannot deregister the newer live peer when its old teardown runs.
Update the header-sync service and tests for registration-aware disconnect events used by stale teardown filtering.
@v12-auditor

v12-auditor Bot commented Jul 6, 2026

Copy link
Copy Markdown

Note

Complete: Audit complete. V12 found two issues worth reviewing.

Open the full results here.

FindingSeverityDetails
F-96424 🟡 Medium
Unusable retries grow queues

The new unusable-response backoff helper requeues failed ranges through a path that does not deduplicate scheduler queues. Rootless non-empty and empty Headers responses call delay_unusable_response_retry, which stores the same OutstandingRange on the peer with clear_assignment_on_timeout: true. When the short backoff expires, handle_timeouts clears the assignment and calls schedule.retry(range). RangeScheduler::retry unconditionally push_fronts the range, while pop_assignable leaves a range in the queue until all fanout slots are assigned. With fewer than the full fanout of responsive peers, every empty or rootless retry cycle adds another copy of the same uncovered range to the scheduler queue.

F-96425 🟡 Medium
Stale duplicate teardown can remove replacement sync sessions

Duplicate-connection arbitration assigns a new conn_id, but teardown for displaced sessions still removes admitted services before supervisor.deregister() proves that the cleanup belongs to the current registration. Both sync services then translate that stale removal into lifecycle disconnects that are not safely scoped to the torn-down generation: header sync emits PeerDisconnected { registration_id: None }, while block sync emits PeerDisconnected(peer) with no generation at all. Because those events are sent after the service-local maps release their locks, a replacement session can be inserted and announced before the stale disconnect reaches the reactors. The header-sync reactor only rejects stale events when they carry Some(registration_id), and the block-sync reactor never has generation data to compare, so each reactor can tear down state for the winning replacement. In block sync, this also leaves the replacement stranded in the service-local peer map, where it still counts against bounded admission slots even though reactor and registry state have been removed.

Analyzed eight files, diff ee09881...57c5b0c.

@p0mvn p0mvn closed this Jul 6, 2026
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.

1 participant