Skip to content

refactor(swarm): add future support for handle_pending_outbound_connection#6492

Draft
dariusc93 wants to merge 4 commits into
libp2p:masterfrom
dariusc93:refactor/concurrent-handle-pending-outbound
Draft

refactor(swarm): add future support for handle_pending_outbound_connection#6492
dariusc93 wants to merge 4 commits into
libp2p:masterfrom
dariusc93:refactor/concurrent-handle-pending-outbound

Conversation

@dariusc93

Copy link
Copy Markdown
Member

Description

This PR changes the return signature of NetworkBehaviour::handle_pending_outbound_connection to use OutboundAddresses, which via OutboundAddresses::Pending can pass a future that is polled by Swarm::poll_next_event and resolved later for dialing, while OutboundAddresses::Ready allows passing the previous result used for addresses that are already available (or to deny a connection).

AI Assistance Disclosure

Tools used (required — write none if no AI was used): none

Attestation (required):

  • I have read every line of this diff, understand what it does, and can explain it in review.

Notes & open questions

This PR is just a PoC (which I had a similar design last year, but ended up rewriting it this time around), to mainly act as a playground to see how it would look and possibly open up discussions on how we could possibly move forward with a similar (if not the same design) in the future, which could possibly scale up to looking into similar implementations for libp2p-kad record store (since we are purely limited to just a memory store there this time), etc. I did add a small example that uses sqlite (via sqlx) as a means to store a peer address and be able to dial the peer, purely acting as a demo for the change.

Originally, I thought about using impl Future (which in the past, I used a type PendingOutbound that takes one, but now we could use it on the trait), but while we could use Ready to instantly return the addresses, that is kinda a waste to poll them as well as other futures when you could return them instantly, hence OutboundAddresses::{Ready,Pending}, although I am open for thoughts and opinions on what might be beneficial.

Motivation is largely due to wanting to access addresses of peers from other datastores that isnt stored in memory. This could be on something like sqlx, redb, files (would use something like tokio or some equiv that spawn a thread in the background for it). While one could possibly store them in memory, it becomes a bit wasteful especially if you dont dial those peers for a long time and want to evict them from memory but without forgetting the address, but if you were to dial them without knowing the address, you would want access to that data.

I dont expect to see something like this in rust-libp2p, but it would be nice to have.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • A changelog entry has been made in the appropriate crates

@jxs

jxs commented Jun 22, 2026

Copy link
Copy Markdown
Member

Hi Darius, thanks for this, it's quite interesting. We can discuss it, wdyt of the idea @elenaf9?
cc @drHuangMHT

@drHuangMHT drHuangMHT 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.

It is a feature, but also contains breaking change. Are we planning to ship this directly to downstream users in the next major release?

Comment thread swarm/src/lib.rs
// across a `Deref`.
let this = &mut *self;

// This loop polls the components below in a prioritized order.

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.

might as well document the polling priority here? not necessary, but do weigh the priority among other jobs.

@dariusc93

Copy link
Copy Markdown
Member Author

It is a feature, but also contains breaking change. Are we planning to ship this directly to downstream users in the next major release?

I dont expect this to be in the next upcoming major release (0.57), and honestly, if it were to be, we might want to consider a different name for the trait member and deprecate handle_pending_outbound_connection so users arent blind side by the change. I originally used resolve_pending_outbound_connection before going back to the original name (since it was more of a placeholder) just to keep the naming more consistent.

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