From 0f6eb71190f13ccba843d824b70d0cec0e0d8cea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Fri, 9 May 2025 11:55:16 +0100 Subject: [PATCH 1/6] move PeerScoreThresholds and the decay interval to PeerScore --- protocols/gossipsub/src/behaviour.rs | 87 +++++---- protocols/gossipsub/src/behaviour/tests.rs | 184 +++++++++----------- protocols/gossipsub/src/peer_score.rs | 14 +- protocols/gossipsub/src/peer_score/tests.rs | 30 ++-- 4 files changed, 147 insertions(+), 168 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 60d1561b69b..e43dc25ddc1 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -312,7 +312,7 @@ pub struct Behaviour { /// Stores optional peer score data together with thresholds, decay interval and gossip /// promises. - peer_score: Option<(PeerScore, PeerScoreThresholds, Delay)>, + peer_score: Option, /// Counts the number of `IHAVE` received from each peer since the last heartbeat. count_received_ihave: HashMap, @@ -514,9 +514,7 @@ where /// Returns the gossipsub score for a given peer, if one exists. pub fn peer_score(&self, peer_id: &PeerId) -> Option { - self.peer_score - .as_ref() - .map(|(score, ..)| score.score(peer_id)) + self.peer_score.as_ref().map(|score| score.score(peer_id)) } /// Subscribe to a topic. @@ -856,7 +854,7 @@ where // Tell peer_score about reject // Reject the original source, and any duplicates we've seen from other peers. - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.reject_message( propagation_source, msg_id, @@ -921,19 +919,19 @@ where pub fn with_peer_score_and_message_delivery_time_callback( &mut self, params: PeerScoreParams, - threshold: PeerScoreThresholds, + thresholds: PeerScoreThresholds, callback: Option, ) -> Result<(), String> { params.validate()?; - threshold.validate()?; + thresholds.validate()?; if self.peer_score.is_some() { return Err("Peer score set twice".into()); } - let interval = Delay::new(params.decay_interval); - let peer_score = PeerScore::new_with_message_delivery_time_callback(params, callback); - self.peer_score = Some((peer_score, threshold, interval)); + let peer_score = + PeerScore::new_with_message_delivery_time_callback(params, thresholds, callback); + self.peer_score = Some(peer_score); Ok(()) } @@ -945,7 +943,7 @@ where topic: Topic, params: TopicScoreParams, ) -> Result<(), &'static str> { - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.set_topic_params(topic.hash(), params); Ok(()) } else { @@ -955,13 +953,13 @@ where /// Returns a scoring parameters for a topic if existent. pub fn get_topic_params(&self, topic: &Topic) -> Option<&TopicScoreParams> { - self.peer_score.as_ref()?.0.get_topic_params(&topic.hash()) + self.peer_score.as_ref()?.get_topic_params(&topic.hash()) } /// Sets the application specific score for a peer. Returns true if scoring is active and /// the peer is connected or if the score of the peer is not yet expired, false otherwise. pub fn set_application_score(&mut self, peer_id: &PeerId, new_score: f64) -> bool { - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.set_application_score(peer_id, new_score) } else { false @@ -1050,7 +1048,7 @@ where for peer_id in added_peers { // Send a GRAFT control message tracing::debug!(peer=%peer_id, "JOIN: Sending Graft message to peer"); - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.graft(&peer_id, topic_hash.clone()); } self.send_message( @@ -1086,7 +1084,7 @@ where do_px: bool, on_unsubscribe: bool, ) -> Prune { - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.prune(peer, topic_hash.clone()); } @@ -1192,13 +1190,13 @@ where } fn score_below_threshold_from_scores( - peer_score: &Option<(PeerScore, PeerScoreThresholds, Delay)>, + peer_score: &Option, peer_id: &PeerId, threshold: impl Fn(&PeerScoreThresholds) -> f64, ) -> (bool, f64) { - if let Some((peer_score, thresholds, ..)) = peer_score { + if let Some(peer_score) = peer_score { let score = peer_score.score(peer_id); - if score < threshold(thresholds) { + if score < threshold(&peer_score.thresholds) { return (true, score); } (false, score) @@ -1427,7 +1425,7 @@ where "[Penalty] Peer attempted graft within backoff time, penalizing" ); // add behavioural penalty - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { if let Some(metrics) = self.metrics.as_mut() { metrics.register_score_penalty(Penalty::GraftBackoff); } @@ -1500,7 +1498,7 @@ where &self.connected_peers, ); - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.graft(peer_id, topic_hash); } } else { @@ -1558,7 +1556,7 @@ where m.peers_removed(topic_hash, reason, 1) } - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.prune(peer_id, topic_hash.clone()); } @@ -1677,7 +1675,7 @@ where ); self.gossip_promises .reject_message(msg_id, &RejectReason::BlackListedPeer); - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.reject_message( propagation_source, msg_id, @@ -1805,7 +1803,7 @@ where if !self.duplicate_cache.insert(msg_id.clone()) { tracing::debug!(message_id=%msg_id, "Message already received, ignoring"); - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.duplicated_message(propagation_source, &msg_id, &message.topic); } self.mcache.observe_duplicate(&msg_id, propagation_source); @@ -1827,7 +1825,7 @@ where self.gossip_promises.message_delivered(&msg_id); // Tells score that message arrived (but is maybe not fully validated yet). - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.validate_message(propagation_source, &msg_id, &message.topic); } @@ -1882,7 +1880,7 @@ where // Valid transformation without peer scoring self.gossip_promises.reject_message(msg_id, &reject_reason); } - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { // The compiler will optimize this pattern-matching if let Some(msg_id) = message_id { // The message itself is valid, but is from a banned peer or @@ -1989,7 +1987,7 @@ where topic=%topic_hash, "Sending GRAFT to peer for topic" ); - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.graft(propagation_source, topic_hash.clone()); } topics_to_graft.push(topic_hash.clone()); @@ -2064,7 +2062,7 @@ where /// Applies penalties to peers that did not respond to our IWANT requests. fn apply_iwant_penalties(&mut self) { - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { for (peer, count) in self.gossip_promises.get_broken_promises() { peer_score.add_penalty(&peer, count); if let Some(metrics) = self.metrics.as_mut() { @@ -2114,7 +2112,7 @@ where // Cache the scores of all connected peers, and record metrics for current penalties. let mut scores = HashMap::with_capacity(self.connected_peers.len()); - if let Some((peer_score, ..)) = &self.peer_score { + if let Some(peer_score) = &self.peer_score { for peer_id in self.connected_peers.keys() { scores .entry(peer_id) @@ -2306,7 +2304,7 @@ where && peers.len() > 1 && self.peer_score.is_some() { - if let Some((_, thresholds, _)) = &self.peer_score { + if let Some(peer_score) = &self.peer_score { // Opportunistic grafting works as follows: we check the median score of peers // in the mesh; if this score is below the opportunisticGraftThreshold, we // select a few peers at random with score over the median. @@ -2342,7 +2340,7 @@ where // if the median score is below the threshold, select a better peer (if any) and // GRAFT - if median < thresholds.opportunistic_graft_threshold { + if median < peer_score.thresholds.opportunistic_graft_threshold { let peer_list = get_random_peers( &self.connected_peers, topic_hash, @@ -2399,7 +2397,7 @@ where for (topic_hash, peers) in self.fanout.iter_mut() { let mut to_remove_peers = Vec::new(); let publish_threshold = match &self.peer_score { - Some((_, thresholds, _)) => thresholds.publish_threshold, + Some(peer_score) => peer_score.thresholds.publish_threshold, _ => 0.0, }; let mesh_n = self.config.mesh_n_for_topic(topic_hash); @@ -2461,7 +2459,6 @@ where self.peer_score .as_ref() .expect("peer_score.is_some()") - .0 .mesh_message_deliveries(p, t) .unwrap_or(0.0), ) @@ -2590,7 +2587,7 @@ where for (peer_id, topics) in to_graft.into_iter() { for topic in &topics { // inform scoring of graft - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.graft(&peer_id, topic.clone()); } @@ -2671,7 +2668,7 @@ where originating_peers: HashSet, ) -> bool { // message is fully validated inform peer_score - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { if let Some(peer) = propagation_source { peer_score.deliver_message(peer, msg_id, &message.topic); } @@ -2886,7 +2883,7 @@ where } // Update peer score. - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.failed_message_slow_peer(&peer_id); } @@ -2905,7 +2902,7 @@ where }: ConnectionEstablished, ) { // Add the IP to the peer scoring system - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { if let Some(ip) = get_ip_addr(endpoint.get_remote_address()) { peer_score.add_ip(&peer_id, ip); } else { @@ -2921,7 +2918,7 @@ where return; // Not our first connection to this peer, hence nothing to do. } - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.add_peer(peer_id); } @@ -2949,7 +2946,7 @@ where }: ConnectionClosed, ) { // Remove IP from peer scoring system - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { if let Some(ip) = get_ip_addr(endpoint.get_remote_address()) { peer_score.remove_ip(&peer_id, &ip); } else { @@ -3029,7 +3026,7 @@ where self.connected_peers.remove(&peer_id); - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.remove_peer(&peer_id); } } @@ -3045,7 +3042,7 @@ where }: AddressChange, ) { // Exchange IP in peer scoring system - if let Some((peer_score, ..)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { if let Some(ip) = get_ip_addr(endpoint_old.get_remote_address()) { peer_score.remove_ip(&peer_id, &ip); } else { @@ -3186,7 +3183,7 @@ where } HandlerEvent::MessageDropped(rpc) => { // Account for this in the scoring logic - if let Some((peer_score, _, _)) = &mut self.peer_score { + if let Some(peer_score) = &mut self.peer_score { peer_score.failed_message_slow_peer(&propagation_source); } @@ -3350,10 +3347,12 @@ where } // update scores - if let Some((peer_score, _, delay)) = &mut self.peer_score { - if delay.poll_unpin(cx).is_ready() { + if let Some(peer_score) = &mut self.peer_score { + if peer_score.decay_interval.poll_unpin(cx).is_ready() { peer_score.refresh_scores(); - delay.reset(peer_score.params.decay_interval); + peer_score + .decay_interval + .reset(peer_score.params.decay_interval); } } diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index 533891d9f58..ba5dfaff553 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -2519,7 +2519,7 @@ fn test_prune_negative_scored_peers() { .create_network(); // add penalty to peer - gs.peer_score.as_mut().unwrap().0.add_penalty(&peers[0], 1); + gs.peer_score.as_mut().unwrap().add_penalty(&peers[0], 1); // execute heartbeat gs.heartbeat(); @@ -2567,7 +2567,7 @@ fn test_dont_graft_to_negative_scored_peers() { let (p2, _receiver2) = add_peer(&mut gs, &topics, false, false); // reduce score of p1 to negative - gs.peer_score.as_mut().unwrap().0.add_penalty(&p1, 1); + gs.peer_score.as_mut().unwrap().add_penalty(&p1, 1); // handle prunes of all other peers for p in peers { @@ -2602,7 +2602,7 @@ fn test_ignore_px_from_negative_scored_peer() { .create_network(); // penalize peer - gs.peer_score.as_mut().unwrap().0.add_penalty(&peers[0], 1); + gs.peer_score.as_mut().unwrap().add_penalty(&peers[0], 1); // handle prune from single peer with px peers let px = vec![PeerInfo { @@ -2651,7 +2651,7 @@ fn test_only_send_nonnegative_scoring_peers_in_px() { .create_network(); // Penalize first peer - gs.peer_score.as_mut().unwrap().0.add_penalty(&peers[0], 1); + gs.peer_score.as_mut().unwrap().add_penalty(&peers[0], 1); // Prune second peer gs.send_graft_prune( @@ -2713,10 +2713,10 @@ fn test_do_not_gossip_to_peers_below_gossip_threshold() { // Reduce score of p1 below peer_score_thresholds.gossip_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().0.add_penalty(&p1, 2); + gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); // Reduce score of p2 below 0 but not below peer_score_thresholds.gossip_threshold - gs.peer_score.as_mut().unwrap().0.add_penalty(&p2, 1); + gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); // Receive message let raw_message = RawMessage { @@ -2790,10 +2790,10 @@ fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { // Reduce score of p1 below peer_score_thresholds.gossip_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().0.add_penalty(&p1, 2); + gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); // Reduce score of p2 below 0 but not below peer_score_thresholds.gossip_threshold - gs.peer_score.as_mut().unwrap().0.add_penalty(&p2, 1); + gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); // Receive message let raw_message = RawMessage { @@ -2880,10 +2880,10 @@ fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { // reduce score of p1 below peer_score_thresholds.gossip_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().0.add_penalty(&p1, 2); + gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); // reduce score of p2 below 0 but not below peer_score_thresholds.gossip_threshold - gs.peer_score.as_mut().unwrap().0.add_penalty(&p2, 1); + gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); // message that other peers have let raw_message = RawMessage { @@ -2951,10 +2951,10 @@ fn test_do_not_publish_to_peer_below_publish_threshold() { // reduce score of p1 below peer_score_thresholds.publish_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().0.add_penalty(&p1, 2); + gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); // reduce score of p2 below 0 but not below peer_score_thresholds.publish_threshold - gs.peer_score.as_mut().unwrap().0.add_penalty(&p2, 1); + gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); // a heartbeat will remove the peers from the mesh gs.heartbeat(); @@ -3006,10 +3006,10 @@ fn test_do_not_flood_publish_to_peer_below_publish_threshold() { // reduce score of p1 below peer_score_thresholds.publish_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().0.add_penalty(&p1, 2); + gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); // reduce score of p2 below 0 but not below peer_score_thresholds.publish_threshold - gs.peer_score.as_mut().unwrap().0.add_penalty(&p2, 1); + gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); // a heartbeat will remove the peers from the mesh gs.heartbeat(); @@ -3061,10 +3061,10 @@ fn test_ignore_rpc_from_peers_below_graylist_threshold() { // reduce score of p1 below peer_score_thresholds.graylist_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().0.add_penalty(&p1, 2); + gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); // reduce score of p2 below publish_threshold but not below graylist_threshold - gs.peer_score.as_mut().unwrap().0.add_penalty(&p2, 1); + gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); let raw_message1 = RawMessage { source: Some(PeerId::random()), @@ -3322,14 +3322,14 @@ fn test_scoring_p1() { // sleep for 2 times the mesh_quantum sleep(topic_params.time_in_mesh_quantum * 2); // refresh scores - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); assert!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]) + gs.peer_score.as_ref().unwrap().score(&peers[0]) >= 2.0 * topic_params.time_in_mesh_weight * topic_params.topic_weight, "score should be at least 2 * time_in_mesh_weight * topic_weight" ); assert!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]) + gs.peer_score.as_ref().unwrap().score(&peers[0]) < 3.0 * topic_params.time_in_mesh_weight * topic_params.topic_weight, "score should be less than 3 * time_in_mesh_weight * topic_weight" ); @@ -3337,9 +3337,9 @@ fn test_scoring_p1() { // sleep again for 2 times the mesh_quantum sleep(topic_params.time_in_mesh_quantum * 2); // refresh scores - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); assert!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]) + gs.peer_score.as_ref().unwrap().score(&peers[0]) >= 2.0 * topic_params.time_in_mesh_weight * topic_params.topic_weight, "score should be at least 4 * time_in_mesh_weight * topic_weight" ); @@ -3347,9 +3347,9 @@ fn test_scoring_p1() { // sleep for enough periods to reach maximum sleep(topic_params.time_in_mesh_quantum * (topic_params.time_in_mesh_cap - 3.0) as u32); // refresh scores - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), topic_params.time_in_mesh_cap * topic_params.time_in_mesh_weight * topic_params.topic_weight, @@ -3413,13 +3413,13 @@ fn test_scoring_p2() { deliver_message(&mut gs, 1, m1); assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), 1.0 * topic_params.first_message_deliveries_weight * topic_params.topic_weight, "score should be exactly first_message_deliveries_weight * topic_weight" ); assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[1]), + gs.peer_score.as_ref().unwrap().score(&peers[1]), 0.0, "there should be no score for second message deliveries * topic_weight" ); @@ -3428,16 +3428,16 @@ fn test_scoring_p2() { deliver_message(&mut gs, 1, random_message(&mut seq, &topics)); deliver_message(&mut gs, 1, random_message(&mut seq, &topics)); assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[1]), + gs.peer_score.as_ref().unwrap().score(&peers[1]), 2.0 * topic_params.first_message_deliveries_weight * topic_params.topic_weight, "score should be exactly 2 * first_message_deliveries_weight * topic_weight" ); // test decaying - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), 1.0 * topic_params.first_message_deliveries_decay * topic_params.first_message_deliveries_weight * topic_params.topic_weight, @@ -3446,7 +3446,7 @@ fn test_scoring_p2() { ); assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[1]), + gs.peer_score.as_ref().unwrap().score(&peers[1]), 2.0 * topic_params.first_message_deliveries_decay * topic_params.first_message_deliveries_weight * topic_params.topic_weight, @@ -3460,7 +3460,7 @@ fn test_scoring_p2() { } assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[1]), + gs.peer_score.as_ref().unwrap().score(&peers[1]), topic_params.first_message_deliveries_cap * topic_params.first_message_deliveries_weight * topic_params.topic_weight, @@ -3535,11 +3535,11 @@ fn test_scoring_p3() { // message deliveries penalties get activated, peer 0 has only delivered 3 messages and // therefore gets a penalty - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); expected_message_deliveries *= 0.9; // decay assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), (5f64 - expected_message_deliveries).powi(2) * -2.0 * 0.7 ); @@ -3550,16 +3550,16 @@ fn test_scoring_p3() { expected_message_deliveries = 10.0; - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[0]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); // apply 10 decays for _ in 0..10 { - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); expected_message_deliveries *= 0.9; // decay } assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), (5f64 - expected_message_deliveries).powi(2) * -2.0 * 0.7 ); } @@ -3613,7 +3613,6 @@ fn test_scoring_p3b() { gs.peer_score .as_mut() .unwrap() - .0 .set_application_score(&peers[0], 100.0); // peer 0 delivers two message @@ -3624,7 +3623,7 @@ fn test_scoring_p3b() { sleep(Duration::from_millis(1050)); // activation kicks in - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); expected_message_deliveries *= 0.9; // decay // prune peer @@ -3639,7 +3638,7 @@ fn test_scoring_p3b() { // the score should now consider p3b let mut expected_b3 = (5f64 - expected_message_deliveries).powi(2); assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), 100.0 + expected_b3 * -3.0 * 0.7 ); @@ -3650,12 +3649,12 @@ fn test_scoring_p3b() { expected_message_deliveries += 1.0; sleep(Duration::from_millis(1050)); - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); expected_message_deliveries *= 0.9; // decay expected_b3 *= 0.95; assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), 100.0 + (expected_b3 * -3.0 + (5f64 - expected_message_deliveries).powi(2) * -2.0) * 0.7 ); } @@ -3710,7 +3709,7 @@ fn test_scoring_p4_valid_message() { // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[0]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); // message m1 gets validated gs.report_message_validation_result( @@ -3719,7 +3718,7 @@ fn test_scoring_p4_valid_message() { MessageAcceptance::Accept, ); - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[0]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); } #[test] @@ -3778,10 +3777,7 @@ fn test_scoring_p4_invalid_signature() { }, ); - assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), - -2.0 * 0.7 - ); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); } #[test] @@ -3832,10 +3828,7 @@ fn test_scoring_p4_message_from_self() { m.source = Some(*gs.publish_config.get_own_id().unwrap()); deliver_message(&mut gs, 0, m); - assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), - -2.0 * 0.7 - ); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); } #[test] @@ -3885,7 +3878,7 @@ fn test_scoring_p4_ignored_message() { let m1 = random_message(&mut seq, &topics); deliver_message(&mut gs, 0, m1.clone()); - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[0]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); @@ -3897,7 +3890,7 @@ fn test_scoring_p4_ignored_message() { MessageAcceptance::Ignore, ); - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[0]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); } #[test] @@ -3947,7 +3940,7 @@ fn test_scoring_p4_application_invalidated_message() { let m1 = random_message(&mut seq, &topics); deliver_message(&mut gs, 0, m1.clone()); - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[0]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); @@ -3959,10 +3952,7 @@ fn test_scoring_p4_application_invalidated_message() { MessageAcceptance::Reject, ); - assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), - -2.0 * 0.7 - ); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); } #[test] @@ -4018,8 +4008,8 @@ fn test_scoring_p4_application_invalid_message_from_two_peers() { // peer 1 delivers same message deliver_message(&mut gs, 1, m1); - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[0]), 0.0); - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[1]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[1]), 0.0); // message m1 gets rejected gs.report_message_validation_result( @@ -4028,14 +4018,8 @@ fn test_scoring_p4_application_invalid_message_from_two_peers() { MessageAcceptance::Reject, ); - assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), - -2.0 * 0.7 - ); - assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[1]), - -2.0 * 0.7 - ); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[1]), -2.0 * 0.7); } #[test] @@ -4097,7 +4081,7 @@ fn test_scoring_p4_three_application_invalid_messages() { // Transform the inbound message let message3 = &gs.data_transform.inbound_transform(m3).unwrap(); - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[0]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); // messages gets rejected gs.report_message_validation_result( @@ -4120,7 +4104,7 @@ fn test_scoring_p4_three_application_invalid_messages() { // number of invalid messages gets squared assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), 9.0 * -2.0 * 0.7 ); } @@ -4174,7 +4158,7 @@ fn test_scoring_p4_decay() { // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&peers[0]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); // message m1 gets rejected gs.report_message_validation_result( @@ -4183,17 +4167,14 @@ fn test_scoring_p4_decay() { MessageAcceptance::Reject, ); - assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), - -2.0 * 0.7 - ); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); // we decay - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); // the number of invalids gets decayed to 0.9 and then squared in the score assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.9 * 0.9 * -2.0 * 0.7 ); } @@ -4218,10 +4199,7 @@ fn test_scoring_p5() { gs.set_application_score(&peers[0], 1.1); - assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), - 1.1 * 2.0 - ); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 1.1 * 2.0); } #[test] @@ -4263,7 +4241,7 @@ fn test_scoring_p6() { // no penalties yet for peer in peers.iter().chain(others.iter()) { - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(peer), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 0.0); } // add additional connection for 3 others with addr @@ -4283,10 +4261,10 @@ fn test_scoring_p6() { // penalties apply squared for peer in peers.iter().chain(others.iter().take(3)) { - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(peer), 9.0 * -2.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 9.0 * -2.0); } // fourth other peer still no penalty - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(&others[3]), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&others[3]), 0.0); // add additional connection for 3 of the peers to addr2 for peer in peers.iter().take(3) { @@ -4306,17 +4284,17 @@ fn test_scoring_p6() { // double penalties for the first three of each for peer in peers.iter().take(3).chain(others.iter().take(3)) { assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(peer), + gs.peer_score.as_ref().unwrap().score(peer), (9.0 + 4.0) * -2.0 ); } // single penalties for the rest for peer in peers.iter().skip(3) { - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(peer), 9.0 * -2.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 9.0 * -2.0); } assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&others[3]), + gs.peer_score.as_ref().unwrap().score(&others[3]), 4.0 * -2.0 ); @@ -4337,17 +4315,17 @@ fn test_scoring_p6() { // double penalties for the first three of each for peer in peers.iter().take(3).chain(others.iter().take(3)) { assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(peer), + gs.peer_score.as_ref().unwrap().score(peer), (9.0 + 4.0) * -2.0 ); } // single penalties for the rest for peer in peers.iter().skip(3) { - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(peer), 9.0 * -2.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 9.0 * -2.0); } assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&others[3]), + gs.peer_score.as_ref().unwrap().score(&others[3]), 4.0 * -2.0 ); } @@ -4392,10 +4370,7 @@ fn test_scoring_p7_grafts_before_backoff() { gs.handle_graft(&peers[0], vec![topics[0].clone()]); // double behaviour penalty for first peer (squared) - assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), - 4.0 * -2.0 - ); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 4.0 * -2.0); // wait 100 millisecs sleep(Duration::from_millis(100)); @@ -4404,20 +4379,17 @@ fn test_scoring_p7_grafts_before_backoff() { gs.handle_graft(&peers[1], vec![topics[0].clone()]); // single behaviour penalty for second peer - assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[1]), - 1.0 * -2.0 - ); + assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[1]), 1.0 * -2.0); // test decay - gs.peer_score.as_mut().unwrap().0.refresh_scores(); + gs.peer_score.as_mut().unwrap().refresh_scores(); assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[0]), + gs.peer_score.as_ref().unwrap().score(&peers[0]), 4.0 * 0.9 * 0.9 * -2.0 ); assert_eq!( - gs.peer_score.as_ref().unwrap().0.score(&peers[1]), + gs.peer_score.as_ref().unwrap().score(&peers[1]), 1.0 * 0.9 * 0.9 * -2.0 ); } @@ -4897,7 +4869,7 @@ fn test_iwant_penalties() { gs.heartbeat(); for (peer, _receiver) in &other_peers { - assert_eq!(gs.peer_score.as_ref().unwrap().0.score(peer), 0.0); + assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 0.0); } // receive the first twenty of the other peers then send their response @@ -4927,7 +4899,7 @@ fn test_iwant_penalties() { let mut double_penalized = 0; for (i, (peer, _receiver)) in other_peers.iter().enumerate() { - let score = gs.peer_score.as_ref().unwrap().0.score(peer); + let score = gs.peer_score.as_ref().unwrap().score(peer); if score == 0.0 { not_penalized += 1; } else if score == -1.0 { @@ -5257,9 +5229,9 @@ fn test_subscribe_and_graft_with_negative_score() { let (p1, _receiver2) = add_peer(&mut gs2, &topic_hashes, false, false); // add penalty to peer p2 - gs1.peer_score.as_mut().unwrap().0.add_penalty(&p2, 1); + gs1.peer_score.as_mut().unwrap().add_penalty(&p2, 1); - let original_score = gs1.peer_score.as_ref().unwrap().0.score(&p2); + let original_score = gs1.peer_score.as_ref().unwrap().score(&p2); // subscribe to topic in gs2 gs2.subscribe(&topic).unwrap(); @@ -5301,7 +5273,7 @@ fn test_subscribe_and_graft_with_negative_score() { forward_messages_to_p1(&mut gs1, p1, p2, connection_id, receivers); // nobody got penalized - assert!(gs1.peer_score.as_ref().unwrap().0.score(&p2) >= original_score); + assert!(gs1.peer_score.as_ref().unwrap().score(&p2) >= original_score); } #[test] @@ -6010,7 +5982,7 @@ fn test_slow_peer_is_downscored_on_publish() { dont_send: LinkedHashMap::new(), }, ); - gs.peer_score.as_mut().unwrap().0.add_peer(slow_peer_id); + gs.peer_score.as_mut().unwrap().add_peer(slow_peer_id); let peer_id = PeerId::random(); peers.push(peer_id); gs.connected_peers.insert( diff --git a/protocols/gossipsub/src/peer_score.rs b/protocols/gossipsub/src/peer_score.rs index 87b0b24bdee..07956b388ac 100644 --- a/protocols/gossipsub/src/peer_score.rs +++ b/protocols/gossipsub/src/peer_score.rs @@ -26,6 +26,7 @@ use std::{ time::Duration, }; +use futures_timer::Delay; use libp2p_identity::PeerId; use web_time::Instant; @@ -52,6 +53,10 @@ const TIME_CACHE_DURATION: u64 = 120; pub(crate) struct PeerScore { /// The score parameters. pub(crate) params: PeerScoreParams, + /// The score threshold. + pub(crate) thresholds: PeerScoreThresholds, + /// The peer score decay interval. + pub(crate) decay_interval: Delay, /// The stats per PeerId. peer_stats: HashMap, /// Tracking peers per IP. @@ -210,16 +215,19 @@ impl Default for DeliveryRecord { impl PeerScore { /// Creates a new [`PeerScore`] using a given set of peer scoring parameters. #[allow(dead_code)] - pub(crate) fn new(params: PeerScoreParams) -> Self { - Self::new_with_message_delivery_time_callback(params, None) + pub(crate) fn new(params: PeerScoreParams, thresholds: PeerScoreThresholds) -> Self { + Self::new_with_message_delivery_time_callback(params, thresholds, None) } pub(crate) fn new_with_message_delivery_time_callback( params: PeerScoreParams, + thresholds: PeerScoreThresholds, callback: Option, ) -> Self { PeerScore { + decay_interval: Delay::new(params.decay_interval), params, + thresholds, peer_stats: HashMap::new(), peer_ips: HashMap::new(), deliveries: TimeCache::new(Duration::from_secs(TIME_CACHE_DURATION)), @@ -328,7 +336,7 @@ impl PeerScore { continue; } - // P6 has a cliff (ip_colocation_factor_threshold); it's only applied iff + // P6 has a cliff (ip_colocation_factor_threshold); it's only applied if // at least that many peers are connected to us from that source IP // addr. It is quadratic, and the weight is negative (validated by // peer_score_params.validate()). diff --git a/protocols/gossipsub/src/peer_score/tests.rs b/protocols/gossipsub/src/peer_score/tests.rs index 951f7084347..5ff079b3529 100644 --- a/protocols/gossipsub/src/peer_score/tests.rs +++ b/protocols/gossipsub/src/peer_score/tests.rs @@ -91,7 +91,7 @@ fn test_score_time_in_mesh() { let peer_id = PeerId::random(); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); // Peer score should start at 0 peer_score.add_peer(peer_id); @@ -136,7 +136,7 @@ fn test_score_time_in_mesh_cap() { let peer_id = PeerId::random(); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); // Peer score should start at 0 peer_score.add_peer(peer_id); @@ -186,7 +186,7 @@ fn test_score_first_message_deliveries() { let peer_id = PeerId::random(); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); // Peer score should start at 0 peer_score.add_peer(peer_id); peer_score.graft(&peer_id, topic); @@ -227,7 +227,7 @@ fn test_score_first_message_deliveries_cap() { let peer_id = PeerId::random(); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); // Peer score should start at 0 peer_score.add_peer(peer_id); peer_score.graft(&peer_id, topic); @@ -266,7 +266,7 @@ fn test_score_first_message_deliveries_decay() { params.topics.insert(topic_hash, topic_params.clone()); let peer_id = PeerId::random(); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); peer_score.add_peer(peer_id); peer_score.graft(&peer_id, topic); @@ -318,7 +318,7 @@ fn test_score_mesh_message_deliveries() { }; params.topics.insert(topic_hash, topic_params.clone()); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); // peer A always delivers the message first. // peer B delivers next (within the delivery window). @@ -415,7 +415,7 @@ fn test_score_mesh_message_deliveries_decay() { }; params.topics.insert(topic_hash, topic_params.clone()); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); let peer_id_a = PeerId::random(); peer_score.add_peer(peer_id_a); @@ -484,7 +484,7 @@ fn test_score_mesh_failure_penalty() { }; params.topics.insert(topic_hash, topic_params.clone()); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); let peer_id_a = PeerId::random(); let peer_id_b = PeerId::random(); @@ -561,7 +561,7 @@ fn test_score_invalid_message_deliveries() { }; params.topics.insert(topic_hash, topic_params.clone()); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); let peer_id_a = PeerId::random(); peer_score.add_peer(peer_id_a); @@ -608,7 +608,7 @@ fn test_score_invalid_message_deliveris_decay() { }; params.topics.insert(topic_hash, topic_params.clone()); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); let peer_id_a = PeerId::random(); peer_score.add_peer(peer_id_a); @@ -664,7 +664,7 @@ fn test_score_reject_message_deliveries() { }; params.topics.insert(topic_hash, topic_params); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); let peer_id_a = PeerId::random(); let peer_id_b = PeerId::random(); @@ -781,7 +781,7 @@ fn test_application_score() { }; params.topics.insert(topic_hash, topic_params); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); let peer_id_a = PeerId::random(); peer_score.add_peer(peer_id_a); @@ -823,7 +823,7 @@ fn test_score_ip_colocation() { }; params.topics.insert(topic_hash, topic_params); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); let peer_id_a = PeerId::random(); let peer_id_b = PeerId::random(); @@ -887,7 +887,7 @@ fn test_score_behaviour_penalty() { }; params.topics.insert(topic_hash, topic_params); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); let peer_id_a = PeerId::random(); @@ -940,7 +940,7 @@ fn test_score_retention() { }; params.topics.insert(topic_hash, topic_params); - let mut peer_score = PeerScore::new(params); + let mut peer_score = PeerScore::new(params, PeerScoreThresholds::default()); let peer_id_a = PeerId::random(); peer_score.add_peer(peer_id_a); From f465467d9105c68d82e3e49dbfefca42ace59a83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Fri, 9 May 2025 23:10:02 +0100 Subject: [PATCH 2/6] move peer score calculation to inside PeerScore replace Option with PeerScoreState to allow to ease that move --- protocols/gossipsub/src/behaviour.rs | 174 +++++++++---------- protocols/gossipsub/src/behaviour/tests.rs | 185 ++++++++++----------- protocols/gossipsub/src/peer_score.rs | 25 +++ 3 files changed, 190 insertions(+), 194 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index e43dc25ddc1..08111f1ef35 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -57,7 +57,7 @@ use crate::{ handler::{Handler, HandlerEvent, HandlerIn}, mcache::MessageCache, metrics::{Churn, Config as MetricsConfig, Inclusion, Metrics, Penalty}, - peer_score::{PeerScore, PeerScoreParams, PeerScoreThresholds, RejectReason}, + peer_score::{PeerScore, PeerScoreParams, PeerScoreState, PeerScoreThresholds, RejectReason}, protocol::SIGNING_PREFIX, rpc::Sender, rpc_proto::proto, @@ -312,7 +312,7 @@ pub struct Behaviour { /// Stores optional peer score data together with thresholds, decay interval and gossip /// promises. - peer_score: Option, + peer_score: PeerScoreState, /// Counts the number of `IHAVE` received from each peer since the last heartbeat. count_received_ihave: HashMap, @@ -463,7 +463,7 @@ where heartbeat: Delay::new(config.heartbeat_interval() + config.heartbeat_initial_delay()), heartbeat_ticks: 0, px_peers: HashSet::new(), - peer_score: None, + peer_score: PeerScoreState::Disabled, count_received_ihave: HashMap::new(), count_sent_iwant: HashMap::new(), connected_peers: HashMap::new(), @@ -512,11 +512,6 @@ where self.connected_peers.iter().map(|(k, v)| (k, &v.kind)) } - /// Returns the gossipsub score for a given peer, if one exists. - pub fn peer_score(&self, peer_id: &PeerId) -> Option { - self.peer_score.as_ref().map(|score| score.score(peer_id)) - } - /// Subscribe to a topic. /// /// Returns [`Ok(true)`] if the subscription worked. Returns [`Ok(false)`] if we were already @@ -641,7 +636,10 @@ where // Forward to all peers above score and all explicit peers recipient_peers.extend(peers_on_topic.filter(|p| { self.explicit_peers.contains(*p) - || !self.score_below_threshold(p, |ts| ts.publish_threshold).0 + || !self + .peer_score + .below_threshold(p, |ts| ts.publish_threshold) + .0 })); } else { match self.mesh.get(&topic_hash) { @@ -664,7 +662,8 @@ where !mesh_peers.contains(peer) && !self.explicit_peers.contains(peer) && !self - .score_below_threshold(peer, |pst| pst.publish_threshold) + .peer_score + .below_threshold(peer, |ts| ts.publish_threshold) .0 }, ); @@ -693,7 +692,8 @@ where |p| { !self.explicit_peers.contains(p) && !self - .score_below_threshold(p, |pst| pst.publish_threshold) + .peer_score + .below_threshold(p, |ts| ts.publish_threshold) .0 } }); @@ -719,7 +719,8 @@ where if connections.kind == PeerKind::Floodsub && connections.topics.contains(&topic_hash) && !self - .score_below_threshold(peer, |ts| ts.publish_threshold) + .peer_score + .below_threshold(peer, |ts| ts.publish_threshold) .0 { recipient_peers.insert(*peer); @@ -854,7 +855,7 @@ where // Tell peer_score about reject // Reject the original source, and any duplicates we've seen from other peers. - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.reject_message( propagation_source, msg_id, @@ -925,13 +926,13 @@ where params.validate()?; thresholds.validate()?; - if self.peer_score.is_some() { + if let PeerScoreState::Active(_) = self.peer_score { return Err("Peer score set twice".into()); } let peer_score = PeerScore::new_with_message_delivery_time_callback(params, thresholds, callback); - self.peer_score = Some(peer_score); + self.peer_score = PeerScoreState::Active(Box::new(peer_score)); Ok(()) } @@ -943,7 +944,7 @@ where topic: Topic, params: TopicScoreParams, ) -> Result<(), &'static str> { - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.set_topic_params(topic.hash(), params); Ok(()) } else { @@ -953,13 +954,16 @@ where /// Returns a scoring parameters for a topic if existent. pub fn get_topic_params(&self, topic: &Topic) -> Option<&TopicScoreParams> { - self.peer_score.as_ref()?.get_topic_params(&topic.hash()) + match &self.peer_score { + PeerScoreState::Active(peer_score) => peer_score.get_topic_params(&topic.hash()), + PeerScoreState::Disabled => None, + } } /// Sets the application specific score for a peer. Returns true if scoring is active and /// the peer is connected or if the score of the peer is not yet expired, false otherwise. pub fn set_application_score(&mut self, peer_id: &PeerId, new_score: f64) -> bool { - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.set_application_score(peer_id, new_score) } else { false @@ -987,7 +991,7 @@ where // remove explicit peers, peers with negative scores, and backoffed peers peers.retain(|p| { !self.explicit_peers.contains(p) - && !self.score_below_threshold(p, |_| 0.0).0 + && !self.peer_score.below_threshold(p, |_| 0.0).0 && !self.backoffs.is_backoff_with_slack(topic_hash, p) }); @@ -1025,7 +1029,7 @@ where |peer| { !added_peers.contains(peer) && !self.explicit_peers.contains(peer) - && !self.score_below_threshold(peer, |_| 0.0).0 + && !self.peer_score.below_threshold(peer, |_| 0.0).0 && !self.backoffs.is_backoff_with_slack(topic_hash, peer) }, ); @@ -1048,7 +1052,7 @@ where for peer_id in added_peers { // Send a GRAFT control message tracing::debug!(peer=%peer_id, "JOIN: Sending Graft message to peer"); - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.graft(&peer_id, topic_hash.clone()); } self.send_message( @@ -1084,7 +1088,7 @@ where do_px: bool, on_unsubscribe: bool, ) -> Prune { - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.prune(peer, topic_hash.clone()); } @@ -1112,7 +1116,7 @@ where &self.connected_peers, topic_hash, self.config.prune_peers(), - |p| p != peer && !self.score_below_threshold(p, |_| 0.0).0, + |p| p != peer && !self.peer_score.below_threshold(p, |_| 0.0).0, ) .into_iter() .map(|p| PeerInfo { peer_id: Some(p) }) @@ -1179,37 +1183,14 @@ where } } - /// Determines if a peer's score is below a given `PeerScoreThreshold` chosen via the - /// `threshold` parameter. - fn score_below_threshold( - &self, - peer_id: &PeerId, - threshold: impl Fn(&PeerScoreThresholds) -> f64, - ) -> (bool, f64) { - Self::score_below_threshold_from_scores(&self.peer_score, peer_id, threshold) - } - - fn score_below_threshold_from_scores( - peer_score: &Option, - peer_id: &PeerId, - threshold: impl Fn(&PeerScoreThresholds) -> f64, - ) -> (bool, f64) { - if let Some(peer_score) = peer_score { - let score = peer_score.score(peer_id); - if score < threshold(&peer_score.thresholds) { - return (true, score); - } - (false, score) - } else { - (false, 0.0) - } - } - /// Handles an IHAVE control message. Checks our cache of messages. If the message is unknown, /// requests it with an IWANT control message. fn handle_ihave(&mut self, peer_id: &PeerId, ihave_msgs: Vec<(TopicHash, Vec)>) { // We ignore IHAVE gossip from any peer whose score is below the gossip threshold - if let (true, score) = self.score_below_threshold(peer_id, |pst| pst.gossip_threshold) { + if let (true, score) = self + .peer_score + .below_threshold(peer_id, |ts| ts.gossip_threshold) + { tracing::debug!( peer=%peer_id, %score, @@ -1321,7 +1302,10 @@ where /// forwarded to the requesting peer. fn handle_iwant(&mut self, peer_id: &PeerId, iwant_msgs: Vec) { // We ignore IWANT gossip from any peer whose score is below the gossip threshold - if let (true, score) = self.score_below_threshold(peer_id, |pst| pst.gossip_threshold) { + if let (true, score) = self + .peer_score + .below_threshold(peer_id, |ts| ts.gossip_threshold) + { tracing::debug!( peer=%peer_id, "IWANT: ignoring peer with score below threshold [score = {}]", @@ -1402,7 +1386,7 @@ where // but don't PX do_px = false } else { - let (below_zero, score) = self.score_below_threshold(peer_id, |_| 0.0); + let (below_zero, score) = self.peer_score.below_threshold(peer_id, |_| 0.0); let now = Instant::now(); for topic_hash in topics { if let Some(peers) = self.mesh.get_mut(&topic_hash) { @@ -1425,7 +1409,7 @@ where "[Penalty] Peer attempted graft within backoff time, penalizing" ); // add behavioural penalty - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { if let Some(metrics) = self.metrics.as_mut() { metrics.register_score_penalty(Penalty::GraftBackoff); } @@ -1498,7 +1482,7 @@ where &self.connected_peers, ); - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.graft(peer_id, topic_hash); } } else { @@ -1556,7 +1540,7 @@ where m.peers_removed(topic_hash, reason, 1) } - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.prune(peer_id, topic_hash.clone()); } @@ -1590,8 +1574,9 @@ where prune_data: Vec<(TopicHash, Vec, Option)>, ) { tracing::debug!(peer=%peer_id, "Handling PRUNE message for peer"); - let (below_threshold, score) = - self.score_below_threshold(peer_id, |pst| pst.accept_px_threshold); + let (below_threshold, score) = self + .peer_score + .below_threshold(peer_id, |ts| ts.accept_px_threshold); for (topic_hash, px, backoff) in prune_data { self.remove_peer_from_mesh(peer_id, &topic_hash, backoff, true, Churn::Prune); @@ -1675,7 +1660,7 @@ where ); self.gossip_promises .reject_message(msg_id, &RejectReason::BlackListedPeer); - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.reject_message( propagation_source, msg_id, @@ -1803,7 +1788,7 @@ where if !self.duplicate_cache.insert(msg_id.clone()) { tracing::debug!(message_id=%msg_id, "Message already received, ignoring"); - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.duplicated_message(propagation_source, &msg_id, &message.topic); } self.mcache.observe_duplicate(&msg_id, propagation_source); @@ -1825,7 +1810,7 @@ where self.gossip_promises.message_delivered(&msg_id); // Tells score that message arrived (but is maybe not fully validated yet). - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.validate_message(propagation_source, &msg_id, &message.topic); } @@ -1880,7 +1865,7 @@ where // Valid transformation without peer scoring self.gossip_promises.reject_message(msg_id, &reject_reason); } - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { // The compiler will optimize this pattern-matching if let Some(msg_id) = message_id { // The message itself is valid, but is from a banned peer or @@ -1959,12 +1944,10 @@ where // if the mesh needs peers add the peer to the mesh if !self.explicit_peers.contains(propagation_source) && peer.kind.is_gossipsub() - && !Self::score_below_threshold_from_scores( - &self.peer_score, - propagation_source, - |_| 0.0, - ) - .0 + && !self + .peer_score + .below_threshold(propagation_source, |_| 0.0) + .0 && !self .backoffs .is_backoff_with_slack(topic_hash, propagation_source) @@ -1987,7 +1970,7 @@ where topic=%topic_hash, "Sending GRAFT to peer for topic" ); - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.graft(propagation_source, topic_hash.clone()); } topics_to_graft.push(topic_hash.clone()); @@ -2062,7 +2045,7 @@ where /// Applies penalties to peers that did not respond to our IWANT requests. fn apply_iwant_penalties(&mut self) { - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { for (peer, count) in self.gossip_promises.get_broken_promises() { peer_score.add_penalty(&peer, count); if let Some(metrics) = self.metrics.as_mut() { @@ -2112,7 +2095,7 @@ where // Cache the scores of all connected peers, and record metrics for current penalties. let mut scores = HashMap::with_capacity(self.connected_peers.len()); - if let Some(peer_score) = &self.peer_score { + if let PeerScoreState::Active(peer_score) = &self.peer_score { for peer_id in self.connected_peers.keys() { scores .entry(peer_id) @@ -2302,9 +2285,8 @@ where // should we try to improve the mesh with opportunistic grafting? if self.heartbeat_ticks % self.config.opportunistic_graft_ticks() == 0 && peers.len() > 1 - && self.peer_score.is_some() { - if let Some(peer_score) = &self.peer_score { + if let PeerScoreState::Active(peer_score) = &self.peer_score { // Opportunistic grafting works as follows: we check the median score of peers // in the mesh; if this score is below the opportunisticGraftThreshold, we // select a few peers at random with score over the median. @@ -2397,7 +2379,7 @@ where for (topic_hash, peers) in self.fanout.iter_mut() { let mut to_remove_peers = Vec::new(); let publish_threshold = match &self.peer_score { - Some(peer_score) => peer_score.thresholds.publish_threshold, + PeerScoreState::Active(peer_score) => peer_score.thresholds.publish_threshold, _ => 0.0, }; let mesh_n = self.config.mesh_n_for_topic(topic_hash); @@ -2444,7 +2426,7 @@ where } } - if self.peer_score.is_some() { + if let PeerScoreState::Active(peer_score) = &self.peer_score { tracing::trace!("Mesh message deliveries: {:?}", { self.mesh .iter() @@ -2454,14 +2436,7 @@ where peers .iter() .map(|p| { - ( - *p, - self.peer_score - .as_ref() - .expect("peer_score.is_some()") - .mesh_message_deliveries(p, t) - .unwrap_or(0.0), - ) + (*p, peer_score.mesh_message_deliveries(p, t).unwrap_or(0.0)) }) .collect::>(), ) @@ -2544,7 +2519,10 @@ where get_random_peers_dynamic(&self.connected_peers, topic_hash, n_map, |peer| { !peers.contains(peer) && !self.explicit_peers.contains(peer) - && !self.score_below_threshold(peer, |ts| ts.gossip_threshold).0 + && !self + .peer_score + .below_threshold(peer, |ts| ts.gossip_threshold) + .0 }); tracing::debug!("Gossiping IHAVE to {} peers", to_msg_peers.len()); @@ -2587,7 +2565,7 @@ where for (peer_id, topics) in to_graft.into_iter() { for topic in &topics { // inform scoring of graft - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.graft(&peer_id, topic.clone()); } @@ -2668,7 +2646,7 @@ where originating_peers: HashSet, ) -> bool { // message is fully validated inform peer_score - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { if let Some(peer) = propagation_source { peer_score.deliver_message(peer, msg_id, &message.topic); } @@ -2688,7 +2666,8 @@ where && (self.explicit_peers.contains(peer_id) || (peer.kind == PeerKind::Floodsub && !self - .score_below_threshold(peer_id, |ts| ts.publish_threshold) + .peer_score + .below_threshold(peer_id, |ts| ts.publish_threshold) .0)) { recipient_peers.insert(*peer_id); @@ -2883,7 +2862,7 @@ where } // Update peer score. - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.failed_message_slow_peer(&peer_id); } @@ -2902,7 +2881,7 @@ where }: ConnectionEstablished, ) { // Add the IP to the peer scoring system - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { if let Some(ip) = get_ip_addr(endpoint.get_remote_address()) { peer_score.add_ip(&peer_id, ip); } else { @@ -2918,7 +2897,7 @@ where return; // Not our first connection to this peer, hence nothing to do. } - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.add_peer(peer_id); } @@ -2946,7 +2925,7 @@ where }: ConnectionClosed, ) { // Remove IP from peer scoring system - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { if let Some(ip) = get_ip_addr(endpoint.get_remote_address()) { peer_score.remove_ip(&peer_id, &ip); } else { @@ -3026,7 +3005,7 @@ where self.connected_peers.remove(&peer_id); - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.remove_peer(&peer_id); } } @@ -3042,7 +3021,7 @@ where }: AddressChange, ) { // Exchange IP in peer scoring system - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { if let Some(ip) = get_ip_addr(endpoint_old.get_remote_address()) { peer_score.remove_ip(&peer_id, &ip); } else { @@ -3183,7 +3162,7 @@ where } HandlerEvent::MessageDropped(rpc) => { // Account for this in the scoring logic - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.failed_message_slow_peer(&propagation_source); } @@ -3228,15 +3207,16 @@ where } // Check if peer is graylisted in which case we ignore the event - if let (true, _) = - self.score_below_threshold(&propagation_source, |pst| pst.graylist_threshold) + if let (true, _) = self + .peer_score + .below_threshold(&propagation_source, |pst| pst.graylist_threshold) { tracing::debug!(peer=%propagation_source, "RPC Dropped from greylisted peer"); return; } // Handle any invalid messages from this peer - if self.peer_score.is_some() { + if let PeerScoreState::Active(_) = self.peer_score { for (raw_message, validation_error) in invalid_messages { self.handle_invalid_message( &propagation_source, @@ -3347,7 +3327,7 @@ where } // update scores - if let Some(peer_score) = &mut self.peer_score { + if let PeerScoreState::Active(peer_score) = &mut self.peer_score { if peer_score.decay_interval.poll_unpin(cx).is_ready() { peer_score.refresh_scores(); peer_score diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index ba5dfaff553..927035f4121 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -422,6 +422,14 @@ fn proto_to_message(rpc: &proto::RPC) -> Rpc { } } +impl Behaviour { + fn as_peer_score_mut(&mut self) -> &mut PeerScore { + match self.peer_score { + PeerScoreState::Active(ref mut peer_score) => peer_score, + PeerScoreState::Disabled => panic!("PeerScore is deactivated"), + } + } +} #[test] /// Test local node subscribing to a topic fn test_subscribe() { @@ -2519,7 +2527,7 @@ fn test_prune_negative_scored_peers() { .create_network(); // add penalty to peer - gs.peer_score.as_mut().unwrap().add_penalty(&peers[0], 1); + gs.as_peer_score_mut().add_penalty(&peers[0], 1); // execute heartbeat gs.heartbeat(); @@ -2567,7 +2575,7 @@ fn test_dont_graft_to_negative_scored_peers() { let (p2, _receiver2) = add_peer(&mut gs, &topics, false, false); // reduce score of p1 to negative - gs.peer_score.as_mut().unwrap().add_penalty(&p1, 1); + gs.as_peer_score_mut().add_penalty(&p1, 1); // handle prunes of all other peers for p in peers { @@ -2602,7 +2610,7 @@ fn test_ignore_px_from_negative_scored_peer() { .create_network(); // penalize peer - gs.peer_score.as_mut().unwrap().add_penalty(&peers[0], 1); + gs.as_peer_score_mut().add_penalty(&peers[0], 1); // handle prune from single peer with px peers let px = vec![PeerInfo { @@ -2651,7 +2659,7 @@ fn test_only_send_nonnegative_scoring_peers_in_px() { .create_network(); // Penalize first peer - gs.peer_score.as_mut().unwrap().add_penalty(&peers[0], 1); + gs.as_peer_score_mut().add_penalty(&peers[0], 1); // Prune second peer gs.send_graft_prune( @@ -2713,10 +2721,10 @@ fn test_do_not_gossip_to_peers_below_gossip_threshold() { // Reduce score of p1 below peer_score_thresholds.gossip_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); + gs.as_peer_score_mut().add_penalty(&p1, 2); // Reduce score of p2 below 0 but not below peer_score_thresholds.gossip_threshold - gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); + gs.as_peer_score_mut().add_penalty(&p2, 1); // Receive message let raw_message = RawMessage { @@ -2790,10 +2798,10 @@ fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { // Reduce score of p1 below peer_score_thresholds.gossip_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); + gs.as_peer_score_mut().add_penalty(&p1, 2); // Reduce score of p2 below 0 but not below peer_score_thresholds.gossip_threshold - gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); + gs.as_peer_score_mut().add_penalty(&p2, 1); // Receive message let raw_message = RawMessage { @@ -2880,10 +2888,10 @@ fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { // reduce score of p1 below peer_score_thresholds.gossip_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); + gs.as_peer_score_mut().add_penalty(&p1, 2); // reduce score of p2 below 0 but not below peer_score_thresholds.gossip_threshold - gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); + gs.as_peer_score_mut().add_penalty(&p2, 1); // message that other peers have let raw_message = RawMessage { @@ -2951,10 +2959,10 @@ fn test_do_not_publish_to_peer_below_publish_threshold() { // reduce score of p1 below peer_score_thresholds.publish_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); + gs.as_peer_score_mut().add_penalty(&p1, 2); // reduce score of p2 below 0 but not below peer_score_thresholds.publish_threshold - gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); + gs.as_peer_score_mut().add_penalty(&p2, 1); // a heartbeat will remove the peers from the mesh gs.heartbeat(); @@ -3006,10 +3014,10 @@ fn test_do_not_flood_publish_to_peer_below_publish_threshold() { // reduce score of p1 below peer_score_thresholds.publish_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); + gs.as_peer_score_mut().add_penalty(&p1, 2); // reduce score of p2 below 0 but not below peer_score_thresholds.publish_threshold - gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); + gs.as_peer_score_mut().add_penalty(&p2, 1); // a heartbeat will remove the peers from the mesh gs.heartbeat(); @@ -3061,10 +3069,10 @@ fn test_ignore_rpc_from_peers_below_graylist_threshold() { // reduce score of p1 below peer_score_thresholds.graylist_threshold // note that penalties get squared so two penalties means a score of // 4 * peer_score_params.behaviour_penalty_weight. - gs.peer_score.as_mut().unwrap().add_penalty(&p1, 2); + gs.as_peer_score_mut().add_penalty(&p1, 2); // reduce score of p2 below publish_threshold but not below graylist_threshold - gs.peer_score.as_mut().unwrap().add_penalty(&p2, 1); + gs.as_peer_score_mut().add_penalty(&p2, 1); let raw_message1 = RawMessage { source: Some(PeerId::random()), @@ -3322,14 +3330,14 @@ fn test_scoring_p1() { // sleep for 2 times the mesh_quantum sleep(topic_params.time_in_mesh_quantum * 2); // refresh scores - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); assert!( - gs.peer_score.as_ref().unwrap().score(&peers[0]) + gs.as_peer_score_mut().score(&peers[0]) >= 2.0 * topic_params.time_in_mesh_weight * topic_params.topic_weight, "score should be at least 2 * time_in_mesh_weight * topic_weight" ); assert!( - gs.peer_score.as_ref().unwrap().score(&peers[0]) + gs.as_peer_score_mut().score(&peers[0]) < 3.0 * topic_params.time_in_mesh_weight * topic_params.topic_weight, "score should be less than 3 * time_in_mesh_weight * topic_weight" ); @@ -3337,9 +3345,9 @@ fn test_scoring_p1() { // sleep again for 2 times the mesh_quantum sleep(topic_params.time_in_mesh_quantum * 2); // refresh scores - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); assert!( - gs.peer_score.as_ref().unwrap().score(&peers[0]) + gs.as_peer_score_mut().score(&peers[0]) >= 2.0 * topic_params.time_in_mesh_weight * topic_params.topic_weight, "score should be at least 4 * time_in_mesh_weight * topic_weight" ); @@ -3347,9 +3355,9 @@ fn test_scoring_p1() { // sleep for enough periods to reach maximum sleep(topic_params.time_in_mesh_quantum * (topic_params.time_in_mesh_cap - 3.0) as u32); // refresh scores - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), + gs.as_peer_score_mut().score(&peers[0]), topic_params.time_in_mesh_cap * topic_params.time_in_mesh_weight * topic_params.topic_weight, @@ -3413,13 +3421,13 @@ fn test_scoring_p2() { deliver_message(&mut gs, 1, m1); assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), + gs.as_peer_score_mut().score(&peers[0]), 1.0 * topic_params.first_message_deliveries_weight * topic_params.topic_weight, "score should be exactly first_message_deliveries_weight * topic_weight" ); assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[1]), + gs.as_peer_score_mut().score(&peers[1]), 0.0, "there should be no score for second message deliveries * topic_weight" ); @@ -3428,16 +3436,16 @@ fn test_scoring_p2() { deliver_message(&mut gs, 1, random_message(&mut seq, &topics)); deliver_message(&mut gs, 1, random_message(&mut seq, &topics)); assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[1]), + gs.as_peer_score_mut().score(&peers[1]), 2.0 * topic_params.first_message_deliveries_weight * topic_params.topic_weight, "score should be exactly 2 * first_message_deliveries_weight * topic_weight" ); // test decaying - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), + gs.as_peer_score_mut().score(&peers[0]), 1.0 * topic_params.first_message_deliveries_decay * topic_params.first_message_deliveries_weight * topic_params.topic_weight, @@ -3446,7 +3454,7 @@ fn test_scoring_p2() { ); assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[1]), + gs.as_peer_score_mut().score(&peers[1]), 2.0 * topic_params.first_message_deliveries_decay * topic_params.first_message_deliveries_weight * topic_params.topic_weight, @@ -3460,7 +3468,7 @@ fn test_scoring_p2() { } assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[1]), + gs.as_peer_score_mut().score(&peers[1]), topic_params.first_message_deliveries_cap * topic_params.first_message_deliveries_weight * topic_params.topic_weight, @@ -3535,11 +3543,11 @@ fn test_scoring_p3() { // message deliveries penalties get activated, peer 0 has only delivered 3 messages and // therefore gets a penalty - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); expected_message_deliveries *= 0.9; // decay assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), + gs.as_peer_score_mut().score(&peers[0]), (5f64 - expected_message_deliveries).powi(2) * -2.0 * 0.7 ); @@ -3550,16 +3558,16 @@ fn test_scoring_p3() { expected_message_deliveries = 10.0; - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); // apply 10 decays for _ in 0..10 { - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); expected_message_deliveries *= 0.9; // decay } assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), + gs.as_peer_score_mut().score(&peers[0]), (5f64 - expected_message_deliveries).powi(2) * -2.0 * 0.7 ); } @@ -3610,9 +3618,7 @@ fn test_scoring_p3b() { let mut expected_message_deliveries = 0.0; // add some positive score - gs.peer_score - .as_mut() - .unwrap() + gs.as_peer_score_mut() .set_application_score(&peers[0], 100.0); // peer 0 delivers two message @@ -3623,7 +3629,7 @@ fn test_scoring_p3b() { sleep(Duration::from_millis(1050)); // activation kicks in - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); expected_message_deliveries *= 0.9; // decay // prune peer @@ -3638,7 +3644,7 @@ fn test_scoring_p3b() { // the score should now consider p3b let mut expected_b3 = (5f64 - expected_message_deliveries).powi(2); assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), + gs.as_peer_score_mut().score(&peers[0]), 100.0 + expected_b3 * -3.0 * 0.7 ); @@ -3649,12 +3655,12 @@ fn test_scoring_p3b() { expected_message_deliveries += 1.0; sleep(Duration::from_millis(1050)); - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); expected_message_deliveries *= 0.9; // decay expected_b3 *= 0.95; assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), + gs.as_peer_score_mut().score(&peers[0]), 100.0 + (expected_b3 * -3.0 + (5f64 - expected_message_deliveries).powi(2) * -2.0) * 0.7 ); } @@ -3709,7 +3715,7 @@ fn test_scoring_p4_valid_message() { // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); // message m1 gets validated gs.report_message_validation_result( @@ -3718,7 +3724,7 @@ fn test_scoring_p4_valid_message() { MessageAcceptance::Accept, ); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); } #[test] @@ -3777,7 +3783,7 @@ fn test_scoring_p4_invalid_signature() { }, ); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); } #[test] @@ -3828,7 +3834,7 @@ fn test_scoring_p4_message_from_self() { m.source = Some(*gs.publish_config.get_own_id().unwrap()); deliver_message(&mut gs, 0, m); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); } #[test] @@ -3878,7 +3884,7 @@ fn test_scoring_p4_ignored_message() { let m1 = random_message(&mut seq, &topics); deliver_message(&mut gs, 0, m1.clone()); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); @@ -3890,7 +3896,7 @@ fn test_scoring_p4_ignored_message() { MessageAcceptance::Ignore, ); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); } #[test] @@ -3940,7 +3946,7 @@ fn test_scoring_p4_application_invalidated_message() { let m1 = random_message(&mut seq, &topics); deliver_message(&mut gs, 0, m1.clone()); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); @@ -3952,7 +3958,7 @@ fn test_scoring_p4_application_invalidated_message() { MessageAcceptance::Reject, ); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); } #[test] @@ -4008,8 +4014,8 @@ fn test_scoring_p4_application_invalid_message_from_two_peers() { // peer 1 delivers same message deliver_message(&mut gs, 1, m1); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[1]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[1]), 0.0); // message m1 gets rejected gs.report_message_validation_result( @@ -4018,8 +4024,8 @@ fn test_scoring_p4_application_invalid_message_from_two_peers() { MessageAcceptance::Reject, ); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[1]), -2.0 * 0.7); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); + assert_eq!(gs.as_peer_score_mut().score(&peers[1]), -2.0 * 0.7); } #[test] @@ -4081,7 +4087,7 @@ fn test_scoring_p4_three_application_invalid_messages() { // Transform the inbound message let message3 = &gs.data_transform.inbound_transform(m3).unwrap(); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); // messages gets rejected gs.report_message_validation_result( @@ -4103,10 +4109,7 @@ fn test_scoring_p4_three_application_invalid_messages() { ); // number of invalid messages gets squared - assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), - 9.0 * -2.0 * 0.7 - ); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 9.0 * -2.0 * 0.7); } #[test] @@ -4158,7 +4161,7 @@ fn test_scoring_p4_decay() { // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); // message m1 gets rejected gs.report_message_validation_result( @@ -4167,14 +4170,14 @@ fn test_scoring_p4_decay() { MessageAcceptance::Reject, ); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), -2.0 * 0.7); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); // we decay - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); // the number of invalids gets decayed to 0.9 and then squared in the score assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), + gs.as_peer_score_mut().score(&peers[0]), 0.9 * 0.9 * -2.0 * 0.7 ); } @@ -4199,7 +4202,7 @@ fn test_scoring_p5() { gs.set_application_score(&peers[0], 1.1); - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 1.1 * 2.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 1.1 * 2.0); } #[test] @@ -4241,7 +4244,7 @@ fn test_scoring_p6() { // no penalties yet for peer in peers.iter().chain(others.iter()) { - assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 0.0); + assert_eq!(gs.as_peer_score_mut().score(peer), 0.0); } // add additional connection for 3 others with addr @@ -4261,10 +4264,10 @@ fn test_scoring_p6() { // penalties apply squared for peer in peers.iter().chain(others.iter().take(3)) { - assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 9.0 * -2.0); + assert_eq!(gs.as_peer_score_mut().score(peer), 9.0 * -2.0); } // fourth other peer still no penalty - assert_eq!(gs.peer_score.as_ref().unwrap().score(&others[3]), 0.0); + assert_eq!(gs.as_peer_score_mut().score(&others[3]), 0.0); // add additional connection for 3 of the peers to addr2 for peer in peers.iter().take(3) { @@ -4283,20 +4286,14 @@ fn test_scoring_p6() { // double penalties for the first three of each for peer in peers.iter().take(3).chain(others.iter().take(3)) { - assert_eq!( - gs.peer_score.as_ref().unwrap().score(peer), - (9.0 + 4.0) * -2.0 - ); + assert_eq!(gs.as_peer_score_mut().score(peer), (9.0 + 4.0) * -2.0); } // single penalties for the rest for peer in peers.iter().skip(3) { - assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 9.0 * -2.0); + assert_eq!(gs.as_peer_score_mut().score(peer), 9.0 * -2.0); } - assert_eq!( - gs.peer_score.as_ref().unwrap().score(&others[3]), - 4.0 * -2.0 - ); + assert_eq!(gs.as_peer_score_mut().score(&others[3]), 4.0 * -2.0); // two times same ip doesn't count twice gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished { @@ -4314,20 +4311,14 @@ fn test_scoring_p6() { // nothing changed // double penalties for the first three of each for peer in peers.iter().take(3).chain(others.iter().take(3)) { - assert_eq!( - gs.peer_score.as_ref().unwrap().score(peer), - (9.0 + 4.0) * -2.0 - ); + assert_eq!(gs.as_peer_score_mut().score(peer), (9.0 + 4.0) * -2.0); } // single penalties for the rest for peer in peers.iter().skip(3) { - assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 9.0 * -2.0); + assert_eq!(gs.as_peer_score_mut().score(peer), 9.0 * -2.0); } - assert_eq!( - gs.peer_score.as_ref().unwrap().score(&others[3]), - 4.0 * -2.0 - ); + assert_eq!(gs.as_peer_score_mut().score(&others[3]), 4.0 * -2.0); } #[test] @@ -4370,7 +4361,7 @@ fn test_scoring_p7_grafts_before_backoff() { gs.handle_graft(&peers[0], vec![topics[0].clone()]); // double behaviour penalty for first peer (squared) - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[0]), 4.0 * -2.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 4.0 * -2.0); // wait 100 millisecs sleep(Duration::from_millis(100)); @@ -4379,17 +4370,17 @@ fn test_scoring_p7_grafts_before_backoff() { gs.handle_graft(&peers[1], vec![topics[0].clone()]); // single behaviour penalty for second peer - assert_eq!(gs.peer_score.as_ref().unwrap().score(&peers[1]), 1.0 * -2.0); + assert_eq!(gs.as_peer_score_mut().score(&peers[1]), 1.0 * -2.0); // test decay - gs.peer_score.as_mut().unwrap().refresh_scores(); + gs.as_peer_score_mut().refresh_scores(); assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[0]), + gs.as_peer_score_mut().score(&peers[0]), 4.0 * 0.9 * 0.9 * -2.0 ); assert_eq!( - gs.peer_score.as_ref().unwrap().score(&peers[1]), + gs.as_peer_score_mut().score(&peers[1]), 1.0 * 0.9 * 0.9 * -2.0 ); } @@ -4869,7 +4860,7 @@ fn test_iwant_penalties() { gs.heartbeat(); for (peer, _receiver) in &other_peers { - assert_eq!(gs.peer_score.as_ref().unwrap().score(peer), 0.0); + assert_eq!(gs.as_peer_score_mut().score(peer), 0.0); } // receive the first twenty of the other peers then send their response @@ -4899,7 +4890,7 @@ fn test_iwant_penalties() { let mut double_penalized = 0; for (i, (peer, _receiver)) in other_peers.iter().enumerate() { - let score = gs.peer_score.as_ref().unwrap().score(peer); + let score = gs.as_peer_score_mut().score(peer); if score == 0.0 { not_penalized += 1; } else if score == -1.0 { @@ -5229,9 +5220,9 @@ fn test_subscribe_and_graft_with_negative_score() { let (p1, _receiver2) = add_peer(&mut gs2, &topic_hashes, false, false); // add penalty to peer p2 - gs1.peer_score.as_mut().unwrap().add_penalty(&p2, 1); + gs1.as_peer_score_mut().add_penalty(&p2, 1); - let original_score = gs1.peer_score.as_ref().unwrap().score(&p2); + let original_score = gs1.as_peer_score_mut().score(&p2); // subscribe to topic in gs2 gs2.subscribe(&topic).unwrap(); @@ -5273,7 +5264,7 @@ fn test_subscribe_and_graft_with_negative_score() { forward_messages_to_p1(&mut gs1, p1, p2, connection_id, receivers); // nobody got penalized - assert!(gs1.peer_score.as_ref().unwrap().score(&p2) >= original_score); + assert!(gs1.as_peer_score_mut().score(&p2) >= original_score); } #[test] @@ -5982,7 +5973,7 @@ fn test_slow_peer_is_downscored_on_publish() { dont_send: LinkedHashMap::new(), }, ); - gs.peer_score.as_mut().unwrap().add_peer(slow_peer_id); + gs.as_peer_score_mut().add_peer(slow_peer_id); let peer_id = PeerId::random(); peers.push(peer_id); gs.connected_peers.insert( @@ -6003,7 +5994,7 @@ fn test_slow_peer_is_downscored_on_publish() { let publish_data = vec![2; 59]; gs.publish(topic_hash.clone(), publish_data).unwrap(); gs.heartbeat(); - let slow_peer_score = gs.peer_score(&slow_peer_id).unwrap(); + let slow_peer_score = gs.as_peer_score_mut().score(&slow_peer_id); assert_eq!(slow_peer_score, slow_peer_params.slow_peer_weight); } diff --git a/protocols/gossipsub/src/peer_score.rs b/protocols/gossipsub/src/peer_score.rs index 07956b388ac..e2afe38a3ab 100644 --- a/protocols/gossipsub/src/peer_score.rs +++ b/protocols/gossipsub/src/peer_score.rs @@ -50,6 +50,31 @@ mod tests; /// The number of seconds delivery messages are stored in the cache. const TIME_CACHE_DURATION: u64 = 120; +/// Represents the state of the peer scoring system, which can either be active +/// with a configured `PeerScore`, or disabled entirely. +pub(crate) enum PeerScoreState { + Active(Box), + Disabled, +} + +impl PeerScoreState { + /// Determines if a peer's score is below a given `PeerScoreThreshold` chosen via the + /// `threshold` parameter. + pub(crate) fn below_threshold( + &self, + peer_id: &PeerId, + threshold: impl Fn(&PeerScoreThresholds) -> f64, + ) -> (bool, f64) { + match self { + PeerScoreState::Active(active) => { + let score = active.metric_score(peer_id, None); + (score < threshold(&active.thresholds), score) + } + PeerScoreState::Disabled => (false, 0.0), + } + } +} + pub(crate) struct PeerScore { /// The score parameters. pub(crate) params: PeerScoreParams, From 07114a42cc7b9b933d11d9c6dd24d2b9886eeb6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Mon, 12 May 2025 10:21:31 +0100 Subject: [PATCH 3/6] decouple metrics from score by returning the penalties in the score report insteadw --- protocols/gossipsub/src/behaviour.rs | 44 +++--- protocols/gossipsub/src/behaviour/tests.rs | 144 +++++++++++++------- protocols/gossipsub/src/metrics.rs | 2 +- protocols/gossipsub/src/peer_score.rs | 57 ++++---- protocols/gossipsub/src/peer_score/tests.rs | 86 ++++++------ 5 files changed, 191 insertions(+), 142 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 08111f1ef35..6285178514e 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -2097,9 +2097,14 @@ where let mut scores = HashMap::with_capacity(self.connected_peers.len()); if let PeerScoreState::Active(peer_score) = &self.peer_score { for peer_id in self.connected_peers.keys() { - scores + let report = scores .entry(peer_id) - .or_insert_with(|| peer_score.metric_score(peer_id, self.metrics.as_mut())); + .or_insert_with(|| peer_score.score_report(peer_id)); + if let Some(metrics) = self.metrics.as_mut() { + for penalty in &report.penalties { + metrics.register_score_penalty(*penalty); + } + } } } @@ -2118,7 +2123,7 @@ where // written more efficiently with retain. let mut to_remove_peers = Vec::new(); for peer_id in peers.iter() { - let peer_score = *scores.get(peer_id).unwrap_or(&0.0); + let peer_score = scores.get(peer_id).map(|r| r.score).unwrap_or_default(); // Record the score per mesh if let Some(metrics) = self.metrics.as_mut() { @@ -2163,7 +2168,7 @@ where !peers.contains(peer) && !explicit_peers.contains(peer) && !backoffs.is_backoff_with_slack(topic_hash, peer) - && *scores.get(peer).unwrap_or(&0.0) >= 0.0 + && scores.get(peer).map(|r| r.score).unwrap_or_default() >= 0.0 }); for peer in &peer_list { let current_topic = to_graft.entry(*peer).or_insert_with(Vec::new); @@ -2192,8 +2197,8 @@ where let mut shuffled = peers.iter().copied().collect::>(); shuffled.shuffle(&mut rng); shuffled.sort_by(|p1, p2| { - let score_p1 = *scores.get(p1).unwrap_or(&0.0); - let score_p2 = *scores.get(p2).unwrap_or(&0.0); + let score_p1 = scores.get(p1).map(|r| r.score).unwrap_or_default(); + let score_p2 = scores.get(p2).map(|r| r.score).unwrap_or_default(); score_p1.partial_cmp(&score_p2).unwrap_or(Ordering::Equal) }); @@ -2262,7 +2267,7 @@ where !peers.contains(peer_id) && !explicit_peers.contains(peer_id) && !backoffs.is_backoff_with_slack(topic_hash, peer_id) - && *scores.get(peer_id).unwrap_or(&0.0) >= 0.0 + && scores.get(peer_id).map(|r| r.score).unwrap_or_default() >= 0.0 && self .connected_peers .get(peer_id) @@ -2298,8 +2303,8 @@ where // now compute the median peer score in the mesh let mut peers_by_score: Vec<_> = peers.iter().collect(); peers_by_score.sort_by(|p1, p2| { - let p1_score = *scores.get(p1).unwrap_or(&0.0); - let p2_score = *scores.get(p2).unwrap_or(&0.0); + let p1_score = scores.get(p1).map(|r| r.score).unwrap_or_default(); + let p2_score = scores.get(p2).map(|r| r.score).unwrap_or_default(); p1_score.partial_cmp(&p2_score).unwrap_or(Equal) }); @@ -2308,16 +2313,21 @@ where let sub_middle_peer = *peers_by_score .get(middle - 1) .expect("middle < vector length and middle > 0 since peers.len() > 0"); - let sub_middle_score = *scores.get(sub_middle_peer).unwrap_or(&0.0); + let sub_middle_score = scores + .get(sub_middle_peer) + .map(|r| r.score) + .unwrap_or_default(); let middle_peer = *peers_by_score.get(middle).expect("middle < vector length"); - let middle_score = *scores.get(middle_peer).unwrap_or(&0.0); + let middle_score = + scores.get(middle_peer).map(|r| r.score).unwrap_or_default(); (sub_middle_score + middle_score) * 0.5 } else { - *scores + scores .get(*peers_by_score.get(middle).expect("middle < vector length")) - .unwrap_or(&0.0) + .map(|r| r.score) + .unwrap_or_default() }; // if the median score is below the threshold, select a better peer (if any) and @@ -2331,7 +2341,8 @@ where !peers.contains(peer_id) && !explicit_peers.contains(peer_id) && !backoffs.is_backoff_with_slack(topic_hash, peer_id) - && *scores.get(peer_id).unwrap_or(&0.0) > median + && scores.get(peer_id).map(|r| r.score).unwrap_or_default() + > median }, ); for peer in &peer_list { @@ -2386,7 +2397,7 @@ where for peer_id in peers.iter() { // is the peer still subscribed to the topic? - let peer_score = *scores.get(peer_id).unwrap_or(&0.0); + let peer_score = scores.get(peer_id).map(|r| r.score).unwrap_or_default(); match self.connected_peers.get(peer_id) { Some(peer) => { if !peer.topics.contains(topic_hash) || peer_score < publish_threshold { @@ -2420,7 +2431,8 @@ where get_random_peers(&self.connected_peers, topic_hash, needed_peers, |peer_id| { !peers.contains(peer_id) && !explicit_peers.contains(peer_id) - && *scores.get(peer_id).unwrap_or(&0.0) < publish_threshold + && scores.get(peer_id).map(|r| r.score).unwrap_or_default() + < publish_threshold }); peers.extend(new_peers); } diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index 927035f4121..d4ed72fac05 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -3332,12 +3332,12 @@ fn test_scoring_p1() { // refresh scores gs.as_peer_score_mut().refresh_scores(); assert!( - gs.as_peer_score_mut().score(&peers[0]) + gs.as_peer_score_mut().score_report(&peers[0]).score >= 2.0 * topic_params.time_in_mesh_weight * topic_params.topic_weight, "score should be at least 2 * time_in_mesh_weight * topic_weight" ); assert!( - gs.as_peer_score_mut().score(&peers[0]) + gs.as_peer_score_mut().score_report(&peers[0]).score < 3.0 * topic_params.time_in_mesh_weight * topic_params.topic_weight, "score should be less than 3 * time_in_mesh_weight * topic_weight" ); @@ -3347,7 +3347,7 @@ fn test_scoring_p1() { // refresh scores gs.as_peer_score_mut().refresh_scores(); assert!( - gs.as_peer_score_mut().score(&peers[0]) + gs.as_peer_score_mut().score_report(&peers[0]).score >= 2.0 * topic_params.time_in_mesh_weight * topic_params.topic_weight, "score should be at least 4 * time_in_mesh_weight * topic_weight" ); @@ -3357,7 +3357,7 @@ fn test_scoring_p1() { // refresh scores gs.as_peer_score_mut().refresh_scores(); assert_eq!( - gs.as_peer_score_mut().score(&peers[0]), + gs.as_peer_score_mut().score_report(&peers[0]).score, topic_params.time_in_mesh_cap * topic_params.time_in_mesh_weight * topic_params.topic_weight, @@ -3421,13 +3421,13 @@ fn test_scoring_p2() { deliver_message(&mut gs, 1, m1); assert_eq!( - gs.as_peer_score_mut().score(&peers[0]), + gs.as_peer_score_mut().score_report(&peers[0]).score, 1.0 * topic_params.first_message_deliveries_weight * topic_params.topic_weight, "score should be exactly first_message_deliveries_weight * topic_weight" ); assert_eq!( - gs.as_peer_score_mut().score(&peers[1]), + gs.as_peer_score_mut().score_report(&peers[1]).score, 0.0, "there should be no score for second message deliveries * topic_weight" ); @@ -3436,7 +3436,7 @@ fn test_scoring_p2() { deliver_message(&mut gs, 1, random_message(&mut seq, &topics)); deliver_message(&mut gs, 1, random_message(&mut seq, &topics)); assert_eq!( - gs.as_peer_score_mut().score(&peers[1]), + gs.as_peer_score_mut().score_report(&peers[1]).score, 2.0 * topic_params.first_message_deliveries_weight * topic_params.topic_weight, "score should be exactly 2 * first_message_deliveries_weight * topic_weight" ); @@ -3445,7 +3445,7 @@ fn test_scoring_p2() { gs.as_peer_score_mut().refresh_scores(); assert_eq!( - gs.as_peer_score_mut().score(&peers[0]), + gs.as_peer_score_mut().score_report(&peers[0]).score, 1.0 * topic_params.first_message_deliveries_decay * topic_params.first_message_deliveries_weight * topic_params.topic_weight, @@ -3454,7 +3454,7 @@ fn test_scoring_p2() { ); assert_eq!( - gs.as_peer_score_mut().score(&peers[1]), + gs.as_peer_score_mut().score_report(&peers[1]).score, 2.0 * topic_params.first_message_deliveries_decay * topic_params.first_message_deliveries_weight * topic_params.topic_weight, @@ -3468,7 +3468,7 @@ fn test_scoring_p2() { } assert_eq!( - gs.as_peer_score_mut().score(&peers[1]), + gs.as_peer_score_mut().score_report(&peers[1]).score, topic_params.first_message_deliveries_cap * topic_params.first_message_deliveries_weight * topic_params.topic_weight, @@ -3547,7 +3547,7 @@ fn test_scoring_p3() { expected_message_deliveries *= 0.9; // decay assert_eq!( - gs.as_peer_score_mut().score(&peers[0]), + gs.as_peer_score_mut().score_report(&peers[0]).score, (5f64 - expected_message_deliveries).powi(2) * -2.0 * 0.7 ); @@ -3558,7 +3558,7 @@ fn test_scoring_p3() { expected_message_deliveries = 10.0; - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[0]).score, 0.0); // apply 10 decays for _ in 0..10 { @@ -3567,7 +3567,7 @@ fn test_scoring_p3() { } assert_eq!( - gs.as_peer_score_mut().score(&peers[0]), + gs.as_peer_score_mut().score_report(&peers[0]).score, (5f64 - expected_message_deliveries).powi(2) * -2.0 * 0.7 ); } @@ -3644,7 +3644,7 @@ fn test_scoring_p3b() { // the score should now consider p3b let mut expected_b3 = (5f64 - expected_message_deliveries).powi(2); assert_eq!( - gs.as_peer_score_mut().score(&peers[0]), + gs.as_peer_score_mut().score_report(&peers[0]).score, 100.0 + expected_b3 * -3.0 * 0.7 ); @@ -3660,7 +3660,7 @@ fn test_scoring_p3b() { expected_b3 *= 0.95; assert_eq!( - gs.as_peer_score_mut().score(&peers[0]), + gs.as_peer_score_mut().score_report(&peers[0]).score, 100.0 + (expected_b3 * -3.0 + (5f64 - expected_message_deliveries).powi(2) * -2.0) * 0.7 ); } @@ -3715,7 +3715,7 @@ fn test_scoring_p4_valid_message() { // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[0]).score, 0.0); // message m1 gets validated gs.report_message_validation_result( @@ -3724,7 +3724,7 @@ fn test_scoring_p4_valid_message() { MessageAcceptance::Accept, ); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[0]).score, 0.0); } #[test] @@ -3783,7 +3783,10 @@ fn test_scoring_p4_invalid_signature() { }, ); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[0]).score, + -2.0 * 0.7 + ); } #[test] @@ -3834,7 +3837,10 @@ fn test_scoring_p4_message_from_self() { m.source = Some(*gs.publish_config.get_own_id().unwrap()); deliver_message(&mut gs, 0, m); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[0]).score, + -2.0 * 0.7 + ); } #[test] @@ -3884,7 +3890,7 @@ fn test_scoring_p4_ignored_message() { let m1 = random_message(&mut seq, &topics); deliver_message(&mut gs, 0, m1.clone()); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[0]).score, 0.0); // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); @@ -3896,7 +3902,7 @@ fn test_scoring_p4_ignored_message() { MessageAcceptance::Ignore, ); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[0]).score, 0.0); } #[test] @@ -3946,7 +3952,7 @@ fn test_scoring_p4_application_invalidated_message() { let m1 = random_message(&mut seq, &topics); deliver_message(&mut gs, 0, m1.clone()); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[0]).score, 0.0); // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); @@ -3958,7 +3964,10 @@ fn test_scoring_p4_application_invalidated_message() { MessageAcceptance::Reject, ); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[0]).score, + -2.0 * 0.7 + ); } #[test] @@ -4014,8 +4023,8 @@ fn test_scoring_p4_application_invalid_message_from_two_peers() { // peer 1 delivers same message deliver_message(&mut gs, 1, m1); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); - assert_eq!(gs.as_peer_score_mut().score(&peers[1]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[0]).score, 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[1]).score, 0.0); // message m1 gets rejected gs.report_message_validation_result( @@ -4024,8 +4033,14 @@ fn test_scoring_p4_application_invalid_message_from_two_peers() { MessageAcceptance::Reject, ); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); - assert_eq!(gs.as_peer_score_mut().score(&peers[1]), -2.0 * 0.7); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[0]).score, + -2.0 * 0.7 + ); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[1]).score, + -2.0 * 0.7 + ); } #[test] @@ -4087,7 +4102,7 @@ fn test_scoring_p4_three_application_invalid_messages() { // Transform the inbound message let message3 = &gs.data_transform.inbound_transform(m3).unwrap(); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[0]).score, 0.0); // messages gets rejected gs.report_message_validation_result( @@ -4109,7 +4124,10 @@ fn test_scoring_p4_three_application_invalid_messages() { ); // number of invalid messages gets squared - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 9.0 * -2.0 * 0.7); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[0]).score, + 9.0 * -2.0 * 0.7 + ); } #[test] @@ -4161,7 +4179,7 @@ fn test_scoring_p4_decay() { // Transform the inbound message let message1 = &gs.data_transform.inbound_transform(m1).unwrap(); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&peers[0]).score, 0.0); // message m1 gets rejected gs.report_message_validation_result( @@ -4170,14 +4188,17 @@ fn test_scoring_p4_decay() { MessageAcceptance::Reject, ); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), -2.0 * 0.7); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[0]).score, + -2.0 * 0.7 + ); // we decay gs.as_peer_score_mut().refresh_scores(); // the number of invalids gets decayed to 0.9 and then squared in the score assert_eq!( - gs.as_peer_score_mut().score(&peers[0]), + gs.as_peer_score_mut().score_report(&peers[0]).score, 0.9 * 0.9 * -2.0 * 0.7 ); } @@ -4202,7 +4223,10 @@ fn test_scoring_p5() { gs.set_application_score(&peers[0], 1.1); - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 1.1 * 2.0); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[0]).score, + 1.1 * 2.0 + ); } #[test] @@ -4244,7 +4268,7 @@ fn test_scoring_p6() { // no penalties yet for peer in peers.iter().chain(others.iter()) { - assert_eq!(gs.as_peer_score_mut().score(peer), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(peer).score, 0.0); } // add additional connection for 3 others with addr @@ -4264,10 +4288,10 @@ fn test_scoring_p6() { // penalties apply squared for peer in peers.iter().chain(others.iter().take(3)) { - assert_eq!(gs.as_peer_score_mut().score(peer), 9.0 * -2.0); + assert_eq!(gs.as_peer_score_mut().score_report(peer).score, 9.0 * -2.0); } // fourth other peer still no penalty - assert_eq!(gs.as_peer_score_mut().score(&others[3]), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(&others[3]).score, 0.0); // add additional connection for 3 of the peers to addr2 for peer in peers.iter().take(3) { @@ -4286,14 +4310,20 @@ fn test_scoring_p6() { // double penalties for the first three of each for peer in peers.iter().take(3).chain(others.iter().take(3)) { - assert_eq!(gs.as_peer_score_mut().score(peer), (9.0 + 4.0) * -2.0); + assert_eq!( + gs.as_peer_score_mut().score_report(peer).score, + (9.0 + 4.0) * -2.0 + ); } // single penalties for the rest for peer in peers.iter().skip(3) { - assert_eq!(gs.as_peer_score_mut().score(peer), 9.0 * -2.0); + assert_eq!(gs.as_peer_score_mut().score_report(peer).score, 9.0 * -2.0); } - assert_eq!(gs.as_peer_score_mut().score(&others[3]), 4.0 * -2.0); + assert_eq!( + gs.as_peer_score_mut().score_report(&others[3]).score, + 4.0 * -2.0 + ); // two times same ip doesn't count twice gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished { @@ -4311,14 +4341,20 @@ fn test_scoring_p6() { // nothing changed // double penalties for the first three of each for peer in peers.iter().take(3).chain(others.iter().take(3)) { - assert_eq!(gs.as_peer_score_mut().score(peer), (9.0 + 4.0) * -2.0); + assert_eq!( + gs.as_peer_score_mut().score_report(peer).score, + (9.0 + 4.0) * -2.0 + ); } // single penalties for the rest for peer in peers.iter().skip(3) { - assert_eq!(gs.as_peer_score_mut().score(peer), 9.0 * -2.0); + assert_eq!(gs.as_peer_score_mut().score_report(peer).score, 9.0 * -2.0); } - assert_eq!(gs.as_peer_score_mut().score(&others[3]), 4.0 * -2.0); + assert_eq!( + gs.as_peer_score_mut().score_report(&others[3]).score, + 4.0 * -2.0 + ); } #[test] @@ -4361,7 +4397,10 @@ fn test_scoring_p7_grafts_before_backoff() { gs.handle_graft(&peers[0], vec![topics[0].clone()]); // double behaviour penalty for first peer (squared) - assert_eq!(gs.as_peer_score_mut().score(&peers[0]), 4.0 * -2.0); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[0]).score, + 4.0 * -2.0 + ); // wait 100 millisecs sleep(Duration::from_millis(100)); @@ -4370,17 +4409,20 @@ fn test_scoring_p7_grafts_before_backoff() { gs.handle_graft(&peers[1], vec![topics[0].clone()]); // single behaviour penalty for second peer - assert_eq!(gs.as_peer_score_mut().score(&peers[1]), 1.0 * -2.0); + assert_eq!( + gs.as_peer_score_mut().score_report(&peers[1]).score, + 1.0 * -2.0 + ); // test decay gs.as_peer_score_mut().refresh_scores(); assert_eq!( - gs.as_peer_score_mut().score(&peers[0]), + gs.as_peer_score_mut().score_report(&peers[0]).score, 4.0 * 0.9 * 0.9 * -2.0 ); assert_eq!( - gs.as_peer_score_mut().score(&peers[1]), + gs.as_peer_score_mut().score_report(&peers[1]).score, 1.0 * 0.9 * 0.9 * -2.0 ); } @@ -4860,7 +4902,7 @@ fn test_iwant_penalties() { gs.heartbeat(); for (peer, _receiver) in &other_peers { - assert_eq!(gs.as_peer_score_mut().score(peer), 0.0); + assert_eq!(gs.as_peer_score_mut().score_report(peer).score, 0.0); } // receive the first twenty of the other peers then send their response @@ -4890,7 +4932,7 @@ fn test_iwant_penalties() { let mut double_penalized = 0; for (i, (peer, _receiver)) in other_peers.iter().enumerate() { - let score = gs.as_peer_score_mut().score(peer); + let score = gs.as_peer_score_mut().score_report(peer).score; if score == 0.0 { not_penalized += 1; } else if score == -1.0 { @@ -5222,7 +5264,7 @@ fn test_subscribe_and_graft_with_negative_score() { // add penalty to peer p2 gs1.as_peer_score_mut().add_penalty(&p2, 1); - let original_score = gs1.as_peer_score_mut().score(&p2); + let original_score = gs1.as_peer_score_mut().score_report(&p2).score; // subscribe to topic in gs2 gs2.subscribe(&topic).unwrap(); @@ -5264,7 +5306,7 @@ fn test_subscribe_and_graft_with_negative_score() { forward_messages_to_p1(&mut gs1, p1, p2, connection_id, receivers); // nobody got penalized - assert!(gs1.as_peer_score_mut().score(&p2) >= original_score); + assert!(gs1.as_peer_score_mut().score_report(&p2).score >= original_score); } #[test] @@ -5994,7 +6036,7 @@ fn test_slow_peer_is_downscored_on_publish() { let publish_data = vec![2; 59]; gs.publish(topic_hash.clone(), publish_data).unwrap(); gs.heartbeat(); - let slow_peer_score = gs.as_peer_score_mut().score(&slow_peer_id); + let slow_peer_score = gs.as_peer_score_mut().score_report(&slow_peer_id).score; assert_eq!(slow_peer_score, slow_peer_params.slow_peer_weight); } diff --git a/protocols/gossipsub/src/metrics.rs b/protocols/gossipsub/src/metrics.rs index 2ac070c9771..e702eb00f63 100644 --- a/protocols/gossipsub/src/metrics.rs +++ b/protocols/gossipsub/src/metrics.rs @@ -686,7 +686,7 @@ pub(crate) enum Churn { } /// Kinds of reasons a peer's score has been penalized -#[derive(PartialEq, Eq, Hash, EncodeLabelValue, Clone, Debug)] +#[derive(PartialEq, Eq, Hash, EncodeLabelValue, Clone, Copy, Debug)] pub(crate) enum Penalty { /// A peer grafted before waiting the back-off time. GraftBackoff, diff --git a/protocols/gossipsub/src/peer_score.rs b/protocols/gossipsub/src/peer_score.rs index e2afe38a3ab..bcebd7e6c18 100644 --- a/protocols/gossipsub/src/peer_score.rs +++ b/protocols/gossipsub/src/peer_score.rs @@ -30,11 +30,7 @@ use futures_timer::Delay; use libp2p_identity::PeerId; use web_time::Instant; -use crate::{ - metrics::{Metrics, Penalty}, - time_cache::TimeCache, - MessageId, TopicHash, -}; +use crate::{metrics::Penalty, time_cache::TimeCache, MessageId, TopicHash}; mod params; pub use params::{ @@ -67,7 +63,7 @@ impl PeerScoreState { ) -> (bool, f64) { match self { PeerScoreState::Active(active) => { - let score = active.metric_score(peer_id, None); + let score = active.score_report(peer_id).score; (score < threshold(&active.thresholds), score) } PeerScoreState::Disabled => (false, 0.0), @@ -75,6 +71,14 @@ impl PeerScoreState { } } +/// Result of a peer score calculation, detailing the peer's +/// computed score and a list of any incurred penalties. +#[derive(Default)] +pub(crate) struct PeerScoreReport { + pub(crate) score: f64, + pub(crate) penalties: Vec, +} + pub(crate) struct PeerScore { /// The score parameters. pub(crate) params: PeerScoreParams, @@ -260,18 +264,13 @@ impl PeerScore { } } - /// Returns the score for a peer - pub(crate) fn score(&self, peer_id: &PeerId) -> f64 { - self.metric_score(peer_id, None) - } - - /// Returns the score for a peer, logging metrics. This is called from the heartbeat and - /// increments the metric counts for penalties. - pub(crate) fn metric_score(&self, peer_id: &PeerId, mut metrics: Option<&mut Metrics>) -> f64 { + /// Returns the score report for a peer, with applied penalties. + /// This is called from the heartbeat + pub(crate) fn score_report(&self, peer_id: &PeerId) -> PeerScoreReport { + let mut report = PeerScoreReport::default(); let Some(peer_stats) = self.peer_stats.get(peer_id) else { - return 0.0; + return report; }; - let mut score = 0.0; // topic scores for (topic, topic_stats) in peer_stats.topics.iter() { @@ -316,9 +315,7 @@ impl PeerScore { - topic_stats.mesh_message_deliveries; let p3 = deficit * deficit; topic_score += p3 * topic_params.mesh_message_deliveries_weight; - if let Some(metrics) = metrics.as_mut() { - metrics.register_score_penalty(Penalty::MessageDeficit); - } + report.penalties.push(Penalty::MessageDeficit); tracing::debug!( peer=%peer_id, %topic, @@ -342,18 +339,18 @@ impl PeerScore { topic_score += p4 * topic_params.invalid_message_deliveries_weight; // update score, mixing with topic weight - score += topic_score * topic_params.topic_weight; + report.score += topic_score * topic_params.topic_weight; } } // apply the topic score cap, if any - if self.params.topic_score_cap > 0f64 && score > self.params.topic_score_cap { - score = self.params.topic_score_cap; + if self.params.topic_score_cap > 0f64 && report.score > self.params.topic_score_cap { + report.score = self.params.topic_score_cap; } // P5: application-specific score let p5 = peer_stats.application_score; - score += p5 * self.params.app_specific_weight; + report.score += p5 * self.params.app_specific_weight; // P6: IP collocation factor for ip in peer_stats.known_ips.iter() { @@ -369,16 +366,14 @@ impl PeerScore { if (peers_in_ip as f64) > self.params.ip_colocation_factor_threshold { let surplus = (peers_in_ip as f64) - self.params.ip_colocation_factor_threshold; let p6 = surplus * surplus; - if let Some(metrics) = metrics.as_mut() { - metrics.register_score_penalty(Penalty::IPColocation); - } + report.penalties.push(Penalty::IPColocation); tracing::debug!( peer=%peer_id, surplus_ip=%ip, surplus=%surplus, "[Penalty] The peer gets penalized because of too many peers with the same ip" ); - score += p6 * self.params.ip_colocation_factor_weight; + report.score += p6 * self.params.ip_colocation_factor_weight; } } } @@ -387,16 +382,16 @@ impl PeerScore { if peer_stats.behaviour_penalty > self.params.behaviour_penalty_threshold { let excess = peer_stats.behaviour_penalty - self.params.behaviour_penalty_threshold; let p7 = excess * excess; - score += p7 * self.params.behaviour_penalty_weight; + report.score += p7 * self.params.behaviour_penalty_weight; } // Slow peer weighting. if peer_stats.slow_peer_penalty > self.params.slow_peer_threshold { let excess = peer_stats.slow_peer_penalty - self.params.slow_peer_threshold; - score += excess * self.params.slow_peer_weight; + report.score += excess * self.params.slow_peer_weight; } - score + report } pub(crate) fn add_penalty(&mut self, peer_id: &PeerId, count: usize) { @@ -554,7 +549,7 @@ impl PeerScore { /// non-positive. pub(crate) fn remove_peer(&mut self, peer_id: &PeerId) { // we only retain non-positive scores of peers - if self.score(peer_id) > 0f64 { + if self.score_report(peer_id).score > 0f64 { if let hash_map::Entry::Occupied(entry) = self.peer_stats.entry(*peer_id) { Self::remove_ips_for_peer(entry.get(), &mut self.peer_ips, peer_id); entry.remove(); diff --git a/protocols/gossipsub/src/peer_score/tests.rs b/protocols/gossipsub/src/peer_score/tests.rs index 5ff079b3529..e9a9e1e7800 100644 --- a/protocols/gossipsub/src/peer_score/tests.rs +++ b/protocols/gossipsub/src/peer_score/tests.rs @@ -95,7 +95,7 @@ fn test_score_time_in_mesh() { // Peer score should start at 0 peer_score.add_peer(peer_id); - let score = peer_score.score(&peer_id); + let score = peer_score.score_report(&peer_id).score; assert!( score == 0.0, "expected score to start at zero. Score found: {score}" @@ -107,7 +107,7 @@ fn test_score_time_in_mesh() { std::thread::sleep(elapsed); peer_score.refresh_scores(); - let score = peer_score.score(&peer_id); + let score = peer_score.score_report(&peer_id).score; let expected = topic_params.topic_weight * topic_params.time_in_mesh_weight * (elapsed.as_millis() / topic_params.time_in_mesh_quantum.as_millis()) as f64; @@ -140,7 +140,7 @@ fn test_score_time_in_mesh_cap() { // Peer score should start at 0 peer_score.add_peer(peer_id); - let score = peer_score.score(&peer_id); + let score = peer_score.score_report(&peer_id).score; assert!( score == 0.0, "expected score to start at zero. Score found: {score}" @@ -152,7 +152,7 @@ fn test_score_time_in_mesh_cap() { std::thread::sleep(elapsed); peer_score.refresh_scores(); - let score = peer_score.score(&peer_id); + let score = peer_score.score_report(&peer_id).score; let expected = topic_params.topic_weight * topic_params.time_in_mesh_weight * topic_params.time_in_mesh_cap; @@ -201,7 +201,7 @@ fn test_score_first_message_deliveries() { peer_score.refresh_scores(); - let score = peer_score.score(&peer_id); + let score = peer_score.score_report(&peer_id).score; let expected = topic_params.topic_weight * topic_params.first_message_deliveries_weight * messages as f64; assert!(score == expected, "The score: {score} should be {expected}"); @@ -241,7 +241,7 @@ fn test_score_first_message_deliveries_cap() { } peer_score.refresh_scores(); - let score = peer_score.score(&peer_id); + let score = peer_score.score_report(&peer_id).score; let expected = topic_params.topic_weight * topic_params.first_message_deliveries_weight * topic_params.first_message_deliveries_cap; @@ -279,7 +279,7 @@ fn test_score_first_message_deliveries_decay() { } peer_score.refresh_scores(); - let score = peer_score.score(&peer_id); + let score = peer_score.score_report(&peer_id).score; let mut expected = topic_params.topic_weight * topic_params.first_message_deliveries_weight * topic_params.first_message_deliveries_decay @@ -292,7 +292,7 @@ fn test_score_first_message_deliveries_decay() { peer_score.refresh_scores(); expected *= topic_params.first_message_deliveries_decay; } - let score = peer_score.score(&peer_id); + let score = peer_score.score_report(&peer_id).score; assert!(score == expected, "The score: {score} should be {expected}"); } @@ -339,7 +339,7 @@ fn test_score_mesh_message_deliveries() { // assert that nobody has been penalized yet for not delivering messages before activation time peer_score.refresh_scores(); for peer_id in &peers { - let score = peer_score.score(peer_id); + let score = peer_score.score_report(peer_id).score; assert!( score >= 0.0, "expected no mesh delivery penalty before activation time, got score {score}" @@ -369,9 +369,9 @@ fn test_score_mesh_message_deliveries() { } peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); - let score_b = peer_score.score(&peer_id_b); - let score_c = peer_score.score(&peer_id_c); + let score_a = peer_score.score_report(&peer_id_a).score; + let score_b = peer_score.score_report(&peer_id_b).score; + let score_c = peer_score.score_report(&peer_id_c).score; assert!( score_a >= 0.0, @@ -432,7 +432,7 @@ fn test_score_mesh_message_deliveries_decay() { // we should have a positive score, since we delivered more messages than the threshold peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert!( score_a >= 0.0, "expected non-negative score for Peer A, got score {score_a}" @@ -444,7 +444,7 @@ fn test_score_mesh_message_deliveries_decay() { decayed_delivery_count *= topic_params.mesh_message_deliveries_decay; } - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; // the penalty is the difference between the threshold and the (decayed) // mesh deliveries, squared. let deficit = topic_params.mesh_message_deliveries_threshold - decayed_delivery_count; @@ -507,8 +507,8 @@ fn test_score_mesh_failure_penalty() { // peers A and B should both have zero scores, since the failure penalty hasn't been applied yet peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); - let score_b = peer_score.score(&peer_id_b); + let score_a = peer_score.score_report(&peer_id_a).score; + let score_b = peer_score.score_report(&peer_id_b).score; assert!( score_a >= 0.0, "expected non-negative score for Peer A, got score {score_a}" @@ -521,7 +521,7 @@ fn test_score_mesh_failure_penalty() { // prune peer B to apply the penalty peer_score.prune(&peer_id_b, topic.hash()); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!(score_a, 0.0, "expected Peer A to have a 0"); @@ -532,7 +532,7 @@ fn test_score_mesh_failure_penalty() { * topic_params.mesh_message_deliveries_threshold; let expected = topic_params.topic_weight * topic_params.mesh_failure_penalty_weight * penalty; - let score_b = peer_score.score(&peer_id_b); + let score_b = peer_score.score_report(&peer_id_b).score; assert_eq!(score_b, expected, "Peer B should have expected score",); } @@ -575,7 +575,7 @@ fn test_score_invalid_message_deliveries() { } peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; let expected = topic_params.topic_weight * topic_params.invalid_message_deliveries_weight @@ -628,7 +628,7 @@ fn test_score_invalid_message_deliveris_decay() { let mut expected = topic_params.topic_weight * topic_params.invalid_message_deliveries_weight * decay * decay; - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!(score_a, expected, "Peer has unexpected score"); // refresh scores a few times to apply decay @@ -638,7 +638,7 @@ fn test_score_invalid_message_deliveris_decay() { * topic_params.invalid_message_deliveries_decay; } - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!(score_a, expected, "Peer has unexpected score"); } @@ -683,8 +683,8 @@ fn test_score_reject_message_deliveries() { peer_score.reject_message(&peer_id_a, &id, &msg.topic, RejectReason::ValidationIgnored); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); - let score_b = peer_score.score(&peer_id_b); + let score_a = peer_score.score_report(&peer_id_a).score; + let score_b = peer_score.score_report(&peer_id_b).score; assert_eq!(score_a, 0.0, "Should have no effect on the score"); assert_eq!(score_b, 0.0, "Should have no effect on the score"); @@ -698,8 +698,8 @@ fn test_score_reject_message_deliveries() { peer_score.duplicated_message(&peer_id_b, &id, &msg.topic); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); - let score_b = peer_score.score(&peer_id_b); + let score_a = peer_score.score_report(&peer_id_a).score; + let score_b = peer_score.score_report(&peer_id_b).score; assert_eq!(score_a, 0.0, "Should have no effect on the score"); assert_eq!(score_b, 0.0, "Should have no effect on the score"); @@ -716,8 +716,8 @@ fn test_score_reject_message_deliveries() { peer_score.duplicated_message(&peer_id_b, &id, &msg.topic); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); - let score_b = peer_score.score(&peer_id_b); + let score_a = peer_score.score_report(&peer_id_a).score; + let score_b = peer_score.score_report(&peer_id_b).score; assert_eq!(score_a, 0.0, "Should have no effect on the score"); assert_eq!(score_b, 0.0, "Should have no effect on the score"); @@ -733,8 +733,8 @@ fn test_score_reject_message_deliveries() { peer_score.duplicated_message(&peer_id_b, &id, &msg.topic); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); - let score_b = peer_score.score(&peer_id_b); + let score_a = peer_score.score_report(&peer_id_a).score; + let score_b = peer_score.score_report(&peer_id_b).score; assert_eq!(score_a, -1.0, "Score should be effected"); assert_eq!(score_b, -1.0, "Score should be effected"); @@ -750,8 +750,8 @@ fn test_score_reject_message_deliveries() { peer_score.reject_message(&peer_id_a, &id, &msg.topic, RejectReason::ValidationFailed); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); - let score_b = peer_score.score(&peer_id_b); + let score_a = peer_score.score_report(&peer_id_a).score; + let score_b = peer_score.score_report(&peer_id_b).score; assert_eq!(score_a, -4.0, "Score should be effected"); assert_eq!(score_b, -4.0, "Score should be effected"); @@ -792,7 +792,7 @@ fn test_application_score() { let app_score_value = i as f64; peer_score.set_application_score(&peer_id_a, app_score_value); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; let expected = (i as f64) * app_specific_weight; assert_eq!(score_a, expected, "Peer has unexpected score"); } @@ -844,10 +844,10 @@ fn test_score_ip_colocation() { peer_score.add_ip(&peer_id_d, "2.3.4.5".parse().unwrap()); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); - let score_b = peer_score.score(&peer_id_b); - let score_c = peer_score.score(&peer_id_c); - let score_d = peer_score.score(&peer_id_d); + let score_a = peer_score.score_report(&peer_id_a).score; + let score_b = peer_score.score_report(&peer_id_b).score; + let score_c = peer_score.score_report(&peer_id_c).score; + let score_d = peer_score.score_report(&peer_id_d).score; assert_eq!(score_a, 0.0, "Peer A should be unaffected"); @@ -894,7 +894,7 @@ fn test_score_behaviour_penalty() { // add a penalty to a non-existent peer. peer_score.add_penalty(&peer_id_a, 1); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!(score_a, 0.0, "Peer A should be unaffected"); // add the peer and test penalties @@ -903,16 +903,16 @@ fn test_score_behaviour_penalty() { peer_score.add_penalty(&peer_id_a, 1); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!(score_a, -1.0, "Peer A should have been penalized"); peer_score.add_penalty(&peer_id_a, 1); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!(score_a, -4.0, "Peer A should have been penalized"); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!(score_a, -3.9204, "Peer A should have been penalized"); } @@ -950,7 +950,7 @@ fn test_score_retention() { // score should equal -1000 (app specific score) peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!( score_a, app_score_value, "Score should be the application specific score" @@ -960,7 +960,7 @@ fn test_score_retention() { peer_score.remove_peer(&peer_id_a); std::thread::sleep(retain_score / 2); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!( score_a, app_score_value, "Score should be the application specific score" @@ -969,7 +969,7 @@ fn test_score_retention() { // wait remaining time (plus a little slop) and the score should reset to zero std::thread::sleep(retain_score / 2 + Duration::from_millis(50)); peer_score.refresh_scores(); - let score_a = peer_score.score(&peer_id_a); + let score_a = peer_score.score_report(&peer_id_a).score; assert_eq!( score_a, 0.0, "Score should be the application specific score" From 0b15de2b28308afa6ea26b38b2948b031b5c6d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Mon, 12 May 2025 15:30:08 +0100 Subject: [PATCH 4/6] feature gate metrics code --- protocols/gossipsub/CHANGELOG.md | 2 + protocols/gossipsub/Cargo.toml | 5 +- protocols/gossipsub/src/behaviour.rs | 137 ++++++++++++++------- protocols/gossipsub/src/behaviour/tests.rs | 1 - protocols/gossipsub/src/lib.rs | 5 +- protocols/gossipsub/src/metrics.rs | 8 +- protocols/gossipsub/src/peer_score.rs | 13 +- protocols/gossipsub/src/rpc.rs | 2 + protocols/gossipsub/src/topic.rs | 7 +- protocols/gossipsub/src/types.rs | 7 +- 10 files changed, 126 insertions(+), 61 deletions(-) diff --git a/protocols/gossipsub/CHANGELOG.md b/protocols/gossipsub/CHANGELOG.md index 875fe257a60..09641beda8b 100644 --- a/protocols/gossipsub/CHANGELOG.md +++ b/protocols/gossipsub/CHANGELOG.md @@ -1,4 +1,6 @@ ## 0.49.0 +- Feature gate metrics related code. This changes some `Behaviour` constructor methods. + See [PR XXXX](https://github.com/libp2p/rust-libp2p/pull/XXXX) - Send IDONTWANT before Publishing a new message. See [PR 6017](https://github.com/libp2p/rust-libp2p/pull/6017) diff --git a/protocols/gossipsub/Cargo.toml b/protocols/gossipsub/Cargo.toml index 99213860a27..bd706175890 100644 --- a/protocols/gossipsub/Cargo.toml +++ b/protocols/gossipsub/Cargo.toml @@ -12,6 +12,7 @@ categories = ["network-programming", "asynchronous"] [features] wasm-bindgen = ["getrandom/js", "futures-timer/wasm-bindgen"] +metrics = ["prometheus-client"] [dependencies] async-channel = "2.3.1" @@ -24,7 +25,7 @@ fnv = "1.0.7" futures = { workspace = true } futures-timer = "3.0.2" getrandom = { workspace = true } -hashlink = { workspace = true} +hashlink = { workspace = true } hex_fmt = "0.3.0" web-time = { workspace = true } libp2p-core = { workspace = true } @@ -39,7 +40,7 @@ sha2 = "0.10.8" tracing = { workspace = true } # Metrics dependencies -prometheus-client = { workspace = true } +prometheus-client = { workspace = true, optional = true } [dev-dependencies] libp2p-core = { workspace = true } diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 6285178514e..330d774507c 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -45,18 +45,20 @@ use libp2p_swarm::{ ConnectionDenied, ConnectionId, NetworkBehaviour, NotifyHandler, THandler, THandlerInEvent, THandlerOutEvent, ToSwarm, }; +#[cfg(feature = "metrics")] use prometheus_client::registry::Registry; use quick_protobuf::{MessageWrite, Writer}; use rand::{seq::SliceRandom, thread_rng}; use web_time::{Instant, SystemTime}; +#[cfg(feature = "metrics")] +use crate::metrics::{Churn, Config as MetricsConfig, Inclusion, Metrics, Penalty}; use crate::{ backoff::BackoffStorage, config::{Config, ValidationMode}, gossip_promises::GossipPromises, handler::{Handler, HandlerEvent, HandlerIn}, mcache::MessageCache, - metrics::{Churn, Config as MetricsConfig, Inclusion, Metrics, Penalty}, peer_score::{PeerScore, PeerScoreParams, PeerScoreState, PeerScoreThresholds, RejectReason}, protocol::SIGNING_PREFIX, rpc::Sender, @@ -333,6 +335,7 @@ pub struct Behaviour { data_transform: D, /// Keep track of a set of internal metrics relating to gossipsub. + #[cfg(feature = "metrics")] metrics: Option, /// Tracks the numbers of failed messages per peer-id. @@ -353,28 +356,21 @@ where Self::new_with_subscription_filter_and_transform( privacy, config, - None, F::default(), D::default(), ) } - /// Creates a Gossipsub [`Behaviour`] struct given a set of parameters specified via a - /// [`Config`]. This has no subscription filter and uses no compression. + /// Allow the [`Behaviour`] to also record metrics. /// Metrics can be evaluated by passing a reference to a [`Registry`]. - pub fn new_with_metrics( - privacy: MessageAuthenticity, - config: Config, + #[cfg(feature = "metrics")] + pub fn with_metrics( + mut self, metrics_registry: &mut Registry, metrics_config: MetricsConfig, - ) -> Result { - Self::new_with_subscription_filter_and_transform( - privacy, - config, - Some((metrics_registry, metrics_config)), - F::default(), - D::default(), - ) + ) -> Self { + self.metrics = Some(Metrics::new(metrics_registry, metrics_config)); + self } } @@ -388,13 +384,11 @@ where pub fn new_with_subscription_filter( privacy: MessageAuthenticity, config: Config, - metrics: Option<(&mut Registry, MetricsConfig)>, subscription_filter: F, ) -> Result { Self::new_with_subscription_filter_and_transform( privacy, config, - metrics, subscription_filter, D::default(), ) @@ -408,16 +402,15 @@ where { /// Creates a Gossipsub [`Behaviour`] struct given a set of parameters specified via a /// [`Config`] and a custom data transform. + /// Metrics are disabled by default. pub fn new_with_transform( privacy: MessageAuthenticity, config: Config, - metrics: Option<(&mut Registry, MetricsConfig)>, data_transform: D, ) -> Result { Self::new_with_subscription_filter_and_transform( privacy, config, - metrics, F::default(), data_transform, ) @@ -431,10 +424,10 @@ where { /// Creates a Gossipsub [`Behaviour`] struct given a set of parameters specified via a /// [`Config`] and a custom subscription filter and data transform. + /// Metrics are disabled by default. pub fn new_with_subscription_filter_and_transform( privacy: MessageAuthenticity, config: Config, - metrics: Option<(&mut Registry, MetricsConfig)>, subscription_filter: F, data_transform: D, ) -> Result { @@ -445,7 +438,8 @@ where validate_config(&privacy, config.validation_mode())?; Ok(Behaviour { - metrics: metrics.map(|(registry, cfg)| Metrics::new(registry, cfg)), + #[cfg(feature = "metrics")] + metrics: None, events: VecDeque::new(), publish_config: privacy.into(), duplicate_cache: DuplicateCache::new(config.duplicate_cache_time()), @@ -782,6 +776,7 @@ where tracing::debug!(message_id=%msg_id, "Published message"); + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.register_published_message(&topic_hash); } @@ -825,6 +820,7 @@ where message_id=%msg_id, "Message not in cache. Ignoring forwarding" ); + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.memcache_miss(); } @@ -832,6 +828,7 @@ where } }; + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.register_msg_validation(&raw_message.topic, &acceptance); } @@ -849,6 +846,7 @@ where }; if let Some((raw_message, originating_peers)) = self.mcache.remove(msg_id) { + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.register_msg_validation(&raw_message.topic, &acceptance); } @@ -976,6 +974,7 @@ where let mut added_peers = HashSet::new(); let mesh_n = self.config.mesh_n_for_topic(topic_hash); + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.joined(topic_hash) } @@ -1014,15 +1013,15 @@ where self.fanout_last_pub.remove(topic_hash); } - let fanout_added = added_peers.len(); + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { - m.peers_included(topic_hash, Inclusion::Fanout, fanout_added) + m.peers_included(topic_hash, Inclusion::Fanout, added_peers.len()) } // check if we need to get more peers, which we randomly select if added_peers.len() < mesh_n { // get the peers - let new_peers = get_random_peers( + let random_added = get_random_peers( &self.connected_peers, topic_hash, mesh_n - added_peers.len(), @@ -1034,19 +1033,20 @@ where }, ); - added_peers.extend(new_peers.clone()); + added_peers.extend(random_added.clone()); // add them to the mesh tracing::debug!( "JOIN: Inserting {:?} random peers into the mesh", - new_peers.len() + random_added.len() ); - let mesh_peers = self.mesh.entry(topic_hash.clone()).or_default(); - mesh_peers.extend(new_peers); - } - let random_added = added_peers.len() - fanout_added; - if let Some(m) = self.metrics.as_mut() { - m.peers_included(topic_hash, Inclusion::Random, random_added) + #[cfg(feature = "metrics")] + if let Some(m) = self.metrics.as_mut() { + m.peers_included(topic_hash, Inclusion::Random, random_added.len()) + } + + let mesh_peers = self.mesh.entry(topic_hash.clone()).or_default(); + mesh_peers.extend(random_added); } for peer_id in added_peers { @@ -1072,9 +1072,12 @@ where ); } - let mesh_peers = self.mesh_peers(topic_hash).count(); - if let Some(m) = self.metrics.as_mut() { - m.set_mesh_peers(topic_hash, mesh_peers) + #[cfg(feature = "metrics")] + { + let mesh_peers = self.mesh_peers(topic_hash).count(); + if let Some(m) = self.metrics.as_mut() { + m.set_mesh_peers(topic_hash, mesh_peers) + } } tracing::debug!(topic=%topic_hash, "Completed JOIN for topic"); @@ -1147,6 +1150,7 @@ where // If our mesh contains the topic, send prune to peers and delete it from the mesh if let Some((_, peers)) = self.mesh.remove_entry(topic_hash) { + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.left(topic_hash) } @@ -1247,6 +1251,7 @@ where // have not seen this message and are not currently requesting it if iwant_ids.insert(id) { // Register the IWANT metric + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.register_iwant(&topic); } @@ -1372,6 +1377,7 @@ where // and they must be subscribed to the topic. Ensure we have recorded the mapping. for topic in &topics { if connected_peer.topics.insert(topic.clone()) { + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.inc_topic_peers(topic); } @@ -1410,6 +1416,7 @@ where ); // add behavioural penalty if let PeerScoreState::Active(peer_score) = &mut self.peer_score { + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.register_score_penalty(Penalty::GraftBackoff); } @@ -1468,6 +1475,7 @@ where ); if peers.insert(*peer_id) { + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.peers_included(&topic_hash, Inclusion::Subscribed, 1) } @@ -1519,33 +1527,29 @@ where tracing::debug!(peer=%peer_id, "Completed GRAFT handling for peer"); } + /// Removes the specified peer from the mesh, returning true if it was present. fn remove_peer_from_mesh( &mut self, peer_id: &PeerId, topic_hash: &TopicHash, backoff: Option, always_update_backoff: bool, - reason: Churn, - ) { - let mut update_backoff = always_update_backoff; + ) -> bool { + let mut peer_removed = false; if let Some(peers) = self.mesh.get_mut(topic_hash) { // remove the peer if it exists in the mesh - if peers.remove(peer_id) { + peer_removed = peers.remove(peer_id); + if peer_removed { tracing::debug!( peer=%peer_id, topic=%topic_hash, "PRUNE: Removing peer from the mesh for topic" ); - if let Some(m) = self.metrics.as_mut() { - m.peers_removed(topic_hash, reason, 1) - } if let PeerScoreState::Active(peer_score) = &mut self.peer_score { peer_score.prune(peer_id, topic_hash.clone()); } - update_backoff = true; - // inform the handler peer_removed_from_mesh( *peer_id, @@ -1556,7 +1560,7 @@ where ); } } - if update_backoff { + if always_update_backoff || peer_removed { let time = if let Some(backoff) = backoff { Duration::from_secs(backoff) } else { @@ -1565,6 +1569,7 @@ where // is there a backoff specified by the peer? if so obey it. self.backoffs.update_backoff(topic_hash, peer_id, time); } + peer_removed } /// Handles PRUNE control messages. Removes peer from the mesh. @@ -1578,7 +1583,12 @@ where .peer_score .below_threshold(peer_id, |ts| ts.accept_px_threshold); for (topic_hash, px, backoff) in prune_data { - self.remove_peer_from_mesh(peer_id, &topic_hash, backoff, true, Churn::Prune); + if self.remove_peer_from_mesh(peer_id, &topic_hash, backoff, true) { + #[cfg(feature = "metrics")] + if let Some(m) = self.metrics.as_mut() { + m.peers_removed(&topic_hash, Churn::Prune, 1); + } + } if self.mesh.contains_key(&topic_hash) { // connect to px peers @@ -1732,6 +1742,7 @@ where propagation_source: &PeerId, ) { // Record the received metric + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.msg_recvd_unfiltered(&raw_message.topic, raw_message.raw_protobuf_len()); } @@ -1801,6 +1812,7 @@ where ); // Record the received message with the metrics + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.msg_recvd(&message.topic); } @@ -1858,6 +1870,7 @@ where message_id: Option<&MessageId>, reject_reason: RejectReason, ) { + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.register_invalid_message(topic_hash); } @@ -1936,6 +1949,7 @@ where "SUBSCRIPTION: Adding gossip peer to topic" ); + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.inc_topic_peers(topic_hash); } @@ -1961,6 +1975,7 @@ where topic=%topic_hash, "SUBSCRIPTION: Adding peer to the mesh for topic" ); + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.peers_included(topic_hash, Inclusion::Subscribed, 1) } @@ -1991,6 +2006,7 @@ where "SUBSCRIPTION: Removing gossip peer from topic" ); + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.dec_topic_peers(topic_hash); } @@ -2011,7 +2027,12 @@ where self.fanout .get_mut(&topic_hash) .map(|peers| peers.remove(&peer_id)); - self.remove_peer_from_mesh(&peer_id, &topic_hash, None, false, Churn::Unsub); + if self.remove_peer_from_mesh(&peer_id, &topic_hash, None, false) { + #[cfg(feature = "metrics")] + if let Some(m) = self.metrics.as_mut() { + m.peers_removed(&topic_hash, Churn::Unsub, 1); + } + }; } // Potentially inform the handler if we have added this peer to a mesh for the first time. @@ -2048,6 +2069,7 @@ where if let PeerScoreState::Active(peer_score) = &mut self.peer_score { for (peer, count) in self.gossip_promises.get_broken_promises() { peer_score.add_penalty(&peer, count); + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.register_score_penalty(Penalty::BrokenPromise); } @@ -2058,11 +2080,13 @@ where /// Heartbeat function which shifts the memcache and updates the mesh. fn heartbeat(&mut self) { tracing::debug!("Starting heartbeat"); + #[cfg(feature = "metrics")] let start = Instant::now(); // Every heartbeat we sample the send queues to add to our metrics. We do this intentionally // before we add all the gossip from this heartbeat in order to gain a true measure of // steady-state size of the queues. + #[cfg(feature = "metrics")] if let Some(m) = &mut self.metrics { for sender_queue in self.connected_peers.values().map(|v| &v.sender) { m.observe_priority_queue_size(sender_queue.priority_queue_len()); @@ -2097,9 +2121,12 @@ where let mut scores = HashMap::with_capacity(self.connected_peers.len()); if let PeerScoreState::Active(peer_score) = &self.peer_score { for peer_id in self.connected_peers.keys() { + #[allow(unused_variables)] let report = scores .entry(peer_id) .or_insert_with(|| peer_score.score_report(peer_id)); + + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { for penalty in &report.penalties { metrics.register_score_penalty(*penalty); @@ -2126,6 +2153,7 @@ where let peer_score = scores.get(peer_id).map(|r| r.score).unwrap_or_default(); // Record the score per mesh + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.observe_mesh_peers_score(topic_hash, peer_score); } @@ -2145,6 +2173,7 @@ where } } + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.peers_removed(topic_hash, Churn::BadScore, to_remove_peers.len()) } @@ -2176,6 +2205,7 @@ where } // update the mesh tracing::debug!("Updating mesh, new mesh: {:?}", peer_list); + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.peers_included(topic_hash, Inclusion::Random, peer_list.len()) } @@ -2242,6 +2272,7 @@ where removed += 1; } + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.peers_removed(topic_hash, Churn::Excess, removed) } @@ -2280,6 +2311,7 @@ where } // update the mesh tracing::debug!("Updating mesh, new mesh: {:?}", peer_list); + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.peers_included(topic_hash, Inclusion::Outbound, peer_list.len()) } @@ -2355,6 +2387,7 @@ where "Opportunistically graft in topic with peers {:?}", peer_list ); + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.peers_included(topic_hash, Inclusion::Random, peer_list.len()) } @@ -2363,6 +2396,7 @@ where } } // Register the final count of peers in the mesh + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.set_mesh_peers(topic_hash, peers.len()) } @@ -2490,6 +2524,7 @@ where } tracing::debug!("Completed Heartbeat"); + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { let duration = u64::try_from(start.elapsed().as_millis()).unwrap_or(u64::MAX); metrics.observe_heartbeat_duration(duration); @@ -2826,6 +2861,7 @@ where /// sending the message failed due to the channel to the connection handler being /// full (which indicates a slow peer). fn send_message(&mut self, peer_id: PeerId, rpc: RpcOut) -> bool { + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { if let RpcOut::Publish { ref message, .. } | RpcOut::Forward { ref message, .. } = rpc { // register bytes sent on the internal metrics. @@ -2990,6 +3026,7 @@ where if let Some(mesh_peers) = self.mesh.get_mut(topic) { // check if the peer is in the mesh and remove it if mesh_peers.remove(&peer_id) { + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.peers_removed(topic, Churn::Dc, 1); m.set_mesh_peers(topic, mesh_peers.len()); @@ -2997,6 +3034,7 @@ where }; } + #[cfg(feature = "metrics")] if let Some(m) = self.metrics.as_mut() { m.dec_topic_peers(topic); } @@ -3011,6 +3049,7 @@ where self.px_peers.remove(&peer_id); // If metrics are enabled, register the disconnection of a peer based on its protocol. + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.peer_protocol_disconnected(connected_peer.kind); } @@ -3055,6 +3094,7 @@ where } } + #[cfg(feature = "metrics")] /// Register topics to ensure metrics are recorded correctly for these topics. pub fn register_topics_for_metrics(&mut self, topics: Vec) { if let Some(metrics) = &mut self.metrics { @@ -3145,6 +3185,7 @@ where HandlerEvent::PeerKind(kind) => { // We have identified the protocol this peer is using + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.peer_protocol_connected(kind); } @@ -3192,6 +3233,7 @@ where } // Record metrics on the failure. + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { match rpc { RpcOut::Publish { message, .. } => { @@ -3303,6 +3345,7 @@ where "Could not handle IDONTWANT, peer doesn't exist in connected peer list"); continue; }; + #[cfg(feature = "metrics")] if let Some(metrics) = self.metrics.as_mut() { metrics.register_idontwant(message_ids.len()); } diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index d4ed72fac05..6600f532a11 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -71,7 +71,6 @@ where let mut gs: Behaviour = Behaviour::new_with_subscription_filter_and_transform( MessageAuthenticity::Signed(keypair), self.gs_config, - None, self.subscription_filter, self.data_transform, ) diff --git a/protocols/gossipsub/src/lib.rs b/protocols/gossipsub/src/lib.rs index 87db1b771d1..ae614367e14 100644 --- a/protocols/gossipsub/src/lib.rs +++ b/protocols/gossipsub/src/lib.rs @@ -101,6 +101,7 @@ mod error; mod gossip_promises; mod handler; mod mcache; +#[cfg(feature = "metrics")] mod metrics; mod peer_score; mod protocol; @@ -112,11 +113,13 @@ mod topic; mod transform; mod types; +#[cfg(feature = "metrics")] +pub use metrics::Config as MetricsConfig; + pub use self::{ behaviour::{Behaviour, Event, MessageAuthenticity}, config::{Config, ConfigBuilder, ValidationMode, Version}, error::{ConfigBuilderError, PublishError, SubscriptionError, ValidationError}, - metrics::Config as MetricsConfig, peer_score::{ score_parameter_decay, score_parameter_decay_with_base, PeerScoreParams, PeerScoreThresholds, TopicScoreParams, diff --git a/protocols/gossipsub/src/metrics.rs b/protocols/gossipsub/src/metrics.rs index e702eb00f63..7805cf7f310 100644 --- a/protocols/gossipsub/src/metrics.rs +++ b/protocols/gossipsub/src/metrics.rs @@ -686,7 +686,7 @@ pub(crate) enum Churn { } /// Kinds of reasons a peer's score has been penalized -#[derive(PartialEq, Eq, Hash, EncodeLabelValue, Clone, Copy, Debug)] +#[derive(PartialEq, Eq, Hash, Clone, Copy, Debug, EncodeLabelValue)] pub(crate) enum Penalty { /// A peer grafted before waiting the back-off time. GraftBackoff, @@ -713,7 +713,11 @@ struct ChurnLabel { } /// Label for the kinds of protocols peers can connect as. -#[derive(PartialEq, Eq, Hash, EncodeLabelSet, Clone, Debug)] +#[derive(PartialEq, Eq, Hash, Clone, Debug)] +#[cfg_attr( + feature = "metrics", + derive(prometheus_client::encoding::EncodeLabelSet) +)] struct ProtocolLabel { protocol: PeerKind, } diff --git a/protocols/gossipsub/src/peer_score.rs b/protocols/gossipsub/src/peer_score.rs index bcebd7e6c18..1e1c099fd27 100644 --- a/protocols/gossipsub/src/peer_score.rs +++ b/protocols/gossipsub/src/peer_score.rs @@ -30,7 +30,7 @@ use futures_timer::Delay; use libp2p_identity::PeerId; use web_time::Instant; -use crate::{metrics::Penalty, time_cache::TimeCache, MessageId, TopicHash}; +use crate::{time_cache::TimeCache, MessageId, TopicHash}; mod params; pub use params::{ @@ -76,7 +76,8 @@ impl PeerScoreState { #[derive(Default)] pub(crate) struct PeerScoreReport { pub(crate) score: f64, - pub(crate) penalties: Vec, + #[cfg(feature = "metrics")] + pub(crate) penalties: Vec, } pub(crate) struct PeerScore { @@ -315,7 +316,10 @@ impl PeerScore { - topic_stats.mesh_message_deliveries; let p3 = deficit * deficit; topic_score += p3 * topic_params.mesh_message_deliveries_weight; - report.penalties.push(Penalty::MessageDeficit); + #[cfg(feature = "metrics")] + report + .penalties + .push(crate::metrics::Penalty::MessageDeficit); tracing::debug!( peer=%peer_id, %topic, @@ -366,7 +370,8 @@ impl PeerScore { if (peers_in_ip as f64) > self.params.ip_colocation_factor_threshold { let surplus = (peers_in_ip as f64) - self.params.ip_colocation_factor_threshold; let p6 = surplus * surplus; - report.penalties.push(Penalty::IPColocation); + #[cfg(feature = "metrics")] + report.penalties.push(crate::metrics::Penalty::IPColocation); tracing::debug!( peer=%peer_id, surplus_ip=%ip, diff --git a/protocols/gossipsub/src/rpc.rs b/protocols/gossipsub/src/rpc.rs index 41b338267e9..943df31f599 100644 --- a/protocols/gossipsub/src/rpc.rs +++ b/protocols/gossipsub/src/rpc.rs @@ -97,11 +97,13 @@ impl Sender { } /// Returns the current size of the priority queue. + #[cfg(feature = "metrics")] pub(crate) fn priority_queue_len(&self) -> usize { self.len.load(Ordering::Relaxed) } /// Returns the current size of the non-priority queue. + #[cfg(feature = "metrics")] pub(crate) fn non_priority_queue_len(&self) -> usize { self.non_priority_sender.len() } diff --git a/protocols/gossipsub/src/topic.rs b/protocols/gossipsub/src/topic.rs index fa5ea3c2348..53e9fe2c172 100644 --- a/protocols/gossipsub/src/topic.rs +++ b/protocols/gossipsub/src/topic.rs @@ -21,7 +21,6 @@ use std::fmt; use base64::prelude::*; -use prometheus_client::encoding::EncodeLabelSet; use quick_protobuf::Writer; use sha2::{Digest, Sha256}; @@ -66,7 +65,11 @@ impl Hasher for Sha256Hash { } } -#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, EncodeLabelSet)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[cfg_attr( + feature = "metrics", + derive(prometheus_client::encoding::EncodeLabelSet) +)] pub struct TopicHash { /// The topic hash. Stored as a string to align with the protobuf API. hash: String, diff --git a/protocols/gossipsub/src/types.rs b/protocols/gossipsub/src/types.rs index 4c8a3e8f54f..4af8e4666da 100644 --- a/protocols/gossipsub/src/types.rs +++ b/protocols/gossipsub/src/types.rs @@ -25,7 +25,6 @@ use futures_timer::Delay; use hashlink::LinkedHashMap; use libp2p_identity::PeerId; use libp2p_swarm::ConnectionId; -use prometheus_client::encoding::EncodeLabelValue; use quick_protobuf::MessageWrite; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; @@ -119,7 +118,11 @@ pub(crate) struct PeerDetails { } /// Describes the types of peers that can exist in the gossipsub context. -#[derive(Debug, Clone, Copy, PartialEq, Hash, EncodeLabelValue, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq)] +#[cfg_attr( + feature = "metrics", + derive(prometheus_client::encoding::EncodeLabelValue) +)] pub enum PeerKind { /// A gossipsub 1.2 peer. Gossipsubv1_2, From 4d9f3bc080782269883fcb6b6a5b009921a800a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Tue, 13 May 2025 08:41:42 +0100 Subject: [PATCH 5/6] Update protocols/gossipsub/CHANGELOG.md Co-authored-by: Darius Clark --- protocols/gossipsub/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/gossipsub/CHANGELOG.md b/protocols/gossipsub/CHANGELOG.md index 09641beda8b..9f2d0ad5f72 100644 --- a/protocols/gossipsub/CHANGELOG.md +++ b/protocols/gossipsub/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.49.0 - Feature gate metrics related code. This changes some `Behaviour` constructor methods. - See [PR XXXX](https://github.com/libp2p/rust-libp2p/pull/XXXX) + See [PR 6020](https://github.com/libp2p/rust-libp2p/pull/6020) - Send IDONTWANT before Publishing a new message. See [PR 6017](https://github.com/libp2p/rust-libp2p/pull/6017) From f3a809e3cd69014db839c86052a574998ee4dc1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Thu, 15 May 2025 15:56:14 +0100 Subject: [PATCH 6/6] address Elena's review --- protocols/gossipsub/src/behaviour.rs | 8 ++++++++ protocols/gossipsub/src/metrics.rs | 6 +----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 330d774507c..45c84a39ffd 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -506,6 +506,14 @@ where self.connected_peers.iter().map(|(k, v)| (k, &v.kind)) } + /// Returns the gossipsub score for a given peer, if one exists. + pub fn peer_score(&self, peer_id: &PeerId) -> Option { + match &self.peer_score { + PeerScoreState::Active(peer_score) => Some(peer_score.score_report(peer_id).score), + PeerScoreState::Disabled => None, + } + } + /// Subscribe to a topic. /// /// Returns [`Ok(true)`] if the subscription worked. Returns [`Ok(false)`] if we were already diff --git a/protocols/gossipsub/src/metrics.rs b/protocols/gossipsub/src/metrics.rs index 7805cf7f310..37fe5481689 100644 --- a/protocols/gossipsub/src/metrics.rs +++ b/protocols/gossipsub/src/metrics.rs @@ -713,11 +713,7 @@ struct ChurnLabel { } /// Label for the kinds of protocols peers can connect as. -#[derive(PartialEq, Eq, Hash, Clone, Debug)] -#[cfg_attr( - feature = "metrics", - derive(prometheus_client::encoding::EncodeLabelSet) -)] +#[derive(PartialEq, Eq, Hash, Clone, Debug, EncodeLabelSet)] struct ProtocolLabel { protocol: PeerKind, }