From a16b0d368becf367638cfd7c7531ac0ea1ea5a4e Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Thu, 4 Apr 2019 15:26:57 +0200 Subject: [PATCH 1/6] fix: remove unncessary puts, switch handles it --- src/index.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/index.js b/src/index.js index 0496401c63..315fec1120 100644 --- a/src/index.js +++ b/src/index.js @@ -78,7 +78,6 @@ class Node extends EventEmitter { // reuse this muxed connection this._switch.on('peer-mux-established', (peerInfo) => { this.emit('peer:connect', peerInfo) - this.peerBook.put(peerInfo) }) this._switch.on('peer-mux-closed', (peerInfo) => { @@ -246,7 +245,6 @@ class Node extends EventEmitter { this._switch.dial(peerInfo, protocol, (err, conn) => { if (err) { return callback(err) } - this.peerBook.put(peerInfo) callback(null, conn) }) }) @@ -275,9 +273,6 @@ class Node extends EventEmitter { if (err) { return callback(err) } this._switch.dialFSM(peerInfo, protocol, (err, connFSM) => { - if (!err) { - this.peerBook.put(peerInfo) - } callback(err, connFSM) }) }) From 0f25975993d5e2c6deb997f68ac1336d9311a97c Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Thu, 4 Apr 2019 15:27:13 +0200 Subject: [PATCH 2/6] test: fix outdated tests --- package.json | 2 +- test/circuit-relay.node.js | 10 ++-- test/peer-discovery.node.js | 107 +++++++++++++++++++++++++++--------- 3 files changed, 85 insertions(+), 34 deletions(-) diff --git a/package.json b/package.json index 15db9687b8..99baddd9be 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "libp2p-connection-manager": "~0.0.2", "libp2p-floodsub": "~0.15.8", "libp2p-ping": "~0.8.5", - "libp2p-switch": "~0.42.1", + "libp2p-switch": "~0.42.7", "libp2p-websockets": "~0.12.2", "mafmt": "^6.0.7", "multiaddr": "^6.0.6", diff --git a/test/circuit-relay.node.js b/test/circuit-relay.node.js index 764db00ce7..0caf05f2b3 100644 --- a/test/circuit-relay.node.js +++ b/test/circuit-relay.node.js @@ -114,10 +114,9 @@ describe('circuit relay', () => { nodeWS2 = node cb() }), - // set up node with TCP and listening on relay1 + // set up node with TCP (cb) => setupNode([ - '/ip4/0.0.0.0/tcp/0', - `/p2p/${relayNode1.peerInfo.id.toB58String()}/p2p-circuit` + '/ip4/0.0.0.0/tcp/0' ], { config: { relay: { @@ -128,10 +127,9 @@ describe('circuit relay', () => { nodeTCP1 = node cb() }), - // set up node with TCP and listening on relay2 over TCP transport + // set up node with TCP (cb) => setupNode([ - '/ip4/0.0.0.0/tcp/0', - `/ip4/0.0.0.0/tcp/0/p2p/${relayNode2.peerInfo.id.toB58String()}/p2p-circuit` + '/ip4/0.0.0.0/tcp/0' ], { config: { relay: { diff --git a/test/peer-discovery.node.js b/test/peer-discovery.node.js index b1aaa53e0e..22f4180afd 100644 --- a/test/peer-discovery.node.js +++ b/test/peer-discovery.node.js @@ -9,15 +9,13 @@ const signalling = require('libp2p-webrtc-star/src/sig-server') const parallel = require('async/parallel') const crypto = require('crypto') -const PeerId = require('peer-id') -const PeerInfo = require('peer-info') - const createNode = require('./utils/create-node') const echo = require('./utils/echo') describe('peer discovery', () => { let nodeA let nodeB + let nodeC let port = 24642 let ss @@ -49,6 +47,15 @@ describe('peer discovery', () => { nodeB = node node.handle('/echo/1.0.0', echo) node.start(cb) + }), + (cb) => createNode([ + '/ip4/0.0.0.0/tcp/0', + `/ip4/127.0.0.1/tcp/${port}/ws/p2p-webrtc-star` + ], options, (err, node) => { + expect(err).to.not.exist() + nodeC = node + node.handle('/echo/1.0.0', echo) + node.start(cb) }) ], done) }) @@ -57,6 +64,7 @@ describe('peer discovery', () => { parallel([ (cb) => nodeA.stop(cb), (cb) => nodeB.stop(cb), + (cb) => nodeC.stop(cb), (cb) => ss.stop(cb) ], done) }) @@ -250,6 +258,7 @@ describe('peer discovery', () => { peerDiscovery: { mdns: { enabled: true, + interval: 1e3, // discover quickly // use a random tag to prevent CI collision serviceTag: crypto.randomBytes(10).toString('hex') } @@ -257,13 +266,23 @@ describe('peer discovery', () => { } }) - it('find a peer', function (done) { - this.timeout(15 * 1000) + it('find peers', function (done) { + let expectedPeers = new Set([ + nodeB.peerInfo.id.toB58String(), + nodeC.peerInfo.id.toB58String() + ]) - nodeA.once('peer:discovery', (peerInfo) => { - expect(nodeB.peerInfo.id.toB58String()) - .to.eql(peerInfo.id.toB58String()) + function finish () { + nodeA.removeAllListeners('peer:discovery') + expect(expectedPeers.size).to.eql(0) done() + } + + nodeA.on('peer:discovery', (peerInfo) => { + expectedPeers.delete(peerInfo.id.toB58String()) + if (expectedPeers.size === 0) { + finish() + } }) }) }) @@ -283,12 +302,24 @@ describe('peer discovery', () => { } }) - it('find a peer', function (done) { - this.timeout(15 * 1000) - nodeA.once('peer:discovery', (peerInfo) => { - expect(nodeB.peerInfo.id.toB58String()) - .to.eql(peerInfo.id.toB58String()) + it('find peers', function (done) { + this.timeout(20e3) + let expectedPeers = new Set([ + nodeB.peerInfo.id.toB58String(), + nodeC.peerInfo.id.toB58String() + ]) + + function finish () { + nodeA.removeAllListeners('peer:discovery') + expect(expectedPeers.size).to.eql(0) done() + } + + nodeA.on('peer:discovery', (peerInfo) => {s + expectedPeers.delete(peerInfo.id.toB58String()) + if (expectedPeers.size === 0) { + finish() + } }) }) }) @@ -302,6 +333,7 @@ describe('peer discovery', () => { peerDiscovery: { mdns: { enabled: true, + interval: 1e3, // discovery quickly // use a random tag to prevent CI collision serviceTag: crypto.randomBytes(10).toString('hex') }, @@ -312,12 +344,23 @@ describe('peer discovery', () => { } }) - it('find a peer', function (done) { - this.timeout(15 * 1000) - nodeA.once('peer:discovery', (peerInfo) => { - expect(nodeB.peerInfo.id.toB58String()) - .to.eql(peerInfo.id.toB58String()) + it('find peers', function (done) { + let expectedPeers = new Set([ + nodeB.peerInfo.id.toB58String(), + nodeC.peerInfo.id.toB58String() + ]) + + function finish () { + nodeA.removeAllListeners('peer:discovery') + expect(expectedPeers.size).to.eql(0) done() + } + + nodeA.on('peer:discovery', (peerInfo) => { + expectedPeers.delete(peerInfo.id.toB58String()) + if (expectedPeers.size === 0) { + finish() + } }) }) }) @@ -332,24 +375,34 @@ describe('peer discovery', () => { webRTCStar: { enabled: false } + }, + dht: { + enabled: true, + kBucketSize: 20, + randomWalk: { + enabled: true, + queriesPerPeriod: 1, + interval: 1000, // start the query sooner + timeout: 3000 + } } } }) - it('find a peer', function (done) { - this.timeout(15 * 1000) - + it('find a peer through another dht node', function (done) { nodeA.once('peer:discovery', (peerInfo) => { - expect(nodeB.peerInfo.id.toB58String()) + expect(nodeC.peerInfo.id.toB58String()) .to.eql(peerInfo.id.toB58String()) done() }) - // connect two dhts - const publicPeerId = new PeerId(nodeB._dht.peerInfo.id.id, null, nodeB._dht.peerInfo.id.pubKey) - const target = new PeerInfo(publicPeerId) - target.multiaddrs = nodeB._dht.peerInfo.multiaddrs - nodeA._dht.switch.dial(target, (err) => { + // Topology: + // A -> B + // C -> B + nodeA.dial(nodeB.peerInfo, (err) => { + expect(err).to.not.exist() + }) + nodeC.dial(nodeB.peerInfo, (err) => { expect(err).to.not.exist() }) }) From d3d125ca719ad63bd3ef36d51e5fb36c2081d545 Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Thu, 4 Apr 2019 15:56:12 +0200 Subject: [PATCH 3/6] chore: remove rogue char --- test/peer-discovery.node.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/peer-discovery.node.js b/test/peer-discovery.node.js index 22f4180afd..48bc11d33a 100644 --- a/test/peer-discovery.node.js +++ b/test/peer-discovery.node.js @@ -315,7 +315,7 @@ describe('peer discovery', () => { done() } - nodeA.on('peer:discovery', (peerInfo) => {s + nodeA.on('peer:discovery', (peerInfo) => { expectedPeers.delete(peerInfo.id.toB58String()) if (expectedPeers.size === 0) { finish() From 6de3e2dfd1a083b8862cb97ddf7c8b4275ca153a Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Thu, 4 Apr 2019 16:20:08 +0200 Subject: [PATCH 4/6] test: improve resilience of test --- test/peer-discovery.node.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/test/peer-discovery.node.js b/test/peer-discovery.node.js index 48bc11d33a..881532049b 100644 --- a/test/peer-discovery.node.js +++ b/test/peer-discovery.node.js @@ -389,11 +389,23 @@ describe('peer discovery', () => { } }) - it('find a peer through another dht node', function (done) { - nodeA.once('peer:discovery', (peerInfo) => { - expect(nodeC.peerInfo.id.toB58String()) - .to.eql(peerInfo.id.toB58String()) + it('find peers through the dht', function (done) { + let expectedPeers = new Set([ + nodeB.peerInfo.id.toB58String(), + nodeC.peerInfo.id.toB58String() + ]) + + function finish () { + nodeA.removeAllListeners('peer:discovery') + expect(expectedPeers.size).to.eql(0) done() + } + + nodeA.on('peer:discovery', (peerInfo) => { + expectedPeers.delete(peerInfo.id.toB58String()) + if (expectedPeers.size === 0) { + finish() + } }) // Topology: From 4c18f09cc4901118c274767a0540df8ec4b275b7 Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Thu, 4 Apr 2019 18:23:58 +0200 Subject: [PATCH 5/6] chore: bump bundlesize allowance --- .aegir.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.aegir.js b/.aegir.js index fac6d00324..e086b67e0c 100644 --- a/.aegir.js +++ b/.aegir.js @@ -79,7 +79,7 @@ const after = (done) => { } module.exports = { - bundlesize: { maxSize: '215kB' }, + bundlesize: { maxSize: '217kB' }, hooks: { pre: before, post: after From f47036faefa7f9b3543824284906e9bf4716c7d1 Mon Sep 17 00:00:00 2001 From: Jacob Heun Date: Fri, 5 Apr 2019 13:32:41 +0200 Subject: [PATCH 6/6] refactor: remove unneeded callback wrappers --- src/index.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/index.js b/src/index.js index 315fec1120..46b949765b 100644 --- a/src/index.js +++ b/src/index.js @@ -243,10 +243,7 @@ class Node extends EventEmitter { this._getPeerInfo(peer, (err, peerInfo) => { if (err) { return callback(err) } - this._switch.dial(peerInfo, protocol, (err, conn) => { - if (err) { return callback(err) } - callback(null, conn) - }) + this._switch.dial(peerInfo, protocol, callback) }) } @@ -272,9 +269,7 @@ class Node extends EventEmitter { this._getPeerInfo(peer, (err, peerInfo) => { if (err) { return callback(err) } - this._switch.dialFSM(peerInfo, protocol, (err, connFSM) => { - callback(err, connFSM) - }) + this._switch.dialFSM(peerInfo, protocol, callback) }) }