From 30b0ae90d5e64de0d3752c9f00232100ee3d3ad6 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Thu, 2 Apr 2020 10:47:53 +0200 Subject: [PATCH] chore: apply suggestions from code review Co-Authored-By: Jacob Heun --- doc/API.md | 225 ++++++++++++++------------- src/dialer/index.js | 2 +- src/get-peer-info.js | 4 +- src/identify/index.js | 3 - src/index.js | 6 +- src/peer-store/README.md | 32 ++-- src/peer-store/address-book.js | 110 +++++++------ src/peer-store/book.js | 19 ++- src/peer-store/proto-book.js | 86 ++++------ test/dialing/direct.node.js | 6 +- test/dialing/direct.spec.js | 12 +- test/identify/index.spec.js | 5 +- test/peer-store/address-book.spec.js | 121 ++++++++------ test/peer-store/proto-book.spec.js | 158 ++++++++----------- test/registrar/registrar.spec.js | 4 +- 15 files changed, 382 insertions(+), 411 deletions(-) diff --git a/doc/API.md b/doc/API.md index 6051362324..11ce149c21 100644 --- a/doc/API.md +++ b/doc/API.md @@ -17,16 +17,17 @@ * [`contentRouting.put`](#contentroutingput) * [`contentRouting.get`](#contentroutingget) * [`contentRouting.getMany`](#contentroutinggetmany) + * [`peerStore.addressBook.add`](#peerstoreaddressbookadd) * [`peerStore.addressBook.delete`](#peerstoreaddressbookdelete) * [`peerStore.addressBook.get`](#peerstoreaddressbookget) * [`peerStore.addressBook.getMultiaddrsForPeer`](#peerstoreaddressbookgetmultiaddrsforpeer) * [`peerStore.addressBook.set`](#peerstoreaddressbookset) + * [`peerStore.protoBook.add`](#peerstoreprotobookadd) * [`peerStore.protoBook.delete`](#peerstoreprotobookdelete) * [`peerStore.protoBook.get`](#peerstoreprotobookget) * [`peerStore.protoBook.set`](#peerstoreprotobookset) - * [`peerStore.protoBook.supports`](#peerstoreprotobooksupports) * [`peerStore.delete`](#peerstoredelete) - * [`peerStore.find`](#peerstorefind) + * [`peerStore.get`](#peerstoreget) * [`peerStore.peers`](#peerstorepeers) * [`pubsub.getSubscribers`](#pubsubgetsubscribers) * [`pubsub.getTopics`](#pubsubgettopics) @@ -55,13 +56,13 @@ Creates an instance of Libp2p. | Name | Type | Description | |------|------|-------------| -| options | `Object` | libp2p options | -| options.modules | `Array` | libp2p modules to use | -| [options.config] | `Object` | libp2p modules configuration and core configuration | -| [options.connectionManager] | `Object` | libp2p Connection Manager configuration | -| [options.datastore] | `Object` | must implement [ipfs/interface-datastore](https://github.com/ipfs/interface-datastore) (in memory datastore will be used if not provided) | -| [options.dialer] | `Object` | libp2p Dialer configuration -| [options.metrics] | `Object` | libp2p Metrics configuration +| options | `object` | libp2p options | +| options.modules | `Array` | libp2p modules to use | +| [options.config] | `object` | libp2p modules configuration and core configuration | +| [options.connectionManager] | `object` | libp2p Connection Manager configuration | +| [options.datastore] | `object` | must implement [ipfs/interface-datastore](https://github.com/ipfs/interface-datastore) (in memory datastore will be used if not provided) | +| [options.dialer] | `object` | libp2p Dialer configuration +| [options.metrics] | `object` | libp2p Metrics configuration | [options.peerInfo] | [`PeerInfo`][peer-info] | peerInfo instance (it will be created if not provided) | For Libp2p configurations and modules details read the [Configuration Document](./CONFIGURATION.md). @@ -192,7 +193,7 @@ for (const [peerId, connections] of libp2p.connections) { | Name | Type | Description | |------|------|-------------| | peer | [`PeerInfo`][peer-info]\|[`PeerId`][peer-id]\|[`Multiaddr`][multiaddr]\|`string` | The peer to dial. If a [`Multiaddr`][multiaddr] or its string is provided, it **must** include the peer id | -| [options] | `Object` | dial options | +| [options] | `object` | dial options | | [options.signal] | [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) | An `AbortSignal` instance obtained from an [`AbortController`](https://developer.mozilla.org/en-US/docs/Web/API/AbortController) that can be used to abort the connection before it completes | #### Returns @@ -227,8 +228,8 @@ Dials to another peer in the network and selects a protocol to communicate with | Name | Type | Description | |------|------|-------------| | peer | [`PeerInfo`][peer-info]\|[`PeerId`][peer-id]\|[`Multiaddr`][multiaddr]\|`string` | The peer to dial. If a [`Multiaddr`][multiaddr] or its string is provided, it **must** include the peer id | -| protocols | `String|Array` | A list of protocols (or single protocol) to negotiate with. Protocols are attempted in order until a match is made. (e.g '/ipfs/bitswap/1.1.0') | -| [options] | `Object` | dial options | +| protocols | `string|Array` | A list of protocols (or single protocol) to negotiate with. Protocols are attempted in order until a match is made. (e.g '/ipfs/bitswap/1.1.0') | +| [options] | `object` | dial options | | [options.signal] | [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) | An `AbortSignal` instance obtained from an [`AbortController`](https://developer.mozilla.org/en-US/docs/Web/API/AbortController) that can be used to abort the connection before it completes | #### Returns @@ -286,7 +287,7 @@ In the event of a new handler for the same protocol being added, the first one i | Name | Type | Description | |------|------|-------------| -| protocols | `Array|String` | protocols to register | +| protocols | `Array|string` | protocols to register | | handler | `function({ connection:*, stream:*, protocol:string })` | handler to call | @@ -311,7 +312,7 @@ Unregisters all handlers with the given protocols | Name | Type | Description | |------|------|-------------| -| protocols | `Array|String` | protocols to unregister | +| protocols | `Array|string` | protocols to unregister | #### Example @@ -356,7 +357,7 @@ Iterates over all peer routers in series to find the given peer. If the DHT is e | Name | Type | Description | |------|------|-------------| | peerId | [`PeerId`][peer-id] | ID of the peer to find | -| options | `Object` | operation options | +| options | `object` | operation options | | options.timeout | `number` | maximum time the query should run | #### Returns @@ -384,7 +385,7 @@ Once a content router succeeds, the iteration will stop. If the DHT is enabled, | Name | Type | Description | |------|------|-------------| | cid | [`CID`][cid] | cid to find | -| options | `Object` | operation options | +| options | `object` | operation options | | options.timeout | `number` | maximum time the query should run | | options.maxNumProviders | `number` | maximum number of providers to find | @@ -438,9 +439,9 @@ Writes a value to a key in the DHT. | Name | Type | Description | |------|------|-------------| -| key | `String` | key to add to the dht | +| key | `string` | key to add to the dht | | value | `Buffer` | value to add to the dht | -| [options] | `Object` | put options | +| [options] | `object` | put options | | [options.minPeers] | `number` | minimum number of peers required to successfully put (default: closestPeers.length) | #### Returns @@ -469,8 +470,8 @@ Queries the DHT for a value stored for a given key. | Name | Type | Description | |------|------|-------------| -| key | `String` | key to get from the dht | -| [options] | `Object` | get options | +| key | `string` | key to get from the dht | +| [options] | `object` | get options | | [options.timeout] | `number` | maximum time the query should run | #### Returns @@ -498,9 +499,9 @@ Queries the DHT for the n values stored for the given key (without sorting). | Name | Type | Description | |------|------|-------------| -| key | `String` | key to get from the dht | +| key | `string` | key to get from the dht | | nvals | `number` | number of values aimed | -| [options] | `Object` | get options | +| [options] | `object` | get options | | [options.timeout] | `number` | maximum time the query should run | #### Returns @@ -518,6 +519,31 @@ const key = '/key' const { from, val } = await libp2p.contentRouting.get(key) ``` +### peerStore.addressBook.Add + +Adds known `multiaddrs` of a given peer. If the peer is not known, it will be set with the provided multiaddrs. + +`peerStore.addressBook.add(peerId, multiaddrs)` + +#### Parameters + +| Name | Type | Description | +|------|------|-------------| +| peerId | [`PeerId`][peer-id] | peerId to set | +| multiaddrs | [`Multiaddr`][multiaddr]\|`Array` | multiaddrs to add | + +#### Returns + +| Type | Description | +|------|-------------| +| `Map` | Map of known peers' string identifier with their relevant information [`MultiaddrInfo`](multiaddr-info) | + +#### Example + +```js +peerStore.addressBook.add(peerId, multiaddr) +``` + ### peerStore.addressBook.delete Delete the provided peer from the book. @@ -528,7 +554,7 @@ Delete the provided peer from the book. | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to remove | +| peerId | [`PeerId`][peer-id] | peerId to remove | #### Returns @@ -548,7 +574,7 @@ peerStore.addressBook.delete(peerId) ### peerStore.addressBook.get -Get the known `multiaddrInfos` of a provided peer. +Get the known [`MultiaddrInfos`](multiaddr-info) of a provided peer. `peerStore.addressBook.get(peerId)` @@ -556,20 +582,20 @@ Get the known `multiaddrInfos` of a provided peer. | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to get | +| peerId | [`PeerId`][peer-id] | peerId to get | #### Returns | Type | Description | |------|-------------| -| `Array` | Array of peer's multiaddr with their relevant information | +| `Array` | Array of peer's multiaddr with their relevant information [`MultiaddrInfo`](multiaddr-info) | #### Example ```js peerStore.addressBook.get(peerId) // undefined -peerStore.addressBook.set(peerId, discoveredMultiaddr) +peerStore.addressBook.set(peerId, multiaddr) peerStore.addressBook.get(peerId) // [ // { @@ -585,7 +611,7 @@ peerStore.addressBook.get(peerId) ## peerStore.addressBook.getMultiaddrsForPeer -Get the known `multiaddr` of a provided peer. All returned multiaddrs will include the encapsulated `PeerId` of the peer. +Get the known `Multiaddr` of a provided peer. All returned multiaddrs will include the encapsulated `PeerId` of the peer. `peerStore.addressBook.getMultiaddrsForPeer(peerId)` @@ -593,20 +619,20 @@ Get the known `multiaddr` of a provided peer. All returned multiaddrs will inclu | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to get | +| peerId | [`PeerId`][peer-id] | peerId to get | #### Returns | Type | Description | |------|-------------| -| `Array` | Array of peer's multiaddr | +| `Array` | Array of peer's multiaddr | #### Example ```js peerStore.addressBook.getMultiaddrsForPeer(peerId) // undefined -peerStore.addressBook.set(peerId, discoveredMultiaddr) +peerStore.addressBook.set(peerId, multiaddr) peerStore.addressBook.getMultiaddrsForPeer(peerId) // [ // /ip4/140.10.2.1/tcp/8000/p2p/QmW8rAgaaA6sRydK1k6vonShQME47aDxaFidbtMevWs73t @@ -618,109 +644,91 @@ peerStore.addressBook.getMultiaddrsForPeer(peerId) Set known `multiaddrs` of a given peer. -`peerStore.addressBook.set(peerId, multiaddrs, options)` +`peerStore.addressBook.set(peerId, multiaddrs)` #### Parameters | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to set | -| multiaddrs | `multiaddr|Array` | multiaddrs to store | -| [options] | `object` | options to set | -| [options.replace] | `Object` | replace stored data (if exists) or unique union (default: true) | +| peerId | [`PeerId`][peer-id] | peerId to set | +| multiaddrs | [`Multiaddr`][multiaddr]\|`Array` | multiaddrs to store | #### Returns | Type | Description | |------|-------------| -| `Array` | Array of peer's multiaddr with their relevant information | +| `Map` | Map of known peers' string identifier with their relevant information [`MultiaddrInfo`](multiaddr-info) | #### Example ```js -peerStore.addressBook.set(peerId, discoveredMultiaddr) -// [ -// { -// multiaddr: /ip4/140.10.2.1/tcp/8000, -// ... -// }, -// { -// multiaddr: /ip4/140.10.2.1/ws/8001 -// ... -// }, -// ] +peerStore.addressBook.add(peerId, multiaddr) ``` -### peerStore.protoBook.delete +### peerStore.protoBook.add -Delete the provided peer from the book. +Add known `protocols` of a given peer. -`peerStore.protoBook.delete(peerId)` +`peerStore.protoBook.add(peerId, protocols)` #### Parameters | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to remove | +| peerId | [`PeerId`][peer-id] | peerId to set | +| protocols | `string|Array` | protocols to add | #### Returns | Type | Description | |------|-------------| -| `boolean` | true if found and removed | +| `Map` | Map of known peers' string identifier with their supported protocols | #### Example ```js -peerStore.protoBook.delete(peerId) -// false -peerStore.protoBook.set(peerId, supportedProtocols) -peerStore.protoBook.delete(peerId) -// true +peerStore.protoBook.add(peerId, protocols) ``` -### peerStore.protoBook.get +### peerStore.protoBook.delete -Get the known `protocols` of a provided peer. +Delete the provided peer from the book. -`peerStore.protoBook.get(peerId)` +`peerStore.protoBook.delete(peerId)` #### Parameters | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to get | +| peerId | [`PeerId`][peer-id] | peerId to remove | #### Returns | Type | Description | |------|-------------| -| `Array` | Array of peer's supported protocols | +| `boolean` | true if found and removed | #### Example ```js -peerStore.protoBook.get(peerId) -// undefined -peerStore.protoBook.set(peerId, supportedProtocols) -peerStore.protoBook.get(peerId) -// [ '/proto/1.0.0', '/proto/1.1.0' ] +peerStore.protoBook.delete(peerId) +// false +peerStore.protoBook.set(peerId, protocols) +peerStore.protoBook.delete(peerId) +// true ``` -### peerStore.protoBook.set +### peerStore.protoBook.get -Set known `protocols` of a given peer. +Get the known `protocols` of a provided peer. -`peerStore.protoBook.set(peerId, protocols, options)` +`peerStore.protoBook.get(peerId)` #### Parameters | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to set | -| protocols | `string|Array` | protocols to store | -| [options] | `object` | options to set | -| [options.replace] | `Object` | replace stored data (if exists) or unique union (default: true) | +| peerId | [`PeerId`][peer-id] | peerId to get | #### Returns @@ -731,42 +739,36 @@ Set known `protocols` of a given peer. #### Example ```js -peerStore.protoBook.set(peerId, supportedProtocols) +peerStore.protoBook.get(peerId) +// undefined +peerStore.protoBook.set(peerId, [ '/proto/1.0.0', '/proto/1.1.0' ]) +peerStore.protoBook.get(peerId) // [ '/proto/1.0.0', '/proto/1.1.0' ] ``` -### peerStore.protoBook.supports +### peerStore.protoBook.set -Verify if the provided peer supports the given `protocols`. +Set known `protocols` of a given peer. -`peerStore.protoBook.supports(peerId, protocols)` +`peerStore.protoBook.set(peerId, protocols)` #### Parameters | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to get | -| protocols | `string|Array` | protocols to verify | +| peerId | [`PeerId`][peer-id] | peerId to set | +| protocols | `string|Array` | protocols to store | #### Returns | Type | Description | |------|-------------| -| `boolean` | true if found and removed | +| `Map` | Map of known peers' string identifier with their supported protocols | #### Example ```js -const supportedProtocols = [ '/proto/1.0.0', '/proto/1.1.0' ] -peerStore.protoBook.supports(peerId, supportedProtocols) -// false -peerStore.protoBook.supports(peerId, supportedProtocols[0]) -// false peerStore.protoBook.set(peerId, supportedProtocols) -peerStore.protoBook.supports(peerId, supportedProtocols) -// true -peerStore.protoBook.supports(peerId, supportedProtocols[0]) -// true ``` ### peerStore.delete @@ -779,7 +781,7 @@ Delete the provided peer from every book. | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to remove | +| peerId | [`PeerId`][peer-id] | peerId to remove | #### Returns @@ -803,34 +805,36 @@ peerStore.delete(peerId2) // true ``` -### peerStore.find +### peerStore.get -Find the stored information of a given peer. +Get the stored information of a given peer. -`peerStore.find(peerId)` +`peerStore.get(peerId)` #### Parameters | Name | Type | Description | |------|------|-------------| -| peerId | `peerid` | peerId to find | +| peerId | [`PeerId`][peer-id] | peerId to get | #### Returns | Type | Description | |------|-------------| -| `peerInfo` | Peer information of the provided peer | +| [`PeerInfo`][peer-info] | Peer information of the provided peer | + +TODO: change when `peer-info` is deprecated to new pointer #### Example ```js -peerStore.find(peerId) +peerStore.get(peerId) // false peerStore.addressBook.set(peerId, discoveredMultiaddrs) peerStore.protoBook.set(peerId, supportedProtocols) -peerStore.find(peerId) +peerStore.get(peerId) // { -// multiaddrInfos: [...], +// MultiaddrInfos: [...], // protocols: [...] // } ``` @@ -839,7 +843,7 @@ peerStore.find(peerId) Get all the stored information of every peer. -`peerStore.peers()` +`peerStore.peers` #### Returns @@ -847,12 +851,12 @@ Get all the stored information of every peer. |------|-------------| | `Map` | Peer information of every peer | -TODO: change when `peer-info` is deprecated (breaking change) +TODO: change when `peer-info` is deprecated to new pointer (breaking change) #### Example ```js -for (peer of peerStore.peers().values()) { +for (let [peerIdString, peerInfo] of peerStore.peers.entries()) { // peerInfo instance } ``` @@ -873,7 +877,7 @@ Gets a list of the peer-ids that are subscribed to one topic. | Type | Description | |------|-------------| -| `Array` | peer-id subscribed to the topic | +| `Array` | peer-id subscribed to the topic | #### Example @@ -891,7 +895,7 @@ Gets a list of topics the node is subscribed to. | Type | Description | |------|-------------| -| `Array` | topics the node is subscribed to | +| `Array` | topics the node is subscribed to | #### Example @@ -938,7 +942,7 @@ Subscribes the given handler to a pubsub topic. | Name | Type | Description | |------|------|-------------| | topic | `string` | topic to subscribe | -| handler | `function({ from: String, data: Buffer, seqno: Buffer, topicIDs: Array, signature: Buffer, key: Buffer })` | handler for new data on topic | +| handler | `function({ from: string, data: Buffer, seqno: Buffer, topicIDs: Array, signature: Buffer, key: Buffer })` | handler for new data on topic | #### Returns @@ -968,7 +972,7 @@ Unsubscribes the given handler from a pubsub topic. If no handler is provided, a | Name | Type | Description | |------|------|-------------| | topic | `string` | topic to unsubscribe | -| handler | `function()` | handler subscribed | +| handler | `function()` | handler subscribed | #### Returns @@ -1137,9 +1141,9 @@ This event will be triggered anytime we are disconnected from another peer, rega - `dataReceived`: The stringified value of total incoming data for this stat. - `dataSent`: The stringified value of total outgoing data for this stat. - `movingAverages`: The properties are dependent on the configuration of the moving averages interval. Defaults are listed here. - - `['60000']`: The calculated moving average at a 1 minute interval. - - `['300000']`: The calculated moving average at a 5 minute interval. - - `['900000']`: The calculated moving average at a 15 minute interval. + - `['60000']`: The calculated moving average at a 1 minute interval. + - `['300000']`: The calculated moving average at a 5 minute interval. + - `['900000']`: The calculated moving average at a 15 minute interval. - `snapshot`: A getter that returns a clone of the raw stats. - `dataReceived`: A [`BigNumber`](https://github.com/MikeMcl/bignumber.js/) of the amount of incoming data - `dataSent`: A [`BigNumber`](https://github.com/MikeMcl/bignumber.js/) of the amount of outgoing data @@ -1148,6 +1152,7 @@ This event will be triggered anytime we are disconnected from another peer, rega - `['300000']`: The [MovingAverage](https://www.npmjs.com/package/moving-averages) at a 5 minute interval. - `['900000']`: The [MovingAverage](https://www.npmjs.com/package/moving-averages) at a 15 minute interval. +[multiaddr-info]: https://github.com/libp2p/js-libp2p/tree/master/src/peer-store/address-book.js [cid]: https://github.com/multiformats/js-cid [connection]: https://github.com/libp2p/js-interfaces/tree/master/src/connection [multiaddr]: https://github.com/multiformats/js-multiaddr diff --git a/src/dialer/index.js b/src/dialer/index.js index 9d257cb5f3..de9a394394 100644 --- a/src/dialer/index.js +++ b/src/dialer/index.js @@ -111,7 +111,7 @@ class Dialer { } } - dialable.multiaddrs && this.peerStore.addressBook.set(dialable.id, Array.from(dialable.multiaddrs), { replace: false }) + dialable.multiaddrs && this.peerStore.addressBook.add(dialable.id, Array.from(dialable.multiaddrs)) const addrs = this.peerStore.addressBook.getMultiaddrsForPeer(dialable.id) return { diff --git a/src/get-peer-info.js b/src/get-peer-info.js index 122a8ddc6b..5b0748ea8e 100644 --- a/src/get-peer-info.js +++ b/src/get-peer-info.js @@ -39,8 +39,8 @@ function getPeerInfo (peer, peerStore) { addr && peer.multiaddrs.add(addr) if (peerStore) { - peerStore.addressBook.set(peer.id, peer.multiaddrs.toArray(), { replace: false }) - peerStore.protoBook.set(peer.id, Array.from(peer.protocols), { replace: false }) + peerStore.addressBook.add(peer.id, peer.multiaddrs.toArray()) + peerStore.protoBook.add(peer.id, Array.from(peer.protocols)) } return peer diff --git a/src/identify/index.js b/src/identify/index.js index 75a685948c..3fe06f4f5c 100644 --- a/src/identify/index.js +++ b/src/identify/index.js @@ -206,9 +206,6 @@ class IdentifyService { protocols: Array.from(this._protocols.keys()) }) - // TODO: should we add to peerStore.addressBook.set() here? - // We can have an inbound connection from an unkwown peer - pipe( [message], lp.encode(), diff --git a/src/index.js b/src/index.js index db597cae8b..a2028dccb4 100644 --- a/src/index.js +++ b/src/index.js @@ -292,7 +292,7 @@ class Libp2p extends EventEmitter { // TODO Inconsistency from: getDialable adds a set, while regular peerInfo uses a Multiaddr set // This should be handled on `peer-info` removal const multiaddrs = dialable.multiaddrs.toArray ? dialable.multiaddrs.toArray() : Array.from(dialable.multiaddrs) - this.peerStore.addressBook.set(dialable.id, multiaddrs, { replace: false }) + this.peerStore.addressBook.add(dialable.id, multiaddrs) connection = this.registrar.getConnection(dialable) } @@ -436,8 +436,8 @@ class Libp2p extends EventEmitter { } // TODO: once we deprecate peer-info, we should only set if we have data - this.peerStore.addressBook.set(peerInfo.id, peerInfo.multiaddrs.toArray(), { replace: false }) - this.peerStore.protoBook.set(peerInfo.id, Array.from(peerInfo.protocols), { replace: false }) + this.peerStore.addressBook.add(peerInfo.id, peerInfo.multiaddrs.toArray()) + this.peerStore.protoBook.set(peerInfo.id, Array.from(peerInfo.protocols)) } /** diff --git a/src/peer-store/README.md b/src/peer-store/README.md index 835c8c7ea5..a17b26c79a 100644 --- a/src/peer-store/README.md +++ b/src/peer-store/README.md @@ -1,18 +1,16 @@ -# Peerstore +# PeerStore -Libp2p's Peerstore is responsible for keeping an updated register with the relevant information of the known peers. It should gather environment changes, be able to take decisions and notice interested parties of relevant changes. The Peerstore comprises four main components: `addressBook`, `keyBook`, `protocolBook` and `metadataBook`. These book components have similar characteristics with the `Javascript Map` implementation. +Libp2p's PeerStore is responsible for keeping an updated register with the relevant information of the known peers. It should be the single source of truth for all peer data, where a subsystem can learn about peers' data and where someone can listen for updates. The PeerStore comprises four main components: `addressBook`, `keyBook`, `protocolBook` and `metadataBook`. -The PeerStore manages the high level operations on its inner books. Moreover, the peerStore should be responsible for notifying interested parties of relevant events, through its Event Emitter. - -(Future considerations: Peerstore should manage a job runner to trigger books runners for data trimming or computations) +The PeerStore manages the high level operations on its inner books. Moreover, the PeerStore should be responsible for notifying interested parties of relevant events, through its Event Emitter. ## Data gathering Several libp2p subsystems will perform operations, which will gather relevant information about peers. Some operations might not have this as an end goal, but can also gather important data. -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). +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. 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. @@ -22,11 +20,11 @@ In the background, the Identify Service is also waiting for protocol change noti While it is currently not supported in js-libp2p, future iterations may also support the [IdentifyDelta protocol](https://github.com/libp2p/specs/pull/176). -It is also possible to gather relevant information for peers from other protocols / subsystems. For instance, in `DHT` operations, nodes can exchange peer data as part of the `DHT` operation. In this case, we can learn additional information about a peer we already know. In this scenario the `PeerStore` should not replace the existing data it has, just add it. +It is also possible to gather relevant information for peers from other protocols / subsystems. For instance, in `DHT` operations, nodes can exchange peer data as part of the `DHT` operation. In this case, we can learn additional information about a peer we already know. In this scenario the PeerStore should not replace the existing data it has, just add it. ## Data Consumption -When the `PeerStore` data is updated, this information might be important for different parties. +When the PeerStore data is updated, this information might be important for different parties. Every time a peer needs to dial another peer, it is essential that it knows the multiaddrs used by the peer, in order to perform a successful dial to it. The same is true for pinging a peer. While the `AddressBook` is going to keep its data updated, it will also emit `change:multiaddrs` events so that subsystems/users interested in knowing these changes can be notifyied instead of pooling the `AddressBook`. @@ -34,7 +32,7 @@ Everytime a peer starts/stops supporting a protocol, libp2p subsystems or users ## PeerStore implementation -The Peerstore wraps four main components: `addressBook`, `keyBook`, `protocolBook` and `metadataBook`. Moreover, it provides a high level API for those components, as well as data events. +The PeerStore wraps four main components: `addressBook`, `keyBook`, `protocolBook` and `metadataBook`. Moreover, it provides a high level API for those components, as well as data events. ### API @@ -51,9 +49,9 @@ High level operations: Deletes the provided peer from every book. -- [`peerStore.find(peerId)`](../../doc/API.md#peerstorefind) +- [`peerStore.get(peerId)`](../../doc/API.md#peerstoreget) -Finds the stored information of a given peer. +Gets the stored information of a given peer. - [`peerStore.peers()`](../../doc/API.md#peerstorepeers) @@ -78,14 +76,13 @@ A `peerId.toString()` identifier mapping to a `multiaddrInfo` object, which shou ```js { multiaddr: , - validity: , - confidence: } ``` **Note:** except for multiaddr naming, the other properties are placeholders for now and might not be as described in the future milestones. - `addressBook.data` +- [`addressBook.add()`](../../doc/API.md#peerstoreaddressbookadd) - [`addressBook.delete()`](../../doc/API.md#peerstoreaddressbookdelete) - [`addressBook.get()`](../../doc/API.md#peerstoreaddressbookget) - [`addressBook.getMultiaddrsForPeer()`](../../doc/API.md#peerstoreaddressbookgetmultiaddrsforpeer) @@ -110,11 +107,16 @@ The `protoBook` holds the identifiers of the protocols supported by each peer. T A `peerId.toString()` identifier mapping to a `Set` of protocol identifier strings. - `protoBook.data` +- [`protoBook.add()`](../../doc/API.md#peerstoreprotobookadd) - [`protoBook.delete()`](../../doc/API.md#peerstoreprotobookdelete) - [`protoBook.get()`](../../doc/API.md#peerstoreprotobookget) - [`protoBook.set()`](../../doc/API.md#peerstoreprotobookset) -- [`protoBook.supports()`](../../doc/API.md#peerstoreprotobooksupports) #### Metadata Book **Not Yet Implemented** + +## Future Considerations + +- If multiaddr TTLs are added, the PeerStore may schedule jobs to delete all addresses that exceed the TTL to prevent AddressBook bloating +- Further API methods will probably need to be added in the context of multiaddr validity and confidence. diff --git a/src/peer-store/address-book.js b/src/peer-store/address-book.js index c9a54bb493..304024efb6 100644 --- a/src/peer-store/address-book.js +++ b/src/peer-store/address-book.js @@ -22,10 +22,8 @@ const { class AddressBook extends Book { /** * MultiaddrInfo object - * @typedef {Object} multiaddrInfo + * @typedef {Object} MultiaddrInfo * @property {Multiaddr} multiaddr peer multiaddr. - * @property {number} validity NOT USED YET - * @property {number} confidence NOT USED YET */ /** @@ -33,78 +31,40 @@ class AddressBook extends Book { * @param {EventEmitter} peerStore */ constructor (peerStore) { - super(peerStore, 'change:multiaddrs', 'multiaddrs') /** * PeerStore Event emitter, used by the AddressBook to emit: * "peer" - emitted when a peer is discovered by the node. * "change:multiaddrs" - emitted when the known multiaddrs of a peer change. */ - this._ps = peerStore + super(peerStore, 'change:multiaddrs', 'multiaddrs') /** * Map known peers to their known multiaddrs. - * @type {Map} + * @type {Map>} */ this.data = new Map() } /** * Set known addresses of a provided peer. + * @override * @param {PeerId} peerId * @param {Array|Multiaddr} addresses - * @param {Object} [options] - * @param {boolean} [options.replace = true] whether addresses received replace stored ones or a unique union is performed. - * @returns {Array} + * @returns {Map>} */ - set (peerId, addresses, { replace = true } = {}) { + set (peerId, addresses) { 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) } - if (!addresses) { - log.error('addresses must be provided to store data') - throw errcode(new Error('addresses must be provided'), ERR_INVALID_PARAMETERS) - } - - if (!Array.isArray(addresses)) { - addresses = [addresses] - } - - // create multiaddrInfo for each address - const multiaddrInfos = [] - addresses.forEach((addr) => { - if (!multiaddr.isMultiaddr(addr)) { - log.error(`multiaddr ${addr} must be an instance of multiaddr`) - throw errcode(new Error(`multiaddr ${addr} must be an instance of multiaddr`), ERR_INVALID_PARAMETERS) - } - - multiaddrInfos.push({ - multiaddr: addr - }) - }) - - if (replace) { - return this._replace(peerId, multiaddrInfos) - } - - return this._add(peerId, multiaddrInfos) - } - - /** - * Replace known addresses of a provided peer. - * If the peer is not known, it is set with the given addresses. - * @param {PeerId} peerId - * @param {Array} multiaddrInfos - * @returns {Array} - */ - _replace (peerId, multiaddrInfos) { + const multiaddrInfos = this._toMultiaddrInfos(addresses) const id = peerId.toString() const rec = this.data.get(id) // Not replace multiaddrs if (!multiaddrInfos.length) { - return rec ? [...rec] : [] + return this.data } // Already knows the peer @@ -115,7 +75,7 @@ class AddressBook extends Book { // If yes, no changes needed! if (intersection.length === rec.length) { log(`the addresses provided to store are equal to the already stored for ${id}`) - return [...multiaddrInfos] + return this.data } } @@ -138,17 +98,24 @@ class AddressBook extends Book { multiaddrs: multiaddrInfos.map((mi) => mi.multiaddr) }) - return [...multiaddrInfos] + return this.data } /** - * Add new known addresses to a provided peer. + * Add known addresses of a provided peer. * If the peer is not known, it is set with the given addresses. + * @override * @param {PeerId} peerId - * @param {Array} multiaddrInfos - * @returns {Array} + * @param {Array|Multiaddr} addresses + * @returns {Map>} */ - _add (peerId, multiaddrInfos) { + add (peerId, addresses) { + 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 multiaddrInfos = this._toMultiaddrInfos(addresses) const id = peerId.toString() const rec = this.data.get(id) @@ -163,7 +130,7 @@ class AddressBook extends Book { // The content is the same, no need to update. if (rec && rec.length === multiaddrInfos.length) { log(`the addresses provided to store are already stored for ${id}`) - return [...multiaddrInfos] + return this.data } this.data.set(id, multiaddrInfos) @@ -186,7 +153,38 @@ class AddressBook extends Book { this._ps.emit('peer', peerInfo) } - return [...multiaddrInfos] + return this.data + } + + /** + * Transforms received multiaddrs into MultiaddrInfo. + * @param {Array|Multiaddr} addresses + * @returns {Array} + */ + _toMultiaddrInfos (addresses) { + if (!addresses) { + log.error('addresses must be provided to store data') + throw errcode(new Error('addresses must be provided'), ERR_INVALID_PARAMETERS) + } + + if (!Array.isArray(addresses)) { + addresses = [addresses] + } + + // create MultiaddrInfo for each address + const multiaddrInfos = [] + addresses.forEach((addr) => { + if (!multiaddr.isMultiaddr(addr)) { + log.error(`multiaddr ${addr} must be an instance of multiaddr`) + throw errcode(new Error(`multiaddr ${addr} must be an instance of multiaddr`), ERR_INVALID_PARAMETERS) + } + + multiaddrInfos.push({ + multiaddr: addr + }) + }) + + return multiaddrInfos } /** diff --git a/src/peer-store/book.js b/src/peer-store/book.js index 02168d2386..cbbb6e18fa 100644 --- a/src/peer-store/book.js +++ b/src/peer-store/book.js @@ -12,8 +12,8 @@ const { * The Book is the skeleton for the PeerStore books. */ class Book { - constructor (eventEmitter, eventName, eventProperty) { - this.eventEmitter = eventEmitter + constructor (peerStore, eventName, eventProperty) { + this._ps = peerStore this.eventName = eventName this.eventProperty = eventProperty @@ -28,10 +28,17 @@ class Book { * Set known data of a provided peer. * @param {PeerId} peerId * @param {Array|Data} data - * @param {Object} [options] - * @param {boolean} [options.replace = true] wether data received replace stored the one or a unique union is performed. */ - set (peerId, data, options) { + set (peerId, data) { + throw errcode(new Error('set must be implemented by the subclass'), 'ERR_NOT_IMPLEMENTED') + } + + /** + * Add known data of a provided peer. + * @param {PeerId} peerId + * @param {Array|Data} data + */ + add (peerId, data) { throw errcode(new Error('set must be implemented by the subclass'), 'ERR_NOT_IMPLEMENTED') } @@ -67,7 +74,7 @@ class Book { // TODO: Remove peerInfo and its usage on peer-info deprecate const peerInfo = new PeerInfo(peerId) - this.eventEmitter.emit(this.eventName, { + this._ps.emit(this.eventName, { peerId, peerInfo, [this.eventProperty]: [] diff --git a/src/peer-store/proto-book.js b/src/peer-store/proto-book.js index be7762e4b0..960b57162f 100644 --- a/src/peer-store/proto-book.js +++ b/src/peer-store/proto-book.js @@ -25,17 +25,15 @@ class ProtoBook extends Book { * @param {EventEmitter} peerStore */ constructor (peerStore) { - super(peerStore, 'change:protocols', 'protocols') - /** * PeerStore Event emitter, used by the ProtoBook to emit: * "change:protocols" - emitted when the known protocols of a peer change. */ - this._ps = peerStore + super(peerStore, 'change:protocols', 'protocols') /** * Map known peers to their known protocols. - * @type {Map} + * @type {Map>} */ this.data = new Map() } @@ -46,11 +44,9 @@ class ProtoBook extends Book { * @override * @param {PeerId} peerId * @param {Array|string} protocols - * @param {Object} [options] - * @param {boolean} [options.replace = true] whether protocols received replace stored ones or a unique union is performed. - * @returns {Array} + * @returns {Map>} */ - set (peerId, protocols, { replace = true } = {}) { + set (peerId, protocols) { 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) @@ -65,21 +61,6 @@ class ProtoBook extends Book { protocols = [protocols] } - if (replace) { - return this._replace(peerId, protocols) - } - - return this._add(peerId, protocols) - } - - /** - * Replace known protocols of a provided peer. - * If the peer is not known, it is set with the given protocols. - * @param {PeerId} peerId - * @param {Array} protocols - * @returns {Array} - */ - _replace (peerId, protocols) { const id = peerId.toString() const recSet = this.data.get(id) const newSet = new Set(protocols) @@ -90,7 +71,7 @@ class ProtoBook extends Book { // If yes, no changes needed! if (recSet && isSetEqual(recSet, newSet)) { log(`the protocols provided to store are equal to the already stored for ${id}`) - return protocols + return this.data } this.data.set(id, newSet) @@ -106,17 +87,32 @@ class ProtoBook extends Book { protocols }) - return protocols + return this.data } /** - * Add new known protocols to a provided peer. - * If the peer is not known, it is set with the given protocols. + * Adds known protocols of a provided peer. + * If the peer was not known before, it will be added. + * @override * @param {PeerId} peerId * @param {Array|string} protocols - * @returns {Array} + * @returns {Map>} */ - _add (peerId, protocols) { + add (peerId, protocols) { + 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) + } + + if (!protocols) { + log.error('protocols must be provided to store data') + throw errcode(new Error('protocols must be provided'), ERR_INVALID_PARAMETERS) + } + + if (!Array.isArray(protocols)) { + protocols = [protocols] + } + const id = peerId.toString() const recSet = this.data.get(id) || new Set() const newSet = new Set([...recSet, ...protocols]) // Set Union @@ -124,7 +120,7 @@ class ProtoBook extends Book { // Any new protocol added? if (recSet.size === newSet.size) { log(`the protocols provided to store are already stored for ${id}`) - return protocols + return this.data } protocols = [...newSet] @@ -142,35 +138,7 @@ class ProtoBook extends Book { protocols }) - return protocols - } - - /** - * Verify if the provided peer supports the given protocols. - * @param {PeerId} peerId - * @param {Array|string} protocols - * @returns {boolean} - */ - supports (peerId, protocols) { - if (!PeerId.isPeerId(peerId)) { - throw errcode(new Error('peerId must be an instance of peer-id'), ERR_INVALID_PARAMETERS) - } - - if (!protocols) { - throw errcode(new Error('protocols must be provided'), ERR_INVALID_PARAMETERS) - } - - if (!Array.isArray(protocols)) { - protocols = [protocols] - } - - const recSet = this.data.get(peerId.toString()) - - if (!recSet) { - return false - } - - return [...recSet].filter((p) => protocols.includes(p)).length === protocols.length + return this.data } } diff --git a/test/dialing/direct.node.js b/test/dialing/direct.node.js index dccc583f2d..734687d0a2 100644 --- a/test/dialing/direct.node.js +++ b/test/dialing/direct.node.js @@ -100,7 +100,7 @@ describe('Dialing (direct, TCP)', () => { transportManager: localTM, peerStore: { addressBook: { - set: () => {}, + add: () => {}, getMultiaddrsForPeer: () => [remoteAddr] } } @@ -135,7 +135,7 @@ describe('Dialing (direct, TCP)', () => { transportManager: localTM, peerStore: { addressBook: { - set: () => {}, + add: () => {}, getMultiaddrsForPeer: () => [unsupportedAddr] } } @@ -179,7 +179,7 @@ describe('Dialing (direct, TCP)', () => { concurrency: 2, peerStore: { addressBook: { - set: () => {}, + add: () => {}, getMultiaddrsForPeer: () => addrs } } diff --git a/test/dialing/direct.spec.js b/test/dialing/direct.spec.js index 6651b3de61..3d4041120b 100644 --- a/test/dialing/direct.spec.js +++ b/test/dialing/direct.spec.js @@ -88,7 +88,7 @@ describe('Dialing (direct, WebSockets)', () => { transportManager: localTM, peerStore: { addressBook: { - set: () => {}, + add: () => {}, getMultiaddrsForPeer: () => [remoteAddr] } } @@ -104,7 +104,7 @@ describe('Dialing (direct, WebSockets)', () => { transportManager: localTM, peerStore: { addressBook: { - set: () => {}, + add: () => {}, getMultiaddrsForPeer: () => [remoteAddr] } } @@ -128,7 +128,7 @@ describe('Dialing (direct, WebSockets)', () => { transportManager: localTM, peerStore: { addressBook: { - set: () => {}, + add: () => {}, getMultiaddrsForPeer: () => [remoteAddr] } } @@ -163,7 +163,7 @@ describe('Dialing (direct, WebSockets)', () => { timeout: 50, peerStore: { addressBook: { - set: () => {}, + add: () => {}, getMultiaddrsForPeer: () => [remoteAddr] } } @@ -337,7 +337,7 @@ describe('Dialing (direct, WebSockets)', () => { }) sinon.spy(libp2p.dialer, 'connectToPeer') - sinon.spy(libp2p.peerStore.addressBook, 'set') + sinon.spy(libp2p.peerStore.addressBook, 'add') const connection = await libp2p.dial(remoteAddr) expect(connection).to.exist() @@ -346,7 +346,7 @@ describe('Dialing (direct, WebSockets)', () => { expect(protocol).to.equal('/echo/1.0.0') await connection.close() expect(libp2p.dialer.connectToPeer.callCount).to.equal(1) - expect(libp2p.peerStore.addressBook.set.callCount).to.be.at.least(1) + expect(libp2p.peerStore.addressBook.add.callCount).to.be.at.least(1) }) it('should run identify automatically after connecting', async () => { diff --git a/test/identify/index.spec.js b/test/identify/index.spec.js index e585233f73..7af2f67c84 100644 --- a/test/identify/index.spec.js +++ b/test/identify/index.spec.js @@ -220,14 +220,15 @@ describe('Identify', () => { }) sinon.spy(libp2p.identifyService, 'identify') - const peerStoreSpy = sinon.spy(libp2p.peerStore.addressBook, 'set') + const peerStoreSpySet = sinon.spy(libp2p.peerStore.addressBook, 'set') + const peerStoreSpyAdd = sinon.spy(libp2p.peerStore.addressBook, 'add') const connection = await libp2p.dialer.connectToPeer(remoteAddr) expect(connection).to.exist() // Wait for peer store to be updated // Dialer._createDialTarget (add), Identify (replace) - await pWaitFor(() => peerStoreSpy.callCount === 2) + await pWaitFor(() => peerStoreSpySet.callCount === 1 && peerStoreSpyAdd.callCount === 1) expect(libp2p.identifyService.identify.callCount).to.equal(1) // The connection should have no open streams diff --git a/test/peer-store/address-book.spec.js b/test/peer-store/address-book.spec.js index 140699eb05..1ec11eb742 100644 --- a/test/peer-store/address-book.spec.js +++ b/test/peer-store/address-book.spec.js @@ -4,7 +4,6 @@ const chai = require('chai') chai.use(require('dirty-chai')) const { expect } = chai -const sinon = require('sinon') const { EventEmitter } = require('events') const pDefer = require('p-defer') @@ -62,9 +61,6 @@ describe('addressBook', () => { it('replaces the stored content by default and emit change event', () => { const defer = pDefer() - sinon.spy(ab, '_replace') - sinon.spy(ab, '_add') - const supportedMultiaddrs = [addr1, addr2] ee.once('change:multiaddrs', ({ peerId, multiaddrs }) => { @@ -73,51 +69,15 @@ describe('addressBook', () => { defer.resolve() }) - const multiaddrInfos = ab.set(peerId, supportedMultiaddrs) + ab.set(peerId, supportedMultiaddrs) + const multiaddrInfos = ab.get(peerId) const multiaddrs = multiaddrInfos.map((mi) => mi.multiaddr) - - expect(ab._replace.callCount).to.equal(1) - expect(ab._add.callCount).to.equal(0) expect(multiaddrs).to.have.deep.members(supportedMultiaddrs) return defer.promise }) - it('adds the new content if replace is disabled and emit change event', () => { - const defer = pDefer() - sinon.spy(ab, '_replace') - sinon.spy(ab, '_add') - - const supportedMultiaddrsA = [addr1, addr2] - const supportedMultiaddrsB = [addr3] - const finalMultiaddrs = supportedMultiaddrsA.concat(supportedMultiaddrsB) - - let changeTrigger = 2 - ee.on('change:multiaddrs', ({ multiaddrs }) => { - changeTrigger-- - if (changeTrigger === 0 && arraysAreEqual(multiaddrs, finalMultiaddrs)) { - defer.resolve() - } - }) - - // Replace - let multiaddrInfos = ab.set(peerId, supportedMultiaddrsA) - let multiaddrs = multiaddrInfos.map((mi) => mi.multiaddr) - expect(ab._replace.callCount).to.equal(1) - expect(ab._add.callCount).to.equal(0) - expect(multiaddrs).to.have.deep.members(supportedMultiaddrsA) - - // Add - multiaddrInfos = ab.set(peerId, supportedMultiaddrsB, { replace: false }) - multiaddrs = multiaddrInfos.map((mi) => mi.multiaddr) - expect(ab._replace.callCount).to.equal(1) - expect(ab._add.callCount).to.equal(1) - expect(multiaddrs).to.have.deep.members(finalMultiaddrs) - - return defer.promise - }) - - it('emits on set (replace) if not storing the exact same content', async () => { + it('emits on set if not storing the exact same content', async () => { const defer = pDefer() const supportedMultiaddrsA = [addr1, addr2] @@ -135,14 +95,15 @@ describe('addressBook', () => { ab.set(peerId, supportedMultiaddrsA) // set 2 (same content) - const multiaddrInfos = ab.set(peerId, supportedMultiaddrsB) + ab.set(peerId, supportedMultiaddrsB) + const multiaddrInfos = ab.get(peerId) const multiaddrs = multiaddrInfos.map((mi) => mi.multiaddr) expect(multiaddrs).to.have.deep.members(supportedMultiaddrsB) await defer.promise }) - it('does not emit on set (replace) if it is storing the exact same content', async () => { + it('does not emit on set if it is storing the exact same content', async () => { const defer = pDefer() const supportedMultiaddrs = [addr1, addr2] @@ -168,8 +129,69 @@ describe('addressBook', () => { await defer.promise }) + }) + + describe('addressBook.add', () => { + let ee, ab + + beforeEach(() => { + ee = new EventEmitter() + ab = new AddressBook(ee) + }) + + afterEach(() => { + ee.removeAllListeners() + }) - it('emits on set (add) if the content to add not exists', async () => { + it('throwns invalid parameters error if invalid PeerId is provided', () => { + expect(() => { + ab.add('invalid peerId') + }).to.throw(ERR_INVALID_PARAMETERS) + }) + + it('throwns invalid parameters error if no addresses provided', () => { + expect(() => { + ab.add(peerId) + }).to.throw(ERR_INVALID_PARAMETERS) + }) + + it('throwns invalid parameters error if invalid multiaddrs are provided', () => { + expect(() => { + ab.add(peerId, 'invalid multiaddr') + }).to.throw(ERR_INVALID_PARAMETERS) + }) + + it('adds the new content and emits change event', () => { + const defer = pDefer() + + const supportedMultiaddrsA = [addr1, addr2] + const supportedMultiaddrsB = [addr3] + const finalMultiaddrs = supportedMultiaddrsA.concat(supportedMultiaddrsB) + + let changeTrigger = 2 + ee.on('change:multiaddrs', ({ multiaddrs }) => { + changeTrigger-- + if (changeTrigger === 0 && arraysAreEqual(multiaddrs, finalMultiaddrs)) { + defer.resolve() + } + }) + + // Replace + ab.set(peerId, supportedMultiaddrsA) + let multiaddrInfos = ab.get(peerId) + let multiaddrs = multiaddrInfos.map((mi) => mi.multiaddr) + expect(multiaddrs).to.have.deep.members(supportedMultiaddrsA) + + // Add + ab.add(peerId, supportedMultiaddrsB) + multiaddrInfos = ab.get(peerId) + multiaddrs = multiaddrInfos.map((mi) => mi.multiaddr) + expect(multiaddrs).to.have.deep.members(finalMultiaddrs) + + return defer.promise + }) + + it('emits on add if the content to add not exists', async () => { const defer = pDefer() const supportedMultiaddrsA = [addr1] @@ -188,14 +210,15 @@ describe('addressBook', () => { ab.set(peerId, supportedMultiaddrsA) // set 2 (content already existing) - const multiaddrInfos = ab.set(peerId, supportedMultiaddrsB, { replace: false }) + ab.add(peerId, supportedMultiaddrsB) + const multiaddrInfos = ab.get(peerId) const multiaddrs = multiaddrInfos.map((mi) => mi.multiaddr) expect(multiaddrs).to.have.deep.members(finalMultiaddrs) await defer.promise }) - it('does not emit on set (merge) if the content to add already exists', async () => { + it('does not emit on add if the content to add already exists', async () => { const defer = pDefer() const supportedMultiaddrsA = [addr1, addr2] @@ -213,7 +236,7 @@ describe('addressBook', () => { ab.set(peerId, supportedMultiaddrsA) // set 2 (content already existing) - ab.set(peerId, supportedMultiaddrsB, { replace: false }) + ab.add(peerId, supportedMultiaddrsB) // Wait 50ms for incorrect second event setTimeout(() => { diff --git a/test/peer-store/proto-book.spec.js b/test/peer-store/proto-book.spec.js index 5c9fe5b983..7985dfe1dc 100644 --- a/test/peer-store/proto-book.spec.js +++ b/test/peer-store/proto-book.spec.js @@ -4,7 +4,6 @@ const chai = require('chai') chai.use(require('dirty-chai')) const { expect } = chai -const sinon = require('sinon') const { EventEmitter } = require('events') const pDefer = require('p-defer') @@ -51,9 +50,6 @@ describe('protoBook', () => { it('replaces the stored content by default and emit change event', () => { const defer = pDefer() - sinon.spy(pb, '_replace') - sinon.spy(pb, '_add') - const supportedProtocols = ['protocol1', 'protocol2'] ee.once('change:protocols', ({ peerId, protocols }) => { @@ -62,48 +58,14 @@ describe('protoBook', () => { defer.resolve() }) - const protocols = pb.set(peerId, supportedProtocols) - - expect(pb._replace.callCount).to.equal(1) - expect(pb._add.callCount).to.equal(0) + pb.set(peerId, supportedProtocols) + const protocols = pb.get(peerId) expect(protocols).to.have.deep.members(supportedProtocols) return defer.promise }) - it('adds the new content if replace is disabled and emit change event', () => { - const defer = pDefer() - sinon.spy(pb, '_replace') - sinon.spy(pb, '_add') - - const supportedProtocolsA = ['protocol1', 'protocol2'] - const supportedProtocolsB = ['protocol3'] - const finalProtocols = supportedProtocolsA.concat(supportedProtocolsB) - - let changeTrigger = 2 - ee.on('change:protocols', ({ protocols }) => { - changeTrigger-- - if (changeTrigger === 0 && arraysAreEqual(protocols, finalProtocols)) { - defer.resolve() - } - }) - - // Replace - let protocols = pb.set(peerId, supportedProtocolsA) - expect(pb._replace.callCount).to.equal(1) - expect(pb._add.callCount).to.equal(0) - expect(protocols).to.have.deep.members(supportedProtocolsA) - - // Add - protocols = pb.set(peerId, supportedProtocolsB, { replace: false }) - expect(pb._replace.callCount).to.equal(1) - expect(pb._add.callCount).to.equal(1) - expect(protocols).to.have.deep.members(finalProtocols) - - return defer.promise - }) - - it('emits on set (replace) if not storing the exact same content', () => { + it('emits on set if not storing the exact same content', () => { const defer = pDefer() const supportedProtocolsA = ['protocol1', 'protocol2'] @@ -121,13 +83,14 @@ describe('protoBook', () => { pb.set(peerId, supportedProtocolsA) // set 2 (same content) - const protocols = pb.set(peerId, supportedProtocolsB) + pb.set(peerId, supportedProtocolsB) + const protocols = pb.get(peerId) expect(protocols).to.have.deep.members(supportedProtocolsB) return defer.promise }) - it('does not emit on set (replace) if it is storing the exact same content', () => { + it('does not emit on set if it is storing the exact same content', () => { const defer = pDefer() const supportedProtocols = ['protocol1', 'protocol2'] @@ -153,8 +116,61 @@ describe('protoBook', () => { return defer.promise }) + }) + + describe('protoBook.add', () => { + let ee, pb + + beforeEach(() => { + ee = new EventEmitter() + pb = new ProtoBook(ee) + }) + + afterEach(() => { + ee.removeAllListeners() + }) + + it('throwns invalid parameters error if invalid PeerId is provided', () => { + expect(() => { + pb.add('invalid peerId') + }).to.throw(ERR_INVALID_PARAMETERS) + }) + + it('throwns invalid parameters error if no protocols provided', () => { + expect(() => { + pb.add(peerId) + }).to.throw(ERR_INVALID_PARAMETERS) + }) + + it('adds the new content and emits change event', () => { + const defer = pDefer() + + const supportedProtocolsA = ['protocol1', 'protocol2'] + const supportedProtocolsB = ['protocol3'] + const finalProtocols = supportedProtocolsA.concat(supportedProtocolsB) + + let changeTrigger = 2 + ee.on('change:protocols', ({ protocols }) => { + changeTrigger-- + if (changeTrigger === 0 && arraysAreEqual(protocols, finalProtocols)) { + defer.resolve() + } + }) + + // Replace + pb.set(peerId, supportedProtocolsA) + let protocols = pb.get(peerId) + expect(protocols).to.have.deep.members(supportedProtocolsA) + + // Add + pb.add(peerId, supportedProtocolsB) + protocols = pb.get(peerId) + expect(protocols).to.have.deep.members(finalProtocols) + + return defer.promise + }) - it('emits on set (add) if the content to add not exists', () => { + it('emits on add if the content to add not exists', () => { const defer = pDefer() const supportedProtocolsA = ['protocol1'] @@ -173,13 +189,14 @@ describe('protoBook', () => { pb.set(peerId, supportedProtocolsA) // set 2 (content already existing) - const protocols = pb.set(peerId, supportedProtocolsB, { replace: false }) + pb.add(peerId, supportedProtocolsB) + const protocols = pb.get(peerId) expect(protocols).to.have.deep.members(finalProtocols) return defer.promise }) - it('does not emit on set (merge) if the content to add already exists', () => { + it('does not emit on add if the content to add already exists', () => { const defer = pDefer() const supportedProtocolsA = ['protocol1', 'protocol2'] @@ -197,7 +214,7 @@ describe('protoBook', () => { pb.set(peerId, supportedProtocolsA) // set 2 (content already existing) - pb.set(peerId, supportedProtocolsB, { replace: false }) + pb.add(peerId, supportedProtocolsB) // Wait 50ms for incorrect second event setTimeout(() => { @@ -238,53 +255,6 @@ describe('protoBook', () => { }) }) - describe('protoBook.supports', () => { - let ee, pb - - beforeEach(() => { - ee = new EventEmitter() - pb = new ProtoBook(ee) - }) - - it('throwns invalid parameters error if invalid PeerId is provided', () => { - expect(() => { - pb.supports('invalid peerId') - }).to.throw(ERR_INVALID_PARAMETERS) - }) - - it('throwns invalid parameters error if no protocols provided', () => { - expect(() => { - pb.supports(peerId) - }).to.throw(ERR_INVALID_PARAMETERS) - }) - - it('returns false if no records exist for the peer', () => { - const supportedProtocols = ['protocol1'] - const supports = pb.supports(peerId, supportedProtocols[0]) - - expect(supports).to.equal(false) - }) - - it('returns true if the protocol is supported', () => { - const supportedProtocols = ['protocol1', 'protocol2'] - - pb.set(peerId, supportedProtocols) - - const supports = pb.supports(peerId, supportedProtocols[1]) - expect(supports).to.equal(true) - }) - - it('returns false if part of the protocols is supported', () => { - const supportedProtocols = ['protocol1', 'protocol2'] - const otherProtocols = ['protocol3', 'protocol4'] - - pb.set(peerId, supportedProtocols) - - const supports = pb.supports(peerId, [supportedProtocols[0], ...otherProtocols]) - expect(supports).to.equal(false) - }) - }) - describe('protoBook.delete', () => { let ee, pb diff --git a/test/registrar/registrar.spec.js b/test/registrar/registrar.spec.js index a8c17c935e..3e2744901e 100644 --- a/test/registrar/registrar.spec.js +++ b/test/registrar/registrar.spec.js @@ -165,8 +165,8 @@ describe('registrar', () => { // Add protocol to peer and update it peerInfo.protocols.add(multicodec) - peerStore.addressBook.set(peerInfo.id, peerInfo.multiaddrs.toArray(), { replace: false }) - peerStore.protoBook.set(peerInfo.id, Array.from(peerInfo.protocols), { replace: false }) + peerStore.addressBook.add(peerInfo.id, peerInfo.multiaddrs.toArray()) + peerStore.protoBook.add(peerInfo.id, Array.from(peerInfo.protocols)) await onConnectDefer.promise