From 6610ef33f9800f34449ddee66e568d22aca5ad7d Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 28 Apr 2020 15:03:16 +0200 Subject: [PATCH 1/4] feat: keybook --- doc/API.md | 86 ++++++++++++++++++++ package.json | 1 + src/peer-store/README.md | 13 +-- src/peer-store/address-book.js | 10 --- src/peer-store/book.js | 41 ++++------ src/peer-store/index.js | 22 ++--- src/peer-store/key-book.js | 83 +++++++++++++++++++ src/peer-store/persistent/consts.js | 3 + src/peer-store/persistent/index.js | 45 +++++++++- test/peer-store/key-book.spec.js | 68 ++++++++++++++++ test/peer-store/peer-store.spec.js | 24 ++++++ test/peer-store/persisted-peer-store.spec.js | 39 ++++----- 12 files changed, 363 insertions(+), 72 deletions(-) create mode 100644 src/peer-store/key-book.js create mode 100644 test/peer-store/key-book.spec.js diff --git a/doc/API.md b/doc/API.md index 7c74d82dc3..04278fb475 100644 --- a/doc/API.md +++ b/doc/API.md @@ -26,6 +26,9 @@ * [`peerStore.addressBook.get`](#peerstoreaddressbookget) * [`peerStore.addressBook.getMultiaddrsForPeer`](#peerstoreaddressbookgetmultiaddrsforpeer) * [`peerStore.addressBook.set`](#peerstoreaddressbookset) + * [`peerStore.keyBook.delete`](#peerstorekeybookdelete) + * [`peerStore.keyBook.get`](#peerstorekeybookget) + * [`peerStore.keyBook.set`](#peerstorekeybookset) * [`peerStore.protoBook.add`](#peerstoreprotobookadd) * [`peerStore.protoBook.delete`](#peerstoreprotobookdelete) * [`peerStore.protoBook.get`](#peerstoreprotobookget) @@ -811,6 +814,89 @@ Add known `protocols` of a given peer. peerStore.protoBook.add(peerId, protocols) ``` +* [`peerStore.keyBook.get`](#peerstorekeybookget) +* [`peerStore.keyBook.set`](#peerstorekeybookset) + +### peerStore.keyBook.delete + +Delete the provided peer from the book. + +`peerStore.keyBook.delete(peerId)` + +#### Parameters + +| Name | Type | Description | +|------|------|-------------| +| peerId | [`PeerId`][peer-id] | peerId to remove | + +#### Returns + +| Type | Description | +|------|-------------| +| `boolean` | true if found and removed | + +#### Example + +```js +peerStore.keyBook.delete(peerId) +// false +peerStore.keyBook.set(peerId) +peerStore.keyBook.delete(peerId) +// true +``` + +### peerStore.keyBook.get + +Get the known `PublicKey` of a provided peer. + +`peerStore.keyBook.get(peerId)` + +#### Parameters + +| Name | Type | Description | +|------|------|-------------| +| peerId | [`PeerId`][peer-id] | peerId to get | + +#### Returns + +| Type | Description | +|------|-------------| +| `RsaPublicKey|Ed25519PublicKey|Secp256k1PublicKey` | Peer PublicKey | + +#### Example + +```js +peerStore.keyBook.get(peerId) +// undefined +peerStore.keyBook.set(peerId) // with inline public key +peerStore.keyBook.get(peerId) +// PublicKey +``` + +### peerStore.keyBook.set + +Set known `peerId`. This can include its Public Key. + +`peerStore.keyBook.set(peerId)` + +#### Parameters + +| Name | Type | Description | +|------|------|-------------| +| peerId | [`PeerId`][peer-id] | peerId to set | + +#### Returns + +| Type | Description | +|------|-------------| +| `KeyBook` | Returns the Key Book component | + +#### Example + +```js +peerStore.keyBook.set(peerId) +``` + ### peerStore.protoBook.delete Delete the provided peer from the book. diff --git a/package.json b/package.json index d29f1b3192..1335bdc286 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "aegir": "^21.9.0", "chai": "^4.2.0", "chai-as-promised": "^7.1.1", + "chai-bytes": "^0.1.2", "cids": "^0.8.0", "delay": "^4.3.0", "dirty-chai": "^2.0.1", diff --git a/src/peer-store/README.md b/src/peer-store/README.md index 2b4dd35ed9..be2dbb906d 100644 --- a/src/peer-store/README.md +++ b/src/peer-store/README.md @@ -52,9 +52,11 @@ A `peerId.toString()` identifier mapping to a `Address` object, which should hav #### Key Book -The `keyBook` tracks the keys of the peers. +The `keyBook` tracks the publick keys of the peers by keeping their [`PeerId`][peer-id]. -**Not Yet Implemented** +`Map` @@ -127,3 +128,5 @@ Metadata is stored under the following key pattern: - Further API methods will probably need to be added in the context of multiaddr validity and confidence. - When improving libp2p configuration for specific runtimes, we should take into account the PeerStore recommended datastore. - When improving libp2p configuration, we should think about a possible way of allowing the configuration of Bootstrap to be influenced by the persisted peers, as a way to decrease the load on Bootstrap nodes. + +[peer-id]: https://github.com/libp2p/js-peer-id \ No newline at end of file diff --git a/src/peer-store/address-book.js b/src/peer-store/address-book.js index 54b23013c4..32d11fefc4 100644 --- a/src/peer-store/address-book.js +++ b/src/peer-store/address-book.js @@ -86,11 +86,6 @@ class AddressBook extends Book { this._setData(peerId, addresses) log(`stored provided multiaddrs for ${id}`) - // Notify the existance of a new peer - if (!rec) { - this._ps.emit('peer', peerId) - } - return this } @@ -130,11 +125,6 @@ class AddressBook extends Book { log(`added provided multiaddrs for ${id}`) - // Notify the existance of a new peer - if (!rec) { - this._ps.emit('peer', peerId) - } - return this } diff --git a/src/peer-store/book.js b/src/peer-store/book.js index 72c70ff417..f2e62eea72 100644 --- a/src/peer-store/book.js +++ b/src/peer-store/book.js @@ -47,7 +47,7 @@ class Book { * Set data into the datastructure, persistence and emit it using the provided transformers. * @private * @param {PeerId} peerId peerId of the data to store - * @param {Array<*>} data data to store. + * @param {*} data data to store. * @param {Object} [options] storing options. * @param {boolean} [options.emit = true] emit the provided data. * @return {void} @@ -57,22 +57,27 @@ class Book { // Store data in memory this.data.set(b58key, data) - this._setPeerId(peerId) + + // Store PeerId + if (!PeerId.isPeerId(data)) { + this._ps.keyBook.set(peerId) + } // Emit event - emit && this._ps.emit(this.eventName, { - peerId, - [this.eventProperty]: this.eventTransformer(data) - }) + emit && this._emit(peerId, data) } /** - * Add known data of a provided peer. + * Emit data. + * @private * @param {PeerId} peerId - * @param {Array|Data} data + * @param {*} data */ - add (peerId, data) { - throw errcode(new Error('set must be implemented by the subclass'), 'ERR_NOT_IMPLEMENTED') + _emit (peerId, data) { + this._ps.emit(this.eventName, { + peerId, + [this.eventProperty]: this.eventTransformer(data) + }) } /** @@ -104,24 +109,10 @@ class Book { return false } - this._ps.emit(this.eventName, { - peerId, - [this.eventProperty]: [] - }) + this._emit(peerId, []) return true } - - /** - * Set PeerId into peerStore datastructure. - * @private - * @param {PeerId} peerId - */ - _setPeerId (peerId) { - if (!this._ps.peerIds.get(peerId)) { - this._ps.peerIds.set(peerId.toB58String(), peerId) - } - } } module.exports = Book diff --git a/src/peer-store/index.js b/src/peer-store/index.js index a88b860009..d19b2de8c3 100644 --- a/src/peer-store/index.js +++ b/src/peer-store/index.js @@ -9,6 +9,7 @@ const { EventEmitter } = require('events') const PeerId = require('peer-id') const AddressBook = require('./address-book') +const KeyBook = require('./key-book') const ProtoBook = require('./proto-book') const { @@ -42,16 +43,14 @@ class PeerStore extends EventEmitter { this.addressBook = new AddressBook(this) /** - * ProtoBook containing a map of peerIdStr to supported protocols. + * KeyBook containing a map of peerIdStr to their PeerId with public keys. */ - this.protoBook = new ProtoBook(this) + this.keyBook = new KeyBook(this) /** - * TODO: this should only exist until we have the key-book - * Map known peers to their peer-id. - * @type {Map} + * ProtoBook containing a map of peerIdStr to supported protocols. */ - this.peerIds = new Map() + this.protoBook = new ProtoBook(this) } /** @@ -73,7 +72,7 @@ class PeerStore extends EventEmitter { // AddressBook for (const [idStr, addresses] of this.addressBook.data.entries()) { - const id = PeerId.createFromCID(idStr) + const id = this.keyBook.data.get(idStr) || PeerId.createFromCID(idStr) peersData.set(idStr, { id, addresses, @@ -84,10 +83,11 @@ class PeerStore extends EventEmitter { // ProtoBook for (const [idStr, protocols] of this.protoBook.data.entries()) { const pData = peersData.get(idStr) + const id = this.keyBook.data.get(idStr) || PeerId.createFromCID(idStr) if (!pData) { peersData.set(idStr, { - id: PeerId.createFromCID(idStr), + id, addresses: [], protocols: Array.from(protocols) }) @@ -104,8 +104,10 @@ class PeerStore extends EventEmitter { */ delete (peerId) { const addressesDeleted = this.addressBook.delete(peerId) + const keyDeleted = this.keyBook.delete(peerId) const protocolsDeleted = this.protoBook.delete(peerId) - return addressesDeleted || protocolsDeleted + + return addressesDeleted || keyDeleted || protocolsDeleted } /** @@ -118,7 +120,7 @@ class PeerStore extends EventEmitter { throw errcode(new Error('peerId must be an instance of peer-id'), ERR_INVALID_PARAMETERS) } - const id = this.peerIds.get(peerId.toB58String()) + const id = this.keyBook.data.get(peerId.toB58String()) const addresses = this.addressBook.get(peerId) const protocols = this.protoBook.get(peerId) diff --git a/src/peer-store/key-book.js b/src/peer-store/key-book.js new file mode 100644 index 0000000000..894b7b9989 --- /dev/null +++ b/src/peer-store/key-book.js @@ -0,0 +1,83 @@ +'use strict' + +const errcode = require('err-code') +const debug = require('debug') +const log = debug('libp2p:peer-store:key-book') +log.error = debug('libp2p:peer-store:key-book:error') + +const PeerId = require('peer-id') + +const Book = require('./book') + +const { + codes: { ERR_INVALID_PARAMETERS } +} = require('../errors') + +/** + * The KeyBook is responsible for keeping the known public keys of a peer. + */ +class KeyBook extends Book { + /** + * @constructor + * @param {PeerStore} peerStore + */ + constructor (peerStore) { + super({ + peerStore, + eventName: 'change:pubkey', // TODO: the name is not probably the best!? + eventProperty: 'pubkey', + eventTransformer: (data) => data.pubKey + }) + + /** + * Map known peers to their known Public Key. + * @type {Map} + */ + this.data = new Map() + } + + /** + * Set PeerId. If the peer was not known before, it will be added. + * @override + * @param {PeerId} peerId + * @return {KeyBook} + */ + set (peerId) { + if (!PeerId.isPeerId(peerId)) { + log.error('peerId must be an instance of peer-id to store data') + throw errcode(new Error('peerId must be an instance of peer-id'), ERR_INVALID_PARAMETERS) + } + + const id = peerId.toB58String() + const recPeerId = this.data.get(id) + + !recPeerId && this._ps.emit('peer', peerId) + // If no record available, or it is incomplete + if (!recPeerId || (peerId.pubKey && !recPeerId.pubKey)) { + this._setData(peerId, peerId, { + emit: Boolean(peerId.pubKey) // No persistence if no public key + }) + log(`stored provided public key for ${id}`) + } + + return this + } + + /** + * Get Public key of the given PeerId, if stored. + * @override + * @param {PeerId} peerId + * @return {PublicKey} + */ + get (peerId) { + if (!PeerId.isPeerId(peerId)) { + throw errcode(new Error('peerId must be an instance of peer-id'), ERR_INVALID_PARAMETERS) + } + + const rec = this.data.get(peerId.toB58String()) + + return rec ? rec.pubKey : undefined + } +} + +module.exports = KeyBook diff --git a/src/peer-store/persistent/consts.js b/src/peer-store/persistent/consts.js index 86b0ec61e1..9679cc5728 100644 --- a/src/peer-store/persistent/consts.js +++ b/src/peer-store/persistent/consts.js @@ -5,5 +5,8 @@ module.exports.NAMESPACE_COMMON = '/peers/' // /peers/protos/ module.exports.NAMESPACE_ADDRESS = '/peers/addrs/' +// /peers/keys/ +module.exports.NAMESPACE_KEYS = '/peers/keys/' + // /peers/addrs/ module.exports.NAMESPACE_PROTOCOL = '/peers/protos/' diff --git a/src/peer-store/persistent/index.js b/src/peer-store/persistent/index.js index 66572b7bb0..e8e6f9a604 100644 --- a/src/peer-store/persistent/index.js +++ b/src/peer-store/persistent/index.js @@ -13,6 +13,7 @@ const PeerStore = require('..') const { NAMESPACE_ADDRESS, NAMESPACE_COMMON, + NAMESPACE_KEYS, NAMESPACE_PROTOCOL } = require('./consts') @@ -56,10 +57,11 @@ class PersistentPeerStore extends PeerStore { // Handlers for dirty peers this.on('change:protocols', this._addDirtyPeer) this.on('change:multiaddrs', this._addDirtyPeer) + this.on('change:pubkey', this._addDirtyPeer) // Load data for await (const entry of this._datastore.query({ prefix: NAMESPACE_COMMON })) { - this._processDatastoreEntry(entry) + await this._processDatastoreEntry(entry) } log('PeerStore started') @@ -110,11 +112,14 @@ class PersistentPeerStore extends PeerStore { const batch = this._datastore.batch() for (const peerIdStr of commitPeers) { // PeerId (replace by keyBook) - const peerId = this.peerIds.get(peerIdStr) + const peerId = this.keyBook.data.get(peerIdStr) || PeerId.createFromB58String(peerIdStr) // Address Book this._batchAddressBook(peerId, batch) + // Key Book + this._batchKeyBook(peerId, batch) + // Proto Book this._batchProtoBook(peerId, batch) } @@ -154,6 +159,31 @@ class PersistentPeerStore extends PeerStore { } } + /** + * Add Key book data of the peer to the batch. + * @private + * @param {PeerId} peerId + * @param {Object} batch + */ + _batchKeyBook (peerId, batch) { + const b32key = peerId.toString() + const key = new Key(`${NAMESPACE_KEYS}${b32key}`) + + try { + // Deleted from the book + if (!peerId.pubKey) { + batch.delete(key) + return + } + + const encodedData = peerId.marshalPubKey() + + batch.put(key, encodedData) + } catch (err) { + log.error(err) + } + } + /** * Add proto book data of the peer to the batch. * @private @@ -187,8 +217,9 @@ class PersistentPeerStore extends PeerStore { * @param {Object} params * @param {Key} params.key datastore key * @param {Buffer} params.value datastore value stored + * @return {Promise} */ - _processDatastoreEntry ({ key, value }) { + async _processDatastoreEntry ({ key, value }) { try { const keyParts = key.toString().split('/') const peerId = PeerId.createFromCID(keyParts[3]) @@ -205,6 +236,14 @@ class PersistentPeerStore extends PeerStore { })), { emit: false }) break + case 'keys': + decoded = await PeerId.createFromPubKey(value) + + this.keyBook._setData( + decoded, + decoded, + { emit: false }) + break case 'protos': decoded = Protocols.decode(value) diff --git a/test/peer-store/key-book.spec.js b/test/peer-store/key-book.spec.js new file mode 100644 index 0000000000..1179e1ec5f --- /dev/null +++ b/test/peer-store/key-book.spec.js @@ -0,0 +1,68 @@ +'use strict' +/* eslint-env mocha */ + +const chai = require('chai') +chai.use(require('dirty-chai')) +chai.use(require('chai-bytes')) +const { expect } = chai +const sinon = require('sinon') + +const PeerId = require('peer-id') +const PeerStore = require('../../src/peer-store') + +const peerUtils = require('../utils/creators/peer') +const { + codes: { ERR_INVALID_PARAMETERS } +} = require('../../src/errors') + +describe('keyBook', () => { + let peerId, peerStore, kb + + beforeEach(async () => { + [peerId] = await peerUtils.createPeerId() + peerStore = new PeerStore() + kb = peerStore.keyBook + }) + + it('throwns invalid parameters error if invalid PeerId is provided', () => { + try { + kb.set('invalid peerId') + } catch (err) { + expect(err.code).to.equal(ERR_INVALID_PARAMETERS) + return + } + throw new Error('invalid peerId should throw error') + }) + + it('stores the peerId in the book and returns the public key', () => { + // Set PeerId + kb.set(peerId) + + // Get public key + const pubKey = kb.get(peerId) + expect(peerId.pubKey.bytes).to.equalBytes(pubKey.bytes) + }) + + it('should not store if already stored', () => { + const spy = sinon.spy(kb, '_setData') + + // Set PeerId + kb.set(peerId) + kb.set(peerId) + + expect(spy).to.have.property('callCount', 1) + }) + + it('stores if already stored but there was no public key stored', () => { + const spy = sinon.spy(kb, '_setData') + + // Set PeerId without public key + const p = PeerId.createFromB58String(peerId.toB58String()) + kb.set(p) + + // Set complete peerId + kb.set(peerId) + + expect(spy).to.have.property('callCount', 2) + }) +}) diff --git a/test/peer-store/peer-store.spec.js b/test/peer-store/peer-store.spec.js index c4f9fbed39..4f65eb1511 100644 --- a/test/peer-store/peer-store.spec.js +++ b/test/peer-store/peer-store.spec.js @@ -4,6 +4,7 @@ const chai = require('chai') chai.use(require('dirty-chai')) const { expect } = chai +const sinon = require('sinon') const PeerStore = require('../../src/peer-store') const multiaddr = require('multiaddr') @@ -48,6 +49,27 @@ describe('peer-store', () => { const peer = peerStore.get(peerIds[0]) expect(peer).to.not.exist() }) + + it('sets the peer to the KeyBook when added to the AddressBook', () => { + const spyPeerStore = sinon.spy(peerStore.keyBook, 'set') + + peerStore.addressBook.set(peerIds[0], [addr1, addr2]) + expect(spyPeerStore).to.have.property('callCount', 1) + }) + + it('sets the peer to the KeyBook when added to the ProtoBook', () => { + const spyPeerStore = sinon.spy(peerStore.keyBook, 'set') + + peerStore.protoBook.set(peerIds[0], [proto1]) + expect(spyPeerStore).to.have.property('callCount', 1) + }) + + it('does not re-set the to the KeyBook when directly added to it', () => { + const spyPeerStore = sinon.spy(peerStore.keyBook, 'set') + + peerStore.keyBook.set(peerIds[0]) + expect(spyPeerStore).to.have.property('callCount', 1) + }) }) describe('previously populated books', () => { @@ -108,6 +130,8 @@ describe('peer-store', () => { const peerMultiaddrs = peer.addresses.map((mi) => mi.multiaddr) expect(peerMultiaddrs).to.have.members([addr1, addr2]) + + expect(peer.id).to.exist() }) it('gets the stored information of a peer that is not present in all its books', () => { diff --git a/test/peer-store/persisted-peer-store.spec.js b/test/peer-store/persisted-peer-store.spec.js index 9f8a2f5146..0da71e15f8 100644 --- a/test/peer-store/persisted-peer-store.spec.js +++ b/test/peer-store/persisted-peer-store.spec.js @@ -68,16 +68,16 @@ describe('Persisted PeerStore', () => { // AddressBook peerStore.addressBook.set(peer, multiaddrs) - expect(spyDirty).to.have.property('callCount', 1) - expect(spyDs).to.have.property('callCount', 1) + expect(spyDirty).to.have.property('callCount', 2) // Address + PeerId + expect(spyDs).to.have.property('callCount', 2) // ProtoBook peerStore.protoBook.set(peer, protocols) - expect(spyDirty).to.have.property('callCount', 2) - expect(spyDs).to.have.property('callCount', 2) + expect(spyDirty).to.have.property('callCount', 3) // Protocol + expect(spyDs).to.have.property('callCount', 3) - // Should have two peer records stored in the datastore + // Should have three peer records stored in the datastore const queryParams = { prefix: '/peers/' } @@ -86,7 +86,7 @@ describe('Persisted PeerStore', () => { for await (const _ of datastore.query(queryParams)) { // eslint-disable-line count++ } - expect(count).to.equal(2) + expect(count).to.equal(3) // Validate data const storedPeer = peerStore.get(peer) @@ -114,11 +114,11 @@ describe('Persisted PeerStore', () => { peerStore.protoBook.set(peers[0], protocols) peerStore.protoBook.set(peers[1], protocols) - expect(spyDs).to.have.property('callCount', 4) + expect(spyDs).to.have.property('callCount', 6) // 2 AddressBook + 2 ProtoBook + 2 KeyBook expect(peerStore.peers.size).to.equal(2) await peerStore.stop() - peerStore.peerIds.clear() + peerStore.keyBook.data.clear() peerStore.addressBook.data.clear() peerStore.protoBook.data.clear() @@ -127,8 +127,8 @@ describe('Persisted PeerStore', () => { await peerStore.start() - expect(spy).to.have.property('callCount', 4) // 4 datastore entries - expect(spyDs).to.have.property('callCount', 4) // 4 previous operations + expect(spy).to.have.property('callCount', 6) // 6 datastore entries + expect(spyDs).to.have.property('callCount', 6) // 6 previous operations expect(peerStore.peers.size).to.equal(2) expect(peerStore.addressBook.data.size).to.equal(2) @@ -149,6 +149,7 @@ describe('Persisted PeerStore', () => { const spyDs = sinon.spy(datastore, 'batch') const spyAddressBook = sinon.spy(peerStore.addressBook, 'delete') + const spyKeyBook = sinon.spy(peerStore.keyBook, 'delete') const spyProtoBook = sinon.spy(peerStore.protoBook, 'delete') // Delete from PeerStore @@ -156,8 +157,9 @@ describe('Persisted PeerStore', () => { await peerStore.stop() expect(spyAddressBook).to.have.property('callCount', 1) + expect(spyKeyBook).to.have.property('callCount', 1) expect(spyProtoBook).to.have.property('callCount', 1) - expect(spyDs).to.have.property('callCount', 2) + expect(spyDs).to.have.property('callCount', 3) // Should have zero peer records stored in the datastore const queryParams = { @@ -199,7 +201,7 @@ describe('Persisted PeerStore', () => { // Remove data from the same Peer peerStore.addressBook.delete(peers[0]) - expect(spyDirty).to.have.property('callCount', 3) + expect(spyDirty).to.have.property('callCount', 4) // 2 AddrBook ops, 1 ProtoBook op, 1 KeyBook op expect(peerStore._dirtyPeers.size).to.equal(1) expect(spyDs).to.have.property('callCount', 0) @@ -213,16 +215,15 @@ describe('Persisted PeerStore', () => { // Add data for second book peerStore.addressBook.set(peers[1], multiaddrs) - expect(spyDirty).to.have.property('callCount', 4) + expect(spyDirty).to.have.property('callCount', 6) expect(spyDs).to.have.property('callCount', 1) - expect(peerStore._dirtyPeers.size).to.equal(0) // Reset // Should have two peer records stored in the datastore let count = 0 for await (const _ of datastore.query(queryParams)) { // eslint-disable-line count++ } - expect(count).to.equal(2) + expect(count).to.equal(4) expect(peerStore.peers.size).to.equal(2) }) @@ -239,7 +240,7 @@ describe('Persisted PeerStore', () => { peerStore.protoBook.set(peer, protocols) expect(spyDs).to.have.property('callCount', 0) - expect(spyDirty).to.have.property('callCount', 1) + expect(spyDirty).to.have.property('callCount', 2) // ProtoBook + KeyBook expect(peerStore._dirtyPeers.size).to.equal(1) const queryParams = { @@ -251,7 +252,7 @@ describe('Persisted PeerStore', () => { await peerStore.stop() - expect(spyDirty).to.have.property('callCount', 1) + expect(spyDirty).to.have.property('callCount', 2) expect(spyDs).to.have.property('callCount', 1) expect(peerStore._dirtyPeers.size).to.equal(0) // Reset @@ -260,7 +261,7 @@ describe('Persisted PeerStore', () => { for await (const _ of datastore.query(queryParams)) { // eslint-disable-line count++ } - expect(count).to.equal(1) + expect(count).to.equal(2) expect(peerStore.peers.size).to.equal(1) }) }) @@ -353,7 +354,7 @@ describe('libp2p.peerStore (Persisted)', () => { await newNode.start() - expect(spy).to.have.property('callCount', 4) // 4 datastore entries + expect(spy).to.have.property('callCount', 6) // 6 datastore entries expect(newNode.peerStore.peers.size).to.equal(2) From 1c2c4324bf6a210830cc175050b6c3f77b582cee Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Wed, 6 May 2020 16:55:43 +0200 Subject: [PATCH 2/4] chore: apply suggestions from code review Co-authored-by: Jacob Heun --- doc/API.md | 13 +++---- src/peer-store/README.md | 14 ++++---- src/peer-store/address-book.js | 10 ++++++ src/peer-store/book.js | 5 --- src/peer-store/key-book.js | 24 +++++++------ src/peer-store/persistent/index.js | 2 +- test/peer-store/key-book.spec.js | 32 ++++++++--------- test/peer-store/peer-store.node.js | 34 ++++++++++++++++++ test/peer-store/peer-store.spec.js | 23 +++--------- test/peer-store/persisted-peer-store.spec.js | 37 +++++++++++--------- 10 files changed, 112 insertions(+), 82 deletions(-) create mode 100644 test/peer-store/peer-store.node.js diff --git a/doc/API.md b/doc/API.md index 04278fb475..883c67f5d5 100644 --- a/doc/API.md +++ b/doc/API.md @@ -814,8 +814,6 @@ Add known `protocols` of a given peer. peerStore.protoBook.add(peerId, protocols) ``` -* [`peerStore.keyBook.get`](#peerstorekeybookget) -* [`peerStore.keyBook.set`](#peerstorekeybookset) ### peerStore.keyBook.delete @@ -840,7 +838,7 @@ Delete the provided peer from the book. ```js peerStore.keyBook.delete(peerId) // false -peerStore.keyBook.set(peerId) +peerStore.keyBook.set(peerId, publicKey) peerStore.keyBook.delete(peerId) // true ``` @@ -868,7 +866,7 @@ Get the known `PublicKey` of a provided peer. ```js peerStore.keyBook.get(peerId) // undefined -peerStore.keyBook.set(peerId) // with inline public key +peerStore.keyBook.set(peerId, publicKey) peerStore.keyBook.get(peerId) // PublicKey ``` @@ -877,13 +875,14 @@ peerStore.keyBook.get(peerId) Set known `peerId`. This can include its Public Key. -`peerStore.keyBook.set(peerId)` +`peerStore.keyBook.set(peerId, publicKey)` #### Parameters | Name | Type | Description | |------|------|-------------| | peerId | [`PeerId`][peer-id] | peerId to set | +| publicKey | [`RsaPublicKey|Ed25519PublicKey|Secp256k1PublicKey`][keys] | peer's public key | #### Returns @@ -894,7 +893,8 @@ Set known `peerId`. This can include its Public Key. #### Example ```js -peerStore.keyBook.set(peerId) +const publicKey = peerId.pubKey +peerStore.keyBook.set(peerId, publicKey) ``` ### peerStore.protoBook.delete @@ -1420,3 +1420,4 @@ This event will be triggered anytime we are disconnected from another peer, rega [connection]: https://github.com/libp2p/js-interfaces/tree/master/src/connection [multiaddr]: https://github.com/multiformats/js-multiaddr [peer-id]: https://github.com/libp2p/js-peer-id +[keys]: https://github.com/libp2p/js-libp2p-crypto/tree/master/src/keys \ No newline at end of file diff --git a/src/peer-store/README.md b/src/peer-store/README.md index be2dbb906d..3c1750571d 100644 --- a/src/peer-store/README.md +++ b/src/peer-store/README.md @@ -10,7 +10,9 @@ Several libp2p subsystems will perform operations, which will gather relevant in In a libp2p node's life, it will discover peers through its discovery protocols. In a typical discovery protocol, addresses of the peer are discovered along with its peer id. Once this happens, the PeerStore should collect this information for future (or immediate) usage by other subsystems. When the information is stored, the PeerStore should inform interested parties of the peer discovered (`peer` event). -Taking into account a different scenario, a peer might perform/receive a dial request to/from a unkwown peer. In such a scenario, the PeerStore must store the peer's multiaddr once a connection is established. +Taking into account a different scenario, a peer might perform/receive a dial request to/from a unkwown peer. In such a scenario, the PeerStore must store the peer's multiaddr once a connection is established. + +When a connection is being upgraded, more precisely after its encryption, or even in a discovery protocol, a libp2p node can get to know other parties public keys. In this scenario, libp2p will add the peer's public key to its `KeyBook`. After a connection is established with a peer, the Identify protocol will run automatically. A stream is created and peers exchange their information (Multiaddrs, running protocols and their public key). Once this information is obtained, it should be added to the PeerStore. In this specific case, as we are speaking to the source of truth, we should ensure the PeerStore is prioritizing these records. If the recorded `multiaddrs` or `protocols` have changed, interested parties must be informed via the `change:multiaddrs` or `change:protocols` events respectively. @@ -42,7 +44,7 @@ The `addressBook` keeps the known multiaddrs of a peer. The multiaddrs of each p `Map` -A `peerId.toString()` identifier mapping to a `Address` object, which should have the following structure: +A `peerId.toB58String()` identifier mapping to a `Address` object, which should have the following structure: ```js { @@ -52,11 +54,11 @@ A `peerId.toString()` identifier mapping to a `Address` object, which should hav #### Key Book -The `keyBook` tracks the publick keys of the peers by keeping their [`PeerId`][peer-id]. +The `keyBook` tracks the public keys of the peers by keeping their [`PeerId`][peer-id]. `Map>` -A `peerId.toString()` identifier mapping to a `Set` of protocol identifier strings. +A `peerId.toB58String()` identifier mapping to a `Set` of protocol identifier strings. #### Metadata Book @@ -129,4 +131,4 @@ Metadata is stored under the following key pattern: - When improving libp2p configuration for specific runtimes, we should take into account the PeerStore recommended datastore. - When improving libp2p configuration, we should think about a possible way of allowing the configuration of Bootstrap to be influenced by the persisted peers, as a way to decrease the load on Bootstrap nodes. -[peer-id]: https://github.com/libp2p/js-peer-id \ No newline at end of file +[peer-id]: https://github.com/libp2p/js-peer-id diff --git a/src/peer-store/address-book.js b/src/peer-store/address-book.js index 32d11fefc4..54b23013c4 100644 --- a/src/peer-store/address-book.js +++ b/src/peer-store/address-book.js @@ -86,6 +86,11 @@ class AddressBook extends Book { this._setData(peerId, addresses) log(`stored provided multiaddrs for ${id}`) + // Notify the existance of a new peer + if (!rec) { + this._ps.emit('peer', peerId) + } + return this } @@ -125,6 +130,11 @@ class AddressBook extends Book { log(`added provided multiaddrs for ${id}`) + // Notify the existance of a new peer + if (!rec) { + this._ps.emit('peer', peerId) + } + return this } diff --git a/src/peer-store/book.js b/src/peer-store/book.js index f2e62eea72..34d77e1da6 100644 --- a/src/peer-store/book.js +++ b/src/peer-store/book.js @@ -58,11 +58,6 @@ class Book { // Store data in memory this.data.set(b58key, data) - // Store PeerId - if (!PeerId.isPeerId(data)) { - this._ps.keyBook.set(peerId) - } - // Emit event emit && this._emit(peerId, data) } diff --git a/src/peer-store/key-book.js b/src/peer-store/key-book.js index 894b7b9989..a5b26edce3 100644 --- a/src/peer-store/key-book.js +++ b/src/peer-store/key-book.js @@ -24,7 +24,7 @@ class KeyBook extends Book { constructor (peerStore) { super({ peerStore, - eventName: 'change:pubkey', // TODO: the name is not probably the best!? + eventName: 'change:pubkey', eventProperty: 'pubkey', eventTransformer: (data) => data.pubKey }) @@ -37,12 +37,13 @@ class KeyBook extends Book { } /** - * Set PeerId. If the peer was not known before, it will be added. + * Set the Peer public key. * @override * @param {PeerId} peerId + * @param {RsaPublicKey|Ed25519PublicKey|Secp256k1PublicKey} publicKey * @return {KeyBook} - */ - set (peerId) { + */ + set (peerId, publicKey) { if (!PeerId.isPeerId(peerId)) { log.error('peerId must be an instance of peer-id to store data') throw errcode(new Error('peerId must be an instance of peer-id'), ERR_INVALID_PARAMETERS) @@ -51,12 +52,13 @@ class KeyBook extends Book { const id = peerId.toB58String() const recPeerId = this.data.get(id) - !recPeerId && this._ps.emit('peer', peerId) - // If no record available, or it is incomplete - if (!recPeerId || (peerId.pubKey && !recPeerId.pubKey)) { - this._setData(peerId, peerId, { - emit: Boolean(peerId.pubKey) // No persistence if no public key - }) + // If no record available, and this is valid + if (!recPeerId && publicKey) { + // This might be unecessary, but we want to store the PeerId + // to avoid an async operation when reconstructing the PeerId + peerId.pubKey = publicKey + + this._setData(peerId, peerId) log(`stored provided public key for ${id}`) } @@ -67,7 +69,7 @@ class KeyBook extends Book { * Get Public key of the given PeerId, if stored. * @override * @param {PeerId} peerId - * @return {PublicKey} + * @return {RsaPublicKey|Ed25519PublicKey|Secp256k1PublicKey} */ get (peerId) { if (!PeerId.isPeerId(peerId)) { diff --git a/src/peer-store/persistent/index.js b/src/peer-store/persistent/index.js index e8e6f9a604..8e7c459fb8 100644 --- a/src/peer-store/persistent/index.js +++ b/src/peer-store/persistent/index.js @@ -111,7 +111,7 @@ class PersistentPeerStore extends PeerStore { log('create batch commit') const batch = this._datastore.batch() for (const peerIdStr of commitPeers) { - // PeerId (replace by keyBook) + // PeerId const peerId = this.keyBook.data.get(peerIdStr) || PeerId.createFromB58String(peerIdStr) // Address Book diff --git a/test/peer-store/key-book.spec.js b/test/peer-store/key-book.spec.js index 1179e1ec5f..c5eab517a5 100644 --- a/test/peer-store/key-book.spec.js +++ b/test/peer-store/key-book.spec.js @@ -7,7 +7,6 @@ chai.use(require('chai-bytes')) const { expect } = chai const sinon = require('sinon') -const PeerId = require('peer-id') const PeerStore = require('../../src/peer-store') const peerUtils = require('../utils/creators/peer') @@ -24,7 +23,7 @@ describe('keyBook', () => { kb = peerStore.keyBook }) - it('throwns invalid parameters error if invalid PeerId is provided', () => { + it('throwns invalid parameters error if invalid PeerId is provided in set', () => { try { kb.set('invalid peerId') } catch (err) { @@ -34,9 +33,19 @@ describe('keyBook', () => { throw new Error('invalid peerId should throw error') }) + it('throwns invalid parameters error if invalid PeerId is provided in get', () => { + try { + kb.get('invalid peerId') + } catch (err) { + expect(err.code).to.equal(ERR_INVALID_PARAMETERS) + return + } + throw new Error('invalid peerId should throw error') + }) + it('stores the peerId in the book and returns the public key', () => { // Set PeerId - kb.set(peerId) + kb.set(peerId, peerId.pubKey) // Get public key const pubKey = kb.get(peerId) @@ -47,22 +56,9 @@ describe('keyBook', () => { const spy = sinon.spy(kb, '_setData') // Set PeerId - kb.set(peerId) - kb.set(peerId) + kb.set(peerId, peerId.pubKey) + kb.set(peerId, peerId.pubKey) expect(spy).to.have.property('callCount', 1) }) - - it('stores if already stored but there was no public key stored', () => { - const spy = sinon.spy(kb, '_setData') - - // Set PeerId without public key - const p = PeerId.createFromB58String(peerId.toB58String()) - kb.set(p) - - // Set complete peerId - kb.set(peerId) - - expect(spy).to.have.property('callCount', 2) - }) }) diff --git a/test/peer-store/peer-store.node.js b/test/peer-store/peer-store.node.js new file mode 100644 index 0000000000..4b151424fd --- /dev/null +++ b/test/peer-store/peer-store.node.js @@ -0,0 +1,34 @@ +'use strict' +/* eslint-env mocha */ + +const chai = require('chai') +chai.use(require('dirty-chai')) +chai.use(require('chai-as-promised')) +const { expect } = chai +const sinon = require('sinon') + +const baseOptions = require('../utils/base-options') +const peerUtils = require('../utils/creators/peer') + +describe('libp2p.peerStore', () => { + let libp2p, remoteLibp2p + + beforeEach(async () => { + [libp2p, remoteLibp2p] = await peerUtils.createPeer({ + number: 2, + populateAddressBooks: false, + config: { + ...baseOptions + } + }) + }) + + it('adds peer address to AddressBook when establishing connection', async () => { + const spyAddressBook = sinon.spy(libp2p.peerStore.addressBook, 'add') + const remoteMultiaddr = `${remoteLibp2p.multiaddrs[0]}/p2p/${remoteLibp2p.peerId.toB58String()}` + const conn = await libp2p.dial(remoteMultiaddr) + + expect(conn).to.exist() + expect(spyAddressBook).to.have.property('callCount', 1) + }) +}) diff --git a/test/peer-store/peer-store.spec.js b/test/peer-store/peer-store.spec.js index 4f65eb1511..1884ea4dc6 100644 --- a/test/peer-store/peer-store.spec.js +++ b/test/peer-store/peer-store.spec.js @@ -4,7 +4,6 @@ const chai = require('chai') chai.use(require('dirty-chai')) const { expect } = chai -const sinon = require('sinon') const PeerStore = require('../../src/peer-store') const multiaddr = require('multiaddr') @@ -50,25 +49,11 @@ describe('peer-store', () => { expect(peer).to.not.exist() }) - it('sets the peer to the KeyBook when added to the AddressBook', () => { - const spyPeerStore = sinon.spy(peerStore.keyBook, 'set') + it('sets the peer\'s public key to the KeyBook', () => { + peerStore.keyBook.set(peerIds[0], peerIds[0].pubKey) - peerStore.addressBook.set(peerIds[0], [addr1, addr2]) - expect(spyPeerStore).to.have.property('callCount', 1) - }) - - it('sets the peer to the KeyBook when added to the ProtoBook', () => { - const spyPeerStore = sinon.spy(peerStore.keyBook, 'set') - - peerStore.protoBook.set(peerIds[0], [proto1]) - expect(spyPeerStore).to.have.property('callCount', 1) - }) - - it('does not re-set the to the KeyBook when directly added to it', () => { - const spyPeerStore = sinon.spy(peerStore.keyBook, 'set') - - peerStore.keyBook.set(peerIds[0]) - expect(spyPeerStore).to.have.property('callCount', 1) + const pubKey = peerStore.keyBook.get(peerIds[0]) + expect(pubKey).to.exist() }) }) diff --git a/test/peer-store/persisted-peer-store.spec.js b/test/peer-store/persisted-peer-store.spec.js index 0da71e15f8..a8b0fea1ba 100644 --- a/test/peer-store/persisted-peer-store.spec.js +++ b/test/peer-store/persisted-peer-store.spec.js @@ -68,14 +68,14 @@ describe('Persisted PeerStore', () => { // AddressBook peerStore.addressBook.set(peer, multiaddrs) - expect(spyDirty).to.have.property('callCount', 2) // Address + PeerId - expect(spyDs).to.have.property('callCount', 2) + expect(spyDirty).to.have.property('callCount', 1) // Address + expect(spyDs).to.have.property('callCount', 1) // ProtoBook peerStore.protoBook.set(peer, protocols) - expect(spyDirty).to.have.property('callCount', 3) // Protocol - expect(spyDs).to.have.property('callCount', 3) + expect(spyDirty).to.have.property('callCount', 2) // Protocol + expect(spyDs).to.have.property('callCount', 2) // Should have three peer records stored in the datastore const queryParams = { @@ -86,7 +86,7 @@ describe('Persisted PeerStore', () => { for await (const _ of datastore.query(queryParams)) { // eslint-disable-line count++ } - expect(count).to.equal(3) + expect(count).to.equal(2) // Validate data const storedPeer = peerStore.get(peer) @@ -110,11 +110,15 @@ describe('Persisted PeerStore', () => { peerStore.addressBook.set(peers[0], [multiaddrs[0]]) peerStore.addressBook.set(peers[1], [multiaddrs[1]]) + // KeyBook + peerStore.keyBook.set(peers[0], peers[0].pubKey) + peerStore.keyBook.set(peers[1], peers[1].pubKey) + // ProtoBook peerStore.protoBook.set(peers[0], protocols) peerStore.protoBook.set(peers[1], protocols) - expect(spyDs).to.have.property('callCount', 6) // 2 AddressBook + 2 ProtoBook + 2 KeyBook + expect(spyDs).to.have.property('callCount', 6) // 2 AddressBook + 2 KeyBook + 2 ProtoBook expect(peerStore.peers.size).to.equal(2) await peerStore.stop() @@ -127,11 +131,12 @@ describe('Persisted PeerStore', () => { await peerStore.start() - expect(spy).to.have.property('callCount', 6) // 6 datastore entries - expect(spyDs).to.have.property('callCount', 6) // 6 previous operations + expect(spy).to.have.property('callCount', 6) + expect(spyDs).to.have.property('callCount', 6) expect(peerStore.peers.size).to.equal(2) expect(peerStore.addressBook.data.size).to.equal(2) + expect(peerStore.keyBook.data.size).to.equal(2) expect(peerStore.protoBook.data.size).to.equal(2) }) @@ -159,7 +164,7 @@ describe('Persisted PeerStore', () => { expect(spyAddressBook).to.have.property('callCount', 1) expect(spyKeyBook).to.have.property('callCount', 1) expect(spyProtoBook).to.have.property('callCount', 1) - expect(spyDs).to.have.property('callCount', 3) + expect(spyDs).to.have.property('callCount', 2) // Should have zero peer records stored in the datastore const queryParams = { @@ -201,7 +206,7 @@ describe('Persisted PeerStore', () => { // Remove data from the same Peer peerStore.addressBook.delete(peers[0]) - expect(spyDirty).to.have.property('callCount', 4) // 2 AddrBook ops, 1 ProtoBook op, 1 KeyBook op + expect(spyDirty).to.have.property('callCount', 3) // 2 AddrBook ops, 1 ProtoBook op expect(peerStore._dirtyPeers.size).to.equal(1) expect(spyDs).to.have.property('callCount', 0) @@ -215,7 +220,7 @@ describe('Persisted PeerStore', () => { // Add data for second book peerStore.addressBook.set(peers[1], multiaddrs) - expect(spyDirty).to.have.property('callCount', 6) + expect(spyDirty).to.have.property('callCount', 4) expect(spyDs).to.have.property('callCount', 1) // Should have two peer records stored in the datastore @@ -223,7 +228,7 @@ describe('Persisted PeerStore', () => { for await (const _ of datastore.query(queryParams)) { // eslint-disable-line count++ } - expect(count).to.equal(4) + expect(count).to.equal(2) expect(peerStore.peers.size).to.equal(2) }) @@ -240,7 +245,7 @@ describe('Persisted PeerStore', () => { peerStore.protoBook.set(peer, protocols) expect(spyDs).to.have.property('callCount', 0) - expect(spyDirty).to.have.property('callCount', 2) // ProtoBook + KeyBook + expect(spyDirty).to.have.property('callCount', 1) // ProtoBook expect(peerStore._dirtyPeers.size).to.equal(1) const queryParams = { @@ -252,7 +257,7 @@ describe('Persisted PeerStore', () => { await peerStore.stop() - expect(spyDirty).to.have.property('callCount', 2) + expect(spyDirty).to.have.property('callCount', 1) expect(spyDs).to.have.property('callCount', 1) expect(peerStore._dirtyPeers.size).to.equal(0) // Reset @@ -261,7 +266,7 @@ describe('Persisted PeerStore', () => { for await (const _ of datastore.query(queryParams)) { // eslint-disable-line count++ } - expect(count).to.equal(2) + expect(count).to.equal(1) expect(peerStore.peers.size).to.equal(1) }) }) @@ -354,7 +359,7 @@ describe('libp2p.peerStore (Persisted)', () => { await newNode.start() - expect(spy).to.have.property('callCount', 6) // 6 datastore entries + expect(spy).to.have.property('callCount', 4) // 4 datastore entries expect(newNode.peerStore.peers.size).to.equal(2) From 7969151458300ebbe54189c01a094713e635eaa5 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Thu, 7 May 2020 10:46:00 +0200 Subject: [PATCH 3/4] chore: add keys to keybook on connection upgraded --- src/connection-manager/index.js | 14 +++++++++----- test/identify/index.spec.js | 4 ++-- test/peer-store/peer-store.node.js | 25 +++++++++++++++++++++---- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/connection-manager/index.js b/src/connection-manager/index.js index 8df9521b1e..cc64468d1a 100644 --- a/src/connection-manager/index.js +++ b/src/connection-manager/index.js @@ -171,18 +171,22 @@ class ConnectionManager extends EventEmitter { * @param {Connection} connection */ onConnect (connection) { - const peerId = connection.remotePeer.toB58String() - const storedConn = this.connections.get(peerId) + const peerId = connection.remotePeer + const peerIdStr = peerId.toB58String() + const storedConn = this.connections.get(peerIdStr) if (storedConn) { storedConn.push(connection) } else { - this.connections.set(peerId, [connection]) + this.connections.set(peerIdStr, [connection]) this.emit('peer:connect', connection) } - if (!this._peerValues.has(peerId)) { - this._peerValues.set(peerId, this._options.defaultPeerValue) + this._libp2p.peerStore.addressBook.add(peerId, [connection.remoteAddr]) + this._libp2p.peerStore.keyBook.set(peerId, peerId.pubKey) + + if (!this._peerValues.has(peerIdStr)) { + this._peerValues.set(peerIdStr, this._options.defaultPeerValue) } this._checkLimit('maxConnections', this.size) diff --git a/test/identify/index.spec.js b/test/identify/index.spec.js index 9ca892ff05..abcbbeb34e 100644 --- a/test/identify/index.spec.js +++ b/test/identify/index.spec.js @@ -242,8 +242,8 @@ describe('Identify', () => { expect(connection).to.exist() // Wait for peer store to be updated - // Dialer._createDialTarget (add), Identify (replace) - await pWaitFor(() => peerStoreSpySet.callCount === 1 && peerStoreSpyAdd.callCount === 1) + // Dialer._createDialTarget (add), Connected (add), Identify (replace) + await pWaitFor(() => peerStoreSpySet.callCount === 1 && peerStoreSpyAdd.callCount === 2) expect(libp2p.identifyService.identify.callCount).to.equal(1) // The connection should have no open streams diff --git a/test/peer-store/peer-store.node.js b/test/peer-store/peer-store.node.js index 4b151424fd..30c2230eb8 100644 --- a/test/peer-store/peer-store.node.js +++ b/test/peer-store/peer-store.node.js @@ -3,7 +3,7 @@ const chai = require('chai') chai.use(require('dirty-chai')) -chai.use(require('chai-as-promised')) +chai.use(require('chai-bytes')) const { expect } = chai const sinon = require('sinon') @@ -23,12 +23,29 @@ describe('libp2p.peerStore', () => { }) }) - it('adds peer address to AddressBook when establishing connection', async () => { + it('adds peer address to AddressBook and keys to the keybook when establishing connection', async () => { + const idStr = libp2p.peerId.toB58String() + const remoteIdStr = remoteLibp2p.peerId.toB58String() + const spyAddressBook = sinon.spy(libp2p.peerStore.addressBook, 'add') - const remoteMultiaddr = `${remoteLibp2p.multiaddrs[0]}/p2p/${remoteLibp2p.peerId.toB58String()}` + const spyKeyBook = sinon.spy(libp2p.peerStore.keyBook, 'set') + + const remoteMultiaddr = `${remoteLibp2p.multiaddrs[0]}/p2p/${remoteIdStr}` const conn = await libp2p.dial(remoteMultiaddr) expect(conn).to.exist() - expect(spyAddressBook).to.have.property('callCount', 1) + expect(spyAddressBook).to.have.property('called', true) + expect(spyKeyBook).to.have.property('called', true) + + const localPeers = libp2p.peerStore.peers + expect(localPeers.size).to.equal(1) + // const publicKeyInLocalPeer = localPeers.get(remoteIdStr).id.pubKey + // expect(publicKeyInLocalPeer.bytes).to.equalBytes(remoteLibp2p.peerId.pubKey.bytes) + + const remotePeers = remoteLibp2p.peerStore.peers + expect(remotePeers.size).to.equal(1) + const publicKeyInRemotePeer = remotePeers.get(idStr).id.pubKey + expect(publicKeyInRemotePeer).to.exist() + expect(publicKeyInRemotePeer.bytes).to.equalBytes(libp2p.peerId.pubKey.bytes) }) }) From 14bd946990a4f5bd0af0b92a2adf3b1831535282 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Thu, 7 May 2020 15:49:03 +0200 Subject: [PATCH 4/4] chore: apply suggestions from code review Co-authored-by: Jacob Heun --- src/peer-store/persistent/index.js | 2 +- test/peer-store/key-book.spec.js | 4 ++-- test/peer-store/peer-store.node.js | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/peer-store/persistent/index.js b/src/peer-store/persistent/index.js index 8e7c459fb8..366e9bc48f 100644 --- a/src/peer-store/persistent/index.js +++ b/src/peer-store/persistent/index.js @@ -112,7 +112,7 @@ class PersistentPeerStore extends PeerStore { const batch = this._datastore.batch() for (const peerIdStr of commitPeers) { // PeerId - const peerId = this.keyBook.data.get(peerIdStr) || PeerId.createFromB58String(peerIdStr) + const peerId = this.keyBook.data.get(peerIdStr) || PeerId.createFromCID(peerIdStr) // Address Book this._batchAddressBook(peerId, batch) diff --git a/test/peer-store/key-book.spec.js b/test/peer-store/key-book.spec.js index c5eab517a5..22b708fbf2 100644 --- a/test/peer-store/key-book.spec.js +++ b/test/peer-store/key-book.spec.js @@ -23,7 +23,7 @@ describe('keyBook', () => { kb = peerStore.keyBook }) - it('throwns invalid parameters error if invalid PeerId is provided in set', () => { + it('throws invalid parameters error if invalid PeerId is provided in set', () => { try { kb.set('invalid peerId') } catch (err) { @@ -33,7 +33,7 @@ describe('keyBook', () => { throw new Error('invalid peerId should throw error') }) - it('throwns invalid parameters error if invalid PeerId is provided in get', () => { + it('throws invalid parameters error if invalid PeerId is provided in get', () => { try { kb.get('invalid peerId') } catch (err) { diff --git a/test/peer-store/peer-store.node.js b/test/peer-store/peer-store.node.js index 30c2230eb8..c6b34eb541 100644 --- a/test/peer-store/peer-store.node.js +++ b/test/peer-store/peer-store.node.js @@ -39,6 +39,8 @@ describe('libp2p.peerStore', () => { const localPeers = libp2p.peerStore.peers expect(localPeers.size).to.equal(1) + + // TODO: needs https://github.com/NodeFactoryIo/js-libp2p-noise/issues/58 // const publicKeyInLocalPeer = localPeers.get(remoteIdStr).id.pubKey // expect(publicKeyInLocalPeer.bytes).to.equalBytes(remoteLibp2p.peerId.pubKey.bytes)