feat: enable event emission for peer discovery/connection in ConnectionManager #1438
Conversation
size-limit report 📦
|
| const peersConnectedByPeerExchange: Peer[] = []; | ||
|
|
||
| for (const peer of peersDiscovered) { | ||
| const tags = await this.getTagNamesForPeer(peer.id); |
There was a problem hiding this comment.
you don't need to use getTagNamesForPeer but you can directly refer to peer.tags.keys() to avoid async operation
There was a problem hiding this comment.
it is done to keep fetching tags consistent through this function; agreed that it is async, but it is computationally low demand so should be ok
There was a problem hiding this comment.
I am wondering what is the difference if the same tags are present on peer in the loop as well as those you'd get from this.getTagNamesForPeer?
| @@ -44,12 +52,57 @@ export class ConnectionManager { | |||
| return instance; | |||
| } | |||
|
|
|||
| public async getPeersByDiscovery(): Promise<PeersByDiscovery> { | |||
| const peersDiscovered = await this.libp2p.peerStore.all(); | |||
| const peersConnected = this.libp2p | |||
There was a problem hiding this comment.
I guess we can optimize here and just getConnections which returns tags as well so that later there is no reason to await for peer store
There was a problem hiding this comment.
correct. that is a more efficient way, agreed.
related comment: #1438 (comment)
|
Thank you for promptly addressing the comments! |
danisharora099
force-pushed
the
feat/improve-conn-manager
branch
from
353d819
to
9d777c9
Compare
Problem
This PR aims to make the process of finding/displaying total peers found via a discovery protocol vs actually connected.
It is part of https://github.com/orgs/waku-org/projects/2/views/1?pane=issue&itemId=26409725
Solution
ConnectionManagerto emit events as a new peer is discovered/connectedNotes