diff --git a/packages/kad-dht/src/rpc/handlers/find-node.ts b/packages/kad-dht/src/rpc/handlers/find-node.ts index 162a8ce775..8ad6bf8c08 100644 --- a/packages/kad-dht/src/rpc/handlers/find-node.ts +++ b/packages/kad-dht/src/rpc/handlers/find-node.ts @@ -1,13 +1,10 @@ import { CodeError } from '@libp2p/interface' -import { protocols } from '@multiformats/multiaddr' -import { equals as uint8ArrayEquals } from 'uint8arrays' import { MessageType } from '../../message/dht.js' import type { PeerInfoMapper } from '../../index.js' import type { Message } from '../../message/dht.js' import type { PeerRouting } from '../../peer-routing/index.js' import type { DHTMessageHandler } from '../index.js' import type { ComponentLogger, Logger, PeerId, PeerInfo } from '@libp2p/interface' -import type { AddressManager } from '@libp2p/interface-internal' export interface FindNodeHandlerInit { peerRouting: PeerRouting @@ -17,7 +14,6 @@ export interface FindNodeHandlerInit { export interface FindNodeHandlerComponents { peerId: PeerId - addressManager: AddressManager logger: ComponentLogger } @@ -25,7 +21,6 @@ export class FindNodeHandler implements DHTMessageHandler { private readonly peerRouting: PeerRouting private readonly peerInfoMapper: PeerInfoMapper private readonly peerId: PeerId - private readonly addressManager: AddressManager private readonly log: Logger constructor (components: FindNodeHandlerComponents, init: FindNodeHandlerInit) { @@ -33,7 +28,6 @@ export class FindNodeHandler implements DHTMessageHandler { this.log = components.logger.forComponent(`${logPrefix}:rpc:handlers:find-node`) this.peerId = components.peerId - this.addressManager = components.addressManager this.peerRouting = peerRouting this.peerInfoMapper = init.peerInfoMapper } @@ -44,20 +38,11 @@ export class FindNodeHandler implements DHTMessageHandler { async handle (peerId: PeerId, msg: Message): Promise { this.log('incoming request from %p for peers closer to %b', peerId, msg.key) - let closer: PeerInfo[] = [] - if (msg.key == null) { throw new CodeError('Invalid FIND_NODE message received - key was missing', 'ERR_INVALID_MESSAGE') } - if (uint8ArrayEquals(this.peerId.toBytes(), msg.key)) { - closer = [{ - id: this.peerId, - multiaddrs: this.addressManager.getAddresses().map(ma => ma.decapsulateCode(protocols('p2p').code)) - }] - } else { - closer = await this.peerRouting.getCloserPeersOffline(msg.key, peerId) - } + const closer: PeerInfo[] = await this.peerRouting.getCloserPeersOffline(msg.key, peerId) const response: Message = { type: MessageType.FIND_NODE, @@ -65,6 +50,7 @@ export class FindNodeHandler implements DHTMessageHandler { closer: closer .map(this.peerInfoMapper) .filter(({ multiaddrs }) => multiaddrs.length) + .filter(({ id }) => !id.equals(this.peerId)) .map(peerInfo => ({ id: peerInfo.id.toBytes(), multiaddrs: peerInfo.multiaddrs.map(ma => ma.bytes) diff --git a/packages/kad-dht/src/rpc/handlers/get-providers.ts b/packages/kad-dht/src/rpc/handlers/get-providers.ts index ef13fa5353..f8bb968626 100644 --- a/packages/kad-dht/src/rpc/handlers/get-providers.ts +++ b/packages/kad-dht/src/rpc/handlers/get-providers.ts @@ -40,7 +40,7 @@ export class GetProvidersHandler implements DHTMessageHandler { async handle (peerId: PeerId, msg: Message): Promise { if (msg.key == null) { - throw new CodeError('Invalid FIND_NODE message received - key was missing', 'ERR_INVALID_MESSAGE') + throw new CodeError('Invalid GET_PROVIDERS message received - key was missing', 'ERR_INVALID_MESSAGE') } let cid diff --git a/packages/kad-dht/test/kad-dht.spec.ts b/packages/kad-dht/test/kad-dht.spec.ts index 70d331212c..73df181518 100644 --- a/packages/kad-dht/test/kad-dht.spec.ts +++ b/packages/kad-dht/test/kad-dht.spec.ts @@ -799,6 +799,37 @@ describe('KadDHT', () => { expect(res).to.not.be.empty() }) + + it('should not include itself in getClosestPeers PEER_RESPONSE', async function () { + this.timeout(240 * 1000) + + const nDHTs = 30 + const dhts = await Promise.all( + new Array(nDHTs).fill(0).map(async () => tdht.spawn()) + ) + + const connected: Array> = [] + + for (let i = 0; i < dhts.length - 1; i++) { + connected.push(tdht.connect(dhts[i], dhts[(i + 1) % dhts.length])) + } + + await Promise.all(connected) + + const res = await all(dhts[1].getClosestPeers(dhts[2].components.peerId.toBytes())) + expect(res).to.not.be.empty() + + // no peer should include itself in the response, only other peers that it + // knows who are closer + for (const event of res) { + if (event.name !== 'PEER_RESPONSE') { + continue + } + + expect(event.closer.map(peer => peer.id.toString())) + .to.not.include(event.from.toString()) + } + }) }) describe('errors', () => { 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 8ca6535470..e4ed94d6be 100644 --- a/packages/kad-dht/test/rpc/handlers/find-node.spec.ts +++ b/packages/kad-dht/test/rpc/handlers/find-node.spec.ts @@ -5,7 +5,6 @@ import { peerIdFromBytes } from '@libp2p/peer-id' import { multiaddr } from '@multiformats/multiaddr' import { expect } from 'aegir/chai' import Sinon, { type SinonStubbedInstance } from 'sinon' -import { stubInterface } from 'sinon-ts' import { type Message, MessageType } from '../../../src/message/dht.js' import { PeerRouting } from '../../../src/peer-routing/index.js' import { FindNodeHandler } from '../../../src/rpc/handlers/find-node.js' @@ -13,8 +12,6 @@ import { passthroughMapper, removePrivateAddressesMapper, removePublicAddressesM import { createPeerId } from '../../utils/create-peer-id.js' import type { DHTMessageHandler } from '../../../src/rpc/index.js' import type { PeerId } from '@libp2p/interface' -import type { AddressManager } from '@libp2p/interface-internal' -import type { StubbedInstance } from 'sinon-ts' const T = MessageType.FIND_NODE @@ -24,18 +21,15 @@ describe('rpc - handlers - FindNode', () => { let targetPeer: PeerId let handler: DHTMessageHandler let peerRouting: SinonStubbedInstance - let addressManager: StubbedInstance beforeEach(async () => { peerId = await createPeerId() sourcePeer = await createPeerId() targetPeer = await createPeerId() peerRouting = Sinon.createStubInstance(PeerRouting) - addressManager = stubInterface() handler = new FindNodeHandler({ peerId, - addressManager, logger: defaultLogger() }, { peerRouting, @@ -44,7 +38,7 @@ describe('rpc - handlers - FindNode', () => { }) }) - it('returns self, if asked for self', async () => { + it('returns nodes close to self but excludes self, if asked for self', async () => { const msg: Message = { type: T, key: peerId.multihash.bytes, @@ -52,13 +46,25 @@ describe('rpc - handlers - FindNode', () => { providers: [] } - addressManager.getAddresses.returns([ - multiaddr(`/ip4/127.0.0.1/tcp/4002/p2p/${peerId.toString()}`), - multiaddr(`/ip4/192.168.1.5/tcp/4002/p2p/${peerId.toString()}`), - multiaddr(`/ip4/221.4.67.0/tcp/4002/p2p/${peerId.toString()}`) - ]) + peerRouting.getCloserPeersOffline + .withArgs(peerId.multihash.bytes, peerId) + .resolves([{ + id: targetPeer, // closer peer + multiaddrs: [ + multiaddr('/ip4/127.0.0.1/tcp/4002'), + multiaddr('/ip4/192.168.1.5/tcp/4002'), + multiaddr('/ip4/221.4.67.0/tcp/4002') + ] + }, { + id: peerId, // self peer + multiaddrs: [ + multiaddr('/ip4/127.0.0.1/tcp/4002'), + multiaddr('/ip4/192.168.1.5/tcp/4002'), + multiaddr('/ip4/221.4.67.0/tcp/4002') + ] + }]) - const response = await handler.handle(sourcePeer, msg) + const response = await handler.handle(peerId, msg) if (response == null) { throw new Error('No response received from handler') @@ -67,7 +73,8 @@ describe('rpc - handlers - FindNode', () => { expect(response.closer).to.have.length(1) const peer = response.closer[0] - expect(peerIdFromBytes(peer.id).toString()).to.equal(peerId.toString()) + expect(peerIdFromBytes(peer.id).toString()).to.equal(targetPeer.toString()) + expect(peer.multiaddrs).to.not.be.empty() }) it('returns closer peers', async () => { @@ -138,7 +145,6 @@ describe('rpc - handlers - FindNode', () => { handler = new FindNodeHandler({ peerId, - addressManager, logger: defaultLogger() }, { peerRouting, @@ -181,7 +187,6 @@ describe('rpc - handlers - FindNode', () => { handler = new FindNodeHandler({ peerId, - addressManager, logger: defaultLogger() }, { peerRouting, diff --git a/packages/kad-dht/test/rpc/index.node.ts b/packages/kad-dht/test/rpc/index.node.ts index c4886d22dc..8233b04f22 100644 --- a/packages/kad-dht/test/rpc/index.node.ts +++ b/packages/kad-dht/test/rpc/index.node.ts @@ -24,7 +24,6 @@ import { passthroughMapper } from '../../src/utils.js' import { createPeerId } from '../utils/create-peer-id.js' import type { Validators } from '../../src/index.js' import type { Libp2pEvents, Connection, PeerId, PeerStore } from '@libp2p/interface' -import type { AddressManager } from '@libp2p/interface-internal' import type { Datastore } from 'interface-datastore' import type { Duplex, Source } from 'it-stream-types' @@ -45,7 +44,6 @@ describe('rpc', () => { peerId, datastore, peerStore: stubInterface(), - addressManager: stubInterface(), logger: defaultLogger() } components.peerStore = new PersistentPeerStore({