Get rid of Peerset compatibility layer (#14337)

* Move bootnodes from individual `SetConfig`s to `PeersetConfig` * Move `SetId` & `SetConfig` from `peerset` to `protocol_controller` * Remove unused `DropReason` * Move `Message` & `IncomingIndex` from `peerset` to `protocol_controller` * Restore running fuzz test * Get rid of `Peerset` in `fuzz` test * Spawn runners instead of manual polling in `fuzz` test * Migrate `Protocol` from `Peerset` to `PeerStore` & `ProtocolController` * Migrate `NetworkService` from `Peerset` to `PeerStore` & `ProtocolController` * Migrate `Notifications` from `Peerset` to `ProtocolController`s * Migrate `Notifications` tests from `Peerset` to `ProtocolController` * Fix compilation of `NetworkService` & `Protocol` * Fix borrowing issues in `Notifications` * Migrate `RequestResponse`from `Peerset` to `PeerStore` * rustfmt * Migrate request-response tests from `Peerset` to `PeerStore` * Migrate `reconnect_after_disconnect` test to `PeerStore` & `ProtocolController` * Fix `Notifications` tests * Remove `Peerset` completely * Fix bug with counting sync peers in `Protocol` * Eliminate indirect calls to `PeerStore` via `Protocol` * Eliminate indirect calls to `ProtocolController` via `Protocol` * Handle `Err` outcome from `remove_peers_from_reserved_set` * Add note about disconnecting sync peers in `Protocol` * minor: remove unneeded `clone()` * minor: extra comma removed * minor: use `Stream` API of `from_protocol_controllers` channel * minor: remove TODO * minor: replace `.map().flatten()` with `.flat_map()` * minor: update `ProtocolController` docs * rustfmt * Apply suggestions from code review Co-authored-by: Aaro Altonen <[email protected]> * Extract `MockPeerStore` to `mock.rs` * Move `PeerStore` initialization to `build_network` * minor: remove unused import * minor: clarify error message * Convert `syncs_header_only_forks` test into single-threaded --------- Co-authored-by: Aaro Altonen <[email protected]>
paritytech · Aug 2, 2023 · 109e1d8 · 109e1d8
1 parent 4d9d911
commit 109e1d8
Show file tree

Hide file tree

Showing 27 changed files with 859 additions and 1,215 deletions.
diff --git a/client/consensus/grandpa/src/communication/tests.rs b/client/consensus/grandpa/src/communication/tests.rs
@@ -114,7 +114,13 @@ impl NetworkPeers for TestNetwork {
 		unimplemented!();
 	}
 
-	fn remove_peers_from_reserved_set(&self, _protocol: ProtocolName, _peers: Vec<PeerId>) {}
+	fn remove_peers_from_reserved_set(
+		&self,
+		_protocol: ProtocolName,
+		_peers: Vec<PeerId>,
+	) -> Result<(), String> {
+		unimplemented!();
+	}
 
 	fn sync_num_connected(&self) -> usize {
 		unimplemented!();

diff --git a/client/network-gossip/src/bridge.rs b/client/network-gossip/src/bridge.rs
@@ -238,10 +238,7 @@ impl<B: BlockT> Future for GossipEngine<B> {
 							SyncEvent::PeerConnected(remote) =>
 								this.network.add_set_reserved(remote, this.protocol.clone()),
 							SyncEvent::PeerDisconnected(remote) =>
-								this.network.remove_peers_from_reserved_set(
-									this.protocol.clone(),
-									vec![remote],
-								),
+								this.network.remove_set_reserved(remote, this.protocol.clone()),
 						},
 						// The sync event stream closed. Do the same for [`GossipValidator`].
 						Poll::Ready(None) => {
@@ -414,7 +411,13 @@ mod tests {
 			unimplemented!();
 		}
 
-		fn remove_peers_from_reserved_set(&self, _protocol: ProtocolName, _peers: Vec<PeerId>) {}
+		fn remove_peers_from_reserved_set(
+			&self,
+			_protocol: ProtocolName,
+			_peers: Vec<PeerId>,
+		) -> Result<(), String> {
+			unimplemented!();
+		}
 
 		fn sync_num_connected(&self) -> usize {
 			unimplemented!();

diff --git a/client/network-gossip/src/lib.rs b/client/network-gossip/src/lib.rs
@@ -89,6 +89,12 @@ pub trait Network<B: BlockT>: NetworkPeers + NetworkEventStream + NetworkNotific
 			log::error!(target: "gossip", "add_set_reserved failed: {}", err);
 		}
 	}
+	fn remove_set_reserved(&self, who: PeerId, protocol: ProtocolName) {
+		let result = self.remove_peers_from_reserved_set(protocol, iter::once(who).collect());
+		if let Err(err) = result {
+			log::error!(target: "gossip", "remove_set_reserved failed: {}", err);
+		}
+	}
 }
 
 impl<T, B: BlockT> Network<B> for T where T: NetworkPeers + NetworkEventStream + NetworkNotification {}

diff --git a/client/network-gossip/src/state_machine.rs b/client/network-gossip/src/state_machine.rs
@@ -640,7 +640,13 @@ mod tests {
 			unimplemented!();
 		}
 
-		fn remove_peers_from_reserved_set(&self, _protocol: ProtocolName, _peers: Vec<PeerId>) {}
+		fn remove_peers_from_reserved_set(
+			&self,
+			_protocol: ProtocolName,
+			_peers: Vec<PeerId>,
+		) -> Result<(), String> {
+			unimplemented!();
+		}
 
 		fn sync_num_connected(&self) -> usize {
 			unimplemented!();

diff --git a/client/network/src/behaviour.rs b/client/network/src/behaviour.rs
@@ -20,7 +20,7 @@ use crate::{
 	discovery::{DiscoveryBehaviour, DiscoveryConfig, DiscoveryOut},
 	event::DhtEvent,
 	peer_info,
-	peerset::PeersetHandle,
+	peer_store::PeerStoreHandle,
 	protocol::{CustomMessageOutcome, NotificationsSink, Protocol},
 	request_responses::{self, IfDisconnected, ProtocolConfig, RequestFailure},
 	types::ProtocolName,
@@ -173,7 +173,7 @@ impl<B: BlockT> Behaviour<B> {
 		local_public_key: PublicKey,
 		disco_config: DiscoveryConfig,
 		request_response_protocols: Vec<ProtocolConfig>,
-		peerset: PeersetHandle,
+		peer_store_handle: PeerStoreHandle,
 		connection_limits: ConnectionLimits,
 		external_addresses: Arc<Mutex<HashSet<Multiaddr>>>,
 	) -> Result<Self, request_responses::RegisterError> {
@@ -188,7 +188,7 @@ impl<B: BlockT> Behaviour<B> {
 			connection_limits: libp2p::connection_limits::Behaviour::new(connection_limits),
 			request_responses: request_responses::RequestResponsesBehaviour::new(
 				request_response_protocols.into_iter(),
-				peerset,
+				Box::new(peer_store_handle),
 			)?,
 		})
 	}

diff --git a/client/network/src/config.rs b/client/network/src/config.rs
@@ -32,6 +32,7 @@ pub use crate::{
 
 pub use libp2p::{identity::Keypair, multiaddr, Multiaddr, PeerId};
 
+use crate::peer_store::PeerStoreHandle;
 use codec::Encode;
 use prometheus_endpoint::Registry;
 use zeroize::Zeroize;
@@ -270,6 +271,11 @@ impl NonReservedPeerMode {
 			_ => None,
 		}
 	}
+
+	/// If we are in "reserved-only" peer mode.
+	pub fn is_reserved_only(&self) -> bool {
+		matches!(self, NonReservedPeerMode::Deny)
+	}
 }
 
 /// The configuration of a node's secret key, describing the type of key
@@ -674,6 +680,9 @@ pub struct Params<Block: BlockT> {
 	/// Network layer configuration.
 	pub network_config: FullNetworkConfiguration,
 
+	/// Peer store with known nodes, peer reputations, etc.
+	pub peer_store: PeerStoreHandle,
+
 	/// Legacy name of the protocol to use on the wire. Should be different for each chain.
 	pub protocol_id: ProtocolId,
 

diff --git a/client/network/src/lib.rs b/client/network/src/lib.rs
@@ -243,18 +243,20 @@
 //! More precise usage details are still being worked on and will likely change in the future.
 
 mod behaviour;
-mod peer_store;
-mod peerset;
 mod protocol;
-mod protocol_controller;
 mod service;
 
+#[cfg(test)]
+mod mock;
+
 pub mod config;
 pub mod discovery;
 pub mod error;
 pub mod event;
 pub mod network_state;
 pub mod peer_info;
+pub mod peer_store;
+pub mod protocol_controller;
 pub mod request_responses;
 pub mod transport;
 pub mod types;

diff --git a/client/network/src/mock.rs b/client/network/src/mock.rs
@@ -0,0 +1,55 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
+
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+//! Mocked components for tests.
+
+use crate::{peer_store::PeerStoreProvider, protocol_controller::ProtocolHandle, ReputationChange};
+use libp2p::PeerId;
+use std::collections::HashSet;
+
+/// No-op `PeerStore`.
+#[derive(Debug)]
+pub struct MockPeerStore {}
+
+impl PeerStoreProvider for MockPeerStore {
+	fn is_banned(&self, _peer_id: &PeerId) -> bool {
+		// Make sure that the peer is not banned.
+		false
+	}
+
+	fn register_protocol(&self, _protocol_handle: ProtocolHandle) {
+		// Make sure not to fail.
+	}
+
+	fn report_disconnect(&mut self, _peer_id: PeerId) {
+		// Make sure not to fail.
+	}
+
+	fn report_peer(&mut self, _peer_id: PeerId, _change: ReputationChange) {
+		// Make sure not to fail.
+	}
+
+	fn peer_reputation(&self, _peer_id: &PeerId) -> i32 {
+		// Make sure that the peer is not banned.
+		0
+	}
+
+	fn outgoing_candidates(&self, _count: usize, _ignored: HashSet<&PeerId>) -> Vec<PeerId> {
+		unimplemented!()
+	}
+}
diff --git a/client/network/src/peer_store.rs b/client/network/src/peer_store.rs
@@ -16,6 +16,9 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
+//! [`PeerStore`] manages peer reputations and provides connection candidates to
+//! [`crate::protocol_controller::ProtocolController`].
+
 use libp2p::PeerId;
 use log::trace;
 use parking_lot::Mutex;
@@ -49,6 +52,7 @@ const INVERSE_DECREMENT: i32 = 50;
 /// remove it, once the reputation value reaches 0.
 const FORGET_AFTER: Duration = Duration::from_secs(3600);
 
+/// Trait providing peer reputation management and connection candidates.
 pub trait PeerStoreProvider: Debug + Send {
 	/// Check whether the peer is banned.
 	fn is_banned(&self, peer_id: &PeerId) -> bool;
@@ -69,6 +73,7 @@ pub trait PeerStoreProvider: Debug + Send {
 	fn outgoing_candidates(&self, count: usize, ignored: HashSet<&PeerId>) -> Vec<PeerId>;
 }
 
+/// Actual implementation of peer reputations and connection candidates provider.
 #[derive(Debug, Clone)]
 pub struct PeerStoreHandle {
 	inner: Arc<Mutex<PeerStoreInner>>,
@@ -289,6 +294,7 @@ impl PeerStoreInner {
 	}
 }
 
+/// Worker part of [`PeerStoreHandle`]
 #[derive(Debug)]
 pub struct PeerStore {
 	inner: Arc<Mutex<PeerStoreInner>>,