fix: code review

Author: vasco-santos

README.md

@@ -34,10 +34,6 @@ You can check the development status at the [Kanban Board](https://waffle.io/ipf
 
 [![Throughput Graph](https://graphs.waffle.io/ipfs/js-ipfs/throughput.svg)](https://waffle.io/ipfs/js-ipfs/metrics/throughput)
 
-**Please read this:** Circuit Relay (pierce through NATs and dial between any node in the network) is a fundamental piece that is not finalized yet. There are multiple applications that can be built without this service but nevertheless it is fundamental to get that magic IPFS experience. If you want to track progress or contribute, please follow:
-
-- ✅ Relay: https://github.com/ipfs/js-ipfs/pull/1063
-
 [**`Weekly Core Dev Calls`**](https://github.com/ipfs/pm/issues/650)
 
 ## Tech Lead

src/cli/commands/dht.js

@@ -1,8 +1,5 @@
 'use strict'
 
-/*
-Issue commands directly through the DHT.
-*/
 module.exports = {
   command: 'dht <command>',
 

src/cli/commands/dht/find-peer.js

@@ -10,14 +10,14 @@ module.exports = {
   builder: {},
 
   handler (argv) {
-    argv.ipfs.dht.findpeer(argv.peerID, (err, result) => {
+    argv.ipfs.dht.findPeer(argv.peerID, (err, result) => {
       if (err) {
         throw err
       }
 
-      result.responses[0].addrs.forEach((element) => {
-        print(element)
-      })
+      const addresses = result.multiaddrs.toArray().map((ma) => ma.toString())
+
+      print(addresses)
     })
   }
 }

src/cli/commands/dht/find-providers.js

@@ -17,16 +17,16 @@ module.exports = {
 
   handler (argv) {
     const opts = {
-      'num-providers': argv['num-providers']
+      numProviders: argv['num-providers']
     }
 
-    argv.ipfs.dht.findprovs(argv.key, opts, (err, result) => {
+    argv.ipfs.dht.findProvs(argv.key, opts, (err, result) => {
       if (err) {
         throw err
       }
 
-      result.responses.forEach((element) => {
-        print(element.id)
+      result.forEach((element) => {
+        print(element.id.toB58String())
       })
     })
   }

src/cli/commands/dht/query.js

@@ -15,8 +15,8 @@ module.exports = {
         throw err
       }
 
-      result.forEach((element) => {
-        print(element.ID)
+      result.forEach((peerID) => {
+        print(peerID.id.toB58String())
       })
     })
   }

src/core/components/dht.js

@@ -3,8 +3,8 @@
 const promisify = require('promisify-es6')
 const every = require('async/every')
 const PeerId = require('peer-id')
+const PeerInfo = require('peer-info')
 const CID = require('cids')
-const multihash = require('multihashes')
 const each = require('async/each')
 const setImmediate = require('async/setImmediate')
 const errCode = require('err-code')
@@ -15,6 +15,8 @@ module.exports = (self) => {
      * Given a key, query the DHT for its best value.
      *
      * @param {Buffer} key
+     * @param {Object} options - get options
+     * @param {number} options.maxTimeout - optional timeout
      * @param {function(Error)} [callback]
      * @returns {Promise|void}
      */
@@ -28,9 +30,9 @@ module.exports = (self) => {
 
       if (!Buffer.isBuffer(key)) {
         try {
-          key = multihash.fromB58String(key)
+          key = (new CID(key)).buffer()
         } catch (err) {
-          return callback(err)
+          return setImmediate(() => callback(errCode(err, 'ERR_INVALID_CID')))
         }
       }
 
@@ -52,9 +54,9 @@ module.exports = (self) => {
     put: promisify((key, value, callback) => {
       if (!Buffer.isBuffer(key)) {
         try {
-          key = multihash.fromB58String(key)
+          key = (new CID(key)).buffer()
         } catch (err) {
-          return callback(err)
+          return setImmediate(() => callback(errCode(err, 'ERR_INVALID_CID')))
         }
       }
 
@@ -65,17 +67,21 @@ module.exports = (self) => {
      * Find peers in the DHT that can provide a specific value, given a key.
      *
      * @param {CID} key - They key to find providers for.
+     * @param {Object} options - findProviders options
+     * @param {number} options.maxTimeout - how long the query should maximally run, in milliseconds (default: 60000)
+     * @param {number} options.maxNumProviders - maximum number of providers to find
      * @param {function(Error, Array<PeerInfo>)} [callback]
      * @returns {Promise<PeerInfo>|void}
      */
-    findprovs: promisify((key, opts, callback) => {
-      if (typeof opts === 'function') {
-        callback = opts
-        opts = {}
+    findProvs: promisify((key, options, callback) => {
+      if (typeof options === 'function') {
+        callback = options
+        options = {}
       }
 
-      opts = opts || {}
-      opts.maxNumProviders = opts['num-providers']
+      options = options || {}
+      options.timeout = options.maxTimeout // TODO create PR kad-dht
+      options.maxNumProviders = options.numProviders
 
       if (typeof key === 'string') {
         try {
@@ -85,61 +91,29 @@ module.exports = (self) => {
         }
       }
 
-      self.libp2p.contentRouting.findProviders(key, opts, (err, res) => {
-        if (err) {
-          return callback(err)
-        }
-
-        // convert to go-ipfs return value, we need to revisit
-        // this. For now will just conform.
-        const goResult = {
-          responses: res.map((peerInfo) => ({
-            id: peerInfo.id.toB58String(),
-            addrs: peerInfo.multiaddrs.toArray().map((a) => a.toString())
-          })),
-          type: 4
-        }
-
-        callback(null, goResult)
-      })
+      self.libp2p.contentRouting.findProviders(key, options, callback)
     }),
 
     /**
      * Query the DHT for all multiaddresses associated with a `PeerId`.
      *
      * @param {PeerId} peer - The id of the peer to search for.
-     * @param {function(Error, Array<Multiaddr>)} [callback]
-     * @returns {Promise<Array<Multiaddr>>|void}
+     * @param {function(Error, Array<PeerInfo>)} [callback]
+     * @returns {Promise<Array<PeerInfo>>|void}
      */
-    findpeer: promisify((peer, callback) => {
+    findPeer: promisify((peer, callback) => {
       if (typeof peer === 'string') {
         peer = PeerId.createFromB58String(peer)
       }
 
-      self.libp2p.peerRouting.findPeer(peer, (err, info) => {
-        if (err) {
-          return callback(err)
-        }
-
-        // convert to go-ipfs return value, we need to revisit
-        // this. For now will just conform.
-        const goResult = {
-          responses: [{
-            id: info.id.toB58String(),
-            addrs: info.multiaddrs.toArray().map((a) => a.toString())
-          }],
-          type: 2
-        }
-
-        callback(null, goResult)
-      })
+      self.libp2p.peerRouting.findPeer(peer, callback)
     }),
 
     /**
      * Announce to the network that we are providing given values.
      *
      * @param {CID|Array<CID>} keys - The keys that should be announced.
-     * @param {Object} [options={}]
+     * @param {Object} options - provide options
      * @param {bool} [options.recursive=false] - Provide not only the given object but also all objects linked from it.
      * @param {function(Error)} [callback]
      * @returns {Promise|void}
@@ -181,15 +155,15 @@ module.exports = (self) => {
      * Find the closest peers to a given `PeerId`, by querying the DHT.
      *
      * @param {PeerId} peer - The `PeerId` to run the query agains.
-     * @param {function(Error, Array<PeerId>)} [callback]
-     * @returns {Promise<Array<PeerId>>|void}
+     * @param {function(Error, Array<PeerInfo>)} [callback]
+     * @returns {Promise<Array<PeerInfo>>|void}
      */
     query: promisify((peerId, callback) => {
       if (typeof peerId === 'string') {
         try {
           peerId = PeerId.createFromB58String(peerId)
         } catch (err) {
-          callback(err)
+          return callback(err)
         }
       }
 
@@ -198,9 +172,9 @@ module.exports = (self) => {
         if (err) {
           return callback(err)
         }
-        callback(null, peerIds.map((id) => {
-          return { ID: id.toB58String() }
-        }))
+
+        // callback(null, peerIds)
+        callback(null, peerIds.map((id) => new PeerInfo(id)))
       })
     })
   }