fix: accept two incoming PING streams per peer (#1617)

Modify the default configuration for the PING protocol to accept at most two streams per peer, as recommended in the PING protocol spec. Signed-off-by: Miroslav Bajtoš <[email protected]>
libp2p · Mar 9, 2023 · afaee4c · afaee4c
1 parent 4e01f48
commit afaee4c
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 1 deletion.
diff --git a/src/config.ts b/src/config.ts
@@ -79,7 +79,13 @@ const DefaultConfig: Partial<Libp2pInit> = {
   },
   ping: {
     protocolPrefix: 'ipfs',
-    maxInboundStreams: 1,
+    // See https://github.com/libp2p/specs/blob/d4b5fb0152a6bb86cfd9ea/ping/ping.md?plain=1#L38-L43
+    // The dialing peer MUST NOT keep more than one outbound stream for the ping protocol per peer.
+    // The listening peer SHOULD accept at most two streams per peer since cross-stream behavior is
+    // non-linear and stream writes occur asynchronously. The listening peer may perceive the
+    // dialing peer closing and opening the wrong streams (for instance, closing stream B and
+    // opening stream A even though the dialing peer is opening stream B and closing stream A).
+    maxInboundStreams: 2,
     maxOutboundStreams: 1,
     timeout: 10000
   },

diff --git a/test/ping/ping.node.ts b/test/ping/ping.node.ts
@@ -75,4 +75,34 @@ describe('ping', () => {
 
     defer.resolve()
   })
+
+  it('allows two incoming streams from the same peer', async () => {
+    const remote = nodes[0]
+    const client = await createNode({
+      config: createBaseOptions({
+        ping: {
+        // Allow two outbound ping streams.
+        // It is not allowed by the spec, but this test needs to open two concurrent streams.
+          maxOutboundStreams: 2
+        }
+      })
+    })
+    await client.components.peerStore.addressBook.set(remote.peerId, remote.getMultiaddrs())
+    // register our new node for shutdown after the test finishes
+    // otherwise the Mocha/Node.js process never finishes
+    nodes.push(client)
+
+    // Send two ping requests in parallel, this should open two concurrent streams
+    const results = await Promise.allSettled([
+      client.ping(remote.peerId),
+      client.ping(remote.peerId)
+    ])
+
+    // Verify that the remote peer accepted both inbound streams
+    expect(results.map(describe)).to.deep.equal(['fulfilled', 'fulfilled'])
+
+    function describe (result: PromiseSettledResult<number>): string {
+      return result.status === 'fulfilled' ? result.status : result.reason ?? result.status
+    }
+  })
 })