Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

chore: peer-discovery not using peer-info #90

Merged
merged 4 commits into from
Apr 23, 2020
Merged

chore: peer-discovery not using peer-info #90

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3ba8b9e
chore: peer-discovery not using peer-info
vasco-santos Apr 6, 2020
5c6daaf
chore: add tests for peer-discovery interface
vasco-santos Apr 7, 2020
2b52761
chore: apply suggestions from code review
vasco-santos Apr 21, 2020
cdfc308
chore: update readme with peerData and peerId
vasco-santos Apr 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,14 @@
"chai": "^4.2.0",
"delay": "^4.3.0",
"dirty-chai": "^2.0.1",
"interface-discovery": "^0.1.1",
"libp2p-interfaces": "libp2p/js-interfaces#v0.3.x",
"p-defer": "^3.0.0"
},
"dependencies": {
"debug": "^4.1.1",
"multiaddr": "^7.1.0",
"multicast-dns": "^7.2.0",
"peer-id": "~0.13.3",
"peer-info": "~0.17.0"
"peer-id": "~0.13.3"
},
"contributors": [
"David Dias <daviddias.p@gmail.com>",
Expand Down
9 changes: 5 additions & 4 deletions src/compat/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ const Responder = require('./responder')
const Querier = require('./querier')

class GoMulticastDNS extends EE {
constructor (peerInfo) {
constructor (peerId, multiaddrs) {
vasco-santos marked this conversation as resolved.
Show resolved Hide resolved
super()
this._started = false
this._peerInfo = peerInfo
this._peerId = peerId
this._multiaddrs = multiaddrs
this._onPeer = this._onPeer.bind(this)
}

Expand All @@ -20,8 +21,8 @@ class GoMulticastDNS extends EE {
}

this._started = true
this._responder = new Responder(this._peerInfo)
this._querier = new Querier(this._peerInfo.id)
this._responder = new Responder(this._peerId, this._multiaddrs)
this._querier = new Querier(this._peerId)

this._querier.on('peer', this._onPeer)

Expand Down
18 changes: 7 additions & 11 deletions src/compat/querier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const EE = require('events')
const MDNS = require('multicast-dns')
const Multiaddr = require('multiaddr')
const PeerInfo = require('peer-info')
const PeerId = require('peer-id')
const debug = require('debug')
const log = debug('libp2p:mdns:compat:querier')
Expand All @@ -13,9 +12,11 @@ const { SERVICE_TAG_LOCAL, MULTICAST_IP, MULTICAST_PORT } = require('./constants
class Querier extends EE {
constructor (peerId, options) {
super()

if (!peerId) {
throw new Error('missing peerId parameter')
}

options = options || {}
this._peerIdStr = peerId.toB58String()
// Re-query every 60s, in leu of network change detection
Expand Down Expand Up @@ -57,7 +58,7 @@ class Querier extends EE {
})
}

async _onResponse (event, info) {
_onResponse (event, info) {
const answers = event.answers || []
const ptrRecord = answers.find(a => a.type === 'PTR' && a.name === SERVICE_TAG_LOCAL)

Expand Down Expand Up @@ -87,13 +88,6 @@ class Querier extends EE {
return log('failed to create peer ID from TXT record data', peerIdStr, err)
}

let peerInfo
try {
peerInfo = await PeerInfo.create(peerId)
} catch (err) {
return log.error('failed to create peer info from peer ID', peerId, err)
}

const srvRecord = answers.find(a => a.type === 'SRV')
if (!srvRecord) return log('missing SRV record in response')

Expand All @@ -115,8 +109,10 @@ class Querier extends EE {
return addrs
}, [])

multiaddrs.forEach(addr => peerInfo.multiaddrs.add(addr))
this.emit('peer', peerInfo)
this.emit('peer', {
id: peerId,
multiaddrs
})
}

stop () {
Expand Down
13 changes: 7 additions & 6 deletions src/compat/responder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ const log = require('debug')('libp2p:mdns:compat:responder')
const { SERVICE_TAG_LOCAL } = require('./constants')

class Responder {
constructor (peerInfo) {
if (!peerInfo) {
throw new Error('missing peerInfo parameter')
constructor (peerId, multiaddrs) {
vasco-santos marked this conversation as resolved.
Show resolved Hide resolved
if (!peerId) {
throw new Error('missing peerId parameter')
}

this._peerInfo = peerInfo
this._peerIdStr = peerInfo.id.toB58String()
this._peerId = peerId
this._peerIdStr = peerId.toB58String()
this._multiaddrs = multiaddrs
this._onQuery = this._onQuery.bind(this)
}

Expand All @@ -22,7 +23,7 @@ class Responder {
}

_onQuery (event, info) {
const addresses = this._peerInfo.multiaddrs.toArray().map(ma => ma.toOptions())
const addresses = this._multiaddrs.map(ma => ma.toOptions())
// Only announce TCP for now
if (!addresses.length) return

Expand Down
16 changes: 9 additions & 7 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict'

const multicastDNS = require('multicast-dns')
const EventEmitter = require('events').EventEmitter
const { EventEmitter } = require('events')
const debug = require('debug')
const log = debug('libp2p:mdns')
const query = require('./query')
Expand All @@ -10,20 +10,22 @@ const GoMulticastDNS = require('./compat')
class MulticastDNS extends EventEmitter {
constructor (options = {}) {
super()
if (!options.peerInfo) {
throw new Error('needs a PeerInfo to work')

if (!options.peerId) {
throw new Error('needs own PeerId to work')
}

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably just needs a rename to peerData to avoid confusion


if (options.compat !== false) {
this._goMdns = new GoMulticastDNS(options.peerInfo, {
this._goMdns = new GoMulticastDNS(options.peerId, this.peerMultiaddrs, {
queryPeriod: options.compatQueryPeriod,
queryInterval: options.compatQueryInterval
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing bug, we're apparently not passing these options into the querier GoMulticastDNS creates. It's constructor doesn't take options but it should

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix this

Expand Down Expand Up @@ -51,12 +53,12 @@ class MulticastDNS extends EventEmitter {
}

_onMdnsQuery (event) {
query.gotQuery(event, this.mdns, this.peerInfo, this.serviceTag, this.broadcast)
query.gotQuery(event, this.mdns, this.peerId, this.peerMultiaddrs, this.serviceTag, this.broadcast)
}

async _onMdnsResponse (event) {
try {
const foundPeer = await query.gotResponse(event, this.peerInfo, this.serviceTag)
const foundPeer = await query.gotResponse(event, this.peerId, this.serviceTag)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotResponse is not async anymore, don't await.


if (foundPeer) {
this.emit('peer', foundPeer)
Expand Down
41 changes: 22 additions & 19 deletions src/query.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict'

const Peer = require('peer-info')
const os = require('os')
const debug = require('debug')
const log = debug('libp2p:mdns')
Expand All @@ -26,7 +25,7 @@ module.exports = {
return setInterval(query, interval)
},

gotResponse: async function (rsp, peerInfo, serviceTag) {
gotResponse: function (rsp, localPeerId, serviceTag) {
if (!rsp.answers) { return }

const answers = {
Expand Down Expand Up @@ -57,33 +56,37 @@ module.exports = {
const multiaddrs = []

answers.a.forEach((a) => {
multiaddrs.push(new Multiaddr('/ip4/' + a.data + '/tcp/' + port))
const ma = new Multiaddr('/ip4/' + a.data + '/tcp/' + port)

if (!multiaddrs.some((m) => m.equals(ma))) {
multiaddrs.push(ma)
}
})

answers.aaaa.forEach((a) => {
multiaddrs.push(new Multiaddr('/ip6/' + a.data + '/tcp/' + port))
const ma = new Multiaddr('/ip6/' + a.data + '/tcp/' + port)

if (!multiaddrs.some((m) => m.equals(ma))) {
multiaddrs.push(ma)
}
})

if (peerInfo.id.toB58String() === b58Id) {
if (localPeerId.toB58String() === b58Id) {
return // replied to myself, ignore
}

log('peer found -', b58Id)

const peerId = Id.createFromB58String(b58Id)

try {
const peerFound = await Peer.create(peerId)
multiaddrs.forEach((addr) => peerFound.multiaddrs.add(addr))
return peerFound
} catch (err) {
log.error('Error creating PeerInfo from new found peer', err)
return {
id: Id.createFromB58String(b58Id),
multiaddrs
}
},

gotQuery: function (qry, mdns, peerInfo, serviceTag, broadcast) {
gotQuery: function (qry, mdns, peerId, multiaddrs, serviceTag, broadcast) {
if (!broadcast) { return }

const addresses = peerInfo.multiaddrs.toArray().map(ma => ma.toOptions())
const addresses = multiaddrs.map(ma => ma.toOptions())
// Only announce TCP for now
if (addresses.length === 0) { return }

Expand All @@ -95,14 +98,14 @@ module.exports = {
type: 'PTR',
class: 'IN',
ttl: 120,
data: peerInfo.id.toB58String() + '.' + serviceTag
data: peerId.toB58String() + '.' + serviceTag
})

// Only announce TCP multiaddrs for now
const port = addresses[0].port

answers.push({
name: peerInfo.id.toB58String() + '.' + serviceTag,
name: peerId.toB58String() + '.' + serviceTag,
type: 'SRV',
class: 'IN',
ttl: 120,
Expand All @@ -115,11 +118,11 @@ module.exports = {
})

answers.push({
name: peerInfo.id.toB58String() + '.' + serviceTag,
name: peerId.toB58String() + '.' + serviceTag,
type: 'TXT',
class: 'IN',
ttl: 120,
data: peerInfo.id.toB58String()
data: peerId.toB58String()
})

addresses.forEach((addr) => {
Expand Down
37 changes: 18 additions & 19 deletions test/compat/go-multicast-dns.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,36 @@ const chai = require('chai')
const dirtyChai = require('dirty-chai')
const expect = chai.expect
chai.use(dirtyChai)
const PeerInfo = require('peer-info')

const multiaddr = require('multiaddr')
const PeerId = require('peer-id')
const pDefer = require('p-defer')

const GoMulticastDNS = require('../../src/compat')

describe('GoMulticastDNS', () => {
const peerAddrs = [
'/ip4/127.0.0.1/tcp/20001',
'/ip4/127.0.0.1/tcp/20002'
multiaddr('/ip4/127.0.0.1/tcp/20001'),
multiaddr('/ip4/127.0.0.1/tcp/20002')
]
let peerInfos
let peerIds

before(async () => {
peerInfos = await Promise.all([
PeerInfo.create(),
PeerInfo.create()
peerIds = await Promise.all([
PeerId.create(),
PeerId.create()
])

peerInfos.forEach((peer, index) => {
peer.multiaddrs.add(peerAddrs[index])
})
})

it('should start and stop', async () => {
const mdns = new GoMulticastDNS(peerInfos[0])
const mdns = new GoMulticastDNS(peerIds[0], [peerAddrs[0]])

await mdns.start()
return mdns.stop()
})

it('should ignore multiple start calls', async () => {
const mdns = new GoMulticastDNS(peerInfos[0])
const mdns = new GoMulticastDNS(peerIds[0], [peerAddrs[0]])

await mdns.start()
await mdns.start()
Expand All @@ -45,18 +43,19 @@ describe('GoMulticastDNS', () => {
})

it('should ignore unnecessary stop calls', async () => {
const mdns = new GoMulticastDNS(peerInfos[0])
const mdns = new GoMulticastDNS(peerIds[0], [peerAddrs[0]])
await mdns.stop()
})

it('should emit peer info when peer is discovered', async () => {
const mdnsA = new GoMulticastDNS(peerInfos[0])
const mdnsB = new GoMulticastDNS(peerInfos[1])
const mdnsA = new GoMulticastDNS(peerIds[0], [peerAddrs[0]])
const mdnsB = new GoMulticastDNS(peerIds[1], [peerAddrs[1]])
const defer = pDefer()

mdnsA.on('peer', info => {
if (!info.id.isEqual(peerInfos[1].id)) return
expect(info.multiaddrs.has(peerAddrs[1])).to.be.true()
mdnsA.on('peer', ({ id, multiaddrs }) => {
if (!id.isEqual(peerIds[1])) return

expect(multiaddrs.some((m) => m.equals(peerAddrs[1]))).to.be.true()
defer.resolve()
})

Expand Down
Loading