From a15e72ba4905e9ae213a868d69d1626a5ffe5923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Wed, 22 Nov 2023 23:17:38 +0000 Subject: [PATCH 01/11] feat(gossipsub): introduce backpressure --- Cargo.lock | 1 + protocols/gossipsub/Cargo.toml | 1 + protocols/gossipsub/src/behaviour.rs | 79 ++- protocols/gossipsub/src/behaviour/tests.rs | 764 +++++++++++---------- protocols/gossipsub/src/config.rs | 11 + protocols/gossipsub/src/handler.rs | 23 +- protocols/gossipsub/src/types.rs | 103 ++- 7 files changed, 582 insertions(+), 400 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e037dcac20b..2cb7737b373 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2623,6 +2623,7 @@ dependencies = [ name = "libp2p-gossipsub" version = "0.46.1" dependencies = [ + "async-channel", "async-std", "asynchronous-codec", "base64 0.21.5", diff --git a/protocols/gossipsub/Cargo.toml b/protocols/gossipsub/Cargo.toml index 37873de39f9..2a0510038f1 100644 --- a/protocols/gossipsub/Cargo.toml +++ b/protocols/gossipsub/Cargo.toml @@ -37,6 +37,7 @@ sha2 = "0.10.8" smallvec = "1.11.2" tracing = "0.1.37" void = "1.0.2" +async-channel = "1.9.0" # Metrics dependencies prometheus-client = { workspace = true } diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 24a32de4cc7..af8abd0725f 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -45,8 +45,6 @@ use libp2p_swarm::{ THandlerOutEvent, ToSwarm, }; -use crate::backoff::BackoffStorage; -use crate::config::{Config, ValidationMode}; use crate::gossip_promises::GossipPromises; use crate::handler::{Handler, HandlerEvent, HandlerIn}; use crate::mcache::MessageCache; @@ -62,6 +60,11 @@ use crate::types::{ SubscriptionAction, }; use crate::types::{PeerConnections, PeerKind, RpcOut}; +use crate::{backoff::BackoffStorage, types::RpcSender}; +use crate::{ + config::{Config, ValidationMode}, + types::rpc_channel, +}; use crate::{rpc_proto::proto, TopicScoreParams}; use crate::{PublishError, SubscriptionError, ValidationError}; use instant::SystemTime; @@ -332,6 +335,9 @@ pub struct Behaviour { /// Keep track of a set of internal metrics relating to gossipsub. metrics: Option, + + /// Connection handler message queue channels. + handler_send_queues: HashMap, } impl Behaviour @@ -471,6 +477,7 @@ where config, subscription_filter, data_transform, + handler_send_queues: Default::default(), }) } } @@ -537,7 +544,8 @@ where for peer in self.peer_topics.keys().copied().collect::>() { tracing::debug!(%peer, "Sending SUBSCRIBE to peer"); let event = RpcOut::Subscribe(topic_hash.clone()); - self.send_message(peer, event); + self.send_message(peer, event) + .expect("Subscribe messages should be always sent"); } // call JOIN(topic) @@ -564,7 +572,8 @@ where for peer in self.peer_topics.keys().copied().collect::>() { tracing::debug!(%peer, "Sending UNSUBSCRIBE to peer"); let event = RpcOut::Unsubscribe(topic_hash.clone()); - self.send_message(peer, event); + self.send_message(peer, event) + .expect("Subscribe messages should be always sent"); } // call LEAVE(topic) @@ -711,9 +720,18 @@ where } // Send to peers we know are subscribed to the topic. + let mut errors = 0; for peer_id in recipient_peers.iter() { tracing::trace!(peer=%peer_id, "Sending message to peer"); - self.send_message(*peer_id, RpcOut::Publish(raw_message.clone())); + if self + .send_message(*peer_id, RpcOut::Publish(raw_message.clone())) + .is_err() + { + errors += 1; + } + } + if errors == recipient_peers.len() { + return Err(PublishError::InsufficientPeers); } tracing::debug!(message=%msg_id, "Published message"); @@ -1311,7 +1329,7 @@ where ); } else { tracing::debug!(peer=%peer_id, "IWANT: Sending cached messages to peer"); - self.send_message(*peer_id, RpcOut::Forward(msg)); + let _ = self.send_message(*peer_id, RpcOut::Forward(msg)); } } } @@ -1469,7 +1487,8 @@ where .map(|t| self.make_prune(t, peer_id, do_px, on_unsubscribe)) .collect::>() { - self.send_message(*peer_id, RpcOut::Control(action)); + self.send_message(*peer_id, RpcOut::Control(action)) + .expect("PRUNE messages should always be sent"); } // Send the prune messages to the peer tracing::debug!( @@ -1970,6 +1989,7 @@ where .collect::>() { self.send_message(*propagation_source, RpcOut::Control(action)) + .expect("GRAFT messages should always be sent"); } // Notify the application of the subscriptions @@ -2520,7 +2540,8 @@ where // send the control messages for msg in control_msgs.chain(prunes).collect::>() { - self.send_message(peer, RpcOut::Control(msg)); + self.send_message(peer, RpcOut::Control(msg)) + .expect("PRUNE messages should always be sent"); } } @@ -2534,7 +2555,8 @@ where self.config.do_px() && !no_px.contains(peer), false, ); - self.send_message(*peer, RpcOut::Control(prune)); + self.send_message(*peer, RpcOut::Control(prune)) + .expect("PRUNE messages should always be sent"); // inform the handler peer_removed_from_mesh( @@ -2606,7 +2628,7 @@ where for peer in recipient_peers.iter() { tracing::debug!(%peer, message=%msg_id, "Sending message to peer"); - self.send_message(*peer, event.clone()); + let _ = self.send_message(*peer, event.clone()); } tracing::debug!("Completed forwarding message"); Ok(true) @@ -2720,7 +2742,7 @@ where fn flush_control_pool(&mut self) { for (peer, controls) in self.control_pool.drain().collect::>() { for msg in controls { - self.send_message(peer, RpcOut::Control(msg)); + let _ = self.send_message(peer, RpcOut::Control(msg)); } } @@ -2730,19 +2752,24 @@ where /// Send a [`RpcOut`] message to a peer. This will wrap the message in an arc if it /// is not already an arc. - fn send_message(&mut self, peer_id: PeerId, rpc: RpcOut) { + fn send_message(&mut self, peer_id: PeerId, rpc: RpcOut) -> Result<(), ()> { + let sender = self + .handler_send_queues + .get_mut(&peer_id) + .expect("Peerid should exist"); + + if sender.try_send(rpc.clone()).is_err() { + tracing::debug!(peer=%peer_id, "Dropping message as peer is full"); + return Err(()); + } + 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. m.msg_sent(&message.topic, message.raw_protobuf_len()); } } - - self.events.push_back(ToSwarm::NotifyHandler { - peer_id, - event: HandlerIn::Message(rpc), - handler: NotifyHandler::Any, - }); + Ok(()) } fn on_connection_established( @@ -2811,7 +2838,8 @@ where tracing::debug!(peer=%peer_id, "New peer connected"); // We need to send our subscriptions to the newly-connected node. for topic_hash in self.mesh.clone().into_keys() { - self.send_message(peer_id, RpcOut::Subscribe(topic_hash)); + self.send_message(peer_id, RpcOut::Subscribe(topic_hash)) + .expect("Subscribe messages should be always sent"); } } @@ -2939,6 +2967,7 @@ where } self.connected_peers.remove(&peer_id); + self.handler_send_queues.remove(&peer_id); if let Some((peer_score, ..)) = &mut self.peer_score { peer_score.remove_peer(&peer_id); @@ -2998,21 +3027,25 @@ where fn handle_established_inbound_connection( &mut self, _: ConnectionId, - _: PeerId, + peer_id: PeerId, _: &Multiaddr, _: &Multiaddr, ) -> Result, ConnectionDenied> { - Ok(Handler::new(self.config.protocol_config())) + let (sender, receiver) = rpc_channel(self.config.connection_handler_queue_len()); + self.handler_send_queues.insert(peer_id, sender); + Ok(Handler::new(self.config.protocol_config(), receiver)) } fn handle_established_outbound_connection( &mut self, _: ConnectionId, - _: PeerId, + peer_id: PeerId, _: &Multiaddr, _: Endpoint, ) -> Result, ConnectionDenied> { - Ok(Handler::new(self.config.protocol_config())) + let (sender, receiver) = rpc_channel(self.config.connection_handler_queue_len()); + self.handler_send_queues.insert(peer_id, sender); + Ok(Handler::new(self.config.protocol_config(), receiver)) } fn on_connection_handler_event( diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index 570cdf43f90..050b387a5e3 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -23,6 +23,7 @@ use super::*; use crate::subscription_filter::WhitelistSubscriptionFilter; use crate::transform::{DataTransform, IdentityTransform}; +use crate::types::RpcReceiver; use crate::ValidationError; use crate::{ config::Config, config::ConfigBuilder, types::Rpc, IdentTopic as Topic, TopicScoreParams, @@ -58,7 +59,14 @@ where D: DataTransform + Default + Clone + Send + 'static, F: TopicSubscriptionFilter + Clone + Default + Send + 'static, { - pub(crate) fn create_network(self) -> (Behaviour, Vec, Vec) { + pub(crate) fn create_network( + self, + ) -> ( + Behaviour, + Vec, + HashMap, + Vec, + ) { let keypair = libp2p_identity::Keypair::generate_ed25519(); // create a gossipsub struct let mut gs: Behaviour = Behaviour::new_with_subscription_filter_and_transform( @@ -86,10 +94,11 @@ where // build and connect peer_no random peers let mut peers = vec![]; + let mut receiver_queues = HashMap::new(); let empty = vec![]; for i in 0..self.peer_no { - peers.push(add_peer( + let (peer, receiver) = add_peer( &mut gs, if self.to_subscribe { &topic_hashes @@ -98,10 +107,12 @@ where }, i < self.outbound, i < self.explicit, - )); + ); + peers.push(peer); + receiver_queues.insert(peer, receiver); } - (gs, peers, topic_hashes) + (gs, peers, receiver_queues, topic_hashes) } fn peer_no(mut self, peer_no: usize) -> Self { @@ -165,7 +176,7 @@ fn add_peer( topic_hashes: &Vec, outbound: bool, explicit: bool, -) -> PeerId +) -> (PeerId, RpcReceiver) where D: DataTransform + Default + Clone + Send + 'static, F: TopicSubscriptionFilter + Clone + Default + Send + 'static, @@ -179,7 +190,7 @@ fn add_peer_with_addr( outbound: bool, explicit: bool, address: Multiaddr, -) -> PeerId +) -> (PeerId, RpcReceiver) where D: DataTransform + Default + Clone + Send + 'static, F: TopicSubscriptionFilter + Clone + Default + Send + 'static, @@ -201,7 +212,7 @@ fn add_peer_with_addr_and_kind( explicit: bool, address: Multiaddr, kind: Option, -) -> PeerId +) -> (PeerId, RpcReceiver) where D: DataTransform + Default + Clone + Send + 'static, F: TopicSubscriptionFilter + Clone + Default + Send + 'static, @@ -219,6 +230,9 @@ where } }; + let (sender, receiver) = rpc_channel(100); + gs.handler_send_queues.insert(peer, sender); + gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished { peer_id: peer, connection_id: ConnectionId::new_unchecked(0), @@ -249,7 +263,7 @@ where &peer, ); } - peer + (peer, receiver) } fn disconnect_peer(gs: &mut Behaviour, peer_id: &PeerId) @@ -389,7 +403,7 @@ fn test_subscribe() { // - run JOIN(topic) let subscribe_topic = vec![String::from("test_subscribe")]; - let (gs, _, topic_hashes) = inject_nodes1() + let (gs, _, queues, topic_hashes) = inject_nodes1() .peer_no(20) .topics(subscribe_topic) .to_subscribe(true) @@ -401,19 +415,16 @@ fn test_subscribe() { ); // collect all the subscriptions - let subscriptions = gs - .events - .iter() - .filter(|e| { - matches!( - e, - ToSwarm::NotifyHandler { - event: HandlerIn::Message(RpcOut::Subscribe(_)), - .. + let subscriptions = queues + .into_values() + .fold(0, |mut collected_subscriptions, c| { + while !c.priority.is_empty() { + if let Ok(RpcOut::Subscribe(_)) = c.priority.try_recv() { + collected_subscriptions = collected_subscriptions + 1 } - ) - }) - .count(); + } + collected_subscriptions + }); // we sent a subscribe to all known peers assert_eq!(subscriptions, 20); @@ -434,7 +445,7 @@ fn test_unsubscribe() { .collect::>(); // subscribe to topic_strings - let (mut gs, _, topic_hashes) = inject_nodes1() + let (mut gs, _, queues, topic_hashes) = inject_nodes1() .peer_no(20) .topics(topic_strings) .to_subscribe(true) @@ -462,15 +473,15 @@ fn test_unsubscribe() { ); // collect all the subscriptions - let subscriptions = gs - .events - .iter() - .fold(0, |collected_subscriptions, e| match e { - ToSwarm::NotifyHandler { - event: HandlerIn::Message(RpcOut::Subscribe(_)), - .. - } => collected_subscriptions + 1, - _ => collected_subscriptions, + let subscriptions = queues + .into_values() + .fold(0, |mut collected_subscriptions, c| { + while !c.priority.is_empty() { + if let Ok(RpcOut::Subscribe(_)) = c.priority.try_recv() { + collected_subscriptions = collected_subscriptions + 1 + } + } + collected_subscriptions }); // we sent a unsubscribe to all known peers, for two topics @@ -503,7 +514,7 @@ fn test_join() { .map(|t| Topic::new(t.clone())) .collect::>(); - let (mut gs, _, topic_hashes) = inject_nodes1() + let (mut gs, _, _receivers, topic_hashes) = inject_nodes1() .peer_no(20) .topics(topic_strings) .to_subscribe(true) @@ -557,14 +568,27 @@ fn test_join() { gs.fanout .insert(topic_hashes[1].clone(), Default::default()); let mut new_peers: Vec = vec![]; + + let mut peers = vec![]; for _ in 0..3 { let random_peer = PeerId::random(); // inform the behaviour of a new peer + let address = "/ip4/127.0.0.1".parse::().unwrap(); + let peer = gs + .handle_established_inbound_connection( + ConnectionId::new_unchecked(0), + random_peer, + &address, + &address, + ) + .unwrap(); + peers.push(peer); + gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished { peer_id: random_peer, connection_id: ConnectionId::new_unchecked(0), endpoint: &ConnectedPoint::Dialer { - address: "/ip4/127.0.0.1".parse::().unwrap(), + address, role_override: Endpoint::Dialer, }, failed_addresses: &[], @@ -616,7 +640,7 @@ fn test_publish_without_flood_publishing() { .unwrap(); let publish_topic = String::from("test_publish"); - let (mut gs, _, topic_hashes) = inject_nodes1() + let (mut gs, _, queues, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![publish_topic.clone()]) .to_subscribe(true) @@ -640,18 +664,15 @@ fn test_publish_without_flood_publishing() { gs.publish(Topic::new(publish_topic), publish_data).unwrap(); // Collect all publish messages - let publishes = gs - .events - .into_iter() - .fold(vec![], |mut collected_publish, e| match e { - ToSwarm::NotifyHandler { - event: HandlerIn::Message(RpcOut::Publish(message)), - .. - } => { - collected_publish.push(message); - collected_publish + let publishes = queues + .into_values() + .fold(vec![], |mut collected_publish, c| { + while !c.priority.is_empty() { + if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { + collected_publish.push(message); + } } - _ => collected_publish, + collected_publish }); // Transform the inbound message @@ -695,7 +716,7 @@ fn test_fanout() { .unwrap(); let fanout_topic = String::from("test_fanout"); - let (mut gs, _, topic_hashes) = inject_nodes1() + let (mut gs, _, queues, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![fanout_topic.clone()]) .to_subscribe(true) @@ -727,18 +748,15 @@ fn test_fanout() { ); // Collect all publish messages - let publishes = gs - .events - .into_iter() - .fold(vec![], |mut collected_publish, e| match e { - ToSwarm::NotifyHandler { - event: HandlerIn::Message(RpcOut::Publish(message)), - .. - } => { - collected_publish.push(message); - collected_publish + let publishes = queues + .into_values() + .fold(vec![], |mut collected_publish, c| { + while !c.priority.is_empty() { + if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { + collected_publish.push(message); + } } - _ => collected_publish, + collected_publish }); // Transform the inbound message @@ -769,7 +787,7 @@ fn test_fanout() { #[test] /// Test the gossipsub NetworkBehaviour peer connection logic. fn test_inject_connected() { - let (gs, peers, topic_hashes) = inject_nodes1() + let (gs, peers, queues, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![String::from("topic1"), String::from("topic2")]) .to_subscribe(true) @@ -777,26 +795,19 @@ fn test_inject_connected() { // check that our subscriptions are sent to each of the peers // collect all the SendEvents - let subscriptions = gs - .events - .into_iter() - .filter_map(|e| match e { - ToSwarm::NotifyHandler { - event: HandlerIn::Message(RpcOut::Subscribe(topic)), - peer_id, - .. - } => Some((peer_id, topic)), - _ => None, - }) - .fold( - HashMap::>::new(), - |mut subs, (peer, sub)| { - let mut peer_subs = subs.remove(&peer).unwrap_or_default(); - peer_subs.push(sub.into_string()); - subs.insert(peer, peer_subs); - subs - }, - ); + let subscriptions = queues.into_iter().fold( + HashMap::>::new(), + |mut collected_subscriptions, (peer, c)| { + while !c.priority.is_empty() { + if let Ok(RpcOut::Subscribe(topic)) = c.priority.try_recv() { + let mut peer_subs = collected_subscriptions.remove(&peer).unwrap_or_default(); + peer_subs.push(topic.into_string()); + collected_subscriptions.insert(peer, peer_subs); + } + } + collected_subscriptions + }, + ); // check that there are two subscriptions sent to each peer for peer_subs in subscriptions.values() { @@ -831,7 +842,7 @@ fn test_handle_received_subscriptions() { .iter() .map(|&t| String::from(t)) .collect(); - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, _receivers, topic_hashes) = inject_nodes1() .peer_no(20) .topics(topics) .to_subscribe(false) @@ -991,7 +1002,7 @@ fn test_get_random_peers() { /// Tests that the correct message is sent when a peer asks for a message in our cache. #[test] fn test_handle_iwant_msg_cached() { - let (mut gs, peers, _) = inject_nodes1() + let (mut gs, peers, queues, _) = inject_nodes1() .peer_no(20) .topics(Vec::new()) .to_subscribe(true) @@ -1019,18 +1030,16 @@ fn test_handle_iwant_msg_cached() { gs.handle_iwant(&peers[7], vec![msg_id.clone()]); // the messages we are sending - let sent_messages = gs.events.into_iter().fold( - Vec::::new(), - |mut collected_messages, e| match e { - ToSwarm::NotifyHandler { event, .. } => { - if let HandlerIn::Message(RpcOut::Forward(message)) = event { - collected_messages.push(message); + let sent_messages = queues + .into_values() + .fold(vec![], |mut collected_messages, c| { + while !c.non_priority.is_empty() { + if let Ok(RpcOut::Forward(msg)) = c.non_priority.try_recv() { + collected_messages.push(msg) } - collected_messages } - _ => collected_messages, - }, - ); + collected_messages + }); assert!( sent_messages @@ -1044,7 +1053,7 @@ fn test_handle_iwant_msg_cached() { /// Tests that messages are sent correctly depending on the shifting of the message cache. #[test] fn test_handle_iwant_msg_cached_shifted() { - let (mut gs, peers, _) = inject_nodes1() + let (mut gs, peers, queues, _) = inject_nodes1() .peer_no(20) .topics(Vec::new()) .to_subscribe(true) @@ -1077,18 +1086,20 @@ fn test_handle_iwant_msg_cached_shifted() { gs.handle_iwant(&peers[7], vec![msg_id.clone()]); // is the message is being sent? - let message_exists = gs.events.iter().any(|e| match e { - ToSwarm::NotifyHandler { - event: HandlerIn::Message(RpcOut::Forward(message)), - .. - } => { - gs.config.message_id( - &gs.data_transform - .inbound_transform(message.clone()) - .unwrap(), - ) == msg_id + let message_exists = queues.values().any(|c| { + let mut out = false; + while !c.non_priority.is_empty() { + if matches!(c.non_priority.try_recv(), Ok(RpcOut::Forward(message)) if + gs.config.message_id( + &gs.data_transform + .inbound_transform(message.clone()) + .unwrap(), + ) == msg_id) + { + out = true; + } } - _ => false, + out }); // default history_length is 5, expect no messages after shift > 5 if shift < 5 { @@ -1108,7 +1119,7 @@ fn test_handle_iwant_msg_cached_shifted() { #[test] // tests that an event is not created when a peers asks for a message not in our cache fn test_handle_iwant_msg_not_cached() { - let (mut gs, peers, _) = inject_nodes1() + let (mut gs, peers, _, _) = inject_nodes1() .peer_no(20) .topics(Vec::new()) .to_subscribe(true) @@ -1127,7 +1138,7 @@ fn test_handle_iwant_msg_not_cached() { #[test] // tests that an event is created when a peer shares that it has a message we want fn test_handle_ihave_subscribed_and_msg_not_cached() { - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, _, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![String::from("topic1")]) .to_subscribe(true) @@ -1159,7 +1170,7 @@ fn test_handle_ihave_subscribed_and_msg_not_cached() { // tests that an event is not created when a peer shares that it has a message that // we already have fn test_handle_ihave_subscribed_and_msg_cached() { - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, _, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![String::from("topic1")]) .to_subscribe(true) @@ -1181,7 +1192,7 @@ fn test_handle_ihave_subscribed_and_msg_cached() { // test that an event is not created when a peer shares that it has a message in // a topic that we are not subscribed to fn test_handle_ihave_not_subscribed() { - let (mut gs, peers, _) = inject_nodes1() + let (mut gs, peers, _, _) = inject_nodes1() .peer_no(20) .topics(vec![]) .to_subscribe(true) @@ -1207,7 +1218,7 @@ fn test_handle_ihave_not_subscribed() { // tests that a peer is added to our mesh when we are both subscribed // to the same topic fn test_handle_graft_is_subscribed() { - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, _, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![String::from("topic1")]) .to_subscribe(true) @@ -1225,7 +1236,7 @@ fn test_handle_graft_is_subscribed() { // tests that a peer is not added to our mesh when they are subscribed to // a topic that we are not fn test_handle_graft_is_not_subscribed() { - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, _, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![String::from("topic1")]) .to_subscribe(true) @@ -1250,7 +1261,7 @@ fn test_handle_graft_multiple_topics() { .map(|&t| String::from(t)) .collect(); - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, _, topic_hashes) = inject_nodes1() .peer_no(20) .topics(topics) .to_subscribe(true) @@ -1280,7 +1291,7 @@ fn test_handle_graft_multiple_topics() { #[test] // tests that a peer is removed from our mesh fn test_handle_prune_peer_in_mesh() { - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, _, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![String::from("topic1")]) .to_subscribe(true) @@ -1309,34 +1320,50 @@ fn test_handle_prune_peer_in_mesh() { fn count_control_msgs( gs: &Behaviour, + queues: &HashMap, mut filter: impl FnMut(&PeerId, &ControlAction) -> bool, ) -> usize { gs.control_pool .iter() .map(|(peer_id, actions)| actions.iter().filter(|m| filter(peer_id, m)).count()) .sum::() - + gs.events + + queues .iter() - .filter(|e| match e { - ToSwarm::NotifyHandler { - peer_id, - event: HandlerIn::Message(RpcOut::Control(action)), - .. - } => filter(peer_id, action), - _ => false, + .fold(0, |mut collected_messages, (peer_id, c)| { + while !c.priority.is_empty() || !c.non_priority.is_empty() { + if let Ok(RpcOut::Control(action)) = c.priority.try_recv() { + if filter(peer_id, &action) { + collected_messages = collected_messages + 1; + } + } + if let Ok(RpcOut::Control(action)) = c.non_priority.try_recv() { + if filter(peer_id, &action) { + collected_messages = collected_messages + 1; + } + } + } + collected_messages }) - .count() } -fn flush_events(gs: &mut Behaviour) { +fn flush_events( + gs: &mut Behaviour, + receiver_queues: &mut HashMap, +) { gs.control_pool.clear(); gs.events.clear(); + for c in receiver_queues.values_mut() { + while !c.priority.is_empty() || !c.non_priority.is_empty() { + let _ = c.priority.try_recv(); + let _ = c.non_priority.try_recv(); + } + } } #[test] // tests that a peer added as explicit peer gets connected to fn test_explicit_peer_gets_connected() { - let (mut gs, _, _) = inject_nodes1() + let (mut gs, _, _, _) = inject_nodes1() .peer_no(0) .topics(Vec::new()) .to_subscribe(true) @@ -1369,7 +1396,7 @@ fn test_explicit_peer_reconnects() { .check_explicit_peers_ticks(2) .build() .unwrap(); - let (mut gs, others, _) = inject_nodes1() + let (mut gs, others, mut queues, _) = inject_nodes1() .peer_no(1) .topics(Vec::new()) .to_subscribe(true) @@ -1381,7 +1408,7 @@ fn test_explicit_peer_reconnects() { //add peer as explicit peer gs.add_explicit_peer(peer); - flush_events(&mut gs); + flush_events(&mut gs, &mut queues); //disconnect peer disconnect_peer(&mut gs, peer); @@ -1419,7 +1446,7 @@ fn test_explicit_peer_reconnects() { #[test] fn test_handle_graft_explicit_peer() { - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, queues, topic_hashes) = inject_nodes1() .peer_no(1) .topics(vec![String::from("topic1"), String::from("topic2")]) .to_subscribe(true) @@ -1437,7 +1464,7 @@ fn test_handle_graft_explicit_peer() { //check prunes assert!( - count_control_msgs(&gs, |peer_id, m| peer_id == peer + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == peer && match m { ControlAction::Prune { topic_hash, .. } => topic_hash == &topic_hashes[0] || topic_hash == &topic_hashes[1], @@ -1450,7 +1477,7 @@ fn test_handle_graft_explicit_peer() { #[test] fn explicit_peers_not_added_to_mesh_on_receiving_subscription() { - let (gs, peers, topic_hashes) = inject_nodes1() + let (gs, peers, queues, topic_hashes) = inject_nodes1() .peer_no(2) .topics(vec![String::from("topic1")]) .to_subscribe(true) @@ -1466,7 +1493,7 @@ fn explicit_peers_not_added_to_mesh_on_receiving_subscription() { //assert that graft gets created to non-explicit peer assert!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[1] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[1] && matches!(m, ControlAction::Graft { .. })) >= 1, "No graft message got created to non-explicit peer" @@ -1474,7 +1501,7 @@ fn explicit_peers_not_added_to_mesh_on_receiving_subscription() { //assert that no graft gets created to explicit peer assert_eq!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[0] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] && matches!(m, ControlAction::Graft { .. })), 0, "A graft message got created to an explicit peer" @@ -1483,7 +1510,7 @@ fn explicit_peers_not_added_to_mesh_on_receiving_subscription() { #[test] fn do_not_graft_explicit_peer() { - let (mut gs, others, topic_hashes) = inject_nodes1() + let (mut gs, others, queues, topic_hashes) = inject_nodes1() .peer_no(1) .topics(vec![String::from("topic")]) .to_subscribe(true) @@ -1498,7 +1525,7 @@ fn do_not_graft_explicit_peer() { //assert that no graft gets created to explicit peer assert_eq!( - count_control_msgs(&gs, |peer_id, m| peer_id == &others[0] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &others[0] && matches!(m, ControlAction::Graft { .. })), 0, "A graft message got created to an explicit peer" @@ -1507,7 +1534,7 @@ fn do_not_graft_explicit_peer() { #[test] fn do_forward_messages_to_explicit_peers() { - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, queues, topic_hashes) = inject_nodes1() .peer_no(2) .topics(vec![String::from("topic1"), String::from("topic2")]) .to_subscribe(true) @@ -1527,21 +1554,15 @@ fn do_forward_messages_to_explicit_peers() { validated: true, }; gs.handle_received_message(message.clone(), &local_id); - assert_eq!( - gs.events - .iter() - .filter(|e| match e { - ToSwarm::NotifyHandler { - peer_id, - event: HandlerIn::Message(RpcOut::Forward(m)), - .. - } => { - peer_id == &peers[0] && m.data == message.data + queues.into_iter().fold(0, |mut fwds, (peer_id, c)| { + while !c.non_priority.is_empty() { + if matches!(c.non_priority.try_recv(), Ok(RpcOut::Forward(m)) if peer_id == peers[0] && m.data == message.data) { + fwds = fwds +1; + } } - _ => false, - }) - .count(), + fwds + }), 1, "The message did not get forwarded to the explicit peer" ); @@ -1549,7 +1570,7 @@ fn do_forward_messages_to_explicit_peers() { #[test] fn explicit_peers_not_added_to_mesh_on_subscribe() { - let (mut gs, peers, _) = inject_nodes1() + let (mut gs, peers, queues, _) = inject_nodes1() .peer_no(2) .topics(Vec::new()) .to_subscribe(true) @@ -1578,7 +1599,7 @@ fn explicit_peers_not_added_to_mesh_on_subscribe() { //assert that graft gets created to non-explicit peer assert!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[1] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[1] && matches!(m, ControlAction::Graft { .. })) > 0, "No graft message got created to non-explicit peer" @@ -1586,7 +1607,7 @@ fn explicit_peers_not_added_to_mesh_on_subscribe() { //assert that no graft gets created to explicit peer assert_eq!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[0] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] && matches!(m, ControlAction::Graft { .. })), 0, "A graft message got created to an explicit peer" @@ -1595,7 +1616,7 @@ fn explicit_peers_not_added_to_mesh_on_subscribe() { #[test] fn explicit_peers_not_added_to_mesh_from_fanout_on_subscribe() { - let (mut gs, peers, _) = inject_nodes1() + let (mut gs, peers, queues, _) = inject_nodes1() .peer_no(2) .topics(Vec::new()) .to_subscribe(true) @@ -1627,7 +1648,7 @@ fn explicit_peers_not_added_to_mesh_from_fanout_on_subscribe() { //assert that graft gets created to non-explicit peer assert!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[1] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[1] && matches!(m, ControlAction::Graft { .. })) >= 1, "No graft message got created to non-explicit peer" @@ -1635,7 +1656,7 @@ fn explicit_peers_not_added_to_mesh_from_fanout_on_subscribe() { //assert that no graft gets created to explicit peer assert_eq!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[0] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] && matches!(m, ControlAction::Graft { .. })), 0, "A graft message got created to an explicit peer" @@ -1644,7 +1665,7 @@ fn explicit_peers_not_added_to_mesh_from_fanout_on_subscribe() { #[test] fn no_gossip_gets_sent_to_explicit_peers() { - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, _, topic_hashes) = inject_nodes1() .peer_no(2) .topics(vec![String::from("topic1"), String::from("topic2")]) .to_subscribe(true) @@ -1691,7 +1712,7 @@ fn test_mesh_addition() { let config: Config = Config::default(); // Adds mesh_low peers and PRUNE 2 giving us a deficit. - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _queues, topics) = inject_nodes1() .peer_no(config.mesh_n() + 1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1727,7 +1748,7 @@ fn test_mesh_subtraction() { // Adds mesh_low peers and PRUNE 2 giving us a deficit. let n = config.mesh_n_high() + 10; //make all outbound connections so that we allow grafting to all - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _receivers, topics) = inject_nodes1() .peer_no(n) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1751,7 +1772,7 @@ fn test_mesh_subtraction() { fn test_connect_to_px_peers_on_handle_prune() { let config: Config = Config::default(); - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1807,7 +1828,7 @@ fn test_send_px_and_backoff_in_prune() { let config: Config = Config::default(); //build mesh with enough peers for px - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, queues, topics) = inject_nodes1() .peer_no(config.prune_peers() + 1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1824,7 +1845,7 @@ fn test_send_px_and_backoff_in_prune() { //check prune message assert_eq!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[0] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] && match m { ControlAction::Prune { topic_hash, @@ -1848,7 +1869,7 @@ fn test_prune_backoffed_peer_on_graft() { let config: Config = Config::default(); //build mesh with enough peers for px - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, mut queues, topics) = inject_nodes1() .peer_no(config.prune_peers() + 1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1865,14 +1886,14 @@ fn test_prune_backoffed_peer_on_graft() { ); //ignore all messages until now - gs.events.clear(); + flush_events(&mut gs, &mut queues); //handle graft gs.handle_graft(&peers[0], vec![topics[0].clone()]); //check prune message assert_eq!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[0] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] && match m { ControlAction::Prune { topic_hash, @@ -1897,7 +1918,7 @@ fn test_do_not_graft_within_backoff_period() { .build() .unwrap(); //only one peer => mesh too small and will try to regraft as early as possible - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, mut queues, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1908,7 +1929,7 @@ fn test_do_not_graft_within_backoff_period() { gs.handle_prune(&peers[0], vec![(topics[0].clone(), Vec::new(), Some(1))]); //forget all events until now - flush_events(&mut gs); + flush_events(&mut gs, &mut queues); //call heartbeat gs.heartbeat(); @@ -1922,7 +1943,10 @@ fn test_do_not_graft_within_backoff_period() { //Check that no graft got created (we have backoff_slack = 1 therefore one more heartbeat // is needed). assert_eq!( - count_control_msgs(&gs, |_, m| matches!(m, ControlAction::Graft { .. })), + count_control_msgs(&gs, &queues, |_, m| matches!( + m, + ControlAction::Graft { .. } + )), 0, "Graft message created too early within backoff period" ); @@ -1933,7 +1957,10 @@ fn test_do_not_graft_within_backoff_period() { //check that graft got created assert!( - count_control_msgs(&gs, |_, m| matches!(m, ControlAction::Graft { .. })) > 0, + count_control_msgs(&gs, &queues, |_, m| matches!( + m, + ControlAction::Graft { .. } + )) > 0, "No graft message was created after backoff period" ); } @@ -1948,7 +1975,7 @@ fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without .build() .unwrap(); //only one peer => mesh too small and will try to regraft as early as possible - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, mut queues, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1959,7 +1986,7 @@ fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without gs.handle_prune(&peers[0], vec![(topics[0].clone(), Vec::new(), None)]); //forget all events until now - flush_events(&mut gs); + flush_events(&mut gs, &mut queues); //call heartbeat gs.heartbeat(); @@ -1971,7 +1998,10 @@ fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without //Check that no graft got created (we have backoff_slack = 1 therefore one more heartbeat // is needed). assert_eq!( - count_control_msgs(&gs, |_, m| matches!(m, ControlAction::Graft { .. })), + count_control_msgs(&gs, &queues, |_, m| matches!( + m, + ControlAction::Graft { .. } + )), 0, "Graft message created too early within backoff period" ); @@ -1982,7 +2012,10 @@ fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without //check that graft got created assert!( - count_control_msgs(&gs, |_, m| matches!(m, ControlAction::Graft { .. })) > 0, + count_control_msgs(&gs, &queues, |_, m| matches!( + m, + ControlAction::Graft { .. } + )) > 0, "No graft message was created after backoff period" ); } @@ -2001,7 +2034,7 @@ fn test_unsubscribe_backoff() { let topic = String::from("test"); // only one peer => mesh too small and will try to regraft as early as possible - let (mut gs, _, topics) = inject_nodes1() + let (mut gs, _, mut queues, topics) = inject_nodes1() .peer_no(1) .topics(vec![topic.clone()]) .to_subscribe(true) @@ -2011,7 +2044,7 @@ fn test_unsubscribe_backoff() { let _ = gs.unsubscribe(&Topic::new(topic)); assert_eq!( - count_control_msgs(&gs, |_, m| match m { + count_control_msgs(&gs, &queues, |_, m| match m { ControlAction::Prune { backoff, .. } => backoff == &Some(1), _ => false, }), @@ -2022,7 +2055,7 @@ fn test_unsubscribe_backoff() { let _ = gs.subscribe(&Topic::new(topics[0].to_string())); // forget all events until now - flush_events(&mut gs); + flush_events(&mut gs, &mut queues); // call heartbeat gs.heartbeat(); @@ -2036,7 +2069,10 @@ fn test_unsubscribe_backoff() { // Check that no graft got created (we have backoff_slack = 1 therefore one more heartbeat // is needed). assert_eq!( - count_control_msgs(&gs, |_, m| matches!(m, ControlAction::Graft { .. })), + count_control_msgs(&gs, &queues, |_, m| matches!( + m, + ControlAction::Graft { .. } + )), 0, "Graft message created too early within backoff period" ); @@ -2047,7 +2083,10 @@ fn test_unsubscribe_backoff() { // check that graft got created assert!( - count_control_msgs(&gs, |_, m| matches!(m, ControlAction::Graft { .. })) > 0, + count_control_msgs(&gs, &queues, |_, m| matches!( + m, + ControlAction::Graft { .. } + )) > 0, "No graft message was created after backoff period" ); } @@ -2058,7 +2097,7 @@ fn test_flood_publish() { let topic = "test"; // Adds more peers than mesh can hold to test flood publishing - let (mut gs, _, _) = inject_nodes1() + let (mut gs, _, queues, _) = inject_nodes1() .peer_no(config.mesh_n_high() + 10) .topics(vec![topic.into()]) .to_subscribe(true) @@ -2069,17 +2108,15 @@ fn test_flood_publish() { gs.publish(Topic::new(topic), publish_data).unwrap(); // Collect all publish messages - let publishes = gs - .events - .into_iter() - .fold(vec![], |mut collected_publish, e| match e { - ToSwarm::NotifyHandler { event, .. } => { - if let HandlerIn::Message(RpcOut::Publish(message)) = event { + let publishes = queues + .into_values() + .fold(vec![], |mut collected_publish, c| { + while !c.priority.is_empty() { + if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { collected_publish.push(message); } - collected_publish } - _ => collected_publish, + collected_publish }); // Transform the inbound message @@ -2114,7 +2151,7 @@ fn test_gossip_to_at_least_gossip_lazy_peers() { //add more peers than in mesh to test gossipping //by default only mesh_n_low peers will get added to mesh - let (mut gs, _, topic_hashes) = inject_nodes1() + let (mut gs, _, queues, topic_hashes) = inject_nodes1() .peer_no(config.mesh_n_low() + config.gossip_lazy() + 1) .topics(vec!["topic".into()]) .to_subscribe(true) @@ -2142,7 +2179,7 @@ fn test_gossip_to_at_least_gossip_lazy_peers() { //check that exactly config.gossip_lazy() many gossip messages were sent. assert_eq!( - count_control_msgs(&gs, |_, action| match action { + count_control_msgs(&gs, &queues, |_, action| match action { ControlAction::IHave { topic_hash, message_ids, @@ -2159,7 +2196,7 @@ fn test_gossip_to_at_most_gossip_factor_peers() { //add a lot of peers let m = config.mesh_n_low() + config.gossip_lazy() * (2.0 / config.gossip_factor()) as usize; - let (mut gs, _, topic_hashes) = inject_nodes1() + let (mut gs, _, queues, topic_hashes) = inject_nodes1() .peer_no(m) .topics(vec!["topic".into()]) .to_subscribe(true) @@ -2186,7 +2223,7 @@ fn test_gossip_to_at_most_gossip_factor_peers() { let msg_id = gs.config.message_id(message); //check that exactly config.gossip_lazy() many gossip messages were sent. assert_eq!( - count_control_msgs(&gs, |_, action| match action { + count_control_msgs(&gs, &queues, |_, action| match action { ControlAction::IHave { topic_hash, message_ids, @@ -2202,7 +2239,7 @@ fn test_accept_only_outbound_peer_grafts_when_mesh_full() { let config: Config = Config::default(); //enough peers to fill the mesh - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2217,8 +2254,8 @@ fn test_accept_only_outbound_peer_grafts_when_mesh_full() { assert_eq!(gs.mesh[&topics[0]].len(), config.mesh_n_high()); //create an outbound and an inbound peer - let inbound = add_peer(&mut gs, &topics, false, false); - let outbound = add_peer(&mut gs, &topics, true, false); + let (inbound, _in_reciver) = add_peer(&mut gs, &topics, false, false); + let (outbound, _out_receiver) = add_peer(&mut gs, &topics, true, false); //send grafts gs.handle_graft(&inbound, vec![topics[0].clone()]); @@ -2248,7 +2285,7 @@ fn test_do_not_remove_too_many_outbound_peers() { .unwrap(); //fill the mesh with inbound connections - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _receivers, topics) = inject_nodes1() .peer_no(n) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2263,7 +2300,7 @@ fn test_do_not_remove_too_many_outbound_peers() { //create m outbound connections and graft (we will accept the graft) let mut outbound = HashSet::new(); for _ in 0..m { - let peer = add_peer(&mut gs, &topics, true, false); + let (peer, _) = add_peer(&mut gs, &topics, true, false); outbound.insert(peer); gs.handle_graft(&peer, topics.clone()); } @@ -2286,7 +2323,7 @@ fn test_add_outbound_peers_if_min_is_not_satisfied() { let config: Config = Config::default(); // Fill full mesh with inbound peers - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2298,8 +2335,9 @@ fn test_add_outbound_peers_if_min_is_not_satisfied() { } //create config.mesh_outbound_min() many outbound connections without grafting + let mut peers = vec![]; for _ in 0..config.mesh_outbound_min() { - add_peer(&mut gs, &topics, true, false); + peers.push(add_peer(&mut gs, &topics, true, false)); } // Nothing changed in the mesh yet @@ -2320,7 +2358,7 @@ fn test_prune_negative_scored_peers() { let config = Config::default(); //build mesh with one peer - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, queues, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2344,7 +2382,7 @@ fn test_prune_negative_scored_peers() { //check prune message assert_eq!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[0] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] && match m { ControlAction::Prune { topic_hash, @@ -2365,7 +2403,7 @@ fn test_prune_negative_scored_peers() { fn test_dont_graft_to_negative_scored_peers() { let config = Config::default(); //init full mesh - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2377,8 +2415,8 @@ fn test_dont_graft_to_negative_scored_peers() { .create_network(); //add two additional peers that will not be part of the mesh - let p1 = add_peer(&mut gs, &topics, false, false); - let p2 = add_peer(&mut gs, &topics, false, false); + let (p1, _receiver1) = add_peer(&mut gs, &topics, false, false); + 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); @@ -2404,7 +2442,7 @@ fn test_ignore_px_from_negative_scored_peer() { let config = Config::default(); //build mesh with one peer - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2451,7 +2489,7 @@ fn test_only_send_nonnegative_scoring_peers_in_px() { .unwrap(); // Build mesh with three peer - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, queues, topics) = inject_nodes1() .peer_no(3) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2478,7 +2516,7 @@ fn test_only_send_nonnegative_scoring_peers_in_px() { // Check that px in prune message only contains third peer assert_eq!( - count_control_msgs(&gs, |peer_id, m| peer_id == &peers[1] + count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[1] && match m { ControlAction::Prune { topic_hash, @@ -2504,7 +2542,7 @@ fn test_do_not_gossip_to_peers_below_gossip_threshold() { }; // Build full mesh - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, queues, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2518,8 +2556,8 @@ fn test_do_not_gossip_to_peers_below_gossip_threshold() { } // Add two additional peers that will not be part of the mesh - let p1 = add_peer(&mut gs, &topics, false, false); - let p2 = add_peer(&mut gs, &topics, false, false); + let (p1, _receiver1) = add_peer(&mut gs, &topics, false, false); + let (p2, _receiver2) = add_peer(&mut gs, &topics, false, false); // Reduce score of p1 below peer_score_thresholds.gossip_threshold // note that penalties get squared so two penalties means a score of @@ -2551,7 +2589,7 @@ fn test_do_not_gossip_to_peers_below_gossip_threshold() { // Check that exactly one gossip messages got sent and it got sent to p2 assert_eq!( - count_control_msgs(&gs, |peer, action| match action { + count_control_msgs(&gs, &queues, |peer, action| match action { ControlAction::IHave { topic_hash, message_ids, @@ -2579,7 +2617,7 @@ fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { }; // Build full mesh - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, mut queues, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2595,8 +2633,10 @@ fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { } // Add two additional peers that will not be part of the mesh - let p1 = add_peer(&mut gs, &topics, false, false); - let p2 = add_peer(&mut gs, &topics, false, false); + let (p1, receiver1) = add_peer(&mut gs, &topics, false, false); + queues.insert(p1, receiver1); + let (p2, receiver2) = add_peer(&mut gs, &topics, false, false); + queues.insert(p2, receiver2); // Reduce score of p1 below peer_score_thresholds.gossip_threshold // note that penalties get squared so two penalties means a score of @@ -2627,17 +2667,15 @@ fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { gs.handle_iwant(&p2, vec![msg_id.clone()]); // the messages we are sending - let sent_messages = gs - .events + let sent_messages = queues .into_iter() - .fold(vec![], |mut collected_messages, e| match e { - ToSwarm::NotifyHandler { event, peer_id, .. } => { - if let HandlerIn::Message(RpcOut::Forward(message)) = event { + .fold(vec![], |mut collected_messages, (peer_id, c)| { + while !c.non_priority.is_empty() { + if let Ok(RpcOut::Forward(message)) = c.non_priority.try_recv() { collected_messages.push((peer_id, message)); } - collected_messages } - _ => collected_messages, + collected_messages }); //the message got sent to p2 @@ -2667,7 +2705,7 @@ fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { ..PeerScoreThresholds::default() }; //build full mesh - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, queues, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2683,8 +2721,8 @@ fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { } //add two additional peers that will not be part of the mesh - let p1 = add_peer(&mut gs, &topics, false, false); - let p2 = add_peer(&mut gs, &topics, false, false); + let (p1, _receiver1) = add_peer(&mut gs, &topics, false, false); + let (p2, _receiver2) = add_peer(&mut gs, &topics, false, false); //reduce score of p1 below peer_score_thresholds.gossip_threshold //note that penalties get squared so two penalties means a score of @@ -2715,7 +2753,7 @@ fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { // check that we sent exactly one IWANT request to p2 assert_eq!( - count_control_msgs(&gs, |peer, c| match c { + count_control_msgs(&gs, &queues, |peer, c| match c { ControlAction::IWant { message_ids } => if message_ids.iter().any(|m| m == &msg_id) { assert_eq!(peer, &p2); @@ -2743,7 +2781,7 @@ fn test_do_not_publish_to_peer_below_publish_threshold() { }; //build mesh with no peers and no subscribed topics - let (mut gs, _, _) = inject_nodes1() + let (mut gs, _, mut queues, _) = inject_nodes1() .gs_config(config) .scoring(Some((peer_score_params, peer_score_thresholds))) .create_network(); @@ -2753,8 +2791,10 @@ fn test_do_not_publish_to_peer_below_publish_threshold() { let topics = vec![topic.hash()]; //add two additional peers that will be added to the mesh - let p1 = add_peer(&mut gs, &topics, false, false); - let p2 = add_peer(&mut gs, &topics, false, false); + let (p1, receiver1) = add_peer(&mut gs, &topics, false, false); + queues.insert(p1, receiver1); + let (p2, receiver2) = add_peer(&mut gs, &topics, false, false); + queues.insert(p2, receiver2); //reduce score of p1 below peer_score_thresholds.publish_threshold //note that penalties get squared so two penalties means a score of @@ -2772,17 +2812,15 @@ fn test_do_not_publish_to_peer_below_publish_threshold() { gs.publish(topic, publish_data).unwrap(); // Collect all publish messages - let publishes = gs - .events + let publishes = queues .into_iter() - .fold(vec![], |mut collected_publish, e| match e { - ToSwarm::NotifyHandler { event, peer_id, .. } => { - if let HandlerIn::Message(RpcOut::Publish(message)) = event { + .fold(vec![], |mut collected_publish, (peer_id, c)| { + while !c.priority.is_empty() { + if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { collected_publish.push((peer_id, message)); } - collected_publish } - _ => collected_publish, + collected_publish }); //assert only published to p2 @@ -2800,15 +2838,17 @@ fn test_do_not_flood_publish_to_peer_below_publish_threshold() { ..PeerScoreThresholds::default() }; //build mesh with no peers - let (mut gs, _, topics) = inject_nodes1() + let (mut gs, _, mut queues, topics) = inject_nodes1() .topics(vec!["test".into()]) .gs_config(config) .scoring(Some((peer_score_params, peer_score_thresholds))) .create_network(); //add two additional peers that will be added to the mesh - let p1 = add_peer(&mut gs, &topics, false, false); - let p2 = add_peer(&mut gs, &topics, false, false); + let (p1, receiver1) = add_peer(&mut gs, &topics, false, false); + queues.insert(p1, receiver1); + let (p2, receiver2) = add_peer(&mut gs, &topics, false, false); + queues.insert(p2, receiver2); //reduce score of p1 below peer_score_thresholds.publish_threshold //note that penalties get squared so two penalties means a score of @@ -2826,17 +2866,15 @@ fn test_do_not_flood_publish_to_peer_below_publish_threshold() { gs.publish(Topic::new("test"), publish_data).unwrap(); // Collect all publish messages - let publishes = gs - .events + let publishes = queues .into_iter() - .fold(vec![], |mut collected_publish, e| match e { - ToSwarm::NotifyHandler { event, peer_id, .. } => { - if let HandlerIn::Message(RpcOut::Publish(message)) = event { - collected_publish.push((peer_id, message)); + .fold(vec![], |mut collected_publish, (peer_id, c)| { + while !c.priority.is_empty() { + if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { + collected_publish.push((peer_id, message)) } - collected_publish } - _ => collected_publish, + collected_publish }); //assert only published to p2 @@ -2856,15 +2894,15 @@ fn test_ignore_rpc_from_peers_below_graylist_threshold() { }; //build mesh with no peers - let (mut gs, _, topics) = inject_nodes1() + let (mut gs, _, _, topics) = inject_nodes1() .topics(vec!["test".into()]) .gs_config(config.clone()) .scoring(Some((peer_score_params, peer_score_thresholds))) .create_network(); //add two additional peers that will be added to the mesh - let p1 = add_peer(&mut gs, &topics, false, false); - let p2 = add_peer(&mut gs, &topics, false, false); + let (p1, _receiver1) = add_peer(&mut gs, &topics, false, false); + let (p2, _receiver2) = add_peer(&mut gs, &topics, false, false); //reduce score of p1 below peer_score_thresholds.graylist_threshold //note that penalties get squared so two penalties means a score of @@ -2986,7 +3024,7 @@ fn test_ignore_px_from_peers_below_accept_px_threshold() { ..PeerScoreThresholds::default() }; // Build mesh with two peers - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(2) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3057,7 +3095,7 @@ fn test_keep_best_scoring_peers_on_oversubscription() { //build mesh with more peers than mesh can hold let n = config.mesh_n_high() + 1; - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _receivers, topics) = inject_nodes1() .peer_no(n) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3117,7 +3155,7 @@ fn test_scoring_p1() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with one peer - let (mut gs, peers, _) = inject_nodes1() + let (mut gs, peers, _, _) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3199,7 +3237,7 @@ fn test_scoring_p2() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with one peer - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(2) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3299,7 +3337,7 @@ fn test_scoring_p3() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with two peers - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(2) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3400,7 +3438,7 @@ fn test_scoring_p3b() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with one peer - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3492,7 +3530,7 @@ fn test_scoring_p4_valid_message() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with two peers - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3551,7 +3589,7 @@ fn test_scoring_p4_invalid_signature() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with one peer - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3609,7 +3647,7 @@ fn test_scoring_p4_message_from_self() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with two peers - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3659,7 +3697,7 @@ fn test_scoring_p4_ignored_message() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with two peers - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3718,7 +3756,7 @@ fn test_scoring_p4_application_invalidated_message() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with two peers - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3780,7 +3818,7 @@ fn test_scoring_p4_application_invalid_message_from_two_peers() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with two peers - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(2) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3850,7 +3888,7 @@ fn test_scoring_p4_three_application_invalid_messages() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with one peer - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3934,7 +3972,7 @@ fn test_scoring_p4_decay() { let peer_score_thresholds = PeerScoreThresholds::default(); //build mesh with one peer - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -3988,7 +4026,7 @@ fn test_scoring_p5() { }; //build mesh with one peer - let (mut gs, peers, _) = inject_nodes1() + let (mut gs, peers, _, _) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -4014,7 +4052,7 @@ fn test_scoring_p6() { ..Default::default() }; - let (mut gs, _, _) = inject_nodes1() + let (mut gs, _, _, _) = inject_nodes1() .peer_no(0) .topics(vec![]) .to_subscribe(false) @@ -4027,20 +4065,20 @@ fn test_scoring_p6() { //create 5 peers with the same ip let addr = Multiaddr::from(Ipv4Addr::new(10, 1, 2, 3)); let peers = vec![ - add_peer_with_addr(&mut gs, &vec![], false, false, addr.clone()), - add_peer_with_addr(&mut gs, &vec![], false, false, addr.clone()), - add_peer_with_addr(&mut gs, &vec![], true, false, addr.clone()), - add_peer_with_addr(&mut gs, &vec![], true, false, addr.clone()), - add_peer_with_addr(&mut gs, &vec![], true, true, addr.clone()), + add_peer_with_addr(&mut gs, &vec![], false, false, addr.clone()).0, + add_peer_with_addr(&mut gs, &vec![], false, false, addr.clone()).0, + add_peer_with_addr(&mut gs, &vec![], true, false, addr.clone()).0, + add_peer_with_addr(&mut gs, &vec![], true, false, addr.clone()).0, + add_peer_with_addr(&mut gs, &vec![], true, true, addr.clone()).0, ]; //create 4 other peers with other ip let addr2 = Multiaddr::from(Ipv4Addr::new(10, 1, 2, 4)); let others = vec![ - add_peer_with_addr(&mut gs, &vec![], false, false, addr2.clone()), - add_peer_with_addr(&mut gs, &vec![], false, false, addr2.clone()), - add_peer_with_addr(&mut gs, &vec![], true, false, addr2.clone()), - add_peer_with_addr(&mut gs, &vec![], true, false, addr2.clone()), + add_peer_with_addr(&mut gs, &vec![], false, false, addr2.clone()).0, + add_peer_with_addr(&mut gs, &vec![], false, false, addr2.clone()).0, + add_peer_with_addr(&mut gs, &vec![], true, false, addr2.clone()).0, + add_peer_with_addr(&mut gs, &vec![], true, false, addr2.clone()).0, ]; //no penalties yet @@ -4144,7 +4182,7 @@ fn test_scoring_p7_grafts_before_backoff() { ..Default::default() }; - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _receivers, topics) = inject_nodes1() .peer_no(2) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4221,7 +4259,7 @@ fn test_opportunistic_grafting() { ..Default::default() }; - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _receivers, topics) = inject_nodes1() .peer_no(5) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4250,7 +4288,7 @@ fn test_opportunistic_grafting() { } //set scores for peers in the mesh - for (i, peer) in others.iter().enumerate().take(5) { + for (i, (peer, _receiver)) in others.iter().enumerate().take(5) { gs.set_application_score(peer, 0.0 + i as f64); } @@ -4290,7 +4328,7 @@ fn test_opportunistic_grafting() { ); assert!( - gs.mesh[&topics[0]].is_disjoint(&others.iter().cloned().take(2).collect()), + gs.mesh[&topics[0]].is_disjoint(&others.iter().map(|(p, _)| p).cloned().take(2).collect()), "peers below or equal to median should not be added in opportunistic grafting" ); } @@ -4298,7 +4336,7 @@ fn test_opportunistic_grafting() { #[test] fn test_ignore_graft_from_unknown_topic() { //build gossipsub without subscribing to any topics - let (mut gs, _, _) = inject_nodes1() + let (mut gs, _, queues, _) = inject_nodes1() .peer_no(0) .topics(vec![]) .to_subscribe(false) @@ -4309,7 +4347,10 @@ fn test_ignore_graft_from_unknown_topic() { //assert that no prune got created assert_eq!( - count_control_msgs(&gs, |_, a| matches!(a, ControlAction::Prune { .. })), + count_control_msgs(&gs, &queues, |_, a| matches!( + a, + ControlAction::Prune { .. } + )), 0, "we should not prune after graft in unknown topic" ); @@ -4319,14 +4360,15 @@ fn test_ignore_graft_from_unknown_topic() { fn test_ignore_too_many_iwants_from_same_peer_for_same_message() { let config = Config::default(); //build gossipsub with full mesh - let (mut gs, _, topics) = inject_nodes1() + let (mut gs, _, mut queues, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(false) .create_network(); //add another peer not in the mesh - let peer = add_peer(&mut gs, &topics, false, false); + let (peer, receiver) = add_peer(&mut gs, &topics, false, false); + queues.insert(peer, receiver); //receive a message let mut seq = 0; @@ -4340,7 +4382,7 @@ fn test_ignore_too_many_iwants_from_same_peer_for_same_message() { gs.handle_received_message(m1, &PeerId::random()); //clear events - gs.events.clear(); + flush_events(&mut gs, &mut queues); //the first gossip_retransimission many iwants return the valid message, all others are // ignored. @@ -4349,16 +4391,14 @@ fn test_ignore_too_many_iwants_from_same_peer_for_same_message() { } assert_eq!( - gs.events - .iter() - .filter(|e| matches!( - e, - ToSwarm::NotifyHandler { - event: HandlerIn::Message(RpcOut::Forward(_)), - .. + queues.into_values().fold(0, |mut fwds, c| { + while !c.non_priority.is_empty() { + if let Ok(RpcOut::Forward(_)) = dbg!(c.non_priority.try_recv()) { + fwds = fwds + 1; } - )) - .count(), + } + fwds + }), config.gossip_retransimission() as usize, "not more then gossip_retransmission many messages get sent back" ); @@ -4371,7 +4411,7 @@ fn test_ignore_too_many_ihaves() { .build() .unwrap(); //build gossipsub with full mesh - let (mut gs, _, topics) = inject_nodes1() + let (mut gs, _, mut queues, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4379,7 +4419,8 @@ fn test_ignore_too_many_ihaves() { .create_network(); //add another peer not in the mesh - let peer = add_peer(&mut gs, &topics, false, false); + let (peer, receiver) = add_peer(&mut gs, &topics, false, false); + queues.insert(peer, receiver); //peer has 20 messages let mut seq = 0; @@ -4408,7 +4449,7 @@ fn test_ignore_too_many_ihaves() { //we send iwant only for the first 10 messages assert_eq!( - count_control_msgs(&gs, |p, action| p == &peer + count_control_msgs(&gs, &queues, |p, action| p == &peer && matches!(action, ControlAction::IWant { message_ids } if message_ids.len() == 1 && first_ten.contains(&message_ids[0]))), 10, "exactly the first ten ihaves should be processed and one iwant for each created" @@ -4431,7 +4472,7 @@ fn test_ignore_too_many_ihaves() { //we sent iwant for all 20 messages assert_eq!( - count_control_msgs(&gs, |p, action| p == &peer + count_control_msgs(&gs, &queues, |p, action| p == &peer && matches!(action, ControlAction::IWant { message_ids } if message_ids.len() == 1)), 20, "all 20 should get sent" @@ -4446,7 +4487,7 @@ fn test_ignore_too_many_messages_in_ihave() { .build() .unwrap(); //build gossipsub with full mesh - let (mut gs, _, topics) = inject_nodes1() + let (mut gs, _, mut queues, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4454,7 +4495,8 @@ fn test_ignore_too_many_messages_in_ihave() { .create_network(); //add another peer not in the mesh - let peer = add_peer(&mut gs, &topics, false, false); + let (peer, receiver) = add_peer(&mut gs, &topics, false, false); + queues.insert(peer, receiver); //peer has 20 messages let mut seq = 0; @@ -4480,7 +4522,7 @@ fn test_ignore_too_many_messages_in_ihave() { //we send iwant only for the first 10 messages let mut sum = 0; assert_eq!( - count_control_msgs(&gs, |p, action| match action { + count_control_msgs(&gs, &queues, |p, action| match action { ControlAction::IWant { message_ids } => p == &peer && { assert!(first_twelve.is_superset(&message_ids.iter().collect())); @@ -4505,7 +4547,7 @@ fn test_ignore_too_many_messages_in_ihave() { //we sent 20 iwant messages let mut sum = 0; assert_eq!( - count_control_msgs(&gs, |p, action| match action { + count_control_msgs(&gs, &queues, |p, action| match action { ControlAction::IWant { message_ids } => p == &peer && { sum += message_ids.len(); @@ -4526,7 +4568,7 @@ fn test_limit_number_of_message_ids_inside_ihave() { .build() .unwrap(); //build gossipsub with full mesh - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, queues, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4539,8 +4581,8 @@ fn test_limit_number_of_message_ids_inside_ihave() { } //add two other peers not in the mesh - let p1 = add_peer(&mut gs, &topics, false, false); - let p2 = add_peer(&mut gs, &topics, false, false); + let (p1, _) = add_peer(&mut gs, &topics, false, false); + let (p2, _) = add_peer(&mut gs, &topics, false, false); //receive 200 messages from another peer let mut seq = 0; @@ -4559,7 +4601,7 @@ fn test_limit_number_of_message_ids_inside_ihave() { let mut ihaves2 = HashSet::new(); assert_eq!( - count_control_msgs(&gs, |p, action| match action { + count_control_msgs(&gs, &queues, |p, action| match action { ControlAction::IHave { message_ids, .. } => { if p == &p1 { ihaves1 = message_ids.iter().cloned().collect(); @@ -4616,7 +4658,7 @@ fn test_iwant_penalties() { }; // fill the mesh - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, _, topics) = inject_nodes1() .peer_no(2) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4640,7 +4682,7 @@ fn test_iwant_penalties() { let mut first_messages = Vec::new(); let mut second_messages = Vec::new(); let mut seq = 0; - for peer in &other_peers { + for (peer, _receiver) in &other_peers { let msg1 = random_message(&mut seq, &topics); let msg2 = random_message(&mut seq, &topics); @@ -4663,19 +4705,19 @@ fn test_iwant_penalties() { } // the peers send us all the first message ids in time - for (index, peer) in other_peers.iter().enumerate() { + for (index, (peer, _receiver)) in other_peers.iter().enumerate() { gs.handle_received_message(first_messages[index].clone(), peer); } // now we do a heartbeat no penalization should have been applied yet gs.heartbeat(); - for peer in &other_peers { + for (peer, _receiver) in &other_peers { assert_eq!(gs.peer_score.as_ref().unwrap().0.score(peer), 0.0); } // receive the first twenty of the other peers then send their response - for (index, peer) in other_peers.iter().enumerate().take(20) { + for (index, (peer, _receiver)) in other_peers.iter().enumerate().take(20) { gs.handle_received_message(second_messages[index].clone(), peer); } @@ -4686,7 +4728,7 @@ fn test_iwant_penalties() { gs.heartbeat(); // now we get the second messages from the last 80 peers. - for (index, peer) in other_peers.iter().enumerate() { + for (index, (peer, _receiver)) in other_peers.iter().enumerate() { if index > 19 { gs.handle_received_message(second_messages[index].clone(), peer); } @@ -4700,7 +4742,7 @@ fn test_iwant_penalties() { let mut single_penalized = 0; let mut double_penalized = 0; - for (i, peer) in other_peers.iter().enumerate() { + for (i, (peer, _receiver)) in other_peers.iter().enumerate() { let score = gs.peer_score.as_ref().unwrap().0.score(peer); if score == 0.0 { not_penalized += 1; @@ -4728,7 +4770,7 @@ fn test_publish_to_floodsub_peers_without_flood_publish() { .flood_publish(false) .build() .unwrap(); - let (mut gs, _, topics) = inject_nodes1() + let (mut gs, _, mut queues, topics) = inject_nodes1() .peer_no(config.mesh_n_low() - 1) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4736,7 +4778,7 @@ fn test_publish_to_floodsub_peers_without_flood_publish() { .create_network(); //add two floodsub peer, one explicit, one implicit - let p1 = add_peer_with_addr_and_kind( + let (p1, receiver1) = add_peer_with_addr_and_kind( &mut gs, &topics, false, @@ -4744,7 +4786,11 @@ fn test_publish_to_floodsub_peers_without_flood_publish() { Multiaddr::empty(), Some(PeerKind::Floodsub), ); - let p2 = add_peer_with_addr_and_kind(&mut gs, &topics, false, false, Multiaddr::empty(), None); + queues.insert(p1, receiver1); + + let (p2, receiver2) = + add_peer_with_addr_and_kind(&mut gs, &topics, false, false, Multiaddr::empty(), None); + queues.insert(p2, receiver2); //p1 and p2 are not in the mesh assert!(!gs.mesh[&topics[0]].contains(&p1) && !gs.mesh[&topics[0]].contains(&p2)); @@ -4754,24 +4800,21 @@ fn test_publish_to_floodsub_peers_without_flood_publish() { gs.publish(Topic::new("test"), publish_data).unwrap(); // Collect publish messages to floodsub peers - let publishes = gs - .events + let publishes = queues .iter() - .fold(vec![], |mut collected_publish, e| match e { - ToSwarm::NotifyHandler { peer_id, event, .. } => { - if peer_id == &p1 || peer_id == &p2 { - if let HandlerIn::Message(RpcOut::Publish(message)) = event { - collected_publish.push(message); - } + .fold(0, |mut collected_publish, (peer_id, c)| { + while !c.priority.is_empty() { + if matches!(c.priority.try_recv(), + Ok(RpcOut::Publish(_)) if peer_id == &p1 || peer_id == &p2) + { + collected_publish = collected_publish + 1; } - collected_publish } - _ => collected_publish, + collected_publish }); assert_eq!( - publishes.len(), - 2, + publishes, 2, "Should send a publish message to all floodsub peers" ); } @@ -4782,7 +4825,7 @@ fn test_do_not_use_floodsub_in_fanout() { .flood_publish(false) .build() .unwrap(); - let (mut gs, _, _) = inject_nodes1() + let (mut gs, _, mut queues, _) = inject_nodes1() .peer_no(config.mesh_n_low() - 1) .topics(Vec::new()) .to_subscribe(false) @@ -4793,7 +4836,7 @@ fn test_do_not_use_floodsub_in_fanout() { let topics = vec![topic.hash()]; //add two floodsub peer, one explicit, one implicit - let p1 = add_peer_with_addr_and_kind( + let (p1, receiver1) = add_peer_with_addr_and_kind( &mut gs, &topics, false, @@ -4801,31 +4844,32 @@ fn test_do_not_use_floodsub_in_fanout() { Multiaddr::empty(), Some(PeerKind::Floodsub), ); - let p2 = add_peer_with_addr_and_kind(&mut gs, &topics, false, false, Multiaddr::empty(), None); + queues.insert(p1, receiver1); + let (p2, receiver2) = + add_peer_with_addr_and_kind(&mut gs, &topics, false, false, Multiaddr::empty(), None); + + queues.insert(p2, receiver2); //publish a message let publish_data = vec![0; 42]; gs.publish(Topic::new("test"), publish_data).unwrap(); // Collect publish messages to floodsub peers - let publishes = gs - .events + let publishes = queues .iter() - .fold(vec![], |mut collected_publish, e| match e { - ToSwarm::NotifyHandler { peer_id, event, .. } => { - if peer_id == &p1 || peer_id == &p2 { - if let HandlerIn::Message(RpcOut::Publish(message)) = event { - collected_publish.push(message); - } + .fold(0, |mut collected_publish, (peer_id, c)| { + while !c.priority.is_empty() { + if matches!(c.priority.try_recv(), + Ok(RpcOut::Publish(_)) if peer_id == &p1 || peer_id == &p2) + { + collected_publish = collected_publish + 1; } - collected_publish } - _ => collected_publish, + collected_publish }); assert_eq!( - publishes.len(), - 2, + publishes, 2, "Should send a publish message to all floodsub peers" ); @@ -4837,7 +4881,7 @@ fn test_do_not_use_floodsub_in_fanout() { #[test] fn test_dont_add_floodsub_peers_to_mesh_on_join() { - let (mut gs, _, _) = inject_nodes1() + let (mut gs, _, _, _) = inject_nodes1() .peer_no(0) .topics(Vec::new()) .to_subscribe(false) @@ -4867,14 +4911,14 @@ fn test_dont_add_floodsub_peers_to_mesh_on_join() { #[test] fn test_dont_send_px_to_old_gossipsub_peers() { - let (mut gs, _, topics) = inject_nodes1() + let (mut gs, _, queues, topics) = inject_nodes1() .peer_no(0) .topics(vec!["test".into()]) .to_subscribe(false) .create_network(); //add an old gossipsub peer - let p1 = add_peer_with_addr_and_kind( + let (p1, _receiver1) = add_peer_with_addr_and_kind( &mut gs, &topics, false, @@ -4892,7 +4936,7 @@ fn test_dont_send_px_to_old_gossipsub_peers() { //check that prune does not contain px assert_eq!( - count_control_msgs(&gs, |_, m| match m { + count_control_msgs(&gs, &queues, |_, m| match m { ControlAction::Prune { peers: px, .. } => !px.is_empty(), _ => false, }), @@ -4904,7 +4948,7 @@ fn test_dont_send_px_to_old_gossipsub_peers() { #[test] fn test_dont_send_floodsub_peers_in_px() { //build mesh with one peer - let (mut gs, peers, topics) = inject_nodes1() + let (mut gs, peers, queues, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -4930,7 +4974,7 @@ fn test_dont_send_floodsub_peers_in_px() { //check that px in prune message is empty assert_eq!( - count_control_msgs(&gs, |_, m| match m { + count_control_msgs(&gs, &queues, |_, m| match m { ControlAction::Prune { peers: px, .. } => !px.is_empty(), _ => false, }), @@ -4941,7 +4985,7 @@ fn test_dont_send_floodsub_peers_in_px() { #[test] fn test_dont_add_floodsub_peers_to_mesh_in_heartbeat() { - let (mut gs, _, topics) = inject_nodes1() + let (mut gs, _, _, topics) = inject_nodes1() .peer_no(0) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4969,7 +5013,7 @@ fn test_dont_add_floodsub_peers_to_mesh_in_heartbeat() { // Some very basic test of public api methods. #[test] fn test_public_api() { - let (gs, peers, topic_hashes) = inject_nodes1() + let (gs, peers, _, topic_hashes) = inject_nodes1() .peer_no(4) .topics(vec![String::from("topic1")]) .to_subscribe(true) @@ -5001,7 +5045,7 @@ fn test_public_api() { fn test_subscribe_to_invalid_topic() { let t1 = Topic::new("t1"); let t2 = Topic::new("t2"); - let (mut gs, _, _) = inject_nodes::() + let (mut gs, _, _, _) = inject_nodes::() .subscription_filter(WhitelistSubscriptionFilter( vec![t1.hash()].into_iter().collect(), )) @@ -5015,7 +5059,7 @@ fn test_subscribe_to_invalid_topic() { #[test] fn test_subscribe_and_graft_with_negative_score() { //simulate a communication between two gossipsub instances - let (mut gs1, _, topic_hashes) = inject_nodes1() + let (mut gs1, _, _, topic_hashes) = inject_nodes1() .topics(vec!["test".into()]) .scoring(Some(( PeerScoreParams::default(), @@ -5023,14 +5067,14 @@ fn test_subscribe_and_graft_with_negative_score() { ))) .create_network(); - let (mut gs2, _, _) = inject_nodes1().create_network(); + let (mut gs2, _, queues, _) = inject_nodes1().create_network(); let connection_id = ConnectionId::new_unchecked(0); let topic = Topic::new("test"); - let p2 = add_peer(&mut gs1, &Vec::new(), true, false); - let p1 = add_peer(&mut gs2, &topic_hashes, false, false); + let (p2, _receiver1) = add_peer(&mut gs1, &Vec::new(), true, false); + 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); @@ -5040,22 +5084,16 @@ fn test_subscribe_and_graft_with_negative_score() { //subscribe to topic in gs2 gs2.subscribe(&topic).unwrap(); - let forward_messages_to_p1 = |gs1: &mut Behaviour<_, _>, gs2: &mut Behaviour<_, _>| { + let forward_messages_to_p1 = |gs1: &mut Behaviour<_, _>, _gs2: &mut Behaviour<_, _>| { //collect messages to p1 - let messages_to_p1 = gs2.events.drain(..).filter_map(|e| match e { - ToSwarm::NotifyHandler { peer_id, event, .. } => { - if peer_id == p1 { - if let HandlerIn::Message(m) = event { - Some(m) - } else { - None - } - } else { - None - } - } - _ => None, - }); + let messages_to_p1 = + queues + .iter() + .filter_map(|(peer_id, c)| match c.non_priority.try_recv() { + Ok(rpc) if peer_id == &p1 => Some(rpc), + _ => None, + }); + for message in messages_to_p1 { gs1.on_connection_handler_event( p2, @@ -5093,7 +5131,7 @@ fn test_graft_without_subscribe() { let topic = String::from("test_subscribe"); let subscribe_topic = vec![topic.clone()]; let subscribe_topic_hash = vec![Topic::new(topic.clone()).hash()]; - let (mut gs, peers, topic_hashes) = inject_nodes1() + let (mut gs, peers, _, topic_hashes) = inject_nodes1() .peer_no(1) .topics(subscribe_topic) .to_subscribe(false) diff --git a/protocols/gossipsub/src/config.rs b/protocols/gossipsub/src/config.rs index 7e79912cc4a..b99f8548067 100644 --- a/protocols/gossipsub/src/config.rs +++ b/protocols/gossipsub/src/config.rs @@ -95,6 +95,7 @@ pub struct Config { max_ihave_messages: usize, iwant_followup_time: Duration, published_message_ids_cache_time: Duration, + connection_handler_queue_len: usize, } impl Config { @@ -350,6 +351,11 @@ impl Config { pub fn published_message_ids_cache_time(&self) -> Duration { self.published_message_ids_cache_time } + + /// The max number of messages a `ConnectionHandler` can buffer. + pub fn connection_handler_queue_len(&self) -> usize { + self.connection_handler_queue_len + } } impl Default for Config { @@ -417,6 +423,7 @@ impl Default for ConfigBuilder { max_ihave_messages: 10, iwant_followup_time: Duration::from_secs(3), published_message_ids_cache_time: Duration::from_secs(10), + connection_handler_queue_len: 100, }, invalid_protocol: false, } @@ -782,6 +789,10 @@ impl ConfigBuilder { self } + pub fn connection_handler_queue_len(&mut self, len: usize) { + self.config.connection_handler_queue_len = len; + } + /// Constructs a [`Config`] from the given configuration and validates the settings. pub fn build(&self) -> Result { // check all constraints on config diff --git a/protocols/gossipsub/src/handler.rs b/protocols/gossipsub/src/handler.rs index e91f81776e7..dd048c7e2fd 100644 --- a/protocols/gossipsub/src/handler.rs +++ b/protocols/gossipsub/src/handler.rs @@ -20,7 +20,7 @@ use crate::protocol::{GossipsubCodec, ProtocolConfig}; use crate::rpc_proto::proto; -use crate::types::{PeerKind, RawMessage, Rpc, RpcOut}; +use crate::types::{PeerKind, RawMessage, Rpc, RpcReceiver}; use crate::ValidationError; use asynchronous_codec::Framed; use futures::future::Either; @@ -33,7 +33,6 @@ use libp2p_swarm::handler::{ FullyNegotiatedInbound, FullyNegotiatedOutbound, StreamUpgradeError, SubstreamProtocol, }; use libp2p_swarm::Stream; -use smallvec::SmallVec; use std::{ pin::Pin, task::{Context, Poll}, @@ -61,8 +60,6 @@ pub enum HandlerEvent { #[allow(clippy::large_enum_variant)] #[derive(Debug)] pub enum HandlerIn { - /// A gossipsub message to send. - Message(RpcOut), /// The peer has joined the mesh. JoinedMesh, /// The peer has left the mesh. @@ -94,8 +91,8 @@ pub struct EnabledHandler { /// The single long-lived inbound substream. inbound_substream: Option, - /// Queue of values that we want to send to the remote. - send_queue: SmallVec<[proto::RPC; 16]>, + /// Queue of values that we want to send to the remote + send_queue: RpcReceiver, /// Flag indicating that an outbound substream is being established to prevent duplicate /// requests. @@ -159,7 +156,7 @@ enum OutboundSubstreamState { impl Handler { /// Builds a new [`Handler`]. - pub fn new(protocol_config: ProtocolConfig) -> Self { + pub fn new(protocol_config: ProtocolConfig, message_queue: RpcReceiver) -> Self { Handler::Enabled(EnabledHandler { listen_protocol: protocol_config, inbound_substream: None, @@ -167,11 +164,11 @@ impl Handler { outbound_substream_establishing: false, outbound_substream_attempts: 0, inbound_substream_attempts: 0, - send_queue: SmallVec::new(), peer_kind: None, peer_kind_sent: false, last_io_activity: Instant::now(), in_mesh: false, + send_queue: message_queue, }) } } @@ -250,10 +247,11 @@ impl EnabledHandler { ) { // outbound idle state Some(OutboundSubstreamState::WaitingOutput(substream)) => { - if let Some(message) = self.send_queue.pop() { - self.send_queue.shrink_to_fit(); - self.outbound_substream = - Some(OutboundSubstreamState::PendingSend(substream, message)); + if let Poll::Ready(Some(message)) = self.send_queue.poll_next_unpin(cx) { + self.outbound_substream = Some(OutboundSubstreamState::PendingSend( + substream, + message.into_protobuf(), + )); continue; } @@ -409,7 +407,6 @@ impl ConnectionHandler for Handler { fn on_behaviour_event(&mut self, message: HandlerIn) { match self { Handler::Enabled(handler) => match message { - HandlerIn::Message(m) => handler.send_queue.push(m.into_protobuf()), HandlerIn::JoinedMesh => { handler.in_mesh = true; } diff --git a/protocols/gossipsub/src/types.rs b/protocols/gossipsub/src/types.rs index d1b92ff0ba8..a1d9ff3fae2 100644 --- a/protocols/gossipsub/src/types.rs +++ b/protocols/gossipsub/src/types.rs @@ -20,12 +20,17 @@ //! A collection of types using the Gossipsub system. use crate::TopicHash; +use async_channel::{Receiver, Sender}; +use futures::Stream; use libp2p_identity::PeerId; use libp2p_swarm::ConnectionId; use prometheus_client::encoding::EncodeLabelValue; use quick_protobuf::MessageWrite; -use std::fmt; use std::fmt::Debug; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::Arc; +use std::task::Poll; +use std::{fmt, pin::Pin}; use crate::rpc_proto::proto; #[cfg(feature = "serde")] @@ -512,3 +517,99 @@ impl fmt::Display for PeerKind { f.write_str(self.as_ref()) } } + +/// Create `RpcOut` channel that is priority aware. +pub(crate) fn rpc_channel(cap: usize) -> (RpcSender, RpcReceiver) { + let (priority_sender, priority_receiver) = async_channel::unbounded(); + let (non_priority_sender, non_priority_receiver) = async_channel::bounded(cap / 2); + let len = Arc::new(AtomicUsize::new(0)); + ( + RpcSender { + cap: cap / 2, + len: len.clone(), + priority: priority_sender, + non_priority: non_priority_sender, + }, + RpcReceiver { + len, + priority: priority_receiver, + non_priority: non_priority_receiver, + }, + ) +} + +/// `RpcOut` sender that is priority aware. +pub(crate) struct RpcSender { + cap: usize, + len: Arc, + priority: Sender, + non_priority: Sender, +} + +impl RpcSender { + /// Send `RpcOut`s to the `ConnectionHandler` according to their priority. + pub(crate) fn try_send(&mut self, rpc: RpcOut) -> Result<(), ()> { + // Forward messages, IWANT and IHAVE control messages are regarded as low priority. + match rpc { + rpc @ RpcOut::Forward(_) + | rpc @ RpcOut::Control(ControlAction::IHave { .. }) + | rpc @ RpcOut::Control(ControlAction::IWant { .. }) => { + if let Err(err) = self.non_priority.try_send(rpc) { + let rpc = err.into_inner(); + tracing::trace!("{rpc:?} message dropped, queue is full"); + } + } + // GRAFT and PRUNE control messages, Subscription, and Publishes messages. + // Publish messages are limited to the capacity of the queue. + rpc @ RpcOut::Control(_) + | rpc @ RpcOut::Subscribe(_) + | rpc @ RpcOut::Unsubscribe(_) => { + self.priority.try_send(rpc).expect("Channel is unbounded"); + } + rpc @ RpcOut::Publish(_) => { + if self.len.load(Ordering::Relaxed) >= self.cap { + return Err(()); + } + self.priority.try_send(rpc).expect("Channel is unbounded"); + self.len.fetch_add(1, Ordering::Relaxed); + } + } + Ok(()) + } +} + +/// `RpcOut` sender that is priority aware. +pub struct RpcReceiver { + len: Arc, + pub(crate) priority: Receiver, + pub(crate) non_priority: Receiver, +} + +impl RpcReceiver { + /// Check if both queues are empty. + pub(crate) fn is_empty(&self) -> bool { + self.priority.is_empty() && self.non_priority.is_empty() + } +} + +impl Stream for RpcReceiver { + type Item = RpcOut; + + fn poll_next( + mut self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + // The control queue is polled first. + if let Poll::Ready(rpc) = Pin::new(&mut self.priority).poll_next(cx) { + if let Some(RpcOut::Publish(_)) = rpc { + self.len.fetch_sub(1, Ordering::Relaxed); + } + return Poll::Ready(rpc); + } + // The priority queue is then polled. + if let Poll::Ready(rpc) = Pin::new(&mut self.priority).poll_next(cx) { + return Poll::Ready(rpc); + } + Pin::new(&mut self.non_priority).poll_next(cx) + } +} From 76fb737906597469dcfc87dc793af1a6337da473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Thu, 23 Nov 2023 15:19:01 +0000 Subject: [PATCH 02/11] remove duplicated poll from RpcReceiver --- protocols/gossipsub/src/types.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/protocols/gossipsub/src/types.rs b/protocols/gossipsub/src/types.rs index a1d9ff3fae2..e8318401d51 100644 --- a/protocols/gossipsub/src/types.rs +++ b/protocols/gossipsub/src/types.rs @@ -599,17 +599,14 @@ impl Stream for RpcReceiver { mut self: std::pin::Pin<&mut Self>, cx: &mut std::task::Context<'_>, ) -> std::task::Poll> { - // The control queue is polled first. + // The priority queue is first polled. if let Poll::Ready(rpc) = Pin::new(&mut self.priority).poll_next(cx) { if let Some(RpcOut::Publish(_)) = rpc { self.len.fetch_sub(1, Ordering::Relaxed); } return Poll::Ready(rpc); } - // The priority queue is then polled. - if let Poll::Ready(rpc) = Pin::new(&mut self.priority).poll_next(cx) { - return Poll::Ready(rpc); - } + // Then we poll the non priority. Pin::new(&mut self.non_priority).poll_next(cx) } } From dd13fcd6f8a0980996c2ea9c947df402726954c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Sat, 25 Nov 2023 22:24:03 +0000 Subject: [PATCH 03/11] make RpcReceiver part of RpcSender, and clone it per ConnectionHandler, this will allow us to always have an open Receiver. --- protocols/gossipsub/src/behaviour.rs | 27 +++++++----- protocols/gossipsub/src/behaviour/tests.rs | 5 ++- protocols/gossipsub/src/types.rs | 49 +++++++++++++--------- 3 files changed, 49 insertions(+), 32 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index af8abd0725f..523687e3852 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -45,6 +45,7 @@ use libp2p_swarm::{ THandlerOutEvent, ToSwarm, }; +use crate::config::{Config, ValidationMode}; use crate::gossip_promises::GossipPromises; use crate::handler::{Handler, HandlerEvent, HandlerIn}; use crate::mcache::MessageCache; @@ -61,10 +62,6 @@ use crate::types::{ }; use crate::types::{PeerConnections, PeerKind, RpcOut}; use crate::{backoff::BackoffStorage, types::RpcSender}; -use crate::{ - config::{Config, ValidationMode}, - types::rpc_channel, -}; use crate::{rpc_proto::proto, TopicScoreParams}; use crate::{PublishError, SubscriptionError, ValidationError}; use instant::SystemTime; @@ -3031,9 +3028,14 @@ where _: &Multiaddr, _: &Multiaddr, ) -> Result, ConnectionDenied> { - let (sender, receiver) = rpc_channel(self.config.connection_handler_queue_len()); - self.handler_send_queues.insert(peer_id, sender); - Ok(Handler::new(self.config.protocol_config(), receiver)) + let sender = self + .handler_send_queues + .entry(peer_id) + .or_insert_with(|| RpcSender::new(peer_id, self.config.connection_handler_queue_len())); + Ok(Handler::new( + self.config.protocol_config(), + sender.new_receiver(), + )) } fn handle_established_outbound_connection( @@ -3043,9 +3045,14 @@ where _: &Multiaddr, _: Endpoint, ) -> Result, ConnectionDenied> { - let (sender, receiver) = rpc_channel(self.config.connection_handler_queue_len()); - self.handler_send_queues.insert(peer_id, sender); - Ok(Handler::new(self.config.protocol_config(), receiver)) + let sender = self + .handler_send_queues + .entry(peer_id) + .or_insert_with(|| RpcSender::new(peer_id, self.config.connection_handler_queue_len())); + Ok(Handler::new( + self.config.protocol_config(), + sender.new_receiver(), + )) } fn on_connection_handler_event( diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index 050b387a5e3..8612a841e19 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -23,7 +23,7 @@ use super::*; use crate::subscription_filter::WhitelistSubscriptionFilter; use crate::transform::{DataTransform, IdentityTransform}; -use crate::types::RpcReceiver; +use crate::types::{RpcOut, RpcReceiver}; use crate::ValidationError; use crate::{ config::Config, config::ConfigBuilder, types::Rpc, IdentTopic as Topic, TopicScoreParams, @@ -230,7 +230,8 @@ where } }; - let (sender, receiver) = rpc_channel(100); + let sender = RpcSender::new(peer, 100); + let receiver = sender.new_receiver(); gs.handler_send_queues.insert(peer, sender); gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished { diff --git a/protocols/gossipsub/src/types.rs b/protocols/gossipsub/src/types.rs index e8318401d51..f503fb61a63 100644 --- a/protocols/gossipsub/src/types.rs +++ b/protocols/gossipsub/src/types.rs @@ -518,35 +518,43 @@ impl fmt::Display for PeerKind { } } -/// Create `RpcOut` channel that is priority aware. -pub(crate) fn rpc_channel(cap: usize) -> (RpcSender, RpcReceiver) { - let (priority_sender, priority_receiver) = async_channel::unbounded(); - let (non_priority_sender, non_priority_receiver) = async_channel::bounded(cap / 2); - let len = Arc::new(AtomicUsize::new(0)); - ( - RpcSender { - cap: cap / 2, - len: len.clone(), - priority: priority_sender, - non_priority: non_priority_sender, - }, - RpcReceiver { - len, - priority: priority_receiver, - non_priority: non_priority_receiver, - }, - ) -} - /// `RpcOut` sender that is priority aware. +#[derive(Debug, Clone)] pub(crate) struct RpcSender { + peer_id: PeerId, cap: usize, len: Arc, priority: Sender, non_priority: Sender, + receiver: RpcReceiver, } impl RpcSender { + /// Create a RpcSender. + pub(crate) fn new(peer_id: PeerId, cap: usize) -> RpcSender { + let (priority_sender, priority_receiver) = async_channel::unbounded(); + let (non_priority_sender, non_priority_receiver) = async_channel::bounded(cap / 2); + let len = Arc::new(AtomicUsize::new(0)); + let receiver = RpcReceiver { + len: len.clone(), + priority: priority_receiver, + non_priority: non_priority_receiver, + }; + RpcSender { + peer_id, + cap: cap / 2, + len, + priority: priority_sender, + non_priority: non_priority_sender, + receiver: receiver.clone(), + } + } + + /// Create a new Receiver to the sender. + pub(crate) fn new_receiver(&self) -> RpcReceiver { + self.receiver.clone() + } + /// Send `RpcOut`s to the `ConnectionHandler` according to their priority. pub(crate) fn try_send(&mut self, rpc: RpcOut) -> Result<(), ()> { // Forward messages, IWANT and IHAVE control messages are regarded as low priority. @@ -579,6 +587,7 @@ impl RpcSender { } /// `RpcOut` sender that is priority aware. +#[derive(Debug, Clone)] pub struct RpcReceiver { len: Arc, pub(crate) priority: Receiver, From 92a171cc429a28dfeef5bb34c445af663414b365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Sat, 25 Nov 2023 22:41:13 +0000 Subject: [PATCH 04/11] split send_message into multiple RpcSender methods, to allow for better handling of each message send. --- protocols/gossipsub/src/behaviour.rs | 129 ++++++++++++++++----------- protocols/gossipsub/src/types.rs | 87 ++++++++++++------ 2 files changed, 137 insertions(+), 79 deletions(-) diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index 523687e3852..c822e02c2c5 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -60,7 +60,7 @@ use crate::types::{ ControlAction, Message, MessageAcceptance, MessageId, PeerInfo, RawMessage, Subscription, SubscriptionAction, }; -use crate::types::{PeerConnections, PeerKind, RpcOut}; +use crate::types::{PeerConnections, PeerKind}; use crate::{backoff::BackoffStorage, types::RpcSender}; use crate::{rpc_proto::proto, TopicScoreParams}; use crate::{PublishError, SubscriptionError, ValidationError}; @@ -538,11 +538,14 @@ where } // send subscription request to all peers - for peer in self.peer_topics.keys().copied().collect::>() { + for peer in self.peer_topics.keys() { tracing::debug!(%peer, "Sending SUBSCRIBE to peer"); - let event = RpcOut::Subscribe(topic_hash.clone()); - self.send_message(peer, event) - .expect("Subscribe messages should be always sent"); + let sender = self + .handler_send_queues + .get_mut(peer) + .expect("Peerid should exist"); + + sender.subscribe(topic_hash.clone()); } // call JOIN(topic) @@ -566,11 +569,14 @@ where } // announce to all peers - for peer in self.peer_topics.keys().copied().collect::>() { + for peer in self.peer_topics.keys() { tracing::debug!(%peer, "Sending UNSUBSCRIBE to peer"); - let event = RpcOut::Unsubscribe(topic_hash.clone()); - self.send_message(peer, event) - .expect("Subscribe messages should be always sent"); + let sender = self + .handler_send_queues + .get_mut(peer) + .expect("Peerid should exist"); + + sender.unsubscribe(topic_hash.clone()); } // call LEAVE(topic) @@ -720,8 +726,13 @@ where let mut errors = 0; for peer_id in recipient_peers.iter() { tracing::trace!(peer=%peer_id, "Sending message to peer"); - if self - .send_message(*peer_id, RpcOut::Publish(raw_message.clone())) + let sender = self + .handler_send_queues + .get_mut(peer_id) + .expect("Peerid should exist"); + + if sender + .publish(raw_message.clone(), self.metrics.as_mut()) .is_err() { errors += 1; @@ -1326,7 +1337,12 @@ where ); } else { tracing::debug!(peer=%peer_id, "IWANT: Sending cached messages to peer"); - let _ = self.send_message(*peer_id, RpcOut::Forward(msg)); + let sender = self + .handler_send_queues + .get_mut(peer_id) + .expect("Peerid should exist"); + + sender.forward(msg, self.metrics.as_mut()); } } } @@ -1479,13 +1495,17 @@ where if !to_prune_topics.is_empty() { // build the prune messages to send let on_unsubscribe = false; + let mut sender = self + .handler_send_queues + .get_mut(peer_id) + .expect("Peerid should exist") + .clone(); + for action in to_prune_topics .iter() .map(|t| self.make_prune(t, peer_id, do_px, on_unsubscribe)) - .collect::>() { - self.send_message(*peer_id, RpcOut::Control(action)) - .expect("PRUNE messages should always be sent"); + sender.control(action); } // Send the prune messages to the peer tracing::debug!( @@ -1980,13 +2000,16 @@ where // If we need to send grafts to peer, do so immediately, rather than waiting for the // heartbeat. + let sender = self + .handler_send_queues + .get_mut(propagation_source) + .expect("Peerid should exist"); + for action in topics_to_graft .into_iter() .map(|topic_hash| ControlAction::Graft { topic_hash }) - .collect::>() { - self.send_message(*propagation_source, RpcOut::Control(action)) - .expect("GRAFT messages should always be sent"); + sender.control(action); } // Notify the application of the subscriptions @@ -2521,6 +2544,13 @@ where // It therefore must be in at least one mesh and we do not need to inform the handler // of its removal from another. + // send the control messages + let mut sender = self + .handler_send_queues + .get_mut(&peer) + .expect("Peerid should exist") + .clone(); + // The following prunes are not due to unsubscribing. let prunes = to_prune .remove(&peer) @@ -2535,10 +2565,8 @@ where ) }); - // send the control messages - for msg in control_msgs.chain(prunes).collect::>() { - self.send_message(peer, RpcOut::Control(msg)) - .expect("PRUNE messages should always be sent"); + for msg in control_msgs.chain(prunes) { + sender.control(msg); } } @@ -2552,8 +2580,13 @@ where self.config.do_px() && !no_px.contains(peer), false, ); - self.send_message(*peer, RpcOut::Control(prune)) - .expect("PRUNE messages should always be sent"); + let mut sender = self + .handler_send_queues + .get_mut(peer) + .expect("Peerid should exist") + .clone(); + + sender.control(prune); // inform the handler peer_removed_from_mesh( @@ -2621,11 +2654,13 @@ where // forward the message to peers if !recipient_peers.is_empty() { - let event = RpcOut::Forward(message.clone()); - for peer in recipient_peers.iter() { tracing::debug!(%peer, message=%msg_id, "Sending message to peer"); - let _ = self.send_message(*peer, event.clone()); + let sender = self + .handler_send_queues + .get_mut(peer) + .expect("Peerid should exist"); + sender.forward(message.clone(), self.metrics.as_mut()); } tracing::debug!("Completed forwarding message"); Ok(true) @@ -2739,7 +2774,12 @@ where fn flush_control_pool(&mut self) { for (peer, controls) in self.control_pool.drain().collect::>() { for msg in controls { - let _ = self.send_message(peer, RpcOut::Control(msg)); + let sender = self + .handler_send_queues + .get_mut(&peer) + .expect("Peerid should exist"); + + sender.control(msg); } } @@ -2747,28 +2787,6 @@ where self.pending_iwant_msgs.clear(); } - /// Send a [`RpcOut`] message to a peer. This will wrap the message in an arc if it - /// is not already an arc. - fn send_message(&mut self, peer_id: PeerId, rpc: RpcOut) -> Result<(), ()> { - let sender = self - .handler_send_queues - .get_mut(&peer_id) - .expect("Peerid should exist"); - - if sender.try_send(rpc.clone()).is_err() { - tracing::debug!(peer=%peer_id, "Dropping message as peer is full"); - return Err(()); - } - - 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. - m.msg_sent(&message.topic, message.raw_protobuf_len()); - } - } - Ok(()) - } - fn on_connection_established( &mut self, ConnectionEstablished { @@ -2834,9 +2852,14 @@ where tracing::debug!(peer=%peer_id, "New peer connected"); // We need to send our subscriptions to the newly-connected node. + let mut sender = self + .handler_send_queues + .get_mut(&peer_id) + .expect("Peerid should exist") + .clone(); + for topic_hash in self.mesh.clone().into_keys() { - self.send_message(peer_id, RpcOut::Subscribe(topic_hash)) - .expect("Subscribe messages should be always sent"); + sender.subscribe(topic_hash); } } @@ -3420,7 +3443,7 @@ impl fmt::Debug for PublishConfig { #[cfg(test)] mod local_test { use super::*; - use crate::IdentTopic; + use crate::{types::RpcOut, IdentTopic}; use quickcheck::*; fn test_message() -> RawMessage { diff --git a/protocols/gossipsub/src/types.rs b/protocols/gossipsub/src/types.rs index f503fb61a63..6799b6a2b15 100644 --- a/protocols/gossipsub/src/types.rs +++ b/protocols/gossipsub/src/types.rs @@ -19,6 +19,7 @@ // DEALINGS IN THE SOFTWARE. //! A collection of types using the Gossipsub system. +use crate::metrics::Metrics; use crate::TopicHash; use async_channel::{Receiver, Sender}; use futures::Stream; @@ -555,35 +556,69 @@ impl RpcSender { self.receiver.clone() } - /// Send `RpcOut`s to the `ConnectionHandler` according to their priority. - pub(crate) fn try_send(&mut self, rpc: RpcOut) -> Result<(), ()> { - // Forward messages, IWANT and IHAVE control messages are regarded as low priority. - match rpc { - rpc @ RpcOut::Forward(_) - | rpc @ RpcOut::Control(ControlAction::IHave { .. }) - | rpc @ RpcOut::Control(ControlAction::IWant { .. }) => { - if let Err(err) = self.non_priority.try_send(rpc) { - let rpc = err.into_inner(); - tracing::trace!("{rpc:?} message dropped, queue is full"); - } - } - // GRAFT and PRUNE control messages, Subscription, and Publishes messages. - // Publish messages are limited to the capacity of the queue. - rpc @ RpcOut::Control(_) - | rpc @ RpcOut::Subscribe(_) - | rpc @ RpcOut::Unsubscribe(_) => { - self.priority.try_send(rpc).expect("Channel is unbounded"); - } - rpc @ RpcOut::Publish(_) => { - if self.len.load(Ordering::Relaxed) >= self.cap { - return Err(()); - } - self.priority.try_send(rpc).expect("Channel is unbounded"); - self.len.fetch_add(1, Ordering::Relaxed); - } + /// Send a `RpcOut::Control` message to the `RpcReceiver` + /// this is high priority. + pub(crate) fn control(&mut self, control: ControlAction) { + self.priority + .try_send(RpcOut::Control(control)) + .expect("Channel is unbounded and should always be open"); + } + + /// Send a `RpcOut::Subscribe` message to the `RpcReceiver` + /// this is high priority. + pub(crate) fn subscribe(&mut self, topic: TopicHash) { + self.priority + .try_send(RpcOut::Subscribe(topic)) + .expect("Channel is unbounded and should always be open"); + } + + /// Send a `RpcOut::Unsubscribe` message to the `RpcReceiver` + /// this is high priority. + pub(crate) fn unsubscribe(&mut self, topic: TopicHash) { + self.priority + .try_send(RpcOut::Unsubscribe(topic)) + .expect("Channel is unbounded and should always be open"); + } + + /// Send a `RpcOut::Publish` message to the `RpcReceiver` + /// this is high priority. If message sending fails, an `Err` is returned. + pub(crate) fn publish( + &mut self, + message: RawMessage, + metrics: Option<&mut Metrics>, + ) -> Result<(), ()> { + if self.len.load(Ordering::Relaxed) >= self.cap { + return Err(()); } + self.priority + .try_send(RpcOut::Publish(message.clone())) + .expect("Channel is unbounded and Should always be open"); + self.len.fetch_add(1, Ordering::Relaxed); + + if let Some(m) = metrics { + m.msg_sent(&message.topic, message.raw_protobuf_len()); + } + Ok(()) } + + /// Send a `RpcOut::Forward` message to the `RpcReceiver` + /// this is high priority. If the queue is full the message is discarded. + pub(crate) fn forward(&mut self, message: RawMessage, metrics: Option<&mut Metrics>) { + if let Err(err) = self.non_priority.try_send(RpcOut::Forward(message.clone())) { + let rpc = err.into_inner(); + tracing::trace!( + "{:?} message to peer {} dropped, queue is full", + rpc, + self.peer_id + ); + return; + } + + if let Some(m) = metrics { + m.msg_sent(&message.topic, message.raw_protobuf_len()); + } + } } /// `RpcOut` sender that is priority aware. From 547db09a63205fe179110e49463aa9caa8b3605b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Mon, 27 Nov 2023 16:38:44 +0000 Subject: [PATCH 05/11] cargo clippy --- protocols/gossipsub/src/behaviour/tests.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index 8612a841e19..e6c52526f63 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -421,7 +421,7 @@ fn test_subscribe() { .fold(0, |mut collected_subscriptions, c| { while !c.priority.is_empty() { if let Ok(RpcOut::Subscribe(_)) = c.priority.try_recv() { - collected_subscriptions = collected_subscriptions + 1 + collected_subscriptions += 1 } } collected_subscriptions @@ -479,7 +479,7 @@ fn test_unsubscribe() { .fold(0, |mut collected_subscriptions, c| { while !c.priority.is_empty() { if let Ok(RpcOut::Subscribe(_)) = c.priority.try_recv() { - collected_subscriptions = collected_subscriptions + 1 + collected_subscriptions += 1 } } collected_subscriptions @@ -1334,12 +1334,12 @@ fn count_control_msgs( while !c.priority.is_empty() || !c.non_priority.is_empty() { if let Ok(RpcOut::Control(action)) = c.priority.try_recv() { if filter(peer_id, &action) { - collected_messages = collected_messages + 1; + collected_messages += 1; } } if let Ok(RpcOut::Control(action)) = c.non_priority.try_recv() { if filter(peer_id, &action) { - collected_messages = collected_messages + 1; + collected_messages += 1; } } } @@ -1559,7 +1559,7 @@ fn do_forward_messages_to_explicit_peers() { queues.into_iter().fold(0, |mut fwds, (peer_id, c)| { while !c.non_priority.is_empty() { if matches!(c.non_priority.try_recv(), Ok(RpcOut::Forward(m)) if peer_id == peers[0] && m.data == message.data) { - fwds = fwds +1; + fwds +=1; } } fwds @@ -4394,8 +4394,8 @@ fn test_ignore_too_many_iwants_from_same_peer_for_same_message() { assert_eq!( queues.into_values().fold(0, |mut fwds, c| { while !c.non_priority.is_empty() { - if let Ok(RpcOut::Forward(_)) = dbg!(c.non_priority.try_recv()) { - fwds = fwds + 1; + if let Ok(RpcOut::Forward(_)) = c.non_priority.try_recv() { + fwds += 1; } } fwds @@ -4808,7 +4808,7 @@ fn test_publish_to_floodsub_peers_without_flood_publish() { if matches!(c.priority.try_recv(), Ok(RpcOut::Publish(_)) if peer_id == &p1 || peer_id == &p2) { - collected_publish = collected_publish + 1; + collected_publish += 1; } } collected_publish @@ -4863,7 +4863,7 @@ fn test_do_not_use_floodsub_in_fanout() { if matches!(c.priority.try_recv(), Ok(RpcOut::Publish(_)) if peer_id == &p1 || peer_id == &p2) { - collected_publish = collected_publish + 1; + collected_publish += 1; } } collected_publish From b08a4eba9a6fe4039e35d4805ab396490d07e094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Mon, 27 Nov 2023 16:45:10 +0000 Subject: [PATCH 06/11] add CHANGELOG.md entry --- protocols/gossipsub/CHANGELOG.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/protocols/gossipsub/CHANGELOG.md b/protocols/gossipsub/CHANGELOG.md index 5ff4cfa27d6..10b709cb46f 100644 --- a/protocols/gossipsub/CHANGELOG.md +++ b/protocols/gossipsub/CHANGELOG.md @@ -1,7 +1,11 @@ -## 0.46.1 +## 0.46.1 - unreleased + +- Implement backpressure by diferentiating between priority and non priority messages. + Drop `Publish` and `Forward` messages when the queue becomes full. + See [PR 4914](https://github.com/libp2p/rust-libp2p/pull/4914) - Deprecate `Rpc` in preparation for removing it from the public API because it is an internal type. - See [PR 4833](https://github.com/libp2p/rust-libp2p/pull/4833). + See [PR 4833](https://github.com/libp2p/rust-libp2p/pull/4833). ## 0.46.0 From da9008ba88c6fbd2cbcfd557f682bee302b4f9d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Sat, 9 Dec 2023 16:43:20 +0000 Subject: [PATCH 07/11] update to use futures::channel and connected_peers --- protocols/gossipsub/Cargo.toml | 2 +- protocols/gossipsub/src/behaviour.rs | 154 ++-- protocols/gossipsub/src/behaviour/tests.rs | 901 ++++++++++++--------- protocols/gossipsub/src/handler.rs | 2 +- protocols/gossipsub/src/types.rs | 239 +++--- 5 files changed, 723 insertions(+), 575 deletions(-) diff --git a/protocols/gossipsub/Cargo.toml b/protocols/gossipsub/Cargo.toml index 2a0510038f1..e0d37819ebe 100644 --- a/protocols/gossipsub/Cargo.toml +++ b/protocols/gossipsub/Cargo.toml @@ -43,7 +43,7 @@ async-channel = "1.9.0" prometheus-client = { workspace = true } [dev-dependencies] -async-std = { version = "1.6.3", features = ["unstable"] } +async-std = { version = "1.6.3", features = ["unstable", "attributes"] } hex = "0.4.2" libp2p-core = { workspace = true } libp2p-yamux = { workspace = true } diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index c822e02c2c5..c591ebb4d4c 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -29,6 +29,7 @@ use std::{ time::Duration, }; +use futures::channel::mpsc::channel; use futures::StreamExt; use futures_ticker::Ticker; use prometheus_client::registry::Registry; @@ -45,7 +46,6 @@ use libp2p_swarm::{ THandlerOutEvent, ToSwarm, }; -use crate::config::{Config, ValidationMode}; use crate::gossip_promises::GossipPromises; use crate::handler::{Handler, HandlerEvent, HandlerIn}; use crate::mcache::MessageCache; @@ -62,6 +62,10 @@ use crate::types::{ }; use crate::types::{PeerConnections, PeerKind}; use crate::{backoff::BackoffStorage, types::RpcSender}; +use crate::{ + config::{Config, ValidationMode}, + types::RpcReceiver, +}; use crate::{rpc_proto::proto, TopicScoreParams}; use crate::{PublishError, SubscriptionError, ValidationError}; use instant::SystemTime; @@ -541,7 +545,7 @@ where for peer in self.peer_topics.keys() { tracing::debug!(%peer, "Sending SUBSCRIBE to peer"); let sender = self - .handler_send_queues + .connected_peers .get_mut(peer) .expect("Peerid should exist"); @@ -572,7 +576,7 @@ where for peer in self.peer_topics.keys() { tracing::debug!(%peer, "Sending UNSUBSCRIBE to peer"); let sender = self - .handler_send_queues + .connected_peers .get_mut(peer) .expect("Peerid should exist"); @@ -723,22 +727,19 @@ where } // Send to peers we know are subscribed to the topic. - let mut errors = 0; + let mut publish_failed = true; for peer_id in recipient_peers.iter() { tracing::trace!(peer=%peer_id, "Sending message to peer"); let sender = self - .handler_send_queues + .connected_peers .get_mut(peer_id) .expect("Peerid should exist"); - if sender + publish_failed &= sender .publish(raw_message.clone(), self.metrics.as_mut()) - .is_err() - { - errors += 1; - } + .is_err(); } - if errors == recipient_peers.len() { + if publish_failed { return Err(PublishError::InsufficientPeers); } @@ -1338,7 +1339,7 @@ where } else { tracing::debug!(peer=%peer_id, "IWANT: Sending cached messages to peer"); let sender = self - .handler_send_queues + .connected_peers .get_mut(peer_id) .expect("Peerid should exist"); @@ -1496,10 +1497,9 @@ where // build the prune messages to send let on_unsubscribe = false; let mut sender = self - .handler_send_queues - .get_mut(peer_id) - .expect("Peerid should exist") - .clone(); + .connected_peers + .remove(peer_id) + .expect("Peerid should exist"); for action in to_prune_topics .iter() @@ -1507,6 +1507,7 @@ where { sender.control(action); } + self.connected_peers.insert(*peer_id, sender); // Send the prune messages to the peer tracing::debug!( peer=%peer_id, @@ -2001,7 +2002,7 @@ where // If we need to send grafts to peer, do so immediately, rather than waiting for the // heartbeat. let sender = self - .handler_send_queues + .connected_peers .get_mut(propagation_source) .expect("Peerid should exist"); @@ -2546,7 +2547,7 @@ where // send the control messages let mut sender = self - .handler_send_queues + .connected_peers .get_mut(&peer) .expect("Peerid should exist") .clone(); @@ -2581,7 +2582,7 @@ where false, ); let mut sender = self - .handler_send_queues + .connected_peers .get_mut(peer) .expect("Peerid should exist") .clone(); @@ -2657,7 +2658,7 @@ where for peer in recipient_peers.iter() { tracing::debug!(%peer, message=%msg_id, "Sending message to peer"); let sender = self - .handler_send_queues + .connected_peers .get_mut(peer) .expect("Peerid should exist"); sender.forward(message.clone(), self.metrics.as_mut()); @@ -2775,7 +2776,7 @@ where for (peer, controls) in self.control_pool.drain().collect::>() { for msg in controls { let sender = self - .handler_send_queues + .connected_peers .get_mut(&peer) .expect("Peerid should exist"); @@ -2791,7 +2792,7 @@ where &mut self, ConnectionEstablished { peer_id, - connection_id, + connection_id: _, endpoint, other_established, .. @@ -2819,20 +2820,6 @@ where } } - // By default we assume a peer is only a floodsub peer. - // - // The protocol negotiation occurs once a message is sent/received. Once this happens we - // update the type of peer that this is in order to determine which kind of routing should - // occur. - self.connected_peers - .entry(peer_id) - .or_insert(PeerConnections { - kind: PeerKind::Floodsub, - connections: vec![], - }) - .connections - .push(connection_id); - if other_established > 0 { return; // Not our first connection to this peer, hence nothing to do. } @@ -2853,7 +2840,7 @@ where tracing::debug!(peer=%peer_id, "New peer connected"); // We need to send our subscriptions to the newly-connected node. let mut sender = self - .handler_send_queues + .connected_peers .get_mut(&peer_id) .expect("Peerid should exist") .clone(); @@ -2889,16 +2876,11 @@ where if remaining_established != 0 { // Remove the connection from the list if let Some(connections) = self.connected_peers.get_mut(&peer_id) { - let index = connections - .connections - .iter() - .position(|v| v == &connection_id) - .expect("Previously established connection to peer must be present"); - connections.connections.remove(index); + connections.connections.remove(&connection_id); // If there are more connections and this peer is in a mesh, inform the first connection // handler. - if !connections.connections.is_empty() { + if let Some(alternative_connection_id) = connections.connections.keys().next() { if let Some(topics) = self.peer_topics.get(&peer_id) { for topic in topics { if let Some(mesh_peers) = self.mesh.get(topic) { @@ -2906,7 +2888,7 @@ where self.events.push_back(ToSwarm::NotifyHandler { peer_id, event: HandlerIn::JoinedMesh, - handler: NotifyHandler::One(connections.connections[0]), + handler: NotifyHandler::One(*alternative_connection_id), }); break; } @@ -3046,36 +3028,72 @@ where fn handle_established_inbound_connection( &mut self, - _: ConnectionId, + connection_id: ConnectionId, peer_id: PeerId, _: &Multiaddr, _: &Multiaddr, ) -> Result, ConnectionDenied> { - let sender = self - .handler_send_queues - .entry(peer_id) - .or_insert_with(|| RpcSender::new(peer_id, self.config.connection_handler_queue_len())); - Ok(Handler::new( - self.config.protocol_config(), - sender.new_receiver(), - )) + let (priority_sender, priority_receiver) = + channel(self.config.connection_handler_queue_len()); + let (non_priority_sender, non_priority_receiver) = + channel(self.config.connection_handler_queue_len()); + let sender = RpcSender { + priority: priority_sender, + non_priority: non_priority_sender, + }; + let receiver = RpcReceiver { + priority: priority_receiver.peekable(), + non_priority: non_priority_receiver.peekable(), + }; + // By default we assume a peer is only a floodsub peer. + // + // The protocol negotiation occurs once a message is sent/received. Once this happens we + // update the type of peer that this is in order to determine which kind of routing should + // occur. + let peer_info = + self.connected_peers + .entry(peer_id) + .or_insert_with_key(|_| PeerConnections { + kind: PeerKind::Floodsub, + connections: Default::default(), + }); + peer_info.connections.insert(connection_id, sender); + Ok(Handler::new(self.config.protocol_config(), receiver)) } fn handle_established_outbound_connection( &mut self, - _: ConnectionId, + connection_id: ConnectionId, peer_id: PeerId, _: &Multiaddr, _: Endpoint, ) -> Result, ConnectionDenied> { - let sender = self - .handler_send_queues - .entry(peer_id) - .or_insert_with(|| RpcSender::new(peer_id, self.config.connection_handler_queue_len())); - Ok(Handler::new( - self.config.protocol_config(), - sender.new_receiver(), - )) + let (priority_sender, priority_receiver) = + channel(self.config.connection_handler_queue_len()); + let (non_priority_sender, non_priority_receiver) = + channel(self.config.connection_handler_queue_len()); + let sender = RpcSender { + priority: priority_sender, + non_priority: non_priority_sender, + }; + let receiver = RpcReceiver { + priority: priority_receiver.peekable(), + non_priority: non_priority_receiver.peekable(), + }; + // By default we assume a peer is only a floodsub peer. + // + // The protocol negotiation occurs once a message is sent/received. Once this happens we + // update the type of peer that this is in order to determine which kind of routing should + // occur. + let peer_info = + self.connected_peers + .entry(peer_id) + .or_insert_with_key(|_| PeerConnections { + kind: PeerKind::Floodsub, + connections: Default::default(), + }); + peer_info.connections.insert(connection_id, sender); + Ok(Handler::new(self.config.protocol_config(), receiver)) } fn on_connection_handler_event( @@ -3260,7 +3278,10 @@ fn peer_added_to_mesh( !conn.connections.is_empty(), "Must have at least one connection" ); - conn.connections[0] + conn.connections + .keys() + .next() + .expect("To be connected to peer") }; if let Some(topics) = known_topics { @@ -3279,7 +3300,7 @@ fn peer_added_to_mesh( events.push_back(ToSwarm::NotifyHandler { peer_id, event: HandlerIn::JoinedMesh, - handler: NotifyHandler::One(connection_id), + handler: NotifyHandler::One(*connection_id), }); } @@ -3299,7 +3320,8 @@ fn peer_removed_from_mesh( .get(&peer_id) .expect("To be connected to peer.") .connections - .first() + .keys() + .next() .expect("There should be at least one connection to a peer."); if let Some(topics) = known_topics { diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index e6c52526f63..e1f39af1010 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -30,8 +30,10 @@ use crate::{ }; use async_std::net::Ipv4Addr; use byteorder::{BigEndian, ByteOrder}; +use futures::stream; use libp2p_core::{ConnectedPoint, Endpoint}; use rand::Rng; +use std::future::poll_fn; use std::thread::sleep; use std::time::Duration; @@ -94,7 +96,7 @@ where // build and connect peer_no random peers let mut peers = vec![]; - let mut receiver_queues = HashMap::new(); + let mut receivers = HashMap::new(); let empty = vec![]; for i in 0..self.peer_no { @@ -109,10 +111,10 @@ where i < self.explicit, ); peers.push(peer); - receiver_queues.insert(peer, receiver); + receivers.insert(peer, receiver); } - (gs, peers, receiver_queues, topic_hashes) + (gs, peers, receivers, topic_hashes) } fn peer_no(mut self, peer_no: usize) -> Self { @@ -230,9 +232,26 @@ where } }; - let sender = RpcSender::new(peer, 100); - let receiver = sender.new_receiver(); - gs.handler_send_queues.insert(peer, sender); + let (priority_sender, priority_receiver) = channel(gs.config.connection_handler_queue_len()); + let (non_priority_sender, non_priority_receiver) = + channel(gs.config.connection_handler_queue_len()); + let sender = RpcSender { + priority: priority_sender, + non_priority: non_priority_sender, + }; + let receiver = RpcReceiver { + priority: priority_receiver.peekable(), + non_priority: non_priority_receiver.peekable(), + }; + + let mut peer_info = PeerConnections { + kind: PeerKind::Floodsub, + connections: Default::default(), + }; + peer_info + .connections + .insert(ConnectionId::new_unchecked(0), sender); + gs.connected_peers.insert(peer, peer_info); gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished { peer_id: peer, @@ -280,12 +299,12 @@ where // peer_connections.connections should never be empty. let mut active_connections = peer_connections.connections.len(); - for connection_id in peer_connections.connections.clone() { + for connection_id in peer_connections.connections.clone().keys() { active_connections = active_connections.checked_sub(1).unwrap(); gs.on_swarm_event(FromSwarm::ConnectionClosed(ConnectionClosed { peer_id: *peer_id, - connection_id, + connection_id: *connection_id, endpoint: &fake_endpoint, remaining_established: active_connections, })); @@ -395,16 +414,16 @@ fn proto_to_message(rpc: &proto::RPC) -> Rpc { } } -#[test] +#[async_std::test] /// Test local node subscribing to a topic -fn test_subscribe() { +async fn test_subscribe() { // The node should: // - Create an empty vector in mesh[topic] // - Send subscription request to all peers // - run JOIN(topic) let subscribe_topic = vec![String::from("test_subscribe")]; - let (gs, _, queues, topic_hashes) = inject_nodes1() + let (mut gs, _, receivers, topic_hashes) = inject_nodes1() .peer_no(20) .topics(subscribe_topic) .to_subscribe(true) @@ -416,24 +435,22 @@ fn test_subscribe() { ); // collect all the subscriptions - let subscriptions = queues - .into_values() - .fold(0, |mut collected_subscriptions, c| { - while !c.priority.is_empty() { - if let Ok(RpcOut::Subscribe(_)) = c.priority.try_recv() { - collected_subscriptions += 1 - } - } - collected_subscriptions - }); + close_senders(&mut gs.connected_peers); + let subscriptions = stream::select_all(receivers.into_values()) + .filter(|e| { + let matches = matches!(e, RpcOut::Subscribe(_)); + async move { matches } + }) + .count() + .await; // we sent a subscribe to all known peers assert_eq!(subscriptions, 20); } -#[test] /// Test unsubscribe. -fn test_unsubscribe() { +#[async_std::test] +async fn test_unsubscribe() { // Unsubscribe should: // - Remove the mesh entry for topic // - Send UNSUBSCRIBE to all known peers @@ -446,7 +463,7 @@ fn test_unsubscribe() { .collect::>(); // subscribe to topic_strings - let (mut gs, _, queues, topic_hashes) = inject_nodes1() + let (mut gs, _, receivers, topic_hashes) = inject_nodes1() .peer_no(20) .topics(topic_strings) .to_subscribe(true) @@ -474,16 +491,15 @@ fn test_unsubscribe() { ); // collect all the subscriptions - let subscriptions = queues - .into_values() - .fold(0, |mut collected_subscriptions, c| { - while !c.priority.is_empty() { - if let Ok(RpcOut::Subscribe(_)) = c.priority.try_recv() { - collected_subscriptions += 1 - } + close_senders(&mut gs.connected_peers); + let subscriptions = stream::select_all(receivers.into_values()) + .fold(0, |collected_subscriptions, e| async move { + match e { + RpcOut::Subscribe(_) => collected_subscriptions + 1, + _ => collected_subscriptions, } - collected_subscriptions - }); + }) + .await; // we sent a unsubscribe to all known peers, for two topics assert_eq!(subscriptions, 40); @@ -628,8 +644,8 @@ fn test_join() { } /// Test local node publish to subscribed topic -#[test] -fn test_publish_without_flood_publishing() { +#[async_std::test] +async fn test_publish_without_flood_publishing() { // node should: // - Send publish message to all peers // - Insert message into gs.mcache and gs.received @@ -641,7 +657,7 @@ fn test_publish_without_flood_publishing() { .unwrap(); let publish_topic = String::from("test_publish"); - let (mut gs, _, queues, topic_hashes) = inject_nodes1() + let (mut gs, _, receivers, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![publish_topic.clone()]) .to_subscribe(true) @@ -665,16 +681,18 @@ fn test_publish_without_flood_publishing() { gs.publish(Topic::new(publish_topic), publish_data).unwrap(); // Collect all publish messages - let publishes = queues - .into_values() - .fold(vec![], |mut collected_publish, c| { - while !c.priority.is_empty() { - if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { + close_senders(&mut gs.connected_peers); + let publishes = stream::select_all(receivers.into_values()) + .fold(vec![], |mut collected_publish, e| async move { + match e { + RpcOut::Publish(message) => { collected_publish.push(message); + collected_publish } + _ => collected_publish, } - collected_publish - }); + }) + .await; // Transform the inbound message let message = &gs @@ -703,8 +721,8 @@ fn test_publish_without_flood_publishing() { } /// Test local node publish to unsubscribed topic -#[test] -fn test_fanout() { +#[async_std::test] +async fn test_fanout() { // node should: // - Populate fanout peers // - Send publish message to fanout peers @@ -717,7 +735,7 @@ fn test_fanout() { .unwrap(); let fanout_topic = String::from("test_fanout"); - let (mut gs, _, queues, topic_hashes) = inject_nodes1() + let (mut gs, _, receivers, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![fanout_topic.clone()]) .to_subscribe(true) @@ -749,16 +767,18 @@ fn test_fanout() { ); // Collect all publish messages - let publishes = queues - .into_values() - .fold(vec![], |mut collected_publish, c| { - while !c.priority.is_empty() { - if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { + close_senders(&mut gs.connected_peers); + let publishes = stream::select_all(receivers.into_values()) + .fold(vec![], |mut collected_publish, e| async move { + match e { + RpcOut::Publish(message) => { collected_publish.push(message); + collected_publish } + _ => collected_publish, } - collected_publish - }); + }) + .await; // Transform the inbound message let message = &gs @@ -785,10 +805,10 @@ fn test_fanout() { ); } -#[test] +#[async_std::test] /// Test the gossipsub NetworkBehaviour peer connection logic. -fn test_inject_connected() { - let (gs, peers, queues, topic_hashes) = inject_nodes1() +async fn test_inject_connected() { + let (mut gs, peers, receivers, topic_hashes) = inject_nodes1() .peer_no(20) .topics(vec![String::from("topic1"), String::from("topic2")]) .to_subscribe(true) @@ -796,19 +816,29 @@ fn test_inject_connected() { // check that our subscriptions are sent to each of the peers // collect all the SendEvents - let subscriptions = queues.into_iter().fold( + close_senders(&mut gs.connected_peers); + let subscriptions = stream::select_all( + receivers + .into_iter() + .map(|(peer_id, receiver)| stream::repeat(peer_id).zip(receiver)), + ) + .filter_map(|(peer_id, e)| async move { + dbg!(peer_id, &e); + match e { + RpcOut::Subscribe(topic) => Some((peer_id, topic)), + _ => None, + } + }) + .fold( HashMap::>::new(), - |mut collected_subscriptions, (peer, c)| { - while !c.priority.is_empty() { - if let Ok(RpcOut::Subscribe(topic)) = c.priority.try_recv() { - let mut peer_subs = collected_subscriptions.remove(&peer).unwrap_or_default(); - peer_subs.push(topic.into_string()); - collected_subscriptions.insert(peer, peer_subs); - } - } - collected_subscriptions + |mut subs, (peer, sub)| async move { + let mut peer_subs = subs.remove(&peer).unwrap_or_default(); + peer_subs.push(sub.into_string()); + subs.insert(peer, peer_subs); + subs }, - ); + ) + .await; // check that there are two subscriptions sent to each peer for peer_subs in subscriptions.values() { @@ -948,7 +978,7 @@ fn test_get_random_peers() { *p, PeerConnections { kind: PeerKind::Gossipsubv1_1, - connections: vec![ConnectionId::new_unchecked(0)], + connections: Default::default(), }, ) }) @@ -1001,9 +1031,9 @@ fn test_get_random_peers() { } /// Tests that the correct message is sent when a peer asks for a message in our cache. -#[test] -fn test_handle_iwant_msg_cached() { - let (mut gs, peers, queues, _) = inject_nodes1() +#[async_std::test] +async fn test_handle_iwant_msg_cached() { + let (mut gs, peers, receivers, _) = inject_nodes1() .peer_no(20) .topics(Vec::new()) .to_subscribe(true) @@ -1031,16 +1061,18 @@ fn test_handle_iwant_msg_cached() { gs.handle_iwant(&peers[7], vec![msg_id.clone()]); // the messages we are sending - let sent_messages = queues - .into_values() - .fold(vec![], |mut collected_messages, c| { - while !c.non_priority.is_empty() { - if let Ok(RpcOut::Forward(msg)) = c.non_priority.try_recv() { + close_senders(&mut gs.connected_peers); + let sent_messages = stream::select_all(receivers.into_values()) + .fold( + Vec::::new(), + |mut collected_messages, e| async move { + if let RpcOut::Forward(msg) = e { collected_messages.push(msg) } - } - collected_messages - }); + collected_messages + }, + ) + .await; assert!( sent_messages @@ -1052,14 +1084,15 @@ fn test_handle_iwant_msg_cached() { } /// Tests that messages are sent correctly depending on the shifting of the message cache. -#[test] -fn test_handle_iwant_msg_cached_shifted() { - let (mut gs, peers, queues, _) = inject_nodes1() +#[async_std::test] +async fn test_handle_iwant_msg_cached_shifted() { + let (mut gs, peers, receivers, _) = inject_nodes1() .peer_no(20) .topics(Vec::new()) .to_subscribe(true) .create_network(); + let mut rpcs = stream::select_all(receivers.into_values()); // perform 10 memshifts and check that it leaves the cache for shift in 1..10 { let raw_message = RawMessage { @@ -1086,31 +1119,31 @@ fn test_handle_iwant_msg_cached_shifted() { gs.handle_iwant(&peers[7], vec![msg_id.clone()]); - // is the message is being sent? - let message_exists = queues.values().any(|c| { - let mut out = false; - while !c.non_priority.is_empty() { - if matches!(c.non_priority.try_recv(), Ok(RpcOut::Forward(message)) if - gs.config.message_id( - &gs.data_transform - .inbound_transform(message.clone()) - .unwrap(), - ) == msg_id) + // default history_length is 5, expect no messages after shift > 5 + if shift < 5 { + // is the message being sent? + let mut message_exists = false; + if let Some(RpcOut::Forward(message)) = rpcs.next().await { + if gs.config.message_id( + &gs.data_transform + .inbound_transform(message.clone()) + .unwrap(), + ) == msg_id { - out = true; + message_exists = true; } } - out - }); - // default history_length is 5, expect no messages after shift > 5 - if shift < 5 { + assert!( message_exists, "Expected the cached message to be sent to an IWANT peer before 5 shifts" ); } else { + // assert all receivers are empty. + close_senders(&mut gs.connected_peers); + let queue_is_empty = rpcs.next().await.is_none(); assert!( - !message_exists, + queue_is_empty, "Expected the cached message to not be sent to an IWANT peer after 5 shifts" ); } @@ -1319,44 +1352,46 @@ fn test_handle_prune_peer_in_mesh() { ); } -fn count_control_msgs( +async fn count_control_msgs( gs: &Behaviour, - queues: &HashMap, + receivers: &mut HashMap, mut filter: impl FnMut(&PeerId, &ControlAction) -> bool, ) -> usize { - gs.control_pool + let mut collected_messages = gs + .control_pool .iter() .map(|(peer_id, actions)| actions.iter().filter(|m| filter(peer_id, m)).count()) - .sum::() - + queues - .iter() - .fold(0, |mut collected_messages, (peer_id, c)| { - while !c.priority.is_empty() || !c.non_priority.is_empty() { - if let Ok(RpcOut::Control(action)) = c.priority.try_recv() { - if filter(peer_id, &action) { - collected_messages += 1; - } - } - if let Ok(RpcOut::Control(action)) = c.non_priority.try_recv() { - if filter(peer_id, &action) { - collected_messages += 1; - } - } + .sum::(); + for (peer_id, receiver) in receivers.iter_mut() { + while !poll_fn(|cx| Poll::Ready(receiver.poll_is_empty(cx))).await { + if let Some(RpcOut::Control(action)) = receiver.next().await { + if filter(&peer_id, &action) { + collected_messages += 1 } - collected_messages - }) + } + } + } + collected_messages } -fn flush_events( - gs: &mut Behaviour, - receiver_queues: &mut HashMap, -) { +fn flush_events(gs: &mut Behaviour) { gs.control_pool.clear(); gs.events.clear(); - for c in receiver_queues.values_mut() { - while !c.priority.is_empty() || !c.non_priority.is_empty() { - let _ = c.priority.try_recv(); - let _ = c.non_priority.try_recv(); +} + +async fn flush_receivers(receivers: &mut HashMap) { + for c in receivers.values_mut() { + while !poll_fn(|cx| Poll::Ready(c.poll_is_empty(cx))).await { + let _ = c.next().await; + } + } +} + +fn close_senders(peers: &mut HashMap) { + for peer in peers.values_mut() { + for sender in peer.connections.values_mut() { + sender.priority.close_channel(); + sender.non_priority.close_channel(); } } } @@ -1397,7 +1432,7 @@ fn test_explicit_peer_reconnects() { .check_explicit_peers_ticks(2) .build() .unwrap(); - let (mut gs, others, mut queues, _) = inject_nodes1() + let (mut gs, others, _, _) = inject_nodes1() .peer_no(1) .topics(Vec::new()) .to_subscribe(true) @@ -1409,7 +1444,7 @@ fn test_explicit_peer_reconnects() { //add peer as explicit peer gs.add_explicit_peer(peer); - flush_events(&mut gs, &mut queues); + flush_events(&mut gs); //disconnect peer disconnect_peer(&mut gs, peer); @@ -1445,9 +1480,9 @@ fn test_explicit_peer_reconnects() { ); } -#[test] -fn test_handle_graft_explicit_peer() { - let (mut gs, peers, queues, topic_hashes) = inject_nodes1() +#[async_std::test] +async fn test_handle_graft_explicit_peer() { + let (mut gs, peers, mut receivers, topic_hashes) = inject_nodes1() .peer_no(1) .topics(vec![String::from("topic1"), String::from("topic2")]) .to_subscribe(true) @@ -1464,21 +1499,23 @@ fn test_handle_graft_explicit_peer() { assert!(gs.mesh[&topic_hashes[1]].is_empty()); //check prunes + close_senders(&mut gs.connected_peers); assert!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == peer + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == peer && match m { ControlAction::Prune { topic_hash, .. } => topic_hash == &topic_hashes[0] || topic_hash == &topic_hashes[1], _ => false, }) + .await >= 2, "Not enough prunes sent when grafting from explicit peer" ); } -#[test] -fn explicit_peers_not_added_to_mesh_on_receiving_subscription() { - let (gs, peers, queues, topic_hashes) = inject_nodes1() +#[async_std::test] +async fn explicit_peers_not_added_to_mesh_on_receiving_subscription() { + let (gs, peers, mut receivers, topic_hashes) = inject_nodes1() .peer_no(2) .topics(vec![String::from("topic1")]) .to_subscribe(true) @@ -1494,24 +1531,26 @@ fn explicit_peers_not_added_to_mesh_on_receiving_subscription() { //assert that graft gets created to non-explicit peer assert!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[1] + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[1] && matches!(m, ControlAction::Graft { .. })) + .await >= 1, "No graft message got created to non-explicit peer" ); //assert that no graft gets created to explicit peer assert_eq!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] - && matches!(m, ControlAction::Graft { .. })), + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[0] + && matches!(m, ControlAction::Graft { .. })) + .await, 0, "A graft message got created to an explicit peer" ); } -#[test] -fn do_not_graft_explicit_peer() { - let (mut gs, others, queues, topic_hashes) = inject_nodes1() +#[async_std::test] +async fn do_not_graft_explicit_peer() { + let (mut gs, others, mut receivers, topic_hashes) = inject_nodes1() .peer_no(1) .topics(vec![String::from("topic")]) .to_subscribe(true) @@ -1526,16 +1565,17 @@ fn do_not_graft_explicit_peer() { //assert that no graft gets created to explicit peer assert_eq!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &others[0] - && matches!(m, ControlAction::Graft { .. })), + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &others[0] + && matches!(m, ControlAction::Graft { .. })) + .await, 0, "A graft message got created to an explicit peer" ); } -#[test] -fn do_forward_messages_to_explicit_peers() { - let (mut gs, peers, queues, topic_hashes) = inject_nodes1() +#[async_std::test] +async fn do_forward_messages_to_explicit_peers() { + let (mut gs, peers, receivers, topic_hashes) = inject_nodes1() .peer_no(2) .topics(vec![String::from("topic1"), String::from("topic2")]) .to_subscribe(true) @@ -1555,23 +1595,33 @@ fn do_forward_messages_to_explicit_peers() { validated: true, }; gs.handle_received_message(message.clone(), &local_id); + close_senders(&mut gs.connected_peers); assert_eq!( - queues.into_iter().fold(0, |mut fwds, (peer_id, c)| { - while !c.non_priority.is_empty() { - if matches!(c.non_priority.try_recv(), Ok(RpcOut::Forward(m)) if peer_id == peers[0] && m.data == message.data) { - fwds +=1; - } + stream::select_all( + receivers + .into_iter() + .map(|(peer_id, receiver)| stream::repeat(peer_id).zip(receiver)) + ) + .filter_map(|(peer_id, e)| { + let peers = peers.clone(); + let message = message.clone(); + async move { + match e { + RpcOut::Forward(m) if peer_id == peers[0] && m.data == message.data => Some(()), + _ => None, } - fwds - }), + } + }) + .count() + .await, 1, "The message did not get forwarded to the explicit peer" ); } -#[test] -fn explicit_peers_not_added_to_mesh_on_subscribe() { - let (mut gs, peers, queues, _) = inject_nodes1() +#[async_std::test] +async fn explicit_peers_not_added_to_mesh_on_subscribe() { + let (mut gs, peers, mut receivers, _) = inject_nodes1() .peer_no(2) .topics(Vec::new()) .to_subscribe(true) @@ -1600,24 +1650,26 @@ fn explicit_peers_not_added_to_mesh_on_subscribe() { //assert that graft gets created to non-explicit peer assert!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[1] + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[1] && matches!(m, ControlAction::Graft { .. })) + .await > 0, "No graft message got created to non-explicit peer" ); //assert that no graft gets created to explicit peer assert_eq!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] - && matches!(m, ControlAction::Graft { .. })), + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[0] + && matches!(m, ControlAction::Graft { .. })) + .await, 0, "A graft message got created to an explicit peer" ); } -#[test] -fn explicit_peers_not_added_to_mesh_from_fanout_on_subscribe() { - let (mut gs, peers, queues, _) = inject_nodes1() +#[async_std::test] +async fn explicit_peers_not_added_to_mesh_from_fanout_on_subscribe() { + let (mut gs, peers, mut receivers, _) = inject_nodes1() .peer_no(2) .topics(Vec::new()) .to_subscribe(true) @@ -1649,16 +1701,18 @@ fn explicit_peers_not_added_to_mesh_from_fanout_on_subscribe() { //assert that graft gets created to non-explicit peer assert!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[1] + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[1] && matches!(m, ControlAction::Graft { .. })) + .await >= 1, "No graft message got created to non-explicit peer" ); //assert that no graft gets created to explicit peer assert_eq!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] - && matches!(m, ControlAction::Graft { .. })), + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[0] + && matches!(m, ControlAction::Graft { .. })) + .await, 0, "A graft message got created to an explicit peer" ); @@ -1824,12 +1878,12 @@ fn test_connect_to_px_peers_on_handle_prune() { )); } -#[test] -fn test_send_px_and_backoff_in_prune() { +#[async_std::test] +async fn test_send_px_and_backoff_in_prune() { let config: Config = Config::default(); //build mesh with enough peers for px - let (mut gs, peers, queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(config.prune_peers() + 1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1846,7 +1900,7 @@ fn test_send_px_and_backoff_in_prune() { //check prune message assert_eq!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[0] && match m { ControlAction::Prune { topic_hash, @@ -1860,17 +1914,18 @@ fn test_send_px_and_backoff_in_prune() { config.prune_peers() && backoff.unwrap() == config.prune_backoff().as_secs(), _ => false, - }), + }) + .await, 1 ); } -#[test] -fn test_prune_backoffed_peer_on_graft() { +#[async_std::test] +async fn test_prune_backoffed_peer_on_graft() { let config: Config = Config::default(); //build mesh with enough peers for px - let (mut gs, peers, mut queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(config.prune_peers() + 1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1887,14 +1942,15 @@ fn test_prune_backoffed_peer_on_graft() { ); //ignore all messages until now - flush_events(&mut gs, &mut queues); + flush_events(&mut gs); + flush_receivers(&mut receivers).await; //handle graft gs.handle_graft(&peers[0], vec![topics[0].clone()]); //check prune message assert_eq!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[0] && match m { ControlAction::Prune { topic_hash, @@ -1906,20 +1962,21 @@ fn test_prune_backoffed_peer_on_graft() { peers.is_empty() && backoff.unwrap() == config.prune_backoff().as_secs(), _ => false, - }), + }) + .await, 1 ); } -#[test] -fn test_do_not_graft_within_backoff_period() { +#[async_std::test] +async fn test_do_not_graft_within_backoff_period() { let config = ConfigBuilder::default() .backoff_slack(1) .heartbeat_interval(Duration::from_millis(100)) .build() .unwrap(); //only one peer => mesh too small and will try to regraft as early as possible - let (mut gs, peers, mut queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1930,7 +1987,8 @@ fn test_do_not_graft_within_backoff_period() { gs.handle_prune(&peers[0], vec![(topics[0].clone(), Vec::new(), Some(1))]); //forget all events until now - flush_events(&mut gs, &mut queues); + flush_events(&mut gs); + flush_receivers(&mut receivers).await; //call heartbeat gs.heartbeat(); @@ -1944,10 +2002,11 @@ fn test_do_not_graft_within_backoff_period() { //Check that no graft got created (we have backoff_slack = 1 therefore one more heartbeat // is needed). assert_eq!( - count_control_msgs(&gs, &queues, |_, m| matches!( + count_control_msgs(&gs, &mut receivers, |_, m| matches!( m, ControlAction::Graft { .. } - )), + )) + .await, 0, "Graft message created too early within backoff period" ); @@ -1958,16 +2017,18 @@ fn test_do_not_graft_within_backoff_period() { //check that graft got created assert!( - count_control_msgs(&gs, &queues, |_, m| matches!( + count_control_msgs(&gs, &mut receivers, |_, m| matches!( m, ControlAction::Graft { .. } - )) > 0, + )) + .await + > 0, "No graft message was created after backoff period" ); } -#[test] -fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without_backoff() { +#[async_std::test] +async fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without_backoff() { //set default backoff period to 1 second let config = ConfigBuilder::default() .prune_backoff(Duration::from_millis(90)) @@ -1976,7 +2037,7 @@ fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without .build() .unwrap(); //only one peer => mesh too small and will try to regraft as early as possible - let (mut gs, peers, mut queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -1987,7 +2048,8 @@ fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without gs.handle_prune(&peers[0], vec![(topics[0].clone(), Vec::new(), None)]); //forget all events until now - flush_events(&mut gs, &mut queues); + flush_events(&mut gs); + flush_receivers(&mut receivers).await; //call heartbeat gs.heartbeat(); @@ -1999,10 +2061,11 @@ fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without //Check that no graft got created (we have backoff_slack = 1 therefore one more heartbeat // is needed). assert_eq!( - count_control_msgs(&gs, &queues, |_, m| matches!( + count_control_msgs(&gs, &mut receivers, |_, m| matches!( m, ControlAction::Graft { .. } - )), + )) + .await, 0, "Graft message created too early within backoff period" ); @@ -2013,16 +2076,18 @@ fn test_do_not_graft_within_default_backoff_period_after_receiving_prune_without //check that graft got created assert!( - count_control_msgs(&gs, &queues, |_, m| matches!( + count_control_msgs(&gs, &mut receivers, |_, m| matches!( m, ControlAction::Graft { .. } - )) > 0, + )) + .await + > 0, "No graft message was created after backoff period" ); } -#[test] -fn test_unsubscribe_backoff() { +#[async_std::test] +async fn test_unsubscribe_backoff() { const HEARTBEAT_INTERVAL: Duration = Duration::from_millis(100); let config = ConfigBuilder::default() .backoff_slack(1) @@ -2035,7 +2100,7 @@ fn test_unsubscribe_backoff() { let topic = String::from("test"); // only one peer => mesh too small and will try to regraft as early as possible - let (mut gs, _, mut queues, topics) = inject_nodes1() + let (mut gs, _, mut receivers, topics) = inject_nodes1() .peer_no(1) .topics(vec![topic.clone()]) .to_subscribe(true) @@ -2045,10 +2110,11 @@ fn test_unsubscribe_backoff() { let _ = gs.unsubscribe(&Topic::new(topic)); assert_eq!( - count_control_msgs(&gs, &queues, |_, m| match m { + count_control_msgs(&gs, &mut receivers, |_, m| match m { ControlAction::Prune { backoff, .. } => backoff == &Some(1), _ => false, - }), + }) + .await, 1, "Peer should be pruned with `unsubscribe_backoff`." ); @@ -2056,7 +2122,8 @@ fn test_unsubscribe_backoff() { let _ = gs.subscribe(&Topic::new(topics[0].to_string())); // forget all events until now - flush_events(&mut gs, &mut queues); + flush_events(&mut gs); + flush_receivers(&mut receivers).await; // call heartbeat gs.heartbeat(); @@ -2070,10 +2137,11 @@ fn test_unsubscribe_backoff() { // Check that no graft got created (we have backoff_slack = 1 therefore one more heartbeat // is needed). assert_eq!( - count_control_msgs(&gs, &queues, |_, m| matches!( + count_control_msgs(&gs, &mut receivers, |_, m| matches!( m, ControlAction::Graft { .. } - )), + )) + .await, 0, "Graft message created too early within backoff period" ); @@ -2084,21 +2152,23 @@ fn test_unsubscribe_backoff() { // check that graft got created assert!( - count_control_msgs(&gs, &queues, |_, m| matches!( + count_control_msgs(&gs, &mut receivers, |_, m| matches!( m, ControlAction::Graft { .. } - )) > 0, + )) + .await + > 0, "No graft message was created after backoff period" ); } -#[test] -fn test_flood_publish() { +#[async_std::test] +async fn test_flood_publish() { let config: Config = Config::default(); let topic = "test"; // Adds more peers than mesh can hold to test flood publishing - let (mut gs, _, queues, _) = inject_nodes1() + let (mut gs, _, receivers, _) = inject_nodes1() .peer_no(config.mesh_n_high() + 10) .topics(vec![topic.into()]) .to_subscribe(true) @@ -2109,16 +2179,18 @@ fn test_flood_publish() { gs.publish(Topic::new(topic), publish_data).unwrap(); // Collect all publish messages - let publishes = queues - .into_values() - .fold(vec![], |mut collected_publish, c| { - while !c.priority.is_empty() { - if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { + close_senders(&mut gs.connected_peers); + let publishes = stream::select_all(receivers.into_values()) + .fold(vec![], |mut collected_publish, e| async move { + match e { + RpcOut::Publish(message) => { collected_publish.push(message); + collected_publish } + _ => collected_publish, } - collected_publish - }); + }) + .await; // Transform the inbound message let message = &gs @@ -2146,13 +2218,13 @@ fn test_flood_publish() { ); } -#[test] -fn test_gossip_to_at_least_gossip_lazy_peers() { +#[async_std::test] +async fn test_gossip_to_at_least_gossip_lazy_peers() { let config: Config = Config::default(); //add more peers than in mesh to test gossipping //by default only mesh_n_low peers will get added to mesh - let (mut gs, _, queues, topic_hashes) = inject_nodes1() + let (mut gs, _, mut receivers, topic_hashes) = inject_nodes1() .peer_no(config.mesh_n_low() + config.gossip_lazy() + 1) .topics(vec!["topic".into()]) .to_subscribe(true) @@ -2180,24 +2252,25 @@ fn test_gossip_to_at_least_gossip_lazy_peers() { //check that exactly config.gossip_lazy() many gossip messages were sent. assert_eq!( - count_control_msgs(&gs, &queues, |_, action| match action { + count_control_msgs(&gs, &mut receivers, |_, action| match action { ControlAction::IHave { topic_hash, message_ids, } => topic_hash == &topic_hashes[0] && message_ids.iter().any(|id| id == &msg_id), _ => false, - }), + }) + .await, config.gossip_lazy() ); } -#[test] -fn test_gossip_to_at_most_gossip_factor_peers() { +#[async_std::test] +async fn test_gossip_to_at_most_gossip_factor_peers() { let config: Config = Config::default(); //add a lot of peers let m = config.mesh_n_low() + config.gossip_lazy() * (2.0 / config.gossip_factor()) as usize; - let (mut gs, _, queues, topic_hashes) = inject_nodes1() + let (mut gs, _, mut receivers, topic_hashes) = inject_nodes1() .peer_no(m) .topics(vec!["topic".into()]) .to_subscribe(true) @@ -2224,13 +2297,14 @@ fn test_gossip_to_at_most_gossip_factor_peers() { let msg_id = gs.config.message_id(message); //check that exactly config.gossip_lazy() many gossip messages were sent. assert_eq!( - count_control_msgs(&gs, &queues, |_, action| match action { + count_control_msgs(&gs, &mut receivers, |_, action| match action { ControlAction::IHave { topic_hash, message_ids, } => topic_hash == &topic_hashes[0] && message_ids.iter().any(|id| id == &msg_id), _ => false, - }), + }) + .await, ((m - config.mesh_n_low()) as f64 * config.gossip_factor()) as usize ); } @@ -2354,12 +2428,12 @@ fn test_add_outbound_peers_if_min_is_not_satisfied() { ); } -#[test] -fn test_prune_negative_scored_peers() { +#[async_std::test] +async fn test_prune_negative_scored_peers() { let config = Config::default(); //build mesh with one peer - let (mut gs, peers, queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2383,7 +2457,7 @@ fn test_prune_negative_scored_peers() { //check prune message assert_eq!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[0] + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[0] && match m { ControlAction::Prune { topic_hash, @@ -2395,7 +2469,8 @@ fn test_prune_negative_scored_peers() { peers.is_empty() && backoff.unwrap() == config.prune_backoff().as_secs(), _ => false, - }), + }) + .await, 1 ); } @@ -2481,8 +2556,8 @@ fn test_ignore_px_from_negative_scored_peer() { ); } -#[test] -fn test_only_send_nonnegative_scoring_peers_in_px() { +#[async_std::test] +async fn test_only_send_nonnegative_scoring_peers_in_px() { let config = ConfigBuilder::default() .prune_peers(16) .do_px() @@ -2490,7 +2565,7 @@ fn test_only_send_nonnegative_scoring_peers_in_px() { .unwrap(); // Build mesh with three peer - let (mut gs, peers, queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(3) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2517,7 +2592,7 @@ fn test_only_send_nonnegative_scoring_peers_in_px() { // Check that px in prune message only contains third peer assert_eq!( - count_control_msgs(&gs, &queues, |peer_id, m| peer_id == &peers[1] + count_control_msgs(&gs, &mut receivers, |peer_id, m| peer_id == &peers[1] && match m { ControlAction::Prune { topic_hash, @@ -2528,13 +2603,14 @@ fn test_only_send_nonnegative_scoring_peers_in_px() { && px.len() == 1 && px[0].peer_id.as_ref().unwrap() == &peers[2], _ => false, - }), + }) + .await, 1 ); } -#[test] -fn test_do_not_gossip_to_peers_below_gossip_threshold() { +#[async_std::test] +async fn test_do_not_gossip_to_peers_below_gossip_threshold() { let config = Config::default(); let peer_score_params = PeerScoreParams::default(); let peer_score_thresholds = PeerScoreThresholds { @@ -2543,7 +2619,7 @@ fn test_do_not_gossip_to_peers_below_gossip_threshold() { }; // Build full mesh - let (mut gs, peers, queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2590,7 +2666,7 @@ fn test_do_not_gossip_to_peers_below_gossip_threshold() { // Check that exactly one gossip messages got sent and it got sent to p2 assert_eq!( - count_control_msgs(&gs, &queues, |peer, action| match action { + count_control_msgs(&gs, &mut receivers, |peer, action| match action { ControlAction::IHave { topic_hash, message_ids, @@ -2603,13 +2679,14 @@ fn test_do_not_gossip_to_peers_below_gossip_threshold() { } } _ => false, - }), + }) + .await, 1 ); } -#[test] -fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { +#[async_std::test] +async fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { let config = Config::default(); let peer_score_params = PeerScoreParams::default(); let peer_score_thresholds = PeerScoreThresholds { @@ -2618,7 +2695,7 @@ fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { }; // Build full mesh - let (mut gs, peers, mut queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2635,9 +2712,9 @@ fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { // Add two additional peers that will not be part of the mesh let (p1, receiver1) = add_peer(&mut gs, &topics, false, false); - queues.insert(p1, receiver1); + receivers.insert(p1, receiver1); let (p2, receiver2) = add_peer(&mut gs, &topics, false, false); - queues.insert(p2, receiver2); + receivers.insert(p2, receiver2); // Reduce score of p1 below peer_score_thresholds.gossip_threshold // note that penalties get squared so two penalties means a score of @@ -2668,16 +2745,22 @@ fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { gs.handle_iwant(&p2, vec![msg_id.clone()]); // the messages we are sending - let sent_messages = queues - .into_iter() - .fold(vec![], |mut collected_messages, (peer_id, c)| { - while !c.non_priority.is_empty() { - if let Ok(RpcOut::Forward(message)) = c.non_priority.try_recv() { - collected_messages.push((peer_id, message)); - } + close_senders(&mut gs.connected_peers); + let sent_messages = stream::select_all( + receivers + .into_iter() + .map(|(peer_id, receiver)| stream::repeat(peer_id).zip(receiver)), + ) + .fold(vec![], |mut collected_messages, (peer_id, e)| async move { + match e { + RpcOut::Forward(message) => { + collected_messages.push((peer_id, message)); + collected_messages } - collected_messages - }); + _ => collected_messages, + } + }) + .await; //the message got sent to p2 assert!(sent_messages @@ -2697,8 +2780,8 @@ fn test_iwant_msg_from_peer_below_gossip_threshold_gets_ignored() { .all(|(peer_id, msg)| !(peer_id == &p1 && gs.config.message_id(&msg) == msg_id))); } -#[test] -fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { +#[async_std::test] +async fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { let config = Config::default(); let peer_score_params = PeerScoreParams::default(); let peer_score_thresholds = PeerScoreThresholds { @@ -2706,7 +2789,7 @@ fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { ..PeerScoreThresholds::default() }; //build full mesh - let (mut gs, peers, queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(true) @@ -2754,7 +2837,7 @@ fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { // check that we sent exactly one IWANT request to p2 assert_eq!( - count_control_msgs(&gs, &queues, |peer, c| match c { + count_control_msgs(&gs, &mut receivers, |peer, c| match c { ControlAction::IWant { message_ids } => if message_ids.iter().any(|m| m == &msg_id) { assert_eq!(peer, &p2); @@ -2763,13 +2846,14 @@ fn test_ihave_msg_from_peer_below_gossip_threshold_gets_ignored() { false }, _ => false, - }), + }) + .await, 1 ); } -#[test] -fn test_do_not_publish_to_peer_below_publish_threshold() { +#[async_std::test] +async fn test_do_not_publish_to_peer_below_publish_threshold() { let config = ConfigBuilder::default() .flood_publish(false) .build() @@ -2782,7 +2866,7 @@ fn test_do_not_publish_to_peer_below_publish_threshold() { }; //build mesh with no peers and no subscribed topics - let (mut gs, _, mut queues, _) = inject_nodes1() + let (mut gs, _, mut receivers, _) = inject_nodes1() .gs_config(config) .scoring(Some((peer_score_params, peer_score_thresholds))) .create_network(); @@ -2793,9 +2877,9 @@ fn test_do_not_publish_to_peer_below_publish_threshold() { //add two additional peers that will be added to the mesh let (p1, receiver1) = add_peer(&mut gs, &topics, false, false); - queues.insert(p1, receiver1); + receivers.insert(p1, receiver1); let (p2, receiver2) = add_peer(&mut gs, &topics, false, false); - queues.insert(p2, receiver2); + receivers.insert(p2, receiver2); //reduce score of p1 below peer_score_thresholds.publish_threshold //note that penalties get squared so two penalties means a score of @@ -2813,24 +2897,30 @@ fn test_do_not_publish_to_peer_below_publish_threshold() { gs.publish(topic, publish_data).unwrap(); // Collect all publish messages - let publishes = queues - .into_iter() - .fold(vec![], |mut collected_publish, (peer_id, c)| { - while !c.priority.is_empty() { - if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { - collected_publish.push((peer_id, message)); - } + close_senders(&mut gs.connected_peers); + let publishes = stream::select_all( + receivers + .into_iter() + .map(|(peer_id, receiver)| stream::repeat(peer_id).zip(receiver)), + ) + .fold(vec![], |mut collected_publish, (peer_id, e)| async move { + match e { + RpcOut::Publish(message) => { + collected_publish.push((peer_id, message)); + collected_publish } - collected_publish - }); + _ => collected_publish, + } + }) + .await; //assert only published to p2 assert_eq!(publishes.len(), 1); assert_eq!(publishes[0].0, p2); } -#[test] -fn test_do_not_flood_publish_to_peer_below_publish_threshold() { +#[async_std::test] +async fn test_do_not_flood_publish_to_peer_below_publish_threshold() { let config = Config::default(); let peer_score_params = PeerScoreParams::default(); let peer_score_thresholds = PeerScoreThresholds { @@ -2839,7 +2929,7 @@ fn test_do_not_flood_publish_to_peer_below_publish_threshold() { ..PeerScoreThresholds::default() }; //build mesh with no peers - let (mut gs, _, mut queues, topics) = inject_nodes1() + let (mut gs, _, mut receivers, topics) = inject_nodes1() .topics(vec!["test".into()]) .gs_config(config) .scoring(Some((peer_score_params, peer_score_thresholds))) @@ -2847,9 +2937,9 @@ fn test_do_not_flood_publish_to_peer_below_publish_threshold() { //add two additional peers that will be added to the mesh let (p1, receiver1) = add_peer(&mut gs, &topics, false, false); - queues.insert(p1, receiver1); + receivers.insert(p1, receiver1); let (p2, receiver2) = add_peer(&mut gs, &topics, false, false); - queues.insert(p2, receiver2); + receivers.insert(p2, receiver2); //reduce score of p1 below peer_score_thresholds.publish_threshold //note that penalties get squared so two penalties means a score of @@ -2867,16 +2957,22 @@ fn test_do_not_flood_publish_to_peer_below_publish_threshold() { gs.publish(Topic::new("test"), publish_data).unwrap(); // Collect all publish messages - let publishes = queues - .into_iter() - .fold(vec![], |mut collected_publish, (peer_id, c)| { - while !c.priority.is_empty() { - if let Ok(RpcOut::Publish(message)) = c.priority.try_recv() { - collected_publish.push((peer_id, message)) - } + close_senders(&mut gs.connected_peers); + let publishes = stream::select_all( + receivers + .into_iter() + .map(|(peer_id, receiver)| stream::repeat(peer_id).zip(receiver)), + ) + .fold(vec![], |mut collected_publish, (peer_id, e)| async move { + match e { + RpcOut::Publish(message) => { + collected_publish.push((peer_id, message)); + collected_publish } - collected_publish - }); + _ => collected_publish, + } + }) + .await; //assert only published to p2 assert_eq!(publishes.len(), 1); @@ -4334,10 +4430,10 @@ fn test_opportunistic_grafting() { ); } -#[test] -fn test_ignore_graft_from_unknown_topic() { +#[async_std::test] +async fn test_ignore_graft_from_unknown_topic() { //build gossipsub without subscribing to any topics - let (mut gs, _, queues, _) = inject_nodes1() + let (mut gs, _, mut receivers, _) = inject_nodes1() .peer_no(0) .topics(vec![]) .to_subscribe(false) @@ -4348,20 +4444,21 @@ fn test_ignore_graft_from_unknown_topic() { //assert that no prune got created assert_eq!( - count_control_msgs(&gs, &queues, |_, a| matches!( + count_control_msgs(&gs, &mut receivers, |_, a| matches!( a, ControlAction::Prune { .. } - )), + )) + .await, 0, "we should not prune after graft in unknown topic" ); } -#[test] -fn test_ignore_too_many_iwants_from_same_peer_for_same_message() { +#[async_std::test] +async fn test_ignore_too_many_iwants_from_same_peer_for_same_message() { let config = Config::default(); //build gossipsub with full mesh - let (mut gs, _, mut queues, topics) = inject_nodes1() + let (mut gs, _, mut receivers, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4369,7 +4466,7 @@ fn test_ignore_too_many_iwants_from_same_peer_for_same_message() { //add another peer not in the mesh let (peer, receiver) = add_peer(&mut gs, &topics, false, false); - queues.insert(peer, receiver); + receivers.insert(peer, receiver); //receive a message let mut seq = 0; @@ -4383,7 +4480,8 @@ fn test_ignore_too_many_iwants_from_same_peer_for_same_message() { gs.handle_received_message(m1, &PeerId::random()); //clear events - flush_events(&mut gs, &mut queues); + flush_events(&mut gs); + flush_receivers(&mut receivers).await; //the first gossip_retransimission many iwants return the valid message, all others are // ignored. @@ -4391,28 +4489,29 @@ fn test_ignore_too_many_iwants_from_same_peer_for_same_message() { gs.handle_iwant(&peer, vec![id.clone()]); } + close_senders(&mut gs.connected_peers); assert_eq!( - queues.into_values().fold(0, |mut fwds, c| { - while !c.non_priority.is_empty() { - if let Ok(RpcOut::Forward(_)) = c.non_priority.try_recv() { + stream::select_all(receivers.into_values()) + .fold(0, |mut fwds, e| async move { + if let RpcOut::Forward(_) = e { fwds += 1; } - } - fwds - }), + fwds + }) + .await, config.gossip_retransimission() as usize, "not more then gossip_retransmission many messages get sent back" ); } -#[test] -fn test_ignore_too_many_ihaves() { +#[async_std::test] +async fn test_ignore_too_many_ihaves() { let config = ConfigBuilder::default() .max_ihave_messages(10) .build() .unwrap(); //build gossipsub with full mesh - let (mut gs, _, mut queues, topics) = inject_nodes1() + let (mut gs, _, mut receivers, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4421,7 +4520,7 @@ fn test_ignore_too_many_ihaves() { //add another peer not in the mesh let (peer, receiver) = add_peer(&mut gs, &topics, false, false); - queues.insert(peer, receiver); + receivers.insert(peer, receiver); //peer has 20 messages let mut seq = 0; @@ -4450,8 +4549,8 @@ fn test_ignore_too_many_ihaves() { //we send iwant only for the first 10 messages assert_eq!( - count_control_msgs(&gs, &queues, |p, action| p == &peer - && matches!(action, ControlAction::IWant { message_ids } if message_ids.len() == 1 && first_ten.contains(&message_ids[0]))), + count_control_msgs(&gs, &mut receivers, |p, action| p == &peer + && matches!(action, ControlAction::IWant { message_ids } if message_ids.len() == 1 && first_ten.contains(&message_ids[0]))).await, 10, "exactly the first ten ihaves should be processed and one iwant for each created" ); @@ -4473,22 +4572,23 @@ fn test_ignore_too_many_ihaves() { //we sent iwant for all 20 messages assert_eq!( - count_control_msgs(&gs, &queues, |p, action| p == &peer - && matches!(action, ControlAction::IWant { message_ids } if message_ids.len() == 1)), + count_control_msgs(&gs, &mut receivers, |p, action| p == &peer + && matches!(action, ControlAction::IWant { message_ids } if message_ids.len() == 1)) + .await, 20, "all 20 should get sent" ); } -#[test] -fn test_ignore_too_many_messages_in_ihave() { +#[async_std::test] +async fn test_ignore_too_many_messages_in_ihave() { let config = ConfigBuilder::default() .max_ihave_messages(10) .max_ihave_length(10) .build() .unwrap(); //build gossipsub with full mesh - let (mut gs, _, mut queues, topics) = inject_nodes1() + let (mut gs, _, mut receivers, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4497,7 +4597,7 @@ fn test_ignore_too_many_messages_in_ihave() { //add another peer not in the mesh let (peer, receiver) = add_peer(&mut gs, &topics, false, false); - queues.insert(peer, receiver); + receivers.insert(peer, receiver); //peer has 20 messages let mut seq = 0; @@ -4523,7 +4623,7 @@ fn test_ignore_too_many_messages_in_ihave() { //we send iwant only for the first 10 messages let mut sum = 0; assert_eq!( - count_control_msgs(&gs, &queues, |p, action| match action { + count_control_msgs(&gs, &mut receivers, |p, action| match action { ControlAction::IWant { message_ids } => p == &peer && { assert!(first_twelve.is_superset(&message_ids.iter().collect())); @@ -4531,7 +4631,8 @@ fn test_ignore_too_many_messages_in_ihave() { true }, _ => false, - }), + }) + .await, 2, "the third ihave should get ignored and no iwant sent" ); @@ -4548,28 +4649,29 @@ fn test_ignore_too_many_messages_in_ihave() { //we sent 20 iwant messages let mut sum = 0; assert_eq!( - count_control_msgs(&gs, &queues, |p, action| match action { + count_control_msgs(&gs, &mut receivers, |p, action| match action { ControlAction::IWant { message_ids } => p == &peer && { sum += message_ids.len(); true }, _ => false, - }), + }) + .await, 3 ); assert_eq!(sum, 20, "exactly 20 iwants should get sent"); } -#[test] -fn test_limit_number_of_message_ids_inside_ihave() { +#[async_std::test] +async fn test_limit_number_of_message_ids_inside_ihave() { let config = ConfigBuilder::default() .max_ihave_messages(10) .max_ihave_length(100) .build() .unwrap(); //build gossipsub with full mesh - let (mut gs, peers, queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(config.mesh_n_high()) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4602,7 +4704,7 @@ fn test_limit_number_of_message_ids_inside_ihave() { let mut ihaves2 = HashSet::new(); assert_eq!( - count_control_msgs(&gs, &queues, |p, action| match action { + count_control_msgs(&gs, &mut receivers, |p, action| match action { ControlAction::IHave { message_ids, .. } => { if p == &p1 { ihaves1 = message_ids.iter().cloned().collect(); @@ -4615,7 +4717,8 @@ fn test_limit_number_of_message_ids_inside_ihave() { } } _ => false, - }), + }) + .await, 2, "should have emitted one ihave to p1 and one to p2" ); @@ -4765,13 +4868,13 @@ fn test_iwant_penalties() { assert_eq!(double_penalized, 0); } -#[test] -fn test_publish_to_floodsub_peers_without_flood_publish() { +#[async_std::test] +async fn test_publish_to_floodsub_peers_without_flood_publish() { let config = ConfigBuilder::default() .flood_publish(false) .build() .unwrap(); - let (mut gs, _, mut queues, topics) = inject_nodes1() + let (mut gs, _, mut receivers, topics) = inject_nodes1() .peer_no(config.mesh_n_low() - 1) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4787,11 +4890,11 @@ fn test_publish_to_floodsub_peers_without_flood_publish() { Multiaddr::empty(), Some(PeerKind::Floodsub), ); - queues.insert(p1, receiver1); + receivers.insert(p1, receiver1); let (p2, receiver2) = add_peer_with_addr_and_kind(&mut gs, &topics, false, false, Multiaddr::empty(), None); - queues.insert(p2, receiver2); + receivers.insert(p2, receiver2); //p1 and p2 are not in the mesh assert!(!gs.mesh[&topics[0]].contains(&p1) && !gs.mesh[&topics[0]].contains(&p2)); @@ -4801,18 +4904,21 @@ fn test_publish_to_floodsub_peers_without_flood_publish() { gs.publish(Topic::new("test"), publish_data).unwrap(); // Collect publish messages to floodsub peers - let publishes = queues - .iter() - .fold(0, |mut collected_publish, (peer_id, c)| { - while !c.priority.is_empty() { - if matches!(c.priority.try_recv(), - Ok(RpcOut::Publish(_)) if peer_id == &p1 || peer_id == &p2) - { - collected_publish += 1; - } - } - collected_publish - }); + close_senders(&mut gs.connected_peers); + let publishes = stream::select_all( + receivers + .into_iter() + .map(|(peer_id, receiver)| stream::repeat(peer_id).zip(receiver)), + ) + .fold(0, |mut collected_publish, (peer_id, e)| async move { + if matches!(e, + RpcOut::Publish(_) if peer_id == p1 || peer_id == p2) + { + collected_publish += 1; + } + collected_publish + }) + .await; assert_eq!( publishes, 2, @@ -4820,13 +4926,13 @@ fn test_publish_to_floodsub_peers_without_flood_publish() { ); } -#[test] -fn test_do_not_use_floodsub_in_fanout() { +#[async_std::test] +async fn test_do_not_use_floodsub_in_fanout() { let config = ConfigBuilder::default() .flood_publish(false) .build() .unwrap(); - let (mut gs, _, mut queues, _) = inject_nodes1() + let (mut gs, _, mut receivers, _) = inject_nodes1() .peer_no(config.mesh_n_low() - 1) .topics(Vec::new()) .to_subscribe(false) @@ -4846,28 +4952,31 @@ fn test_do_not_use_floodsub_in_fanout() { Some(PeerKind::Floodsub), ); - queues.insert(p1, receiver1); + receivers.insert(p1, receiver1); let (p2, receiver2) = add_peer_with_addr_and_kind(&mut gs, &topics, false, false, Multiaddr::empty(), None); - queues.insert(p2, receiver2); + receivers.insert(p2, receiver2); //publish a message let publish_data = vec![0; 42]; gs.publish(Topic::new("test"), publish_data).unwrap(); // Collect publish messages to floodsub peers - let publishes = queues - .iter() - .fold(0, |mut collected_publish, (peer_id, c)| { - while !c.priority.is_empty() { - if matches!(c.priority.try_recv(), - Ok(RpcOut::Publish(_)) if peer_id == &p1 || peer_id == &p2) - { - collected_publish += 1; - } - } - collected_publish - }); + close_senders(&mut gs.connected_peers); + let publishes = stream::select_all( + receivers + .into_iter() + .map(|(peer_id, receiver)| stream::repeat(peer_id).zip(receiver)), + ) + .fold(0, |mut collected_publish, (peer_id, e)| async move { + if matches!(e, + RpcOut::Publish(_) if peer_id == p1 || peer_id == p2) + { + collected_publish += 1; + } + collected_publish + }) + .await; assert_eq!( publishes, 2, @@ -4910,9 +5019,9 @@ fn test_dont_add_floodsub_peers_to_mesh_on_join() { ); } -#[test] -fn test_dont_send_px_to_old_gossipsub_peers() { - let (mut gs, _, queues, topics) = inject_nodes1() +#[async_std::test] +async fn test_dont_send_px_to_old_gossipsub_peers() { + let (mut gs, _, mut receivers, topics) = inject_nodes1() .peer_no(0) .topics(vec!["test".into()]) .to_subscribe(false) @@ -4937,19 +5046,20 @@ fn test_dont_send_px_to_old_gossipsub_peers() { //check that prune does not contain px assert_eq!( - count_control_msgs(&gs, &queues, |_, m| match m { + count_control_msgs(&gs, &mut receivers, |_, m| match m { ControlAction::Prune { peers: px, .. } => !px.is_empty(), _ => false, - }), + }) + .await, 0, "Should not send px to floodsub peers" ); } -#[test] -fn test_dont_send_floodsub_peers_in_px() { +#[async_std::test] +async fn test_dont_send_floodsub_peers_in_px() { //build mesh with one peer - let (mut gs, peers, queues, topics) = inject_nodes1() + let (mut gs, peers, mut receivers, topics) = inject_nodes1() .peer_no(1) .topics(vec!["test".into()]) .to_subscribe(true) @@ -4975,10 +5085,11 @@ fn test_dont_send_floodsub_peers_in_px() { //check that px in prune message is empty assert_eq!( - count_control_msgs(&gs, &queues, |_, m| match m { + count_control_msgs(&gs, &mut receivers, |_, m| match m { ControlAction::Prune { peers: px, .. } => !px.is_empty(), _ => false, - }), + }) + .await, 0, "Should not include floodsub peers in px" ); @@ -5057,8 +5168,8 @@ fn test_subscribe_to_invalid_topic() { assert!(gs.subscribe(&t2).is_err()); } -#[test] -fn test_subscribe_and_graft_with_negative_score() { +#[async_std::test] +async fn test_subscribe_and_graft_with_negative_score() { //simulate a communication between two gossipsub instances let (mut gs1, _, _, topic_hashes) = inject_nodes1() .topics(vec!["test".into()]) @@ -5068,7 +5179,7 @@ fn test_subscribe_and_graft_with_negative_score() { ))) .create_network(); - let (mut gs2, _, queues, _) = inject_nodes1().create_network(); + let (mut gs2, _, mut receivers, _) = inject_nodes1().create_network(); let connection_id = ConnectionId::new_unchecked(0); @@ -5085,37 +5196,43 @@ fn test_subscribe_and_graft_with_negative_score() { //subscribe to topic in gs2 gs2.subscribe(&topic).unwrap(); - let forward_messages_to_p1 = |gs1: &mut Behaviour<_, _>, _gs2: &mut Behaviour<_, _>| { - //collect messages to p1 - let messages_to_p1 = - queues - .iter() - .filter_map(|(peer_id, c)| match c.non_priority.try_recv() { - Ok(rpc) if peer_id == &p1 => Some(rpc), - _ => None, - }); - - for message in messages_to_p1 { - gs1.on_connection_handler_event( - p2, - connection_id, - HandlerEvent::Message { - rpc: proto_to_message(&message.into_protobuf()), - invalid_messages: vec![], - }, - ); - } - }; - //forward the subscribe message - forward_messages_to_p1(&mut gs1, &mut gs2); + for (peer_id, r) in receivers.iter_mut() { + while !poll_fn(|cx| Poll::Ready(r.poll_is_empty(cx))).await { + if peer_id == &p1 { + let rpc = r.next().await.unwrap(); + gs1.on_connection_handler_event( + p2, + connection_id, + HandlerEvent::Message { + rpc: proto_to_message(&rpc.into_protobuf()), + invalid_messages: vec![], + }, + ); + } + } + } //heartbeats on both gs1.heartbeat(); gs2.heartbeat(); //forward messages again - forward_messages_to_p1(&mut gs1, &mut gs2); + for (peer_id, r) in receivers.iter_mut() { + while !poll_fn(|cx| Poll::Ready(r.poll_is_empty(cx))).await { + if peer_id == &p1 { + let rpc = r.next().await.unwrap(); + gs1.on_connection_handler_event( + p2, + connection_id, + HandlerEvent::Message { + rpc: proto_to_message(&rpc.into_protobuf()), + invalid_messages: vec![], + }, + ); + } + } + } //nobody got penalized assert!(gs1.peer_score.as_ref().unwrap().0.score(&p2) >= original_score); diff --git a/protocols/gossipsub/src/handler.rs b/protocols/gossipsub/src/handler.rs index dd048c7e2fd..ec67f2adf6f 100644 --- a/protocols/gossipsub/src/handler.rs +++ b/protocols/gossipsub/src/handler.rs @@ -229,7 +229,7 @@ impl EnabledHandler { } // determine if we need to create the outbound stream - if !self.send_queue.is_empty() + if !self.send_queue.poll_is_empty(cx) && self.outbound_substream.is_none() && !self.outbound_substream_establishing { diff --git a/protocols/gossipsub/src/types.rs b/protocols/gossipsub/src/types.rs index 6799b6a2b15..1f8e2b26eb8 100644 --- a/protocols/gossipsub/src/types.rs +++ b/protocols/gossipsub/src/types.rs @@ -21,16 +21,16 @@ //! A collection of types using the Gossipsub system. use crate::metrics::Metrics; use crate::TopicHash; -use async_channel::{Receiver, Sender}; +use futures::channel::mpsc::{Receiver, Sender}; +use futures::stream::Peekable; use futures::Stream; use libp2p_identity::PeerId; use libp2p_swarm::ConnectionId; use prometheus_client::encoding::EncodeLabelValue; use quick_protobuf::MessageWrite; +use std::collections::HashMap; use std::fmt::Debug; -use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::Arc; -use std::task::Poll; +use std::task::{Context, Poll}; use std::{fmt, pin::Pin}; use crate::rpc_proto::proto; @@ -77,12 +77,115 @@ impl std::fmt::Debug for MessageId { } } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone)] pub(crate) struct PeerConnections { /// The kind of protocol the peer supports. pub(crate) kind: PeerKind, /// Its current connections. - pub(crate) connections: Vec, + pub(crate) connections: HashMap, +} + +impl PeerConnections { + /// Send a `RpcOut::Control` message to the `RpcReceiver` + /// this is high priority. By cloning `futures::channel::mpsc::Sender` + /// we get one extra slot in the channel's capacity. + pub(crate) fn control(&mut self, control: ControlAction) { + let mut rpc = RpcOut::Control(control); + for sender in self.connections.values_mut() { + match sender.priority.clone().try_send(rpc) { + Ok(_) => { + return; + } + Err(err) => { + rpc = err.into_inner(); + } + } + } + } + + /// Send a `RpcOut::Subscribe` message to the `RpcReceiver` + /// this is high priority. By cloning `futures::channel::mpsc::Sender` + /// we get one extra slot in the channel's capacity. + pub(crate) fn subscribe(&mut self, topic: TopicHash) { + let mut rpc = RpcOut::Subscribe(topic); + for sender in self.connections.values_mut() { + match sender.priority.clone().try_send(rpc) { + Ok(_) => { + return; + } + Err(err) => { + rpc = err.into_inner(); + } + } + } + } + + /// Send a `RpcOut::Unsubscribe` message to the `RpcReceiver` + /// this is high priority. By cloning `futures::channel::mpsc::Sender` + /// we get one extra slot in the channel's capacity. + pub(crate) fn unsubscribe(&mut self, topic: TopicHash) { + let mut rpc = RpcOut::Unsubscribe(topic); + for sender in self.connections.values_mut() { + match sender.priority.clone().try_send(rpc) { + Ok(_) => { + return; + } + Err(err) => { + rpc = err.into_inner(); + } + } + } + } + + /// Send a `RpcOut::Publish` message to the `RpcReceiver` + /// this is high priority. If message sending fails, an `Err` is returned. + pub(crate) fn publish( + &mut self, + message: RawMessage, + metrics: Option<&mut Metrics>, + ) -> Result<(), ()> { + let mut rpc = RpcOut::Publish(message.clone()); + for sender in self.connections.values_mut() { + match sender.priority.try_send(rpc) { + Ok(_) => { + if let Some(m) = metrics { + m.msg_sent(&message.topic, message.raw_protobuf_len()); + } + return Ok(()); + } + Err(err) if err.is_full() => return Err(()), + // Channel is closed, try another sender. + Err(err) => { + rpc = err.into_inner(); + } + } + } + unreachable!("At least one peer should be available"); + } + + /// Send a `RpcOut::Forward` message to the `RpcReceiver` + /// this is low priority. If the queue is full the message is discarded. + pub(crate) fn forward(&mut self, message: RawMessage, metrics: Option<&mut Metrics>) { + let mut rpc = RpcOut::Forward(message.clone()); + for sender in self.connections.values_mut() { + match sender.non_priority.try_send(rpc) { + Ok(_) => { + if let Some(m) = metrics { + m.msg_sent(&message.topic, message.raw_protobuf_len()); + } + return; + } + Err(err) if err.is_full() => { + tracing::trace!("Queue is full, dropped Forward message"); + return; + } + // Channel is closed, try another sender. + Err(err) => { + rpc = err.into_inner(); + } + } + } + } } /// Describes the types of peers that can exist in the gossipsub context. @@ -522,117 +625,26 @@ impl fmt::Display for PeerKind { /// `RpcOut` sender that is priority aware. #[derive(Debug, Clone)] pub(crate) struct RpcSender { - peer_id: PeerId, - cap: usize, - len: Arc, - priority: Sender, - non_priority: Sender, - receiver: RpcReceiver, -} - -impl RpcSender { - /// Create a RpcSender. - pub(crate) fn new(peer_id: PeerId, cap: usize) -> RpcSender { - let (priority_sender, priority_receiver) = async_channel::unbounded(); - let (non_priority_sender, non_priority_receiver) = async_channel::bounded(cap / 2); - let len = Arc::new(AtomicUsize::new(0)); - let receiver = RpcReceiver { - len: len.clone(), - priority: priority_receiver, - non_priority: non_priority_receiver, - }; - RpcSender { - peer_id, - cap: cap / 2, - len, - priority: priority_sender, - non_priority: non_priority_sender, - receiver: receiver.clone(), - } - } - - /// Create a new Receiver to the sender. - pub(crate) fn new_receiver(&self) -> RpcReceiver { - self.receiver.clone() - } - - /// Send a `RpcOut::Control` message to the `RpcReceiver` - /// this is high priority. - pub(crate) fn control(&mut self, control: ControlAction) { - self.priority - .try_send(RpcOut::Control(control)) - .expect("Channel is unbounded and should always be open"); - } - - /// Send a `RpcOut::Subscribe` message to the `RpcReceiver` - /// this is high priority. - pub(crate) fn subscribe(&mut self, topic: TopicHash) { - self.priority - .try_send(RpcOut::Subscribe(topic)) - .expect("Channel is unbounded and should always be open"); - } - - /// Send a `RpcOut::Unsubscribe` message to the `RpcReceiver` - /// this is high priority. - pub(crate) fn unsubscribe(&mut self, topic: TopicHash) { - self.priority - .try_send(RpcOut::Unsubscribe(topic)) - .expect("Channel is unbounded and should always be open"); - } - - /// Send a `RpcOut::Publish` message to the `RpcReceiver` - /// this is high priority. If message sending fails, an `Err` is returned. - pub(crate) fn publish( - &mut self, - message: RawMessage, - metrics: Option<&mut Metrics>, - ) -> Result<(), ()> { - if self.len.load(Ordering::Relaxed) >= self.cap { - return Err(()); - } - self.priority - .try_send(RpcOut::Publish(message.clone())) - .expect("Channel is unbounded and Should always be open"); - self.len.fetch_add(1, Ordering::Relaxed); - - if let Some(m) = metrics { - m.msg_sent(&message.topic, message.raw_protobuf_len()); - } - - Ok(()) - } - - /// Send a `RpcOut::Forward` message to the `RpcReceiver` - /// this is high priority. If the queue is full the message is discarded. - pub(crate) fn forward(&mut self, message: RawMessage, metrics: Option<&mut Metrics>) { - if let Err(err) = self.non_priority.try_send(RpcOut::Forward(message.clone())) { - let rpc = err.into_inner(); - tracing::trace!( - "{:?} message to peer {} dropped, queue is full", - rpc, - self.peer_id - ); - return; - } - - if let Some(m) = metrics { - m.msg_sent(&message.topic, message.raw_protobuf_len()); - } - } + pub(crate) priority: Sender, + pub(crate) non_priority: Sender, } /// `RpcOut` sender that is priority aware. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct RpcReceiver { - len: Arc, - pub(crate) priority: Receiver, - pub(crate) non_priority: Receiver, + pub(crate) priority: Peekable>, + pub(crate) non_priority: Peekable>, } impl RpcReceiver { - /// Check if both queues are empty. - pub(crate) fn is_empty(&self) -> bool { - self.priority.is_empty() && self.non_priority.is_empty() + pub(crate) fn poll_is_empty(&mut self, cx: &mut Context<'_>) -> bool { + if let Poll::Ready(Some(_)) = Pin::new(&mut self.priority).poll_peek(cx) { + return false; + } + if let Poll::Ready(Some(_)) = Pin::new(&mut self.non_priority).poll_peek(cx) { + return false; + } + true } } @@ -644,11 +656,8 @@ impl Stream for RpcReceiver { cx: &mut std::task::Context<'_>, ) -> std::task::Poll> { // The priority queue is first polled. - if let Poll::Ready(rpc) = Pin::new(&mut self.priority).poll_next(cx) { - if let Some(RpcOut::Publish(_)) = rpc { - self.len.fetch_sub(1, Ordering::Relaxed); - } - return Poll::Ready(rpc); + if let Poll::Ready(Some(rpc)) = Pin::new(&mut self.priority).poll_next(cx) { + return Poll::Ready(Some(rpc)); } // Then we poll the non priority. Pin::new(&mut self.non_priority).poll_next(cx) From 8afbc4fbaceff3620e8e940c1a123cb5d6bcab73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Tue, 12 Dec 2023 08:55:13 +0000 Subject: [PATCH 08/11] address review --- Cargo.lock | 1 - protocols/gossipsub/Cargo.toml | 1 - protocols/gossipsub/src/behaviour.rs | 10 ++-------- protocols/gossipsub/src/types.rs | 2 +- 4 files changed, 3 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2d72dcd555d..8fef5c62105 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2773,7 +2773,6 @@ dependencies = [ name = "libp2p-gossipsub" version = "0.46.1" dependencies = [ - "async-channel", "async-std", "asynchronous-codec", "base64 0.21.5", diff --git a/protocols/gossipsub/Cargo.toml b/protocols/gossipsub/Cargo.toml index e0d37819ebe..4c524cc5a10 100644 --- a/protocols/gossipsub/Cargo.toml +++ b/protocols/gossipsub/Cargo.toml @@ -37,7 +37,6 @@ sha2 = "0.10.8" smallvec = "1.11.2" tracing = "0.1.37" void = "1.0.2" -async-channel = "1.9.0" # Metrics dependencies prometheus-client = { workspace = true } diff --git a/protocols/gossipsub/src/behaviour.rs b/protocols/gossipsub/src/behaviour.rs index c591ebb4d4c..e687d033bef 100644 --- a/protocols/gossipsub/src/behaviour.rs +++ b/protocols/gossipsub/src/behaviour.rs @@ -336,9 +336,6 @@ pub struct Behaviour { /// Keep track of a set of internal metrics relating to gossipsub. metrics: Option, - - /// Connection handler message queue channels. - handler_send_queues: HashMap, } impl Behaviour @@ -478,7 +475,6 @@ where config, subscription_filter, data_transform, - handler_send_queues: Default::default(), }) } } @@ -2839,11 +2835,10 @@ where tracing::debug!(peer=%peer_id, "New peer connected"); // We need to send our subscriptions to the newly-connected node. - let mut sender = self + let sender = self .connected_peers .get_mut(&peer_id) - .expect("Peerid should exist") - .clone(); + .expect("Peerid should exist"); for topic_hash in self.mesh.clone().into_keys() { sender.subscribe(topic_hash); @@ -2969,7 +2964,6 @@ where } self.connected_peers.remove(&peer_id); - self.handler_send_queues.remove(&peer_id); if let Some((peer_score, ..)) = &mut self.peer_score { peer_score.remove_peer(&peer_id); diff --git a/protocols/gossipsub/src/types.rs b/protocols/gossipsub/src/types.rs index 1f8e2b26eb8..98c4e12ebfe 100644 --- a/protocols/gossipsub/src/types.rs +++ b/protocols/gossipsub/src/types.rs @@ -629,7 +629,7 @@ pub(crate) struct RpcSender { pub(crate) non_priority: Sender, } -/// `RpcOut` sender that is priority aware. +/// `RpcOut` receiver that is priority aware. #[derive(Debug)] pub struct RpcReceiver { pub(crate) priority: Peekable>, From 946e1d505280c439a6029b21965de6726c66989d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Thu, 22 Aug 2024 17:28:44 +0100 Subject: [PATCH 09/11] improve wording --- protocols/gossipsub/CHANGELOG.md | 2 +- protocols/gossipsub/src/config.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/protocols/gossipsub/CHANGELOG.md b/protocols/gossipsub/CHANGELOG.md index 25c087d412f..ba86367872c 100644 --- a/protocols/gossipsub/CHANGELOG.md +++ b/protocols/gossipsub/CHANGELOG.md @@ -1,5 +1,5 @@ ## 0.47.1 -- Implement backpressure by diferentiating between priority and non priority messages. +- Implement backpressure by differentiating between priority and non priority messages. Drop `Publish` and `Forward` messages when the queue becomes full. See [PR 4914](https://github.com/libp2p/rust-libp2p/pull/4914) diff --git a/protocols/gossipsub/src/config.rs b/protocols/gossipsub/src/config.rs index a7dec9f4f9b..bd0df9201d7 100644 --- a/protocols/gossipsub/src/config.rs +++ b/protocols/gossipsub/src/config.rs @@ -352,7 +352,7 @@ impl Config { self.published_message_ids_cache_time } - /// The max number of messages a `ConnectionHandler` can buffer. + /// The max number of messages a `ConnectionHandler` can buffer. The default is 1000. pub fn connection_handler_queue_len(&self) -> usize { self.connection_handler_queue_len } @@ -423,7 +423,7 @@ impl Default for ConfigBuilder { max_ihave_messages: 10, iwant_followup_time: Duration::from_secs(3), published_message_ids_cache_time: Duration::from_secs(10), - connection_handler_queue_len: 100, + connection_handler_queue_len: 1000, }, invalid_protocol: false, } @@ -789,6 +789,7 @@ impl ConfigBuilder { self } + /// The max number of messages a `ConnectionHandler` can buffer. The default is 5000. pub fn connection_handler_queue_len(&mut self, len: usize) { self.config.connection_handler_queue_len = len; } From 863ba9f1049003477e10d288cd58c101b0c2a61f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Thu, 22 Aug 2024 17:29:59 +0100 Subject: [PATCH 10/11] clippy --- protocols/gossipsub/src/behaviour/tests.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/protocols/gossipsub/src/behaviour/tests.rs b/protocols/gossipsub/src/behaviour/tests.rs index 83289496ab1..7920d9ec69b 100644 --- a/protocols/gossipsub/src/behaviour/tests.rs +++ b/protocols/gossipsub/src/behaviour/tests.rs @@ -4164,20 +4164,20 @@ fn test_scoring_p6() { //create 5 peers with the same ip let addr = Multiaddr::from(Ipv4Addr::new(10, 1, 2, 3)); let peers = vec![ - add_peer_with_addr(&mut gs, &vec![], false, false, addr.clone()).0, - add_peer_with_addr(&mut gs, &vec![], false, false, addr.clone()).0, - add_peer_with_addr(&mut gs, &vec![], true, false, addr.clone()).0, - add_peer_with_addr(&mut gs, &vec![], true, false, addr.clone()).0, - add_peer_with_addr(&mut gs, &vec![], true, true, addr.clone()).0, + add_peer_with_addr(&mut gs, &[], false, false, addr.clone()).0, + add_peer_with_addr(&mut gs, &[], false, false, addr.clone()).0, + add_peer_with_addr(&mut gs, &[], true, false, addr.clone()).0, + add_peer_with_addr(&mut gs, &[], true, false, addr.clone()).0, + add_peer_with_addr(&mut gs, &[], true, true, addr.clone()).0, ]; //create 4 other peers with other ip let addr2 = Multiaddr::from(Ipv4Addr::new(10, 1, 2, 4)); let others = vec![ - add_peer_with_addr(&mut gs, &vec![], false, false, addr2.clone()).0, - add_peer_with_addr(&mut gs, &vec![], false, false, addr2.clone()).0, - add_peer_with_addr(&mut gs, &vec![], true, false, addr2.clone()).0, - add_peer_with_addr(&mut gs, &vec![], true, false, addr2.clone()).0, + add_peer_with_addr(&mut gs, &[], false, false, addr2.clone()).0, + add_peer_with_addr(&mut gs, &[], false, false, addr2.clone()).0, + add_peer_with_addr(&mut gs, &[], true, false, addr2.clone()).0, + add_peer_with_addr(&mut gs, &[], true, false, addr2.clone()).0, ]; //no penalties yet From 1072cc722cb464b5c6b8be97fed161c7794b507e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Oliveira?= Date: Thu, 22 Aug 2024 17:58:23 +0100 Subject: [PATCH 11/11] update cargo.toml --- Cargo.lock | 2 +- Cargo.toml | 2 +- protocols/gossipsub/Cargo.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8932f40a04c..34c96c38b9f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2838,7 +2838,7 @@ dependencies = [ [[package]] name = "libp2p-gossipsub" -version = "0.47.0" +version = "0.47.1" dependencies = [ "async-std", "asynchronous-codec", diff --git a/Cargo.toml b/Cargo.toml index 31c3a8e4b9e..f4a4e2e9897 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -83,7 +83,7 @@ libp2p-core = { version = "0.42.0", path = "core" } libp2p-dcutr = { version = "0.12.0", path = "protocols/dcutr" } libp2p-dns = { version = "0.42.0", path = "transports/dns" } libp2p-floodsub = { version = "0.45.0", path = "protocols/floodsub" } -libp2p-gossipsub = { version = "0.47.0", path = "protocols/gossipsub" } +libp2p-gossipsub = { version = "0.47.1", path = "protocols/gossipsub" } libp2p-identify = { version = "0.45.0", path = "protocols/identify" } libp2p-identity = { version = "0.2.9" } libp2p-kad = { version = "0.46.1", path = "protocols/kad" } diff --git a/protocols/gossipsub/Cargo.toml b/protocols/gossipsub/Cargo.toml index 2fdff3ac351..3d0b8fddc22 100644 --- a/protocols/gossipsub/Cargo.toml +++ b/protocols/gossipsub/Cargo.toml @@ -3,7 +3,7 @@ name = "libp2p-gossipsub" edition = "2021" rust-version = { workspace = true } description = "Gossipsub protocol for libp2p" -version = "0.47.0" +version = "0.47.1" authors = ["Age Manning "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p"