Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
sc-network: Improve invalid boot node reporting (#14455)
Browse files Browse the repository at this point in the history
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: #13584
Closes: paritytech/polkadot#7385
  • Loading branch information
bkchr authored Jun 28, 2023
1 parent 9f6fecf commit aacc6ca
Showing 1 changed file with 30 additions and 12 deletions.
42 changes: 30 additions & 12 deletions client/network/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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()));
}

Expand Down Expand Up @@ -450,6 +453,7 @@ where
peers_notifications_sinks,
metrics,
boot_node_ids,
reported_invalid_boot_nodes: Default::default(),
_marker: Default::default(),
_block: Default::default(),
})
Expand Down Expand Up @@ -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>>>,
Expand Down Expand Up @@ -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);
}
}
}
}
Expand Down

0 comments on commit aacc6ca

Please sign in to comment.