From b29bb2e037fc209d1c3b39bcd1523091c7bc3224 Mon Sep 17 00:00:00 2001 From: Jack McPherson Date: Tue, 16 May 2023 01:10:47 +0000 Subject: [PATCH] Remove redundant gossipsub tests (#4294) ## Issue Addressed #2335 ## Proposed Changes - Remove the `lighthouse-network::tests::gossipsub_tests` module - Remove dead code from the `lighthouse-network::tests::common` helper module (`build_full_mesh`) ## Additional Info After discussion with both @divagant-martian and @AgeManning, these tests seem to have two main issues in that they are: - Redundant, in that they don't test anything meaningful (due to our handling of duplicate messages) - Out-of-place, in that it doesn't really test Lighthouse-specific functionality (rather libp2p functionality) As such, this PR supersedes #4286. --- .../lighthouse_network/tests/common.rs | 30 --- .../tests/gossipsub_tests.rs | 171 ------------------ 2 files changed, 201 deletions(-) delete mode 100644 beacon_node/lighthouse_network/tests/gossipsub_tests.rs diff --git a/beacon_node/lighthouse_network/tests/common.rs b/beacon_node/lighthouse_network/tests/common.rs index d44f20c0806..64714cbc0a8 100644 --- a/beacon_node/lighthouse_network/tests/common.rs +++ b/beacon_node/lighthouse_network/tests/common.rs @@ -123,36 +123,6 @@ pub fn get_enr(node: &LibP2PService) -> Enr { node.local_enr() } -// Returns `n` libp2p peers in fully connected topology. -#[allow(dead_code)] -/* -pub async fn build_full_mesh( - rt: Weak, - log: slog::Logger, - n: usize, - fork_name: ForkName, -) -> Vec { - let mut nodes = Vec::with_capacity(n); - for _ in 0..n { - nodes.push(build_libp2p_instance(rt.clone(), vec![], log.clone(), fork_name).await); - } - let multiaddrs: Vec = nodes - .iter() - .map(|x| get_enr(x).multiaddr()[1].clone()) - .collect(); - - for (i, node) in nodes.iter_mut().enumerate().take(n) { - for (j, multiaddr) in multiaddrs.iter().enumerate().skip(i) { - if i != j { - match libp2p::Swarm::dial(&mut node.swarm, multiaddr.clone()) { - Ok(()) => debug!(log, "Connected"), - Err(_) => error!(log, "Failed to connect"), - }; - } - } - } - nodes -}*/ // Constructs a pair of nodes with separate loggers. The sender dials the receiver. // This returns a (sender, receiver) pair. #[allow(dead_code)] diff --git a/beacon_node/lighthouse_network/tests/gossipsub_tests.rs b/beacon_node/lighthouse_network/tests/gossipsub_tests.rs deleted file mode 100644 index c5b661cf704..00000000000 --- a/beacon_node/lighthouse_network/tests/gossipsub_tests.rs +++ /dev/null @@ -1,171 +0,0 @@ -/* These are temporarily disabled due to their non-deterministic behaviour and impending update to - * gossipsub 1.1. We leave these here as a template for future test upgrades - - -#![cfg(test)] -use crate::types::GossipEncoding; -use ::types::{BeaconBlock, EthSpec, MinimalEthSpec, Signature, SignedBeaconBlock}; -use lighthouse_network::*; -use slog::{debug, Level}; - -type E = MinimalEthSpec; - -mod common; - -/* Gossipsub tests */ -// Note: The aim of these tests is not to test the robustness of the gossip network -// but to check if the gossipsub implementation is behaving according to the specifications. - -// Test if gossipsub message are forwarded by nodes with a simple linear topology. -// -// Topology used in test -// -// node1 <-> node2 <-> node3 ..... <-> node(n-1) <-> node(n) - -#[tokio::test] -async fn test_gossipsub_forward() { - // set up the logging. The level and enabled or not - let log = common::build_log(Level::Info, false); - - let num_nodes = 20; - let mut nodes = common::build_linear(log.clone(), num_nodes); - let mut received_count = 0; - let spec = E::default_spec(); - let empty_block = BeaconBlock::empty(&spec); - let signed_block = SignedBeaconBlock { - message: empty_block, - signature: Signature::empty_signature(), - }; - let pubsub_message = PubsubMessage::BeaconBlock(Box::new(signed_block)); - let publishing_topic: String = pubsub_message - .topics(GossipEncoding::default(), [0, 0, 0, 0]) - .first() - .unwrap() - .clone() - .into(); - let mut subscribed_count = 0; - let fut = async move { - for node in nodes.iter_mut() { - loop { - match node.next_event().await { - Libp2pEvent::Behaviour(b) => match b { - BehaviourEvent::PubsubMessage { - topics, - message, - source, - id, - } => { - assert_eq!(topics.len(), 1); - // Assert topic is the published topic - assert_eq!( - topics.first().unwrap(), - &TopicHash::from_raw(publishing_topic.clone()) - ); - // Assert message received is the correct one - assert_eq!(message, pubsub_message.clone()); - received_count += 1; - // Since `propagate_message` is false, need to propagate manually - node.swarm.propagate_message(&source, id); - // Test should succeed if all nodes except the publisher receive the message - if received_count == num_nodes - 1 { - debug!(log.clone(), "Received message at {} nodes", num_nodes - 1); - return; - } - } - BehaviourEvent::PeerSubscribed(_, topic) => { - // Publish on beacon block topic - if topic == TopicHash::from_raw(publishing_topic.clone()) { - subscribed_count += 1; - // Every node except the corner nodes are connected to 2 nodes. - if subscribed_count == (num_nodes * 2) - 2 { - node.swarm.publish(vec![pubsub_message.clone()]); - } - } - } - _ => break, - }, - _ => break, - } - } - } - }; - - tokio::select! { - _ = fut => {} - _ = tokio::time::delay_for(tokio::time::Duration::from_millis(800)) => { - panic!("Future timed out"); - } - } -} - -// Test publishing of a message with a full mesh for the topic -// Not very useful but this is the bare minimum functionality. -#[tokio::test] -async fn test_gossipsub_full_mesh_publish() { - // set up the logging. The level and enabled or not - let log = common::build_log(Level::Debug, false); - - // Note: This test does not propagate gossipsub messages. - // Having `num_nodes` > `mesh_n_high` may give inconsistent results - // as nodes may get pruned out of the mesh before the gossipsub message - // is published to them. - let num_nodes = 12; - let mut nodes = common::build_full_mesh(log, num_nodes); - let mut publishing_node = nodes.pop().unwrap(); - let spec = E::default_spec(); - let empty_block = BeaconBlock::empty(&spec); - let signed_block = SignedBeaconBlock { - message: empty_block, - signature: Signature::empty_signature(), - }; - let pubsub_message = PubsubMessage::BeaconBlock(Box::new(signed_block)); - let publishing_topic: String = pubsub_message - .topics(GossipEncoding::default(), [0, 0, 0, 0]) - .first() - .unwrap() - .clone() - .into(); - let mut subscribed_count = 0; - let mut received_count = 0; - let fut = async move { - for node in nodes.iter_mut() { - while let Libp2pEvent::Behaviour(BehaviourEvent::PubsubMessage { - topics, - message, - .. - }) = node.next_event().await - { - assert_eq!(topics.len(), 1); - // Assert topic is the published topic - assert_eq!( - topics.first().unwrap(), - &TopicHash::from_raw(publishing_topic.clone()) - ); - // Assert message received is the correct one - assert_eq!(message, pubsub_message.clone()); - received_count += 1; - if received_count == num_nodes - 1 { - return; - } - } - } - while let Libp2pEvent::Behaviour(BehaviourEvent::PeerSubscribed(_, topic)) = - publishing_node.next_event().await - { - // Publish on beacon block topic - if topic == TopicHash::from_raw(publishing_topic.clone()) { - subscribed_count += 1; - if subscribed_count == num_nodes - 1 { - publishing_node.swarm.publish(vec![pubsub_message.clone()]); - } - } - } - }; - tokio::select! { - _ = fut => {} - _ = tokio::time::delay_for(tokio::time::Duration::from_millis(800)) => { - panic!("Future timed out"); - } - } -} -*/