fix: only update pending incoming connections if connection accepted (#2790)

* fix: only update pending incoming connections if connection accepted

The `afterUpgradeInbound` function decrements the count of pending
(e.g. not upgraded yet) incoming connections, so only invoke that
method if we actually accepted the connection, otherwise we decrement
a counter that was never incremented.

* chore: helpful to include implementation

Author: achingbrain

packages/libp2p/src/upgrader.ts

@@ -182,30 +182,32 @@ export class DefaultUpgrader implements Upgrader {
    * Upgrades an inbound connection
    */
   async upgradeInbound (maConn: MultiaddrConnection, opts: UpgraderOptions = {}): Promise<Connection> {
+    let accepted = false
+
     try {
       this.metrics.dials?.increment({
         inbound: true
       })
 
-      const accept = await this.components.connectionManager.acceptIncomingConnection(maConn)
+      accepted = await this.components.connectionManager.acceptIncomingConnection(maConn)
 
-      if (!accept) {
+      if (!accepted) {
         throw new ConnectionDeniedError('connection denied')
       }
 
       await this.shouldBlockConnection('denyInboundConnection', maConn)
 
-      const conn = await this._performUpgrade(maConn, 'inbound', opts)
-
-      return conn
+      return await this._performUpgrade(maConn, 'inbound', opts)
     } catch (err) {
       this.metrics.errors?.increment({
         inbound: true
       })
 
       throw err
     } finally {
-      this.components.connectionManager.afterUpgradeInbound()
+      if (accepted) {
+        this.components.connectionManager.afterUpgradeInbound()
+      }
     }
   }
 

packages/libp2p/test/upgrading/upgrader.spec.ts

@@ -29,7 +29,8 @@ import { type Components, defaultComponents } from '../../src/components.js'
 import { createLibp2p } from '../../src/index.js'
 import { DEFAULT_MAX_OUTBOUND_STREAMS } from '../../src/registrar.js'
 import { DefaultUpgrader } from '../../src/upgrader.js'
-import type { Libp2p, Connection, ConnectionProtector, Stream, ConnectionEncrypter, SecuredConnection, PeerId, StreamMuxer, StreamMuxerFactory, StreamMuxerInit, Upgrader, PrivateKey } from '@libp2p/interface'
+import type { Libp2p, Connection, ConnectionProtector, Stream, ConnectionEncrypter, SecuredConnection, PeerId, StreamMuxer, StreamMuxerFactory, StreamMuxerInit, Upgrader, PrivateKey, MultiaddrConnection } from '@libp2p/interface'
+import type { ConnectionManager } from '@libp2p/interface-internal'
 
 const addrs = [
   multiaddr('/ip4/127.0.0.1/tcp/0'),
@@ -596,6 +597,22 @@ describe('Upgrader', () => {
     expect(localConnectionProtector.protect.callCount).to.equal(0, 'used local connection protector')
     expect(remoteConnectionProtector.protect.callCount).to.equal(0, 'used remote connection protector')
   })
+
+  it('should not decrement inbound pending connection count if the connection is denied', async () => {
+    const connectionManager = stubInterface<ConnectionManager>()
+
+    // @ts-expect-error private field
+    localUpgrader.components.connectionManager = connectionManager
+
+    const maConn = stubInterface<MultiaddrConnection>()
+
+    connectionManager.acceptIncomingConnection.resolves(false)
+
+    await expect(localUpgrader.upgradeInbound(maConn)).to.eventually.be.rejected
+      .with.property('name', 'ConnectionDeniedError')
+
+    expect(connectionManager.afterUpgradeInbound.called).to.be.false()
+  })
 })
 
 describe('libp2p.upgrader', () => {