From f343ebbb8c9f71b1e091e105d827111f9e8f1531 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Thu, 2 Jun 2022 07:40:34 +0530 Subject: [PATCH 01/15] GH-574: add todo for the fix of the failing test in GH Actions --- node/src/neighborhood/overall_connection_status.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index 507eeb4b4..5af763d9a 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -188,6 +188,7 @@ impl OverallConnectionStatus { event: ConnectionProgressEvent, node_to_ui_recipient: &Recipient, ) { + // TODO: This should return a Result of ConnectionProgress let connection_progress_to_modify = self.get_connection_progress_by_ip(peer_addr); let mut modify_connection_progress = From df8071edd77abdf3f32d9b0c2063170e8c027b47 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Mon, 13 Jun 2022 13:50:37 +0530 Subject: [PATCH 02/15] GH-574: change get_connection_progress_by_ip() to return a Result --- node/src/neighborhood/mod.rs | 2 + .../neighborhood/overall_connection_status.rs | 59 ++++++++++++------- 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index be4e0b0cc..c4d248b4e 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -1171,7 +1171,9 @@ impl Neighborhood { prev_connection_progress: self .overall_connection_status .get_connection_progress_by_ip(current_peer_addr) + .unwrap() .clone(), + // TODO: Do Something with the error - "Cannot send AskAboutDebutGossipMessage for peer with IP Address: {}" }; self.tools.notify_later_ask_about_gossip.notify_later( message, diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index 5af763d9a..b12812bfc 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -8,6 +8,7 @@ use actix::Recipient; use masq_lib::messages::{ToMessageBody, UiConnectionChangeBroadcast, UiConnectionChangeStage}; use masq_lib::ui_gateway::{MessageTarget, NodeToUiMessage}; use std::net::IpAddr; +use std::string::String; #[derive(PartialEq, Debug, Clone)] pub enum ConnectionStageErrors { @@ -147,19 +148,22 @@ impl OverallConnectionStatus { .map(|connection_progress| &connection_progress.initial_node_descriptor) } - pub fn get_connection_progress_by_ip(&mut self, peer_addr: IpAddr) -> &mut ConnectionProgress { - let connection_progress_to_modify = self + pub fn get_connection_progress_by_ip( + &mut self, + peer_addr: IpAddr, + ) -> Result<&mut ConnectionProgress, String> { + let connection_progress_res = self .progress .iter_mut() - .find(|connection_progress| connection_progress.current_peer_addr == peer_addr) - .unwrap_or_else(|| { - panic!( - "Unable to find the Node in connections with IP Address: {}", - peer_addr - ) - }); - - connection_progress_to_modify + .find(|connection_progress| connection_progress.current_peer_addr == peer_addr); + + match connection_progress_res { + Some(connection_progress) => Ok(connection_progress), + None => Err(format!( + "Unable to find the Node in connections with IP Address: {}", + peer_addr + )), + } } pub fn get_connection_progress_by_desc( @@ -188,8 +192,8 @@ impl OverallConnectionStatus { event: ConnectionProgressEvent, node_to_ui_recipient: &Recipient, ) { - // TODO: This should return a Result of ConnectionProgress - let connection_progress_to_modify = self.get_connection_progress_by_ip(peer_addr); + let connection_progress_to_modify = self.get_connection_progress_by_ip(peer_addr).unwrap(); + // TODO: Do Something with the error - "Cannot update stage for the peer addr: {}" let mut modify_connection_progress = |stage: ConnectionStage| connection_progress_to_modify.update_stage(stage); @@ -383,7 +387,7 @@ mod tests { } #[test] - fn can_receive_mut_ref_of_connection_progress_from_peer_addr() { + fn can_recieve_a_result_of_connection_progress_from_peer_addr() { let peer_1_ip = make_ip(1); let peer_2_ip = make_ip(2); let desc_1 = make_node_descriptor(peer_1_ip); @@ -392,13 +396,28 @@ mod tests { let mut subject = OverallConnectionStatus::new(initial_node_descriptors); + let res_1 = subject.get_connection_progress_by_ip(peer_1_ip); + assert_eq!(res_1, Ok(&mut ConnectionProgress::new(desc_1))); + let res_2 = subject.get_connection_progress_by_ip(peer_2_ip); + assert_eq!(res_2, Ok(&mut ConnectionProgress::new(desc_2))); + } + + #[test] + fn receives_an_error_in_receiving_connection_progress_from_unknown_ip_address() { + let peer = make_ip(1); + let desc = make_node_descriptor(peer); + let initial_node_descriptors = vec![desc]; + let unknown_peer = make_ip(2); + + let mut subject = OverallConnectionStatus::new(initial_node_descriptors); + + let res = subject.get_connection_progress_by_ip(unknown_peer); assert_eq!( - subject.get_connection_progress_by_ip(peer_1_ip), - &mut ConnectionProgress::new(desc_1) - ); - assert_eq!( - subject.get_connection_progress_by_ip(peer_2_ip), - &mut ConnectionProgress::new(desc_2) + res, + Err(format!( + "Unable to find the Node in connections with IP Address: {}", + unknown_peer + )) ); } From 3437c8d08ea7c9c828911b9bb685796480d4e59d Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Wed, 15 Jun 2022 10:47:42 +0530 Subject: [PATCH 03/15] GH-574: migrate updation of OCS Stage from update_connection_stage() to handler for ConnectionProgressMessage --- node/src/neighborhood/mod.rs | 152 ++++++++++----- .../neighborhood/overall_connection_status.rs | 176 ++++++++---------- 2 files changed, 186 insertions(+), 142 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index c4d248b4e..46e00fc2c 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -34,7 +34,7 @@ use crate::db_config::persistent_configuration::{ use crate::neighborhood::gossip::{DotGossipEndpoint, GossipNodeRecord, Gossip_0v1}; use crate::neighborhood::gossip_acceptor::GossipAcceptanceResult; use crate::neighborhood::node_record::NodeRecordInner_0v1; -use crate::neighborhood::overall_connection_status::OverallConnectionStatus; +use crate::neighborhood::overall_connection_status::{ConnectionProgress, OverallConnectionStatus}; use crate::stream_messages::RemovedStreamType; use crate::sub_lib::configurator::NewPasswordMessage; use crate::sub_lib::cryptde::PublicKey; @@ -259,16 +259,33 @@ impl Handler for Neighborhood { type Result = (); fn handle(&mut self, msg: ConnectionProgressMessage, ctx: &mut Self::Context) -> Self::Result { - self.overall_connection_status.update_connection_stage( - msg.peer_addr, - msg.event.clone(), - self.node_to_ui_recipient_opt - .as_ref() - .expect("UI Gateway is unbound"), - ); + eprintln!("Handling the Connection Progress Message"); + if let Ok(connection_progress) = self + .overall_connection_status + .get_connection_progress_by_ip(msg.peer_addr) + { + OverallConnectionStatus::update_connection_stage( + connection_progress, + msg.event.clone(), + ); - if msg.event == ConnectionProgressEvent::TcpConnectionSuccessful { - self.send_ask_about_debut_gossip_message(ctx, msg.peer_addr); + eprintln!("Entering match statement"); + match msg.event { + ConnectionProgressEvent::TcpConnectionSuccessful => { + self.send_ask_about_debut_gossip_message(ctx, msg.peer_addr); + } + ConnectionProgressEvent::IntroductionGossipReceived(_) + | ConnectionProgressEvent::StandardGossipReceived => { + eprintln!("Yes, I was called"); + self.overall_connection_status + .update_stage_of_overall_connection_status( + self.node_to_ui_recipient_opt + .as_ref() + .expect("UI Gateway is unbound"), + ); + } + _ => (), + } } } } @@ -281,20 +298,21 @@ impl Handler for Neighborhood { msg: AskAboutDebutGossipMessage, _ctx: &mut Self::Context, ) -> Self::Result { - let new_connection_progress = self - .overall_connection_status - .get_connection_progress_by_desc(&msg.prev_connection_progress.initial_node_descriptor); - - if msg.prev_connection_progress == *new_connection_progress { - // No change, hence no response was received - self.overall_connection_status.update_connection_stage( - msg.prev_connection_progress.current_peer_addr, - ConnectionProgressEvent::NoGossipResponseReceived, - self.node_to_ui_recipient_opt - .as_ref() - .expect("UI Gateway is unbound"), - ); - } + unimplemented!(); + // let new_connection_progress = self + // .overall_connection_status + // .get_connection_progress_by_desc(&msg.prev_connection_progress.initial_node_descriptor); + // + // if msg.prev_connection_progress == *new_connection_progress { + // // No change, hence no response was received + // self.overall_connection_status.update_connection_stage( + // msg.prev_connection_progress.current_peer_addr, + // ConnectionProgressEvent::NoGossipResponseReceived, + // self.node_to_ui_recipient_opt + // .as_ref() + // .expect("UI Gateway is unbound"), + // ); + // } } } @@ -1376,8 +1394,8 @@ mod tests { use crate::test_utils::make_meaningless_route; use crate::test_utils::make_wallet; use crate::test_utils::neighborhood_test_utils::{ - db_from_node, make_global_cryptde_node_record, make_node_descriptor, make_node_record, - make_node_record_f, make_node_to_ui_recipient, neighborhood_from_nodes, + db_from_node, make_global_cryptde_node_record, make_ip, make_node_descriptor, + make_node_record, make_node_record_f, make_node_to_ui_recipient, neighborhood_from_nodes, }; use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock; use crate::test_utils::rate_pack; @@ -1636,6 +1654,34 @@ mod tests { ); } + #[test] + pub fn neighborhood_doesn_t_do_anything_if_it_receives_a_cpm_with_an_unknown_peer_addr() { + let known_peer = make_ip(1); + let unknown_peer = make_ip(2); + let node_descriptor = make_node_descriptor(known_peer); + let mut subject = make_subject_from_node_descriptor( + &node_descriptor, + "neighborhood_doesn_t_do_anything_if_it_receives_a_cpm_with_an_unknown_peer_addr", + ); + let initial_ocs = subject.overall_connection_status.clone(); + let addr = subject.start(); + let cpm_recipient = addr.clone().recipient::(); + let system = System::new("testing"); + let cpm = ConnectionProgressMessage { + peer_addr: unknown_peer, + event: ConnectionProgressEvent::TcpConnectionSuccessful, + }; + + cpm_recipient.try_send(cpm).unwrap(); + + let assertions = Box::new(move |actor: &mut Neighborhood| { + assert_eq!(actor.overall_connection_status, initial_ocs); + }); + addr.try_send(AssertionsMessage { assertions }).unwrap(); + System::current().stop(); + assert_eq!(system.run(), 0); + } + #[test] pub fn neighborhood_handles_connection_progress_message_with_tcp_connection_established() { init_test_logging(); @@ -1698,10 +1744,13 @@ mod tests { &node_descriptor, "ask_about_debut_gossip_message_handles_timeout_in_case_no_response_is_received", ); - subject.overall_connection_status.update_connection_stage( - node_ip_addr, + let connection_progress_to_modify = subject + .overall_connection_status + .get_connection_progress_by_ip(node_ip_addr) + .unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionSuccessful, - subject.node_to_ui_recipient_opt.as_ref().unwrap(), ); let beginning_connection_progress = ConnectionProgress { initial_node_descriptor: node_descriptor.clone(), @@ -1775,10 +1824,13 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_pass_gossip_received", ); - subject.overall_connection_status.update_connection_stage( - node_ip_addr, + let connection_progress_to_modify = subject + .overall_connection_status + .get_connection_progress_by_ip(node_ip_addr) + .unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionSuccessful, - subject.node_to_ui_recipient_opt.as_ref().unwrap(), ); let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); @@ -1815,10 +1867,13 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_pass_loop_found", ); - subject.overall_connection_status.update_connection_stage( - node_ip_addr, + let connection_progress_to_modify = subject + .overall_connection_status + .get_connection_progress_by_ip(node_ip_addr) + .unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionSuccessful, - subject.node_to_ui_recipient_opt.as_ref().unwrap(), ); let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); @@ -1854,10 +1909,13 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_introduction_gossip_received", ); - subject.overall_connection_status.update_connection_stage( - node_ip_addr, + let connection_progress_to_modify = subject + .overall_connection_status + .get_connection_progress_by_ip(node_ip_addr) + .unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionSuccessful, - subject.node_to_ui_recipient_opt.as_ref().unwrap(), ); let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); @@ -1894,10 +1952,13 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_standard_gossip_received", ); - subject.overall_connection_status.update_connection_stage( - node_ip_addr, + let connection_progress_to_modify = subject + .overall_connection_status + .get_connection_progress_by_ip(node_ip_addr) + .unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionSuccessful, - subject.node_to_ui_recipient_opt.as_ref().unwrap(), ); let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); @@ -1933,10 +1994,13 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_no_gossip_response_received", ); - subject.overall_connection_status.update_connection_stage( - node_ip_addr, + let connection_progress_to_modify = subject + .overall_connection_status + .get_connection_progress_by_ip(node_ip_addr) + .unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionSuccessful, - subject.node_to_ui_recipient_opt.as_ref().unwrap(), ); let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index b12812bfc..1729e439d 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -118,7 +118,7 @@ impl From for UiConnectionChangeStage { } } -#[derive(PartialEq, Debug)] +#[derive(PartialEq, Debug, Clone)] pub struct OverallConnectionStatus { // The check_connectedness() updates the boolean when three hops route is found can_make_routes: bool, @@ -187,16 +187,16 @@ impl OverallConnectionStatus { } pub fn update_connection_stage( - &mut self, - peer_addr: IpAddr, + // &mut self, + connection_progress: &mut ConnectionProgress, event: ConnectionProgressEvent, - node_to_ui_recipient: &Recipient, + // node_to_ui_recipient: &Recipient, ) { - let connection_progress_to_modify = self.get_connection_progress_by_ip(peer_addr).unwrap(); + eprintln!("Updated the Connection Stage with event: {:?}", event); // TODO: Do Something with the error - "Cannot update stage for the peer addr: {}" let mut modify_connection_progress = - |stage: ConnectionStage| connection_progress_to_modify.update_stage(stage); + |stage: ConnectionStage| connection_progress.update_stage(stage); match event { ConnectionProgressEvent::TcpConnectionSuccessful => { @@ -207,14 +207,14 @@ impl OverallConnectionStatus { } ConnectionProgressEvent::IntroductionGossipReceived(_new_node) => { modify_connection_progress(ConnectionStage::NeighborshipEstablished); - self.update_stage_of_overall_connection_status(node_to_ui_recipient); + // self.update_stage_of_overall_connection_status(node_to_ui_recipient); } ConnectionProgressEvent::StandardGossipReceived => { modify_connection_progress(ConnectionStage::NeighborshipEstablished); - self.update_stage_of_overall_connection_status(node_to_ui_recipient); + // self.update_stage_of_overall_connection_status(node_to_ui_recipient); } ConnectionProgressEvent::PassGossipReceived(new_pass_target) => { - connection_progress_to_modify.handle_pass_gossip(new_pass_target); + connection_progress.handle_pass_gossip(new_pass_target); } ConnectionProgressEvent::PassLoopFound => { modify_connection_progress(ConnectionStage::Failed(PassLoopFound)); @@ -225,7 +225,7 @@ impl OverallConnectionStatus { } } - fn update_stage_of_overall_connection_status( + pub(crate) fn update_stage_of_overall_connection_status( &mut self, node_to_ui_recipient: &Recipient, ) { @@ -239,6 +239,7 @@ impl OverallConnectionStatus { } else { self.stage = OverallConnectionStage::ConnectedToNeighbor; } + eprintln!("Updated the Overall Connection Stage: {:?}", self.stage); if self.stage as usize > prev_stage as usize { OverallConnectionStatus::send_message_to_ui(self.stage.into(), node_to_ui_recipient); } @@ -470,10 +471,9 @@ mod tests { let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); - subject.update_connection_stage( - node_ip_addr, + OverallConnectionStatus::update_connection_stage( + subject.get_connection_progress_by_ip(node_ip_addr).unwrap(), ConnectionProgressEvent::TcpConnectionSuccessful, - &recipient, ); assert_eq!( @@ -494,11 +494,12 @@ mod tests { fn updates_the_connection_stage_to_failed_when_tcp_connection_fails() { let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); + let connection_progress_to_modify = + subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); - subject.update_connection_stage( - node_ip_addr, + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionFailed, - &recipient, ); assert_eq!( @@ -516,31 +517,27 @@ mod tests { } #[test] - fn updates_the_connection_stage_to_neighborship_established() { + fn updates_the_connection_stage_to_neighborship_established_when_introduction_gossip_is_received( + ) { let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); - subject.update_connection_stage( - node_ip_addr, + let connection_progress = subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress, ConnectionProgressEvent::TcpConnectionSuccessful, - &recipient, ); - subject.update_connection_stage( - node_ip_addr, + OverallConnectionStatus::update_connection_stage( + connection_progress, ConnectionProgressEvent::IntroductionGossipReceived(make_ip(1)), - &recipient, ); assert_eq!( - subject, - OverallConnectionStatus { - can_make_routes: false, - stage: OverallConnectionStage::ConnectedToNeighbor, - progress: vec![ConnectionProgress { - initial_node_descriptor: node_descriptor, - current_peer_addr: node_ip_addr, - connection_stage: ConnectionStage::NeighborshipEstablished - }], + connection_progress, + &mut ConnectionProgress { + initial_node_descriptor: node_descriptor, + current_peer_addr: node_ip_addr, + connection_stage: ConnectionStage::NeighborshipEstablished } ) } @@ -549,28 +546,23 @@ mod tests { fn updates_the_connection_stage_to_neighborship_established_when_standard_gossip_is_received() { let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); - subject.update_connection_stage( - node_ip_addr, + let connection_progress = subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress, ConnectionProgressEvent::TcpConnectionSuccessful, - &recipient, ); - subject.update_connection_stage( - node_ip_addr, + OverallConnectionStatus::update_connection_stage( + connection_progress, ConnectionProgressEvent::StandardGossipReceived, - &recipient, ); assert_eq!( - subject, - OverallConnectionStatus { - can_make_routes: false, - stage: OverallConnectionStage::ConnectedToNeighbor, - progress: vec![ConnectionProgress { - initial_node_descriptor: node_descriptor, - current_peer_addr: node_ip_addr, - connection_stage: ConnectionStage::NeighborshipEstablished - }], + connection_progress, + &mut ConnectionProgress { + initial_node_descriptor: node_descriptor, + current_peer_addr: node_ip_addr, + connection_stage: ConnectionStage::NeighborshipEstablished } ) } @@ -580,16 +572,16 @@ mod tests { let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); let pass_target = make_ip(1); - subject.update_connection_stage( - node_ip_addr, + let connection_progress_to_modify = + subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionSuccessful, - &recipient, ); - subject.update_connection_stage( - node_ip_addr, + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::PassGossipReceived(pass_target), - &recipient, ); assert_eq!( @@ -610,16 +602,16 @@ mod tests { fn updates_connection_stage_to_failed_when_pass_loop_is_found() { let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); - subject.update_connection_stage( - node_ip_addr, + let connection_progress_to_modify = + subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionSuccessful, - &recipient, ); - subject.update_connection_stage( - node_ip_addr, + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::PassLoopFound, - &recipient, ); assert_eq!( @@ -640,16 +632,16 @@ mod tests { fn updates_connection_stage_to_failed_when_no_gossip_response_is_received() { let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); - subject.update_connection_stage( - node_ip_addr, + let connection_progress_to_modify = + subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::TcpConnectionSuccessful, - &recipient, ); - subject.update_connection_stage( - node_ip_addr, + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::NoGossipResponseReceived, - &recipient, ); assert_eq!( @@ -666,20 +658,6 @@ mod tests { ) } - #[test] - #[should_panic(expected = "Unable to find the Node in connections with IP Address: 1.1.1.1")] - fn panics_at_updating_the_connection_stage_if_a_node_is_not_a_part_of_connections() { - let (_node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); - let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); - let non_existing_node_s_ip_addr = make_ip(1); - - subject.update_connection_stage( - non_existing_node_s_ip_addr, - ConnectionProgressEvent::TcpConnectionSuccessful, - &recipient, - ); - } - #[test] fn connection_stage_can_be_converted_to_number() { assert_eq!(usize::try_from(&ConnectionStage::StageZero), Ok(0)); @@ -702,11 +680,12 @@ mod tests { fn can_t_establish_neighborhsip_without_having_a_tcp_connection() { let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); let mut subject = OverallConnectionStatus::new(vec![node_descriptor]); + let connection_progress_to_modify = + subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); - subject.update_connection_stage( - node_ip_addr, + OverallConnectionStatus::update_connection_stage( + connection_progress_to_modify, ConnectionProgressEvent::IntroductionGossipReceived(make_ip(1)), - &recipient, ); } @@ -947,26 +926,24 @@ mod tests { make_node_descriptor(ip_addr_2), ]); let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); - subject.update_connection_stage( - ip_addr_1, + let connection_progress_1 = subject.get_connection_progress_by_ip(ip_addr_1).unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_1, ConnectionProgressEvent::TcpConnectionSuccessful, - &node_to_ui_recipient, ); - subject.update_connection_stage( - ip_addr_1, + OverallConnectionStatus::update_connection_stage( + connection_progress_1, ConnectionProgressEvent::IntroductionGossipReceived(make_ip(3)), - &node_to_ui_recipient, ); - subject.update_connection_stage( - ip_addr_2, + let connection_progress_2 = subject.get_connection_progress_by_ip(ip_addr_2).unwrap(); + OverallConnectionStatus::update_connection_stage( + connection_progress_2, ConnectionProgressEvent::TcpConnectionSuccessful, - &node_to_ui_recipient, ); - subject.update_connection_stage( - ip_addr_2, + OverallConnectionStatus::update_connection_stage( + connection_progress_2, ConnectionProgressEvent::PassGossipReceived(make_ip(4)), - &node_to_ui_recipient, ); assert_eq!(subject.stage, OverallConnectionStage::ConnectedToNeighbor); @@ -982,10 +959,9 @@ mod tests { let mut subject = OverallConnectionStatus::new(vec![node_descriptor]); let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); subject.stage = initial_stage; - subject.update_connection_stage( - peer_addr, + OverallConnectionStatus::update_connection_stage( + subject.get_connection_progress_by_ip(peer_addr).unwrap(), ConnectionProgressEvent::TcpConnectionSuccessful, - &node_to_ui_recipient, ); let system = System::new(test_name); @@ -993,7 +969,11 @@ mod tests { match event { ConnectionProgressEvent::StandardGossipReceived | ConnectionProgressEvent::IntroductionGossipReceived(_) => { - subject.update_connection_stage(peer_addr, event, &node_to_ui_recipient); + OverallConnectionStatus::update_connection_stage( + subject.get_connection_progress_by_ip(peer_addr).unwrap(), + event, + ); + subject.update_stage_of_overall_connection_status(&node_to_ui_recipient); } _ => panic!( "Can't update to event {:?} because it doesn't leads to Neighborship Established", From 702c16f928d610d59c2ea4ed352afdc306d39dec Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Wed, 15 Jun 2022 12:00:05 +0530 Subject: [PATCH 04/15] GH-574: migrate the test to check whether two cpm's progress override each other to neighborhood/mod.rs --- node/src/neighborhood/mod.rs | 70 +++++++++++++++++- .../neighborhood/overall_connection_status.rs | 74 +++++++++++-------- 2 files changed, 111 insertions(+), 33 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 46e00fc2c..965a40045 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -1363,6 +1363,7 @@ mod tests { use actix::Recipient; use actix::System; use itertools::Itertools; + use openssl::init; use serde_cbor; use std::time::Duration; use tokio::prelude::Future; @@ -1413,7 +1414,9 @@ mod tests { use crate::neighborhood::overall_connection_status::ConnectionStageErrors::{ NoGossipResponseReceived, PassLoopFound, TcpConnectionFailed, }; - use crate::neighborhood::overall_connection_status::{ConnectionProgress, ConnectionStage}; + use crate::neighborhood::overall_connection_status::{ + ConnectionProgress, ConnectionStage, OverallConnectionStage, + }; use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; impl Handler> for Neighborhood { @@ -2027,6 +2030,71 @@ mod tests { assert_eq!(system.run(), 0); } + #[test] + pub fn progress_in_the_stage_of_overall_connection_status_made_by_one_cpm_is_not_overriden_by_the_other( + ) { + init_test_logging(); + let peer_1 = make_ip(1); + let peer_2 = make_ip(2); + let initial_node_descriptors = + vec![make_node_descriptor(peer_1), make_node_descriptor(peer_2)]; + let neighborhood_config = NeighborhoodConfig { + mode: NeighborhoodMode::Standard( + NodeAddr::new(&make_ip(3), &[1234]), + initial_node_descriptors, + rate_pack(100), + ), + }; + let mut subject = Neighborhood::new( + main_cryptde(), + &bc_from_nc_plus( + neighborhood_config, + make_wallet("earning"), + None, + "progress_in_the_stage_of_overall_connection_status_made_by_one_cpm_is_not_overriden_by_the_other"), + ); + let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); + subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); + let addr = subject.start(); + let cpm_recipient = addr.clone().recipient(); + let system = System::new("testing"); + cpm_recipient + .try_send(ConnectionProgressMessage { + peer_addr: peer_1, + event: ConnectionProgressEvent::TcpConnectionSuccessful, + }) + .unwrap(); + cpm_recipient + .try_send(ConnectionProgressMessage { + peer_addr: peer_1, + event: ConnectionProgressEvent::IntroductionGossipReceived(make_ip(4)), + }) + .unwrap(); + cpm_recipient + .try_send(ConnectionProgressMessage { + peer_addr: peer_2, + event: ConnectionProgressEvent::TcpConnectionSuccessful, + }) + .unwrap(); + + cpm_recipient + .try_send(ConnectionProgressMessage { + peer_addr: peer_2, + event: ConnectionProgressEvent::PassGossipReceived(make_ip(5)), + }) + .unwrap(); + + let assertions = Box::new(move |actor: &mut Neighborhood| { + assert_eq!( + actor.overall_connection_status.stage(), + OverallConnectionStage::ConnectedToNeighbor + ); + }); + addr.try_send(AssertionsMessage { assertions }).unwrap(); + System::current().stop(); + assert_eq!(system.run(), 0); + } + #[test] fn gossip_failures_eventually_stop_the_neighborhood() { init_test_logging(); diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index 1729e439d..55726859e 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -96,7 +96,7 @@ impl ConnectionProgress { } #[derive(PartialEq, Debug, Copy, Clone)] -enum OverallConnectionStage { +pub enum OverallConnectionStage { NotConnected = 0, ConnectedToNeighbor = 1, // When an Introduction or Standard Gossip (acceptance) is received ThreeHopsRouteFound = 2, // Data can be relayed once this stage is reached @@ -275,6 +275,10 @@ impl OverallConnectionStatus { pub fn update_can_make_routes(&mut self, can_make_routes: bool) { self.can_make_routes = can_make_routes; } + + pub fn stage(&self) -> OverallConnectionStage { + self.stage + } } #[cfg(test)] @@ -917,37 +921,43 @@ mod tests { } #[test] - fn progress_done_by_one_connection_progress_can_not_be_overridden_by_other_in_overall_connection_progress( - ) { - let ip_addr_1 = make_ip(1); - let ip_addr_2 = make_ip(2); - let mut subject = OverallConnectionStatus::new(vec![ - make_node_descriptor(ip_addr_1), - make_node_descriptor(ip_addr_2), - ]); - let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); - let connection_progress_1 = subject.get_connection_progress_by_ip(ip_addr_1).unwrap(); - OverallConnectionStatus::update_connection_stage( - connection_progress_1, - ConnectionProgressEvent::TcpConnectionSuccessful, - ); - OverallConnectionStatus::update_connection_stage( - connection_progress_1, - ConnectionProgressEvent::IntroductionGossipReceived(make_ip(3)), - ); - let connection_progress_2 = subject.get_connection_progress_by_ip(ip_addr_2).unwrap(); - OverallConnectionStatus::update_connection_stage( - connection_progress_2, - ConnectionProgressEvent::TcpConnectionSuccessful, - ); - - OverallConnectionStatus::update_connection_stage( - connection_progress_2, - ConnectionProgressEvent::PassGossipReceived(make_ip(4)), - ); - - assert_eq!(subject.stage, OverallConnectionStage::ConnectedToNeighbor); - } + fn getter_fn_for_the_stage_of_overall_connection_status_exists() { + let subject = OverallConnectionStatus::new(vec![make_node_descriptor(make_ip(1))]); + assert_eq!(subject.stage(), OverallConnectionStage::NotConnected); + } + + // #[test] + // fn progress_done_by_one_connection_progress_can_not_be_overridden_by_other_in_overall_connection_progress( + // ) { + // let ip_addr_1 = make_ip(1); + // let ip_addr_2 = make_ip(2); + // let mut subject = OverallConnectionStatus::new(vec![ + // make_node_descriptor(ip_addr_1), + // make_node_descriptor(ip_addr_2), + // ]); + // let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); + // let connection_progress_1 = subject.get_connection_progress_by_ip(ip_addr_1).unwrap(); + // OverallConnectionStatus::update_connection_stage( + // connection_progress_1, + // ConnectionProgressEvent::TcpConnectionSuccessful, + // ); + // OverallConnectionStatus::update_connection_stage( + // connection_progress_1, + // ConnectionProgressEvent::IntroductionGossipReceived(make_ip(3)), + // ); + // let connection_progress_2 = subject.get_connection_progress_by_ip(ip_addr_2).unwrap(); + // OverallConnectionStatus::update_connection_stage( + // connection_progress_2, + // ConnectionProgressEvent::TcpConnectionSuccessful, + // ); + // + // OverallConnectionStatus::update_connection_stage( + // connection_progress_2, + // ConnectionProgressEvent::PassGossipReceived(make_ip(4)), + // ); + // + // assert_eq!(subject.stage, OverallConnectionStage::ConnectedToNeighbor); + // } fn stage_and_ui_message_by_connection_progress_event_and_can_make_routes( initial_stage: OverallConnectionStage, From d14ba809c0726863b4f13b2ee5dfb666ce66041f Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Thu, 16 Jun 2022 14:36:47 +0530 Subject: [PATCH 05/15] GH-574: Improve the tests in Neighborhood --- node/src/neighborhood/mod.rs | 112 ++++++++++++++---- .../neighborhood/overall_connection_status.rs | 53 ++++++--- 2 files changed, 126 insertions(+), 39 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 965a40045..f2e095b06 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -284,6 +284,7 @@ impl Handler for Neighborhood { .expect("UI Gateway is unbound"), ); } + // _ => todo!("don't do anything!"), _ => (), } } @@ -298,21 +299,18 @@ impl Handler for Neighborhood { msg: AskAboutDebutGossipMessage, _ctx: &mut Self::Context, ) -> Self::Result { - unimplemented!(); - // let new_connection_progress = self - // .overall_connection_status - // .get_connection_progress_by_desc(&msg.prev_connection_progress.initial_node_descriptor); - // - // if msg.prev_connection_progress == *new_connection_progress { - // // No change, hence no response was received - // self.overall_connection_status.update_connection_stage( - // msg.prev_connection_progress.current_peer_addr, - // ConnectionProgressEvent::NoGossipResponseReceived, - // self.node_to_ui_recipient_opt - // .as_ref() - // .expect("UI Gateway is unbound"), - // ); - // } + if let Ok(current_connection_progress) = self + .overall_connection_status + .get_connection_progress_by_desc(&msg.prev_connection_progress.initial_node_descriptor) + { + if msg.prev_connection_progress == *current_connection_progress { + // No change, hence no response was received + OverallConnectionStatus::update_connection_stage( + current_connection_progress, + ConnectionProgressEvent::NoGossipResponseReceived, + ); + } + } } } @@ -1369,9 +1367,10 @@ mod tests { use tokio::prelude::Future; use masq_lib::constants::{DEFAULT_CHAIN, TLS_PORT}; + use masq_lib::messages::{ToMessageBody, UiConnectionChangeBroadcast, UiConnectionChangeStage}; use masq_lib::test_utils::utils::{ensure_node_home_directory_exists, TEST_DEFAULT_CHAIN}; - use masq_lib::ui_gateway::MessageBody; use masq_lib::ui_gateway::MessagePath::Conversation; + use masq_lib::ui_gateway::{MessageBody, MessageTarget}; use masq_lib::utils::running_test; use crate::db_config::persistent_configuration::PersistentConfigError; @@ -1406,6 +1405,7 @@ mod tests { use crate::test_utils::recorder::Recording; use crate::test_utils::unshared_test_utils::{ prove_that_crash_request_handler_is_hooked_up, AssertionsMessage, NotifyLaterHandleMock, + SystemKillerActor, }; use crate::test_utils::vec_to_set; use crate::test_utils::{main_cryptde, make_paying_wallet}; @@ -1695,6 +1695,8 @@ mod tests { "neighborhood_handles_connection_progress_message_with_tcp_connection_established", ); let notify_later_ask_about_gossip_params_arc = Arc::new(Mutex::new(vec![])); + let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); subject.tools.notify_later_ask_about_gossip = Box::new( NotifyLaterHandleMock::default() .notify_later_params(¬ify_later_ask_about_gossip_params_arc), @@ -1727,6 +1729,8 @@ mod tests { assert_eq!(system.run(), 0); let notify_later_ask_about_gossip_params = notify_later_ask_about_gossip_params_arc.lock().unwrap(); + let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); + let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); assert_eq!( *notify_later_ask_about_gossip_params, vec![( @@ -1736,6 +1740,7 @@ mod tests { Duration::from_millis(10) )] ); + assert_eq!(node_to_ui_message_opt, None); } #[test] @@ -1747,6 +1752,8 @@ mod tests { &node_descriptor, "ask_about_debut_gossip_message_handles_timeout_in_case_no_response_is_received", ); + let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status .get_connection_progress_by_ip(node_ip_addr) @@ -1781,6 +1788,39 @@ mod tests { }); addr.try_send(AssertionsMessage { assertions }).unwrap(); System::current().stop(); + let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); + let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); + assert_eq!(system.run(), 0); + assert_eq!(node_to_ui_message_opt, None); + } + + #[test] + pub fn it_doesn_t_cause_a_panic_if_neighborhood_receives_ask_about_debut_message_from_unknown_descriptor( + ) { + let known_ip = make_ip(1); + let known_desc = make_node_descriptor(known_ip); + let unknown_ip = make_ip(2); + let unknown_desc = make_node_descriptor(unknown_ip); + let subject = make_subject_from_node_descriptor(&known_desc, "it_doesn_t_cause_a_panic_if_neighborhood_receives_ask_about_debut_message_from_unknown_descriptor"); + let initial_ocs = subject.overall_connection_status.clone(); + let addr = subject.start(); + let recipient: Recipient = addr.clone().recipient(); + let aadgrm = AskAboutDebutGossipMessage { + prev_connection_progress: ConnectionProgress { + initial_node_descriptor: unknown_desc.clone(), + current_peer_addr: unknown_ip, + connection_stage: ConnectionStage::TcpConnectionEstablished, + }, + }; + let system = System::new("testing"); + + recipient.try_send(aadgrm).unwrap(); + + let assertions = Box::new(move |actor: &mut Neighborhood| { + assert_eq!(actor.overall_connection_status, initial_ocs); + }); + addr.try_send(AssertionsMessage { assertions }).unwrap(); + System::current().stop(); assert_eq!(system.run(), 0); } @@ -1789,10 +1829,12 @@ mod tests { init_test_logging(); let node_ip_addr = IpAddr::from_str("5.4.3.2").unwrap(); let node_descriptor = make_node_descriptor(node_ip_addr); - let subject = make_subject_from_node_descriptor( + let mut subject = make_subject_from_node_descriptor( &node_descriptor, "neighborhood_handles_connection_progress_message_with_tcp_connection_failed", ); + let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); let system = System::new("testing"); @@ -1815,7 +1857,10 @@ mod tests { }); addr.try_send(AssertionsMessage { assertions }).unwrap(); System::current().stop(); + let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); + let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); assert_eq!(system.run(), 0); + assert_eq!(node_to_ui_message_opt, None); } #[test] @@ -1827,6 +1872,8 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_pass_gossip_received", ); + let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status .get_connection_progress_by_ip(node_ip_addr) @@ -1858,7 +1905,10 @@ mod tests { }); addr.try_send(AssertionsMessage { assertions }).unwrap(); System::current().stop(); + let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); + let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); assert_eq!(system.run(), 0); + assert_eq!(node_to_ui_message_opt, None); } #[test] @@ -1870,6 +1920,8 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_pass_loop_found", ); + let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status .get_connection_progress_by_ip(node_ip_addr) @@ -1900,7 +1952,10 @@ mod tests { }); addr.try_send(AssertionsMessage { assertions }).unwrap(); System::current().stop(); + let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); + let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); assert_eq!(system.run(), 0); + assert_eq!(node_to_ui_message_opt, None); } #[test] @@ -1912,6 +1967,8 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_introduction_gossip_received", ); + let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status .get_connection_progress_by_ip(node_ip_addr) @@ -1923,6 +1980,8 @@ mod tests { let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); let system = System::new("testing"); + let killer = SystemKillerActor::new(Duration::from_millis(3_000)); + killer.start(); let new_node = IpAddr::from_str("10.20.30.40").unwrap(); let connection_progress_message = ConnectionProgressMessage { peer_addr: node_ip_addr, @@ -1942,8 +2001,21 @@ mod tests { ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); - System::current().stop(); + // System::current().stop(); assert_eq!(system.run(), 0); + let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); + let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); + assert_eq!(node_to_ui_mutex.len(), 1); + assert_eq!( + node_to_ui_message_opt, + Some(&NodeToUiMessage { + target: MessageTarget::AllClients, + body: UiConnectionChangeBroadcast { + stage: UiConnectionChangeStage::ThreeHopsRouteFound + } + .tmb(0) + }) + ); } #[test] @@ -5006,8 +5078,8 @@ mod tests { let mut neighborhood = Neighborhood::new(main_cryptde(), &bootstrap_config); - let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); - neighborhood.node_to_ui_recipient_opt = Some(node_to_ui_recipient); + // let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); + // neighborhood.node_to_ui_recipient_opt = Some(node_to_ui_recipient); neighborhood } diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index 55726859e..2cca8177c 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -167,23 +167,20 @@ impl OverallConnectionStatus { } pub fn get_connection_progress_by_desc( - &self, + &mut self, initial_node_descriptor: &NodeDescriptor, - ) -> &ConnectionProgress { - let connection_progress = self - .progress - .iter() - .find(|connection_progress| { - &connection_progress.initial_node_descriptor == initial_node_descriptor - }) - .unwrap_or_else(|| { - panic!( - "Unable to find the Node in connections with Node Descriptor: {:?}", - initial_node_descriptor - ) - }); + ) -> Result<&mut ConnectionProgress, String> { + let connection_progress = self.progress.iter_mut().find(|connection_progress| { + &connection_progress.initial_node_descriptor == initial_node_descriptor + }); - connection_progress + match connection_progress { + Some(connection_progress) => Ok(connection_progress), + None => Err(format!( + "Unable to find the Node in connections with Node Descriptor: {:?}", + initial_node_descriptor + )), + } } pub fn update_connection_stage( @@ -225,7 +222,7 @@ impl OverallConnectionStatus { } } - pub(crate) fn update_stage_of_overall_connection_status( + pub fn update_stage_of_overall_connection_status( &mut self, node_to_ui_recipient: &Recipient, ) { @@ -257,6 +254,7 @@ impl OverallConnectionStatus { node_to_ui_recipient .try_send(message) .expect("UI Gateway is unbound."); + eprintln!("message sent"); } pub fn is_empty(&self) -> bool { @@ -432,15 +430,32 @@ mod tests { let desc_2 = make_node_descriptor(make_ip(2)); let initial_node_descriptors = vec![desc_1.clone(), desc_2.clone()]; - let subject = OverallConnectionStatus::new(initial_node_descriptors); + let mut subject = OverallConnectionStatus::new(initial_node_descriptors); assert_eq!( subject.get_connection_progress_by_desc(&desc_1), - &ConnectionProgress::new(desc_1) + Ok(&mut ConnectionProgress::new(desc_1)) ); assert_eq!( subject.get_connection_progress_by_desc(&desc_2), - &ConnectionProgress::new(desc_2) + Ok(&mut ConnectionProgress::new(desc_2)) + ); + } + + #[test] + fn receives_an_error_in_receiving_connection_progress_from_unknown_initial_node_desc() { + let known_desc = make_node_descriptor(make_ip(1)); + let unknown_desc = make_node_descriptor(make_ip(2)); + let initial_node_descriptors = vec![known_desc]; + + let mut subject = OverallConnectionStatus::new(initial_node_descriptors); + + assert_eq!( + subject.get_connection_progress_by_desc(&unknown_desc), + Err(format!( + "Unable to find the Node in connections with Node Descriptor: {:?}", + unknown_desc + )) ); } From 405fff48c2f561eb2d5c9a981c4b3cf609f1d1b0 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Sun, 19 Jun 2022 08:55:33 +0530 Subject: [PATCH 06/15] GH-574: Bert invented a method in Recorder to stop the system after a certain type of message is received. --- node/src/neighborhood/mod.rs | 22 +++++++++++++++++++++- node/src/test_utils/recorder.rs | 13 +++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index f2e095b06..e21e73a7c 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -1349,6 +1349,7 @@ pub fn regenerate_signed_gossip( #[cfg(test)] mod tests { + use std::any::TypeId; use std::cell::RefCell; use std::convert::TryInto; use std::net::{IpAddr, SocketAddr}; @@ -2027,6 +2028,12 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_standard_gossip_received", ); + // let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + let (recorder, _awaiter, node_to_ui_recording_arc) = make_recorder(); + let recorder = recorder.stop_condition(TypeId::of::()); + let addr = recorder.start(); + let node_to_ui_recipient = addr.recipient::(); + subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status .get_connection_progress_by_ip(node_ip_addr) @@ -2056,8 +2063,21 @@ mod tests { ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); - System::current().stop(); + // System::current().stop(); assert_eq!(system.run(), 0); + let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); + let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); + assert_eq!(node_to_ui_mutex.len(), 1); + assert_eq!( + node_to_ui_message_opt, + Some(&NodeToUiMessage { + target: MessageTarget::AllClients, + body: UiConnectionChangeBroadcast { + stage: UiConnectionChangeStage::ConnectedToNeighbor + } + .tmb(0) + }) + ); } #[test] diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index 063eb3360..85b028e8d 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -49,9 +49,11 @@ use actix::Addr; use actix::Context; use actix::Handler; use actix::MessageResult; +use actix::System; use actix::{Actor, Message}; use masq_lib::ui_gateway::{NodeFromUiMessage, NodeToUiMessage}; use std::any::Any; +use std::any::TypeId; use std::sync::{Arc, Mutex}; use std::thread; use std::time::Duration; @@ -62,6 +64,7 @@ pub struct Recorder { recording: Arc>, node_query_responses: Vec>, route_query_responses: Vec>, + stop_condition_opt: Option, } #[derive(Default)] @@ -84,6 +87,11 @@ macro_rules! recorder_message_handler { fn handle(&mut self, msg: $message_type, _ctx: &mut Self::Context) { self.record(msg); + if let Some(expected_type_id) = self.stop_condition_opt { + if expected_type_id == TypeId::of::<$message_type>() { + System::current().stop() + } + } } } }; @@ -217,6 +225,11 @@ impl Recorder { self.route_query_responses.push(response); self } + + pub fn stop_condition(mut self, message_type_id: TypeId) -> Recorder { + self.stop_condition_opt = Some(message_type_id); + self + } } impl Recording { From 6caccf55122c448114de7d0f487ca1ec3f0b4058 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Mon, 20 Jun 2022 12:28:50 +0530 Subject: [PATCH 07/15] GH-574: modify make_recipient_and_recording_arc() to accept stopping_message --- node/src/neighborhood/mod.rs | 24 +++++++++---------- .../src/test_utils/neighborhood_test_utils.rs | 13 +++++++--- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index e21e73a7c..ddedfccc2 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -1396,7 +1396,8 @@ mod tests { use crate::test_utils::make_wallet; use crate::test_utils::neighborhood_test_utils::{ db_from_node, make_global_cryptde_node_record, make_ip, make_node_descriptor, - make_node_record, make_node_record_f, make_node_to_ui_recipient, neighborhood_from_nodes, + make_node_record, make_node_record_f, make_node_to_ui_recipient, + make_recipient_and_recording_arc, neighborhood_from_nodes, }; use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock; use crate::test_utils::rate_pack; @@ -1921,7 +1922,10 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_pass_loop_found", ); - let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + let (recorder, _awaiter, node_to_ui_recording_arc) = make_recorder(); + let recorder = recorder.stop_condition(TypeId::of::()); + let addr = recorder.start(); + let node_to_ui_recipient = addr.recipient::(); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status @@ -1968,7 +1972,8 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_introduction_gossip_received", ); - let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + let (node_to_ui_recipient, node_to_ui_recording_arc) = + make_recipient_and_recording_arc(Some(TypeId::of::())); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status @@ -1981,8 +1986,6 @@ mod tests { let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); let system = System::new("testing"); - let killer = SystemKillerActor::new(Duration::from_millis(3_000)); - killer.start(); let new_node = IpAddr::from_str("10.20.30.40").unwrap(); let connection_progress_message = ConnectionProgressMessage { peer_addr: node_ip_addr, @@ -2002,7 +2005,6 @@ mod tests { ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); - // System::current().stop(); assert_eq!(system.run(), 0); let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); @@ -2012,7 +2014,7 @@ mod tests { Some(&NodeToUiMessage { target: MessageTarget::AllClients, body: UiConnectionChangeBroadcast { - stage: UiConnectionChangeStage::ThreeHopsRouteFound + stage: UiConnectionChangeStage::ConnectedToNeighbor } .tmb(0) }) @@ -2028,11 +2030,8 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_standard_gossip_received", ); - // let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); - let (recorder, _awaiter, node_to_ui_recording_arc) = make_recorder(); - let recorder = recorder.stop_condition(TypeId::of::()); - let addr = recorder.start(); - let node_to_ui_recipient = addr.recipient::(); + let (node_to_ui_recipient, node_to_ui_recording_arc) = + make_recipient_and_recording_arc(Some(TypeId::of::())); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status @@ -2063,7 +2062,6 @@ mod tests { ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); - // System::current().stop(); assert_eq!(system.run(), 0); let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); diff --git a/node/src/test_utils/neighborhood_test_utils.rs b/node/src/test_utils/neighborhood_test_utils.rs index 46a8a35ae..fbc536f87 100644 --- a/node/src/test_utils/neighborhood_test_utils.rs +++ b/node/src/test_utils/neighborhood_test_utils.rs @@ -1,3 +1,4 @@ +use std::any::TypeId; // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. use crate::bootstrapper::BootstrapperConfig; use crate::neighborhood::gossip::GossipNodeRecord; @@ -300,13 +301,19 @@ pub fn make_node_and_recipient() -> (IpAddr, NodeDescriptor, Recipient() -> (Recipient, Arc>) +pub fn make_recipient_and_recording_arc( + stopping_message: Option, +) -> (Recipient, Arc>) where M: Message + Send, ::Result: Send, Recorder: Handler, { let (recorder, _, recording_arc) = make_recorder(); + let recorder = match stopping_message { + Some(type_id) => recorder.stop_condition(type_id), // No need to write stop message after this + None => recorder, + }; let addr = recorder.start(); let recipient = addr.recipient::(); @@ -314,9 +321,9 @@ where } pub fn make_cpm_recipient() -> (Recipient, Arc>) { - make_recipient_and_recording_arc() + make_recipient_and_recording_arc(None) } pub fn make_node_to_ui_recipient() -> (Recipient, Arc>) { - make_recipient_and_recording_arc() + make_recipient_and_recording_arc(None) } From 923274a2e9251afd0965aedd1a234f22c87f316d Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 21 Jun 2022 10:38:59 +0530 Subject: [PATCH 08/15] GH-574: use hashmap for the stop condition inside the Recorder --- node/src/neighborhood/mod.rs | 21 ++++++++++++++------- node/src/test_utils/recorder.rs | 28 +++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index ddedfccc2..3f816f1c9 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -1351,6 +1351,7 @@ pub fn regenerate_signed_gossip( mod tests { use std::any::TypeId; use std::cell::RefCell; + use std::collections::HashMap; use std::convert::TryInto; use std::net::{IpAddr, SocketAddr}; use std::str::FromStr; @@ -1697,7 +1698,8 @@ mod tests { "neighborhood_handles_connection_progress_message_with_tcp_connection_established", ); let notify_later_ask_about_gossip_params_arc = Arc::new(Mutex::new(vec![])); - let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + let (node_to_ui_recipient, node_to_ui_recording_arc) = + make_recipient_and_recording_arc(Some(TypeId::of::())); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); subject.tools.notify_later_ask_about_gossip = Box::new( NotifyLaterHandleMock::default() @@ -1727,7 +1729,6 @@ mod tests { ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); - System::current().stop(); assert_eq!(system.run(), 0); let notify_later_ask_about_gossip_params = notify_later_ask_about_gossip_params_arc.lock().unwrap(); @@ -1754,7 +1755,8 @@ mod tests { &node_descriptor, "ask_about_debut_gossip_message_handles_timeout_in_case_no_response_is_received", ); - let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); + let (node_to_ui_recipient, node_to_ui_recording_arc) = + make_recipient_and_recording_arc(Some(TypeId::of::())); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status @@ -1956,10 +1958,9 @@ mod tests { ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); - System::current().stop(); + assert_eq!(system.run(), 0); let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); - assert_eq!(system.run(), 0); assert_eq!(node_to_ui_message_opt, None); } @@ -1972,8 +1973,14 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_introduction_gossip_received", ); - let (node_to_ui_recipient, node_to_ui_recording_arc) = - make_recipient_and_recording_arc(Some(TypeId::of::())); + let (ui_gateway_recorder, awaiter, node_to_ui_recording_arc) = make_recorder(); + let mut expected_count_by_messages: HashMap = HashMap::new(); + // expected_count_by_messages.insert(TypeId::of::(), 1); + expected_count_by_messages.insert(TypeId::of::(), 1); + let ui_gateway_recorder = + ui_gateway_recorder.stop_after_messages(expected_count_by_messages); + let addr = ui_gateway_recorder.start(); + let node_to_ui_recipient = addr.recipient(); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index 85b028e8d..9815dcc60 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -54,6 +54,7 @@ use actix::{Actor, Message}; use masq_lib::ui_gateway::{NodeFromUiMessage, NodeToUiMessage}; use std::any::Any; use std::any::TypeId; +use std::collections::HashMap; use std::sync::{Arc, Mutex}; use std::thread; use std::time::Duration; @@ -64,7 +65,7 @@ pub struct Recorder { recording: Arc>, node_query_responses: Vec>, route_query_responses: Vec>, - stop_condition_opt: Option, + expected_count_by_msg_type_opt: Option>, } #[derive(Default)] @@ -87,9 +88,18 @@ macro_rules! recorder_message_handler { fn handle(&mut self, msg: $message_type, _ctx: &mut Self::Context) { self.record(msg); - if let Some(expected_type_id) = self.stop_condition_opt { - if expected_type_id == TypeId::of::<$message_type>() { - System::current().stop() + if let Some(expected_count_by_msg_type) = &mut self.expected_count_by_msg_type_opt { + let type_id = TypeId::of::<$message_type>(); + let count = expected_count_by_msg_type.entry(type_id).or_insert(0); + if *count == 0 { + panic!( + "Received a message, which we were not supposed to receive. {:?}", + stringify!($message_type) + ); + }; + *count -= 1; + if !expected_count_by_msg_type.values().any(|&x| x > 0) { + System::current().stop(); } } } @@ -227,7 +237,15 @@ impl Recorder { } pub fn stop_condition(mut self, message_type_id: TypeId) -> Recorder { - self.stop_condition_opt = Some(message_type_id); + // self.stop_condition_opt = Some(message_type_id); + self + } + + pub fn stop_after_messages( + mut self, + expected_count_by_messages: HashMap, + ) -> Recorder { + self.expected_count_by_msg_type_opt = Some(expected_count_by_messages); self } } From 6167e70afc4ee3bc8b64738e93479f1880c0d687 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 21 Jun 2022 10:53:07 +0530 Subject: [PATCH 09/15] GH-574: use stopping_message inside the tests of neighborhood --- node/src/neighborhood/mod.rs | 10 ++-------- node/src/test_utils/recorder.rs | 5 +++-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 3f816f1c9..c445f81ef 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -1973,14 +1973,8 @@ mod tests { &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_introduction_gossip_received", ); - let (ui_gateway_recorder, awaiter, node_to_ui_recording_arc) = make_recorder(); - let mut expected_count_by_messages: HashMap = HashMap::new(); - // expected_count_by_messages.insert(TypeId::of::(), 1); - expected_count_by_messages.insert(TypeId::of::(), 1); - let ui_gateway_recorder = - ui_gateway_recorder.stop_after_messages(expected_count_by_messages); - let addr = ui_gateway_recorder.start(); - let node_to_ui_recipient = addr.recipient(); + let (node_to_ui_recipient, node_to_ui_recording_arc) = + make_recipient_and_recording_arc(Some(TypeId::of::())); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index 9815dcc60..e7a4cb6fa 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -237,8 +237,9 @@ impl Recorder { } pub fn stop_condition(mut self, message_type_id: TypeId) -> Recorder { - // self.stop_condition_opt = Some(message_type_id); - self + let mut expected_count_by_messages: HashMap = HashMap::new(); + expected_count_by_messages.insert(message_type_id, 1); + self.stop_after_messages(expected_count_by_messages) } pub fn stop_after_messages( From 8377f3ec9028483122ed97516c7470bd78141b99 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 21 Jun 2022 12:19:07 +0530 Subject: [PATCH 10/15] GH-574: refactor tests in Overall Connection Status to test whether a message is sent or not and updates the stage --- node/src/neighborhood/mod.rs | 4 +- .../neighborhood/overall_connection_status.rs | 169 ++---------------- 2 files changed, 18 insertions(+), 155 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index c445f81ef..3523fb156 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -278,7 +278,7 @@ impl Handler for Neighborhood { | ConnectionProgressEvent::StandardGossipReceived => { eprintln!("Yes, I was called"); self.overall_connection_status - .update_stage_of_overall_connection_status( + .update_ocs_stage_and_send_message_to_ui( self.node_to_ui_recipient_opt .as_ref() .expect("UI Gateway is unbound"), @@ -1699,7 +1699,7 @@ mod tests { ); let notify_later_ask_about_gossip_params_arc = Arc::new(Mutex::new(vec![])); let (node_to_ui_recipient, node_to_ui_recording_arc) = - make_recipient_and_recording_arc(Some(TypeId::of::())); + make_recipient_and_recording_arc(Some(TypeId::of::())); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); subject.tools.notify_later_ask_about_gossip = Box::new( NotifyLaterHandleMock::default() diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index 2cca8177c..346267aa8 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -222,7 +222,7 @@ impl OverallConnectionStatus { } } - pub fn update_stage_of_overall_connection_status( + pub fn update_ocs_stage_and_send_message_to_ui( &mut self, node_to_ui_recipient: &Recipient, ) { @@ -765,40 +765,16 @@ mod tests { } #[test] - fn updates_from_not_connected_to_connected_to_neighbor_in_case_flag_is_false() { - let (_node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); - let mut subject = OverallConnectionStatus::new(vec![node_descriptor]); - subject.update_can_make_routes(false); - - subject.update_stage_of_overall_connection_status(&recipient); - - assert_eq!(subject.stage, OverallConnectionStage::ConnectedToNeighbor); - } - - #[test] - fn updates_from_not_connected_to_three_hops_route_found_in_case_flag_is_true() { - let (_node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); - let mut subject = OverallConnectionStatus::new(vec![node_descriptor]); - subject.update_can_make_routes(true); - - subject.update_stage_of_overall_connection_status(&recipient); - - assert_eq!(subject.stage, OverallConnectionStage::ThreeHopsRouteFound); - } - - #[test] - fn updates_the_stage_to_three_hops_route_found_in_case_introduction_gossip_is_received_and_flag_is_true( + fn updates_the_stage_to_three_hops_route_found_in_case_introduction_or_standard_gossip_is_received_and_flag_is_true( ) { let initial_stage = OverallConnectionStage::NotConnected; - let event = ConnectionProgressEvent::IntroductionGossipReceived(make_ip(1)); let can_make_routes = true; let (stage, message_opt) = - stage_and_ui_message_by_connection_progress_event_and_can_make_routes( + stage_and_ui_message_by_initial_stage_and_can_make_routes( initial_stage, - event, can_make_routes, - "updates_the_stage_to_three_hops_route_found_in_case_introduction_gossip_is_received_and_flag_is_true" + "updates_the_stage_to_three_hops_route_found_in_case_introduction_or_standard_gossip_is_received_and_flag_is_true" ); assert_eq!(stage, OverallConnectionStage::ThreeHopsRouteFound); @@ -815,18 +791,16 @@ mod tests { } #[test] - fn updates_the_stage_to_connected_to_neighbor_in_case_introduction_gossip_is_received_and_flag_is_false( + fn updates_the_stage_to_connected_to_neighbor_in_case_introduction_or_standard_gossip_is_received_and_flag_is_false( ) { let initial_stage = OverallConnectionStage::NotConnected; - let event = ConnectionProgressEvent::IntroductionGossipReceived(make_ip(1)); let can_make_routes = false; let (stage, message_opt) = - stage_and_ui_message_by_connection_progress_event_and_can_make_routes( + stage_and_ui_message_by_initial_stage_and_can_make_routes( initial_stage, - event, can_make_routes, - "updates_the_stage_to_connected_to_neighbor_in_case_introduction_gossip_is_received_and_flag_is_false" + "updates_the_stage_to_connected_to_neighbor_in_case_introduction_or_standard_gossip_is_received_and_flag_is_false" ); assert_eq!(stage, OverallConnectionStage::ConnectedToNeighbor); @@ -843,74 +817,16 @@ mod tests { } #[test] - fn updates_the_stage_to_three_hops_route_found_in_case_standard_gossip_is_received_and_flag_is_true( - ) { - let initial_stage = OverallConnectionStage::NotConnected; - let event = ConnectionProgressEvent::StandardGossipReceived; - let can_make_routes = true; - - let (stage, message_opt) = - stage_and_ui_message_by_connection_progress_event_and_can_make_routes( - initial_stage, - event, - can_make_routes, - "updates_the_stage_to_three_hops_route_found_in_case_standard_gossip_is_received_and_flag_is_true" - ); - - assert_eq!(stage, OverallConnectionStage::ThreeHopsRouteFound); - assert_eq!( - message_opt, - Some(NodeToUiMessage { - target: MessageTarget::AllClients, - body: UiConnectionChangeBroadcast { - stage: UiConnectionChangeStage::ThreeHopsRouteFound - } - .tmb(0) - }) - ); - } - - #[test] - fn updates_the_stage_to_connected_to_neighbor_in_case_standard_gossip_is_received_and_flag_is_false( + fn doesn_t_send_message_to_the_ui_in_case_introduction_or_standard_gossip_is_received_but_stage_hasn_t_updated( ) { - let initial_stage = OverallConnectionStage::NotConnected; - let event = ConnectionProgressEvent::StandardGossipReceived; - let can_make_routes = false; - - let (stage, message_opt) = - stage_and_ui_message_by_connection_progress_event_and_can_make_routes( - initial_stage, - event, - can_make_routes, - "updates_the_stage_to_connected_to_neighbor_in_case_standard_gossip_is_received_and_flag_is_false" - ); - - assert_eq!(stage, OverallConnectionStage::ConnectedToNeighbor); - assert_eq!( - message_opt, - Some(NodeToUiMessage { - target: MessageTarget::AllClients, - body: UiConnectionChangeBroadcast { - stage: UiConnectionChangeStage::ConnectedToNeighbor - } - .tmb(0) - }) - ); - } - - #[test] - fn doesn_t_send_message_to_the_ui_in_case_gossip_is_received_but_stage_hasn_t_updated() { let initial_stage = OverallConnectionStage::ConnectedToNeighbor; - let event = ConnectionProgressEvent::StandardGossipReceived; let can_make_routes = false; - let (stage, message_opt) = - stage_and_ui_message_by_connection_progress_event_and_can_make_routes( - initial_stage, - event, - can_make_routes, - "doesn_t_send_message_to_the_ui_in_case_gossip_is_received_but_stage_hasn_t_updated" - ); + let (stage, message_opt) = stage_and_ui_message_by_initial_stage_and_can_make_routes( + initial_stage, + can_make_routes, + "doesn_t_send_message_to_the_ui_in_case_introduction_or_standard_gossip_is_received_but_stage_hasn_t_updated", + ); assert_eq!(stage, initial_stage); assert_eq!(message_opt, None); @@ -920,13 +836,11 @@ mod tests { fn doesn_t_send_a_message_to_ui_in_case_connection_drops_from_three_hops_to_connected_to_neighbor( ) { let initial_stage = OverallConnectionStage::ThreeHopsRouteFound; - let event = ConnectionProgressEvent::StandardGossipReceived; let can_make_routes = false; let (stage, message_opt) = - stage_and_ui_message_by_connection_progress_event_and_can_make_routes( + stage_and_ui_message_by_initial_stage_and_can_make_routes( initial_stage, - event, can_make_routes, "doesn_t_send_a_message_to_ui_in_case_connection_drops_from_three_hops_to_connected_to_neighbor" ); @@ -941,42 +855,8 @@ mod tests { assert_eq!(subject.stage(), OverallConnectionStage::NotConnected); } - // #[test] - // fn progress_done_by_one_connection_progress_can_not_be_overridden_by_other_in_overall_connection_progress( - // ) { - // let ip_addr_1 = make_ip(1); - // let ip_addr_2 = make_ip(2); - // let mut subject = OverallConnectionStatus::new(vec![ - // make_node_descriptor(ip_addr_1), - // make_node_descriptor(ip_addr_2), - // ]); - // let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); - // let connection_progress_1 = subject.get_connection_progress_by_ip(ip_addr_1).unwrap(); - // OverallConnectionStatus::update_connection_stage( - // connection_progress_1, - // ConnectionProgressEvent::TcpConnectionSuccessful, - // ); - // OverallConnectionStatus::update_connection_stage( - // connection_progress_1, - // ConnectionProgressEvent::IntroductionGossipReceived(make_ip(3)), - // ); - // let connection_progress_2 = subject.get_connection_progress_by_ip(ip_addr_2).unwrap(); - // OverallConnectionStatus::update_connection_stage( - // connection_progress_2, - // ConnectionProgressEvent::TcpConnectionSuccessful, - // ); - // - // OverallConnectionStatus::update_connection_stage( - // connection_progress_2, - // ConnectionProgressEvent::PassGossipReceived(make_ip(4)), - // ); - // - // assert_eq!(subject.stage, OverallConnectionStage::ConnectedToNeighbor); - // } - - fn stage_and_ui_message_by_connection_progress_event_and_can_make_routes( + fn stage_and_ui_message_by_initial_stage_and_can_make_routes( initial_stage: OverallConnectionStage, - event: ConnectionProgressEvent, can_make_routes: bool, test_name: &str, ) -> (OverallConnectionStage, Option) { @@ -984,27 +864,10 @@ mod tests { let mut subject = OverallConnectionStatus::new(vec![node_descriptor]); let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); subject.stage = initial_stage; - OverallConnectionStatus::update_connection_stage( - subject.get_connection_progress_by_ip(peer_addr).unwrap(), - ConnectionProgressEvent::TcpConnectionSuccessful, - ); let system = System::new(test_name); subject.update_can_make_routes(can_make_routes); - match event { - ConnectionProgressEvent::StandardGossipReceived - | ConnectionProgressEvent::IntroductionGossipReceived(_) => { - OverallConnectionStatus::update_connection_stage( - subject.get_connection_progress_by_ip(peer_addr).unwrap(), - event, - ); - subject.update_stage_of_overall_connection_status(&node_to_ui_recipient); - } - _ => panic!( - "Can't update to event {:?} because it doesn't leads to Neighborship Established", - event - ), - } + subject.update_ocs_stage_and_send_message_to_ui(&node_to_ui_recipient); System::current().stop(); assert_eq!(system.run(), 0); From e2e16c757a010f290c59b2f29278b9b94d16f396 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 21 Jun 2022 13:06:33 +0530 Subject: [PATCH 11/15] GH-574: eliminate warnings, eprintlns and comments --- node/src/neighborhood/mod.rs | 16 +++------- .../neighborhood/overall_connection_status.rs | 32 +++++++------------ .../src/test_utils/neighborhood_test_utils.rs | 7 ++++ node/src/test_utils/recorder.rs | 2 +- 4 files changed, 25 insertions(+), 32 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 3523fb156..fcb4eb344 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -34,7 +34,7 @@ use crate::db_config::persistent_configuration::{ use crate::neighborhood::gossip::{DotGossipEndpoint, GossipNodeRecord, Gossip_0v1}; use crate::neighborhood::gossip_acceptor::GossipAcceptanceResult; use crate::neighborhood::node_record::NodeRecordInner_0v1; -use crate::neighborhood::overall_connection_status::{ConnectionProgress, OverallConnectionStatus}; +use crate::neighborhood::overall_connection_status::OverallConnectionStatus; use crate::stream_messages::RemovedStreamType; use crate::sub_lib::configurator::NewPasswordMessage; use crate::sub_lib::cryptde::PublicKey; @@ -259,7 +259,6 @@ impl Handler for Neighborhood { type Result = (); fn handle(&mut self, msg: ConnectionProgressMessage, ctx: &mut Self::Context) -> Self::Result { - eprintln!("Handling the Connection Progress Message"); if let Ok(connection_progress) = self .overall_connection_status .get_connection_progress_by_ip(msg.peer_addr) @@ -268,15 +267,12 @@ impl Handler for Neighborhood { connection_progress, msg.event.clone(), ); - - eprintln!("Entering match statement"); match msg.event { ConnectionProgressEvent::TcpConnectionSuccessful => { self.send_ask_about_debut_gossip_message(ctx, msg.peer_addr); } ConnectionProgressEvent::IntroductionGossipReceived(_) | ConnectionProgressEvent::StandardGossipReceived => { - eprintln!("Yes, I was called"); self.overall_connection_status .update_ocs_stage_and_send_message_to_ui( self.node_to_ui_recipient_opt @@ -1351,7 +1347,6 @@ pub fn regenerate_signed_gossip( mod tests { use std::any::TypeId; use std::cell::RefCell; - use std::collections::HashMap; use std::convert::TryInto; use std::net::{IpAddr, SocketAddr}; use std::str::FromStr; @@ -1363,7 +1358,7 @@ mod tests { use actix::Recipient; use actix::System; use itertools::Itertools; - use openssl::init; + use serde_cbor; use std::time::Duration; use tokio::prelude::Future; @@ -1408,7 +1403,6 @@ mod tests { use crate::test_utils::recorder::Recording; use crate::test_utils::unshared_test_utils::{ prove_that_crash_request_handler_is_hooked_up, AssertionsMessage, NotifyLaterHandleMock, - SystemKillerActor, }; use crate::test_utils::vec_to_set; use crate::test_utils::{main_cryptde, make_paying_wallet}; @@ -1665,7 +1659,7 @@ mod tests { let known_peer = make_ip(1); let unknown_peer = make_ip(2); let node_descriptor = make_node_descriptor(known_peer); - let mut subject = make_subject_from_node_descriptor( + let subject = make_subject_from_node_descriptor( &node_descriptor, "neighborhood_doesn_t_do_anything_if_it_receives_a_cpm_with_an_unknown_peer_addr", ); @@ -5097,8 +5091,8 @@ mod tests { let mut neighborhood = Neighborhood::new(main_cryptde(), &bootstrap_config); - // let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); - // neighborhood.node_to_ui_recipient_opt = Some(node_to_ui_recipient); + let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); + neighborhood.node_to_ui_recipient_opt = Some(node_to_ui_recipient); neighborhood } diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index 346267aa8..b00318471 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -184,14 +184,9 @@ impl OverallConnectionStatus { } pub fn update_connection_stage( - // &mut self, connection_progress: &mut ConnectionProgress, event: ConnectionProgressEvent, - // node_to_ui_recipient: &Recipient, ) { - eprintln!("Updated the Connection Stage with event: {:?}", event); - // TODO: Do Something with the error - "Cannot update stage for the peer addr: {}" - let mut modify_connection_progress = |stage: ConnectionStage| connection_progress.update_stage(stage); @@ -204,11 +199,9 @@ impl OverallConnectionStatus { } ConnectionProgressEvent::IntroductionGossipReceived(_new_node) => { modify_connection_progress(ConnectionStage::NeighborshipEstablished); - // self.update_stage_of_overall_connection_status(node_to_ui_recipient); } ConnectionProgressEvent::StandardGossipReceived => { modify_connection_progress(ConnectionStage::NeighborshipEstablished); - // self.update_stage_of_overall_connection_status(node_to_ui_recipient); } ConnectionProgressEvent::PassGossipReceived(new_pass_target) => { connection_progress.handle_pass_gossip(new_pass_target); @@ -236,7 +229,6 @@ impl OverallConnectionStatus { } else { self.stage = OverallConnectionStage::ConnectedToNeighbor; } - eprintln!("Updated the Overall Connection Stage: {:?}", self.stage); if self.stage as usize > prev_stage as usize { OverallConnectionStatus::send_message_to_ui(self.stage.into(), node_to_ui_recipient); } @@ -254,7 +246,6 @@ impl OverallConnectionStatus { node_to_ui_recipient .try_send(message) .expect("UI Gateway is unbound."); - eprintln!("message sent"); } pub fn is_empty(&self) -> bool { @@ -287,7 +278,8 @@ mod tests { }; use crate::neighborhood::PublicKey; use crate::test_utils::neighborhood_test_utils::{ - make_ip, make_node_and_recipient, make_node_descriptor, make_node_to_ui_recipient, + make_ip, make_node, make_node_and_recipient, make_node_descriptor, + make_node_to_ui_recipient, }; use actix::System; use masq_lib::blockchains::chains::Chain; @@ -487,7 +479,7 @@ mod tests { #[test] fn updates_the_connection_stage_to_tcp_connection_established() { - let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); OverallConnectionStatus::update_connection_stage( @@ -511,7 +503,7 @@ mod tests { #[test] fn updates_the_connection_stage_to_failed_when_tcp_connection_fails() { - let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); let connection_progress_to_modify = subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); @@ -538,7 +530,7 @@ mod tests { #[test] fn updates_the_connection_stage_to_neighborship_established_when_introduction_gossip_is_received( ) { - let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); let connection_progress = subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); OverallConnectionStatus::update_connection_stage( @@ -563,7 +555,7 @@ mod tests { #[test] fn updates_the_connection_stage_to_neighborship_established_when_standard_gossip_is_received() { - let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); let connection_progress = subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); OverallConnectionStatus::update_connection_stage( @@ -588,7 +580,7 @@ mod tests { #[test] fn updates_the_connection_stage_to_stage_zero_when_pass_gossip_is_received() { - let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); let pass_target = make_ip(1); let connection_progress_to_modify = @@ -619,7 +611,7 @@ mod tests { #[test] fn updates_connection_stage_to_failed_when_pass_loop_is_found() { - let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); let connection_progress_to_modify = subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); @@ -649,7 +641,7 @@ mod tests { #[test] fn updates_connection_stage_to_failed_when_no_gossip_response_is_received() { - let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = OverallConnectionStatus::new(vec![node_descriptor.clone()]); let connection_progress_to_modify = subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); @@ -697,7 +689,7 @@ mod tests { #[test] #[should_panic(expected = "Can't update the stage from StageZero to NeighborshipEstablished")] fn can_t_establish_neighborhsip_without_having_a_tcp_connection() { - let (node_ip_addr, node_descriptor, recipient) = make_node_and_recipient(); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = OverallConnectionStatus::new(vec![node_descriptor]); let connection_progress_to_modify = subject.get_connection_progress_by_ip(node_ip_addr).unwrap(); @@ -860,8 +852,8 @@ mod tests { can_make_routes: bool, test_name: &str, ) -> (OverallConnectionStage, Option) { - let (peer_addr, node_descriptor, _) = make_node_and_recipient(); - let mut subject = OverallConnectionStatus::new(vec![node_descriptor]); + let mut subject = + OverallConnectionStatus::new(vec![make_node_descriptor(make_ip(u8::MAX))]); let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); subject.stage = initial_stage; let system = System::new(test_name); diff --git a/node/src/test_utils/neighborhood_test_utils.rs b/node/src/test_utils/neighborhood_test_utils.rs index fbc536f87..e3e6a8cd3 100644 --- a/node/src/test_utils/neighborhood_test_utils.rs +++ b/node/src/test_utils/neighborhood_test_utils.rs @@ -293,6 +293,13 @@ pub fn make_node_descriptor(ip_addr: IpAddr) -> NodeDescriptor { } } +pub fn make_node(nonce: u8) -> (IpAddr, NodeDescriptor) { + let ip_addr = make_ip(nonce); + let node_descriptor = make_node_descriptor(ip_addr); + + (ip_addr, node_descriptor) +} + pub fn make_node_and_recipient() -> (IpAddr, NodeDescriptor, Recipient) { let ip_addr = make_ip(u8::MAX); let node_descriptor = make_node_descriptor(ip_addr); diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index e7a4cb6fa..caef1add2 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -236,7 +236,7 @@ impl Recorder { self } - pub fn stop_condition(mut self, message_type_id: TypeId) -> Recorder { + pub fn stop_condition(self, message_type_id: TypeId) -> Recorder { let mut expected_count_by_messages: HashMap = HashMap::new(); expected_count_by_messages.insert(message_type_id, 1); self.stop_after_messages(expected_count_by_messages) From 0cc25a21cbc93de15e9582cfa365ba443ea1831e Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 21 Jun 2022 13:14:14 +0530 Subject: [PATCH 12/15] GH-574: eliminate the helper fn make_node_and_recipient() --- node/src/neighborhood/overall_connection_status.rs | 7 +++---- node/src/test_utils/neighborhood_test_utils.rs | 8 -------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index b00318471..38ece9f68 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -278,8 +278,7 @@ mod tests { }; use crate::neighborhood::PublicKey; use crate::test_utils::neighborhood_test_utils::{ - make_ip, make_node, make_node_and_recipient, make_node_descriptor, - make_node_to_ui_recipient, + make_ip, make_node, make_node_descriptor, make_node_to_ui_recipient, }; use actix::System; use masq_lib::blockchains::chains::Chain; @@ -735,7 +734,7 @@ mod tests { #[test] fn we_can_ask_about_can_make_routes() { - let (_node_ip_addr, node_descriptor, _recipient) = make_node_and_recipient(); + let node_descriptor = make_node_descriptor(make_ip(1)); let subject = OverallConnectionStatus::new(vec![node_descriptor]); let can_make_routes = subject.can_make_routes(); @@ -745,7 +744,7 @@ mod tests { #[test] fn can_update_the_boolean_can_make_routes() { - let (_node_ip_addr, node_descriptor, _recipient) = make_node_and_recipient(); + let node_descriptor = make_node_descriptor(make_ip(1)); let mut subject = OverallConnectionStatus::new(vec![node_descriptor]); let can_make_routes_initially = subject.can_make_routes(); diff --git a/node/src/test_utils/neighborhood_test_utils.rs b/node/src/test_utils/neighborhood_test_utils.rs index e3e6a8cd3..f859cb6da 100644 --- a/node/src/test_utils/neighborhood_test_utils.rs +++ b/node/src/test_utils/neighborhood_test_utils.rs @@ -300,14 +300,6 @@ pub fn make_node(nonce: u8) -> (IpAddr, NodeDescriptor) { (ip_addr, node_descriptor) } -pub fn make_node_and_recipient() -> (IpAddr, NodeDescriptor, Recipient) { - let ip_addr = make_ip(u8::MAX); - let node_descriptor = make_node_descriptor(ip_addr); - let (node_to_ui_recipient, _) = make_node_to_ui_recipient(); - - (ip_addr, node_descriptor, node_to_ui_recipient) -} - pub fn make_recipient_and_recording_arc( stopping_message: Option, ) -> (Recipient, Arc>) From 09aca6f703a521259521276e869befdadeb9616f Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 21 Jun 2022 15:06:08 +0530 Subject: [PATCH 13/15] GH-574: add safety measure to recorder and refactor tests for handling CPM in Neighborhood --- node/src/neighborhood/mod.rs | 58 +++++-------------- .../neighborhood/overall_connection_status.rs | 10 ++-- node/src/test_utils/recorder.rs | 7 ++- 3 files changed, 25 insertions(+), 50 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index fcb4eb344..1cf63dd65 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -1391,7 +1391,7 @@ mod tests { use crate::test_utils::make_meaningless_route; use crate::test_utils::make_wallet; use crate::test_utils::neighborhood_test_utils::{ - db_from_node, make_global_cryptde_node_record, make_ip, make_node_descriptor, + db_from_node, make_global_cryptde_node_record, make_ip, make_node, make_node_descriptor, make_node_record, make_node_record_f, make_node_to_ui_recipient, make_recipient_and_recording_arc, neighborhood_from_nodes, }; @@ -1685,16 +1685,12 @@ mod tests { #[test] pub fn neighborhood_handles_connection_progress_message_with_tcp_connection_established() { init_test_logging(); - let node_ip_addr = IpAddr::from_str("5.4.3.2").unwrap(); - let node_descriptor = make_node_descriptor(node_ip_addr); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = make_subject_from_node_descriptor( &node_descriptor, "neighborhood_handles_connection_progress_message_with_tcp_connection_established", ); let notify_later_ask_about_gossip_params_arc = Arc::new(Mutex::new(vec![])); - let (node_to_ui_recipient, node_to_ui_recording_arc) = - make_recipient_and_recording_arc(Some(TypeId::of::())); - subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); subject.tools.notify_later_ask_about_gossip = Box::new( NotifyLaterHandleMock::default() .notify_later_params(¬ify_later_ask_about_gossip_params_arc), @@ -1723,11 +1719,10 @@ mod tests { ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); + System::current().stop(); assert_eq!(system.run(), 0); let notify_later_ask_about_gossip_params = notify_later_ask_about_gossip_params_arc.lock().unwrap(); - let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); - let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); assert_eq!( *notify_later_ask_about_gossip_params, vec![( @@ -1737,14 +1732,12 @@ mod tests { Duration::from_millis(10) )] ); - assert_eq!(node_to_ui_message_opt, None); } #[test] fn ask_about_debut_gossip_message_handles_timeout_in_case_no_response_is_received() { init_test_logging(); - let node_ip_addr = IpAddr::from_str("5.4.3.2").unwrap(); - let node_descriptor = make_node_descriptor(node_ip_addr); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = make_subject_from_node_descriptor( &node_descriptor, "ask_about_debut_gossip_message_handles_timeout_in_case_no_response_is_received", @@ -1795,10 +1788,8 @@ mod tests { #[test] pub fn it_doesn_t_cause_a_panic_if_neighborhood_receives_ask_about_debut_message_from_unknown_descriptor( ) { - let known_ip = make_ip(1); - let known_desc = make_node_descriptor(known_ip); - let unknown_ip = make_ip(2); - let unknown_desc = make_node_descriptor(unknown_ip); + let (_known_ip, known_desc) = make_node(1); + let (unknown_ip, unknown_desc) = make_node(2); let subject = make_subject_from_node_descriptor(&known_desc, "it_doesn_t_cause_a_panic_if_neighborhood_receives_ask_about_debut_message_from_unknown_descriptor"); let initial_ocs = subject.overall_connection_status.clone(); let addr = subject.start(); @@ -1825,8 +1816,7 @@ mod tests { #[test] pub fn neighborhood_handles_connection_progress_message_with_tcp_connection_failed() { init_test_logging(); - let node_ip_addr = IpAddr::from_str("5.4.3.2").unwrap(); - let node_descriptor = make_node_descriptor(node_ip_addr); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = make_subject_from_node_descriptor( &node_descriptor, "neighborhood_handles_connection_progress_message_with_tcp_connection_failed", @@ -1864,14 +1854,11 @@ mod tests { #[test] fn neighborhood_handles_a_connection_progress_message_with_pass_gossip_received() { init_test_logging(); - let node_ip_addr = IpAddr::from_str("5.4.3.2").unwrap(); - let node_descriptor = make_node_descriptor(node_ip_addr); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = make_subject_from_node_descriptor( &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_pass_gossip_received", ); - let (node_to_ui_recipient, node_to_ui_recording_arc) = make_node_to_ui_recipient(); - subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status .get_connection_progress_by_ip(node_ip_addr) @@ -1883,7 +1870,7 @@ mod tests { let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); let system = System::new("testing"); - let new_pass_target = IpAddr::from_str("10.20.30.40").unwrap(); + let new_pass_target = make_ip(2); let connection_progress_message = ConnectionProgressMessage { peer_addr: node_ip_addr, event: ConnectionProgressEvent::PassGossipReceived(new_pass_target), @@ -1903,26 +1890,17 @@ mod tests { }); addr.try_send(AssertionsMessage { assertions }).unwrap(); System::current().stop(); - let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); - let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); assert_eq!(system.run(), 0); - assert_eq!(node_to_ui_message_opt, None); } #[test] fn neighborhood_handles_a_connection_progress_message_with_pass_loop_found() { init_test_logging(); - let node_ip_addr = IpAddr::from_str("5.4.3.2").unwrap(); - let node_descriptor = make_node_descriptor(node_ip_addr); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = make_subject_from_node_descriptor( &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_pass_loop_found", ); - let (recorder, _awaiter, node_to_ui_recording_arc) = make_recorder(); - let recorder = recorder.stop_condition(TypeId::of::()); - let addr = recorder.start(); - let node_to_ui_recipient = addr.recipient::(); - subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); let connection_progress_to_modify = subject .overall_connection_status .get_connection_progress_by_ip(node_ip_addr) @@ -1952,17 +1930,14 @@ mod tests { ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); + System::current().stop(); assert_eq!(system.run(), 0); - let node_to_ui_mutex = node_to_ui_recording_arc.lock().unwrap(); - let node_to_ui_message_opt = node_to_ui_mutex.get_record_opt::(0); - assert_eq!(node_to_ui_message_opt, None); } #[test] fn neighborhood_handles_a_connection_progress_message_with_introduction_gossip_received() { init_test_logging(); - let node_ip_addr = IpAddr::from_str("5.4.3.2").unwrap(); - let node_descriptor = make_node_descriptor(node_ip_addr); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = make_subject_from_node_descriptor( &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_introduction_gossip_received", @@ -1981,10 +1956,9 @@ mod tests { let addr = subject.start(); let cpm_recipient = addr.clone().recipient(); let system = System::new("testing"); - let new_node = IpAddr::from_str("10.20.30.40").unwrap(); let connection_progress_message = ConnectionProgressMessage { peer_addr: node_ip_addr, - event: ConnectionProgressEvent::IntroductionGossipReceived(new_node), + event: ConnectionProgressEvent::IntroductionGossipReceived(make_ip(2)), }; cpm_recipient.try_send(connection_progress_message).unwrap(); @@ -2019,8 +1993,7 @@ mod tests { #[test] fn neighborhood_handles_a_connection_progress_message_with_standard_gossip_received() { init_test_logging(); - let node_ip_addr = IpAddr::from_str("5.4.3.2").unwrap(); - let node_descriptor = make_node_descriptor(node_ip_addr); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = make_subject_from_node_descriptor( &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_standard_gossip_received", @@ -2076,8 +2049,7 @@ mod tests { #[test] fn neighborhood_handles_a_connection_progress_message_with_no_gossip_response_received() { init_test_logging(); - let node_ip_addr = IpAddr::from_str("5.4.3.2").unwrap(); - let node_descriptor = make_node_descriptor(node_ip_addr); + let (node_ip_addr, node_descriptor) = make_node(1); let mut subject = make_subject_from_node_descriptor( &node_descriptor, "neighborhood_handles_a_connection_progress_message_with_no_gossip_response_received", diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index 38ece9f68..ca328b2ce 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -762,7 +762,7 @@ mod tests { let can_make_routes = true; let (stage, message_opt) = - stage_and_ui_message_by_initial_stage_and_can_make_routes( + assert_stage_and_node_to_ui_message( initial_stage, can_make_routes, "updates_the_stage_to_three_hops_route_found_in_case_introduction_or_standard_gossip_is_received_and_flag_is_true" @@ -788,7 +788,7 @@ mod tests { let can_make_routes = false; let (stage, message_opt) = - stage_and_ui_message_by_initial_stage_and_can_make_routes( + assert_stage_and_node_to_ui_message( initial_stage, can_make_routes, "updates_the_stage_to_connected_to_neighbor_in_case_introduction_or_standard_gossip_is_received_and_flag_is_false" @@ -813,7 +813,7 @@ mod tests { let initial_stage = OverallConnectionStage::ConnectedToNeighbor; let can_make_routes = false; - let (stage, message_opt) = stage_and_ui_message_by_initial_stage_and_can_make_routes( + let (stage, message_opt) = assert_stage_and_node_to_ui_message( initial_stage, can_make_routes, "doesn_t_send_message_to_the_ui_in_case_introduction_or_standard_gossip_is_received_but_stage_hasn_t_updated", @@ -830,7 +830,7 @@ mod tests { let can_make_routes = false; let (stage, message_opt) = - stage_and_ui_message_by_initial_stage_and_can_make_routes( + assert_stage_and_node_to_ui_message( initial_stage, can_make_routes, "doesn_t_send_a_message_to_ui_in_case_connection_drops_from_three_hops_to_connected_to_neighbor" @@ -846,7 +846,7 @@ mod tests { assert_eq!(subject.stage(), OverallConnectionStage::NotConnected); } - fn stage_and_ui_message_by_initial_stage_and_can_make_routes( + fn assert_stage_and_node_to_ui_message( initial_stage: OverallConnectionStage, can_make_routes: bool, test_name: &str, diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index caef1add2..c30f1ed13 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -45,6 +45,7 @@ use crate::sub_lib::stream_handler_pool::DispatcherNodeQueryResponse; use crate::sub_lib::stream_handler_pool::TransmitDataMsg; use crate::sub_lib::ui_gateway::UiGatewaySubs; use crate::test_utils::to_millis; +use crate::test_utils::unshared_test_utils::SystemKillerActor; use actix::Addr; use actix::Context; use actix::Handler; @@ -239,13 +240,15 @@ impl Recorder { pub fn stop_condition(self, message_type_id: TypeId) -> Recorder { let mut expected_count_by_messages: HashMap = HashMap::new(); expected_count_by_messages.insert(message_type_id, 1); - self.stop_after_messages(expected_count_by_messages) + self.stop_after_messages_and_start_system_killer(expected_count_by_messages) } - pub fn stop_after_messages( + pub fn stop_after_messages_and_start_system_killer( mut self, expected_count_by_messages: HashMap, ) -> Recorder { + let system_killer = SystemKillerActor::new(Duration::from_secs(60)); + system_killer.start(); self.expected_count_by_msg_type_opt = Some(expected_count_by_messages); self } From 0392704c44d1480cb9e20229122b970874096450 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 21 Jun 2022 15:28:07 +0530 Subject: [PATCH 14/15] GH-574: strengthen tests for handling CPM by asserting on the complete struct of OverallConnectionStatus --- node/src/neighborhood/mod.rs | 120 +++++++++++------- .../neighborhood/overall_connection_status.rs | 4 +- 2 files changed, 78 insertions(+), 46 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 1cf63dd65..cfe35f2cd 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -1714,8 +1714,12 @@ mod tests { let assertions = Box::new(move |actor: &mut Neighborhood| { assert_eq!( - actor.overall_connection_status.progress, - vec![beginning_connection_progress_clone] + actor.overall_connection_status, + OverallConnectionStatus { + can_make_routes: false, + stage: OverallConnectionStage::NotConnected, + progress: vec![beginning_connection_progress_clone] + } ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); @@ -1769,12 +1773,16 @@ mod tests { let assertions = Box::new(move |actor: &mut Neighborhood| { assert_eq!( - actor.overall_connection_status.progress, - vec![ConnectionProgress { - initial_node_descriptor: node_descriptor, - current_peer_addr: node_ip_addr, - connection_stage: ConnectionStage::Failed(NoGossipResponseReceived), - }] + actor.overall_connection_status, + OverallConnectionStatus { + can_make_routes: false, + stage: OverallConnectionStage::NotConnected, + progress: vec![ConnectionProgress { + initial_node_descriptor: node_descriptor, + current_peer_addr: node_ip_addr, + connection_stage: ConnectionStage::Failed(NoGossipResponseReceived), + }] + } ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); @@ -1835,12 +1843,16 @@ mod tests { let assertions = Box::new(move |actor: &mut Neighborhood| { assert_eq!( - actor.overall_connection_status.progress, - vec![ConnectionProgress { - initial_node_descriptor: node_descriptor.clone(), - current_peer_addr: node_ip_addr, - connection_stage: ConnectionStage::Failed(TcpConnectionFailed) - }] + actor.overall_connection_status, + OverallConnectionStatus { + can_make_routes: false, + stage: OverallConnectionStage::NotConnected, + progress: vec![ConnectionProgress { + initial_node_descriptor: node_descriptor.clone(), + current_peer_addr: node_ip_addr, + connection_stage: ConnectionStage::Failed(TcpConnectionFailed) + }] + } ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); @@ -1880,12 +1892,16 @@ mod tests { let assertions = Box::new(move |actor: &mut Neighborhood| { assert_eq!( - actor.overall_connection_status.progress, - vec![ConnectionProgress { - initial_node_descriptor: node_descriptor.clone(), - current_peer_addr: new_pass_target, - connection_stage: ConnectionStage::StageZero - }] + actor.overall_connection_status, + OverallConnectionStatus { + can_make_routes: false, + stage: OverallConnectionStage::NotConnected, + progress: vec![ConnectionProgress { + initial_node_descriptor: node_descriptor.clone(), + current_peer_addr: new_pass_target, + connection_stage: ConnectionStage::StageZero + }] + } ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); @@ -1921,12 +1937,16 @@ mod tests { let assertions = Box::new(move |actor: &mut Neighborhood| { assert_eq!( - actor.overall_connection_status.progress, - vec![ConnectionProgress { - initial_node_descriptor: node_descriptor.clone(), - current_peer_addr: node_ip_addr, - connection_stage: ConnectionStage::Failed(PassLoopFound) - }] + actor.overall_connection_status, + OverallConnectionStatus { + can_make_routes: false, + stage: OverallConnectionStage::NotConnected, + progress: vec![ConnectionProgress { + initial_node_descriptor: node_descriptor.clone(), + current_peer_addr: node_ip_addr, + connection_stage: ConnectionStage::Failed(PassLoopFound) + }] + } ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); @@ -1965,12 +1985,16 @@ mod tests { let assertions = Box::new(move |actor: &mut Neighborhood| { assert_eq!( - actor.overall_connection_status.progress, - vec![ConnectionProgress { - initial_node_descriptor: node_descriptor.clone(), - current_peer_addr: node_ip_addr, - connection_stage: ConnectionStage::NeighborshipEstablished - }] + actor.overall_connection_status, + OverallConnectionStatus { + can_make_routes: false, + stage: OverallConnectionStage::ConnectedToNeighbor, + progress: vec![ConnectionProgress { + initial_node_descriptor: node_descriptor.clone(), + current_peer_addr: node_ip_addr, + connection_stage: ConnectionStage::NeighborshipEstablished + }] + } ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); @@ -2021,12 +2045,16 @@ mod tests { let assertions = Box::new(move |actor: &mut Neighborhood| { assert_eq!( - actor.overall_connection_status.progress, - vec![ConnectionProgress { - initial_node_descriptor: node_descriptor.clone(), - current_peer_addr: node_ip_addr, - connection_stage: ConnectionStage::NeighborshipEstablished - }] + actor.overall_connection_status, + OverallConnectionStatus { + can_make_routes: false, + stage: OverallConnectionStage::ConnectedToNeighbor, + progress: vec![ConnectionProgress { + initial_node_descriptor: node_descriptor.clone(), + current_peer_addr: node_ip_addr, + connection_stage: ConnectionStage::NeighborshipEstablished + }] + } ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); @@ -2074,12 +2102,16 @@ mod tests { let assertions = Box::new(move |actor: &mut Neighborhood| { assert_eq!( - actor.overall_connection_status.progress, - vec![ConnectionProgress { - initial_node_descriptor: node_descriptor.clone(), - current_peer_addr: node_ip_addr, - connection_stage: ConnectionStage::Failed(NoGossipResponseReceived) - }] + actor.overall_connection_status, + OverallConnectionStatus { + can_make_routes: false, + stage: OverallConnectionStage::NotConnected, + progress: vec![ConnectionProgress { + initial_node_descriptor: node_descriptor.clone(), + current_peer_addr: node_ip_addr, + connection_stage: ConnectionStage::Failed(NoGossipResponseReceived) + }] + } ); }); addr.try_send(AssertionsMessage { assertions }).unwrap(); diff --git a/node/src/neighborhood/overall_connection_status.rs b/node/src/neighborhood/overall_connection_status.rs index ca328b2ce..7fae3d666 100644 --- a/node/src/neighborhood/overall_connection_status.rs +++ b/node/src/neighborhood/overall_connection_status.rs @@ -121,9 +121,9 @@ impl From for UiConnectionChangeStage { #[derive(PartialEq, Debug, Clone)] pub struct OverallConnectionStatus { // The check_connectedness() updates the boolean when three hops route is found - can_make_routes: bool, + pub can_make_routes: bool, // Transition depends on the ConnectionProgressMessage & check_connectedness(), they may not be in sync - stage: OverallConnectionStage, + pub stage: OverallConnectionStage, // Corresponds to the initial_node_descriptors, that are entered by the user using --neighbors pub progress: Vec, } From a4a070ff340509bb0ed1cd12b9f37fdb0e55cc43 Mon Sep 17 00:00:00 2001 From: utkarshg6 Date: Tue, 21 Jun 2022 15:33:16 +0530 Subject: [PATCH 15/15] GH-574: remove comment --- node/src/neighborhood/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index cfe35f2cd..549daba51 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -280,7 +280,6 @@ impl Handler for Neighborhood { .expect("UI Gateway is unbound"), ); } - // _ => todo!("don't do anything!"), _ => (), } }