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

* chore: peer-discovery not using peer-info BREAKING CHANGE: peer event emitted with id and multiaddrs properties instead of peer-info * chore: add tests for peer-discovery interface * chore: apply suggestions from code review Co-Authored-By: Jacob Heun <jacobheun@gmail.com> * chore: update readme with peerData and peerId Co-authored-by: Jacob Heun <jacobheun@gmail.com>
libp2p · Apr 23, 2020 · fca175e · fca175e
1 parent ecdda6e
commit fca175e
Show file tree

Hide file tree

Showing 12 changed files with 270 additions and 205 deletions.
diff --git a/README.md b/README.md
@@ -23,8 +23,8 @@ const MDNS = require('libp2p-mdns')
 
 const mdns = new MDNS(options)
 
-mdns.on('peer', (peerInfo) => {
-  console.log('Found a peer in the local network', peerInfo.id.toB58String())
+mdns.on('peer', (peerData) => {
+  console.log('Found a peer in the local network', peerData.id.toB58String(), peerData.multiaddrs)
 })
 
 // Broadcast for 20 seconds
@@ -33,7 +33,8 @@ setTimeout(() => mdns.stop(), 20 * 1000)
 ```
 
 - options
-  - `peerInfo` - PeerInfo to announce
+  - `peerId` - PeerId to announce
+  - `multiaddrs` - multiaddrs to announce
   - `broadcast` - (true/false) announce our presence through mDNS, default `false`
   - `interval` - query interval, default 10 * 1000 (10 seconds)
   - `serviceTag` - name of the service announce , default 'ipfs.local`

diff --git a/package.json b/package.json
@@ -38,15 +38,14 @@
     "chai": "^4.2.0",
     "delay": "^4.3.0",
     "dirty-chai": "^2.0.1",
-    "interface-discovery": "^0.1.1",
+    "libp2p-interfaces": "^0.3.0",
     "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>",

diff --git a/src/compat/index.js b/src/compat/index.js
@@ -7,10 +7,13 @@ const Responder = require('./responder')
 const Querier = require('./querier')
 
 class GoMulticastDNS extends EE {
-  constructor (peerInfo) {
+  constructor ({ peerId, multiaddrs, queryPeriod, queryInterval }) {
     super()
     this._started = false
-    this._peerInfo = peerInfo
+    this._peerId = peerId
+    this._multiaddrs = multiaddrs
+    this._queryPeriod = queryPeriod
+    this._queryInterval = queryInterval
     this._onPeer = this._onPeer.bind(this)
   }
 
@@ -20,8 +23,15 @@ class GoMulticastDNS extends EE {
     }
 
     this._started = true
-    this._responder = new Responder(this._peerInfo)
-    this._querier = new Querier(this._peerInfo.id)
+    this._responder = new Responder({
+      peerId: this._peerId,
+      multiaddrs: this._multiaddrs
+    })
+    this._querier = new Querier({
+      peerId: this._peerId,
+      queryInterval: this._queryInterval,
+      queryPeriod: this._queryPeriod
+    })
 
     this._querier.on('peer', this._onPeer)
 
@@ -31,8 +41,8 @@ class GoMulticastDNS extends EE {
     ])
   }
 
-  _onPeer (peerInfo) {
-    this.emit('peer', peerInfo)
+  _onPeer (peerData) {
+    this.emit('peer', peerData)
   }
 
   stop () {

diff --git a/src/compat/querier.js b/src/compat/querier.js
@@ -3,30 +3,31 @@
 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')
 log.error = debug('libp2p:mdns:compat:querier:error')
 const { SERVICE_TAG_LOCAL, MULTICAST_IP, MULTICAST_PORT } = require('./constants')
 
 class Querier extends EE {
-  constructor (peerId, options) {
+  constructor ({ peerId, queryInterval = 60000, queryPeriod }) {
     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
-    options.queryInterval = options.queryInterval || 60000
-    // Time for which the MDNS server will stay alive waiting for responses
-    // Must be less than options.queryInterval!
-    options.queryPeriod = Math.min(
-      options.queryInterval,
-      options.queryPeriod == null ? 5000 : options.queryPeriod
-    )
-    this._options = options
+    this._options = {
+      // Re-query in leu of network change detection (every 60s by default)
+      queryInterval: queryInterval,
+      // Time for which the MDNS server will stay alive waiting for responses
+      // Must be less than options.queryInterval!
+      queryPeriod: Math.min(
+        queryInterval,
+        queryPeriod == null ? 5000 : queryPeriod
+      )
+    }
     this._onResponse = this._onResponse.bind(this)
   }
 
@@ -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)
 
@@ -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')
 
@@ -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 () {

diff --git a/src/compat/responder.js b/src/compat/responder.js
@@ -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 }) {
+    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)
   }
 
@@ -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
 

diff --git a/src/index.js b/src/index.js
@@ -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')
@@ -10,20 +10,24 @@ 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)
 
     if (options.compat !== false) {
-      this._goMdns = new GoMulticastDNS(options.peerInfo, {
+      this._goMdns = new GoMulticastDNS({
+        multiaddrs: this.peerMultiaddrs,
+        peerId: options.peerId,
         queryPeriod: options.compatQueryPeriod,
         queryInterval: options.compatQueryInterval
       })
@@ -51,12 +55,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) {
+  _onMdnsResponse (event) {
     try {
-      const foundPeer = await query.gotResponse(event, this.peerInfo, this.serviceTag)
+      const foundPeer = query.gotResponse(event, this.peerId, this.serviceTag)
 
       if (foundPeer) {
         this.emit('peer', foundPeer)
@@ -66,8 +70,8 @@ class MulticastDNS extends EventEmitter {
     }
   }
 
-  _onPeer (peerInfo) {
-    this.emit('peer', peerInfo)
+  _onPeer (peerData) {
+    this.emit('peer', peerData)
   }
 
   /**

diff --git a/src/query.js b/src/query.js
@@ -1,6 +1,5 @@
 'use strict'
 
-const Peer = require('peer-info')
 const os = require('os')
 const debug = require('debug')
 const log = debug('libp2p:mdns')
@@ -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 = {
@@ -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 }
 
@@ -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,
@@ -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) => {