chore: remove extra defaults

Require at least four peers to report the same observed address before
we start returning it as part of `node.multiaddrs`.

In order to not let the list of observed addresses grow forever we
enforce a 10 minute timeout, that is for any observed address, at least
four different peers have to report it to us within ten minutes otherwise
we discard the address.

The number of peers and timeout are both configurable.

.aegir.js

@@ -48,7 +48,7 @@ const after = async () => {
 }
 
 module.exports = {
-  bundlesize: { maxSize: '215kB' },
+  bundlesize: { maxSize: '216kB' },
   hooks: {
     pre: before,
     post: after

doc/CONFIGURATION.md

@@ -23,6 +23,7 @@
       - [Setup with Auto Relay](#setup-with-auto-relay)
       - [Setup with Keychain](#setup-with-keychain)
       - [Configuring Dialing](#configuring-dialing)
+      - [Configuring Address Manager](#configuring-address-manager)
       - [Configuring Connection Manager](#configuring-connection-manager)
       - [Configuring Transport Manager](#configuring-transport-manager)
       - [Configuring Metrics](#configuring-metrics)
@@ -549,6 +550,26 @@ const node = await Libp2p.create({
   }
 ```
 
+#### Configuring Address Manager
+
+The address manager receives observed addresses from network peers. We accept observed addresses once a certain number of peers have reported the same observed address within a certain window of time.
+
+```js
+const node = await Libp2p.create({
+  addressManager: {
+    observedAddresses: {
+      // we must receive the same observed address from this many
+      // peers before we start believe it
+      minConfidence: 4,
+      // an address must reach the minimum level of confidence within
+      // this timeout otherwise it will be ignored
+      maxLifetimeBeforeEviction: (60 * 10) * 1000 // ten minutes in ms
+    }
+  },
+  // ...other options
+})
+```
+
 #### Configuring Connection Manager
 
 The Connection Manager prunes Connections in libp2p whenever certain limits are exceeded. If Metrics are enabled, you can also configure the Connection Manager to monitor the bandwidth of libp2p and prune connections as needed. You can read more about what Connection Manager does at [./CONNECTION_MANAGER.md](./CONNECTION_MANAGER.md). The configuration values below show the defaults for Connection Manager. See [./CONNECTION_MANAGER.md](./CONNECTION_MANAGER.md#options) for a full description of the parameters.

src/address-manager/index.js

@@ -31,14 +31,18 @@ class AddressManager extends EventEmitter {
    * @param {object} [options]
    * @param {Array<string>} [options.listen = []] - list of multiaddrs string representation to listen.
    * @param {Array<string>} [options.announce = []] - list of multiaddrs string representation to announce.
+   * @param {object} [options.observedAddresses = { minConfidence: 4, maxLifetimeBeforeEviction: 600000 }] - configuration options for observed addresses
    */
-  constructor (peerId, { listen = [], announce = [] } = {}) {
+  constructor (peerId, { listen = [], announce = [], observedAddresses = { minConfidence: 4, maxLifetimeBeforeEviction: (60 * 10) * 1000 } } = {}) {
     super()
 
     this.peerId = peerId
     this.listen = new Set(listen.map(ma => ma.toString()))
     this.announce = new Set(announce.map(ma => ma.toString()))
-    this.observed = new Set()
+    this.observed = new Map()
+    this.config = {
+      observedAddresses
+    }
   }
 
   /**
@@ -65,15 +69,25 @@ class AddressManager extends EventEmitter {
    * @returns {Array<Multiaddr>}
    */
   getObservedAddrs () {
-    return Array.from(this.observed).map((a) => multiaddr(a))
+    const output = []
+
+    this.observed.forEach(({ confidence }, addr) => {
+      if (confidence >= this.config.observedAddresses.minConfidence) {
+        output.push(multiaddr(addr))
+      }
+    })
+
+    return output
   }
 
   /**
    * Add peer observed addresses
    *
    * @param {string | Multiaddr} addr
+   * @param {PeerId} reporter
+   * @param {number} [confidence=1]
    */
-  addObservedAddr (addr) {
+  addObservedAddr (addr, reporter, confidence = 1) {
     let ma = multiaddr(addr)
     const remotePeer = ma.getPeerId()
 
@@ -87,15 +101,41 @@ class AddressManager extends EventEmitter {
       }
     }
 
+    const now = Date.now()
     const addrString = ma.toString()
-
-    // do not trigger the change:addresses event if we already know about this address
-    if (this.observed.has(addrString)) {
-      return
+    const wasNewAddr = !this.observed.has(addrString)
+    let addrRecord = {
+      confidence,
+      reporters: [
+        reporter.toB58String()
+      ],
+      expires: now + this.config.observedAddresses.maxLifetimeBeforeEviction
     }
 
-    this.observed.add(addrString)
-    this.emit('change:addresses')
+    // we've seen this address before, increase the confidence we have in it
+    if (!wasNewAddr) {
+      addrRecord = this.observed.get(addrString)
+
+      if (!addrRecord.reporters.includes(reporter.toB58String())) {
+        addrRecord.confidence++
+        addrRecord.reporters.push(reporter.toB58String())
+        addrRecord.expires = now + this.config.observedAddresses.maxLifetimeBeforeEviction
+      }
+    }
+
+    this.observed.set(addrString, addrRecord)
+
+    // only emit event if we've reached the minimum confidence
+    if (addrRecord.confidence === this.config.observedAddresses.minConfidence) {
+      this.emit('change:addresses')
+    }
+
+    // evict addresses older than MAX_LOW_CONFIDENCE_ADDR_LIFETIME_MS we are not confident in
+    this.observed.forEach(({ confidence, expires }, key, map) => {
+      if (confidence < this.config.observedAddresses.minConfidence && expires < now) {
+        map.delete(key)
+      }
+    })
   }
 }
 

src/config.js

@@ -16,6 +16,12 @@ const DefaultConfig = {
     announce: [],
     noAnnounce: []
   },
+  addressManager: {
+    observedAddresses: {
+      minConfidence: 4,
+      maxLifetimeBeforeEviction: (60 * 10) * 1000
+    }
+  },
   connectionManager: {
     minConnections: 25
   },

src/identify/index.js

@@ -202,9 +202,8 @@ class IdentifyService {
     this.peerStore.protoBook.set(id, protocols)
     this.peerStore.metadataBook.set(id, 'AgentVersion', uint8ArrayFromString(message.agentVersion))
 
-    // TODO: Score our observed addr
     log('received observed address of %s', observedAddr)
-    this.addressManager.addObservedAddr(observedAddr)
+    this.addressManager.addObservedAddr(observedAddr, id)
   }
 
   /**

src/index.js

@@ -137,7 +137,10 @@ class Libp2p extends EventEmitter {
 
     // Addresses {listen, announce, noAnnounce}
     this.addresses = this._options.addresses
-    this.addressManager = new AddressManager(this.peerId, this._options.addresses)
+    this.addressManager = new AddressManager(this.peerId, {
+      ...this._options.addresses,
+      ...this._options.addressManager
+    })
 
     // when addresses change, update our peer record
     this.addressManager.on('change:addresses', () => {

src/nat-manager.js

@@ -16,11 +16,11 @@ const {
   codes: { ERR_INVALID_PARAMETERS }
 } = require('./errors')
 const isLoopback = require('libp2p-utils/src/multiaddr/is-loopback')
+const AddressManager = require('./address-manager')
 
 /**
  * @typedef {import('peer-id')} PeerId
  * @typedef {import('./transport-manager')} TransportManager
- * @typedef {import('./address-manager')} AddressManager
  */
 
 function highPort (min = 1024, max = 65535) {
@@ -118,11 +118,12 @@ class NatManager {
         protocol: transport.toUpperCase()
       })
 
+      // add with high confidence
       this._addressManager.addObservedAddr(Multiaddr.fromNodeAddress({
         family: 'IPv4',
         address: publicIp,
         port: `${publicPort}`
-      }, transport))
+      }, transport), this._peerId, this._addressManager.config.observedAddresses.minConfidence)
     }
   }
 

test/addresses/address-manager.spec.js

@@ -15,9 +15,11 @@ const announceAddreses = ['/dns4/peer.io']
 
 describe('Address Manager', () => {
   let peerId
+  let peerIds
 
   before(async () => {
     peerId = await PeerId.createFromJSON(Peers[0])
+    peerIds = await Promise.all(Peers.slice(1).map(peerId => PeerId.createFromJSON(peerId)))
   })
 
   it('should not need any addresses', () => {
@@ -60,7 +62,7 @@ describe('Address Manager', () => {
 
     expect(am.observed).to.be.empty()
 
-    am.addObservedAddr('/ip4/123.123.123.123/tcp/39201')
+    am.addObservedAddr('/ip4/123.123.123.123/tcp/39201', peerId)
 
     expect(am.observed).to.have.property('size', 1)
   })
@@ -71,12 +73,12 @@ describe('Address Manager', () => {
 
     expect(am.observed).to.be.empty()
 
-    am.addObservedAddr(ma)
-    am.addObservedAddr(ma)
-    am.addObservedAddr(ma)
+    am.addObservedAddr(ma, peerId)
+    am.addObservedAddr(ma, peerId)
+    am.addObservedAddr(ma, peerId)
 
     expect(am.observed).to.have.property('size', 1)
-    expect(am.observed).to.include(ma)
+    expect(Array.from(am.observed.keys())).to.include(ma)
   })
 
   it('should only emit one change:addresses event', () => {
@@ -88,11 +90,25 @@ describe('Address Manager', () => {
       eventCount++
     })
 
-    am.addObservedAddr(ma)
-    am.addObservedAddr(ma)
-    am.addObservedAddr(ma)
-    am.addObservedAddr(`${ma}/p2p/${peerId}`)
-    am.addObservedAddr(`${ma}/p2p/${peerId.toB58String()}`)
+    am.addObservedAddr(ma, peerIds[0])
+    am.addObservedAddr(ma, peerIds[1])
+    am.addObservedAddr(ma, peerIds[2])
+    am.addObservedAddr(`${ma}/p2p/${peerId}`, peerIds[3])
+    am.addObservedAddr(`${ma}/p2p/${peerId.toB58String()}`, peerIds[4])
+
+    expect(eventCount).to.equal(1)
+  })
+
+  it('should emit one change:addresses event when specifying confidence', () => {
+    const ma = '/ip4/123.123.123.123/tcp/39201'
+    const am = new AddressManager(peerId)
+    let eventCount = 0
+
+    am.on('change:addresses', () => {
+      eventCount++
+    })
+
+    am.addObservedAddr(ma, peerId, am.config.observedAddresses.minConfidence)
 
     expect(eventCount).to.equal(1)
   })
@@ -103,11 +119,12 @@ describe('Address Manager', () => {
 
     expect(am.observed).to.be.empty()
 
-    am.addObservedAddr(ma)
-    am.addObservedAddr(`${ma}/p2p/${peerId}`)
+    am.addObservedAddr(ma, peerId)
+    am.addObservedAddr(`${ma}/p2p/${peerId}`, peerId)
 
     expect(am.observed).to.have.property('size', 1)
-    expect(am.observed).to.include(ma)
+
+    expect(Array.from(am.observed.keys())).to.include(ma)
   })
 
   it('should strip our peer address from added observed addresses in difference formats', () => {
@@ -116,12 +133,71 @@ describe('Address Manager', () => {
 
     expect(am.observed).to.be.empty()
 
-    am.addObservedAddr(ma)
-    am.addObservedAddr(`${ma}/p2p/${peerId}`) // base32 CID
-    am.addObservedAddr(`${ma}/p2p/${peerId.toB58String()}`) // base58btc
+    am.addObservedAddr(ma, peerId)
+    am.addObservedAddr(`${ma}/p2p/${peerId}`, peerId) // base32 CID
+    am.addObservedAddr(`${ma}/p2p/${peerId.toB58String()}`, peerId) // base58btc
 
     expect(am.observed).to.have.property('size', 1)
-    expect(am.observed).to.include(ma)
+
+    expect(Array.from(am.observed.keys())).to.include(ma)
+  })
+
+  it('should require a number of confirmations before believing address', () => {
+    const ma = '/ip4/123.123.123.123/tcp/39201'
+    const am = new AddressManager(peerId)
+
+    expect(am.observed).to.be.empty()
+
+    am.addObservedAddr(ma, peerId)
+
+    expect(am.getObservedAddrs().map(ma => ma.toString())).to.not.include(ma)
+
+    for (let i = 0; i < am.config.observedAddresses.minConfidence; i++) {
+      am.addObservedAddr(ma, peerIds[i])
+    }
+
+    expect(am.getObservedAddrs().map(ma => ma.toString())).to.include(ma)
+  })
+
+  it('should require a number of confirmations from different peers', () => {
+    const ma = '/ip4/123.123.123.123/tcp/39201'
+    const am = new AddressManager(peerId)
+
+    expect(am.observed).to.be.empty()
+
+    am.addObservedAddr(ma, peerId)
+
+    expect(am.getObservedAddrs().map(ma => ma.toString())).to.not.include(ma)
+
+    for (let i = 0; i < am.config.observedAddresses.minConfidence; i++) {
+      am.addObservedAddr(ma, peerIds[0])
+    }
+
+    expect(am.getObservedAddrs().map(ma => ma.toString())).to.not.include(ma)
+  })
+
+  it('should evict addresses that do not receive enough confirmations within the timeout', () => {
+    const ma1 = '/ip4/123.123.123.123/tcp/39201'
+    const ma2 = '/ip4/124.124.124.124/tcp/39202'
+    const am = new AddressManager(peerId)
+
+    expect(am.observed).to.be.empty()
+
+    am.addObservedAddr(ma1, peerId)
+
+    const observedAddrs = Array.from(am.observed.values())
+
+    expect(Array.from(am.observed.keys())).to.include(ma1)
+
+    // make expiry date a while ago
+    observedAddrs[0].expires = Date.now() - 1000
+
+    // will evict any old multiaddrs
+    am.addObservedAddr(ma2, peerId)
+
+    // should have been evicted
+    expect(Array.from(am.observed.keys())).to.not.include(ma1)
+    expect(Array.from(am.observed.keys())).to.include(ma2)
   })
 })
 

test/addresses/addresses.node.js

@@ -164,7 +164,7 @@ describe('libp2p.multiaddrs', () => {
 
     expect(libp2p.multiaddrs).to.have.lengthOf(listenAddresses.length)
 
-    libp2p.addressManager.addObservedAddr(ma)
+    libp2p.addressManager.addObservedAddr(ma, libp2p.peerId, libp2p.addressManager.config.observedAddresses.minConfidence)
 
     expect(libp2p.multiaddrs).to.have.lengthOf(listenAddresses.length + 1)
     expect(libp2p.multiaddrs.map(ma => ma.toString())).to.include(ma)

test/core/consume-peer-record.spec.js

@@ -37,7 +37,7 @@ describe('Consume peer record', () => {
       done = resolve
     })
 
-    libp2p.addressManager.addObservedAddr('/ip4/123.123.123.123/tcp/3983')
+    libp2p.addressManager.addObservedAddr('/ip4/123.123.123.123/tcp/3983', libp2p.peerId, libp2p.addressManager.config.observedAddresses.minConfidence)
 
     await p