Networking MVP: Refactor + extend functionality

[alrevuelta]

Coming from #622.

Networking MVP Trackers

• feat(networking): add backoff period after failed dial #1462
• feat(networking): add service slots to peer manager #1473
• feat(networking): integrate autonat service to request if we are dialable #1472
• refactor(networking): wait for all futures together in connectToNodes #1471
• feat(networking): add relay connectivity loop #1482
• feat(networking): prune peers from peerstore exceeding capacity #1513
• chore: improve relayConnectivityLoop to make sure in/out conns are relay conns #1566
• feat(networking): Keep given amount inbound/outbound peers. #1495
• feat: curate peers shared over peerexchange #1521
• refactor(networking): unify peer data models, remove StoredInfo #1597
• refactor(px): refactor peer exchange + tests #1527
• chore(networking): decouple discovery, add peers and connect to them #1463
• refactor(networking): peermanager refactor and cleanups #1539
• feat: limit relay connections below max conns #1813
• chore(networking): disconnect due to colocation ip in conn handler #1821
• feat: integrate gossipsub scoring #1756
• feat: blacklist peers sharing more than x IPs #1749

@jm-clius @kaiserd thoughts?

[jm-clius]

select peers for service protocols (e.g. store, filter, lightpush), while prioritising peers that were manually added for these protocols (e.g. via --storenode, --filternode, --lightpushnode) options

Note that this was a previous requirement. Recently we have a proposed API change where service peers can be explicitly included in API calls for service protocols (implying that they must be managed/kept by the application). This would obsolete the --storenode, --filternode, etc. options.

maintain the connected peers in between that thresholds

Related to previous comment, but perhaps worthwhile separating this idea between maintaining managed peers & maintaining connections (i.e. the conn manager should/could have some backup peer options when a connection dies)

We currently discover and connect to all nodes naively (see runDiscv5Loop). We need i) to filter according to some criteria and ii) to decouple discovery from connecting.

Agree on both points, although (i) may just be "supports Waku" as a start.

AFAIK we have no logic for handling peer disconnection

Not sure I follow? Max peer connections is a nim-libp2p setting and peer disconnects are handled there and don't count towards the total connections. We've also registered a callback to handle disconnects on our side and update the state in the current Waku Peer Store.

We may want to add an inbound/outbound balance

Indeed. Mentioned before, but worthwhile to also look at what has been done for nimbus.

We also may want to keep a given balance of connected peers with certain capabilities. i.e. we want that x% of our connections are to peers that support store protocol.

Yes! :)

The PeerManager should also prune peers from the PeerStore from time to time, according to some criteria.

Indeed. And this does indeed seem like you're considering connection management and peer management as logically separate (apologies for being unclear up to this point :D ). Sorting can be very naive in the beginning (what little we have of scoring, source, ability to connect, etc.) and then we just periodically prune the ones at the bottom when we reach capacity.

[jm-clius]

Also cc @LNSD in terms of peer management design requirements from his POV.

[alrevuelta]

Related to previous comment, but perhaps worthwhile separating this idea between maintaining managed peers & maintaining connections (i.e. the conn manager should/could have some backup peer options when a connection dies)

My initial thought was to have both peer-management and connection-management components as part of the PeerManager, at least for the first iteration. These are my definitions:

• peer-management: manages the peerstore by removenig peers that don't meet some criteria, like not supporting waku, missbehaving, or simply because there is no more space. We can't store infinite peers.
• connection-management: manages the connections, trying to keep the number of connection between a min and a max. When low on connections it finds a peer from the peerstore and connects to it. It also allows to directly dial a given peer and has to handle disconnection from another if we already reached the max number of connections. It also updates the peerstore with connection related metrics (such as the existing Connectedness). In general, tries to keep healthy connections, monitoring also existing ones and dropping according to some criteria.

AFAIK we have no logic for handling peer disconnection

Not sure I follow? Max peer connections is a nim-libp2p setting and peer disconnects are handled there and don't count towards the total connections. We've also registered a callback to handle disconnects on our side and update the state in the current Waku Peer Store.

My bad, I'm refering to peer disconnections "on purpose". "triggering" instead of "handling". So disconnections that we explicitly perform, because i) we are no longer interested in that peer or ii) we reached the maximum connections and we want to connect to a given peer. Perhaps ii) is too specific.

And this does indeed seem like you're considering connection management and peer management as logically separate

Yep, I consider them as logically separate but right now I think they can go in the same module.

[fryorcraken]

@alrevuelta there seems to be only one TODO remaining. Is that accurate? When is the work planned?

[alrevuelta]

@fryorcraken The TODO was an old one, removing it. I can consider this as complete since it provides the functionalities that were planed. More peer management and improvements on networking will be added in the future, but they will be tracked with their own issue.