From 9f3c67b4ffbfb9502cdbcfcb64fcf705be118700 Mon Sep 17 00:00:00 2001
From: Nazar Mokrynskyi <nazar@mokrynskyi.com>
Date: Thu, 7 Dec 2023 13:07:12 +0200
Subject: [PATCH] Support querying peer reputation (#2392)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
# 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>
---
.../grandpa/src/communication/tests.rs | 10 +++++---
substrate/client/network-gossip/src/bridge.rs | 8 +++++--
.../network-gossip/src/state_machine.rs | 10 +++++---
substrate/client/network/src/service.rs | 21 +++++++++--------
.../client/network/src/service/traits.rs | 23 +++++++++++--------
.../client/network/sync/src/service/mock.rs | 5 ++--
substrate/client/offchain/src/api.rs | 8 +++++--
substrate/client/offchain/src/lib.rs | 8 +++++--
8 files changed, 60 insertions(+), 33 deletions(-)
diff --git a/substrate/client/consensus/grandpa/src/communication/tests.rs b/substrate/client/consensus/grandpa/src/communication/tests.rs
index b76b1af93da..fe24fb3cb20 100644
--- a/substrate/client/consensus/grandpa/src/communication/tests.rs
+++ b/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) {
- let _ = self.sender.unbounded_send(Event::Report(who, cost_benefit));
+ fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange) {
+ 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!();
diff --git a/substrate/client/network-gossip/src/bridge.rs b/substrate/client/network-gossip/src/bridge.rs
index c1bc414c3a3..1d6a4bdd0c0 100644
--- a/substrate/client/network-gossip/src/bridge.rs
+++ b/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!();
}
diff --git a/substrate/client/network-gossip/src/state_machine.rs b/substrate/client/network-gossip/src/state_machine.rs
index 91b56b0f097..069d7cdba16 100644
--- a/substrate/client/network-gossip/src/state_machine.rs
+++ b/substrate/client/network-gossip/src/state_machine.rs
@@ -621,11 +621,15 @@ mod tests {
unimplemented!();
}
- fn report_peer(&self, who: PeerId, cost_benefit: ReputationChange) {
- self.inner.lock().unwrap().peer_reports.push((who, cost_benefit));
+ fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange) {
+ 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!();
}
diff --git a/substrate/client/network/src/service.rs b/substrate/client/network/src/service.rs
index 43a3ab09115..06db23844d0 100644
--- a/substrate/client/network/src/service.rs
+++ b/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) {
- let _ = self.to_worker.unbounded_send(ServiceToWorkerMsg::ReportPeer(who, cost_benefit));
+ fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange) {
+ self.peer_store_handle.clone().report_peer(peer_id, cost_benefit);
+ }
+
+ fn peer_reputation(&self, peer_id: &PeerId) -> i32 {
+ self.peer_store_handle.peer_reputation(peer_id)
}
- fn disconnect_peer(&self, who: PeerId, protocol: ProtocolName) {
- let _ = self.to_worker.unbounded_send(ServiceToWorkerMsg::DisconnectPeer(who, protocol));
+ 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,
diff --git a/substrate/client/network/src/service/traits.rs b/substrate/client/network/src/service/traits.rs
index f66e810be11..d4d4a05a86f 100644
--- a/substrate/client/network/src/service/traits.rs
+++ b/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) {
- // TODO: when we get rid of `Peerset`, we'll likely need to add some kind of async
- // 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 report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange) {
+ T::report_peer(self, peer_id, cost_benefit)
+ }
+
+ fn peer_reputation(&self, peer_id: &PeerId) -> i32 {
+ T::peer_reputation(self, peer_id)
}
- fn disconnect_peer(&self, who: PeerId, protocol: ProtocolName) {
- T::disconnect_peer(self, who, protocol)
+ fn disconnect_peer(&self, peer_id: PeerId, protocol: ProtocolName) {
+ T::disconnect_peer(self, peer_id, protocol)
}
fn accept_unreserved_peers(&self) {
diff --git a/substrate/client/network/sync/src/service/mock.rs b/substrate/client/network/sync/src/service/mock.rs
index 47986a71d01..6e307d86984 100644
--- a/substrate/client/network/sync/src/service/mock.rs
+++ b/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 disconnect_peer(&self, who: PeerId, protocol: ProtocolName);
+ fn report_peer(&self, peer_id: PeerId, cost_benefit: ReputationChange);
+ 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>;
diff --git a/substrate/client/offchain/src/api.rs b/substrate/client/offchain/src/api.rs
index 2901bab2f26..40f866b6d28 100644
--- a/substrate/client/offchain/src/api.rs
+++ b/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!();
}
diff --git a/substrate/client/offchain/src/lib.rs b/substrate/client/offchain/src/lib.rs
index 8bcfa66a5af..eb3436432f3 100644
--- a/substrate/client/offchain/src/lib.rs
+++ b/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!();
}
--
GitLab