From 8f40eb63ad9abc315dde5878f903b4486e70ed7d Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Thu, 24 Sep 2020 12:49:48 +0200 Subject: [PATCH] chore: address review --- src/record/utils.js | 20 ++++++++++++++ src/transport-manager.js | 27 +++---------------- test/dialing/direct.node.js | 23 ++++++++++------ test/identify/index.spec.js | 25 +++--------------- test/transports/transport-manager.node.js | 32 +++++++++++++++++------ 5 files changed, 66 insertions(+), 61 deletions(-) create mode 100644 src/record/utils.js diff --git a/src/record/utils.js b/src/record/utils.js new file mode 100644 index 0000000000..509fea7eec --- /dev/null +++ b/src/record/utils.js @@ -0,0 +1,20 @@ +'use strict' + +const Envelope = require('./envelope') +const PeerRecord = require('./peer-record') + +/** + * Create (or update if existing) self peer record and store it in the AddressBook. + * @param {libp2p} libp2p + * @returns {Promise} + */ +async function updateSelfPeerRecord (libp2p) { + const peerRecord = new PeerRecord({ + peerId: libp2p.peerId, + multiaddrs: libp2p.multiaddrs + }) + const envelope = await Envelope.seal(peerRecord, libp2p.peerId) + libp2p.peerStore.addressBook.consumePeerRecord(envelope) +} + +module.exports.updateSelfPeerRecord = updateSelfPeerRecord diff --git a/src/transport-manager.js b/src/transport-manager.js index 1ea91b1d90..0467b8ead9 100644 --- a/src/transport-manager.js +++ b/src/transport-manager.js @@ -7,8 +7,7 @@ const debug = require('debug') const log = debug('libp2p:transports') log.error = debug('libp2p:transports:error') -const Envelope = require('./record/envelope') -const PeerRecord = require('./record/peer-record') +const { updateSelfPeerRecord } = require('./record/utils') class TransportManager { /** @@ -156,8 +155,8 @@ class TransportManager { this._listeners.get(key).push(listener) // Track listen/close events - listener.on('listening', () => this._createSelfPeerRecord()) - listener.on('close', () => this._createSelfPeerRecord()) + listener.on('listening', () => updateSelfPeerRecord(this.libp2p)) + listener.on('close', () => updateSelfPeerRecord(this.libp2p)) // We need to attempt to listen on everything tasks.push(listener.listen(addr)) @@ -226,26 +225,6 @@ class TransportManager { await Promise.all(tasks) } - - /** - * Create self signed peer record raw envelope. - * @return {Uint8Array} - */ - async _createSelfPeerRecord () { - try { - const peerRecord = new PeerRecord({ - peerId: this.libp2p.peerId, - multiaddrs: this.libp2p.multiaddrs - }) - const envelope = await Envelope.seal(peerRecord, this.libp2p.peerId) - this.libp2p.peerStore.addressBook.consumePeerRecord(envelope) - - return this.libp2p.peerStore.addressBook.getRawEnvelope(this.libp2p.peerId) - } catch (err) { - log.error('failed to get self peer record') - } - return null - } } /** diff --git a/test/dialing/direct.node.js b/test/dialing/direct.node.js index 401f4eb501..a5407f66e6 100644 --- a/test/dialing/direct.node.js +++ b/test/dialing/direct.node.js @@ -40,21 +40,28 @@ describe('Dialing (direct, TCP)', () => { let peerStore let remoteAddr - before(async () => { - const [remotePeerId] = await Promise.all([ - PeerId.createFromJSON(Peers[0]) + beforeEach(async () => { + const [localPeerId, remotePeerId] = await Promise.all([ + PeerId.createFromJSON(Peers[0]), + PeerId.createFromJSON(Peers[1]) ]) + + peerStore = new PeerStore({ peerId: remotePeerId }) remoteTM = new TransportManager({ libp2p: { - addressManager: new AddressManager({ listen: [listenAddr] }) + addressManager: new AddressManager({ listen: [listenAddr] }), + peerId: remotePeerId, + peerStore }, upgrader: mockUpgrader }) remoteTM.add(Transport.prototype[Symbol.toStringTag], Transport) - peerStore = new PeerStore({ peerId: remotePeerId }) localTM = new TransportManager({ - libp2p: {}, + libp2p: { + peerId: localPeerId, + peerStore: new PeerStore({ peerId: localPeerId }) + }, upgrader: mockUpgrader }) localTM.add(Transport.prototype[Symbol.toStringTag], Transport) @@ -64,7 +71,7 @@ describe('Dialing (direct, TCP)', () => { remoteAddr = remoteTM.getAddrs()[0].encapsulate(`/p2p/${remotePeerId.toB58String()}`) }) - after(() => remoteTM.close()) + afterEach(() => remoteTM.close()) afterEach(() => { sinon.restore() @@ -110,7 +117,7 @@ describe('Dialing (direct, TCP)', () => { peerStore }) - peerStore.addressBook.set(peerId, [remoteAddr]) + peerStore.addressBook.set(peerId, remoteTM.getAddrs()) const connection = await dialer.connectToPeer(peerId) expect(connection).to.exist() diff --git a/test/identify/index.spec.js b/test/identify/index.spec.js index 9052682c3d..e1b741a224 100644 --- a/test/identify/index.spec.js +++ b/test/identify/index.spec.js @@ -8,7 +8,6 @@ const { expect } = chai const sinon = require('sinon') const { EventEmitter } = require('events') -const delay = require('delay') const PeerId = require('peer-id') const duplexPair = require('it-pair/duplex') const multiaddr = require('multiaddr') @@ -20,9 +19,9 @@ const { IdentifyService, multicodecs } = require('../../src/identify') const Peers = require('../fixtures/peers') const Libp2p = require('../../src') const Envelope = require('../../src/record/envelope') -const PeerRecord = require('../../src/record/peer-record') const PeerStore = require('../../src/peer-store') const baseOptions = require('../utils/base-options.browser') +const { updateSelfPeerRecord } = require('../../src/record/utils') const pkg = require('../../package.json') const { MULTIADDRS_WEBSOCKETS } = require('../fixtures/browser') @@ -80,7 +79,7 @@ describe('Identify', () => { sinon.spy(localIdentify.peerStore.protoBook, 'set') // Transport Manager creates signed peer record - await _createSelfPeerRecord(remoteIdentify._libp2p) + await updateSelfPeerRecord(remoteIdentify._libp2p) // Run identify await Promise.all([ @@ -244,8 +243,8 @@ describe('Identify', () => { sinon.spy(remoteIdentify.peerStore.protoBook, 'set') // Transport Manager creates signed peer record - await _createSelfPeerRecord(localIdentify._libp2p) - await _createSelfPeerRecord(remoteIdentify._libp2p) + await updateSelfPeerRecord(localIdentify._libp2p) + await updateSelfPeerRecord(remoteIdentify._libp2p) // Run identify await Promise.all([ @@ -389,8 +388,6 @@ describe('Identify', () => { const connection = await libp2p.dialer.connectToPeer(remoteAddr) expect(connection).to.exist() - // Wait for nextTick to trigger the identify call - await delay(1) // Wait for identify to finish await libp2p.identifyService.identify.firstCall.returnValue @@ -426,8 +423,6 @@ describe('Identify', () => { const connection = await libp2p.dialer.connectToPeer(remoteAddr) expect(connection).to.exist() - // Wait for nextTick to trigger the identify call - await delay(1) // Wait for identify to finish await libp2p.identifyService.identify.firstCall.returnValue @@ -450,15 +445,3 @@ describe('Identify', () => { }) }) }) - -// Self peer record creating on Transport Manager simulation -const _createSelfPeerRecord = async (libp2p) => { - try { - const peerRecord = new PeerRecord({ - peerId: libp2p.peerId, - multiaddrs: libp2p.multiaddrs - }) - const envelope = await Envelope.seal(peerRecord, libp2p.peerId) - libp2p.peerStore.addressBook.consumePeerRecord(envelope) - } catch (_) {} -} diff --git a/test/transports/transport-manager.node.js b/test/transports/transport-manager.node.js index f8a1c633af..e123c5a370 100644 --- a/test/transports/transport-manager.node.js +++ b/test/transports/transport-manager.node.js @@ -4,11 +4,11 @@ const chai = require('chai') chai.use(require('dirty-chai')) const { expect } = chai -const sinon = require('sinon') const AddressManager = require('../../src/address-manager') const TransportManager = require('../../src/transport-manager') const PeerStore = require('../../src/peer-store') +const PeerRecord = require('../../src/record/peer-record') const Transport = require('libp2p-tcp') const PeerId = require('peer-id') const multiaddr = require('multiaddr') @@ -27,11 +27,13 @@ describe('Transport Manager (TCP)', () => { localPeer = await PeerId.createFromJSON(Peers[0]) }) - before(() => { + beforeEach(() => { tm = new TransportManager({ libp2p: { + peerId: localPeer, + multiaddrs: addrs, addressManager: new AddressManager({ listen: addrs }), - PeerStore: new PeerStore({ peerId: localPeer }) + peerStore: new PeerStore({ peerId: localPeer }) }, upgrader: mockUpgrader, onConnection: () => {} @@ -50,22 +52,36 @@ describe('Transport Manager (TCP)', () => { }) it('should be able to listen', async () => { - sinon.spy(tm, '_createSelfPeerRecord') - tm.add(Transport.prototype[Symbol.toStringTag], Transport) await tm.listen(addrs) expect(tm._listeners).to.have.key(Transport.prototype[Symbol.toStringTag]) expect(tm._listeners.get(Transport.prototype[Symbol.toStringTag])).to.have.length(addrs.length) - // Created Self Peer record on new listen address - expect(tm._createSelfPeerRecord.callCount).to.equal(addrs.length) - // Ephemeral ip addresses may result in multiple listeners expect(tm.getAddrs().length).to.equal(addrs.length) await tm.close() expect(tm._listeners.get(Transport.prototype[Symbol.toStringTag])).to.have.length(0) }) + it('should create self signed peer record on listen', async () => { + let signedPeerRecord = await tm.libp2p.peerStore.addressBook.getPeerRecord(localPeer) + expect(signedPeerRecord).to.not.exist() + + tm.add(Transport.prototype[Symbol.toStringTag], Transport) + await tm.listen(addrs) + + // Should created Self Peer record on new listen address + signedPeerRecord = await tm.libp2p.peerStore.addressBook.getPeerRecord(localPeer) + expect(signedPeerRecord).to.exist() + + const record = PeerRecord.createFromProtobuf(signedPeerRecord.payload) + expect(record).to.exist() + expect(record.multiaddrs.length).to.equal(addrs.length) + addrs.forEach((a, i) => { + expect(record.multiaddrs[i].equals(a)).to.be.true() + }) + }) + it('should be able to dial', async () => { tm.add(Transport.prototype[Symbol.toStringTag], Transport) await tm.listen(addrs)