feat(core)!: make ConnectionIds globally unique

.github/workflows/ci.yml

@@ -298,11 +298,12 @@ jobs:
           require_scope: false
 
       - name: Check PR title length
+        env:
+          TITLE: ${{ github.event.pull_request.title }}
         run: |
-          title="${{ github.event.pull_request.title }}"
-          title_length=${#title}
+          title_length=${#TITLE}
           if [ $title_length -gt 72 ]
           then
             echo "PR title is too long (greater than 72 characters)"
             exit 1
-          fi
+          fi

core/CHANGELOG.md

@@ -1,3 +1,9 @@
+# 0.39.0 [unreleased]
+
+- Deprecate `ConnectionId::new` in favor of `ConnectionId::next`. See [PR 3327].
+
+[PR 3327]: https://github.com/libp2p/rust-libp2p/pull/3327
+
 # 0.38.0
 
 - Remove deprecated functions `StreamMuxerExt::next_{inbound,outbound}`. See [PR 3031].

core/src/connection.rs

@@ -19,6 +19,9 @@
 // DEALINGS IN THE SOFTWARE.
 
 use crate::multiaddr::{Multiaddr, Protocol};
+use std::sync::atomic::{AtomicUsize, Ordering};
+
+static NEXT_CONNECTION_ID: AtomicUsize = AtomicUsize::new(0);
 
 /// Connection identifier.
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
@@ -30,16 +33,14 @@ impl ConnectionId {
     /// This is primarily useful for creating connection IDs
     /// in test environments. There is in general no guarantee
     /// that all connection IDs are based on non-negative integers.
+    #[deprecated(since = "0.39.0", note = "Use `ConnectionId::next` instead.")]
     pub fn new(id: usize) -> Self {
         Self(id)
     }
-}
-
-impl std::ops::Add<usize> for ConnectionId {
-    type Output = Self;
 
-    fn add(self, other: usize) -> Self {
-        Self(self.0 + other)
+    /// Returns the next available [`ConnectionId`].
+    pub fn next() -> Self {
+        Self(NEXT_CONNECTION_ID.fetch_add(1, Ordering::SeqCst))
     }
 }
 

protocols/gossipsub/src/behaviour/tests.rs

@@ -225,13 +225,13 @@ where
 
     gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
         peer_id: peer,
-        connection_id: ConnectionId::new(0),
+        connection_id: ConnectionId::next(),
         endpoint: &endpoint,
         failed_addresses: &[],
         other_established: 0, // first connection
     }));
     if let Some(kind) = kind {
-        gs.on_connection_handler_event(peer, ConnectionId::new(1), HandlerEvent::PeerKind(kind));
+        gs.on_connection_handler_event(peer, ConnectionId::next(), HandlerEvent::PeerKind(kind));
     }
     if explicit {
         gs.add_explicit_peer(&peer);
@@ -579,7 +579,7 @@ fn test_join() {
         // inform the behaviour of a new peer
         gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
             peer_id: random_peer,
-            connection_id: ConnectionId::new(1),
+            connection_id: ConnectionId::next(),
             endpoint: &ConnectedPoint::Dialer {
                 address: "/ip4/127.0.0.1".parse::<Multiaddr>().unwrap(),
                 role_override: Endpoint::Dialer,
@@ -959,7 +959,7 @@ fn test_get_random_peers() {
                 *p,
                 PeerConnections {
                     kind: PeerKind::Gossipsubv1_1,
-                    connections: vec![ConnectionId::new(1)],
+                    connections: vec![ConnectionId::next()],
                 },
             )
         })
@@ -3009,7 +3009,7 @@ fn test_ignore_rpc_from_peers_below_graylist_threshold() {
     //receive from p1
     gs.on_connection_handler_event(
         p1,
-        ConnectionId::new(0),
+        ConnectionId::next(),
         HandlerEvent::Message {
             rpc: GossipsubRpc {
                 messages: vec![raw_message1],
@@ -3035,7 +3035,7 @@ fn test_ignore_rpc_from_peers_below_graylist_threshold() {
     //receive from p2
     gs.on_connection_handler_event(
         p2,
-        ConnectionId::new(0),
+        ConnectionId::next(),
         HandlerEvent::Message {
             rpc: GossipsubRpc {
                 messages: vec![raw_message3],
@@ -3647,7 +3647,7 @@ fn test_scoring_p4_invalid_signature() {
 
     gs.on_connection_handler_event(
         peers[0],
-        ConnectionId::new(0),
+        ConnectionId::next(),
         HandlerEvent::Message {
             rpc: GossipsubRpc {
                 messages: vec![],
@@ -4131,7 +4131,7 @@ fn test_scoring_p6() {
     for id in others.iter().take(3) {
         gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
             peer_id: *id,
-            connection_id: ConnectionId::new(0),
+            connection_id: ConnectionId::next(),
             endpoint: &ConnectedPoint::Dialer {
                 address: addr.clone(),
                 role_override: Endpoint::Dialer,
@@ -4152,7 +4152,7 @@ fn test_scoring_p6() {
     for peer in peers.iter().take(3) {
         gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
             peer_id: *peer,
-            connection_id: ConnectionId::new(0),
+            connection_id: ConnectionId::next(),
             endpoint: &ConnectedPoint::Dialer {
                 address: addr2.clone(),
                 role_override: Endpoint::Dialer,
@@ -4182,7 +4182,7 @@ fn test_scoring_p6() {
     //two times same ip doesn't count twice
     gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
         peer_id: peers[0],
-        connection_id: ConnectionId::new(0),
+        connection_id: ConnectionId::next(),
         endpoint: &ConnectedPoint::Dialer {
             address: addr,
             role_override: Endpoint::Dialer,
@@ -5194,7 +5194,7 @@ fn test_subscribe_and_graft_with_negative_score() {
 
     let (mut gs2, _, _) = inject_nodes1().create_network();
 
-    let connection_id = ConnectionId::new(0);
+    let connection_id = ConnectionId::next();
 
     let topic = Topic::new("test");
 

protocols/kad/src/behaviour/test.rs

@@ -1294,7 +1294,7 @@ fn network_behaviour_on_address_change() {
     let local_peer_id = PeerId::random();
 
     let remote_peer_id = PeerId::random();
-    let connection_id = ConnectionId::new(1);
+    let connection_id = ConnectionId::next();
     let old_address: Multiaddr = Protocol::Memory(1).into();
     let new_address: Multiaddr = Protocol::Memory(2).into();
 

swarm/src/connection/pool.rs

@@ -100,9 +100,6 @@ where
     /// The pending connections that are currently being negotiated.
     pending: HashMap<ConnectionId, PendingConnection<THandler>>,
 
-    /// Next available identifier for a new connection / task.
-    next_connection_id: ConnectionId,
-
     /// Size of the task command buffer (per task).
     task_command_buffer_size: usize,
 
@@ -326,7 +323,6 @@ where
             counters: ConnectionCounters::new(limits),
             established: Default::default(),
             pending: Default::default(),
-            next_connection_id: ConnectionId::new(0),
             task_command_buffer_size: config.task_command_buffer_size,
             dial_concurrency_factor: config.dial_concurrency_factor,
             substream_upgrade_protocol_override: config.substream_upgrade_protocol_override,
@@ -412,13 +408,6 @@ where
         self.established.keys()
     }
 
-    fn next_connection_id(&mut self) -> ConnectionId {
-        let connection_id = self.next_connection_id;
-        self.next_connection_id = self.next_connection_id + 1;
-
-        connection_id
-    }
-
     fn spawn(&mut self, task: BoxFuture<'static, ()>) {
         self.executor.spawn(task)
     }
@@ -458,7 +447,7 @@ where
             dial_concurrency_factor_override.unwrap_or(self.dial_concurrency_factor),
         );
 
-        let connection_id = self.next_connection_id();
+        let connection_id = ConnectionId::next();
 
         let (abort_notifier, abort_receiver) = oneshot::channel();
 
@@ -508,7 +497,7 @@ where
             return Err((limit, handler));
         }
 
-        let connection_id = self.next_connection_id();
+        let connection_id = ConnectionId::next();
 
         let (abort_notifier, abort_receiver) = oneshot::channel();