fix: code review

ipfs · Feb 6, 2019 · 9a4083e · 9a4083e
1 parent e859a81
commit 9a4083e
Show file tree

Hide file tree

Showing 17 changed files with 143 additions and 252 deletions.
diff --git a/package.json b/package.json
@@ -72,7 +72,7 @@
     "form-data": "^2.3.3",
     "hat": "0.0.3",
     "interface-ipfs-core": "~0.96.0",
-    "ipfsd-ctl": "~0.40.2",
+    "ipfsd-ctl": "~0.41.0",
     "ncp": "^2.0.0",
     "qs": "^6.5.2",
     "rimraf": "^2.6.2",
@@ -128,7 +128,7 @@
     "joi": "^14.3.0",
     "joi-browser": "^13.4.0",
     "joi-multiaddr": "^4.0.0",
-    "libp2p": "~0.24.3",
+    "libp2p": "libp2p/js-libp2p#master",
     "libp2p-bootstrap": "~0.9.3",
     "libp2p-crypto": "~0.16.0",
     "libp2p-kad-dht": "~0.14.4",

diff --git a/src/cli/commands/daemon.js b/src/cli/commands/daemon.js
@@ -18,10 +18,6 @@ module.exports = {
         type: 'boolean',
         default: false
       })
-      .option('enable-dht-experiment', {
-        type: 'boolean',
-        default: false
-      })
       .option('offline', {
         desc: 'Run offline. Do not connect to the rest of the network but provide local API.',
         default: false

diff --git a/src/cli/commands/dht/find-peer.js b/src/cli/commands/dht/find-peer.js
@@ -14,7 +14,9 @@ module.exports = {
       const peers = await ipfs.dht.findPeer(peerID)
       const addresses = peers.multiaddrs.toArray().map((ma) => ma.toString())
 
-      print(addresses)
+      addresses.forEach((addr) => {
+        print(addr)
+      })
     })())
   }
 }
diff --git a/src/cli/commands/dht/provide.js b/src/cli/commands/dht/provide.js
@@ -13,11 +13,13 @@ module.exports = {
     }
   },
 
-  handler ({ ipfs, key, resolve }) {
-    // TODO add recursive option
+  handler ({ ipfs, key, recursive, resolve }) {
+    const opts = {
+      recursive
+    }
 
     resolve((async () => {
-      await ipfs.dht.provide(key)
+      await ipfs.dht.provide(key, opts)
     })())
   }
 }
diff --git a/src/core/components/dht.js b/src/core/components/dht.js
@@ -6,7 +6,8 @@ const PeerId = require('peer-id')
 const PeerInfo = require('peer-info')
 const CID = require('cids')
 const each = require('async/each')
-const setImmediate = require('async/setImmediate')
+const nextTick = require('async/nextTick')
+
 const errcode = require('err-code')
 
 const debug = require('debug')
@@ -38,7 +39,7 @@ module.exports = (self) => {
         } catch (err) {
           log.error(err)
 
-          return setImmediate(() => callback(errcode(err, 'ERR_INVALID_CID')))
+          return nextTick(() => callback(errcode(err, 'ERR_INVALID_CID')))
         }
       }
 
@@ -64,7 +65,7 @@ module.exports = (self) => {
         } catch (err) {
           log.error(err)
 
-          return setImmediate(() => callback(errcode(err, 'ERR_INVALID_CID')))
+          return nextTick(() => callback(errcode(err, 'ERR_INVALID_CID')))
         }
       }
 
@@ -95,7 +96,7 @@ module.exports = (self) => {
         } catch (err) {
           log.error(err)
 
-          return setImmediate(() => callback(errcode(err, 'ERR_INVALID_CID')))
+          return nextTick(() => callback(errcode(err, 'ERR_INVALID_CID')))
         }
       }
 

