From b5517f6b7f568a96044a4a4ba8ff445f745d8646 Mon Sep 17 00:00:00 2001 From: drHuangMHT Date: Fri, 27 Dec 2024 15:16:01 +0800 Subject: [PATCH 1/3] refactor to remove duplicated call --- protocols/gossipsub/src/behaviour.rs | 36 +++++++++++++++------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 954e87ee470..7f0c972a204 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -1664,7 +1664,8 @@ where ); self.handle_invalid_message( propagation_source, - raw_message, + &raw_message.topic, + Some(msg_id), RejectReason::BlackListedSource, ); return false; @@ -1693,7 +1694,12 @@ where source=%propagation_source, "Dropping message claiming to be from self but forwarded from source" ); - self.handle_invalid_message(propagation_source, raw_message, RejectReason::SelfOrigin); + self.handle_invalid_message( + propagation_source, + &raw_message.topic, + Some(msg_id), + RejectReason::SelfOrigin, + ); return false; } @@ -1721,7 +1727,8 @@ where // Reject the message and return self.handle_invalid_message( propagation_source, - &raw_message, + &raw_message.topic, + None, RejectReason::ValidationError(ValidationError::TransformFailed), ); return; @@ -1799,30 +1806,24 @@ where fn handle_invalid_message( &mut self, propagation_source: &PeerId, - raw_message: &RawMessage, + topic_hash: &TopicHash, + message_id: Option<&MessageId>, reject_reason: RejectReason, ) { if let Some((peer_score, .., gossip_promises)) = &mut self.peer_score { if let Some(metrics) = self.metrics.as_mut() { - metrics.register_invalid_message(&raw_message.topic); + metrics.register_invalid_message(topic_hash); } - if let Ok(message) = self.data_transform.inbound_transform(raw_message.clone()) { - let message_id = self.config.message_id(&message); - - peer_score.reject_message( - propagation_source, - &message_id, - &message.topic, - reject_reason, - ); + if let Some(msg_id) = message_id { + peer_score.reject_message(propagation_source, msg_id, topic_hash, reject_reason); - gossip_promises.reject_message(&message_id, &reject_reason); + gossip_promises.reject_message(msg_id, &reject_reason); } else { // The message is invalid, we reject it ignoring any gossip promises. If a peer is // advertising this message via an IHAVE and it's invalid it will be double // penalized, one for sending us an invalid and again for breaking a promise. - peer_score.reject_invalid_message(propagation_source, &raw_message.topic); + peer_score.reject_invalid_message(propagation_source, topic_hash); } } } @@ -3126,7 +3127,8 @@ where for (raw_message, validation_error) in invalid_messages { self.handle_invalid_message( &propagation_source, - &raw_message, + &raw_message.topic, + None, RejectReason::ValidationError(validation_error), ) } From 21e7e81fa58871ba43b356fb3b3c877373f8d4a8 Mon Sep 17 00:00:00 2001 From: drHuangMHT Date: Thu, 2 Jan 2025 13:47:14 +0800 Subject: [PATCH 2/3] clippy lint --- protocols/gossipsub/src/behaviour.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 3bc904d7d22..62bb2be682a 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -1827,11 +1827,11 @@ where reject_reason: RejectReason, ) { if let Some(metrics) = self.metrics.as_mut() { - metrics.register_invalid_message(&topic_hash); + metrics.register_invalid_message(topic_hash); } if let Some((peer_score, ..)) = &mut self.peer_score { if let Some(msg_id) = message_id { - peer_score.reject_message(propagation_source, &msg_id, topic_hash, reject_reason); + peer_score.reject_message(propagation_source, msg_id, topic_hash, reject_reason); // The message itself is valid, but is from a banned peer or // claiming to be self-origin but is actually forwarded from other peers. return self.gossip_promises.reject_message(msg_id, &reject_reason); From 135cc977685864504ea2896cdd0359ee679f4441 Mon Sep 17 00:00:00 2001 From: drHuangMHT Date: Sun, 5 Jan 2025 11:15:15 +0800 Subject: [PATCH 3/3] reorganize --- protocols/gossipsub/src/behaviour.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 62bb2be682a..537cac5d020 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -1829,23 +1829,23 @@ where if let Some(metrics) = self.metrics.as_mut() { metrics.register_invalid_message(topic_hash); } + if let Some(msg_id) = message_id { + // Valid transformation without peer scoring + self.gossip_promises.reject_message(msg_id, &reject_reason); + } if let Some((peer_score, ..)) = &mut self.peer_score { + // The compiler will optimize this pattern-matching if let Some(msg_id) = message_id { - peer_score.reject_message(propagation_source, msg_id, topic_hash, reject_reason); // The message itself is valid, but is from a banned peer or // claiming to be self-origin but is actually forwarded from other peers. - return self.gossip_promises.reject_message(msg_id, &reject_reason); + peer_score.reject_message(propagation_source, msg_id, topic_hash, reject_reason); } else { // The message is invalid, we reject it ignoring any gossip promises. If a peer is // advertising this message via an IHAVE and it's invalid it will be double // penalized, one for sending us an invalid and again for breaking a promise. - return peer_score.reject_invalid_message(propagation_source, topic_hash); + peer_score.reject_invalid_message(propagation_source, topic_hash); } } - if let Some(msg_id) = message_id { - // Valid transformation without peer scoring - self.gossip_promises.reject_message(msg_id, &reject_reason); - } } /// Handles received subscriptions.