From 8e35ab11da56798633f3525e34e6e415718a7c87 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 | 24 +++--- 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, 364 insertions(+), 73 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 7c74d82d..04278fb4 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 d29f1b31..1335bdc2 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 2b4dd35e..be2dbb90 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 54b23013..32d11fef 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 72c70ff4..f2e62eea 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 a88b8600..d19b2de8 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 { @@ -41,17 +42,15 @@ class PeerStore extends EventEmitter { */ this.addressBook = new AddressBook(this) + /** + * KeyBook containing a map of peerIdStr to their PeerId with public keys. + */ + this.keyBook = new KeyBook(this) + /** * ProtoBook containing a map of peerIdStr to supported protocols. */ this.protoBook = new ProtoBook(this) - - /** - * TODO: this should only exist until we have the key-book - * Map known peers to their peer-id. - * @type {Map} - */ - this.peerIds = new Map() } /** @@ -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 00000000..894b7b99 --- /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 86b0ec61..9679cc57 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 66572b7b..e8e6f9a6 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 00000000..1179e1ec --- /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 c4f9fbed..4f65eb15 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 9f8a2f51..0da71e15 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 16a62cae0d3d3b6a57f574e094a052660f871e13 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 04278fb4..883c67f5 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 be2dbb90..3c175057 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 32d11fef..54b23013 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 f2e62eea..34d77e1d 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 894b7b99..a5b26edc 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 e8e6f9a6..8e7c459f 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 1179e1ec..c5eab517 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 00000000..4b151424 --- /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 4f65eb15..1884ea4d 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 0da71e15..a8b0fea1 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 d21a138d40b63e7a8201c0bca575e5b68c1443aa 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 8df9521b..cc64468d 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 9ca892ff..abcbbeb3 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 4b151424..30c2230e 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 9eaed58604ee705d1300d738ad89a90d189922f9 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 8e7c459f..366e9bc4 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 c5eab517..22b708fb 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 30c2230e..c6b34eb5 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)