From e89629e0e0ccd1775aa3fb269ee2fe7981fc6ab4 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Sat, 11 Apr 2020 15:40:28 +0200 Subject: [PATCH 1/2] chore: peer-discovery not using peer-info BREAKING CHANGE: peer event emits an object with id and multiaddr instead of a peer-info --- package.json | 4 ++-- src/index.js | 24 +++++++++++++++++------- test/compliance.spec.js | 15 +++++++++++---- test/index.spec.js | 39 ++++++++++++++++++++++++--------------- 4 files changed, 54 insertions(+), 28 deletions(-) diff --git a/package.json b/package.json index 48184c1..87072a8 100644 --- a/package.json +++ b/package.json @@ -41,8 +41,9 @@ "devDependencies": { "aegir": "^21.4.5", "chai": "^4.2.0", + "chai-bytes": "^0.1.2", "dirty-chai": "^2.0.1", - "libp2p-interfaces": "^0.2.7", + "libp2p-interfaces": "libp2p/js-interfaces#v0.3.x", "p-defer": "^3.0.0", "p-wait-for": "^3.1.0", "sinon": "^9.0.1" @@ -52,7 +53,6 @@ "emittery": "^0.6.0", "multiaddr": "^7.4.3", "peer-id": "^0.13.11", - "peer-info": "^0.17.5", "protons": "^1.0.2" }, "contributors": [ diff --git a/src/index.js b/src/index.js index 05dedb3..29da24f 100644 --- a/src/index.js +++ b/src/index.js @@ -4,7 +4,6 @@ const Emittery = require('emittery') const debug = require('debug') const PeerId = require('peer-id') -const PeerInfo = require('peer-info') const multiaddr = require('multiaddr') const PB = require('./peer.proto') @@ -31,6 +30,7 @@ class PubsubPeerDiscovery extends Emittery { * @constructor * @param {Libp2p} param0.libp2p Our libp2p node * @param {number} [param0.interval = 10000] How often (ms) we should broadcast our infos + * @param {Array} param0.multiaddrs Multiaddrs used by the node to listen. * @param {Array} [param0.topics = PubsubPeerDiscovery.TOPIC] What topics to subscribe to. If set, the default will NOT be used. * @param {boolean} [param0.listenOnly = false] If true, we will not broadcast our peer data */ @@ -38,10 +38,12 @@ class PubsubPeerDiscovery extends Emittery { libp2p, interval = 10000, topics, + multiaddrs = [], listenOnly = false }) { super() this.libp2p = libp2p + this.multiaddrs = multiaddrs this.interval = interval this._intervalId = null this._listenOnly = listenOnly @@ -54,6 +56,7 @@ class PubsubPeerDiscovery extends Emittery { } this.removeListener = this.off.bind(this) + this.removeAllListeners = this.clearListeners.bind(this) } /** @@ -97,8 +100,8 @@ class PubsubPeerDiscovery extends Emittery { */ _broadcast () { const peer = { - publicKey: this.libp2p.peerInfo.id.pubKey.bytes, - addrs: this.libp2p.peerInfo.multiaddrs.toArray().map(ma => ma.buffer) + publicKey: this.libp2p.peerId.pubKey.bytes, + addrs: this.multiaddrs.map(ma => ma.buffer) } const encodedPeer = PB.Peer.encode(peer) @@ -120,16 +123,23 @@ class PubsubPeerDiscovery extends Emittery { const peerId = await PeerId.createFromPubKey(peer.publicKey) // Ignore if we received our own response - if (peerId.equals(this.libp2p.peerInfo.id)) return + if (peerId.equals(this.libp2p.peerId)) return - const peerInfo = new PeerInfo(peerId) - peer.addrs.forEach(buffer => peerInfo.multiaddrs.add(multiaddr(buffer))) log('discovered peer %j on %j', peerId, message.topicIDs) - this.emit('peer', peerInfo) + this._onNewPeer(peerId, peer.addrs) } catch (err) { log.error(err) } } + + _onNewPeer (peerId, addrs) { + if (!this._intervalId) return + + this.emit('peer', { + id: peerId, + multiaddrs: addrs.map(b => multiaddr(b)) + }) + } } PubsubPeerDiscovery.TOPIC = TOPIC diff --git a/test/compliance.spec.js b/test/compliance.spec.js index 3b95fab..550add8 100644 --- a/test/compliance.spec.js +++ b/test/compliance.spec.js @@ -5,17 +5,17 @@ const tests = require('libp2p-interfaces/src/peer-discovery/tests') const PubsubPeerDiscovery = require('../src') const PeerID = require('peer-id') -const PeerInfo = require('peer-info') describe('compliance tests', () => { + let intervalId tests({ async setup () { const peerId = await PeerID.create({ bits: 512 }) - const peerInfo = new PeerInfo(peerId) await new Promise(resolve => setTimeout(resolve, 10)) - return new PubsubPeerDiscovery({ + + const pubsubDiscovery = new PubsubPeerDiscovery({ libp2p: { - peerInfo, + peerId, pubsub: { subscribe: () => {}, unsubscribe: () => {}, @@ -23,9 +23,16 @@ describe('compliance tests', () => { } } }) + + intervalId = setInterval(() => { + pubsubDiscovery._onNewPeer(peerId, ['/ip4/166.10.1.2/tcp/80']) + }, 1000) + + return pubsubDiscovery }, async teardown () { await new Promise(resolve => setTimeout(resolve, 10)) + clearInterval(intervalId) } }) }) diff --git a/test/index.spec.js b/test/index.spec.js index 0c29352..8153bdf 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -3,27 +3,29 @@ const chai = require('chai') chai.use(require('dirty-chai')) +chai.use(require('chai-bytes')) const { expect } = chai const sinon = require('sinon') const defer = require('p-defer') const pWaitFor = require('p-wait-for') +const multiaddr = require('multiaddr') const PeerID = require('peer-id') -const PeerInfo = require('peer-info') const PubsubPeerDiscovery = require('../src') const PB = require('../src/peer.proto') +const listeningMultiaddrs = multiaddr('/ip4/127.0.0.1/tcp/9000/ws') + describe('Pubsub Peer Discovery', () => { let mockLibp2p let discovery before(async () => { - const peerInfo = await PeerInfo.create() - peerInfo.multiaddrs.add('/ip4/127.0.0.1/tcp/9000/ws') + const peerId = await PeerID.create() mockLibp2p = { - peerInfo, + peerId, pubsub: { subscribe: () => {}, publish: () => {}, @@ -38,7 +40,7 @@ describe('Pubsub Peer Discovery', () => { }) it('should not discover self', async () => { - discovery = new PubsubPeerDiscovery({ libp2p: mockLibp2p }) + discovery = new PubsubPeerDiscovery({ libp2p: mockLibp2p, multiaddrs: [listeningMultiaddrs] }) sinon.spy(mockLibp2p.pubsub, 'publish') discovery._broadcast() expect(mockLibp2p.pubsub.publish.callCount).to.equal(1) @@ -46,10 +48,10 @@ describe('Pubsub Peer Discovery', () => { const [topic, encodedPeer] = mockLibp2p.pubsub.publish.getCall(0).args const peer = PB.Peer.decode(encodedPeer) const peerId = await PeerID.createFromPubKey(peer.publicKey) - expect(peerId.equals(mockLibp2p.peerInfo.id)).to.equal(true) + expect(peerId.equals(mockLibp2p.peerId)).to.equal(true) expect(peer.addrs).to.have.length(1) peer.addrs.forEach((addr) => { - expect(mockLibp2p.peerInfo.multiaddrs.has(addr)).to.equal(true) + expect(addr).to.equalBytes(listeningMultiaddrs.buffer) }) expect(topic).to.equal(PubsubPeerDiscovery.TOPIC) @@ -60,14 +62,20 @@ describe('Pubsub Peer Discovery', () => { }) it('should be able to encode/decode a message', async () => { - discovery = new PubsubPeerDiscovery({ libp2p: mockLibp2p }) + discovery = new PubsubPeerDiscovery({ libp2p: mockLibp2p, multiaddrs: [listeningMultiaddrs] }) + discovery.start() + const peerId = await PeerID.create({ bits: 512 }) - const expectedPeerInfo = new PeerInfo(peerId) - expectedPeerInfo.multiaddrs.add('/ip4/0.0.0.0/tcp/8080/ws') - expectedPeerInfo.multiaddrs.add('/ip4/0.0.0.0/tcp/8081/ws') + const expectedPeerData = { + id: peerId, + multiaddrs: [ + '/ip4/0.0.0.0/tcp/8080/ws', + '/ip4/0.0.0.0/tcp/8081/ws' + ] + } const peer = { publicKey: peerId.pubKey.bytes, - addrs: expectedPeerInfo.multiaddrs.toArray().map(ma => ma.buffer) + addrs: expectedPeerData.multiaddrs.map(ma => multiaddr(ma).buffer) } const deferred = defer() @@ -80,9 +88,10 @@ describe('Pubsub Peer Discovery', () => { await discovery._onMessage({ data: encodedPeer, topicIDs: [PubsubPeerDiscovery.TOPIC] }) const discoveredPeer = await deferred.promise - expect(discoveredPeer.id.equals(expectedPeerInfo.id)).to.equal(true) - expectedPeerInfo.multiaddrs.forEach(addr => { - expect(discoveredPeer.multiaddrs.has(addr)).to.equal(true) + expect(discoveredPeer.id.equals(expectedPeerData.id)).to.equal(true) + + discoveredPeer.multiaddrs.forEach(addr => { + expect(expectedPeerData.multiaddrs.includes(addr.toString())).to.equal(true) }) }) From c181710d6f12aa0a76e6ec02ead8133345e5289a Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 21 Apr 2020 13:36:15 +0200 Subject: [PATCH 2/2] chore: address review --- package.json | 2 +- src/index.js | 11 +++++++---- test/compliance.spec.js | 3 +++ test/index.spec.js | 7 +++++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 87072a8..8334edd 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "chai": "^4.2.0", "chai-bytes": "^0.1.2", "dirty-chai": "^2.0.1", - "libp2p-interfaces": "libp2p/js-interfaces#v0.3.x", + "libp2p-interfaces": "^0.3.0", "p-defer": "^3.0.0", "p-wait-for": "^3.1.0", "sinon": "^9.0.1" diff --git a/src/index.js b/src/index.js index 29da24f..77687f2 100644 --- a/src/index.js +++ b/src/index.js @@ -30,7 +30,6 @@ class PubsubPeerDiscovery extends Emittery { * @constructor * @param {Libp2p} param0.libp2p Our libp2p node * @param {number} [param0.interval = 10000] How often (ms) we should broadcast our infos - * @param {Array} param0.multiaddrs Multiaddrs used by the node to listen. * @param {Array} [param0.topics = PubsubPeerDiscovery.TOPIC] What topics to subscribe to. If set, the default will NOT be used. * @param {boolean} [param0.listenOnly = false] If true, we will not broadcast our peer data */ @@ -38,12 +37,10 @@ class PubsubPeerDiscovery extends Emittery { libp2p, interval = 10000, topics, - multiaddrs = [], listenOnly = false }) { super() this.libp2p = libp2p - this.multiaddrs = multiaddrs this.interval = interval this._intervalId = null this._listenOnly = listenOnly @@ -101,7 +98,7 @@ class PubsubPeerDiscovery extends Emittery { _broadcast () { const peer = { publicKey: this.libp2p.peerId.pubKey.bytes, - addrs: this.multiaddrs.map(ma => ma.buffer) + addrs: this.libp2p.transportManager.getAddrs().map(ma => ma.buffer) } const encodedPeer = PB.Peer.encode(peer) @@ -132,6 +129,12 @@ class PubsubPeerDiscovery extends Emittery { } } + /** + * Emit discovered peers. + * @private + * @param {PeerId} peerId + * @param {Array} addrs + */ _onNewPeer (peerId, addrs) { if (!this._intervalId) return diff --git a/test/compliance.spec.js b/test/compliance.spec.js index 550add8..bd59ac1 100644 --- a/test/compliance.spec.js +++ b/test/compliance.spec.js @@ -15,6 +15,9 @@ describe('compliance tests', () => { const pubsubDiscovery = new PubsubPeerDiscovery({ libp2p: { + transportManager: { + getAddrs: () => [] + }, peerId, pubsub: { subscribe: () => {}, diff --git a/test/index.spec.js b/test/index.spec.js index 8153bdf..c1f0709 100644 --- a/test/index.spec.js +++ b/test/index.spec.js @@ -26,6 +26,9 @@ describe('Pubsub Peer Discovery', () => { mockLibp2p = { peerId, + transportManager: { + getAddrs: () => [listeningMultiaddrs] + }, pubsub: { subscribe: () => {}, publish: () => {}, @@ -40,7 +43,7 @@ describe('Pubsub Peer Discovery', () => { }) it('should not discover self', async () => { - discovery = new PubsubPeerDiscovery({ libp2p: mockLibp2p, multiaddrs: [listeningMultiaddrs] }) + discovery = new PubsubPeerDiscovery({ libp2p: mockLibp2p }) sinon.spy(mockLibp2p.pubsub, 'publish') discovery._broadcast() expect(mockLibp2p.pubsub.publish.callCount).to.equal(1) @@ -62,7 +65,7 @@ describe('Pubsub Peer Discovery', () => { }) it('should be able to encode/decode a message', async () => { - discovery = new PubsubPeerDiscovery({ libp2p: mockLibp2p, multiaddrs: [listeningMultiaddrs] }) + discovery = new PubsubPeerDiscovery({ libp2p: mockLibp2p }) discovery.start() const peerId = await PeerID.create({ bits: 512 })