From 8adab8aa020a2d0535e4dffa2f449d3413efc078 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <info@kchr.de>
Date: Sun, 25 Jun 2023 00:32:57 +0200
Subject: [PATCH] sc-network: Improve invalid boot node reporting

This improves the reporting of invalid boot nodes. First, it will only report each boot node once
as invalid and not every time we try to connect to the node. Second, the node will only report for
addresses that we added as startup and not for addresses of the boot node that the node learned from
other nodes.

Closes: https://github.com/paritytech/substrate/issues/13584
Closes: https://github.com/paritytech/polkadot/issues/7385
---
 client/network/src/service.rs | 42 +++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/client/network/src/service.rs b/client/network/src/service.rs
index 94927b5edf921..b6fda5ab079cd 100644
--- a/client/network/src/service.rs
+++ b/client/network/src/service.rs
@@ -29,7 +29,7 @@
 
 use crate::{
 	behaviour::{self, Behaviour, BehaviourOut},
-	config::{FullNetworkConfiguration, MultiaddrWithPeerId, Params, TransportConfig},
+	config::{parse_addr, FullNetworkConfiguration, MultiaddrWithPeerId, Params, TransportConfig},
 	discovery::DiscoveryConfig,
 	error::Error,
 	event::{DhtEvent, Event},
@@ -271,11 +271,14 @@ where
 		)?;
 
 		// List of multiaddresses that we know in the network.
-		let mut boot_node_ids = HashSet::new();
+		let mut boot_node_ids = HashMap::<PeerId, Vec<Multiaddr>>::new();
 
 		// Process the bootnodes.
 		for bootnode in network_config.boot_nodes.iter() {
-			boot_node_ids.insert(bootnode.peer_id);
+			boot_node_ids
+				.entry(bootnode.peer_id)
+				.or_default()
+				.push(bootnode.multiaddr.clone());
 			known_addresses.push((bootnode.peer_id, bootnode.multiaddr.clone()));
 		}
 
@@ -450,6 +453,7 @@ where
 			peers_notifications_sinks,
 			metrics,
 			boot_node_ids,
+			reported_invalid_boot_nodes: Default::default(),
 			_marker: Default::default(),
 			_block: Default::default(),
 		})
@@ -1144,8 +1148,10 @@ where
 	event_streams: out_events::OutChannels,
 	/// Prometheus network metrics.
 	metrics: Option<Metrics>,
-	/// The `PeerId`'s of all boot nodes.
-	boot_node_ids: Arc<HashSet<PeerId>>,
+	/// The `PeerId`'s of all boot nodes mapped to the registered addresses.
+	boot_node_ids: Arc<HashMap<PeerId, Vec<Multiaddr>>>,
+	/// Boot nodes that we already have reported as invalid.
+	reported_invalid_boot_nodes: HashSet<PeerId>,
 	/// For each peer and protocol combination, an object that allows sending notifications to
 	/// that peer. Shared with the [`NetworkService`].
 	peers_notifications_sinks: Arc<Mutex<HashMap<(PeerId, ProtocolName), NotificationsSink>>>,
@@ -1603,15 +1609,27 @@ where
 						peer_id, error,
 					);
 
-					if self.boot_node_ids.contains(&peer_id) {
+					let not_reported = !self.reported_invalid_boot_nodes.contains(&peer_id);
+
+					if let Some(addresses) =
+						not_reported.then(|| self.boot_node_ids.get(&peer_id)).flatten()
+					{
 						if let DialError::WrongPeerId { obtained, endpoint } = &error {
 							if let ConnectedPoint::Dialer { address, role_override: _ } = endpoint {
-								warn!(
-									"💔 The bootnode you want to connect to at `{}` provided a different peer ID `{}` than the one you expect `{}`.",
-									address,
-									obtained,
-									peer_id,
-								);
+								let address_without_peer_id = parse_addr(address.clone())
+									.map_or_else(|_| address.clone(), |r| r.1);
+
+								// Only report for address of boot node that was added at startup of
+								// the node and not for any address that the node learned of the
+								// boot node.
+								if addresses.iter().any(|a| address_without_peer_id == *a) {
+									warn!(
+										"💔 The bootnode you want to connect to at `{address}` provided a \
+										 different peer ID `{obtained}` than the one you expect `{peer_id}`.",
+									);
+
+									self.reported_invalid_boot_nodes.insert(peer_id);
+								}
 							}
 						}
 					}