chore: allow patching individual addreseses

libp2p · achingbrain · Sep 8, 2023 · Sep 10, 2023 · Sep 10, 2023 · Sep 10, 2023
commit 3b31fdd28234ec92bb074c6ca747a99b6050c848
@@ -13,7 +13,7 @@ export interface Address {
   /**
    * Obtained from a signed peer record
    */
-  isCertified: boolean
+  isCertified?: true
 
   /**
    * A timestamp of the last successful dial of this multiaddr

@@ -170,8 +170,7 @@ export class DialQueue {
     const { peerId, multiaddrs } = getPeerAddress(peerIdOrMultiaddr)
 
     const addrs: Address[] = multiaddrs.map(multiaddr => ({
-      multiaddr,
-      isCertified: false
+      multiaddr
     }))
 
     // create abort conditions - need to do this before `calculateMultiaddrs` as we may be about to
@@ -317,8 +316,7 @@ export class DialQueue {
         }
 
         return result.map(multiaddr => ({
-          multiaddr,
-          isCertified: false
+          multiaddr
         }))
       })
     ))
@@ -346,12 +344,6 @@ export class DialQueue {
 
     for (const addr of filteredAddrs) {
       const maStr = addr.multiaddr.toString()
-      const existing = dedupedAddrs.get(maStr)
-
-      if (existing != null) {
-        existing.isCertified = existing.isCertified || addr.isCertified || false
-        continue
-      }
 
       dedupedAddrs.set(maStr, addr)
     }
@@ -472,41 +464,12 @@ export class DialQueue {
                   signal
                 })
 
-                const peerData = await this.peerStore.get(conn.remotePeer)
-                const addresses = peerData.addresses.map((address) => {
-                  if (address.multiaddr.equals(addr)) {
-                    return {
-                      ...address,
-                      lastSuccess: Date.now()
-                    }
-                  }
-
-                  return address
-                })
-
                 // mark multiaddr dial as successful
-                await this.peerStore.merge(conn.remotePeer, {
-                  addresses
-                })
+                await this._updateAddressStatus(conn.remotePeer, addr, true)
               } catch (err: any) {
                 if (pendingDial.peerId != null) {
                   // mark multiaddr dial as failure
-                  const peerData = await this.peerStore.get(pendingDial.peerId)
-                  const addresses = peerData.addresses.map((address) => {
-                    if (address.multiaddr.equals(addr)) {
-                      return {
-                        ...address,
-                        lastFailure: Date.now()
-                      }
-                    }
-
-                    return address
-                  })
-
-                  // mark multiaddr dial as failed
-                  await this.peerStore.merge(pendingDial.peerId, {
-                    addresses
-                  })
+                  await this._updateAddressStatus(pendingDial.peerId, addr, false)
                 }
 
                 // rethrow error
@@ -558,7 +521,9 @@ export class DialQueue {
           signal.clear()
         })
 
-        return deferred.promise
+        const connection = await deferred.promise
+
+        return connection
       }))
 
       // dial succeeded or failed
@@ -581,6 +546,25 @@ export class DialQueue {
       throw err
     }
   }
+
+  /**
+   * Record the last dial success/failure status of the passed multiaddr
+   */
+  private async _updateAddressStatus (peerId: PeerId, multiaddr: Multiaddr, success: boolean): Promise<void> {
+    const addr: Address = {
+      multiaddr
+    }
+
+    if (success) {
+      addr.lastSuccess = Date.now()
+    } else {
+      addr.lastFailure = Date.now()
+    }
+
+    await this.peerStore.merge(peerId, {
+      addresses: [addr]
+    })
+  }
 }
 
 /**

@@ -430,7 +430,6 @@ export class DefaultIdentifyService implements Startable, IdentifyService {
 
     const peer = {
       addresses: message.listenAddrs.map(buf => ({
-        isCertified: false,
         multiaddr: multiaddr(buf)
       })),
       protocols: message.protocols,

@@ -212,7 +212,7 @@ describe('dialing (direct, WebSockets)', () => {
     await connectionManager.openConnection(remoteComponents.peerId)
 
     const sortedAddresses = peerMultiaddrs
-      .map((m) => ({ multiaddr: m, isCertified: false }))
+      .map((m) => ({ multiaddr: m }))
       .sort(defaultAddressSort)
 
     expect(localTMDialStub.getCall(0).args[0].equals(sortedAddresses[0].multiaddr))

@@ -269,7 +269,6 @@ describe('content-routing', () => {
       await drain(node.contentRouting.findProviders(CID.parse('QmU621oD8AhHw6t25vVyfYKmL9VV3PTgc52FngEhTGACFB')))
 
       await expect(node.peerStore.get(providerPeerId)).to.eventually.have.property('addresses').that.deep.include({
-        isCertified: false,
         multiaddr: result.multiaddrs[0]
       })
     })
@@ -376,10 +375,8 @@ describe('content-routing', () => {
       await drain(node.contentRouting.findProviders(CID.parse('QmU621oD8AhHw6t25vVyfYKmL9VV3PTgc52FngEhTGACFB')))
 
       await expect(node.peerStore.get(providerPeerId)).to.eventually.have.property('addresses').that.deep.include({
-        isCertified: false,
         multiaddr: result1.multiaddrs[0]
       }).and.to.deep.include({
-        isCertified: false,
         multiaddr: result2.multiaddrs[0]
       })
     })

@@ -444,7 +444,6 @@ describe('identify', () => {
     expect(peer.metadata.get('ProtocolVersion')).to.equalBytes(uint8ArrayFromString(message.protocolVersion))
     expect(peer.protocols).to.deep.equal(message.protocols)
     expect(peer.addresses).to.deep.equal([{
-      isCertified: false,
       multiaddr: multiaddr('/ip4/127.0.0.1/tcp/1234')
     }])
     expect(peer.id.publicKey).to.equalBytes(remoteComponents.peerId.publicKey)

@@ -2,7 +2,7 @@ import { peerIdFromPeerId } from '@libp2p/peer-id'
 import { multiaddr } from '@multiformats/multiaddr'
 import { Peer as PeerPB } from '../pb/peer.js'
 import type { PeerId } from '@libp2p/interface/peer-id'
-import type { Peer, Tag } from '@libp2p/interface/peer-store'
+import type { Address, Peer, Tag } from '@libp2p/interface/peer-store'
 
 export function bytesToPeer (peerId: PeerId, buf: Uint8Array): Peer {
   const peer = PeerPB.decode(buf)
@@ -31,12 +31,23 @@ export function bytesToPeer (peerId: PeerId, buf: Uint8Array): Peer {
     ...peer,
     id: peerId,
     addresses: peer.addresses.map(({ multiaddr: ma, isCertified, lastFailure, lastSuccess }) => {
-      return {
-        multiaddr: multiaddr(ma),
-        isCertified: isCertified ?? false,
-        lastFailure: lastFailure != null ? Number(lastFailure) : undefined,
-        lastSuccess: lastSuccess != null ? Number(lastSuccess) : undefined
+      const addr: Address = {
+        multiaddr: multiaddr(ma)
       }
+
+      if (isCertified) {
+        addr.isCertified = true
+      }
+
+      if (lastFailure != null) {
+        addr.lastFailure = Number(lastFailure)
+      }
+
+      if (lastSuccess != null) {
+        addr.lastSuccess = Number(lastSuccess)
+      }
+
+      return addr
     }),
     metadata: peer.metadata,
     peerRecordEnvelope: peer.peerRecordEnvelope ?? undefined,

@@ -26,32 +26,51 @@ export async function dedupeFilterAndSortAddresses (peerId: PeerId, filter: Addr
       continue
     }
 
-    const isCertified = addr.isCertified ?? false
     const maStr = addr.multiaddr.toString()
-    const existingAddr = addressMap.get(maStr)
-
-    if (existingAddr != null) {
-      addr.isCertified = existingAddr.isCertified || isCertified
-      addr.lastFailure = existingAddr.lastFailure != null ? BigInt(existingAddr.lastFailure) : undefined
-      addr.lastSuccess = existingAddr.lastSuccess != null ? BigInt(existingAddr.lastSuccess) : undefined
-    } else {
-      addressMap.set(maStr, {
-        multiaddr: addr.multiaddr,
-        isCertified,
-        lastFailure: addr.lastFailure != null ? Number(addr.lastFailure) : undefined,
-        lastSuccess: addr.lastSuccess != null ? Number(addr.lastSuccess) : undefined
-      })
+    let existingAddr = addressMap.get(maStr)
+
+    if (existingAddr == null) {
+      existingAddr = {
+        multiaddr: addr.multiaddr
+      }
+
+      addressMap.set(maStr, existingAddr)
+    }
+
+    if (addr.isCertified) {
+      existingAddr.isCertified = true
+    }
+
+    if (addr.lastFailure != null) {
+      existingAddr.lastFailure = Number(addr.lastFailure)
+    }
+
+    if (addr.lastSuccess != null) {
+      existingAddr.lastSuccess = Number(addr.lastSuccess)
     }
   }
 
   return [...addressMap.values()]
     .sort((a, b) => {
       return a.multiaddr.toString().localeCompare(b.multiaddr.toString())
     })
-    .map(({ isCertified, multiaddr, lastFailure, lastSuccess }) => ({
-      isCertified,
-      multiaddr: multiaddr.bytes,
-      lastFailure: lastFailure != null ? BigInt(lastFailure) : undefined,
-      lastSuccess: lastSuccess != null ? BigInt(lastSuccess) : undefined
-    }))
+    .map(({ isCertified, multiaddr, lastFailure, lastSuccess }) => {
+      const addr: AddressPB = {
+        multiaddr: multiaddr.bytes,
+      }
+
+      if (isCertified) {
+        addr.isCertified = true
+      }
+
+      if (lastFailure != null) {
+        addr.lastFailure = BigInt(lastFailure)
+      }
+
+      if (lastSuccess != null) {
+        addr.lastSuccess = BigInt(lastSuccess)
+      }
+
+      return addr
+    })
 }
@@ -20,7 +20,7 @@ export function toDatastorePeer (peerId: PeerId, data: PeerData): PeerPB {
 
   const output: PeerPB = {
     addresses: (data.addresses ?? [])
-      .concat((data.multiaddrs ?? []).map(multiaddr => ({ multiaddr, isCertified: false })))
+      .concat((data.multiaddrs ?? []).map(multiaddr => ({ multiaddr })))
       .filter(address => {
         if (!isMultiaddr(address.multiaddr)) {
           throw new CodeError('Invalid mulitaddr', codes.ERR_INVALID_PARAMETERS)

@@ -40,7 +40,6 @@ export async function toPeerPB (peerId: PeerId, data: Partial<PeerData>, strateg
 
       if (data.multiaddrs != null) {
         addresses.push(...data.multiaddrs.map(multiaddr => ({
-          isCertified: false,
           multiaddr
         })))
       }
@@ -80,7 +79,6 @@ export async function toPeerPB (peerId: PeerId, data: Partial<PeerData>, strateg
   if (strategy === 'merge') {
     if (data.multiaddrs != null) {
       addresses.push(...data.multiaddrs.map(multiaddr => ({
-        isCertified: false,
         multiaddr
       })))
     }

@@ -75,14 +75,11 @@ describe('merge', () => {
     })
 
     expect(updated).to.have.property('addresses').that.deep.equals([{
-      multiaddr: addr1,
-      isCertified: false
+      multiaddr: addr1
     }, {
-      multiaddr: addr3,
-      isCertified: false
+      multiaddr: addr3
     }, {
-      multiaddr: addr2,
-      isCertified: false
+      multiaddr: addr2
     }])
 
     // other fields should be untouched
@@ -244,4 +241,53 @@ describe('merge', () => {
     expect(updated).to.have.property('tags').that.deep.equals(original.tags)
     expect(updated).to.have.property('protocols').that.deep.equals(original.protocols)
   })
+
+  it('merges addresses', async () => {
-  it('merges addresses', async () => {
+  it('merges lastFailure & lastSuccess on peer addresses', async () => {
-  it('merges addresses', async () => {
+  it('merges lastFailure & lastSuccess on peer addresses', async () => {
+    const peer: PeerData = {
+      addresses: [{
+        multiaddr: addr1,
+        isCertified: true
+      }, {
+        multiaddr: addr2,
+        lastFailure: 5
+      }],
+      metadata: {
+        foo: Uint8Array.from([0, 1, 2])
+      },
+      tags: {
+        tag1: { value: 10 }
+      },
+      protocols: [
+        '/foo/bar'
+      ],
+      peerRecordEnvelope: Uint8Array.from([3, 4, 5])
+    }
+
+    const original = await peerStore.save(otherPeerId, peer)
+    const updated = await peerStore.merge(otherPeerId, {
+      addresses: [{
+        multiaddr: addr1,
+        lastFailure: 10
+      }, {
+        multiaddr: addr2,
+        lastSuccess: 10
+      }]
+    })
+
+    expect(updated).to.have.property('addresses').that.deep.equals([{
+      multiaddr: addr1,
+      isCertified: true,
+      lastFailure: 10
+    }, {
+      multiaddr: addr2,
+      lastFailure: 5,
+      lastSuccess: 10
+    }])
+
+    // other fields should be untouched
+    expect(updated).to.have.property('metadata').that.deep.equals(original.metadata)
+    expect(updated).to.have.property('tags').that.deep.equals(original.tags)
+    expect(updated).to.have.property('protocols').that.deep.equals(original.protocols)
+    expect(updated).to.have.property('peerRecordEnvelope').that.deep.equals(original.peerRecordEnvelope)
+  })
 })
@@ -76,8 +76,7 @@ describe('patch', () => {
 
     // upated field
     expect(updated).to.have.property('addresses').that.deep.equals([{
-      multiaddr: addr3,
-      isCertified: false
+      multiaddr: addr3
     }])
 
     // other fields should be untouched