From d56fa0e788cc6b79084835f686e4bf02b1378067 Mon Sep 17 00:00:00 2001 From: Alex Potsides Date: Tue, 24 Oct 2023 18:55:05 +0100 Subject: [PATCH] fix!: remove protocols from PeerInfo (#2166) A [PeerInfo](https://docs.libp2p.io/concepts/fundamentals/peers/#peer-info) is a libp2p object that combines a PeerID and some Multiaddrs. We also add a list of protocols. This was a mistake because protocols are exchanged during Identify but the PeerInfo object is used for peer discovery. This is evident because we set the protocol list to an empty array everywhere. PeerInfo is useful for exchanging peer data with other nodes, if we need a more fleshed-out peer representation we'd use the `Peer` interface from the peer store. BREAKING CHANGE: the `.protocols` property has been removed from the `PeerInfo` interface --- .../src/mocks/peer-discovery.ts | 3 +-- packages/interface/src/peer-info/index.ts | 14 +++++++++++++- packages/kad-dht/src/content-routing/index.ts | 6 ++---- packages/kad-dht/src/kad-dht.ts | 2 +- packages/kad-dht/src/message/index.ts | 3 +-- packages/kad-dht/src/peer-routing/index.ts | 9 +++------ packages/kad-dht/src/rpc/handlers/find-node.ts | 3 +-- packages/kad-dht/src/rpc/handlers/get-providers.ts | 3 +-- packages/kad-dht/test/kad-utils.spec.ts | 4 ++-- packages/kad-dht/test/query-self.spec.ts | 6 ++---- packages/kad-dht/test/query.spec.ts | 3 +-- .../kad-dht/test/rpc/handlers/add-provider.spec.ts | 6 ++---- .../kad-dht/test/rpc/handlers/find-node.spec.ts | 9 +++------ .../test/rpc/handlers/get-providers.spec.ts | 6 ++---- .../kad-dht/test/rpc/handlers/get-value.spec.ts | 3 +-- packages/kad-dht/test/utils/test-dht.ts | 3 +-- packages/libp2p/src/libp2p.ts | 6 ++---- packages/libp2p/test/circuit-relay/utils.ts | 3 +-- .../test/content-routing/content-routing.node.ts | 3 +-- packages/peer-discovery-bootstrap/src/index.ts | 3 +-- packages/peer-discovery-mdns/src/query.ts | 3 +-- 21 files changed, 43 insertions(+), 58 deletions(-) diff --git a/packages/interface-compliance-tests/src/mocks/peer-discovery.ts b/packages/interface-compliance-tests/src/mocks/peer-discovery.ts index 69bd2afc29..7fa6abedc1 100644 --- a/packages/interface-compliance-tests/src/mocks/peer-discovery.ts +++ b/packages/interface-compliance-tests/src/mocks/peer-discovery.ts @@ -49,8 +49,7 @@ export class MockDiscovery extends TypedEventEmitter implem this.safeDispatchEvent('peer', { detail: { id: peerId, - multiaddrs: [multiaddr('/ip4/127.0.0.1/tcp/8000')], - protocols: [] + multiaddrs: [multiaddr('/ip4/127.0.0.1/tcp/8000')] } }) }, this.options.discoveryDelay ?? 1000) diff --git a/packages/interface/src/peer-info/index.ts b/packages/interface/src/peer-info/index.ts index 25dbd9f208..5b7c8cce9d 100644 --- a/packages/interface/src/peer-info/index.ts +++ b/packages/interface/src/peer-info/index.ts @@ -1,8 +1,20 @@ import type { PeerId } from '../peer-id/index.js' import type { Multiaddr } from '@multiformats/multiaddr' +/** + * A `PeerInfo` is a lightweight object that represents a remote peer, it can be + * obtained from peer discovery mechanisms, HTTP RPC endpoints, etc. + * + * @see https://docs.libp2p.io/concepts/fundamentals/peers/#peer-info + */ export interface PeerInfo { + /** + * The identifier of the remote peer + */ id: PeerId + + /** + * The multiaddrs a peer is listening on + */ multiaddrs: Multiaddr[] - protocols: string[] } diff --git a/packages/kad-dht/src/content-routing/index.ts b/packages/kad-dht/src/content-routing/index.ts index 2ca2033cd4..569e7942f7 100644 --- a/packages/kad-dht/src/content-routing/index.ts +++ b/packages/kad-dht/src/content-routing/index.ts @@ -64,8 +64,7 @@ export class ContentRouting { const msg = new Message(MESSAGE_TYPE.ADD_PROVIDER, key.multihash.bytes, 0) msg.providerPeers = [{ id: this.components.peerId, - multiaddrs, - protocols: [] + multiaddrs }] let sent = 0 @@ -140,8 +139,7 @@ export class ContentRouting { providers.push({ id: peerId, - multiaddrs: peer.addresses.map(({ multiaddr }) => multiaddr), - protocols: peer.protocols + multiaddrs: peer.addresses.map(({ multiaddr }) => multiaddr) }) } catch (err: any) { if (err.code !== 'ERR_NOT_FOUND') { diff --git a/packages/kad-dht/src/kad-dht.ts b/packages/kad-dht/src/kad-dht.ts index fb5be07aa9..6efd51cb09 100644 --- a/packages/kad-dht/src/kad-dht.ts +++ b/packages/kad-dht/src/kad-dht.ts @@ -218,7 +218,7 @@ export class DefaultKadDHT extends TypedEventEmitter implem } async onPeerConnect (peerData: PeerInfo): Promise { - this.log('peer %p connected with protocols', peerData.id, peerData.protocols) + this.log('peer %p connected', peerData.id) if (this.lan) { peerData = removePublicAddresses(peerData) diff --git a/packages/kad-dht/src/message/index.ts b/packages/kad-dht/src/message/index.ts index 585f83c108..ef24ee4c40 100644 --- a/packages/kad-dht/src/message/index.ts +++ b/packages/kad-dht/src/message/index.ts @@ -104,7 +104,6 @@ function fromPbPeer (peer: PBMessage.Peer): PeerInfo { return { id: peerIdFromBytes(peer.id), - multiaddrs: (peer.addrs ?? []).map((a) => multiaddr(a)), - protocols: [] + multiaddrs: (peer.addrs ?? []).map((a) => multiaddr(a)) } } diff --git a/packages/kad-dht/src/peer-routing/index.ts b/packages/kad-dht/src/peer-routing/index.ts index b356f86263..e2b2b304b5 100644 --- a/packages/kad-dht/src/peer-routing/index.ts +++ b/packages/kad-dht/src/peer-routing/index.ts @@ -85,8 +85,7 @@ export class PeerRouting { return { id: peerData.id, - multiaddrs: peerData.addresses.map((address) => address.multiaddr), - protocols: [] + multiaddrs: peerData.addresses.map((address) => address.multiaddr) } } @@ -226,8 +225,7 @@ export class PeerRouting { from: this.components.peerId, peer: { id: peerId, - multiaddrs: peer.addresses.map(({ multiaddr }) => multiaddr), - protocols: peer.protocols + multiaddrs: peer.addresses.map(({ multiaddr }) => multiaddr) } }, options) } catch (err: any) { @@ -296,8 +294,7 @@ export class PeerRouting { output.push({ id: peerId, - multiaddrs: peer.addresses.map(({ multiaddr }) => multiaddr), - protocols: peer.protocols + multiaddrs: peer.addresses.map(({ multiaddr }) => multiaddr) }) } catch (err: any) { if (err.code !== 'ERR_NOT_FOUND') { diff --git a/packages/kad-dht/src/rpc/handlers/find-node.ts b/packages/kad-dht/src/rpc/handlers/find-node.ts index 979ed1919a..4763ba5376 100644 --- a/packages/kad-dht/src/rpc/handlers/find-node.ts +++ b/packages/kad-dht/src/rpc/handlers/find-node.ts @@ -48,8 +48,7 @@ export class FindNodeHandler implements DHTMessageHandler { if (uint8ArrayEquals(this.components.peerId.toBytes(), msg.key)) { closer = [{ id: this.components.peerId, - multiaddrs: this.components.addressManager.getAddresses().map(ma => ma.decapsulateCode(protocols('p2p').code)), - protocols: [] + multiaddrs: this.components.addressManager.getAddresses().map(ma => ma.decapsulateCode(protocols('p2p').code)) }] } else { closer = await this.peerRouting.getCloserPeersOffline(msg.key, peerId) diff --git a/packages/kad-dht/src/rpc/handlers/get-providers.ts b/packages/kad-dht/src/rpc/handlers/get-providers.ts index 8e0c8563bb..18778887a6 100644 --- a/packages/kad-dht/src/rpc/handlers/get-providers.ts +++ b/packages/kad-dht/src/rpc/handlers/get-providers.ts @@ -86,8 +86,7 @@ export class GetProvidersHandler implements DHTMessageHandler { const peerAfterFilter = addrFilter({ id: peerId, - multiaddrs: peer.addresses.map(({ multiaddr }) => multiaddr), - protocols: peer.protocols + multiaddrs: peer.addresses.map(({ multiaddr }) => multiaddr) }) if (peerAfterFilter.multiaddrs.length > 0) { diff --git a/packages/kad-dht/test/kad-utils.spec.ts b/packages/kad-dht/test/kad-utils.spec.ts index 2d99cc4ebf..4f27ae4a27 100644 --- a/packages/kad-dht/test/kad-utils.spec.ts +++ b/packages/kad-dht/test/kad-utils.spec.ts @@ -73,7 +73,7 @@ describe('kad utils', () => { multiaddr('/dns4/localhost/tcp/4001') ] - const peerInfo = utils.removePrivateAddresses({ id, multiaddrs, protocols: [] }) + const peerInfo = utils.removePrivateAddresses({ id, multiaddrs }) expect(peerInfo.multiaddrs.map((ma) => ma.toString())) .to.eql(['/dns4/example.com/tcp/4001', '/ip4/1.1.1.1/tcp/4001']) }) @@ -90,7 +90,7 @@ describe('kad utils', () => { multiaddr('/dns4/localhost/tcp/4001') ] - const peerInfo = utils.removePublicAddresses({ id, multiaddrs, protocols: [] }) + const peerInfo = utils.removePublicAddresses({ id, multiaddrs }) expect(peerInfo.multiaddrs.map((ma) => ma.toString())) .to.eql(['/ip4/192.168.0.1/tcp/4001', '/dns4/localhost/tcp/4001']) }) diff --git a/packages/kad-dht/test/query-self.spec.ts b/packages/kad-dht/test/query-self.spec.ts index d43313d29e..2c4904276b 100644 --- a/packages/kad-dht/test/query-self.spec.ts +++ b/packages/kad-dht/test/query-self.spec.ts @@ -78,8 +78,7 @@ describe('Query Self', () => { from: remotePeer, peer: { id: remotePeer, - multiaddrs: [], - protocols: [] + multiaddrs: [] } }) }()) @@ -110,8 +109,7 @@ describe('Query Self', () => { from: remotePeer, peer: { id: remotePeer, - multiaddrs: [], - protocols: [] + multiaddrs: [] } }) }()) diff --git a/packages/kad-dht/test/query.spec.ts b/packages/kad-dht/test/query.spec.ts index e1fd7460af..f7e661803c 100644 --- a/packages/kad-dht/test/query.spec.ts +++ b/packages/kad-dht/test/query.spec.ts @@ -534,8 +534,7 @@ describe('QueryManager', () => { messageType: MESSAGE_TYPE.GET_VALUE, closer: [{ id: peers[2], - multiaddrs: [], - protocols: [] + multiaddrs: [] }] }) } diff --git a/packages/kad-dht/test/rpc/handlers/add-provider.spec.ts b/packages/kad-dht/test/rpc/handlers/add-provider.spec.ts index ad9633856d..652060570c 100644 --- a/packages/kad-dht/test/rpc/handlers/add-provider.spec.ts +++ b/packages/kad-dht/test/rpc/handlers/add-provider.spec.ts @@ -68,12 +68,10 @@ describe('rpc - handlers - AddProvider', () => { msg.providerPeers = [{ id: peerIds[0], - multiaddrs: [ma1], - protocols: [] + multiaddrs: [ma1] }, { id: peerIds[1], - multiaddrs: [ma2], - protocols: [] + multiaddrs: [ma2] }] await handler.handle(peerIds[0], msg) diff --git a/packages/kad-dht/test/rpc/handlers/find-node.spec.ts b/packages/kad-dht/test/rpc/handlers/find-node.spec.ts index 5eb13982bf..e197e3770a 100644 --- a/packages/kad-dht/test/rpc/handlers/find-node.spec.ts +++ b/packages/kad-dht/test/rpc/handlers/find-node.spec.ts @@ -71,8 +71,7 @@ describe('rpc - handlers - FindNode', () => { multiaddr('/ip4/127.0.0.1/tcp/4002'), multiaddr('/ip4/192.168.1.5/tcp/4002'), multiaddr('/ip4/221.4.67.0/tcp/4002') - ], - protocols: [] + ] }]) const response = await handler.handle(sourcePeer, msg) @@ -109,8 +108,7 @@ describe('rpc - handlers - FindNode', () => { multiaddr('/ip4/127.0.0.1/tcp/4002'), multiaddr('/ip4/192.168.1.5/tcp/4002'), multiaddr('/ip4/221.4.67.0/tcp/4002') - ], - protocols: [] + ] }]) handler = new FindNodeHandler({ @@ -146,8 +144,7 @@ describe('rpc - handlers - FindNode', () => { multiaddr('/ip4/127.0.0.1/tcp/4002'), multiaddr('/ip4/192.168.1.5/tcp/4002'), multiaddr('/ip4/221.4.67.0/tcp/4002') - ], - protocols: [] + ] }]) const response = await handler.handle(sourcePeer, msg) diff --git a/packages/kad-dht/test/rpc/handlers/get-providers.spec.ts b/packages/kad-dht/test/rpc/handlers/get-providers.spec.ts index 18a3706f64..97688407d4 100644 --- a/packages/kad-dht/test/rpc/handlers/get-providers.spec.ts +++ b/packages/kad-dht/test/rpc/handlers/get-providers.spec.ts @@ -73,8 +73,7 @@ describe('rpc - handlers - GetProviders', () => { multiaddr('/ip4/127.0.0.1/tcp/4002'), multiaddr('/ip4/192.168.2.6/tcp/4002'), multiaddr('/ip4/21.31.57.23/tcp/4002') - ], - protocols: [] + ] }] const provider: PeerInfo[] = [{ @@ -83,8 +82,7 @@ describe('rpc - handlers - GetProviders', () => { multiaddr('/ip4/127.0.0.1/tcp/4002'), multiaddr('/ip4/192.168.1.5/tcp/4002'), multiaddr('/ip4/135.4.67.0/tcp/4002') - ], - protocols: [] + ] }] providers.getProviders.withArgs(v.cid).resolves([providerPeer]) diff --git a/packages/kad-dht/test/rpc/handlers/get-value.spec.ts b/packages/kad-dht/test/rpc/handlers/get-value.spec.ts index ad79dda7c6..61bf6a35aa 100644 --- a/packages/kad-dht/test/rpc/handlers/get-value.spec.ts +++ b/packages/kad-dht/test/rpc/handlers/get-value.spec.ts @@ -94,8 +94,7 @@ describe('rpc - handlers - GetValue', () => { peerRouting.getCloserPeersOffline.withArgs(key, sourcePeer) .resolves([{ id: closerPeer, - multiaddrs: [], - protocols: [] + multiaddrs: [] }]) const msg = new Message(T, key, 0) diff --git a/packages/kad-dht/test/utils/test-dht.ts b/packages/kad-dht/test/utils/test-dht.ts index bfddebdaad..6d81567526 100644 --- a/packages/kad-dht/test/utils/test-dht.ts +++ b/packages/kad-dht/test/utils/test-dht.ts @@ -94,8 +94,7 @@ export class TestDHT { } components.peerStore.merge(peerData.id, { - multiaddrs: peerData.multiaddrs, - protocols: peerData.protocols + multiaddrs: peerData.multiaddrs }) .catch(err => { log.error(err) }) }) diff --git a/packages/libp2p/src/libp2p.ts b/packages/libp2p/src/libp2p.ts index 54327c1c15..0b74308436 100644 --- a/packages/libp2p/src/libp2p.ts +++ b/packages/libp2p/src/libp2p.ts @@ -96,8 +96,7 @@ export class Libp2pNode> extends if (evt.detail.previous == null) { const peerInfo: PeerInfo = { id: evt.detail.peer.id, - multiaddrs: evt.detail.peer.addresses.map(a => a.multiaddr), - protocols: evt.detail.peer.protocols + multiaddrs: evt.detail.peer.addresses.map(a => a.multiaddr) } components.events.safeDispatchEvent('peer:discovery', { detail: peerInfo }) @@ -380,8 +379,7 @@ export class Libp2pNode> extends } void this.components.peerStore.merge(peer.id, { - multiaddrs: peer.multiaddrs, - protocols: peer.protocols + multiaddrs: peer.multiaddrs }) .catch(err => { log.error(err) }) } diff --git a/packages/libp2p/test/circuit-relay/utils.ts b/packages/libp2p/test/circuit-relay/utils.ts index aa3cde6e15..b530b8d241 100644 --- a/packages/libp2p/test/circuit-relay/utils.ts +++ b/packages/libp2p/test/circuit-relay/utils.ts @@ -159,8 +159,7 @@ export class MockContentRouting implements ContentRouting { providers.push({ id: this.peerId, - multiaddrs: this.addressManager.getAddresses(), - protocols: [] + multiaddrs: this.addressManager.getAddresses() }) MockContentRouting.providers.set(cid.toString(), providers) diff --git a/packages/libp2p/test/content-routing/content-routing.node.ts b/packages/libp2p/test/content-routing/content-routing.node.ts index 66967667ea..64f95d5616 100644 --- a/packages/libp2p/test/content-routing/content-routing.node.ts +++ b/packages/libp2p/test/content-routing/content-routing.node.ts @@ -251,8 +251,7 @@ describe('content-routing', () => { id: providerPeerId, multiaddrs: [ multiaddr('/ip4/123.123.123.123/tcp/49320') - ], - protocols: [] + ] } if (node.services.dht == null) { diff --git a/packages/peer-discovery-bootstrap/src/index.ts b/packages/peer-discovery-bootstrap/src/index.ts index 999b14f5e0..f7e3d22da5 100644 --- a/packages/peer-discovery-bootstrap/src/index.ts +++ b/packages/peer-discovery-bootstrap/src/index.ts @@ -134,8 +134,7 @@ class Bootstrap extends TypedEventEmitter implements PeerDi const peerData: PeerInfo = { id: peerIdFromString(peerIdStr), - multiaddrs: [ma], - protocols: [] + multiaddrs: [ma] } this.list.push(peerData) diff --git a/packages/peer-discovery-mdns/src/query.ts b/packages/peer-discovery-mdns/src/query.ts index bb6aa8ba30..45f287a705 100644 --- a/packages/peer-discovery-mdns/src/query.ts +++ b/packages/peer-discovery-mdns/src/query.ts @@ -74,8 +74,7 @@ export function gotResponse (rsp: ResponsePacket, localPeerName: string, service return { id: peerIdFromString(peerId), - multiaddrs: multiaddrs.map(addr => addr.decapsulateCode(protocols('p2p').code)), - protocols: [] + multiaddrs: multiaddrs.map(addr => addr.decapsulateCode(protocols('p2p').code)) } } catch (e) { log.error('failed to parse mdns response', e)