-
Notifications
You must be signed in to change notification settings - Fork 445
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
Deprecate peer-info and consolidate peers' information on PeerStore #589
Labels
exp/expert
Having worked on the specific codebase is important
kind/enhancement
A net-new feature or improvement to an existing feature
status/ready
Ready to be worked
Comments
vasco-santos
changed the title
(WIP) Deprecate peer-info and consolidate peers' information on PeerStore
Deprecate peer-info and consolidate peers' information on PeerStore
Mar 19, 2020
cc @jacobheun I did a list of all the cases where |
Closed
vasco-santos
added
exp/expert
Having worked on the specific codebase is important
kind/enhancement
A net-new feature or improvement to an existing feature
status/ready
Ready to be worked
labels
Mar 25, 2020
This was referenced Apr 6, 2020
Merged
This was referenced Apr 16, 2020
1 task
Released in 0.28 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
exp/expert
Having worked on the specific codebase is important
kind/enhancement
A net-new feature or improvement to an existing feature
status/ready
Ready to be worked
As part of the PeerStore improvements epic, we intend to deprecate
peer-info
and consolidate all peer's information on the PeerStore.Background
On
libp2p@0.27
we introduced the PeerStore on thejs-libp2p
codebase replacing thepeer-book
module. The PeerStore is responsible for managing known peers, as well as their addresses and metadata.The initial version of this PeerStore is similar to the previous
peer-book
. It maps peer identifier strings to theirpeer-info
object and has the common operations of a store, such asput
,get
,has
,remove
andreplace
.Problem
Taking into account that peers' information might vary as the time passes and network operations are performed, we introduced two events in the initial version of the PeerStore:
change:protocols
andchange:multiaddrs
, but some other should be added in the future.Since we currently rely on a PeerInfo instance stored in the PeerStore and a PeerInfo instance, we are prone to some data inconsistencies. For example, if a peer discovery protocol discovers a peer, it will create a PeerInfo with that peer and its discovered multiaddr and add it to the PeerStore. In the case that we already know the peer (other multiaddr or not, and possibly some protocols), we currently need to destruct the old information and merge it to the new one. This also creates complexity on finding the changes in
multiaddr
/protocols
/ ...Therefore,
peer-info
adds an extra layer of complexity without benefits, both in thePeerStore
store and on moving data aroundlibp2p
subsystems. In addition, it is better to move around only the needed data, instead of creating unnecessary instances.PeerInfo usage
libp2p
.create()
ipfs
PeerStore
libp2p-kad-dht
peer-discovery
/dht.findPeer
/dht.findProvs
return PeerInfolibp2p-bootstrap
interface-peer-discovery
libp2p-mdns
interface-peer-discovery
libp2p-webrtc-star
interface-peer-discovery
libp2p-pubsub-discovery
interface-peer-discovery
interface-peer-discovery
libp2p-gossipsub
libp2p-floodsub
libp2p-pubsub
libp2p-delegated-content-routing
.findProvs
libp2p-delegated-peer-routing
.findPeer
libp2p-interfaces
peer-discovery
/topology
/content-routing
andpeer-routing
libp2p-daemon-client
libp2p-daemon
libp2p.*
Solution design
While most usages of
PeerInfo
are easily removed, some will need to break APIs or to receive more parameters to be able to work without thePeerInfo
.interface-peer-discovery
Currently, the peer discovery protocols emit a
peer
event with thepeerInfo
as follows discovery.on('peer', (peerInfo) => {}). We currently rely onpeerInfo
because we need both thepeerId
and the discoveredmultiaddr
.We have 2 possible solutions for this:
We can emit an event with an object as follows:
discovery.emit('peer', { peerId, []multiaddrs })
, while theinterface
should validate that both properties are provided.We can provide the
peerStore
topeer-discovery
protocols and they would interact with it directly.Since
peerStore
will be divided into different components (AddressBook, KeyBook, ProtoBook , peerMetadataStore?), I think thatjs-libp2p
should be responsible for handling the events and update what it needs, instead of delegating the responsibility for other subsystems (consequently less logic and needed code changes)To consider: future discovery mechanisms, like
peer-exchange
might also provide known protocols?interface-topology
A libp2p toplogy has a peers map, which maps peer identifiers to their peerInfo. They rely on the
peerInfo
for checking the protocols they support.We can simplify the Topology by having an array of peers important for the topology and provide access to the
PeerStore
data so that the topology can identify the current known protocols of a peer.pubsub internal peer
We currently have an internal peer instance on pubsub. We rely on this instance for managing the stream with peers, subscribed topics and other relevant information. This
peer
also has aninfo
propery with thePeerInfo
of the peer. This is only used for checking the protocols available (particularly useful for fallback to floodsub on gossipsub)!In this situation, we can just have a property for
protocols
on the internal peer instance and rely on thepeerId
instead. We have access for it on theincomingStream
function.content-routing / peer-routing APIs
We should uniform this like the
ipfs
api and get rid ofpeer-info
. This is a similar approach to theinterface-peer-discovery
solution proposed.libp2p
libp2p.create()
accepts a previously createdPeerInfo
.This is particularly useful for listening on a previously specified set of
multiaddr
. This should be part of the discussion around improving config, orjs-libp2p
will need to improve its API around transport managementlibp2p.contentRouting
,libp2p.peerRouting
Should be the same as above
Should use
peer-id
instead ofpeer-info
and the users can gather the same information from the exposedpeerStore
.Review order
peer-discovery
PRspubsub
PRscontent-routing
andpeer-routing
PRsThe text was updated successfully, but these errors were encountered: