fix: do not overwrite addresses on identify push when none are sent

During identify-push in response to a protocol update go-libp2p does not send a signed peer record or the current list of listen addresses.

Protobuf does not let us differentiate between an empty list and no list items at all because the field is just repeated - an absence of repeated fields doesn't mean the list was omitted.

When the listen address list is empty, preserve any existing addresses.

Author: achingbrain

packages/libp2p/src/identify/identify.ts

@@ -490,6 +490,17 @@ export class DefaultIdentifyService implements Startable, IdentifyService {
       peer.metadata.set('ProtocolVersion', uint8ArrayFromString(message.protocolVersion))
     }
 
+    // if the remote has not sent addresses, preserve the existing ones.
+    // this can happen during an identify-push triggered by a supported protocol
+    // change but the protobuf format makes it impossible to know if the remote
+    // was telling us they have no addresses (unlikely) or if they meant to send
+    // a partial update and omit the list entirely
+    if (peer.addresses.length === 0) {
+      // @ts-expect-error addresses is not optional
+      delete peer.addresses
+    }
+
+    log('patching %p with', peer)
     await this.peerStore.patch(connection.remotePeer, peer)
 
     const result: IdentifyResult = {

packages/libp2p/test/identify/index.spec.ts

@@ -3,7 +3,7 @@
 
 import { TypedEventEmitter } from '@libp2p/interface/events'
 import { start, stop } from '@libp2p/interface/startable'
-import { mockConnectionGater, mockRegistrar, mockUpgrader, connectionPair } from '@libp2p/interface-compliance-tests/mocks'
+import { mockConnectionGater, mockRegistrar, mockUpgrader, connectionPair, mockStream } from '@libp2p/interface-compliance-tests/mocks'
 import { createEd25519PeerId } from '@libp2p/peer-id-factory'
 import { PeerRecord, RecordEnvelope } from '@libp2p/peer-record'
 import { PersistentPeerStore } from '@libp2p/peer-store'
@@ -33,6 +33,9 @@ import { Identify } from '../../src/identify/pb/message.js'
 import { DefaultTransportManager } from '../../src/transport-manager.js'
 import type { IncomingStreamData } from '@libp2p/interface-internal/registrar'
 import type { TransportManager } from '@libp2p/interface-internal/transport-manager'
+import { pushable } from 'it-pushable'
+import type { Duplex, Source } from 'it-stream-types'
+import type { Connection } from '@libp2p/interface/src/connection/index.js'
 
 const listenMaddrs = [multiaddr('/ip4/127.0.0.1/tcp/15002/ws')]
 
@@ -504,4 +507,60 @@ describe('identify', () => {
     }])
     expect(peer.id.publicKey).to.equalBytes(remoteComponents.peerId.publicKey)
   })
+
+  it('should not overwrite addresses when none are sent', async () => {
+    const localIdentify = new DefaultIdentifyService(localComponents, defaultInit)
+    await start(localIdentify)
+
+    const duplex: Duplex<any, Source<any>, any> = {
+      source: pushable(),
+      sink: async (source) => {
+        await drain(source)
+      }
+    }
+
+    duplex.source.push(lp.encode.single(Identify.encode({
+      listenAddrs: [
+        multiaddr('/ip4/127.0.0.1/tcp/4001').bytes
+      ],
+      protocols: [
+        '/proto/1'
+      ]
+    })))
+
+    await localIdentify._handlePush({
+      stream: mockStream(duplex),
+      connection: stubInterface<Connection>({
+        remotePeer: remoteComponents.peerId
+      })
+    })
+
+    const peerData = await localComponents.peerStore.get(remoteComponents.peerId)
+    expect(peerData.addresses[0].multiaddr.toString()).to.equal('/ip4/127.0.0.1/tcp/4001')
+    expect(peerData.protocols).to.deep.equal(['/proto/1'])
+
+    const updateDuplex: Duplex<any, Source<any>, any> = {
+      source: pushable(),
+      sink: async (source) => {
+        await drain(source)
+      }
+    }
+
+    updateDuplex.source.push(lp.encode.single(Identify.encode({
+      protocols: [
+        '/proto/2'
+      ]
+    })))
+
+    await localIdentify._handlePush({
+      stream: mockStream(updateDuplex),
+      connection: stubInterface<Connection>({
+        remotePeer: remoteComponents.peerId
+      })
+    })
+
+    const updatedPeerData = await localComponents.peerStore.get(remoteComponents.peerId)
+    expect(updatedPeerData.addresses[0].multiaddr.toString()).to.equal('/ip4/127.0.0.1/tcp/4001')
+    expect(updatedPeerData.protocols).to.deep.equal(['/proto/2'])
+  })
 })