Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Waku Peer Store: Integrated libp2p peer store #622

Closed
jm-clius opened this issue Jun 14, 2021 · 3 comments · Fixed by #1383
Closed

Waku Peer Store: Integrated libp2p peer store #622

jm-clius opened this issue Jun 14, 2021 · 3 comments · Fixed by #1383
Assignees

Comments

@jm-clius
Copy link
Contributor

jm-clius commented Jun 14, 2021

Background

Waku v2 currently defines its own peer store and manager. nim-libp2p has recently implemented a peer store that replicates much of the functionality provided by the nwaku peer store.

Proposed solution

Waku needs to integrate and use the libp2p peer store in the peer manager, where possible, and extend the store for its self-defined books (such as the ConnectionBook and DisconnectBook - both nwaku-specific).

Some proposals for what the Waku Peer Store should support:

  • a generic interface through which peers can be added, although the source can be tracked (e.g. peers added manually vs peers added via one of the discovery methods)
  • peer selection mechanism for each protocol (prioritising manually-added peers). Peer scoring will eventually play a role here.
  • peer persistence (while providing enough information for the connection manager to respect gossipsub rules here, e.g. the mandatory backoff after a PRUNE before reconnecting)
  • proper housekeeping (stale peers which CannotConnect should eventually be removed)

This store should be the basis from which a separate connection manager can:

  • select the best peers to maintain continuously a rich set of relay connections
  • 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. This could benefit from making these config items repeatable (we currently only support a single service node per protocol).
  • update stored peer information with information coming from the connection (e.g. the disconnection time, supported protocols from identify, etc.)
@jm-clius
Copy link
Contributor Author

Updating milestone from Release v0.11 to next release (v0.12) to limit scope/risk.

@jm-clius jm-clius modified the milestones: Release 0.12, Release 0.13 Sep 2, 2022
@jm-clius jm-clius added this to Waku Sep 2, 2022
@jm-clius jm-clius moved this to Todo in Waku Sep 2, 2022
@jm-clius jm-clius changed the title Integrate libp2p peer store Waku Peer Store: Integrated libp2p peer store Oct 24, 2022
@jm-clius jm-clius modified the milestones: Release 0.13.0, Release 0.14.0 Oct 24, 2022
@alrevuelta alrevuelta self-assigned this Nov 4, 2022
@alrevuelta
Copy link
Contributor

As I see it, while the title is "Waku Peer Store" we are talking about two different things:

  • PeerStore: Stores peers and their information: multiaddr, id, source, connection state, etc. With the capability to persist it, useful across restarts.
  • PeerManager: Takes care of connecting to new peers (i.e. choosing which one) and disconnecting if max connections were reached (chose which one to disconnect from). Perhaps also cleaning the PeerStore and removing stale peers.

Peer Store

Focusing on PeerStore, I would divide this issue into the following subtasks:

  • Reuse nim-libp2p custom PeerBook adding our specific fields (see 27/WAKU2-PEERS)
  • Remove duplicated fields that were recently added to nim-libp2p, such as AddressBook or ProtoBook
  • Add new field tracking peer origin [discv5, bootstrap, ?] + modify spec tracked peer metadata.
  • Add new field tracking if a peer is blacklisted, which involves agreeing on a metric/action to consider that a peer is blacklisted.
  • TBD: Add a new score field?

Since some changes might be enough generic for nim-libp2p, I would suggest some changes in their repo and reuse as much as we can.

Peer Manager

I would track everything related to peer management in a new issue, refer to #1353

peer selection mechanism for each protocol (prioritising manually-added peers). Peer scoring will eventually play a role here.

As I see it this belongs to PeerManager, so I would track it in a separate issue.

proper housekeeping (stale peers which CannotConnect should eventually be removed)

Perhaps trimming peers should be a task of the PeerManager? Also I'm not sure its a good idea to use CannotConnect as criteria, since a peer can reject a connection because it reached its maximum connections, but later on, it might be willing to accept one.

@jm-clius
Copy link
Contributor Author

jm-clius commented Nov 9, 2022

Thanks for this proper breakdown, @alrevuelta. I agree with the separation between peer store and peer manager. Of course, even though it may logically reside in the same module, do you see further scope to separate Peer Management from Connection Management? For example, the peer manager could decide to keep quite a bit more peers than we allow for in the connection manager, whose main job would be to maintain a healthy set of connections. I'm happy for this to be tracked as a single task/module, but logically I think there's a difference between peer housekeeping and connection housekeeping.

TBD: Add a new score field?

Indeed TBD. I think we can track but leave peer scoring as future work for now. Could become quite complex if factors from different layers (App, Waku, libp2p) needs to be considered.

Perhaps trimming peers should be a task of the PeerManager?

Yes, I think so. Although we may want to trim using different rules than for trimming connections (open to be persuaded otherwise, btw). IMO storing peers is a cheaper exercise than maintaining some set of connections.

Also I'm not sure its a good idea to use CannotConnect as criteria

True. It's quite naive currently. One idea would be for this state (and possibly other non-connected states) to decay over time, so that peers become eligible for reconnection attempts.

Will also comment on peer management issue.

Repository owner moved this from In Progress to Done in Waku Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants