iroh icon indicating copy to clipboard operation
iroh copied to clipboard

refactor: add `iroh_net::ConnManager` and use in `iroh_gossip::net`

Open Frando opened this issue 1 year ago • 4 comments

Description

  • Introduce a ConnManager in iroh_net. Quoting the docs:

A connection manager that ensures that only a single connection between two peers prevails. You can start to dial peers by calling [ConnManager::dial]. Note that the method only takes a node id; if you have more addressing info, add it to the endpoint directly with [Endpoint::add_node_addr] before calling dial; The [ConnManager] does not accept connections from the endpoint by itself. Instead, you should run an accept loop yourself, and push connections with a matching ALPN into the manager with [ConnManager::handle_connection] or [ConnManager::handle_connection_sender]. The [ConnManager] is a [Stream] that yields all connections from both accepting and dialing. Before accepting incoming connections, the [ConnManager] makes sure that, if we are dialing the same node, only one of the connections will prevail. In this case, the accepting side rejects the connection if the peer's node id sorts higher than their own node id. To make this reliable even if the dials happen exactly at the same time, a single unidirectional stream is opened, on which a single byte is sent. This additional rountrip ensures that no double connections can prevail.

  • Use the ConnManager in iroh_gossip::net and improve connection handling

Breaking Changes

Notes & open questions

Change checklist

  • [ ] Self-review.
  • [ ] Documentation updates if relevant.
  • [ ] Tests if relevant.
  • [ ] All breaking changes documented.

Frando avatar May 21 '24 13:05 Frando

includes functionality to make sure that only a single connection prevails between ourselves and another peer

In plain TCP simultaneous connect from both sides results in a single connection if socket is bound to a port and the other side tries to connect to the same port: https://stackoverflow.com/questions/2231283/tcp-two-sides-trying-to-connect-simultaneously I have looked a bit into QUIC RFC 9000, seems it cannot do anything similar because of connection IDs.

link2xt avatar May 21 '24 19:05 link2xt

Part of me feels this might belong better in the protocol running on top of the provided quic connection rather than in the endpoint layer itself. And to me iroh-net is about the endpoint layer, not the application on top.

I'm a little sceptical that an application does even need support like this. Could it not keep a map of confirmed connections, i.e. when it received data on a connection. Any time a connection receives data for the first time, whether that connection was dialed or accepted, it can check the map and reset the connection if the NodeId already exists. This is like a very opiniated and specific version of this.

flub avatar May 23 '24 11:05 flub

Yes, I'm also not too sure anymore. I thought initially that this could be done without opening any streams, which would put it more into endpoint-layer land. Alas, an extra roundtrip, and thus opening streams, is required for consistent deduplication it seems. Which puts it more into application protocol land.

At the same time it is a quite common usecase for p2p protocols, and offering something easy-to-use would be nice. Sounds a bit like iroh-net-utils or so material.

Frando avatar May 23 '24 12:05 Frando

I agree something like this is needed. But - how will this play together with the new custom protocols stuff?

The custom protocol handler uses the standard endpoint accept. So maybe this needs to be integrated more deeply into iroh-net or at least into the iroh node?

rklaehn avatar Jun 28 '24 11:06 rklaehn

Closing this PR for now. We'll iterate in userland, and maybe create an iroh-util crate post-1.0. FWIW iroh-blobs now also contains a different version of a connection pool: https://github.com/n0-computer/iroh-blobs/blob/main/src/util/connection_pool.rs

Frando avatar Sep 30 '25 11:09 Frando