fluencelabs/js-libp2p
1
0
Fork 0
mirror of https://github.com/fluencelabs/js-libp2p synced 2025-04-25 10:32:14 +00:00
Code Issues Projects Releases Wiki Activity

chore: apply suggestions from code review

Browse Source 
Co-authored-by: Jacob Heun <jacobheun@gmail.com>
This commit is contained in:
Vasco Santos 2020-05-06 16:55:43 +02:00
parent 6610ef33f9
commit 1c2c4324bf
10 changed files with 112 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
doc/API.md
View File

@ -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

14
src/peer-store/README.md
View File

@ -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<string, Address>`
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<string, PeerId`
A `peerId.toString()` identifier mapping to a `PeerId` of the peer. This instance contains the peer public key.
A `peerId.toB58String()` identifier mapping to a `PeerId` of the peer. This instance contains the peer public key.
#### Protocol Book
@ -64,7 +66,7 @@ The `protoBook` holds the identifiers of the protocols supported by each peer. T
`Map<string, Set<string>>`
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
[peer-id]: https://github.com/libp2p/js-peer-id

10
src/peer-store/address-book.js
View File

@ -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
}

5
src/peer-store/book.js
View File

@ -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)
}

24
src/peer-store/key-book.js
View File

@ -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)) {

2
src/peer-store/persistent/index.js
View File

@ -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

32
test/peer-store/key-book.spec.js
View File

@ -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)
})
})

34
test/peer-store/peer-store.node.js Normal file
View File

@ -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)
})
})

23
test/peer-store/peer-store.spec.js
View File

@ -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()
})
})

37
test/peer-store/persisted-peer-store.spec.js
View File

@ -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)