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

Discovery API #768

Closed
vasco-santos opened this issue Oct 2, 2020 · 3 comments
Closed

Discovery API #768

vasco-santos opened this issue Oct 2, 2020 · 3 comments
Assignees
Labels
kind/discussion Topical discussion; usually not changes to codebase

Comments

@vasco-santos
Copy link
Member

Discovery API

The discovery API aims to provide libp2p users (and other libp2p subsystems) a way for active peer discovery. Peers providing a service will use this API to advertise their presence in some namespace. On the other side, peers seeking a service use the API to discover peers that have previously advertised as service providers.

This issue aims to decide the following:

  • API
  • Libp2p Configuration
  • Implementations
  • Reprovider

API

Based on libp2p/go-libp2p-core/discovery/discovery.go

  • libp2p.discovery.advertise(namespace, options)
  • libp2p.discovery.findPeers(namespace, options)

A draft of it can be seen in libp2p/js-libp2p/blob/feat/discovery-api/doc/API.md#discoveryadvertise.

The discovery implementations (for now contentRouting modules and rendezvous) have some differences that will need to be considered.

Advertise

The advertise is trivial to implement, but the way it will be used over time to reprovide content needs to be considered. Rendezvous supports a given ttl for its records, but the DHT and the delegates currently have a fixed ttl.

🏁 For an initial solution, it will be fine to not support a ttl in advertise and default rendezvous to have the same to ease reprovide implementations. However, we should support this in the long run. I foresee application layer use cases interested in just announcing a service for a small interval and have application logic to understand if should advertise again or not. If we go with this solution, an issue to track an enhancement should be created.

Dropping this ttl for now would make the initial API for advertise: libp2p.discovery.advertise(namespace)

FindPeers

The findPeers API might need additional options from the Rendezvous and content routing current implementations. It should support an option for specifying the max number of distinct peers needed.

If we only have setup one Discovery implementation, it will be trivial to implement. However, if we setup multiple implementations we need to consider how they will be leverage, namely in Series or Parallel.

In series (perhaps following an order configured) will be easy to implement, and once we get the maxNumber we can just return. However, having a parallel approach would result in faster results. In a parallel approach, the implementations will need to be synchronised regarding when there are enough distinct peers. One possible solution is to add Abort Controllers support for rendezvous and content routing modules discovery. Once libp2p found enough peers, both would be aborted.

🏁 For an initial solution, we should probably do the discovery in Series. The main reasons for this are the easy implementation and the lack of Signed Peer Records in the content routing modules. We would use the content routing modules as a fall back, or if rendezvous is not setup. If we go with this solution, an issue to track an enhancement should be created.

Libp2p configuration

In the configuration side of things, we should probably not complicate until we work on improving the configuration. The simplest solution would be:

Libp2p.create({
  contentRouting: [...],
  dht: DHT,
  rendezvous: Rendezvous
})

Libp2p discovery module would be responsible for getting the appropriate modules if available. It would use the contentRouting API (which also includes the DHT).

The ideal solution would be:

Libp2p.create({
  discovery: [ ...]
})

But this introduces a lot of problems, as we would not be able to use libp2p.contentRouting API nor the DHT, as it is created inside libp2p. This would allow a configurable order for discovery modules though.

🏁I think we should go with the simplest approach and properly think about all the configuration issues at the same time, as we are already planning that.

Implementations

Rendezvous and ContentRouting modules will be used, but other modules might be implemented. There are two ways of adding support for the discovery API in these modules.

These modules can implement the API in their codebase, which would be as simples as mapping function namings and eventually rename option parameters (example: limit to maxNumProviders). But, if we do not support from now a configuration with discovery: [ ...], we can also start by having adapters in libp2p to do these mappings.

While adding these to the modules seems the best approach considering the other interfaces we have, it also feels odd to have these mappings in the module codebase. In the future, we can also have modules that extend these modules capabilities with the discovery API. Extend these modules would be tricky, it would depend on how we will go with the configuration improvements.

🏁 At this point, I would go with adapters in libp2p as this WIP branch.

Reprovider

We currently leave for the application layer reproviding content over time. However, one of the goals of the discovery API is to have libp2p subsystems leveraging it (like auto-relay). With this, auto-relay will need to have its own reprovide..

🏁 Taking into account the need to reprovide in the libp2p context, I think we should have a general purpose reprovider available. It would be used not only by the discovery API to reprovide over time (configurable), but also by the DHT. We just need to understand how we would connect all this, considering that the discovery API might use the DHT underneath.

@vasco-santos
Copy link
Member Author

@jacobheun let me know your thoughts on this.

🏁 are the points where we need a decision

@jacobheun jacobheun self-assigned this Oct 15, 2020
@vasco-santos
Copy link
Member Author

Cross posting this:

Not going to block this PR on this (as we're doing it in other content/peer routing calls), but we should think about the parallel actions this is doing. Most people shouldn't be using two, but in the change they are, the parallel calls could cause some odd behavior. This is probably worth discussing in more depth when we look at the discovery api / configuration updates.

ref: #798 (comment)

@vasco-santos vasco-santos added the kind/discussion Topical discussion; usually not changes to codebase label May 21, 2021
@achingbrain
Copy link
Member

Closing as this needs agreement at the specs level before defining the JS API, and even then it will likely be implemented as a service.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/discussion Topical discussion; usually not changes to codebase
Projects
Archived in project
Development

No branches or pull requests

3 participants