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

Include Protocols in PeerInfo Broadcast #106

Open
justin0mcateer opened this issue Aug 10, 2023 · 4 comments
Open

Include Protocols in PeerInfo Broadcast #106

justin0mcateer opened this issue Aug 10, 2023 · 4 comments

Comments

@justin0mcateer
Copy link

justin0mcateer commented Aug 10, 2023

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
Copy link
Member

maschad commented Aug 10, 2023

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
Copy link
Author

justin0mcateer commented Aug 10, 2023

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
Copy link
Member

maschad commented Aug 11, 2023

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
Copy link
Author

justin0mcateer commented Aug 11, 2023 via email

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

No branches or pull requests

2 participants