From 7b69ca44133e06ba2c6cf6c08830bc43af4c2c04 Mon Sep 17 00:00:00 2001 From: Rinse Date: Thu, 14 May 2026 06:35:30 +0000 Subject: [PATCH] fix(bitswap): skip identify race when peer protocols are already known MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Network#connectTo awaits Promise.all([libp2p.dial, raceEvent('peer:identify', filter for BITSWAP_120)]). peer:identify is single-shot per peer, so if the peer was already identified before bitswap calls connectTo (common when the host app dials peers via other subsystems first — pubsub, fetch, kad-dht, manual dial), raceEvent waits for an event that will never re-fire and connectTo hangs indefinitely. This in turn blocks findAndConnect's find-providers fallback, so wantBlock times out instead of resolving from already-connected bitswap-speaking peers. Add a fast path: before the race, check libp2p.peerStore.get(peer). If the peer is already known and advertises BITSWAP_120, return from a plain dial() (cheap no-op when already connected). Otherwise fall through to the existing identify-race code path unchanged. Safety: - connectTo callers (findAndConnect at network.ts:319 and 330) discard the returned Connection and use connectTo only for its peer-warmup side effect. - The actual bitswap stream is opened later in sendMessage via libp2p.dialProtocol(peerId, BITSWAP_120), which performs its own protocol negotiation. So a fast-path-returned connection whose bitswap stream is in fact broken is detected and retried by sendMessage — no invariant is violated. Measured effect: in an IPNS-over-pubsub workload where pubsub-mesh warmup dials the same peers that bitswap later wants providers from, median wantBlock wall time drops ~17% (3823 -> 3192 ms over n=230 fetches). --- packages/bitswap/src/network.ts | 17 ++++++- packages/bitswap/test/network.spec.ts | 67 +++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/packages/bitswap/src/network.ts b/packages/bitswap/src/network.ts index 95ab94cb0..30d11a9b8 100644 --- a/packages/bitswap/src/network.ts +++ b/packages/bitswap/src/network.ts @@ -1,4 +1,4 @@ -import { InvalidParametersError, NotStartedError, TimeoutError, TypedEventEmitter, UnsupportedProtocolError, setMaxListeners } from '@libp2p/interface' +import { InvalidParametersError, NotStartedError, TimeoutError, TypedEventEmitter, UnsupportedProtocolError, isPeerId, setMaxListeners } from '@libp2p/interface' import { PeerQueue } from '@libp2p/utils' import drain from 'it-drain' import * as lp from 'it-length-prefixed' @@ -406,6 +406,21 @@ export class Network extends TypedEventEmitter { options?.onProgress?.(new CustomProgressEvent('bitswap:dial', peer)) + // Fast path: peer:identify is single-shot per peer, so if the peer was + // already identified (e.g. dialed by another subsystem) the raceEvent + // below would wait for an event that has already fired. When the peerStore + // already lists BITSWAP_120 for this peer we can skip the race entirely. + if (isPeerId(peer)) { + try { + const peerData = await this.libp2p.peerStore.get(peer) + if (peerData.protocols.includes(BITSWAP_120)) { + return await this.libp2p.dial(peer, options) + } + } catch { + // peer not in peerStore yet — fall through to the identify race + } + } + // dial and wait for identify - this is to avoid opening a protocol stream // that we are not going to use but depends on the remote node running the // identify protocol diff --git a/packages/bitswap/test/network.spec.ts b/packages/bitswap/test/network.spec.ts index a66bcd9f5..e0a75fc5c 100644 --- a/packages/bitswap/test/network.spec.ts +++ b/packages/bitswap/test/network.spec.ts @@ -66,6 +66,73 @@ describe('network', () => { .with.property('name', 'NotStartedError') }) + it('should not wait for peer:identify when the peer is already known to speak bitswap', async () => { + // peer:identify is single-shot per peer. If the peer was already + // identified by another subsystem (e.g. pubsub mesh warmup) before + // bitswap calls connectTo, awaiting peer:identify hangs forever. When + // the peerStore already lists BITSWAP_120 for the peer the race is + // unnecessary and connectTo must resolve from dial() alone. + const peerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519')) + const connection = stubInterface() + + components.libp2p.peerStore = stubInterface({ + get: Sinon.stub().withArgs(peerId).resolves({ + id: peerId, + addresses: [], + protocols: [BITSWAP_120], + metadata: new Map(), + tags: new Map() + }) + }) as any + + components.libp2p.dial.withArgs(peerId).resolves(connection) + + const result = await network.connectTo(peerId) + expect(result).to.equal(connection) + expect(components.libp2p.dial.calledWith(peerId)).to.be.true() + // raceEvent listens via addEventListener; the fast path skips it entirely + const identifyListens = components.libp2p.addEventListener.getCalls() + .filter(call => call.args[0] === 'peer:identify') + expect(identifyListens, 'should not subscribe to peer:identify on the fast path').to.have.lengthOf(0) + }) + + it('should fall through to the identify race when the peer is not yet in peerStore', async () => { + const peerId = peerIdFromPrivateKey(await generateKeyPair('Ed25519')) + const connection = stubInterface() + const peerStoreError = new Error('NotFoundError') + peerStoreError.name = 'NotFoundError' + + components.libp2p.peerStore = stubInterface({ + get: Sinon.stub().rejects(peerStoreError) + }) as any + + components.libp2p.dial.callsFake(async () => { + // simulate identify firing after dial + setTimeout(() => { + const call = components.libp2p.addEventListener.getCalls() + .find(c => c.args[0] === 'peer:identify') + const callback = call?.args[1] + if (typeof callback === 'function') { + callback(new CustomEvent('peer:identify', { + detail: { + peerId, + protocols: [BITSWAP_120], + listenAddrs: [], + connection + } + })) + } + }, 10) + return connection + }) + + await network.connectTo(peerId) + + const identifyListens = components.libp2p.addEventListener.getCalls() + .filter(call => call.args[0] === 'peer:identify') + expect(identifyListens, 'should subscribe to peer:identify on the slow path').to.have.lengthOf.at.least(1) + }) + it('should register protocol handlers', () => { expect(components.libp2p.handle.called).to.be.true() expect(components.libp2p.register.calledWith(BITSWAP_120)).to.be.true()