From cff05e54ccee38c0cfe322667058a92e8c7357b3 Mon Sep 17 00:00:00 2001 From: Bert Date: Wed, 24 May 2023 19:18:20 +0200 Subject: [PATCH 1/3] GH-706: cleanup done --- node/src/neighborhood/mod.rs | 79 +++++++++++--------------------- node/src/sub_lib/neighborhood.rs | 2 + 2 files changed, 28 insertions(+), 53 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index c0aa0aead..7234cd29f 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -151,29 +151,6 @@ impl Handler for Neighborhood { } } -impl Handler for Neighborhood { - type Result = MessageResult; - - fn handle( - &mut self, - msg: NodeQueryMessage, - _ctx: &mut Self::Context, - ) -> >::Result { - let node_record_ref_opt = match msg { - NodeQueryMessage::IpAddress(ip_addr) => self.neighborhood_database.node_by_ip(&ip_addr), - NodeQueryMessage::PublicKey(key) => self.neighborhood_database.node_by_key(&key), - }; - - MessageResult(node_record_ref_opt.map(|node_record_ref| { - NodeQueryResponseMetadata::new( - node_record_ref.public_key().clone(), - node_record_ref.node_addr_opt(), - *node_record_ref.rate_pack(), - ) - })) - } -} - impl Handler for Neighborhood { type Result = (); @@ -495,6 +472,7 @@ impl Neighborhood { bind: addr.clone().recipient::(), start: addr.clone().recipient::(), new_public_ip: addr.clone().recipient::(), + #[cfg(test)] node_query: addr.clone().recipient::(), route_query: addr.clone().recipient::(), update_node_record_metadata: addr.clone().recipient::(), @@ -1652,6 +1630,31 @@ mod tests { use crate::test_utils::unshared_test_utils::notify_handlers::NotifyLaterHandleMock; use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; + impl Handler for Neighborhood { + type Result = MessageResult; + + fn handle( + &mut self, + msg: NodeQueryMessage, + _ctx: &mut Self::Context, + ) -> >::Result { + let node_record_ref_opt = match msg { + NodeQueryMessage::IpAddress(ip_addr) => { + self.neighborhood_database.node_by_ip(&ip_addr) + } + NodeQueryMessage::PublicKey(key) => self.neighborhood_database.node_by_key(&key), + }; + + MessageResult(node_record_ref_opt.map(|node_record_ref| { + NodeQueryResponseMetadata::new( + node_record_ref.public_key().clone(), + node_record_ref.node_addr_opt(), + *node_record_ref.rate_pack(), + ) + })) + } + } + impl Handler> for Neighborhood { type Result = (); @@ -5978,34 +5981,4 @@ mod tests { neighborhood } - - pub struct NeighborhoodDatabaseMessage {} - - impl Message for NeighborhoodDatabaseMessage { - type Result = NeighborhoodDatabase; - } - - impl MessageResponse for NeighborhoodDatabase - where - A: Actor, - M: Message, - { - fn handle>(self, _: &mut A::Context, tx: Option) { - if let Some(tx) = tx { - tx.send(self); - } - } - } - - impl Handler for Neighborhood { - type Result = NeighborhoodDatabase; - - fn handle( - &mut self, - _msg: NeighborhoodDatabaseMessage, - _ctx: &mut Self::Context, - ) -> Self::Result { - self.neighborhood_database.clone() - } - } } diff --git a/node/src/sub_lib/neighborhood.rs b/node/src/sub_lib/neighborhood.rs index 4a1b7f78f..40ec7d7e0 100644 --- a/node/src/sub_lib/neighborhood.rs +++ b/node/src/sub_lib/neighborhood.rs @@ -381,6 +381,7 @@ pub struct NeighborhoodSubs { pub bind: Recipient, pub start: Recipient, pub new_public_ip: Recipient, + #[cfg(test)] pub node_query: Recipient, pub route_query: Recipient, pub update_node_record_metadata: Recipient, @@ -428,6 +429,7 @@ pub enum NodeQueryMessage { PublicKey(PublicKey), } +#[cfg(test)] impl Message for NodeQueryMessage { type Result = Option; } From 7664a8bd77318a7867a1e5e7389dc4cb799b37f2 Mon Sep 17 00:00:00 2001 From: Bert Date: Wed, 24 May 2023 19:30:20 +0200 Subject: [PATCH 2/3] GH-706: fixes in imports --- node/src/neighborhood/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 7234cd29f..5ff3fc89e 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -1554,8 +1554,6 @@ impl<'a> ComputedRouteSegment<'a> { #[cfg(test)] mod tests { - use actix::dev::{MessageResponse, ResponseChannel}; - use actix::Message; use actix::Recipient; use actix::System; use itertools::Itertools; From 67bd8aecf09c40307baad0dc9c776544d9d9943b Mon Sep 17 00:00:00 2001 From: Bert Date: Thu, 25 May 2023 19:52:48 +0200 Subject: [PATCH 3/3] GH-706: got rid of unnecessary Actix message with usages --- node/src/neighborhood/mod.rs | 247 ++----------------------------- node/src/sub_lib/neighborhood.rs | 8 - node/src/test_utils/recorder.rs | 19 +-- 3 files changed, 17 insertions(+), 257 deletions(-) diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 5ff3fc89e..978799ca7 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -472,8 +472,6 @@ impl Neighborhood { bind: addr.clone().recipient::(), start: addr.clone().recipient::(), new_public_ip: addr.clone().recipient::(), - #[cfg(test)] - node_query: addr.clone().recipient::(), route_query: addr.clone().recipient::(), update_node_record_metadata: addr.clone().recipient::(), from_hopper: addr.clone().recipient::>(), @@ -1628,31 +1626,6 @@ mod tests { use crate::test_utils::unshared_test_utils::notify_handlers::NotifyLaterHandleMock; use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; - impl Handler for Neighborhood { - type Result = MessageResult; - - fn handle( - &mut self, - msg: NodeQueryMessage, - _ctx: &mut Self::Context, - ) -> >::Result { - let node_record_ref_opt = match msg { - NodeQueryMessage::IpAddress(ip_addr) => { - self.neighborhood_database.node_by_ip(&ip_addr) - } - NodeQueryMessage::PublicKey(key) => self.neighborhood_database.node_by_key(&key), - }; - - MessageResult(node_record_ref_opt.map(|node_record_ref| { - NodeQueryResponseMetadata::new( - node_record_ref.public_key().clone(), - node_record_ref.node_addr_opt(), - *node_record_ref.rate_pack(), - ) - })) - } - } - impl Handler> for Neighborhood { type Result = (); @@ -2518,199 +2491,6 @@ mod tests { tlh.exists_log_containing ("ERROR: Neighborhood: None of the Nodes listed in the --neighbors parameter could accept your Debut; shutting down"); } - #[test] - fn node_query_responds_with_none_when_initially_configured_with_no_data() { - let system = System::new("responds_with_none_when_initially_configured_with_no_data"); - let subject = make_standard_subject(); - let addr = subject.start(); - let sub: Recipient = addr.recipient::(); - - let future = sub.send(NodeQueryMessage::PublicKey(PublicKey::new(&b"booga"[..]))); - - System::current().stop_with_code(0); - system.run(); - let result = future.wait().unwrap(); - assert_eq!(result.is_none(), true); - } - - #[test] - fn node_query_responds_with_none_when_key_query_matches_no_configured_data() { - let cryptde: &dyn CryptDE = main_cryptde(); - let earning_wallet = make_wallet("earning"); - let consuming_wallet = Some(make_paying_wallet(b"consuming")); - let system = - System::new("node_query_responds_with_none_when_key_query_matches_no_configured_data"); - let subject = Neighborhood::new( - cryptde, - &bc_from_nc_plus( - NeighborhoodConfig { - mode: NeighborhoodMode::Standard( - NodeAddr::new(&IpAddr::from_str("5.4.3.2").unwrap(), &[5678]), - vec![NodeDescriptor::from(( - &PublicKey::new(&b"booga"[..]), - &NodeAddr::new(&IpAddr::from_str("1.2.3.4").unwrap(), &[1234, 2345]), - Chain::EthRopsten, - cryptde, - ))], - rate_pack(100), - ), - }, - earning_wallet.clone(), - consuming_wallet.clone(), - "node_query_responds_with_none_when_key_query_matches_no_configured_data", - ), - ); - let addr: Addr = subject.start(); - let sub: Recipient = addr.recipient::(); - - let future = sub.send(NodeQueryMessage::PublicKey(PublicKey::new(&b"blah"[..]))); - - System::current().stop_with_code(0); - system.run(); - let result = future.wait().unwrap(); - assert_eq!(result.is_none(), true); - } - - #[test] - fn node_query_responds_with_result_when_key_query_matches_configured_data() { - let cryptde = main_cryptde(); - let earning_wallet = make_wallet("earning"); - let consuming_wallet = Some(make_paying_wallet(b"consuming")); - let system = - System::new("node_query_responds_with_result_when_key_query_matches_configured_data"); - let one_neighbor = make_node_record(2345, true); - let another_neighbor = make_node_record(3456, true); - let mut subject = Neighborhood::new( - cryptde, - &bc_from_nc_plus( - NeighborhoodConfig { - mode: NeighborhoodMode::Standard( - NodeAddr::new(&IpAddr::from_str("5.4.3.2").unwrap(), &[5678]), - vec![node_record_to_neighbor_config(&one_neighbor)], - rate_pack(100), - ), - }, - earning_wallet.clone(), - consuming_wallet.clone(), - "node_query_responds_with_result_when_key_query_matches_configured_data", - ), - ); - subject - .neighborhood_database - .add_node(another_neighbor.clone()) - .unwrap(); - let addr: Addr = subject.start(); - let sub: Recipient = addr.recipient::(); - - let future = sub.send(NodeQueryMessage::PublicKey( - another_neighbor.public_key().clone(), - )); - - System::current().stop_with_code(0); - system.run(); - let result = future.wait().unwrap(); - assert_eq!( - result.unwrap(), - NodeQueryResponseMetadata::new( - another_neighbor.public_key().clone(), - Some(another_neighbor.node_addr_opt().unwrap().clone()), - another_neighbor.rate_pack().clone(), - ) - ); - } - - #[test] - fn node_query_responds_with_none_when_ip_address_query_matches_no_configured_data() { - let cryptde: &dyn CryptDE = main_cryptde(); - let earning_wallet = make_wallet("earning"); - let consuming_wallet = Some(make_paying_wallet(b"consuming")); - let system = System::new( - "node_query_responds_with_none_when_ip_address_query_matches_no_configured_data", - ); - let subject = Neighborhood::new( - cryptde, - &bc_from_nc_plus( - NeighborhoodConfig { - mode: NeighborhoodMode::Standard( - NodeAddr::new(&IpAddr::from_str("5.4.3.2").unwrap(), &[5678]), - vec![NodeDescriptor::from(( - &PublicKey::new(&b"booga"[..]), - &NodeAddr::new(&IpAddr::from_str("1.2.3.4").unwrap(), &[1234, 2345]), - Chain::EthRopsten, - cryptde, - ))], - rate_pack(100), - ), - }, - earning_wallet.clone(), - consuming_wallet.clone(), - "node_query_responds_with_none_when_ip_address_query_matches_no_configured_data", - ), - ); - let addr: Addr = subject.start(); - let sub: Recipient = addr.recipient::(); - - let future = sub.send(NodeQueryMessage::IpAddress( - IpAddr::from_str("2.3.4.5").unwrap(), - )); - - System::current().stop_with_code(0); - system.run(); - let result = future.wait().unwrap(); - assert_eq!(result.is_none(), true); - } - - #[test] - fn node_query_responds_with_result_when_ip_address_query_matches_configured_data() { - let cryptde: &dyn CryptDE = main_cryptde(); - let system = System::new( - "node_query_responds_with_result_when_ip_address_query_matches_configured_data", - ); - let node_record = make_node_record(1234, true); - let another_node_record = make_node_record(2345, true); - let mut subject = Neighborhood::new( - cryptde, - &bc_from_nc_plus( - NeighborhoodConfig { - mode: NeighborhoodMode::Standard( - node_record.node_addr_opt().unwrap(), - vec![NodeDescriptor::from(( - &node_record, - Chain::EthRopsten, - cryptde, - ))], - rate_pack(100), - ), - }, - node_record.earning_wallet(), - None, - "node_query_responds_with_result_when_ip_address_query_matches_configured_data", - ), - ); - subject - .neighborhood_database - .add_node(another_node_record.clone()) - .unwrap(); - let addr: Addr = subject.start(); - let sub: Recipient = addr.recipient::(); - - let future = sub.send(NodeQueryMessage::IpAddress( - IpAddr::from_str("2.3.4.5").unwrap(), - )); - - System::current().stop_with_code(0); - system.run(); - let result = future.wait().unwrap(); - assert_eq!( - result.unwrap(), - NodeQueryResponseMetadata::new( - another_node_record.public_key().clone(), - Some(another_node_record.node_addr_opt().unwrap().clone()), - another_node_record.rate_pack().clone(), - ) - ); - } - #[test] fn route_query_responds_with_none_when_asked_for_route_with_too_many_hops() { let system = @@ -4756,7 +4536,6 @@ mod tests { single_edge(b, c); single_edge(c, b); } - let addr: Addr = subject.start(); let peer_actors = peer_actors_builder().hopper(hopper).build(); addr.try_send(BindMessage { peer_actors }).unwrap(); @@ -4775,19 +4554,25 @@ mod tests { hostname_opt: None, }; let unsuccessful_three_hop_route = addr.send(three_hop_route_request); - let public_key_query = addr.send(NodeQueryMessage::PublicKey(a.public_key().clone())); - let failed_ip_address_query = addr.send(NodeQueryMessage::IpAddress( - a.node_addr_opt().unwrap().ip_addr(), - )); + let asserted_node_record = a.clone(); + let assertion_msg = AssertionsMessage { + assertions: Box::new(move |neighborhood: &mut Neighborhood| { + let database = &neighborhood.neighborhood_database; + let node_record_by_key = + database.node_by_key(&asserted_node_record.public_key().clone()); + let node_record_by_ip = + database.node_by_ip(&asserted_node_record.node_addr_opt().unwrap().ip_addr()); + assert_eq!( + node_record_by_key.unwrap().public_key(), + asserted_node_record.public_key() + ); + assert_eq!(node_record_by_ip, None) + }), + }; + addr.try_send(assertion_msg).unwrap(); System::current().stop_with_code(0); - system.run(); assert_eq!(None, unsuccessful_three_hop_route.wait().unwrap()); - assert_eq!( - a.public_key(), - &public_key_query.wait().unwrap().unwrap().public_key - ); - assert_eq!(None, failed_ip_address_query.wait().unwrap()); } fn node_record_to_neighbor_config(node_record_ref: &NodeRecord) -> NodeDescriptor { diff --git a/node/src/sub_lib/neighborhood.rs b/node/src/sub_lib/neighborhood.rs index 40ec7d7e0..7ee6c3502 100644 --- a/node/src/sub_lib/neighborhood.rs +++ b/node/src/sub_lib/neighborhood.rs @@ -381,8 +381,6 @@ pub struct NeighborhoodSubs { pub bind: Recipient, pub start: Recipient, pub new_public_ip: Recipient, - #[cfg(test)] - pub node_query: Recipient, pub route_query: Recipient, pub update_node_record_metadata: Recipient, pub from_hopper: Recipient>, @@ -429,11 +427,6 @@ pub enum NodeQueryMessage { PublicKey(PublicKey), } -#[cfg(test)] -impl Message for NodeQueryMessage { - type Result = Option; -} - #[derive(Message, Clone, PartialEq, Eq)] pub struct DispatcherNodeQueryMessage { pub query: NodeQueryMessage, @@ -620,7 +613,6 @@ mod tests { bind: recipient!(recorder, BindMessage), start: recipient!(recorder, StartMessage), new_public_ip: recipient!(recorder, NewPublicIp), - node_query: recipient!(recorder, NodeQueryMessage), route_query: recipient!(recorder, RouteQueryMessage), update_node_record_metadata: recipient!(recorder, NodeRecordMetadataMessage), from_hopper: recipient!(recorder, ExpiredCoresPackage), diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index e87cf8d01..e63844706 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -25,7 +25,7 @@ use crate::sub_lib::hopper::{ExpiredCoresPackage, NoLookupIncipientCoresPackage} use crate::sub_lib::hopper::{HopperSubs, MessageType}; use crate::sub_lib::neighborhood::ConnectionProgressMessage; use crate::sub_lib::neighborhood::NeighborhoodSubs; -use crate::sub_lib::neighborhood::NodeQueryMessage; + use crate::sub_lib::neighborhood::NodeQueryResponseMetadata; use crate::sub_lib::neighborhood::NodeRecordMetadataMessage; use crate::sub_lib::neighborhood::RemoveNeighborMessage; @@ -154,22 +154,6 @@ where } } -impl Handler for Recorder { - type Result = MessageResult; - - fn handle( - &mut self, - msg: NodeQueryMessage, - _ctx: &mut Self::Context, - ) -> >::Result { - self.record(msg); - MessageResult(extract_response( - &mut self.node_query_responses, - "No NodeDescriptors prepared for NodeQueryMessage", - )) - } -} - impl Handler for Recorder { type Result = MessageResult; @@ -401,7 +385,6 @@ pub fn make_neighborhood_subs_from(addr: &Addr) -> NeighborhoodSubs { bind: recipient!(addr, BindMessage), start: recipient!(addr, StartMessage), new_public_ip: recipient!(addr, NewPublicIp), - node_query: recipient!(addr, NodeQueryMessage), route_query: recipient!(addr, RouteQueryMessage), update_node_record_metadata: recipient!(addr, NodeRecordMetadataMessage), from_hopper: recipient!(addr, ExpiredCoresPackage),