feat: enable pubsub via config file and enabled by default (#2427)

vasco-santos · Alan Shaw · commit 27751cf76498 · 2019-09-05T17:19:40.000+01:00
As discussed, we will remove the pubsub flags from the js daemon and will use the config as the way to disable it (will be enabled by default now!) Needs: - [x] [ipfs/js-ipfsd-ctl#366](ipfs/js-ipfsd-ctl#366) Reference: - [ipfs/kubo#6621](ipfs/kubo#6621) BREAKING CHANGE: pubsub is now enabled by default and the experimental flag was removed
diff --git a/README.md b/README.md
@@ -328,7 +328,6 @@ Configure remote preload nodes. The remote will preload content added on this no
 
 Enable and configure experimental features.
 
-- `pubsub` (boolean): Enable libp2p pub-sub. (Default: `false`)
 - `ipnsPubsub` (boolean): Enable pub-sub on IPNS. (Default: `false`)
 - `sharding` (boolean): Enable directory sharding. Directories that have many child objects will be represented by multiple DAG nodes instead of just one. It can improve lookup performance when a directory has several thousand files or more. (Default: `false`)
 
diff --git a/doc/config.md b/doc/config.md
@@ -180,6 +180,12 @@ A string value for specifying which pubsub routing protocol to use. You can eith
 
 Default: `gossipsub`
 
+### `Enabled`
+
+A boolean value for wether or not pubsub router should be active.
+
+Default: `true`
+
 ## `Swarm`
 
 Options for configuring the swarm.
diff --git a/package.json b/package.json
@@ -194,8 +194,8 @@
     "execa": "^2.0.4",
     "form-data": "^2.5.1",
     "hat": "0.0.3",
-    "ipfsd-ctl": "~0.45.0",
     "interface-ipfs-core": "^0.111.1",
+    "ipfsd-ctl": "~0.46.0",
     "libp2p-websocket-star": "~0.10.2",
     "ncp": "^2.0.0",
     "p-event": "^4.1.0",
diff --git a/src/cli/commands/daemon.js b/src/cli/commands/daemon.js
@@ -16,11 +16,6 @@ module.exports = {
         type: 'boolean',
         default: false
       })
-      .option('enable-pubsub', {
-        alias: 'enable-pubsub-experiment',
-        type: 'boolean',
-        default: false
-      })
       .option('offline', {
         type: 'boolean',
         desc: 'Run offline. Do not connect to the rest of the network but provide local API.',
@@ -54,7 +49,6 @@ module.exports = {
         offline: argv.offline,
         pass: argv.pass,
         preload: { enabled: argv.enablePreload },
-        pubsub: { enabled: argv.enablePubsub },
         EXPERIMENTAL: {
           ipnsPubsub: argv.enableNamesysPubsub,
           dht: argv.enableDhtExperiment,
diff --git a/src/cli/utils.js b/src/cli/utils.js
@@ -49,10 +49,7 @@ exports.getIPFS = (argv, callback) => {
     repo: exports.getRepoPath(),
     init: false,
     start: false,
-    pass: argv.pass,
-    pubsub: {
-      enabled: true
-    }
+    pass: argv.pass
   })
 
   const cleanup = promisify((cb) => {
diff --git a/src/core/components/libp2p.js b/src/core/components/libp2p.js
@@ -119,7 +119,7 @@ function defaultBundle ({ datastore, peerInfo, peerBook, options, config }) {
         }
       },
       pubsub: {
-        enabled: get(options, 'pubsub.enabled', false)
+        enabled: get(config, 'Pubsub.Enabled', true)
       }
     },
     connectionManager: get(options, 'connectionManager',
diff --git a/src/core/config.js b/src/core/config.js
@@ -32,9 +32,6 @@ const configSchema = s({
     addresses: optional(s(['multiaddr'])),
     interval: 'number?'
   }, { enabled: true, interval: 30 * 1000 }),
-  pubsub: optional(s({
-    enabled: 'boolean?'
-  })),
   init: optional(union(['boolean', s({
     bits: 'number?',
     emptyRepo: 'boolean?',
@@ -72,7 +69,8 @@ const configSchema = s({
     })),
     Bootstrap: optional(s(['multiaddr-ipfs'])),
     Pubsub: optional(s({
-      Router: 'string?'
+      Router: 'string?',
+      Enabled: 'boolean?'
     })),
     Swarm: optional(s({
       ConnMgr: optional(s({
diff --git a/src/core/index.js b/src/core/index.js
@@ -15,7 +15,6 @@ const multihashing = require('multihashing-async')
 const CID = require('cids')
 const debug = require('debug')
 const mergeOptions = require('merge-options')
-const get = require('dlv')
 const EventEmitter = require('events')
 
 const config = require('./config')
@@ -47,9 +46,6 @@ class IPFS extends EventEmitter {
       init: true,
       start: true,
       EXPERIMENTAL: {},
-      pubsub: {
-        enabled: false
-      },
       preload: {
         enabled: true,
         addresses: [
@@ -135,16 +131,7 @@ class IPFS extends EventEmitter {
     this.stats = components.stats(this)
     this.resolve = components.resolve(this)
 
-    if (this._options.pubsub.enabled) {
-      this.log('pubsub is enabled')
-    }
     if (this._options.EXPERIMENTAL.ipnsPubsub) {
-      // if (!this._options.pubsub.enabled) {
-      if (!get(this._options, 'pubsub.enabled', false)) {
-        this.log('pubsub is enabled to use EXPERIMENTAL IPNS pubsub')
-        this._options.pubsub.enabled = true
-      }
-
       this.log('EXPERIMENTAL IPNS pubsub is enabled')
     }
     if (this._options.EXPERIMENTAL.sharding) {
diff --git a/src/core/runtime/config-browser.js b/src/core/runtime/config-browser.js
@@ -27,6 +27,9 @@ module.exports = () => ({
     '/dns4/node0.preload.ipfs.io/tcp/443/wss/ipfs/QmZMxNdpMkewiVZLMRxaNxUeZpDUb34pWjZ1kZvsd16Zic',
     '/dns4/node1.preload.ipfs.io/tcp/443/wss/ipfs/Qmbut9Ywz9YEDrz8ySBSgWyJk41Uvm2QJPhwDJzJyGFsD6'
   ],
+  Pubsub: {
+    Enabled: true
+  },
   Swarm: {
     ConnMgr: {
       LowWater: 200,
diff --git a/src/core/runtime/config-nodejs.js b/src/core/runtime/config-nodejs.js
@@ -41,7 +41,8 @@ module.exports = () => ({
     '/dns4/node1.preload.ipfs.io/tcp/443/wss/ipfs/Qmbut9Ywz9YEDrz8ySBSgWyJk41Uvm2QJPhwDJzJyGFsD6'
   ],
   Pubsub: {
-    Router: 'gossipsub'
+    Router: 'gossipsub',
+    Enabled: true
   },
   Swarm: {
     ConnMgr: {
diff --git a/src/core/runtime/libp2p-browser.js b/src/core/runtime/libp2p-browser.js
@@ -67,7 +67,7 @@ class Node extends libp2p {
           enabled: false
         },
         pubsub: {
-          enabled: false,
+          enabled: true,
           emitSelf: true
         }
       }
diff --git a/src/core/runtime/libp2p-nodejs.js b/src/core/runtime/libp2p-nodejs.js
@@ -70,7 +70,7 @@ class Node extends libp2p {
           }
         },
         pubsub: {
-          enabled: false,
+          enabled: true,
           emitSelf: true
         }
       }
diff --git a/test/cli/pubsub.js b/test/cli/pubsub.js
@@ -43,8 +43,7 @@ describe('pubsub', function () {
     ipfsdA = await df.spawn({
       exec: IPFS,
       initOptions: { bits: 512 },
-      config,
-      args: ['--enable-pubsub']
+      config
     })
     node = ipfsdA.api
   })
@@ -59,7 +58,6 @@ describe('pubsub', function () {
     const df = DaemonFactory.create({ type: 'js' })
     ipfsdB = await df.spawn({
       initOptions: { bits: 512 },
-      args: ['--enable-pubsub'],
       exec: path.resolve(`${__dirname}/../../src/cli/bin.js`),
       config
     })
diff --git a/test/core/config.spec.js b/test/core/config.spec.js
@@ -154,6 +154,9 @@ describe('config', () => {
       { config: { Swarm: { ConnMgr: undefined } } },
       { config: { Swarm: undefined } },
 
+      { config: { Pubsub: { Enabled: true, Router: 'gossipsub' } } },
+      { config: { Pubsub: { Enabled: false } } },
+
       { config: undefined }
     ]
 
@@ -184,6 +187,8 @@ describe('config', () => {
       { config: { Swarm: { ConnMgr: 138 } } },
       { config: { Swarm: 138 } },
 
+      { config: { Pubsub: { Enabled: 1 } } },
+
       { config: 138 }
     ]
 
diff --git a/test/core/create-node.spec.js b/test/core/create-node.spec.js
@@ -387,6 +387,31 @@ describe('create node', function () {
     })
   })
 
+  it('disable pubsub', function (done) {
+    this.timeout(80 * 1000)
+
+    if (!isNode) { return done() }
+
+    const node = new IPFS({
+      repo: tempRepo,
+      init: { bits: 512 },
+      config: {
+        Pubsub: {
+          Enabled: false
+        }
+      }
+    })
+
+    node.once('start', (err) => {
+      expect(err).to.not.exist()
+      node.pubsub.peers('topic', (err) => {
+        expect(err).to.exist()
+        expect(err.code).to.equal('ERR_PUBSUB_DISABLED')
+        node.stop(done)
+      })
+    })
+  })
+
   it('start and stop, start and stop', function (done) {
     this.timeout(80 * 1000)
 
diff --git a/test/core/interface.spec.js b/test/core/interface.spec.js
@@ -141,7 +141,6 @@ describe('interface-ipfs-core tests', function () {
 
   tests.pubsub(CommonFactory.create({
     spawnOptions: {
-      args: ['--enable-pubsub'],
       initOptions: { bits: 512 }
     }
   }), {
diff --git a/test/core/libp2p.spec.js b/test/core/libp2p.spec.js
@@ -148,7 +148,7 @@ describe('libp2p customization', function () {
             }
           },
           pubsub: {
-            enabled: false,
+            enabled: true,
             emitSelf: true,
             signMessages: true,
             strictSigning: true
diff --git a/test/core/pubsub.spec.js b/test/core/pubsub.spec.js
@@ -24,13 +24,13 @@ describe('pubsub disabled', () => {
       config: {
         Addresses: {
           Swarm: []
+        },
+        Pubsub: {
+          Enabled: false
         }
       },
       preload: {
         enabled: false
-      },
-      pubsub: {
-        enabled: false
       }
     })
 
diff --git a/test/http-api/interface.js b/test/http-api/interface.js
@@ -136,7 +136,6 @@ describe('interface-ipfs-core over ipfs-http-client tests', () => {
 
   tests.pubsub(CommonFactory.create({
     spawnOptions: {
-      args: ['--enable-pubsub'],
       initOptions: { bits: 512 }
     }
   }))
diff --git a/test/http-api/routes.js b/test/http-api/routes.js
@@ -26,7 +26,6 @@ describe('HTTP API', () => {
       repo: repoTests,
       pass: hat(),
       config,
-      pubsub: { enabled: true },
       preload: { enabled: false }
     })
     await ncp(repoExample, repoTests)

Original file line number	Diff line number	Diff line change
`@@ -119,7 +119,7 @@ function defaultBundle ({ datastore, peerInfo, peerBook, options, config }) {`
`119`	`119`	`}`
`120`	`120`	`},`
`121`	`121`	`pubsub: {`
`122`		`- enabled: get(options, 'pubsub.enabled', false)`
	`122`	`+ enabled: get(config, 'Pubsub.Enabled', true)`
`123`	`123`	`}`
`124`	`124`	`},`
`125`	`125`	`connectionManager: get(options, 'connectionManager',`