apollo_network: KadRequesting emits RequestDial events via DiallingBehaviour

[sirandreww-starkware]

Note

Medium Risk
Changes peer dialing flow in the discovery subsystem by replacing direct swarm dial requests with delegated dial events and new de-duplication state; this can affect connectivity/retry behavior if the new state handling is incorrect.

Overview
Discovery dialing is now delegated via events instead of direct swarm dials. KadRequestingBehaviour now queues Kademlia results as (PeerId, Vec<Multiaddr>) and emits ToOtherBehaviourEvent::RequestDial (with addresses) rather than ToSwarm::Dial.

It adds tracking for peers with an active dial request to avoid repeated dial dispatches, and cleans up pending_dials/active tracking on connection establishment and when set_target_peers resets the target set. discovery::Behaviour is updated to pass the full Kademlia (peer_id, addresses) tuples through and forward RequestDial to DialingBehaviour.

^{Written by Cursor Bugbot for commit 0912f23. This will update automatically on new commits. Configure here.}

[reviewable-StarkWare]

This change is 

[sirandreww-starkware]

• apollo_network: expose add_epoch as a channel-based API on network manager #13389
• apollo_network: add peer access control behaviour to deny unauthorized inbound connections #13285
• apollo_network: cancel stale DialPeerStreams when target peers are removed #13282
• apollo_network: added end to end discovery test #13097
• apollo_network: added testing event exporter to network manager #13242
• apollo_network: moved route_to_other_behaviour_event to mixed behaviour #13096
• apollo_network: migrate BootstrappingBehaviour to delegate dialling to DiallingBehaviour #13219
• apollo_network: KadRequesting emits RequestDial events via DiallingBehaviour #13218 👈 (View in Graphite)
• apollo_network: wire DiallingBehaviour into Discovery with RequestDial event #13217
• apollo_network: propagate multiaddresses from Kad query results #13216
• apollo_network: implement DiallingBehaviour using DialPeerStream via SelectAll #13215
• apollo_network: add DialPeerStream for dialling behaviour #13214
• apollo_network: add DiallingBehaviour skeleton #13213
• apollo_network: changed KadRequesting to dial the peers returned from Kad query #13116
• apollo_network: removed random KadRequests #13095
• apollo_network: pass allowed peers to KadRequestingBehaviour on epoch registration #13094
• apollo_network: using ActiveCommittees in NetworkManager #13075
• apollo_network: add CommitteeStore with read/write API #12382 : 1 other dependent PR (#12583 )
• apollo_network: add committee_manager module with core types #12381
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[guy-starkware]

@guy-starkware reviewed 2 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ShahakShama and sirandreww-starkware).

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

[sirandreww-starkware]

@sirandreww-starkware made 1 comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on guy-starkware and ShahakShama).

[ShahakShama]

@ShahakShama reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on guy-starkware and sirandreww-starkware).

---

crates/apollo_network/src/discovery/behaviours/kad_requesting.rs line 40 at r4 (raw file):

    pending_dials: VecDeque<(PeerId, Vec<Multiaddr>)>,
    /// Peers for which a `RequestDial` event has been dispatched to `DiallingBehaviour` but no
    /// connection has been established yet.

This is incorrect. Peers that a connection has been established with are in this list as well

[sirandreww-starkware]

@sirandreww-starkware made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on guy-starkware and ShahakShama).

---

crates/apollo_network/src/discovery/behaviours/kad_requesting.rs line 40 at r4 (raw file):

Previously, ShahakShama wrote…

This is incorrect. Peers that a connection has been established with are in this list as well

That's not true, we remove the peer once a ConnectionEstablished event is emitted

---

crates/apollo_network/src/discovery/behaviours/kad_requesting.rs line 150 at r3 (raw file):

    // TODO(AndrewL): When target peers change, cancel DialPeerStreams in DiallingBehaviour
    // for peers no longer in the target set. Currently those streams retry indefinitely.

@ShahakShama here

Code quote:

    // TODO(AndrewL): When target peers change, cancel DialPeerStreams in DiallingBehaviour
    // for peers no longer in the target set. Currently those streams retry indefinitely.

[sirandreww-starkware]

@sirandreww-starkware made 1 comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on guy-starkware and ShahakShama).

---

crates/apollo_network/src/discovery/behaviours/kad_requesting.rs line 150 at r3 (raw file):

Previously, sirandreww-starkware (Andrew Luka) wrote…

@ShahakShama here

solved in #13282

[ShahakShama]

@ShahakShama made 1 comment and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on guy-starkware and sirandreww-starkware).

---

crates/apollo_network/src/discovery/behaviours/kad_requesting.rs line 40 at r4 (raw file):

Previously, sirandreww-starkware (Andrew Luka) wrote…

That's not true, we remove the peer once a ConnectionEstablished event is emitted

So you can return to dialing_peers 😅

[sirandreww-starkware]

@sirandreww-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on guy-starkware and ShahakShama).

---

crates/apollo_network/src/discovery/behaviours/kad_requesting.rs line 40 at r4 (raw file):

Previously, ShahakShama wrote…

So you can return to dialing_peers 😅

I don't know, I kind of like it better now (more descriptive)

[sirandreww-starkware]

@sirandreww-starkware resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on guy-starkware).

[guy-starkware]

@guy-starkware reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on sirandreww-starkware).

[graphite-app]

Merge activity

• Mar 17, 8:27 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.