diff --git a/src/core/components/libp2p.js b/src/core/components/libp2p.js
@@ -77,7 +77,10 @@ function defaultBundle ({ datastore, peerInfo, peerBook, options, config }) {
       },
       dht: {
         kBucketSize: get(options, 'dht.kBucketSize', 20),
-        enabledDiscovery: get(options, 'dht.enabledDiscovery', true),
+        enabled: get(options, 'dht.enabled', true) && !(get(options, 'offline', false)),
+        randomWalk: {
+          enabled: get(options, 'dht.randomWalk.enabled', true)
+        },
         validators: {
           ipns: ipnsUtils.validator
         },
@@ -86,7 +89,6 @@ function defaultBundle ({ datastore, peerInfo, peerBook, options, config }) {
         }
       },
       EXPERIMENTAL: {
-        dht: !(get(options, 'local', false)),
         pubsub: get(options, 'EXPERIMENTAL.pubsub', false)
       }
     },

diff --git a/src/core/runtime/libp2p-browser.js b/src/core/runtime/libp2p-browser.js
@@ -54,11 +54,9 @@ class Node extends libp2p {
           }
         },
         dht: {
-          kBucketSize: 20,
-          enabledDiscovery: true
+          enabled: false
         },
         EXPERIMENTAL: {
-          dht: true,
           pubsub: false
         }
       }

diff --git a/src/core/runtime/libp2p-nodejs.js b/src/core/runtime/libp2p-nodejs.js
@@ -54,10 +54,12 @@ class Node extends libp2p {
         },
         dht: {
           kBucketSize: 20,
-          enabledDiscovery: true
+          enabled: true,
+          randomWalk: {
+            enabled: true
+          }
         },
         EXPERIMENTAL: {
-          dht: true,
           pubsub: false
         }
       }

diff --git a/src/http/api/resources/dht.js b/src/http/api/resources/dht.js
@@ -1,6 +1,7 @@
 'use strict'
 
 const Joi = require('joi')
+const Boom = require('boom')
 
 const CID = require('cids')
 
@@ -16,34 +17,27 @@ exports.findPeer = {
       arg: Joi.string().required()
     }).unknown()
   },
-  handler: (request, reply) => {
+  async handler (request, h) {
     const ipfs = request.server.app.ipfs
     const { arg } = request.query
-
-    ipfs.dht.findPeer(arg, (err, res) => {
-      if (err) {
-        log.error(err)
-
-        if (err.code === 'ERR_LOOKUP_FAILED') {
-          return reply({
-            Message: err.toString(),
-            Code: 0
-          }).code(404)
-        }
-
-        return reply({
-          Message: err.toString(),
-          Code: 0
-        }).code(500)
+    let res
+
+    try {
+      res = await ipfs.dht.findPeer(arg)
+    } catch (err) {
+      if (err.code === 'ERR_LOOKUP_FAILED') {
+        throw Boom.notFound(err.toString())
+      } else {
+        throw Boom.boomify(err, { message: err.toString() })
       }
+    }
 
-      reply({
-        Responses: [{
-          ID: res.id.toB58String(),
-          Addrs: res.multiaddrs.toArray().map((a) => a.toString())
-        }],
-        Type: 2
-      })
+    return h.response({
+      Responses: [{
+        ID: res.id.toB58String(),
+        Addrs: res.multiaddrs.toArray().map((a) => a.toString())
+      }],
+      Type: 2
     })
   }
 }
@@ -56,29 +50,20 @@ exports.findProvs = {
       timeout: Joi.number()
     }).unknown()
   },
-  handler: (request, reply) => {
+  async handler (request, h) {
     const ipfs = request.server.app.ipfs
     const { arg } = request.query
 
     request.query.maxNumProviders = request.query['num-providers']
 
-    ipfs.dht.findProvs(arg, request.query, (err, res) => {
-      if (err) {
-        log.error(err)
-
-        return reply({
-          Message: err.toString(),
-          Code: 0
-        }).code(500)
-      }
+    const res = await ipfs.dht.findProvs(arg, request.query)
 
-      reply({
-        Responses: res.map((peerInfo) => ({
-          ID: peerInfo.id.toB58String(),
-          Addrs: peerInfo.multiaddrs.toArray().map((a) => a.toString())
-        })),
-        Type: 4
-      })
+    return h.response({
+      Responses: res.map((peerInfo) => ({
+        ID: peerInfo.id.toB58String(),
+        Addrs: peerInfo.multiaddrs.toArray().map((a) => a.toString())
+      })),
+      Type: 4
     })
   }
 }
@@ -90,24 +75,15 @@ exports.get = {
       timeout: Joi.number()
     }).unknown()
   },
-  handler: (request, reply) => {
+  async handler (request, h) {
     const ipfs = request.server.app.ipfs
     const { arg } = request.query
 
-    ipfs.dht.get(Buffer.from(arg), (err, res) => {
-      if (err) {
-        log.error(err)
+    const res = await ipfs.dht.get(Buffer.from(arg))
 
-        return reply({
-          Message: err.toString(),
-          Code: 0
-        }).code(500)
-      }
-
-      reply({
-        Extra: res.toString(),
-        Type: 5
-      })
+    return h.response({
+      Extra: res.toString(),
+      Type: 5
     })
   }
 }
@@ -118,7 +94,7 @@ exports.provide = {
       arg: Joi.string().required()
     }).unknown()
   },
-  handler: (request, reply) => {
+  async handler (request, h) {
     const ipfs = request.server.app.ipfs
     const { arg } = request.query
     let cid
@@ -127,25 +103,12 @@ exports.provide = {
       cid = new CID(arg)
     } catch (err) {
       log.error(err)
-
-      return reply({
-        Message: err.toString(),
-        Code: 0
-      }).code(500)
+      throw Boom.boomify(err, { message: err.toString() })
     }
 
-    ipfs.dht.provide(cid, request.query, (err) => {
-      if (err) {
-        log.error(err)
-
-        return reply({
-          Message: err.toString(),
-          Code: 0
-        }).code(500)
-      }
+    await ipfs.dht.provide(cid)
 
-      reply({})
-    })
+    return h.response()
   }
 }
 
@@ -155,29 +118,20 @@ exports.put = {
       arg: Joi.array().items(Joi.string()).length(2).required()
     }).unknown()
   },
