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

misc: add a PeerStore behaviour #4103

Open
thomaseizinger opened this issue Jun 22, 2023 · 8 comments
Open

misc: add a PeerStore behaviour #4103

thomaseizinger opened this issue Jun 22, 2023 · 8 comments

Comments

@thomaseizinger
Copy link
Contributor

Description

Other implementations have the concept of a PeerStore, should we add one to rust-libp2p too? I am thinking of a NetworkBehaviour implementation tracks currently connected peers and allows to add / remove addresses.

See related discussions:

Motivation

Users should be able to query the connected peers in a simple way and add known addresses of peers that will contribute to establishing new connections.

Open questions

  • Are the two topics connected-peers and adding new addresses related enough to merge them in one behaviour?
  • Is storing addresses in-memory fine for now?
  • Should we add a persistent address book as well? e.g. an SQL-based one?

Are you planning to do it yourself in a pull request?

No

@mxinden
Copy link
Member

mxinden commented Jun 23, 2023

In general I am in favor of a PeerStore NetworkBehaviour implementation.

In addition to the above, I suggest adding a new variant to libp2p_swarm::behaviour::ToSwarm, allowing a NetworkBehaviour to inform other NetworkBehaviours of addresses of peers it discovered. Similar to how we have ToSwarm::NewExternalAddressCandidate, but for addresses of remote peers instead of addresses of the local node. libp2p-identify and libp2p-kad can emit these events and PeerStore can consume these events.

We could then deprecate libp2p-identify's discovered_peers cache.

/// The addresses of all peers that we have discovered.
discovered_peers: PeerCache,

@thomaseizinger
Copy link
Contributor Author

Good idea.

That is orthogonal though, correct? We can ship an initial peer store without introducing such a variant (which is a breaking change).

A ToSwarm::NewPeerAddress event should likely also be consumed by libp2p-kad to extend the routing table.

@dariusc93
Copy link
Member

Should be simple to do from the behaviour standpoint. Maybe we could do a trait similar to libp2p-kad store that can be used for others to implement their own backend besides the memory backend. Also, I think storing in memory is fine for now, which is what I usually do although storing it elsewhere (eg database, on-disk, etc) is a nice-to-have solution down the road, but dont think it really fits the scope of rust-libp2p.

@thomaseizinger
Copy link
Contributor Author

The question is, is it worth doing the abstraction of a trait and deal with all the issues of e.g. futures that need to run on a specific executor (thinking tokio-postgresql etc) over just requiring people to implement another NetworkBehaviour for it.

@mxinden
Copy link
Member

mxinden commented Aug 18, 2023

In addition to the above, I suggest adding a new variant to libp2p_swarm::behaviour::ToSwarm, allowing a NetworkBehaviour to inform other NetworkBehaviours of addresses of peers it discovered. Similar to how we have ToSwarm::NewExternalAddressCandidate, but for addresses of remote peers instead of addresses of the local node. libp2p-identify and libp2p-kad can emit these events and PeerStore can consume these events.

Tracked in #4302.

@thomaseizinger
Copy link
Contributor Author

Also, I think storing in memory is fine for now, which is what I usually do although storing it elsewhere (eg database, on-disk, etc) is a nice-to-have solution down the road, but dont think it really fits the scope of rust-libp2p.

Another thing to consider is that reading from this store needs to be synchronous and fast because we do it for every outgoing connection. Thus, the entire list needs to be in-memory anyway.

Storing peer addresses in a database or disk does make sense though. I think we can accommodate for that by:

  • Adding APIs to the peer store for adding and removing entries
  • Adding an API for listing all entries
  • Adding an event that gets emitted every time the peer store is updated

The above would allow users to integrate the peer store into an event loop and synchronize its state with another data source or sink. It would only be eventually-consistent but I think that is fine.

@thomaseizinger
Copy link
Contributor Author

With #4371 approved, this issue is now unblocked.

@guillaumemichel
Copy link
Contributor

Lessons from go-libp2p: libp2p/go-libp2p#2355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants