Change default config from JSON file to JS module

Because the default config was a JSON file, it was treated as a singleton object. In some parts of the code, data got added to it, leading to race conditions when creating multiple nodes simultaneously. To make that harder to do accidentally, the default config is now a *function* that returns a unique configuration object on each call. Fixes ipfs#1316. License: MIT Signed-off-by: Rob Brackett <[email protected]>
Mr0grog · Apr 23, 2018 · fde8227 · fde8227
1 parent 64c3bfb
commit fde8227
Show file tree

Hide file tree

Showing 9 changed files with 66 additions and 12 deletions.
diff --git a/README.md b/README.md
@@ -227,7 +227,7 @@ Creates and returns an instance of an IPFS node. Use the `options` argument to s
             - `enabled` (boolean): Make this node a relay (other nodes can connect *through* it). (Default: `false`)
             - `active` (boolean): Make this an *active* relay node. Active relay nodes will attempt to dial a destination peer even if that peer is not yet connected to the relay. (Default: `false`)
 
-- `config` (object) Modify the default IPFS node config. Find the Node.js defaults at [`src/core/runtime/config-nodejs.json`](https://github.com/ipfs/js-ipfs/tree/master/src/core/runtime/config-nodejs.json) and the browser defaults at [`src/core/runtime/config-browser.json`](https://github.com/ipfs/js-ipfs/tree/master/src/core/runtime/config-browser.json). This object will be *merged* with the default config; it will not replace it.
+- `config` (object) Modify the default IPFS node config. Find the Node.js defaults at [`src/core/runtime/config-nodejs.js`](https://github.com/ipfs/js-ipfs/tree/master/src/core/runtime/config-nodejs.js) and the browser defaults at [`src/core/runtime/config-browser.js`](https://github.com/ipfs/js-ipfs/tree/master/src/core/runtime/config-browser.js). This object will be *merged* with the default config; it will not replace it.
 
 - `libp2p` (object) add custom modules to the libp2p stack of your node
     - `modules` (object):

diff --git a/package.json b/package.json
@@ -8,7 +8,7 @@
   "main": "src/core/index.js",
   "browser": {
     "./src/core/components/init-assets.js": false,
-    "./src/core/runtime/config-nodejs.json": "./src/core/runtime/config-browser.json",
+    "./src/core/runtime/config-nodejs.js": "./src/core/runtime/config-browser.js",
     "./src/core/runtime/libp2p-nodejs.js": "./src/core/runtime/libp2p-browser.js",
     "./src/core/runtime/repo-nodejs.js": "./src/core/runtime/repo-browser.js",
     "./src/core/runtime/dns-nodejs.js": "./src/core/runtime/dns-browser.js",

diff --git a/src/core/components/bootstrap.js b/src/core/components/bootstrap.js
@@ -1,6 +1,6 @@
 'use strict'
 
-const defaultNodes = require('../runtime/config-nodejs.json').Bootstrap
+const defaultConfig = require('../runtime/config-nodejs.js')
 const isMultiaddr = require('mafmt').IPFS.matches
 const promisify = require('promisify-es6')
 
@@ -41,7 +41,7 @@ module.exports = function bootstrap (self) {
           return callback(err)
         }
         if (args.default) {
-          config.Bootstrap = defaultNodes
+          config.Bootstrap = defaultConfig().Bootstrap
         } else if (multiaddr && config.Bootstrap.indexOf(multiaddr) === -1) {
           config.Bootstrap.push(multiaddr)
         }
@@ -51,7 +51,7 @@ module.exports = function bootstrap (self) {
           }
 
           callback(null, {
-            Peers: args.default ? defaultNodes : [multiaddr]
+            Peers: args.default ? defaultConfig().Bootstrap : [multiaddr]
           })
         })
       })

diff --git a/src/core/components/init.js b/src/core/components/init.js
@@ -4,7 +4,7 @@ const peerId = require('peer-id')
 const waterfall = require('async/waterfall')
 const parallel = require('async/parallel')
 const promisify = require('promisify-es6')
-const config = require('../runtime/config-nodejs.json')
+const defaultConfig = require('../runtime/config-nodejs.js')
 const Keychain = require('libp2p-keychain')
 
 const addDefaultAssets = require('./init-assets')
@@ -37,6 +37,7 @@ module.exports = function init (self) {
     opts.emptyRepo = opts.emptyRepo || false
     opts.bits = Number(opts.bits) || 2048
     opts.log = opts.log || function () {}
+    const config = defaultConfig()
     let privateKey
     waterfall([
       // Verify repo does not yet exist.

diff --git a/src/core/runtime/config-browser.json → src/core/runtime/config-browser.js b/src/core/runtime/config-browser.json → src/core/runtime/config-browser.js
@@ -1,4 +1,6 @@
-{
+'use strict'
+
+module.exports = () => ({
   "Addresses": {
     "Swarm": [
     ],
@@ -24,4 +26,4 @@
     "/dns4/wss0.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmZMxNdpMkewiVZLMRxaNxUeZpDUb34pWjZ1kZvsd16Zic",
     "/dns4/wss1.bootstrap.libp2p.io/tcp/443/wss/ipfs/Qmbut9Ywz9YEDrz8ySBSgWyJk41Uvm2QJPhwDJzJyGFsD6"
   ]
-}
+})
diff --git a/src/core/runtime/config-nodejs.json → src/core/runtime/config-nodejs.js b/src/core/runtime/config-nodejs.json → src/core/runtime/config-nodejs.js
@@ -1,4 +1,6 @@
-{
+'use strict'
+
+module.exports = () => ({
   "Addresses": {
     "Swarm": [
       "/ip4/0.0.0.0/tcp/4002",
@@ -37,4 +39,4 @@
     "/dns4/wss0.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmZMxNdpMkewiVZLMRxaNxUeZpDUb34pWjZ1kZvsd16Zic",
     "/dns4/wss1.bootstrap.libp2p.io/tcp/443/wss/ipfs/Qmbut9Ywz9YEDrz8ySBSgWyJk41Uvm2QJPhwDJzJyGFsD6"
   ]
-}
+})
diff --git a/test/bootstrapers.js b/test/bootstrapers.js
@@ -6,7 +6,7 @@ const dirtyChai = require('dirty-chai')
 const expect = chai.expect
 chai.use(dirtyChai)
 const IPFS = require('..')
-const list = require('../src/core/runtime/config-browser.json').Bootstrap
+const list = require('../src/core/runtime/config-browser.js')().Bootstrap
 
 /*
  * These tests were graciously made for lgierth, so that he can test the

diff --git a/test/core/create-node.spec.js b/test/core/create-node.spec.js
@@ -7,6 +7,8 @@ const dirtyChai = require('dirty-chai')
 const expect = chai.expect
 chai.use(dirtyChai)
 const series = require('async/series')
+const waterfall = require('async/waterfall')
+const parallel = require('async/parallel')
 const os = require('os')
 const path = require('path')
 const hat = require('hat')
@@ -305,4 +307,51 @@ describe('create node', function () {
       (cb) => node.stop(cb)
     ], done)
   })
+
+  it('does not share identity with a simultaneously created node', function (done) {
+    this.timeout(80 * 1000)
+
+    let _nodeNumber = 0;
+    function createNode () {
+      _nodeNumber++;
+      return new IPFS({
+        repo: createTempRepo(),
+        init: { emptyRepo: true },
+        config: {
+          Addresses: {
+            API: `/ip4/127.0.0.1/tcp/${5010 + _nodeNumber}`,
+            Gateway: `/ip4/127.0.0.1/tcp/${9090 + _nodeNumber}`,
+            Swarm: [
+              `/ip4/0.0.0.0/tcp/${4010 + _nodeNumber * 2}`,
+              `/ip4/127.0.0.1/tcp/${4011 + _nodeNumber * 2}/ws`
+            ]
+          },
+          Bootstrap: []
+        }
+      })
+    }
+
+    const nodeA = createNode()
+    const nodeB = createNode()
+
+    waterfall([
+      (cb) => parallel([
+        (cb) => nodeA.once('start', cb),
+        (cb) => nodeB.once('start', cb)
+      ], cb),
+      (_, cb) => parallel([
+        (cb) => nodeA.id(cb),
+        (cb) => nodeB.id(cb)
+      ], cb),
+      ([idA, idB], cb) => {
+        expect(idA.id).to.not.equal(idB.id)
+        cb()
+      }
+    ], (error) => {
+      parallel([
+        (cb) => nodeA.stop(cb),
+        (cb) => nodeB.stop(cb)
+      ], (stopError) => done(error || stopError))
+    })
+  })
 })
diff --git a/test/http-api/inject/bootstrap.js b/test/http-api/inject/bootstrap.js
@@ -3,7 +3,7 @@
 
 const expect = require('chai').expect
 const qs = require('qs')
-const defaultList = require('../../../src/core/runtime/config-nodejs.json').Bootstrap
+const defaultList = require('../../../src/core/runtime/config-nodejs.js')().Bootstrap
 
 module.exports = (http) => {
   describe('/bootstrap', () => {