-  parseArgs: (request, reply) => {
-    return reply({
+  parseArgs: (request, h) => {
+    return {
       key: request.query.arg[0],
       value: request.query.arg[1]
-    })
+    }
   },
-  handler: (request, reply) => {
+  async handler (request, h) {
     const key = request.pre.args.key
     const value = request.pre.args.value
     const ipfs = request.server.app.ipfs
 
-    ipfs.dht.put(Buffer.from(key), Buffer.from(value), (err) => {
-      if (err) {
-        log.error(err)
-
-        return reply({
-          Message: err.toString(),
-          Code: 0
-        }).code(500)
-      }
+    await ipfs.dht.put(Buffer.from(key), Buffer.from(value))
 
-      reply({})
-    })
+    return h.response()
   }
 }
 
@@ -187,25 +141,15 @@ exports.query = {
       arg: Joi.string().required()
     }).unknown()
   },
-  handler: (request, reply) => {
+  async handler (request, h) {
     const ipfs = request.server.app.ipfs
     const { arg } = request.query
 
-    ipfs.dht.query(arg, (err, res) => {
-      if (err) {
-        log.error(err)
-
-        return reply({
-          Message: err.toString(),
-          Code: 0
-        }).code(500)
-      }
+    const res = await ipfs.dht.query(arg)
+    const response = res.map((peerInfo) => ({
+      ID: peerInfo.id.toB58String()
+    }))
 
-      const response = res.map((peerInfo) => ({
-        ID: peerInfo.id.toB58String()
-      }))
-
-      reply(response)
-    })
+    return h.response(response)
   }
 }
diff --git a/src/http/api/routes/dht.js b/src/http/api/routes/dht.js
@@ -15,9 +15,9 @@ module.exports = [
     method: '*',
     path: '/api/v0/dht/findprovs',
     options: {
-      validate: resources.dht.findprovs.validate
+      validate: resources.dht.findProvs.validate
     },
-    handler: resources.dht.findprovs.handler
+    handler: resources.dht.findProvs.handler
   },
   {
     method: '*',
@@ -39,6 +39,9 @@ module.exports = [
     method: '*',
     path: '/api/v0/dht/put',
     options: {
+      pre: [
+        { method: resources.dht.put.parseArgs, assign: 'args' }
+      ],
       validate: resources.dht.put.validate
     },
     handler: resources.dht.put.handler