libp2p / js-libp2p-pubsub-peer-discovery

A js-libp2p module that uses pubsub for mdns like peer discovery
Other
11 stars 6 forks source link

Include Protocols in PeerInfo Broadcast #106

Open justin0mcateer opened 1 year ago

justin0mcateer commented 1 year ago

The service is broadcasting a PeerInfo event, but not populating the 'protocols' attribute. This information is available and could be used to proactively update the PeerStore with protocols as we discover Peers. This would seem to speed up the overall process of learning about a Peer and what protocols it supports, very similarly to Identify 'Push'.

This could be implemented with a very small PR, which I would be happy to provide. Is there any reason this couldn't/shouldn't be done?

maschad commented 1 year ago

This information is available

Where would you retrieve it from? A peer would only know what protocols to advertise during negotiation which is after initiating a new stream.

justin0mcateer commented 1 year ago

The PeerStore has a list of the supported protocols for the local peer in the PeerInfo.protocols attribute. This is the same source the Identify protocol uses to return the list of protocols supported by the node.

maschad commented 1 year ago

A few things to note:

The service is broadcasting a PeerInfo event

This discovery module actually broadcasts an encoded protobuf RPC message to all other peers subscribed to that topic.

This information is available and could be used to proactively update the PeerStore with protocols as we discover Peers.

When a peer that is subscribed to the topic receives such an RPC message, the message is decoded and the peerID is derived from the public key but at that point the node running this discovery service does not have access to that newly discovered peer's PeerStore, and so it cannot dispatch a PeerInfo Event containing the protocols, thus identify/identify push is used to discover these.

justin0mcateer commented 1 year ago

The node doing the broadcasting would send its own protocols, in the GossipSub message.

Are you suggesting that a peer receiving the broadcast is not capable of storing the protocols for the remote peer at the time the broadcast is received?


From: Chad Nehemiah @.> Sent: Thursday, August 10, 2023 8:54:13 PM To: libp2p/js-libp2p-pubsub-peer-discovery @.> Cc: justin0mcateer @.>; Author @.> Subject: Re: [libp2p/js-libp2p-pubsub-peer-discovery] Include Protocols in PeerInfo Broadcast (Issue #106)

A few things to note:

The service is broadcasting a PeerInfo event

This discovery module actually broadcastshttps://github.com/libp2p/js-libp2p-pubsub-peer-discovery/blob/master/src/index.ts#L157-L162 an encodedhttps://github.com/libp2p/js-libp2p-pubsub-peer-discovery/blob/master/src/peer.ts#L19-L70 protobuf RPC messagehttps://github.com/libp2p/specs/blob/d2106f43e878ae4c3a1c6465a7c329835290fe22/pubsub/README.md?plain=1#L111-L125 to all other peers subscribed to that topic.

This information is available and could be used to proactively update the PeerStore with protocols as we discover Peers.

When a peer that is subscribed to the topic receives such an RPC messagehttps://github.com/libp2p/js-libp2p-pubsub-peer-discovery/blob/master/src/index.ts#L178, the message is decoded and the peerID is derived from the public keyhttps://github.com/libp2p/js-libp2p-pubsub-peer-discovery/blob/master/src/index.ts#L191-L205 but at that point the node running this discovery service does not have access to that newly discovered peer's PeerStore, and so it cannot dispatch a PeerInfo Event containing the protocols, thus identify/identify push is used to discover these.

— Reply to this email directly, view it on GitHubhttps://github.com/libp2p/js-libp2p-pubsub-peer-discovery/issues/106#issuecomment-1674127732, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAT4Q7KASH5Q4RZBOEFXSD3XUWGELANCNFSM6AAAAAA3LLXFLM. You are receiving this because you authored the thread.Message ID: @.***>