diff --git a/substrate/client/consensus/grandpa/src/communication/tests.rs b/substrate/client/consensus/grandpa/src/communication/tests.rs index b76b1af93dac630efd618c460bd5b00c650b050c..fe24fb3cb20ea055e8ebfe385a974f57af5e4396 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 c1bc414c3a34057815ad11b9501dc4640888820b..1d6a4bdd0c086996ed45a7074a1a51bcf34f12d9 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 91b56b0f097e0fec4ed187be713fd83249f07f49..069d7cdba16599b4b4da0965a5d8e4588478d633 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 43a3ab09115c7e6838988a6cbf6722ddd40bdac6..06db23844d0d9d07a33e5ffb4303cdf1f9179ec8 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 f66e810be11afa78a007ea494f4f51bc88645018..d4d4a05a86f1dad6cc25120f86d8d267eb49220a 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 47986a71d012a5eb55cecba5f28966ab25448aee..6e307d8698444b78ccd12a08e8b7b41681b8ca1b 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 2901bab2f2677a4d88513b4770ca09f0343e309d..40f866b6d28589180d766cd39199be0ef1136d18 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 8bcfa66a5af32346239ea3f94508beae87080bed..eb3436432f342f4de97a5325c7a14ea4ce167581 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!(); }