chore: Remove unused gossipsub items (#1491)

Related issues: - Closes #1376 This PR removes unused gossipsub request and repsonse items `NewBlock` and `ConsensusVote`. This includes implementation details, such as structs and constants, as well as tests.
FuelLabs · Nov 16, 2023 · c75314d · c75314d
1 parent df6a206
commit c75314d
Show file tree

Hide file tree

Showing 8 changed files with 6 additions and 237 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -81,6 +81,7 @@ Description of the upcoming release here.
 - [#1408](https://github.com/FuelLabs/fuel-core/pull/1408): Update gas benchmarks for storage opcodes to use a pre-populated database to get more accurate worst-case costs.
 
 #### Breaking
+- [#1491](https://github.com/FuelLabs/fuel-core/pull/1491): Removed unused request and response variants from the Gossipsub implementation, as well as related definitions and tests. Specifically, this removes gossiping of `ConsensusVote` and `NewBlock` events.
 - [#1472](https://github.com/FuelLabs/fuel-core/pull/1472): Upgraded `fuel-vm` to `v0.42.0`. It introduces transaction policies that changes layout of the transaction. FOr more information check the [v0.42.0](https://github.com/FuelLabs/fuel-vm/pull/635) release.
 - [#1470](https://github.com/FuelLabs/fuel-core/pull/1470): Divide `DependentCost` into "light" and "heavy" operations.
 - [#1464](https://github.com/FuelLabs/fuel-core/pull/1464): Avoid possible truncation of higher bits. It may invalidate the code that truncated higher bits causing different behavior on 32-bit vs. 64-bit systems. The change affects some endpoints that now require lesser integers.

diff --git a/crates/services/p2p/src/codecs/postcard.rs b/crates/services/p2p/src/codecs/postcard.rs
@@ -156,8 +156,6 @@ impl GossipsubCodec for PostcardCodec {
 
     fn encode(&self, data: Self::RequestMessage) -> Result<Vec<u8>, io::Error> {
         let encoded_data = match data {
-            GossipsubBroadcastRequest::ConsensusVote(vote) => postcard::to_stdvec(&*vote),
-            GossipsubBroadcastRequest::NewBlock(block) => postcard::to_stdvec(&*block),
             GossipsubBroadcastRequest::NewTx(tx) => postcard::to_stdvec(&*tx),
         };
 
@@ -173,12 +171,6 @@ impl GossipsubCodec for PostcardCodec {
             GossipTopicTag::NewTx => {
                 GossipsubMessage::NewTx(self.deserialize(encoded_data)?)
             }
-            GossipTopicTag::NewBlock => {
-                GossipsubMessage::NewBlock(self.deserialize(encoded_data)?)
-            }
-            GossipTopicTag::ConsensusVote => {
-                GossipsubMessage::ConsensusVote(self.deserialize(encoded_data)?)
-            }
         };
 
         Ok(decoded_response)

diff --git a/crates/services/p2p/src/gossipsub/config.rs b/crates/services/p2p/src/gossipsub/config.rs
@@ -27,8 +27,6 @@ use std::time::Duration;
 
 use super::topics::{
     GossipTopic,
-    CON_VOTE_GOSSIP_TOPIC,
-    NEW_BLOCK_GOSSIP_TOPIC,
     NEW_TX_GOSSIP_TOPIC,
 };
 
@@ -53,12 +51,6 @@ const MESH_SIZE: usize = 8;
 // The weight applied to the score for delivering new transactions.
 const NEW_TX_GOSSIP_WEIGHT: f64 = 0.05;
 
-// The weight applied to the score for delivering new blocks.
-const NEW_BLOCK_GOSSIP_WEIGHT: f64 = 0.05;
-
-// The weight applied to the score for delivering consensus votes.
-const CON_VOTE_GOSSIP_WEIGHT: f64 = 0.05;
-
 // The threshold for a peer's score to be considered for greylisting.
 // If a peer's score falls below this value, they will be greylisted.
 // Greylisting is a lighter form of banning, where the peer's messages might be ignored or given lower priority,
@@ -238,11 +230,7 @@ fn initialize_gossipsub(gossipsub: &mut Gossipsub, p2p_config: &Config) {
         .with_peer_score(peer_score_params, peer_score_thresholds)
         .expect("gossipsub initialized with peer score");
 
-    let topics = vec![
-        (NEW_TX_GOSSIP_TOPIC, NEW_TX_GOSSIP_WEIGHT),
-        (NEW_BLOCK_GOSSIP_TOPIC, NEW_BLOCK_GOSSIP_WEIGHT),
-        (CON_VOTE_GOSSIP_TOPIC, CON_VOTE_GOSSIP_WEIGHT),
-    ];
+    let topics = vec![(NEW_TX_GOSSIP_TOPIC, NEW_TX_GOSSIP_WEIGHT)];
 
     // subscribe to gossipsub topics with the network name suffix
     for (topic, weight) in topics {

diff --git a/crates/services/p2p/src/gossipsub/messages.rs b/crates/services/p2p/src/gossipsub/messages.rs
@@ -2,10 +2,6 @@ use std::sync::Arc;
 
 use fuel_core_types::fuel_tx::Transaction;
 
-use fuel_core_types::blockchain::{
-    block::Block,
-    consensus::ConsensusVote,
-};
 use serde::{
     Deserialize,
     Serialize,
@@ -16,8 +12,6 @@ use serde::{
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
 pub enum GossipTopicTag {
     NewTx,
-    NewBlock,
-    ConsensusVote,
 }
 
 /// Takes Arc<T> and wraps it in a matching GossipsubBroadcastRequest
@@ -26,13 +20,9 @@ pub enum GossipTopicTag {
 #[derive(Debug, Clone)]
 pub enum GossipsubBroadcastRequest {
     NewTx(Arc<Transaction>),
-    NewBlock(Arc<Block>),
-    ConsensusVote(Arc<ConsensusVote>),
 }
 
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub enum GossipsubMessage {
     NewTx(Transaction),
-    NewBlock(Block),
-    ConsensusVote(ConsensusVote),
 }
diff --git a/crates/services/p2p/src/gossipsub/topics.rs b/crates/services/p2p/src/gossipsub/topics.rs
@@ -11,31 +11,21 @@ use super::messages::{
 
 pub type GossipTopic = Sha256Topic;
 pub const NEW_TX_GOSSIP_TOPIC: &str = "new_tx";
-pub const NEW_BLOCK_GOSSIP_TOPIC: &str = "new_block";
-pub const CON_VOTE_GOSSIP_TOPIC: &str = "consensus_vote";
 
 /// Holds used Gossipsub Topics
 /// Each field contains TopicHash and GossipTopic itself
 /// in order to avoid converting GossipTopic to TopicHash on each received message
 #[derive(Debug)]
 pub struct GossipsubTopics {
     new_tx_topic: (TopicHash, GossipTopic),
-    new_block_topic: (TopicHash, GossipTopic),
-    consensus_vote_topic: (TopicHash, GossipTopic),
 }
 
 impl GossipsubTopics {
     pub fn new(network_name: &str) -> Self {
         let new_tx_topic = Topic::new(format!("{NEW_TX_GOSSIP_TOPIC}/{network_name}"));
-        let new_block_topic =
-            Topic::new(format!("{NEW_BLOCK_GOSSIP_TOPIC}/{network_name}"));
-        let consensus_vote_topic =
-            Topic::new(format!("{CON_VOTE_GOSSIP_TOPIC}/{network_name}"));
 
         Self {
             new_tx_topic: (new_tx_topic.hash(), new_tx_topic),
-            new_block_topic: (new_block_topic.hash(), new_block_topic),
-            consensus_vote_topic: (consensus_vote_topic.hash(), consensus_vote_topic),
         }
     }
 
@@ -44,18 +34,10 @@ impl GossipsubTopics {
         &self,
         incoming_topic: &TopicHash,
     ) -> Option<GossipTopicTag> {
-        let GossipsubTopics {
-            new_tx_topic,
-            new_block_topic,
-            consensus_vote_topic,
-        } = &self;
+        let GossipsubTopics { new_tx_topic } = &self;
 
         match incoming_topic {
             hash if hash == &new_tx_topic.0 => Some(GossipTopicTag::NewTx),
-            hash if hash == &new_block_topic.0 => Some(GossipTopicTag::NewBlock),
-            hash if hash == &consensus_vote_topic.0 => {
-                Some(GossipTopicTag::ConsensusVote)
-            }
             _ => None,
         }
     }
@@ -67,10 +49,6 @@ impl GossipsubTopics {
         outgoing_request: &GossipsubBroadcastRequest,
     ) -> GossipTopic {
         match outgoing_request {
-            GossipsubBroadcastRequest::ConsensusVote(_) => {
-                self.consensus_vote_topic.1.clone()
-            }
-            GossipsubBroadcastRequest::NewBlock(_) => self.new_block_topic.1.clone(),
             GossipsubBroadcastRequest::NewTx(_) => self.new_tx_topic.1.clone(),
         }
     }
@@ -79,13 +57,7 @@ impl GossipsubTopics {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use fuel_core_types::{
-        blockchain::{
-            block::Block,
-            consensus::ConsensusVote,
-        },
-        fuel_tx::Transaction,
-    };
+    use fuel_core_types::fuel_tx::Transaction;
     use libp2p::gossipsub::Topic;
     use std::sync::Arc;
 
@@ -94,50 +66,19 @@ mod tests {
         let network_name = "fuel_test_network";
         let new_tx_topic: GossipTopic =
             Topic::new(format!("{NEW_TX_GOSSIP_TOPIC}/{network_name}"));
-        let new_block_topic: GossipTopic =
-            Topic::new(format!("{NEW_BLOCK_GOSSIP_TOPIC}/{network_name}"));
-        let consensus_vote_topic: GossipTopic =
-            Topic::new(format!("{CON_VOTE_GOSSIP_TOPIC}/{network_name}"));
 
         let gossipsub_topics = GossipsubTopics::new(network_name);
 
         // Test matching Topic Hashes
         assert_eq!(gossipsub_topics.new_tx_topic.0, new_tx_topic.hash());
-        assert_eq!(gossipsub_topics.new_block_topic.0, new_block_topic.hash());
-        assert_eq!(
-            gossipsub_topics.consensus_vote_topic.0,
-            consensus_vote_topic.hash()
-        );
 
         // Test given a TopicHash that `get_gossipsub_tag()` returns matching `GossipTopicTag`
         assert_eq!(
             gossipsub_topics.get_gossipsub_tag(&new_tx_topic.hash()),
             Some(GossipTopicTag::NewTx)
         );
-        assert_eq!(
-            gossipsub_topics.get_gossipsub_tag(&new_block_topic.hash()),
-            Some(GossipTopicTag::NewBlock)
-        );
-        assert_eq!(
-            gossipsub_topics.get_gossipsub_tag(&consensus_vote_topic.hash()),
-            Some(GossipTopicTag::ConsensusVote)
-        );
 
         // Test given a `GossipsubBroadcastRequest` that `get_gossipsub_topic()` returns matching `Topic`
-        let broadcast_req =
-            GossipsubBroadcastRequest::ConsensusVote(Arc::new(ConsensusVote::default()));
-        assert_eq!(
-            gossipsub_topics.get_gossipsub_topic(&broadcast_req).hash(),
-            consensus_vote_topic.hash()
-        );
-
-        let broadcast_req =
-            GossipsubBroadcastRequest::NewBlock(Arc::new(Block::default()));
-        assert_eq!(
-            gossipsub_topics.get_gossipsub_topic(&broadcast_req).hash(),
-            new_block_topic.hash()
-        );
-
         let broadcast_req =
             GossipsubBroadcastRequest::NewTx(Arc::new(Transaction::default_test_tx()));
         assert_eq!(

diff --git a/crates/services/p2p/src/p2p_service.rs b/crates/services/p2p/src/p2p_service.rs
@@ -681,8 +681,6 @@ mod tests {
             },
             topics::{
                 GossipTopic,
-                CON_VOTE_GOSSIP_TOPIC,
-                NEW_BLOCK_GOSSIP_TOPIC,
                 NEW_TX_GOSSIP_TOPIC,
             },
         },
@@ -701,7 +699,6 @@ mod tests {
             consensus::{
                 poa::PoAConsensus,
                 Consensus,
-                ConsensusVote,
             },
             header::{
                 BlockHeader,
@@ -1228,52 +1225,12 @@ mod tests {
         .await;
     }
 
-    #[tokio::test]
-    #[instrument]
-    async fn gossipsub_broadcast_vote_with_accept() {
-        gossipsub_broadcast(
-            GossipsubBroadcastRequest::ConsensusVote(Arc::new(ConsensusVote::default())),
-            GossipsubMessageAcceptance::Accept,
-        )
-        .await;
-    }
-
-    #[tokio::test]
-    #[instrument]
-    async fn gossipsub_broadcast_vote_with_reject() {
-        gossipsub_broadcast(
-            GossipsubBroadcastRequest::ConsensusVote(Arc::new(ConsensusVote::default())),
-            GossipsubMessageAcceptance::Reject,
-        )
-        .await;
-    }
-
-    #[tokio::test]
-    #[instrument]
-    async fn gossipsub_broadcast_block_with_accept() {
-        gossipsub_broadcast(
-            GossipsubBroadcastRequest::NewBlock(Arc::new(Block::default())),
-            GossipsubMessageAcceptance::Accept,
-        )
-        .await;
-    }
-
-    #[tokio::test]
-    #[instrument]
-    async fn gossipsub_broadcast_block_with_ignore() {
-        gossipsub_broadcast(
-            GossipsubBroadcastRequest::NewBlock(Arc::new(Block::default())),
-            GossipsubMessageAcceptance::Ignore,
-        )
-        .await;
-    }
-
     #[tokio::test]
     #[instrument]
     #[ignore]
-    async fn gossipsub_scoring_with_accapted_messages() {
+    async fn gossipsub_scoring_with_accepted_messages() {
         gossipsub_scoring_tester(
-            "gossipsub_scoring_with_accapted_messages",
+            "gossipsub_scoring_with_accepted_messages",
             100,
             GossipsubMessageAcceptance::Accept,
         )
@@ -1365,28 +1322,6 @@ mod tests {
                         }
                     }
 
-                    let mut new_block = Block::default();
-                    *new_block.transactions_mut() = transactions;
-                    let new_block = GossipsubBroadcastRequest::NewBlock(Arc::new(new_block));
-
-                    match rand::thread_rng().gen_range(1..=3) {
-                        1 => {
-                            // Node A sends a Block
-                            let _ = node_a.publish_message(new_block);
-
-                        },
-                        2 => {
-                            // Node B sends a Block
-                            let _ = node_b.publish_message(new_block);
-
-                        },
-                        3 => {
-                            // Node C sends a Block
-                            let _ = node_c.publish_message(new_block);
-                        },
-                        _ => unreachable!("Random number generator is broken")
-                    }
-
                     eprintln!("Node A WORLD VIEW");
                     eprintln!("B score: {:?}", node_a.get_peer_score(&node_b.local_peer_id).unwrap());
                     eprintln!("C score: {:?}", node_a.get_peer_score(&node_c.local_peer_id).unwrap());
@@ -1419,8 +1354,6 @@ mod tests {
 
         let selected_topic: GossipTopic = {
             let topic = match broadcast_request {
-                GossipsubBroadcastRequest::ConsensusVote(_) => CON_VOTE_GOSSIP_TOPIC,
-                GossipsubBroadcastRequest::NewBlock(_) => NEW_BLOCK_GOSSIP_TOPIC,
                 GossipsubBroadcastRequest::NewTx(_) => NEW_TX_GOSSIP_TOPIC,
             };
 
@@ -1480,18 +1413,6 @@ mod tests {
                                     panic!("Wrong GossipsubMessage")
                                 }
                             }
-                            GossipsubMessage::NewBlock(block) => {
-                                if block.header().height() != Block::<Transaction>::default().header().height() {
-                                    tracing::error!("Wrong p2p message {:?}", message);
-                                    panic!("Wrong GossipsubMessage")
-                                }
-                            }
-                            GossipsubMessage::ConsensusVote(vote) => {
-                                if vote != &ConsensusVote::default() {
-                                    tracing::error!("Wrong p2p message {:?}", message);
-                                    panic!("Wrong GossipsubMessage")
-                                }
-                            }
                         }
 
                         // Node B received the correct message