From 7536de97b132aa3e99108a4b98d392554ef55ef9 Mon Sep 17 00:00:00 2001
From: Pierre Krieger <pierre.krieger1708@gmail.com>
Date: Mon, 18 May 2020 18:42:44 +0200
Subject: [PATCH] Print an error if we discover our own network identity
(#6047)
* Add an error if we discover our own network identity
* Fix tests
---
substrate/client/network/src/protocol.rs | 10 +++++++++-
.../src/protocol/generic_proto/behaviour.rs | 18 ++++++++++++++++--
.../src/protocol/generic_proto/tests.rs | 3 ++-
substrate/client/network/src/service.rs | 1 +
4 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/substrate/client/network/src/protocol.rs b/substrate/client/network/src/protocol.rs
index a245659ee05..421035e5705 100644
--- a/substrate/client/network/src/protocol.rs
+++ b/substrate/client/network/src/protocol.rs
@@ -363,6 +363,7 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
/// Create a new instance.
pub fn new(
config: ProtocolConfig,
+ local_peer_id: PeerId,
chain: Arc<dyn Client<B>>,
transaction_pool: Arc<dyn TransactionPool<H, B>>,
finality_proof_provider: Option<Arc<dyn FinalityProofProvider<B>>>,
@@ -396,7 +397,13 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
let (peerset, peerset_handle) = sc_peerset::Peerset::from_config(peerset_config);
let versions = &((MIN_VERSION as u8)..=(CURRENT_VERSION as u8)).collect::<Vec<u8>>();
- let mut behaviour = GenericProto::new(protocol_id.clone(), versions, peerset, queue_size_report);
+ let mut behaviour = GenericProto::new(
+ local_peer_id,
+ protocol_id.clone(),
+ versions,
+ peerset,
+ queue_size_report
+ );
let mut legacy_equiv_by_name = HashMap::new();
@@ -2193,6 +2200,7 @@ mod tests {
let (mut protocol, _) = Protocol::<Block, Hash>::new(
ProtocolConfig::default(),
+ PeerId::random(),
client.clone(),
Arc::new(EmptyTransactionPool),
None,
diff --git a/substrate/client/network/src/protocol/generic_proto/behaviour.rs b/substrate/client/network/src/protocol/generic_proto/behaviour.rs
index 4984c0d86d9..32cf417ec49 100644
--- a/substrate/client/network/src/protocol/generic_proto/behaviour.rs
+++ b/substrate/client/network/src/protocol/generic_proto/behaviour.rs
@@ -109,6 +109,9 @@ use wasm_timer::Instant;
/// tries to connect, the connection is accepted. A ban only delays dialing attempts.
///
pub struct GenericProto {
+ /// `PeerId` of the local node.
+ local_peer_id: PeerId,
+
/// Legacy protocol to open with peers. Never modified.
legacy_protocol: RegisteredProtocol,
@@ -321,6 +324,7 @@ impl GenericProto {
/// The `queue_size_report` is an optional Prometheus metric that can report the size of the
/// messages queue. If passed, it must have one label for the protocol name.
pub fn new(
+ local_peer_id: PeerId,
protocol: impl Into<ProtocolId>,
versions: &[u8],
peerset: sc_peerset::Peerset,
@@ -329,6 +333,7 @@ impl GenericProto {
let legacy_protocol = RegisteredProtocol::new(protocol, versions);
GenericProto {
+ local_peer_id,
legacy_protocol,
notif_protocols: Vec::new(),
peerset,
@@ -507,9 +512,18 @@ impl GenericProto {
///
/// Can be called multiple times with the same `PeerId`s.
pub fn add_discovered_nodes(&mut self, peer_ids: impl Iterator<Item = PeerId>) {
- self.peerset.discovered(peer_ids.map(|peer_id| {
+ let local_peer_id = &self.local_peer_id;
+ self.peerset.discovered(peer_ids.filter_map(|peer_id| {
+ if peer_id == *local_peer_id {
+ error!(
+ target: "sub-libp2p",
+ "Discovered our own identity. This is a minor inconsequential bug."
+ );
+ return None;
+ }
+
debug!(target: "sub-libp2p", "PSM <= Discovered({:?})", peer_id);
- peer_id
+ Some(peer_id)
}));
}
diff --git a/substrate/client/network/src/protocol/generic_proto/tests.rs b/substrate/client/network/src/protocol/generic_proto/tests.rs
index 1bc6e745f88..de02ac5f346 100644
--- a/substrate/client/network/src/protocol/generic_proto/tests.rs
+++ b/substrate/client/network/src/protocol/generic_proto/tests.rs
@@ -42,6 +42,7 @@ fn build_nodes() -> (Swarm<CustomProtoWithAddr>, Swarm<CustomProtoWithAddr>) {
for index in 0 .. 2 {
let keypair = keypairs[index].clone();
+ let local_peer_id = keypair.public().into_peer_id();
let transport = libp2p::core::transport::MemoryTransport
.and_then(move |out, endpoint| {
let secio = libp2p::secio::SecioConfig::new(keypair);
@@ -82,7 +83,7 @@ fn build_nodes() -> (Swarm<CustomProtoWithAddr>, Swarm<CustomProtoWithAddr>) {
});
let behaviour = CustomProtoWithAddr {
- inner: GenericProto::new(&b"test"[..], &[1], peerset, None),
+ inner: GenericProto::new(local_peer_id, &b"test"[..], &[1], peerset, None),
addrs: addrs
.iter()
.enumerate()
diff --git a/substrate/client/network/src/service.rs b/substrate/client/network/src/service.rs
index 132f6be7491..4b05c26335b 100644
--- a/substrate/client/network/src/service.rs
+++ b/substrate/client/network/src/service.rs
@@ -204,6 +204,7 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> {
roles: From::from(¶ms.role),
max_parallel_downloads: params.network_config.max_parallel_downloads,
},
+ local_peer_id.clone(),
params.chain.clone(),
params.transaction_pool,
params.finality_proof_provider.clone(),
--
GitLab