From 5758db8ea9c53fbbcbcd06c1e7fe0867c583714f Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Tue, 27 Oct 2020 15:58:18 +0000 Subject: [PATCH] chore: remove noAnnounce from address manager --- doc/API.md | 25 +----- doc/CONFIGURATION.md | 5 +- src/address-manager/index.js | 20 +---- src/index.js | 24 ++--- test/addresses/address-manager.spec.js | 23 +---- test/addresses/addresses.node.js | 119 ++++++++++--------------- 6 files changed, 61 insertions(+), 155 deletions(-) diff --git a/doc/API.md b/doc/API.md index ee14ee14..b199d556 100644 --- a/doc/API.md +++ b/doc/API.md @@ -13,8 +13,7 @@ * [`ping`](#ping) * [`multiaddrs`](#multiaddrs) * [`addressManager.getListenAddrs`](#addressmanagergetlistenaddrs) - * [`addressmger.getAnnounceAddrs`](#addressmanagergetannounceaddrs) - * [`addressManager.getNoAnnounceAddrs`](#addressmanagergetnoannounceaddrs) + * [`addressManager.getAnnounceAddrs`](#addressmanagergetannounceaddrs) * [`contentRouting.findProviders`](#contentroutingfindproviders) * [`contentRouting.provide`](#contentroutingprovide) * [`contentRouting.put`](#contentroutingput) @@ -91,7 +90,7 @@ Creates an instance of Libp2p. |------|------|-------------| | options | `object` | libp2p options | | options.modules | [`Array`](./CONFIGURATION.md#modules) | libp2p [modules](./CONFIGURATION.md#modules) to use | -| [options.addresses] | `{ listen: Array, announce: Array, noAnnounce: Array }` | Addresses for transport listening and to advertise to the network | +| [options.addresses] | `{ listen: Array, announce: Array, announceFilter: (ma: Array) => Array }` | Addresses for transport listening and to advertise to the network | | [options.config] | `object` | libp2p modules configuration and core configuration | | [options.host] | `{ agentVersion: string }` | libp2p host options | | [options.connectionManager] | [`object`](./CONFIGURATION.md#configuring-connection-manager) | libp2p Connection Manager [configuration](./CONFIGURATION.md#configuring-connection-manager) | @@ -515,26 +514,6 @@ const announceMa = libp2p.addressManager.getAnnounceAddrs() // [ ] ``` -### addressManager.getNoAnnounceAddrs - -Get the multiaddrs that were provided to not announce to the network. - -`libp2p.addressManager.getNoAnnounceAddrs()` - -#### Returns - -| Type | Description | -|------|-------------| -| `Array` | Provided noAnnounce multiaddrs | - -#### Example - -```js -// ... -const noAnnounceMa = libp2p.addressManager.getNoAnnounceAddrs() -// [ ] -``` - ### transportManager.getAddrs Get the multiaddrs that libp2p transports are using to listen on. diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index dcd277a2..1a39381a 100644 --- a/doc/CONFIGURATION.md +++ b/doc/CONFIGURATION.md @@ -211,11 +211,10 @@ Besides the `modules` and `config`, libp2p allows other internal options and con - This is used in modules such as the DHT. If it is not provided, `js-libp2p` will use an in memory datastore. - `peerId`: the identity of the node, an instance of [libp2p/js-peer-id](https://github.com/libp2p/js-peer-id). - This is particularly useful if you want to reuse the same `peer-id`, as well as for modules like `libp2p-delegated-content-routing`, which need a `peer-id` in their instantiation. -- `addresses`: an object containing `listen`, `announce` and `noAnnounce` properties with `Array`: +- `addresses`: an object containing `listen`, `announce` and `announceFilter`: - `listen` addresses will be provided to the libp2p underlying transports for listening on them. - `announce` addresses will be used to compute the advertises that the node should advertise to the network. - - `noAnnounce` addresses will be used as a filter to compute the advertises that the node should advertise to the network. - - `announceFilter`: filter function used to filter announced addresses programmatically: `(ma: Array) => Array`. Default: bypass all addresses. [`libp2p-utils`](https://github.com/libp2p/js-libp2p-utils) provides useful [multiaddr utilities](https://github.com/libp2p/js-libp2p-utils/blob/master/API.md#multiaddr-isloopbackma) to create your filters. + - `announceFilter`: filter function used to filter announced addresses programmatically: `(ma: Array) => Array`. Default: returns all addresses. [`libp2p-utils`](https://github.com/libp2p/js-libp2p-utils) provides useful [multiaddr utilities](https://github.com/libp2p/js-libp2p-utils/blob/master/API.md#multiaddr-isloopbackma) to create your filters. ### Examples diff --git a/src/address-manager/index.js b/src/address-manager/index.js index 0ba0df03..314f0a1a 100644 --- a/src/address-manager/index.js +++ b/src/address-manager/index.js @@ -7,11 +7,10 @@ log.error = debug('libp2p:addresses:error') const multiaddr = require('multiaddr') /** - * Responsible for managing this peers addresses. - * Peers can specify their listen, announce and noAnnounce addresses. + * Responsible for managing the peer addresses. + * Peers can specify their listen and announce addresses. * The listen addresses will be used by the libp2p transports to listen for new connections, - * while the announce an noAnnounce addresses will be combined with the listen addresses for - * address adverstising to other peers in the network. + * while the announce addresses will be used for the peer addresses' to other peers in the network. */ class AddressManager { /** @@ -19,12 +18,10 @@ class AddressManager { * @param {object} [options] * @param {Array} [options.listen = []] - list of multiaddrs string representation to listen. * @param {Array} [options.announce = []] - list of multiaddrs string representation to announce. - * @param {Array} [options.noAnnounce = []] - list of multiaddrs string representation to not announce. */ - constructor ({ listen = [], announce = [], noAnnounce = [] } = {}) { + constructor ({ listen = [], announce = [] } = {}) { this.listen = new Set(listen) this.announce = new Set(announce) - this.noAnnounce = new Set(noAnnounce) } /** @@ -44,15 +41,6 @@ class AddressManager { getAnnounceAddrs () { return Array.from(this.announce).map((a) => multiaddr(a)) } - - /** - * Get peer noAnnouncing multiaddrs. - * - * @returns {Array} - */ - getNoAnnounceAddrs () { - return Array.from(this.noAnnounce).map((a) => multiaddr(a)) - } } module.exports = AddressManager diff --git a/src/index.js b/src/index.js index 222577a2..e2950f1d 100644 --- a/src/index.js +++ b/src/index.js @@ -367,27 +367,15 @@ class Libp2p extends EventEmitter { * @returns {Array} */ get multiaddrs () { + const announceAddrs = this.addressManager.getAnnounceAddrs() + if (announceAddrs.length) { + return announceAddrs + } + const announceFilter = this._options.addresses.announceFilter || ((multiaddrs) => multiaddrs) - // Filter noAnnounce multiaddrs - const filterMa = this.addressManager.getNoAnnounceAddrs() - // Create advertising list - return announceFilter(this.transportManager.getAddrs() - .concat(this.addressManager.getAnnounceAddrs()) - .filter((ma, index, array) => { - // Filter out if repeated - if (array.findIndex((otherMa) => otherMa.equals(ma)) !== index) { - return false - } - - // Filter out if in noAnnounceMultiaddrs - if (filterMa.find((fm) => fm.equals(ma))) { - return false - } - - return true - })) + return announceFilter(this.transportManager.getAddrs()) } /** diff --git a/test/addresses/address-manager.spec.js b/test/addresses/address-manager.spec.js index c1d98e83..4d58387a 100644 --- a/test/addresses/address-manager.spec.js +++ b/test/addresses/address-manager.spec.js @@ -16,7 +16,6 @@ describe('Address Manager', () => { expect(am.listen.size).to.equal(0) expect(am.announce.size).to.equal(0) - expect(am.noAnnounce.size).to.equal(0) }) it('should return listen multiaddrs on get', () => { @@ -26,7 +25,6 @@ describe('Address Manager', () => { expect(am.listen.size).to.equal(listenAddresses.length) expect(am.announce.size).to.equal(0) - expect(am.noAnnounce.size).to.equal(0) const listenMultiaddrs = am.getListenAddrs() expect(listenMultiaddrs.length).to.equal(2) @@ -42,28 +40,11 @@ describe('Address Manager', () => { expect(am.listen.size).to.equal(listenAddresses.length) expect(am.announce.size).to.equal(announceAddreses.length) - expect(am.noAnnounce.size).to.equal(0) const announceMultiaddrs = am.getAnnounceAddrs() expect(announceMultiaddrs.length).to.equal(1) expect(announceMultiaddrs[0].equals(multiaddr(announceAddreses[0]))).to.equal(true) }) - - it('should return noAnnounce multiaddrs on get', () => { - const am = new AddressManager({ - listen: listenAddresses, - noAnnounce: listenAddresses - }) - - expect(am.listen.size).to.equal(listenAddresses.length) - expect(am.announce.size).to.equal(0) - expect(am.noAnnounce.size).to.equal(listenAddresses.length) - - const noAnnounceMultiaddrs = am.getNoAnnounceAddrs() - expect(noAnnounceMultiaddrs.length).to.equal(2) - expect(noAnnounceMultiaddrs[0].equals(multiaddr(listenAddresses[0]))).to.equal(true) - expect(noAnnounceMultiaddrs[1].equals(multiaddr(listenAddresses[1]))).to.equal(true) - }) }) describe('libp2p.addressManager', () => { @@ -76,14 +57,12 @@ describe('libp2p.addressManager', () => { config: { addresses: { listen: listenAddresses, - announce: announceAddreses, - noAnnounce: listenAddresses + announce: announceAddreses } } }) expect(libp2p.addressManager.listen.size).to.equal(listenAddresses.length) expect(libp2p.addressManager.announce.size).to.equal(announceAddreses.length) - expect(libp2p.addressManager.noAnnounce.size).to.equal(listenAddresses.length) }) }) diff --git a/test/addresses/addresses.node.js b/test/addresses/addresses.node.js index 3797f2d5..a45bad00 100644 --- a/test/addresses/addresses.node.js +++ b/test/addresses/addresses.node.js @@ -4,6 +4,7 @@ const { expect } = require('aegir/utils/chai') const sinon = require('sinon') +const multiaddr = require('multiaddr') const isLoopback = require('libp2p-utils/src/multiaddr/is-loopback') const { AddressesOptions } = require('./utils') @@ -44,8 +45,33 @@ describe('libp2p.multiaddrs', () => { expect(listenAddrs.has(listenAddresses[1])).to.equal(true) }) - it('should advertise all addresses if noAnnounce addresses are not provided, but with correct ports', async () => { + it('should announce transport listen addresses if announce addresses are not provided', async () => { [libp2p] = await peerUtils.createPeer({ + started: false, + config: { + ...AddressesOptions, + addresses: { + listen: listenAddresses + } + } + }) + + await libp2p.start() + + const tmListen = libp2p.transportManager.getAddrs().map((ma) => ma.toString()) + + // Announce 2 listen (transport) + const advertiseMultiaddrs = libp2p.multiaddrs.map((ma) => ma.toString()) + expect(advertiseMultiaddrs.length).to.equal(2) + tmListen.forEach((m) => { + expect(advertiseMultiaddrs).to.include(m) + }) + expect(advertiseMultiaddrs).to.not.include(listenAddresses[0]) // Random Port switch + }) + + it('should only announce the given announce addresses when provided', async () => { + [libp2p] = await peerUtils.createPeer({ + started: false, config: { ...AddressesOptions, addresses: { @@ -55,97 +81,44 @@ describe('libp2p.multiaddrs', () => { } }) + await libp2p.start() + const tmListen = libp2p.transportManager.getAddrs().map((ma) => ma.toString()) - const spyAnnounce = sinon.spy(libp2p.addressManager, 'getAnnounceAddrs') - const spyNoAnnounce = sinon.spy(libp2p.addressManager, 'getNoAnnounceAddrs') - const spyListen = sinon.spy(libp2p.addressManager, 'getListenAddrs') - const spyTranspMgr = sinon.spy(libp2p.transportManager, 'getAddrs') - + // Announce 1 announce addr const advertiseMultiaddrs = libp2p.multiaddrs.map((ma) => ma.toString()) - - expect(spyAnnounce).to.have.property('callCount', 1) - expect(spyNoAnnounce).to.have.property('callCount', 1) - expect(spyListen).to.have.property('callCount', 0) // Listen addr should not be used - expect(spyTranspMgr).to.have.property('callCount', 1) - - // Announce 2 listen (transport) + 1 announce - expect(advertiseMultiaddrs.length).to.equal(3) - tmListen.forEach((m) => { - expect(advertiseMultiaddrs).to.include(m) - }) - announceAddreses.forEach((m) => { - expect(advertiseMultiaddrs).to.include(m) - }) - expect(advertiseMultiaddrs).to.not.include(listenAddresses[0]) // Random Port switch - }) - - it('should remove replicated addresses', async () => { - [libp2p] = await peerUtils.createPeer({ - config: { - ...AddressesOptions, - addresses: { - listen: listenAddresses, - announce: [listenAddresses[1]] - } - } - }) - - const advertiseMultiaddrs = libp2p.multiaddrs.map((ma) => ma.toString()) - - // Announce 2 listen (transport), ignoring duplicated in announce - expect(advertiseMultiaddrs.length).to.equal(2) - }) - - it('should not advertise noAnnounce addresses', async () => { - const noAnnounce = [listenAddresses[1]] - ;[libp2p] = await peerUtils.createPeer({ - config: { - ...AddressesOptions, - addresses: { - listen: listenAddresses, - announce: announceAddreses, - noAnnounce - } - } - }) - - const advertiseMultiaddrs = libp2p.multiaddrs.map((ma) => ma.toString()) - - // Announce 1 listen (transport) not in the noAnnounce and the announce - expect(advertiseMultiaddrs.length).to.equal(2) - - announceAddreses.forEach((m) => { - expect(advertiseMultiaddrs).to.include(m) - }) - noAnnounce.forEach((m) => { - expect(advertiseMultiaddrs).to.not.include(m) + expect(advertiseMultiaddrs.length).to.equal(announceAddreses.length) + advertiseMultiaddrs.forEach((m) => { + expect(tmListen).to.not.include(m) + expect(announceAddreses).to.include(m) }) }) - it('can filter out loopback addresses to announced by the announce filter', async () => { + it('can filter out loopback addresses by the announce filter', async () => { [libp2p] = await peerUtils.createPeer({ started: false, config: { ...AddressesOptions, addresses: { listen: listenAddresses, - announce: announceAddreses, announceFilter: (multiaddrs) => multiaddrs.filter(m => !isLoopback(m)) } } }) - const listenAddrs = libp2p.addressManager.listen - expect(listenAddrs.size).to.equal(listenAddresses.length) - expect(listenAddrs.has(listenAddresses[0])).to.equal(true) - expect(listenAddrs.has(listenAddresses[1])).to.equal(true) - await libp2p.start() + expect(libp2p.multiaddrs.length).to.equal(0) + + // Stub transportManager addresses to add a public address + const stubMa = multiaddr('/ip4/120.220.10.1/tcp/1000') + sinon.stub(libp2p.transportManager, 'getAddrs').returns([ + ...listenAddresses.map((a) => multiaddr(a)), + stubMa + ]) + const multiaddrs = libp2p.multiaddrs - expect(multiaddrs.length).to.equal(announceAddreses.length) - expect(multiaddrs.includes(listenAddresses[0])).to.equal(false) - expect(multiaddrs.includes(listenAddresses[1])).to.equal(false) + expect(multiaddrs.length).to.equal(1) + expect(multiaddrs[0].equals(stubMa)).to.eql(true) }) })