fix(transport): record and drop a peer's user agent under its connection entry - #163
Conversation
…ion entry A peer's user agent lived in its own map, written after the peer's entry in `peer_to_channel` was created and removed after that entry was dropped. Between the two steps the maps disagreed: - a registration could land after removal dropped the peer's entry but before it dropped the agent, and the removal then deleted the agent the new connection had just recorded, leaving a connected peer with no user agent until it next disconnected; - a reader could find a connected peer with no agent recorded yet. Both writes now happen while holding the peer's `peer_to_channel` entry, on registration and on both removal paths (`remove_channel_mappings_static` and `disconnect_peer`), so a connected peer always has its user agent and `None` from `peer_user_agent` means the peer is not connected. The lock order is unchanged in kind: the `peer_to_channel` entry is taken first and the agent map inside it, and nothing takes them the other way round. ant-node's migration signal counts peers by reading `connected_peers()` and then each peer's agent; a peer left with no agent is counted as a node that never reported, which is how a live peer could stay in that count indefinitely.
With the agent recorded and dropped under the peer's connection entry, a peer from the connected_peers snapshot with no agent has disconnected since the snapshot; it is not waiting on its identity announce, and no PeerConnected will follow for it. The doc comment and both log lines said otherwise.
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
APPROVE — reviewed at exact head 13c204cfb9d857d24f6cb6597424a82c59582089.
The peer/user-agent invariant is now established in the right place. On first registration the user-agent is inserted before the peer_to_channel entry becomes visible; on final-channel removal and explicit disconnect it is removed while the same peer_to_channel entry lock is still held. Readers of the connection map therefore cannot observe a connected entry without its agent. I found no reverse peer_user_agents → peer_to_channel acquisition, so the new nested locking does not introduce a deadlock cycle.
The previously disclosed channel_to_peers lifecycle/event-ordering races are pre-existing and are not widened by this change. The DHT reconciliation wording matches the resulting operational semantics.
Verification:
cargo test --lib: 535 passed- focused authenticated-registration invariant test: passed
cargo clippy --all-features -- -D warnings -D clippy::unwrap_used -D clippy::expect_used: passedcargo fmt --check: passed- GitHub build/test/lint matrix is green
Non-blocking gap: there is no deterministic connect/disconnect race regression test. The existing registration test covers visibility before event consumers, and the lock/interleaving analysis is sound, so I do not consider the missing stress test a merge blocker.
The failing Security Audit is inherited, not introduced: Cargo.lock is byte-identical to the base and the newly published RUSTSEC-2026-0285 affects that existing rustls version. It should be handled by the release train in a separate dependency bump to rustls >=0.23.45.
3 participants
Linear issue
Closes V2-1260
Risk tier
Moves the user-agent writes inside a lock the transport already takes, and corrects the DHT reconciliation wording that described the old behaviour. No wire, format or routing change.
Compatibility
peer_user_agentkeeps its signature; its doc now says a connected peer always has an agent, soNonemeans not connected.Semver impact
Test evidence
cargo test --lib: 535 passed, 0 failed.cargo clippy --all-features -- -D warnings -D clippy::unwrap_used -D clippy::expect_used(the CI invocation) andcargo fmt --check: clean.peer_to_channelentry.PeerDisconnected, andPeerDisconnectedis sent after the peer's entry is released, so a quick reconnect can emitPeerConnectedfirst. Neither makes a connected peer lose its agent.New dependency
none
ADR
https://github.com/WithAutonomi/ant-node/blob/rc-2026.9.2/docs/adr/ADR-0014-file-based-chunk-store-and-lmdb-retirement.md (the migration signal that reads peer user agents; no saorsa-core ADR covers this bookkeeping)
Mitigation / rollback
Revert the commit. Nothing is persisted, and ant-node's count already tolerates a peer with no agent.