chore: peer-discovery not using peer-info #90
Conversation
BREAKING CHANGE: peer event emitted with id and multiaddrs properties instead of peer-info
vasco-santos
force-pushed
the
chore/peer-discovery-not-using-peer-info
branch
from
4d12931
to
3ba8b9e
Compare
vasco-santos
force-pushed
the
chore/peer-discovery-not-using-peer-info
branch
from
adf6788
to
5c6daaf
Compare
| this._queryInterval = null | ||
| this._onPeer = this._onPeer.bind(this) | ||
|
|
||
| if (options.compat !== false) { | ||
| this._goMdns = new GoMulticastDNS(options.peerInfo, { | ||
| this._goMdns = new GoMulticastDNS(options.peerId, this.peerMultiaddrs, { | ||
| queryPeriod: options.compatQueryPeriod, | ||
| queryInterval: options.compatQueryInterval | ||
| }) |
There was a problem hiding this comment.
Existing bug, we're apparently not passing these options into the querier GoMulticastDNS creates. It's constructor doesn't take options but it should
| } | ||
|
|
||
| this.broadcast = options.broadcast !== false | ||
| this.interval = options.interval || (1e3 * 10) | ||
| this.serviceTag = options.serviceTag || 'ipfs.local' | ||
| this.port = options.port || 5353 | ||
| this.peerInfo = options.peerInfo | ||
| this.peerId = options.peerId | ||
| this.peerMultiaddrs = options.multiaddrs || [] | ||
| this._queryInterval = null | ||
| this._onPeer = this._onPeer.bind(this) |
There was a problem hiding this comment.
This function needs to be fixed. It still takes and emits peerInfo. Recommend doing a project case insensitive search for peerinfo to catch any other items.
There was a problem hiding this comment.
Probably just needs a rename to peerData to avoid confusion
src/index.js
Outdated
| } | ||
|
|
||
| async _onMdnsResponse (event) { | ||
| try { | ||
| const foundPeer = await query.gotResponse(event, this.peerInfo, this.serviceTag) | ||
| const foundPeer = await query.gotResponse(event, this.peerId, this.serviceTag) |
There was a problem hiding this comment.
gotResponse is not async anymore, don't await.
Co-Authored-By: Jacob Heun <jacobheun@gmail.com>
vasco-santos
force-pushed
the
chore/peer-discovery-not-using-peer-info
branch
from
03d68a6
to
2b52761
Compare
|
I addressed the review and also fixed the Querier parameters bug |
|
Changed README to use |
|
Beta tag released: dist-tags:
beta: 0.14.0 latest: 0.13.3 |
In the context of deprecating
peer-infoas described on libp2p/js-libp2p#589, this PR changes thepeer-discoveryinterface to emit an object containing anidproperty and amultiaddrsproperty.This adds a multiaddrs property to the constructor for now. This should receive libp2p and access the libp2p listening multiaddrs instead
BREAKING CHANGE: peer event emits an object with id and multiaddr instead of a peer-info
Needs: