From 10d7212373f09f4d95a22d4070151440f90dd6f5 Mon Sep 17 00:00:00 2001 From: Alan Shaw Date: Thu, 6 Feb 2020 09:23:30 +0000 Subject: [PATCH] fix: protocols change event I was spelunking last night and found that pubsub was constantly having it's `onConnect` handler fired from the multicodec topology. It was because bitswap was calling `dialProtocol` to connect to a peer. `dialProtocol` makes a `PeerInfo` out of the multiaddr it's given but it has no `protocols` i.e. `[]`. This is passed to `peerStore.update()` where we compare `[]` with an array of populated protocols. Obviously there is no intersection here so `change:protocols` was being emitted, even though no protocols were added or removed. This logic needs to be improved and tested properly but I just wanted to send a PR to document my findings. --- src/peer-store/index.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/peer-store/index.js b/src/peer-store/index.js index 72a76675..3ef2a0c5 100644 --- a/src/peer-store/index.js +++ b/src/peer-store/index.js @@ -132,19 +132,16 @@ class PeerStore extends EventEmitter { multiaddrs: recorded.multiaddrs.toArray() }) } - - // Update protocols - // TODO: better track added and removed protocols - const protocolsIntersection = new Set( - [...recorded.protocols].filter((p) => peerInfo.protocols.has(p)) - ) - - if (protocolsIntersection.size !== peerInfo.protocols.size || - protocolsIntersection.size !== recorded.protocols.size) { - for (const protocol of peerInfo.protocols) { + + let isProtocolsChanged = false + for (const protocol of peerInfo.protocols) { + if (!recorded.protocols.has(protocol)) { + isProtocolsChanged = true recorded.protocols.add(protocol) } + } + if (isProtocolsChanged) { this.emit('change:protocols', { peerInfo: recorded, protocols: Array.from(recorded.protocols)