Fix removal of disconnected peers in the peer manager (#1182)

* Explicitly end underlying connection * Fire 'ProtocolStopped' event in any connection-end case * Count dial attempts and signal 'PeerUnreachable' * Temporarly show peer count for gossip-peerlist and protocol-peer_ manager * Display error message when sending the shutdown signal to an autopeering task fails * Remove unreachable discovered peers from peer manager * Remove unreachable discovered peers from gossip and peer manager * Publish PeerUnreachable for peers that don't identify properly; Change some logs; * Reset dial attempt counter only if peer successfully identified itself * Remove temporary logs * Alias macro starts with the significant part of peer id * Send PeerRemoved event for disconnected unknown peers * Alias macro starts with the significant part of peer id (for real) * Align autopeering peer id display * Ignore error if adding a local address fails * Change some log levels * Format; Introduce display offset constant, Update autopeering changelog * Update changelogs; bump versions * Nits * Fix typos and German comma in changelog Co-authored-by: Jochen Görtler <grtlr@users.noreply.github.com> * Fix docs * Address review comments * Simplify 'add_local_addr' method * Add explanation why sending commands to gossip layer cannot fail * Rename to 'identified_at' Co-authored-by: Jochen Görtler <grtlr@users.noreply.github.com>
iotaledger · Mar 2, 2022 · c3f2819 · c3f2819
1 parent 6b4e102
commit c3f2819
Show file tree

Hide file tree

Showing 22 changed files with 369 additions and 188 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bee-api/bee-rest-api/CHANGELOG.md b/bee-api/bee-rest-api/CHANGELOG.md
@@ -19,7 +19,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Security -->
 
-## 0.2.0 - 2022-XX-XX
+## 0.2.1 - 2022-02-28
+
+- Update `bee-gossip` dependency to 0.5.0;
+
+## 0.2.0 - 2022-01-28
 
 ### Added
 

diff --git a/bee-api/bee-rest-api/Cargo.toml b/bee-api/bee-rest-api/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "bee-rest-api"
-version = "0.2.0"
+version = "0.2.1"
 authors = [ "IOTA Stiftung" ]
 edition = "2021"
 description = "The default REST API implementation for the IOTA Bee node software."
@@ -12,7 +12,7 @@ homepage = "https://www.iota.org"
 
 [dependencies]
 bee-common = { version = "0.6.0", path = "../../bee-common/bee-common", default-features = false, optional = true }
-bee-gossip = { version = "0.4.0", path = "../../bee-network/bee-gossip", default-features = false, optional = true }
+bee-gossip = { version = "0.5.0", path = "../../bee-network/bee-gossip", default-features = false, optional = true }
 bee-ledger = { version = "0.6.1", path = "../../bee-ledger", default-features = false }
 bee-message = { version = "0.1.6", path = "../../bee-message", default-features = false }
 bee-pow = { version = "0.2.0", path = "../../bee-pow", default-features = false }

diff --git a/bee-network/bee-autopeering/CHANGELOG.md b/bee-network/bee-autopeering/CHANGELOG.md
@@ -19,6 +19,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Security -->
 
+## 0.4.1 - 2022-02-28
+
+### Changed
+
+- Displayed representation of `PeerId`s to start at the beginning of its significant part;
+
 ## 0.4.0 - 2022-02-11
 
 ### Added

diff --git a/bee-network/bee-autopeering/Cargo.toml b/bee-network/bee-autopeering/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "bee-autopeering"
-version = "0.4.0"
+version = "0.4.1"
 authors = [ "IOTA Stiftung" ]
 edition = "2021"
 description = "Allows peers in the same IOTA network to automatically discover each other."

diff --git a/bee-network/bee-autopeering/src/peer/peer_id.rs b/bee-network/bee-autopeering/src/peer/peer_id.rs
@@ -18,6 +18,7 @@ use std::{
 };
 
 const DISPLAY_LENGTH: usize = 16;
+const DISPLAY_OFFSET: usize = 8;
 
 /// Represents the unique identity of a peer in the network.
 #[derive(Copy, Clone)]
@@ -98,7 +99,7 @@ impl fmt::Debug for PeerId {
 
 impl fmt::Display for PeerId {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.libp2p_peer_id().to_base58()[..DISPLAY_LENGTH].fmt(f)
+        self.libp2p_peer_id().to_base58()[DISPLAY_OFFSET..DISPLAY_OFFSET + DISPLAY_LENGTH].fmt(f)
     }
 }
 

diff --git a/bee-network/bee-autopeering/src/task.rs b/bee-network/bee-autopeering/src/task.rs
@@ -117,7 +117,10 @@ impl<S: PeerStore> TaskManager<S> {
             let shutdown_tx = shutdown_senders.remove(&task_name).unwrap();
 
             log::trace!("Shutting down: {}", task_name);
-            shutdown_tx.send(()).expect("error sending shutdown signal");
+
+            if shutdown_tx.send(()).is_err() {
+                log::error!("Error sending shutdown signal to task {task_name}.");
+            }
         }
 
         // Wait for all tasks to shutdown down in a certain order and maximum amount of time.

diff --git a/bee-network/bee-gossip/CHANGELOG.md b/bee-network/bee-gossip/CHANGELOG.md
@@ -19,6 +19,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Security -->
 
+## 0.5.0 - 2022-02-28
+
+### Added
+
+- `PeerMetrics` type that keeps a count of dial attempts and identification timestamp;
+- `PeerUnreachable` event that is fired after a certain number of unsuccessful dial attempts;
+
+### Changed
+
+- `PeerList` type keeps metrics for each peer;
+
+### Fixed
+
+- Missing `PeerRemoved` event for disconnected unknown peers;
+
 ## 0.4.0 - 2022-01-20
 
 ### Fixed

diff --git a/bee-network/bee-gossip/Cargo.toml b/bee-network/bee-gossip/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "bee-gossip"
-version = "0.4.0"
+version = "0.5.0"
 authors = [ "IOTA Stiftung" ]
 edition = "2021"
 description = "Allows peers in the same IOTA network to exchange gossip messages with each other."

diff --git a/bee-network/bee-gossip/src/alias.rs b/bee-network/bee-gossip/src/alias.rs
@@ -7,9 +7,9 @@
 #[macro_export]
 macro_rules! alias {
     ($peer_id:expr) => {
-        &$peer_id.to_base58()[46..]
+        &$peer_id.to_base58()[8..24]
     };
     ($peer_id:expr, $len:expr) => {
-        &$peer_id.to_base58()[(52 - $len)..]
+        &$peer_id.to_base58()[8..8 + $len]
     };
 }
diff --git a/bee-network/bee-gossip/src/network/host.rs b/bee-network/bee-gossip/src/network/host.rs
@@ -143,12 +143,8 @@ async fn process_swarm_event(
                 })
                 .expect("send error");
 
