fix: use toB58String everywhere to be consistent (#537)

* chore: update deps

* fix: consistently use b58 peerid string

The migration to base32 will happen at a later date

package.json

@@ -57,7 +57,7 @@
     "it-protocol-buffers": "^0.2.0",
     "latency-monitor": "~0.2.1",
     "libp2p-crypto": "^0.17.1",
-    "libp2p-interfaces": "^0.1.5",
+    "libp2p-interfaces": "^0.2.3",
     "mafmt": "^7.0.0",
     "merge-options": "^2.0.0",
     "moving-average": "^1.0.0",
@@ -77,7 +77,7 @@
   "devDependencies": {
     "@nodeutils/defaults-deep": "^1.1.0",
     "abortable-iterator": "^2.1.0",
-    "aegir": "^20.4.1",
+    "aegir": "^20.5.1",
     "chai": "^4.2.0",
     "chai-as-promised": "^7.1.1",
     "cids": "^0.7.1",
@@ -102,7 +102,7 @@
     "p-defer": "^3.0.0",
     "p-times": "^2.1.0",
     "p-wait-for": "^3.1.0",
-    "sinon": "^7.2.7",
+    "sinon": "^8.1.0",
     "streaming-iterables": "^4.1.0",
     "wrtc": "^0.4.1"
   },

src/connection-manager/index.js

@@ -36,7 +36,7 @@ class ConnectionManager {
   constructor (libp2p, options) {
     this._libp2p = libp2p
     this._registrar = libp2p.registrar
-    this._peerId = libp2p.peerInfo.id.toString()
+    this._peerId = libp2p.peerInfo.id.toB58String()
     this._options = mergeOptions.call({ ignoreUndefined: true }, defaultOptions, options)
     assert(
       this._options.maxConnections > this._options.minConnections,
@@ -91,8 +91,8 @@ class ConnectionManager {
     if (value < 0 || value > 1) {
       throw new Error('value should be a number between 0 and 1')
     }
-    if (peerId.toString) {
+    if (peerId.toB58String) {
-      peerId = peerId.toString()
+      peerId = peerId.toB58String()
     }
     this._peerValues.set(peerId, value)
   }
@@ -119,7 +119,7 @@ class ConnectionManager {
    * @param {Connection} connection
    */
   onConnect (connection) {
-    const peerId = connection.remotePeer.toString()
+    const peerId = connection.remotePeer.toB58String()
     this._connections.set(connection.id, connection)
     if (!this._peerValues.has(peerId)) {
       this._peerValues.set(peerId, this._options.defaultPeerValue)
@@ -133,7 +133,7 @@ class ConnectionManager {
    */
   onDisconnect (connection) {
     this._connections.delete(connection.id)
-    this._peerValues.delete(connection.remotePeer.toString())
+    this._peerValues.delete(connection.remotePeer.toB58String())
   }
 
   /**
@@ -175,7 +175,7 @@ class ConnectionManager {
         debug('%s: lowest value peer is %s', this._peerId, peerId)
         debug('%s: closing a connection to %j', this._peerId, peerId)
         for (const connection of this._connections.values()) {
-          if (connection.remotePeer.toString() === peerId) {
+          if (connection.remotePeer.toB58String() === peerId) {
             connection.close()
             break
           }

src/dialer/index.js

@@ -113,7 +113,7 @@ class Dialer {
     }
     const addrs = this.peerStore.multiaddrsForPeer(dialable)
     return {
-      id: dialable.id.toString(),
+      id: dialable.id.toB58String(),
       addrs
     }
   }

src/identify/index.js

@@ -180,7 +180,7 @@ class IdentifyService {
 
     const id = await PeerId.createFromPubKey(publicKey)
     const peerInfo = new PeerInfo(id)
-    if (connection.remotePeer.toString() !== id.toString()) {
+    if (connection.remotePeer.toB58String() !== id.toB58String()) {
       throw errCode(new Error('identified peer does not match the expected peer'), codes.ERR_INVALID_PEER)
     }
 

src/index.js

@@ -314,7 +314,7 @@ class Libp2p extends EventEmitter {
   hangUp (peer) {
     const peerInfo = getPeerInfo(peer, this.peerStore)
     return Promise.all(
-      this.registrar.connections.get(peerInfo.id.toString()).map(connection => {
+      this.registrar.connections.get(peerInfo.id.toB58String()).map(connection => {
         return connection.close()
       })
     )
@@ -426,7 +426,7 @@ class Libp2p extends EventEmitter {
    * @param {PeerInfo} peerInfo
    */
   _onDiscoveryPeer (peerInfo) {
-    if (peerInfo.id.toString() === this.peerInfo.id.toString()) {
+    if (peerInfo.id.toB58String() === this.peerInfo.id.toB58String()) {
       log.error(new Error(codes.ERR_DISCOVERED_SELF))
       return
     }
@@ -445,7 +445,7 @@ class Libp2p extends EventEmitter {
     if (this._config.peerDiscovery.autoDial === true && !this.registrar.getConnection(peerInfo)) {
       const minPeers = this._options.connectionManager.minPeers || 0
       if (minPeers > this.connectionManager._connections.size) {
-        log('connecting to discovered peer %s', peerInfo.id.toString())
+        log('connecting to discovered peer %s', peerInfo.id.toB58String())
         try {
           await this.dialer.connectToPeer(peerInfo)
         } catch (err) {

src/metrics/index.js

@@ -82,7 +82,7 @@ class Metrics {
    * @returns {Stats}
    */
   forPeer (peerId) {
-    const idString = peerId.toString()
+    const idString = peerId.toB58String()
     return this._peerStats.get(idString) || this._oldPeers.get(idString)
   }
 
@@ -110,7 +110,7 @@ class Metrics {
    * @param {PeerId} peerId
    */
   onPeerDisconnected (peerId) {
-    const idString = peerId.toString()
+    const idString = peerId.toB58String()
     const peerStats = this._peerStats.get(idString)
     if (peerStats) {
       peerStats.stop()
@@ -140,7 +140,7 @@ class Metrics {
     let peerStats = this.forPeer(remotePeer)
     if (!peerStats) {
       peerStats = new Stats(initialCounters, this._options)
-      this._peerStats.set(remotePeer.toString(), peerStats)
+      this._peerStats.set(remotePeer.toB58String(), peerStats)
     }
 
     // Peer and global stats
@@ -162,13 +162,13 @@ class Metrics {
    * Replaces the `PeerId` string with the given `peerId`.
    * If stats are already being tracked for the given `peerId`, the
    * placeholder stats will be merged with the existing stats.
-   * @param {string} placeholder A peerId string
+   * @param {PeerId} placeholder A peerId string
    * @param {PeerId} peerId
    */
   updatePlaceholder (placeholder, peerId) {
     if (!this._running) return
     const placeholderStats = this.forPeer(placeholder)
-    const peerIdString = peerId.toString()
+    const peerIdString = peerId.toB58String()
     const existingStats = this.forPeer(peerId)
     let mergedStats = placeholderStats
 
@@ -180,7 +180,7 @@ class Metrics {
       this._oldPeers.delete(peerIdString)
     }
 
-    this._peerStats.delete(placeholder.toString())
+    this._peerStats.delete(placeholder.toB58String())
     this._peerStats.set(peerIdString, mergedStats)
     mergedStats.start()
   }

src/registrar.js

@@ -74,7 +74,7 @@ class Registrar {
     assert(PeerInfo.isPeerInfo(peerInfo), 'peerInfo must be an instance of peer-info')
     assert(Connection.isConnection(conn), 'conn must be an instance of interface-connection')
 
-    const id = peerInfo.id.toString()
+    const id = peerInfo.id.toB58String()
     const storedConn = this.connections.get(id)
 
     if (storedConn) {
@@ -95,7 +95,7 @@ class Registrar {
   onDisconnect (peerInfo, connection, error) {
     assert(PeerInfo.isPeerInfo(peerInfo), 'peerInfo must be an instance of peer-info')
 
-    const id = peerInfo.id.toString()
+    const id = peerInfo.id.toB58String()
     let storedConn = this.connections.get(id)
 
     if (storedConn && storedConn.length > 1) {
@@ -106,7 +106,7 @@ class Registrar {
         topology.disconnect(peerInfo, error)
       }
 
-      this.connections.delete(peerInfo.id.toString())
+      this.connections.delete(peerInfo.id.toB58String())
     }
   }
 
@@ -118,7 +118,7 @@ class Registrar {
   getConnection (peerInfo) {
     assert(PeerInfo.isPeerInfo(peerInfo), 'peerInfo must be an instance of peer-info')
 
-    const connections = this.connections.get(peerInfo.id.toString())
+    const connections = this.connections.get(peerInfo.id.toB58String())
     // Return the first, open connection
     if (connections) {
       return connections.find(connection => connection.stat.status === 'open')

src/upgrader.js

@@ -73,7 +73,7 @@ class Upgrader {
     if (this.metrics) {
       ({ setTarget: setPeer, proxy: proxyPeer } = mutableProxy())
       const idString = (parseInt(Math.random() * 1e9)).toString(36) + Date.now()
-      setPeer({ toString: () => idString })
+      setPeer({ toB58String: () => idString })
       maConn = this.metrics.trackStream({ stream: maConn, remotePeer: proxyPeer })
     }
 
@@ -147,7 +147,7 @@ class Upgrader {
     if (this.metrics) {
       ({ setTarget: setPeer, proxy: proxyPeer } = mutableProxy())
       const idString = (parseInt(Math.random() * 1e9)).toString(36) + Date.now()
-      setPeer({ toString: () => idString })
+      setPeer({ toB58String: () => idString })
       maConn = this.metrics.trackStream({ stream: maConn, remotePeer: proxyPeer })
     }
 

test/connection-manager/index.spec.js

@@ -75,7 +75,7 @@ describe('Connection Manager', () => {
       const spy = sinon.spy(connection, 'close')
       // The connections have the same remote id, give them random ones
       // so that we can verify the correct connection was closed
-      sinon.stub(connection.remotePeer, 'toString').returns(index)
+      sinon.stub(connection.remotePeer, 'toB58String').returns(index)
       const value = Math.random()
       spies.set(value, spy)
       libp2p.connectionManager.setPeerValue(connection.remotePeer, value)

test/dialing/direct.node.js

@@ -306,7 +306,7 @@ describe('Dialing (direct, TCP)', () => {
         }
       })
 
-      const connection = await libp2p.dial(`${remoteAddr.toString()}/p2p/${remotePeerInfo.id.toString()}`)
+      const connection = await libp2p.dial(`${remoteAddr.toString()}/p2p/${remotePeerInfo.id.toB58String()}`)
       expect(connection).to.exist()
       expect(connection.stat.timeline.close).to.not.exist()
       await libp2p.hangUp(connection.remotePeer)
@@ -375,7 +375,7 @@ describe('Dialing (direct, TCP)', () => {
       })
       const dials = 10
 
-      const fullAddress = remoteAddr.encapsulate(`/p2p/${remoteLibp2p.peerInfo.id.toString()}`)
+      const fullAddress = remoteAddr.encapsulate(`/p2p/${remoteLibp2p.peerInfo.id.toB58String()}`)
       const dialResults = await Promise.all([...new Array(dials)].map((_, index) => {
         if (index % 2 === 0) return libp2p.dial(remoteLibp2p.peerInfo)
         return libp2p.dial(fullAddress)
@@ -405,7 +405,7 @@ describe('Dialing (direct, TCP)', () => {
       const error = new Error('Boom')
       sinon.stub(libp2p.transportManager, 'dial').callsFake(() => Promise.reject(error))
 
-      const fullAddress = remoteAddr.encapsulate(`/p2p/${remoteLibp2p.peerInfo.id.toString()}`)
+      const fullAddress = remoteAddr.encapsulate(`/p2p/${remoteLibp2p.peerInfo.id.toB58String()}`)
       const dialResults = await pSettle([...new Array(dials)].map((_, index) => {
         if (index % 2 === 0) return libp2p.dial(remoteLibp2p.peerInfo)
         return libp2p.dial(fullAddress)

test/dialing/relay.node.js

@@ -59,11 +59,11 @@ describe('Dialing (via relay, TCP)', () => {
 
   it('should be able to connect to a peer over a relay with active connections', async () => {
     const relayAddr = relayLibp2p.transportManager.getAddrs()[0]
-    const relayIdString = relayLibp2p.peerInfo.id.toString()
+    const relayIdString = relayLibp2p.peerInfo.id.toB58String()
 
     const dialAddr = relayAddr
       .encapsulate(`/p2p/${relayIdString}`)
-      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toString()}`)
+      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toB58String()}`)
 
     const tcpAddrs = dstLibp2p.transportManager.getAddrs()
     await dstLibp2p.transportManager.listen([multiaddr(`/p2p-circuit${relayAddr}/p2p/${relayIdString}`)])
@@ -94,11 +94,11 @@ describe('Dialing (via relay, TCP)', () => {
 
   it('should fail to connect to a peer over a relay with inactive connections', async () => {
     const relayAddr = relayLibp2p.transportManager.getAddrs()[0]
-    const relayIdString = relayLibp2p.peerInfo.id.toString()
+    const relayIdString = relayLibp2p.peerInfo.id.toB58String()
 
     const dialAddr = relayAddr
       .encapsulate(`/p2p/${relayIdString}`)
-      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toString()}`)
+      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toB58String()}`)
 
     await expect(srcLibp2p.dial(dialAddr))
       .to.eventually.be.rejectedWith(AggregateError)
@@ -107,11 +107,11 @@ describe('Dialing (via relay, TCP)', () => {
 
   it('should not stay connected to a relay when not already connected and HOP fails', async () => {
     const relayAddr = relayLibp2p.transportManager.getAddrs()[0]
-    const relayIdString = relayLibp2p.peerInfo.id.toString()
+    const relayIdString = relayLibp2p.peerInfo.id.toB58String()
 
     const dialAddr = relayAddr
       .encapsulate(`/p2p/${relayIdString}`)
-      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toString()}`)
+      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toB58String()}`)
 
     await expect(srcLibp2p.dial(dialAddr))
       .to.eventually.be.rejectedWith(AggregateError)
@@ -124,11 +124,11 @@ describe('Dialing (via relay, TCP)', () => {
 
   it('dialer should stay connected to an already connected relay on hop failure', async () => {
     const relayAddr = relayLibp2p.transportManager.getAddrs()[0]
-    const relayIdString = relayLibp2p.peerInfo.id.toString()
+    const relayIdString = relayLibp2p.peerInfo.id.toB58String()
 
     const dialAddr = relayAddr
       .encapsulate(`/p2p/${relayIdString}`)
-      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toString()}`)
+      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toB58String()}`)
 
     await srcLibp2p.dial(relayAddr)
 
@@ -143,11 +143,11 @@ describe('Dialing (via relay, TCP)', () => {
 
   it('destination peer should stay connected to an already connected relay on hop failure', async () => {
     const relayAddr = relayLibp2p.transportManager.getAddrs()[0]
-    const relayIdString = relayLibp2p.peerInfo.id.toString()
+    const relayIdString = relayLibp2p.peerInfo.id.toB58String()
 
     const dialAddr = relayAddr
       .encapsulate(`/p2p/${relayIdString}`)
-      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toString()}`)
+      .encapsulate(`/p2p-circuit/p2p/${dstLibp2p.peerInfo.id.toB58String()}`)
 
     // Connect the destination peer and the relay
     const tcpAddrs = dstLibp2p.transportManager.getAddrs()

test/identify/index.spec.js

@@ -84,7 +84,12 @@ describe('Identify', () => {
   it('should throw if identified peer is the wrong peer', async () => {
     const localIdentify = new IdentifyService({
       peerInfo: localPeer,
-      protocols
+      protocols,
+      registrar: {
+        peerStore: {
+          replace: () => {}
+        }
+      }
     })
     const remoteIdentify = new IdentifyService({
       peerInfo: remotePeer,
@@ -92,7 +97,7 @@ describe('Identify', () => {
     })
 
     const observedAddr = multiaddr('/ip4/127.0.0.1/tcp/1234')
-    const localConnectionMock = { newStream: () => {}, remotePeer }
+    const localConnectionMock = { newStream: () => {}, remotePeer: localPeer.id }
     const remoteConnectionMock = { remoteAddr: observedAddr }
 
     const [local, remote] = duplexPair()

test/metrics/index.spec.js

@@ -104,7 +104,7 @@ describe('Metrics', () => {
     expect(results.length).to.eql(bytes.length * 10)
 
     const stats = metrics.forPeer(peerId)
-    expect(metrics.peers).to.eql([peerId.toString()])
+    expect(metrics.peers).to.eql([peerId.toB58String()])
     expect(stats.snapshot.dataReceived.toNumber()).to.equal(results.length)
     expect(stats.snapshot.dataSent.toNumber()).to.equal(results.length)
 
@@ -148,7 +148,7 @@ describe('Metrics', () => {
     // Flush the call stack
     await delay(0)
 
-    expect(metrics.peers).to.eql([peerId.toString(), peerId2.toString()])
+    expect(metrics.peers).to.eql([peerId.toB58String(), peerId2.toB58String()])
     // Verify global metrics
     const globalStats = metrics.global
     expect(globalStats.snapshot.dataReceived.toNumber()).to.equal(bytes.length * 2)
@@ -181,7 +181,7 @@ describe('Metrics', () => {
     pipe(remote, remote)
 
     const mockPeer = {
-      toString: () => 'a temporary id'
+      toB58String: () => 'a temporary id'
     }
     metrics.trackStream({
       stream: local,
@@ -197,8 +197,8 @@ describe('Metrics', () => {
 
     await delay(0)
 
-    metrics.updatePlaceholder(mockPeer.toString(), peerId)
+    metrics.updatePlaceholder(mockPeer, peerId)
-    mockPeer.toString = peerId.toString.bind(peerId)
+    mockPeer.toB58String = peerId.toB58String.bind(peerId)
 
     input.push(bytes)
     input.end()
@@ -206,7 +206,7 @@ describe('Metrics', () => {
     await deferredPromise
     await delay(0)
 
-    expect(metrics.peers).to.eql([peerId.toString()])
+    expect(metrics.peers).to.eql([peerId.toB58String()])
     // Verify global metrics
     const globalStats = metrics.global
     expect(globalStats.snapshot.dataReceived.toNumber()).to.equal(bytes.length * 2)
@@ -237,7 +237,7 @@ describe('Metrics', () => {
     // Disconnect every peer
     for (const id of trackedPeers.keys()) {
       metrics.onPeerDisconnected({
-        toString: () => id
+        toB58String: () => id
       })
     }
 
@@ -245,7 +245,9 @@ describe('Metrics', () => {
     expect(metrics.peers).to.have.length(0)
     const retainedPeers = []
     for (const id of trackedPeers.keys()) {
-      const stat = metrics.forPeer(id)
+      const stat = metrics.forPeer({
+        toB58String: () => id
+      })
       if (stat) retainedPeers.push(id)
     }
     expect(retainedPeers).to.eql(['45', '46', '47', '48', '49'])

test/registrar/registrar.spec.js

@@ -194,7 +194,7 @@ describe('registrar', () => {
       const conn1 = await createMockConnection({ localPeer: localPeer.id, remotePeer: remotePeer.id })
       const conn2 = await createMockConnection({ localPeer: localPeer.id, remotePeer: remotePeer.id })
       const peerInfo = await PeerInfo.create(remotePeer.id)
-      const id = peerInfo.id.toString()
+      const id = peerInfo.id.toB58String()
 
       // Add connection to registrar
       peerStore.put(peerInfo)