From cc8607ce3ec494e86073c4abd6d2491280bb55d0 Mon Sep 17 00:00:00 2001 From: DrHuangMHT Date: Thu, 16 Jan 2025 23:43:11 +0800 Subject: [PATCH] refactor(gossipsub): remove duplicated call to `inbound_transform` May close #4369. Pull-Request: #5767. --- protocols/gossipsub/src/behaviour.rs | 59 +++++++++++++--------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 356f1d6cd77..b803337462d 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -1684,7 +1684,8 @@ where ); self.handle_invalid_message( propagation_source, - raw_message, + &raw_message.topic, + Some(msg_id), RejectReason::BlackListedSource, ); return false; @@ -1713,7 +1714,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; } @@ -1741,7 +1747,8 @@ where // Reject the message and return self.handle_invalid_message( propagation_source, - &raw_message, + &raw_message.topic, + None, RejectReason::ValidationError(ValidationError::TransformFailed), ); return; @@ -1827,42 +1834,29 @@ 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(metrics) = self.metrics.as_mut() { - metrics.register_invalid_message(&raw_message.topic); + metrics.register_invalid_message(topic_hash); } - - let message = self.data_transform.inbound_transform(raw_message.clone()); - - match (&mut self.peer_score, message) { - (Some((peer_score, ..)), Ok(message)) => { - let message_id = self.config.message_id(&message); - - peer_score.reject_message( - propagation_source, - &message_id, - &message.topic, - reject_reason, - ); - - self.gossip_promises - .reject_message(&message_id, &reject_reason); - } - (Some((peer_score, ..)), Err(_)) => { + 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 { + // The message itself is valid, but is from a banned peer or + // claiming to be self-origin but is actually forwarded from other peers. + 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. - peer_score.reject_invalid_message(propagation_source, &raw_message.topic); - } - (None, Ok(message)) => { - // Valid transformation without peer scoring - let message_id = self.config.message_id(&message); - self.gossip_promises - .reject_message(&message_id, &reject_reason); + peer_score.reject_invalid_message(propagation_source, topic_hash); } - (None, Err(_)) => {} } } @@ -3231,7 +3225,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), ) }