From 21a63cbd6cf021201658c9475efbd80fe4ec9828 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 7 Apr 2020 12:16:17 +0200 Subject: [PATCH 1/3] chore: remove peer-info usage BREAKING CHANGE: pubsub internal peer does not have info propery anymore and use the new topology api with peer-id instead of peer-info --- package.json | 5 ++-- src/index.js | 56 +++++++++++++++++++++---------------------- src/peer.js | 15 ++++++++---- test/instance.spec.js | 16 ++++++------- test/pubsub.spec.js | 56 +++++++++++++++++++++---------------------- test/utils/index.js | 9 ++++--- 6 files changed, 79 insertions(+), 78 deletions(-) diff --git a/package.json b/package.json index f1e3ef5225..a9cfee3f98 100644 --- a/package.json +++ b/package.json @@ -49,8 +49,6 @@ "dirty-chai": "^2.0.1", "it-pair": "^1.0.0", "multiaddr": "^7.2.1", - "peer-id": "~0.13.3", - "peer-info": "~0.17.0", "sinon": "^9.0.0" }, "dependencies": { @@ -61,7 +59,8 @@ "it-pipe": "^1.0.1", "it-pushable": "^1.3.2", "libp2p-crypto": "~0.17.0", - "libp2p-interfaces": "^0.2.3", + "libp2p-interfaces": "libp2p/js-interfaces#v0.3.x", + "peer-id": "~0.13.3", "protons": "^1.0.1" }, "contributors": [ diff --git a/src/index.js b/src/index.js index 6765134015..930ac289db 100644 --- a/src/index.js +++ b/src/index.js @@ -4,7 +4,7 @@ const debug = require('debug') const EventEmitter = require('events') const errcode = require('err-code') -const PeerInfo = require('peer-info') +const PeerId = require('peer-id') const MulticodecTopology = require('libp2p-interfaces/src/topology/multicodec-topology') const message = require('./message') @@ -42,7 +42,7 @@ class PubsubBaseProtocol extends EventEmitter { * @param {Object} props * @param {String} props.debugName log namespace * @param {Array|string} props.multicodecs protocol identificers to connect - * @param {PeerInfo} props.peerInfo peer's peerInfo + * @param {PeerId} props.peerId peer's peerId * @param {Object} props.registrar registrar for libp2p protocols * @param {function} props.registrar.handle * @param {function} props.registrar.register @@ -54,7 +54,7 @@ class PubsubBaseProtocol extends EventEmitter { constructor ({ debugName, multicodecs, - peerInfo, + peerId, registrar, signMessages = true, strictSigning = true @@ -67,8 +67,8 @@ class PubsubBaseProtocol extends EventEmitter { throw new Error('multicodecs are required') } - if (!PeerInfo.isPeerInfo(peerInfo)) { - throw new Error('peer info must be an instance of `peer-info`') + if (!PeerId.isPeerId(peerId)) { + throw new Error('peerId must be an instance of `peer-id`') } validateRegistrar(registrar) @@ -79,11 +79,13 @@ class PubsubBaseProtocol extends EventEmitter { this.log.err = debug(`${debugName}:error`) this.multicodecs = utils.ensureArray(multicodecs) - this.peerInfo = peerInfo + this.signMessages = peerId this.registrar = registrar this.started = false + this.peerId = peerId + /** * Map of topics to which peers are subscribed to * @@ -99,9 +101,7 @@ class PubsubBaseProtocol extends EventEmitter { this.peers = new Map() // Message signing - if (signMessages) { - this.peerId = this.peerInfo.id - } + this.signMessages = signMessages /** * If message signing should be required for incoming messages @@ -170,13 +170,11 @@ class PubsubBaseProtocol extends EventEmitter { * @param {DuplexStream} props.strean * @param {Connection} props.connection connection */ - async _onIncomingStream ({ protocol, stream, connection }) { - const peerInfo = await PeerInfo.create(connection.remotePeer) - peerInfo.protocols.add(protocol) - - const idB58Str = peerInfo.id.toB58String() + _onIncomingStream ({ protocol, stream, connection }) { + const peerId = connection.remotePeer + const idB58Str = peerId.toB58String() - const peer = this._addPeer(new Peer(peerInfo)) + const peer = this._addPeer(new Peer(peerId, [protocol])) peer.attachConnection(stream) this._processMessages(idB58Str, stream, peer) @@ -185,14 +183,14 @@ class PubsubBaseProtocol extends EventEmitter { /** * Registrar notifies a connection successfully with pubsub protocol. * @private - * @param {PeerInfo} peerInfo remote peer info + * @param {PeerId} peerId remote peer-id * @param {Connection} conn connection to the peer */ - async _onPeerConnected (peerInfo, conn) { - const idB58Str = peerInfo.id.toB58String() + async _onPeerConnected (peerId, conn) { + const idB58Str = peerId.toB58String() this.log('connected', idB58Str) - const peer = this._addPeer(new Peer(peerInfo)) + const peer = this._addPeer(new Peer(peerId, this.multicodecs)) try { const { stream } = await conn.newStream(this.multicodecs) peer.attachConnection(stream) @@ -205,11 +203,11 @@ class PubsubBaseProtocol extends EventEmitter { /** * Registrar notifies a closing connection with pubsub protocol. * @private - * @param {PeerInfo} peerInfo peer info + * @param {PeerId} peerId peerId * @param {Error} err error for connection end */ - _onPeerDisconnected (peerInfo, err) { - const idB58Str = peerInfo.id.toB58String() + _onPeerDisconnected (peerId, err) { + const idB58Str = peerId.toB58String() const peer = this.peers.get(idB58Str) this.log('connection ended', idB58Str, err ? err.message : '') @@ -219,11 +217,11 @@ class PubsubBaseProtocol extends EventEmitter { /** * Add a new connected peer to the peers map. * @private - * @param {PeerInfo} peer peer info - * @returns {PeerInfo} + * @param {Peer} peer internal peer + * @returns {Peer} */ _addPeer (peer) { - const id = peer.info.id.toB58String() + const id = peer.id.toB58String() let existing = this.peers.get(id) if (!existing) { @@ -242,11 +240,11 @@ class PubsubBaseProtocol extends EventEmitter { * Remove a peer from the peers map. * @private * @param {Peer} peer peer state - * @returns {PeerInfo} + * @returns {Peer} */ _removePeer (peer) { if (!peer) return - const id = peer.info.id.toB58String() + const id = peer.id.toB58String() this.log('remove', id, peer._references) @@ -287,7 +285,7 @@ class PubsubBaseProtocol extends EventEmitter { */ _buildMessage (message) { const msg = utils.normalizeOutRpcMessage(message) - if (this.peerId) { + if (this.signMessages) { return signMessage(this.peerId, msg) } else { return message @@ -310,7 +308,7 @@ class PubsubBaseProtocol extends EventEmitter { return Array.from(this.peers.values()) .filter((peer) => peer.topics.has(topic)) - .map((peer) => peer.info.id.toB58String()) + .map((peer) => peer.id.toB58String()) } /** diff --git a/src/peer.js b/src/peer.js index efaa067bbb..aa016b3a7b 100644 --- a/src/peer.js +++ b/src/peer.js @@ -13,15 +13,20 @@ const { RPC } = require('./message') */ class Peer extends EventEmitter { /** - * @param {PeerInfo} info + * @param {PeerId} id + * @param {Array} protocols */ - constructor (info) { + constructor (id, protocols) { super() /** - * @type {PeerInfo} + * @type {PeerId} */ - this.info = info + this.id = id + /** + * @type {string} + */ + this.protocols = protocols /** * @type {Connection} */ @@ -65,7 +70,7 @@ class Peer extends EventEmitter { */ write (msg) { if (!this.isWritable) { - const id = this.info.id.toB58String() + const id = this.id.toB58String() throw new Error('No writable connection to ' + id) } diff --git a/test/instance.spec.js b/test/instance.spec.js index e57a1076ee..9a9760f34a 100644 --- a/test/instance.spec.js +++ b/test/instance.spec.js @@ -7,13 +7,13 @@ chai.use(require('chai-spies')) const expect = chai.expect const PubsubBaseProtocol = require('../src') -const { createPeerInfo, mockRegistrar } = require('./utils') +const { createPeerId, mockRegistrar } = require('./utils') describe('should validate instance parameters', () => { - let peerInfo + let peerId before(async () => { - peerInfo = await createPeerInfo() + peerId = await createPeerId() }) it('should throw if no debugName is provided', () => { @@ -30,7 +30,7 @@ describe('should validate instance parameters', () => { }).to.throw() }) - it('should throw if no peerInfo is provided', () => { + it('should throw if no peerId is provided', () => { expect(() => { new PubsubBaseProtocol({ // eslint-disable-line no-new debugName: 'pubsub', @@ -39,12 +39,12 @@ describe('should validate instance parameters', () => { }).to.throw() }) - it('should throw if an invalid peerInfo is provided', () => { + it('should throw if an invalid peerId is provided', () => { expect(() => { new PubsubBaseProtocol({ // eslint-disable-line no-new debugName: 'pubsub', multicodecs: '/pubsub/1.0.0', - peerInfo: 'fake-peer-info' + peerId: 'fake-peer-id' }) }).to.throw() }) @@ -54,7 +54,7 @@ describe('should validate instance parameters', () => { new PubsubBaseProtocol({ // eslint-disable-line no-new debugName: 'pubsub', multicodecs: '/pubsub/1.0.0', - peerInfo: peerInfo + peerId: peerId }) }).to.throw() }) @@ -64,7 +64,7 @@ describe('should validate instance parameters', () => { new PubsubBaseProtocol({ // eslint-disable-line no-new debugName: 'pubsub', multicodecs: '/pubsub/1.0.0', - peerInfo: peerInfo, + peerId: peerId, registrar: mockRegistrar }) }).not.to.throw() diff --git a/test/pubsub.spec.js b/test/pubsub.spec.js index 2b989e8242..39f5ebdc58 100644 --- a/test/pubsub.spec.js +++ b/test/pubsub.spec.js @@ -11,7 +11,7 @@ const PubsubBaseProtocol = require('../src') const Peer = require('../src/peer') const { randomSeqno } = require('../src/utils') const { - createPeerInfo, + createPeerId, createMockRegistrar, mockRegistrar, PubsubImplementation, @@ -24,7 +24,7 @@ describe('pubsub base protocol', () => { let sinonMockRegistrar beforeEach(async () => { - const peerInfo = await createPeerInfo() + const peerId = await createPeerId() sinonMockRegistrar = { handle: sinon.stub(), register: sinon.stub(), @@ -34,7 +34,7 @@ describe('pubsub base protocol', () => { pubsub = new PubsubBaseProtocol({ debugName: 'pubsub', multicodecs: '/pubsub/1.0.0', - peerInfo: peerInfo, + peerId: peerId, registrar: sinonMockRegistrar }) @@ -72,15 +72,15 @@ describe('pubsub base protocol', () => { }) describe('should handle message creation and signing', () => { - let peerInfo + let peerId let pubsub before(async () => { - peerInfo = await createPeerInfo() + peerId = await createPeerId() pubsub = new PubsubBaseProtocol({ debugName: 'pubsub', multicodecs: '/pubsub/1.0.0', - peerInfo: peerInfo, + peerId: peerId, registrar: mockRegistrar }) }) @@ -91,7 +91,7 @@ describe('pubsub base protocol', () => { it('_buildMessage normalizes and signs messages', async () => { const message = { - from: peerInfo.id.id, + from: peerId.id, data: 'hello', seqno: randomSeqno(), topicIDs: ['test-topic'] @@ -105,7 +105,7 @@ describe('pubsub base protocol', () => { it('validate with strict signing off will validate a present signature', async () => { const message = { - from: peerInfo.id.id, + from: peerId.id, data: 'hello', seqno: randomSeqno(), topicIDs: ['test-topic'] @@ -121,7 +121,7 @@ describe('pubsub base protocol', () => { it('validate with strict signing requires a signature', async () => { const message = { - from: peerInfo.id.id, + from: peerId.id, data: 'hello', seqno: randomSeqno(), topicIDs: ['test-topic'] @@ -136,17 +136,17 @@ describe('pubsub base protocol', () => { describe('should be able to register two nodes', () => { const protocol = '/pubsub/1.0.0' let pubsubA, pubsubB - let peerInfoA, peerInfoB + let peerIdA, peerIdB const registrarRecordA = {} const registrarRecordB = {} // mount pubsub beforeEach(async () => { - peerInfoA = await createPeerInfo() - peerInfoB = await createPeerInfo() + peerIdA = await createPeerId() + peerIdB = await createPeerId() - pubsubA = new PubsubImplementation(protocol, peerInfoA, createMockRegistrar(registrarRecordA)) - pubsubB = new PubsubImplementation(protocol, peerInfoB, createMockRegistrar(registrarRecordB)) + pubsubA = new PubsubImplementation(protocol, peerIdA, createMockRegistrar(registrarRecordA)) + pubsubB = new PubsubImplementation(protocol, peerIdB, createMockRegistrar(registrarRecordB)) }) // start pubsub @@ -176,12 +176,12 @@ describe('pubsub base protocol', () => { // Notice peers of connection const [c0, c1] = ConnectionPair() - await onConnectA(peerInfoB, c0) + await onConnectA(peerIdB, c0) await handlerB({ protocol, stream: c1.stream, connection: { - remotePeer: peerInfoA.id + remotePeer: peerIdA } }) @@ -198,12 +198,12 @@ describe('pubsub base protocol', () => { const error = new Error('new stream error') sinon.stub(c0, 'newStream').throws(error) - await onConnectA(peerInfoB, c0) + await onConnectA(peerIdB, c0) await handlerB({ protocol, stream: c1.stream, connection: { - remotePeer: peerInfoA.id + remotePeer: peerIdA } }) @@ -219,18 +219,18 @@ describe('pubsub base protocol', () => { // Notice peers of connection const [c0, c1] = ConnectionPair() - await onConnectA(peerInfoB, c0) + await onConnectA(peerIdB, c0) await handlerB({ protocol, stream: c1.stream, connection: { - remotePeer: peerInfoA.id + remotePeer: peerIdA } }) // Notice peers of disconnect - onDisconnectA(peerInfoB) - onDisconnectB(peerInfoA) + onDisconnectA(peerIdB) + onDisconnectB(peerIdA) expect(pubsubA.peers.size).to.be.eql(0) expect(pubsubB.peers.size).to.be.eql(0) @@ -242,22 +242,22 @@ describe('pubsub base protocol', () => { expect(pubsubA.peers.size).to.be.eql(0) // Notice peers of disconnect - onDisconnectA(peerInfoB) + onDisconnectA(peerIdB) expect(pubsubA.peers.size).to.be.eql(0) }) }) describe('getSubscribers', () => { - let peerInfo + let peerId let pubsub beforeEach(async () => { - peerInfo = await createPeerInfo() + peerId = await createPeerId() pubsub = new PubsubBaseProtocol({ debugName: 'pubsub', multicodecs: '/pubsub/1.0.0', - peerInfo: peerInfo, + peerId: peerId, registrar: mockRegistrar }) }) @@ -301,8 +301,8 @@ describe('pubsub base protocol', () => { expect(peersSubscribed).to.be.empty() // Set mock peer subscribed - const peer = new Peer(peerInfo) - const id = peer.info.id.toB58String() + const peer = new Peer(peerId) + const id = peer.id.toB58String() peer.topics.add(topic) pubsub.peers.set(id, peer) diff --git a/test/utils/index.js b/test/utils/index.js index cc41275999..3279798467 100644 --- a/test/utils/index.js +++ b/test/utils/index.js @@ -5,23 +5,22 @@ const pipe = require('it-pipe') const DuplexPair = require('it-pair/duplex') const PeerId = require('peer-id') -const PeerInfo = require('peer-info') const PubsubBaseProtocol = require('../../src') const { message } = require('../../src') -exports.createPeerInfo = async () => { +exports.createPeerId = async () => { const peerId = await PeerId.create({ bits: 1024 }) - return PeerInfo.create(peerId) + return peerId } class PubsubImplementation extends PubsubBaseProtocol { - constructor (protocol, peerInfo, registrar) { + constructor (protocol, peerId, registrar) { super({ debugName: 'libp2p:pubsub', multicodecs: protocol, - peerInfo: peerInfo, + peerId: peerId, registrar: registrar }) } From 7aae30673f6b63637a123a55ff2d37fe472817e9 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 21 Apr 2020 13:40:19 +0200 Subject: [PATCH 2/3] chore: apply suggestions from code review Co-Authored-By: Jacob Heun --- package.json | 2 +- src/index.js | 12 +++++++++--- src/peer.js | 2 +- test/pubsub.spec.js | 2 +- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index a9cfee3f98..3f0316aa2a 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "it-pipe": "^1.0.1", "it-pushable": "^1.3.2", "libp2p-crypto": "~0.17.0", - "libp2p-interfaces": "libp2p/js-interfaces#v0.3.x", + "libp2p-interfaces": "^0.3.0", "peer-id": "~0.13.3", "protons": "^1.0.1" }, diff --git a/src/index.js b/src/index.js index 930ac289db..4429b65be1 100644 --- a/src/index.js +++ b/src/index.js @@ -79,7 +79,6 @@ class PubsubBaseProtocol extends EventEmitter { this.log.err = debug(`${debugName}:error`) this.multicodecs = utils.ensureArray(multicodecs) - this.signMessages = peerId this.registrar = registrar this.started = false @@ -174,7 +173,10 @@ class PubsubBaseProtocol extends EventEmitter { const peerId = connection.remotePeer const idB58Str = peerId.toB58String() - const peer = this._addPeer(new Peer(peerId, [protocol])) + const peer = this._addPeer(new Peer({ + id: peerId, + protocols: [protocol] + })) peer.attachConnection(stream) this._processMessages(idB58Str, stream, peer) @@ -190,7 +192,11 @@ class PubsubBaseProtocol extends EventEmitter { const idB58Str = peerId.toB58String() this.log('connected', idB58Str) - const peer = this._addPeer(new Peer(peerId, this.multicodecs)) + const peer = this._addPeer(new Peer({ + id: peerId, + protocols: this.multicodecs + })) + try { const { stream } = await conn.newStream(this.multicodecs) peer.attachConnection(stream) diff --git a/src/peer.js b/src/peer.js index aa016b3a7b..2e764313a2 100644 --- a/src/peer.js +++ b/src/peer.js @@ -16,7 +16,7 @@ class Peer extends EventEmitter { * @param {PeerId} id * @param {Array} protocols */ - constructor (id, protocols) { + constructor ({ id, protocols }) { super() /** diff --git a/test/pubsub.spec.js b/test/pubsub.spec.js index 39f5ebdc58..a74b6b79de 100644 --- a/test/pubsub.spec.js +++ b/test/pubsub.spec.js @@ -301,7 +301,7 @@ describe('pubsub base protocol', () => { expect(peersSubscribed).to.be.empty() // Set mock peer subscribed - const peer = new Peer(peerId) + const peer = new Peer({ id: peerId }) const id = peer.id.toB58String() peer.topics.add(topic) From 071468f3cdedd299216029980d04c625ea00fd59 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Wed, 22 Apr 2020 16:15:37 +0200 Subject: [PATCH 3/3] chore: update readme constructor example --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9ccb61f20b..56a63aa3aa 100644 --- a/README.md +++ b/README.md @@ -50,11 +50,11 @@ TODO: add explanation for registrar! const Pubsub = require('libp2p-pubsub') class PubsubImplementation extends Pubsub { - constructor({ peerInfo, registrar, ...options }) + constructor({ peerId, registrar, ...options }) super({ debugName: 'libp2p:pubsub', multicodecs: '/pubsub-implementation/1.0.0', - peerInfo: peerInfo, + peerId: peerId, registrar: registrar, signMessages: options.signMessages, strictSigning: options.strictSigning