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 f03043e7..add5b5f3 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "aegir": "^22.0.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)