-            peerlist
-                .0
-                .write()
-                .await
-                .insert_local_addr(address)
-                .expect("insert_local_addr");
+            // Note: We don't care if the inserted address is a duplicate.
+            let _ = peerlist.0.write().await.add_local_addr(address);
         }
         SwarmEvent::ConnectionEstablished { peer_id, .. } => {
             debug!("Swarm event: connection established with {}.", alias!(peer_id));
@@ -182,6 +178,7 @@ async fn process_internal_command(internal_command: Command, swarm: &mut Swarm<S
                 warn!("Dialing peer {} failed. Cause: {}", alias!(peer_id), e);
             }
         }
+        Command::DisconnectPeer { peer_id } => hang_up(swarm, peer_id).await,
         _ => {}
     }
 }
@@ -209,13 +206,36 @@ async fn dial_peer(swarm: &mut Swarm<SwarmBehaviour>, peer_id: PeerId, peerlist:
     // We just checked, that the peer is fine to be dialed.
     let PeerInfo {
         address: addr, alias, ..
-    } = peerlist.0.read().await.info(&peer_id).unwrap();
-
-    debug!("Dialing peer: {} ({}).", alias, alias!(peer_id));
+    } = peerlist.0.read().await.info(&peer_id).expect("peer must exist");
+
+    let mut dial_attempt = 0;
+
+    peerlist
+        .0
+        .write()
+        .await
+        .update_metrics(&peer_id, |m| {
+            m.num_dials += 1;
+            dial_attempt = m.num_dials;
+        })
+        .expect("peer must exist");
+
+    debug!(
+        "Dialing peer: {} ({}) attempt: #{}.",
+        alias,
+        alias!(peer_id),
+        dial_attempt
+    );
 
     // TODO: We also use `Swarm::dial_addr` here (instead of `Swarm::dial`) for now. See if it's better to change
     // that.
     Swarm::dial_addr(swarm, addr).map_err(|e| Error::DialingPeerFailed(peer_id, e))?;
 
     Ok(())
 }
+
+async fn hang_up(swarm: &mut Swarm<SwarmBehaviour>, peer_id: PeerId) {
+    debug!("Hanging up on: {}.", alias!(peer_id));
+
+    let _ = Swarm::disconnect_peer_id(swarm, peer_id);
+}