Support querying peer reputation (#2392)

# Description

Trivial change that resolves
https://github.com/paritytech/polkadot-sdk/issues/2185.

Since there was a mix of `who` and `peer_id` argument names nearby I
changed them all to `peer_id`.

# Checklist

- [x] My PR includes a detailed description as outlined in the
"Description" section above
- [x] My PR follows the [labeling requirements](CONTRIBUTING.md#Process)
of this project (at minimum one label for `T`
  required)
- [x] I have made corresponding changes to the documentation (if
applicable)
- [ ] I have added tests that prove my fix is effective or that my
feature works (if applicable)

---------

Co-authored-by: Bastian Köcher <git@kchr.de>

substrate/client/consensus/grandpa/src/communication/tests.rs

@@ -75,11 +75,15 @@ impl NetworkPeers for TestNetwork {
 		unimplemented!();
 	}
 
-	fn report_peer(&self, who: PeerId, cost_benefit: ReputationChange) {
+	fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange) {
-		let _ = self.sender.unbounded_send(Event::Report(who, cost_benefit));
+		let _ = self.sender.unbounded_send(Event::Report(peer_id, cost_benefit));
 	}
 
-	fn disconnect_peer(&self, _who: PeerId, _protocol: ProtocolName) {}
+	fn peer_reputation(&self, _peer_id: &PeerId) -> i32 {
+		unimplemented!()
+	}
+
+	fn disconnect_peer(&self, _peer_id: PeerId, _protocol: ProtocolName) {}
 
 	fn accept_unreserved_peers(&self) {
 		unimplemented!();

substrate/client/network-gossip/src/bridge.rs

@@ -394,9 +394,13 @@ mod tests {
 			unimplemented!();
 		}
 
-		fn report_peer(&self, _who: PeerId, _cost_benefit: ReputationChange) {}
+		fn report_peer(&self, _peer_id: PeerId, _cost_benefit: ReputationChange) {}
 
-		fn disconnect_peer(&self, _who: PeerId, _protocol: ProtocolName) {
+		fn peer_reputation(&self, _peer_id: &PeerId) -> i32 {
+			unimplemented!()
+		}
+
+		fn disconnect_peer(&self, _peer_id: PeerId, _protocol: ProtocolName) {
 			unimplemented!();
 		}
 

substrate/client/network-gossip/src/state_machine.rs

@@ -621,11 +621,15 @@ mod tests {
 			unimplemented!();
 		}
 
-		fn report_peer(&self, who: PeerId, cost_benefit: ReputationChange) {
+		fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange) {
-			self.inner.lock().unwrap().peer_reports.push((who, cost_benefit));
+			self.inner.lock().unwrap().peer_reports.push((peer_id, cost_benefit));
 		}
 
-		fn disconnect_peer(&self, _who: PeerId, _protocol: ProtocolName) {
+		fn peer_reputation(&self, _peer_id: &PeerId) -> i32 {
+			unimplemented!()
+		}
+
+		fn disconnect_peer(&self, _peer_id: PeerId, _protocol: ProtocolName) {
 			unimplemented!();
 		}
 

substrate/client/network/src/service.rs

@@ -120,6 +120,8 @@ pub struct NetworkService<B: BlockT + 'static, H: ExHashT> {
 	local_identity: Keypair,
 	/// Bandwidth logging system. Can be queried to know the average bandwidth consumed.
 	bandwidth: Arc<transport::BandwidthSinks>,
+	/// Used to query and report reputation changes.
+	peer_store_handle: PeerStoreHandle,
 	/// Channel that sends messages to the actual worker.
 	to_worker: TracingUnboundedSender<ServiceToWorkerMsg>,
 	/// Protocol name -> `SetId` mapping for notification protocols. The map never changes after
@@ -130,8 +132,6 @@ pub struct NetworkService<B: BlockT + 'static, H: ExHashT> {
 	protocol_handles: Vec<protocol_controller::ProtocolHandle>,
 	/// Shortcut to sync protocol handle (`protocol_handles[0]`).
 	sync_protocol_handle: protocol_controller::ProtocolHandle,
-	/// Handle to `PeerStore`.
-	peer_store_handle: PeerStoreHandle,
 	/// Marker to pin the `H` generic. Serves no purpose except to not break backwards
 	/// compatibility.
 	_marker: PhantomData<H>,
@@ -865,12 +865,18 @@ where
 			.unbounded_send(ServiceToWorkerMsg::AddKnownAddress(peer_id, addr));
 	}
 
-	fn report_peer(&self, who: PeerId, cost_benefit: ReputationChange) {
+	fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange) {
-		let _ = self.to_worker.unbounded_send(ServiceToWorkerMsg::ReportPeer(who, cost_benefit));
+		self.peer_store_handle.clone().report_peer(peer_id, cost_benefit);
 	}
 
-	fn disconnect_peer(&self, who: PeerId, protocol: ProtocolName) {
+	fn peer_reputation(&self, peer_id: &PeerId) -> i32 {
-		let _ = self.to_worker.unbounded_send(ServiceToWorkerMsg::DisconnectPeer(who, protocol));
+		self.peer_store_handle.peer_reputation(peer_id)
+	}
+
+	fn disconnect_peer(&self, peer_id: PeerId, protocol: ProtocolName) {
+		let _ = self
+			.to_worker
+			.unbounded_send(ServiceToWorkerMsg::DisconnectPeer(peer_id, protocol));
 	}
 
 	fn accept_unreserved_peers(&self) {
@@ -1149,7 +1155,6 @@ enum ServiceToWorkerMsg {
 	GetValue(KademliaKey),
 	PutValue(KademliaKey, Vec<u8>),
 	AddKnownAddress(PeerId, Multiaddr),
-	ReportPeer(PeerId, ReputationChange),
 	EventStream(out_events::Sender),
 	Request {
 		target: PeerId,
@@ -1277,8 +1282,6 @@ where
 				self.network_service.behaviour_mut().put_value(key, value),
 			ServiceToWorkerMsg::AddKnownAddress(peer_id, addr) =>
 				self.network_service.behaviour_mut().add_known_address(peer_id, addr),
-			ServiceToWorkerMsg::ReportPeer(peer_id, reputation_change) =>
-				self.peer_store_handle.report_peer(peer_id, reputation_change),
 			ServiceToWorkerMsg::EventStream(sender) => self.event_streams.push(sender),
 			ServiceToWorkerMsg::Request {
 				target,

substrate/client/network/src/service/traits.rs

@@ -155,12 +155,15 @@ pub trait NetworkPeers {
 
 	/// Report a given peer as either beneficial (+) or costly (-) according to the
 	/// given scalar.
-	fn report_peer(&self, who: PeerId, cost_benefit: ReputationChange);
+	fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange);
+
+	/// Get peer reputation.
+	fn peer_reputation(&self, peer_id: &PeerId) -> i32;
 
 	/// Disconnect from a node as soon as possible.
 	///
 	/// This triggers the same effects as if the connection had closed itself spontaneously.
-	fn disconnect_peer(&self, who: PeerId, protocol: ProtocolName);
+	fn disconnect_peer(&self, peer_id: PeerId, protocol: ProtocolName);
 
 	/// Connect to unreserved peers and allow unreserved peers to connect for syncing purposes.
 	fn accept_unreserved_peers(&self);
@@ -254,16 +257,16 @@ where
 		T::add_known_address(self, peer_id, addr)
 	}
 
-	fn report_peer(&self, who: PeerId, cost_benefit: ReputationChange) {
+	fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange) {
-		// TODO: when we get rid of `Peerset`, we'll likely need to add some kind of async
+		T::report_peer(self, peer_id, cost_benefit)
-		// interface to `PeerStore`, otherwise we'll have trouble calling functions accepting
-		// `&mut self` via `Arc`.
-		// See https://github.com/paritytech/substrate/issues/14170.
-		T::report_peer(self, who, cost_benefit)
 	}
 
-	fn disconnect_peer(&self, who: PeerId, protocol: ProtocolName) {
+	fn peer_reputation(&self, peer_id: &PeerId) -> i32 {
-		T::disconnect_peer(self, who, protocol)
+		T::peer_reputation(self, peer_id)
+	}
+
+	fn disconnect_peer(&self, peer_id: PeerId, protocol: ProtocolName) {
+		T::disconnect_peer(self, peer_id, protocol)
 	}
 
 	fn accept_unreserved_peers(&self) {

substrate/client/network/sync/src/service/mock.rs

@@ -84,8 +84,9 @@ mockall::mock! {
 		fn set_authorized_peers(&self, peers: HashSet<PeerId>);
 		fn set_authorized_only(&self, reserved_only: bool);
 		fn add_known_address(&self, peer_id: PeerId, addr: Multiaddr);
-		fn report_peer(&self, who: PeerId, cost_benefit: ReputationChange);
+		fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange);
-		fn disconnect_peer(&self, who: PeerId, protocol: ProtocolName);
+		fn peer_reputation(&self, peer_id: &PeerId) -> i32;
+		fn disconnect_peer(&self, peer_id: PeerId, protocol: ProtocolName);
 		fn accept_unreserved_peers(&self);
 		fn deny_unreserved_peers(&self);
 		fn add_reserved_peer(&self, peer: MultiaddrWithPeerId) -> Result<(), String>;

substrate/client/offchain/src/api.rs

@@ -243,11 +243,15 @@ mod tests {
 			unimplemented!();
 		}
 
-		fn report_peer(&self, _who: PeerId, _cost_benefit: ReputationChange) {
+		fn report_peer(&self, _peer_id: PeerId, _cost_benefit: ReputationChange) {
 			unimplemented!();
 		}
 
-		fn disconnect_peer(&self, _who: PeerId, _protocol: ProtocolName) {
+		fn peer_reputation(&self, _peer_id: &PeerId) -> i32 {
+			unimplemented!()
+		}
+
+		fn disconnect_peer(&self, _peer_id: PeerId, _protocol: ProtocolName) {
 			unimplemented!();
 		}
 

substrate/client/offchain/src/lib.rs

@@ -374,11 +374,15 @@ mod tests {
 			unimplemented!();
 		}
 
-		fn report_peer(&self, _who: PeerId, _cost_benefit: ReputationChange) {
+		fn report_peer(&self, _peer_id: PeerId, _cost_benefit: ReputationChange) {
 			unimplemented!();
 		}
 
-		fn disconnect_peer(&self, _who: PeerId, _protocol: ProtocolName) {
+		fn peer_reputation(&self, _peer_id: &PeerId) -> i32 {
+			unimplemented!()
+		}
+
+		fn disconnect_peer(&self, _peer_id: PeerId, _protocol: ProtocolName) {
 			unimplemented!();
 		}