diff --git a/.gitignore b/.gitignore index 91a07eaad..cba858c06 100644 --- a/.gitignore +++ b/.gitignore @@ -65,6 +65,7 @@ Temporary Items **/.idea/**/dataSources.ids **/.idea/**/dataSources.xml **/.idea/**/dataSources.local.xml +**/.idea/**/dbnavigator.xml **/.idea/**/sqlDataSources.xml **/.idea/**/dynamic.xml **/.idea/**/uiDesigner.xml diff --git a/masq/src/commands/set_configuration_command.rs b/masq/src/commands/set_configuration_command.rs index df4dbfbc4..4e6ce7c51 100644 --- a/masq/src/commands/set_configuration_command.rs +++ b/masq/src/commands/set_configuration_command.rs @@ -3,8 +3,8 @@ use crate::commands::commands_common::{transaction, Command, CommandError}; use clap::{App, Arg, ArgGroup, SubCommand}; use masq_lib::implement_as_any; use masq_lib::messages::{UiSetConfigurationRequest, UiSetConfigurationResponse}; -use masq_lib::shared_schema::common_validators; -use masq_lib::shared_schema::GAS_PRICE_HELP; +use masq_lib::shared_schema::gas_price_arg; +use masq_lib::shared_schema::min_hops_arg; use masq_lib::short_writeln; use masq_lib::utils::ExpectValue; #[cfg(test)] @@ -66,15 +66,8 @@ const START_BLOCK_HELP: &str = pub fn set_configuration_subcommand() -> App<'static, 'static> { SubCommand::with_name("set-configuration") .about(SET_CONFIGURATION_ABOUT) - .arg( - Arg::with_name("gas-price") - .help(&GAS_PRICE_HELP) - .long("gas-price") - .value_name("GAS-PRICE") - .takes_value(true) - .required(false) - .validator(common_validators::validate_gas_price), - ) + .arg(gas_price_arg()) + .arg(min_hops_arg()) .arg( Arg::with_name("start-block") .help(START_BLOCK_HELP) @@ -86,7 +79,7 @@ pub fn set_configuration_subcommand() -> App<'static, 'static> { ) .group( ArgGroup::with_name("parameter") - .args(&["gas-price", "start-block"]) + .args(&["gas-price", "min-hops", "start-block"]) .required(true), ) } @@ -135,16 +128,43 @@ mod tests { #[test] fn command_execution_works_all_fine() { + test_command_execution("--start-block", "123456"); + test_command_execution("--gas-price", "123456"); + test_command_execution("--min-hops", "6"); + } + + // TODO: This test only passes when we run through IDE - make it work even without it + #[test] + #[ignore] + fn set_configuration_command_throws_err_for_invalid_arg() { + let (invalid_arg, some_value) = ("--invalid-arg", "123"); + + let result = SetConfigurationCommand::new(&[ + "set-configuration".to_string(), + invalid_arg.to_string(), + some_value.to_string(), + ]); + + let err_msg = result.unwrap_err(); + assert!(err_msg.contains( + "error: Found argument '--invalid-arg' \ + which wasn't expected, or isn't valid in this context" + )); + } + + fn test_command_execution(name: &str, value: &str) { let transact_params_arc = Arc::new(Mutex::new(vec![])); let mut context = CommandContextMock::new() .transact_params(&transact_params_arc) .transact_result(Ok(UiSetConfigurationResponse {}.tmb(4321))); let stdout_arc = context.stdout_arc(); let stderr_arc = context.stderr_arc(); - let subject = SetConfigurationCommand { - name: "start-block".to_string(), - value: "123456".to_string(), - }; + let subject = SetConfigurationCommand::new(&[ + "set-configuration".to_string(), + name.to_string(), + value.to_string(), + ]) + .unwrap(); let result = subject.execute(&mut context); @@ -154,15 +174,15 @@ mod tests { *transact_params, vec![( UiSetConfigurationRequest { - name: "start-block".to_string(), - value: "123456".to_string() + name: name[2..].to_string(), + value: value.to_string(), } .tmb(0), 1000 )] ); let stderr = stderr_arc.lock().unwrap(); - assert_eq!(*stderr.get_string(), String::new()); + assert_eq!(&stderr.get_string(), ""); let stdout = stdout_arc.lock().unwrap(); assert_eq!(&stdout.get_string(), "Parameter was successfully set\n"); } diff --git a/masq_lib/src/shared_schema.rs b/masq_lib/src/shared_schema.rs index 01cbbed5e..7c060e6d2 100644 --- a/masq_lib/src/shared_schema.rs +++ b/masq_lib/src/shared_schema.rs @@ -218,6 +218,16 @@ lazy_static! { // These Args are needed in more than one clap schema. To avoid code duplication, they're defined here and referred // to from multiple places. +pub fn chain_arg<'a>() -> Arg<'a, 'a> { + Arg::with_name("chain") + .long("chain") + .value_name("CHAIN") + .min_values(0) + .max_values(1) + .possible_values(official_chain_names()) + .help(CHAIN_HELP) +} + pub fn config_file_arg<'a>() -> Arg<'a, 'a> { Arg::with_name("config-file") .long("config-file") @@ -240,16 +250,7 @@ pub fn data_directory_arg<'a>() -> Arg<'a, 'a> { .help(DATA_DIRECTORY_HELP) } -pub fn chain_arg<'a>() -> Arg<'a, 'a> { - Arg::with_name("chain") - .long("chain") - .value_name("CHAIN") - .min_values(0) - .max_values(1) - .possible_values(official_chain_names()) - .help(CHAIN_HELP) -} - +// TODO: Not an arg fn, move somewhere else pub fn official_chain_names() -> &'static [&'static str] { &[ POLYGON_MAINNET_FULL_IDENTIFIER, @@ -285,6 +286,27 @@ where .help(help) } +pub fn gas_price_arg<'a>() -> Arg<'a, 'a> { + Arg::with_name("gas-price") + .long("gas-price") + .value_name("GAS-PRICE") + .min_values(0) + .max_values(1) + .validator(common_validators::validate_gas_price) + .help(&GAS_PRICE_HELP) +} + +pub fn min_hops_arg<'a>() -> Arg<'a, 'a> { + Arg::with_name("min-hops") + .long("min-hops") + .value_name("MIN_HOPS") + .required(false) + .min_values(0) + .max_values(1) + .possible_values(&["1", "2", "3", "4", "5", "6"]) + .help(MIN_HOPS_HELP) +} + #[cfg(not(target_os = "windows"))] pub fn real_user_arg<'a>() -> Arg<'a, 'a> { Arg::with_name("real-user") @@ -389,15 +411,7 @@ pub fn shared_app(head: App<'static, 'static>) -> App<'static, 'static> { .max_values(1) .hidden(true), ) - .arg( - Arg::with_name("gas-price") - .long("gas-price") - .value_name("GAS-PRICE") - .min_values(0) - .max_values(1) - .validator(common_validators::validate_gas_price) - .help(&GAS_PRICE_HELP), - ) + .arg(gas_price_arg()) .arg( Arg::with_name("ip") .long("ip") @@ -427,16 +441,7 @@ pub fn shared_app(head: App<'static, 'static>) -> App<'static, 'static> { .case_insensitive(true) .help(MAPPING_PROTOCOL_HELP), ) - .arg( - Arg::with_name("min-hops") - .long("min-hops") - .value_name("MIN_HOPS") - .required(false) - .min_values(0) - .max_values(1) - .possible_values(&["1", "2", "3", "4", "5", "6"]) - .help(MIN_HOPS_HELP), - ) + .arg(min_hops_arg()) .arg( Arg::with_name("neighborhood-mode") .long("neighborhood-mode") diff --git a/multinode_integration_tests/tests/data_routing_test.rs b/multinode_integration_tests/tests/data_routing_test.rs index 61a6d6281..d5bfa7f25 100644 --- a/multinode_integration_tests/tests/data_routing_test.rs +++ b/multinode_integration_tests/tests/data_routing_test.rs @@ -70,50 +70,6 @@ fn http_end_to_end_routing_test() { ); } -fn assert_http_end_to_end_routing_test(min_hops: Hops) { - let mut cluster = MASQNodeCluster::start().unwrap(); - let config = NodeStartupConfigBuilder::standard() - .min_hops(min_hops) - .chain(cluster.chain) - .consuming_wallet_info(make_consuming_wallet_info("first_node")) - .build(); - let first_node = cluster.start_real_node(config); - - let nodes_count = 2 * (min_hops as usize) + 1; - let nodes = (0..nodes_count) - .map(|_| { - cluster.start_real_node( - NodeStartupConfigBuilder::standard() - .neighbor(first_node.node_reference()) - .chain(cluster.chain) - .build(), - ) - }) - .collect::>(); - - thread::sleep(Duration::from_millis(500 * (nodes.len() as u64))); - - let mut client = first_node.make_client(8080, 5000); - client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n"); - let response = client.wait_for_chunk(); - - assert_eq!( - index_of(&response, &b"

Example Domain

"[..]).is_some(), - true, - "Actual response:\n{}", - String::from_utf8(response).unwrap() - ); -} - -#[test] -fn http_end_to_end_routing_test_with_different_min_hops() { - // This test fails sometimes due to a timeout: "Couldn't read chunk: Kind(TimedOut)" - // You may fix it by increasing the timeout for the client. - assert_http_end_to_end_routing_test(Hops::OneHop); - assert_http_end_to_end_routing_test(Hops::TwoHops); - assert_http_end_to_end_routing_test(Hops::SixHops); -} - #[test] fn http_end_to_end_routing_test_with_consume_and_originate_only_nodes() { let mut cluster = MASQNodeCluster::start().unwrap(); diff --git a/multinode_integration_tests/tests/min_hops_test.rs b/multinode_integration_tests/tests/min_hops_test.rs new file mode 100644 index 000000000..037303434 --- /dev/null +++ b/multinode_integration_tests/tests/min_hops_test.rs @@ -0,0 +1,121 @@ +// Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. + +use masq_lib::messages::{ToMessageBody, UiSetConfigurationRequest}; +use masq_lib::utils::{find_free_port, index_of}; +use multinode_integration_tests_lib::masq_node::MASQNode; +use multinode_integration_tests_lib::masq_node_cluster::MASQNodeCluster; +use multinode_integration_tests_lib::masq_real_node::{ + make_consuming_wallet_info, MASQRealNode, NodeStartupConfigBuilder, +}; +use node_lib::sub_lib::neighborhood::Hops; +use std::thread; +use std::time::Duration; + +#[test] +fn http_end_to_end_routing_test_with_different_min_hops() { + // This test fails sometimes due to a timeout: "Couldn't read chunk: Kind(TimedOut)" + // You may fix it by increasing the timeout for the client. + assert_http_end_to_end_routing_test(Hops::OneHop); + assert_http_end_to_end_routing_test(Hops::TwoHops); + assert_http_end_to_end_routing_test(Hops::SixHops); +} + +fn assert_http_end_to_end_routing_test(min_hops: Hops) { + let mut cluster = MASQNodeCluster::start().unwrap(); + let config = NodeStartupConfigBuilder::standard() + .min_hops(min_hops) + .chain(cluster.chain) + .consuming_wallet_info(make_consuming_wallet_info("first_node")) + .build(); + let first_node = cluster.start_real_node(config); + + let nodes_count = 2 * (min_hops as usize) + 1; + let nodes = (0..nodes_count) + .map(|_| { + cluster.start_real_node( + NodeStartupConfigBuilder::standard() + .neighbor(first_node.node_reference()) + .chain(cluster.chain) + .build(), + ) + }) + .collect::>(); + + thread::sleep(Duration::from_millis(500 * (nodes.len() as u64))); + + let mut client = first_node.make_client(8080, 5000); + client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n"); + let response = client.wait_for_chunk(); + + assert_eq!( + index_of(&response, &b"

Example Domain

"[..]).is_some(), + true, + "Actual response:\n{}", + String::from_utf8(response).unwrap() + ); +} + +#[test] +fn min_hops_can_be_changed_during_runtime() { + let initial_min_hops = Hops::OneHop; + let new_min_hops = Hops::TwoHops; + let mut cluster = MASQNodeCluster::start().unwrap(); + let ui_port = find_free_port(); + let first_node_config = NodeStartupConfigBuilder::standard() + .min_hops(initial_min_hops) + .chain(cluster.chain) + .consuming_wallet_info(make_consuming_wallet_info("first_node")) + .ui_port(ui_port) + .build(); + let first_node = cluster.start_real_node(first_node_config); + let ui_client = first_node.make_ui(ui_port); + let mut prev_node_reference = first_node.node_reference(); + + for _ in 0..initial_min_hops as u8 { + let new_node_config = NodeStartupConfigBuilder::standard() + .neighbor(prev_node_reference) + .chain(cluster.chain) + .build(); + let new_node = cluster.start_real_node(new_node_config); + prev_node_reference = new_node.node_reference(); + } + thread::sleep(Duration::from_millis(1000)); + + let mut client = first_node.make_client(8080, 5000); + client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n"); + let response = client.wait_for_chunk(); + + // Client shutdown is necessary to re-initialize stream keys for old requests + client.shutdown(); + + assert_eq!( + index_of(&response, &b"

Example Domain

"[..]).is_some(), + true, + "Actual response:\n{}", + String::from_utf8(response).unwrap() + ); + + ui_client.send_request( + UiSetConfigurationRequest { + name: "min-hops".to_string(), + value: new_min_hops.to_string(), + } + .tmb(1), + ); + let response = ui_client.wait_for_response(1, Duration::from_secs(2)); + assert!(response.payload.is_ok()); + + let mut client = first_node.make_client(8080, 5000); + client.send_chunk(b"GET / HTTP/1.1\r\nHost: www.example.com\r\n\r\n"); + let response = client.wait_for_chunk(); + assert_eq!( + index_of( + &response, + &b"

Subtitle: Can't find a route to www.example.com

"[..] + ) + .is_some(), + true, + "Actual response:\n{}", + String::from_utf8(response).unwrap() + ); +} diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index d3bb43cf4..d63ad4a75 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -19,7 +19,6 @@ use crate::sub_lib::blockchain_bridge::{ BlockchainBridgeSubs, ReportAccountsPayable, RequestBalancesToPayPayables, }; use crate::sub_lib::peer_actors::BindMessage; -use crate::sub_lib::set_consuming_wallet_message::SetConsumingWalletMessage; use crate::sub_lib::utils::{db_connection_launch_panic, handle_ui_crash_request}; use crate::sub_lib::wallet::Wallet; use actix::Actor; @@ -45,7 +44,6 @@ pub struct BlockchainBridge { blockchain_interface: Box>, logger: Logger, persistent_config: Box, - set_consuming_wallet_subs_opt: Option>>, sent_payable_subs_opt: Option>, balances_and_payables_sub_opt: Option>, received_payments_subs_opt: Option>, @@ -67,10 +65,6 @@ impl Handler for BlockchainBridge { type Result = (); fn handle(&mut self, msg: BindMessage, _ctx: &mut Self::Context) -> Self::Result { - self.set_consuming_wallet_subs_opt = Some(vec![ - msg.peer_actors.neighborhood.set_consuming_wallet_sub, - msg.peer_actors.proxy_server.set_consuming_wallet_sub, - ]); self.pending_payable_confirmation .new_pp_fingerprints_sub_opt = Some(msg.peer_actors.accountant.init_pending_payable_fingerprints); @@ -199,7 +193,6 @@ impl BlockchainBridge { consuming_wallet_opt, blockchain_interface, persistent_config, - set_consuming_wallet_subs_opt: None, sent_payable_subs_opt: None, balances_and_payables_sub_opt: None, received_payments_subs_opt: None, diff --git a/node/src/neighborhood/gossip_acceptor.rs b/node/src/neighborhood/gossip_acceptor.rs index 3700a448b..95eb8c0a7 100644 --- a/node/src/neighborhood/gossip_acceptor.rs +++ b/node/src/neighborhood/gossip_acceptor.rs @@ -937,8 +937,11 @@ impl GossipHandler for StandardGossipHandler { let initial_neighborship_status = StandardGossipHandler::check_full_neighbor(database, gossip_source.ip()); - let patch = - self.compute_patch(&agrs, database.root(), neighborhood_metadata.min_hops as u8); + let patch = self.compute_patch( + &agrs, + database.root(), + neighborhood_metadata.db_patch_size as u8, + ); let filtered_agrs = self.filter_agrs_by_patch(agrs, patch); let mut db_changed = self.identify_and_add_non_introductory_new_nodes( @@ -986,7 +989,7 @@ impl StandardGossipHandler { &self, agrs: &[AccessibleGossipRecord], root_node: &NodeRecord, - min_hops: u8, + patch_size: u8, ) -> HashSet { let agrs_by_key = agrs .iter() @@ -998,7 +1001,7 @@ impl StandardGossipHandler { &mut patch, root_node.public_key(), &agrs_by_key, - min_hops, + patch_size, root_node, ); @@ -1366,7 +1369,7 @@ mod tests { NeighborhoodMetadata { connection_progress_peers: vec![], cpm_recipient: make_cpm_recipient().0, - min_hops: MIN_HOPS_FOR_TEST, + db_patch_size: MIN_HOPS_FOR_TEST, } } @@ -2382,7 +2385,7 @@ mod tests { .build(); let agrs: Vec = gossip.try_into().unwrap(); - let result = subject.compute_patch(&agrs, node_a_db.root(), MIN_HOPS_FOR_TEST as u8); + let result = subject.compute_patch(&agrs, node_a_db.root(), 3); let expected_hashset = vec![ node_a.public_key().clone(), @@ -2434,7 +2437,7 @@ mod tests { .build(); let agrs: Vec = gossip.try_into().unwrap(); - let patch = subject.compute_patch(&agrs, node_a_db.root(), MIN_HOPS_FOR_TEST as u8); + let patch = subject.compute_patch(&agrs, node_a_db.root(), 3); let expected_hashset = vec![ node_a.public_key().clone(), @@ -2483,7 +2486,7 @@ mod tests { .build(); let agrs: Vec = gossip.try_into().unwrap(); - let patch = subject.compute_patch(&agrs, node_a_db.root(), MIN_HOPS_FOR_TEST as u8); + let patch = subject.compute_patch(&agrs, node_a_db.root(), 3); let expected_hashset = vec![ node_a.public_key().clone(), @@ -2565,17 +2568,17 @@ mod tests { assert_eq!(result, GossipAcceptanceResult::Ignored); } - fn assert_compute_patch(min_hops: Hops) { + fn assert_compute_patch(db_patch_size: Hops) { let subject = StandardGossipHandler::new(Logger::new("assert_compute_patch")); // one node to finish hops and another node that's outside the patch - let nodes_count = min_hops as usize + 2; + let nodes_count = db_patch_size as usize + 2; let nodes = make_node_records(nodes_count as u16); let db = linearly_connect_nodes(&nodes); // gossip is intended for the first node (also root), thereby it's excluded let gossip = gossip_about_nodes_from_database(&db, &nodes[1..]); let agrs: Vec = gossip.try_into().unwrap(); - let result = subject.compute_patch(&agrs, db.root(), min_hops as u8); + let result = subject.compute_patch(&agrs, db.root(), db_patch_size as u8); // last node is excluded because it is outside the patch let expected_nodes = &nodes[0..nodes_count - 1]; @@ -2584,9 +2587,7 @@ mod tests { } #[test] - fn patch_can_be_calculated_for_different_hops() { - assert_compute_patch(Hops::OneHop); - assert_compute_patch(Hops::TwoHops); + fn patch_can_be_calculated_for_realistic_sizes() { assert_compute_patch(Hops::ThreeHops); assert_compute_patch(Hops::FourHops); assert_compute_patch(Hops::FiveHops); diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 5a68bd485..b16d6185a 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -40,17 +40,16 @@ use crate::neighborhood::overall_connection_status::{ OverallConnectionStage, OverallConnectionStatus, }; use crate::stream_messages::RemovedStreamType; -use crate::sub_lib::configurator::NewPasswordMessage; use crate::sub_lib::cryptde::PublicKey; use crate::sub_lib::cryptde::{CryptDE, CryptData, PlainData}; use crate::sub_lib::dispatcher::{Component, StreamShutdownMsg}; use crate::sub_lib::hopper::{ExpiredCoresPackage, NoLookupIncipientCoresPackage}; use crate::sub_lib::hopper::{IncipientCoresPackage, MessageType}; -use crate::sub_lib::neighborhood::NodeRecordMetadataMessage; -use crate::sub_lib::neighborhood::RemoveNeighborMessage; use crate::sub_lib::neighborhood::RouteQueryMessage; use crate::sub_lib::neighborhood::RouteQueryResponse; use crate::sub_lib::neighborhood::{AskAboutDebutGossipMessage, NodeDescriptor}; +use crate::sub_lib::neighborhood::{ConfigurationChange, RemoveNeighborMessage}; +use crate::sub_lib::neighborhood::{ConfigurationChangeMessage, NodeRecordMetadataMessage}; use crate::sub_lib::neighborhood::{ConnectionProgressEvent, ExpectedServices}; use crate::sub_lib::neighborhood::{ConnectionProgressMessage, ExpectedService}; use crate::sub_lib::neighborhood::{DispatcherNodeQueryMessage, GossipFailure_0v1}; @@ -61,7 +60,6 @@ use crate::sub_lib::node_addr::NodeAddr; use crate::sub_lib::peer_actors::{BindMessage, NewPublicIp, StartMessage}; use crate::sub_lib::route::Route; use crate::sub_lib::route::RouteSegment; -use crate::sub_lib::set_consuming_wallet_message::SetConsumingWalletMessage; use crate::sub_lib::stream_handler_pool::DispatcherNodeQueryResponse; use crate::sub_lib::utils::{ db_connection_launch_panic, handle_ui_crash_request, NODE_MAILBOX_CAPACITY, @@ -95,6 +93,7 @@ pub struct Neighborhood { consuming_wallet_opt: Option, mode: NeighborhoodModeLight, min_hops: Hops, + db_patch_size: Hops, next_return_route_id: u32, overall_connection_status: OverallConnectionStatus, chain: Chain, @@ -138,13 +137,27 @@ impl Handler for Neighborhood { } } -//TODO comes across as basically dead code -// I think the idea was to supply the wallet if wallets hadn't been generated until recently during the ongoing Node's run -impl Handler for Neighborhood { +impl Handler for Neighborhood { type Result = (); - fn handle(&mut self, msg: SetConsumingWalletMessage, _ctx: &mut Self::Context) -> Self::Result { - self.consuming_wallet_opt = Some(msg.wallet); + fn handle( + &mut self, + msg: ConfigurationChangeMessage, + _ctx: &mut Self::Context, + ) -> Self::Result { + match msg.change { + ConfigurationChange::UpdateNewPassword(new_password) => { + self.db_password_opt = Some(new_password) + } + ConfigurationChange::UpdateConsumingWallet(new_wallet) => { + self.consuming_wallet_opt = Some(new_wallet) + } + ConfigurationChange::UpdateMinHops(new_min_hops) => { + self.set_min_hops_and_patch_size(new_min_hops); + // TODO: Should we make the stage transition for OverallConnectionStatus from RouteFound to ConnectedToNeighbor before we search for a new route + self.search_for_a_new_route(); + } + } } } @@ -361,14 +374,6 @@ impl Handler for Neighborhood { } } -impl Handler for Neighborhood { - type Result = (); - - fn handle(&mut self, msg: NewPasswordMessage, _ctx: &mut Self::Context) -> Self::Result { - self.handle_new_password(msg.new_password); - } -} - #[derive(Debug, PartialEq, Eq, Clone)] pub struct AccessibleGossipRecord { pub signed_gossip: PlainData, @@ -411,6 +416,7 @@ impl Neighborhood { pub fn new(cryptde: &'static dyn CryptDE, config: &BootstrapperConfig) -> Self { let neighborhood_config = &config.neighborhood_config; let min_hops = neighborhood_config.min_hops; + let db_patch_size = Neighborhood::calculate_db_patch_size(min_hops); let neighborhood_mode = &neighborhood_config.mode; let mode: NeighborhoodModeLight = neighborhood_mode.into(); let neighbor_configs = neighborhood_mode.neighbor_configs(); @@ -455,6 +461,7 @@ impl Neighborhood { consuming_wallet_opt: config.consuming_wallet_opt.clone(), mode, min_hops, + db_patch_size, next_return_route_id: 0, overall_connection_status, chain: config.blockchain_bridge_config.chain, @@ -480,10 +487,9 @@ impl Neighborhood { .recipient::>(), dispatcher_node_query: addr.clone().recipient::(), remove_neighbor: addr.clone().recipient::(), + configuration_change_msg_sub: addr.clone().recipient::(), stream_shutdown_sub: addr.clone().recipient::(), - set_consuming_wallet_sub: addr.clone().recipient::(), from_ui_message_sub: addr.clone().recipient::(), - new_password_sub: addr.clone().recipient::(), connection_progress_sub: addr.clone().recipient::(), } } @@ -748,7 +754,7 @@ impl Neighborhood { let neighborhood_metadata = NeighborhoodMetadata { connection_progress_peers: self.overall_connection_status.get_peer_addrs(), cpm_recipient, - min_hops: self.min_hops, + db_patch_size: self.db_patch_size, }; let acceptance_result = self.gossip_acceptor.handle( &mut self.neighborhood_database, @@ -828,9 +834,16 @@ impl Neighborhood { } fn check_connectedness(&mut self) { - if self.overall_connection_status.can_make_routes() { - return; + if !self.overall_connection_status.can_make_routes() { + self.search_for_a_new_route(); } + } + + fn search_for_a_new_route(&mut self) { + debug!( + self.logger, + "Searching for a {}-hop route...", self.min_hops + ); let msg = RouteQueryMessage { target_key_opt: None, target_component: Component::ProxyClient, @@ -1542,8 +1555,19 @@ impl Neighborhood { ); } - fn handle_new_password(&mut self, new_password: String) { - self.db_password_opt = Some(new_password); + fn calculate_db_patch_size(min_hops: Hops) -> Hops { + if min_hops <= DEFAULT_MIN_HOPS { + DEFAULT_MIN_HOPS + } else { + min_hops + } + } + + fn set_min_hops_and_patch_size(&mut self, new_min_hops: Hops) { + let (prev_min_hops, prev_db_patch_size) = (self.min_hops, self.db_patch_size); + self.min_hops = new_min_hops; + self.db_patch_size = Neighborhood::calculate_db_patch_size(new_min_hops); + debug!(self.logger, "The value of min_hops ({}-hop -> {}-hop) and db_patch_size ({} -> {}) has been changed", prev_min_hops, self.min_hops, prev_db_patch_size, self.db_patch_size); } } @@ -1618,7 +1642,8 @@ mod tests { use crate::sub_lib::hop::LiveHop; use crate::sub_lib::hopper::MessageType; use crate::sub_lib::neighborhood::{ - AskAboutDebutGossipMessage, ExpectedServices, NeighborhoodMode, + AskAboutDebutGossipMessage, ConfigurationChange, ConfigurationChangeMessage, + ExpectedServices, NeighborhoodMode, }; use crate::sub_lib::neighborhood::{NeighborhoodConfig, DEFAULT_RATE_PACK}; use crate::sub_lib::neighborhood::{NeighborhoodMetadata, RatePack}; @@ -1678,7 +1703,7 @@ mod tests { } #[test] - fn min_hops_is_set_inside_neighborhood() { + fn min_hops_and_db_patch_size_is_set_inside_neighborhood() { let min_hops = Hops::SixHops; let mode = NeighborhoodMode::Standard( NodeAddr::new(&make_ip(1), &[1234, 2345]), @@ -1697,7 +1722,9 @@ mod tests { ), ); - assert_eq!(subject.min_hops, Hops::SixHops); + let expected_db_patch_size = Neighborhood::calculate_db_patch_size(min_hops); + assert_eq!(subject.min_hops, min_hops); + assert_eq!(subject.db_patch_size, expected_db_patch_size); } #[test] @@ -2932,13 +2959,13 @@ mod tests { } #[test] - fn can_update_consuming_wallet() { + fn can_update_consuming_wallet_with_configuration_change_msg() { let cryptde = main_cryptde(); let system = System::new("can_update_consuming_wallet"); let (o, r, e, mut subject) = make_o_r_e_subject(); subject.min_hops = Hops::TwoHops; let addr: Addr = subject.start(); - let set_wallet_sub = addr.clone().recipient::(); + let configuration_change_msg_sub = addr.clone().recipient::(); let route_sub = addr.recipient::(); let expected_new_wallet = make_paying_wallet(b"new consuming wallet"); let expected_before_route = Route::round_trip( @@ -2962,9 +2989,11 @@ mod tests { let route_request_1 = route_sub.send(RouteQueryMessage::data_indefinite_route_request(None, 1000)); - let _ = set_wallet_sub.try_send(SetConsumingWalletMessage { - wallet: expected_new_wallet, - }); + configuration_change_msg_sub + .try_send(ConfigurationChangeMessage { + change: ConfigurationChange::UpdateConsumingWallet(expected_new_wallet), + }) + .unwrap(); let route_request_2 = route_sub.send(RouteQueryMessage::data_indefinite_route_request(None, 2000)); @@ -2978,6 +3007,91 @@ mod tests { assert_eq!(route_2, expected_after_route); } + #[test] + fn can_calculate_db_patch_size_from_min_hops() { + assert_eq!( + Neighborhood::calculate_db_patch_size(Hops::OneHop), + Hops::ThreeHops + ); + assert_eq!( + Neighborhood::calculate_db_patch_size(Hops::TwoHops), + Hops::ThreeHops + ); + assert_eq!( + Neighborhood::calculate_db_patch_size(Hops::ThreeHops), + Hops::ThreeHops + ); + assert_eq!( + Neighborhood::calculate_db_patch_size(Hops::FourHops), + Hops::FourHops + ); + assert_eq!( + Neighborhood::calculate_db_patch_size(Hops::FiveHops), + Hops::FiveHops + ); + assert_eq!( + Neighborhood::calculate_db_patch_size(Hops::SixHops), + Hops::SixHops + ); + } + + #[test] + fn can_set_min_hops_and_db_patch_size() { + init_test_logging(); + let test_name = "can_set_min_hops_and_db_patch_size"; + let initial_min_hops = Hops::TwoHops; + let new_min_hops = Hops::FourHops; + let mut subject = make_standard_subject(); + subject.logger = Logger::new(test_name); + subject.min_hops = initial_min_hops; + + subject.set_min_hops_and_patch_size(new_min_hops); + + let expected_db_patch_size = Neighborhood::calculate_db_patch_size(new_min_hops); + assert_eq!(subject.min_hops, new_min_hops); + assert_eq!(subject.db_patch_size, expected_db_patch_size); + TestLogHandler::new().exists_log_containing(&format!( + "DEBUG: {test_name}: The value of min_hops (2-hop -> 4-hop) and db_patch_size (3 -> 4) has been changed" + )); + } + + #[test] + fn can_update_min_hops_with_configuration_change_msg() { + init_test_logging(); + let test_name = "can_update_min_hops_with_configuration_change_msg"; + let system = System::new(test_name); + let mut subject = make_standard_subject(); + subject.min_hops = Hops::TwoHops; + subject.logger = Logger::new(test_name); + subject.overall_connection_status.stage = OverallConnectionStage::RouteFound; + let new_min_hops = Hops::FourHops; + let subject_addr = subject.start(); + let peer_actors = peer_actors_builder().build(); + subject_addr.try_send(BindMessage { peer_actors }).unwrap(); + + subject_addr + .try_send(ConfigurationChangeMessage { + change: ConfigurationChange::UpdateMinHops(new_min_hops), + }) + .unwrap(); + + subject_addr + .try_send(AssertionsMessage { + assertions: Box::new(move |actor: &mut Neighborhood| { + let expected_db_patch_size = + Neighborhood::calculate_db_patch_size(new_min_hops); + assert_eq!(actor.min_hops, new_min_hops); + assert_eq!(actor.db_patch_size, expected_db_patch_size); + }), + }) + .unwrap(); + System::current().stop(); + system.run(); + TestLogHandler::new().exists_log_containing(&format!( + "DEBUG: {test_name}: Searching for a {new_min_hops}-hop route..." + )); + } + #[test] fn compose_route_query_response_returns_an_error_when_route_segment_keys_is_empty() { let mut subject = make_standard_subject(); @@ -3784,6 +3898,7 @@ mod tests { let mut subject = Neighborhood::new(main_cryptde(), &bootstrap_config); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); subject.gossip_acceptor = Box::new(gossip_acceptor); + subject.db_patch_size = Hops::SixHops; let mut peer_2_db = db_from_node(&peer_2); peer_2_db.add_node(peer_1.clone()).unwrap(); peer_2_db.add_arbitrary_full_neighbor(peer_2.public_key(), peer_1.public_key()); @@ -3795,6 +3910,8 @@ mod tests { subject.handle_agrs(agrs, peer_2_socket_addr, make_cpm_recipient().0); + let (_, _, _, neighborhood_metadata) = handle_params_arc.lock().unwrap().remove(0); + assert_eq!(neighborhood_metadata.db_patch_size, Hops::SixHops); TestLogHandler::new() .exists_log_containing(&format!("Gossip from {} ignored", peer_2_socket_addr)); } @@ -3858,10 +3975,10 @@ mod tests { } #[test] - fn neighborhood_logs_when_three_hops_route_can_not_be_made() { + fn neighborhood_logs_when_min_hops_route_can_not_be_made() { init_test_logging(); - let test_name = "neighborhood_logs_when_three_hops_route_can_not_be_made"; - let mut subject: Neighborhood = make_neighborhood_with_linearly_connected_nodes(3); + let test_name = "neighborhood_logs_when_min_hops_route_can_not_be_made"; + let mut subject: Neighborhood = make_neighborhood_with_linearly_connected_nodes(5); let (ui_gateway, _, ui_gateway_arc) = make_recorder(); let (accountant, _, _) = make_recorder(); let node_to_ui_recipient = ui_gateway.start().recipient::(); @@ -3869,6 +3986,7 @@ mod tests { subject.logger = Logger::new(test_name); subject.node_to_ui_recipient_opt = Some(node_to_ui_recipient); subject.connected_signal_opt = Some(connected_signal); + subject.min_hops = Hops::FiveHops; let system = System::new(test_name); subject.handle_gossip_agrs( @@ -5664,7 +5782,7 @@ mod tests { } #[test] - fn new_password_message_works() { + fn can_update_new_password_with_configuration_change_msg() { let system = System::new("test"); let mut subject = make_standard_subject(); let root_node_record = subject.neighborhood_database.root().clone(); @@ -5678,8 +5796,8 @@ mod tests { subject_addr.try_send(BindMessage { peer_actors }).unwrap(); subject_addr - .try_send(NewPasswordMessage { - new_password: "borkety-bork".to_string(), + .try_send(ConfigurationChangeMessage { + change: ConfigurationChange::UpdateNewPassword("borkety-bork".to_string()), }) .unwrap(); diff --git a/node/src/neighborhood/neighborhood_database.rs b/node/src/neighborhood/neighborhood_database.rs index 79a4d1d1f..ceabe133a 100644 --- a/node/src/neighborhood/neighborhood_database.rs +++ b/node/src/neighborhood/neighborhood_database.rs @@ -161,7 +161,10 @@ impl NeighborhoodDatabase { Err(NodeRecordError::SelfNeighborAttempt(key)) => { Err(NeighborhoodDatabaseError::SelfNeighborAttempt(key)) } - Ok(_) => Ok(true), + Ok(_) => { + node.metadata.last_update = time_t_timestamp(); + Ok(true) + } }, None => Err(NodeKeyNotFound(node_key.clone())), } @@ -662,10 +665,15 @@ mod tests { &CryptDENull::from(this_node.public_key(), TEST_DEFAULT_CHAIN), ); subject.add_node(other_node.clone()).unwrap(); + subject.root_mut().metadata.last_update = time_t_timestamp() - 2; + let before = time_t_timestamp(); let result = subject.add_half_neighbor(other_node.public_key()); + let after = time_t_timestamp(); assert_eq!(Ok(true), result, "add_arbitrary_neighbor done goofed"); + assert!(before <= subject.root().metadata.last_update); + assert!(subject.root().metadata.last_update <= after); } #[test] diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index 7a9da5f62..656531e4f 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -1,6 +1,7 @@ // Copyright (c) 2019-2021, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. use std::path::PathBuf; +use std::str::FromStr; use actix::{Actor, Context, Handler, Recipient}; @@ -25,7 +26,8 @@ use crate::db_config::config_dao::ConfigDaoReal; use crate::db_config::persistent_configuration::{ PersistentConfigError, PersistentConfiguration, PersistentConfigurationReal, }; -use crate::sub_lib::configurator::NewPasswordMessage; +use crate::sub_lib::neighborhood::ConfigurationChange::UpdateMinHops; +use crate::sub_lib::neighborhood::{ConfigurationChange, ConfigurationChangeMessage, Hops}; use crate::sub_lib::peer_actors::BindMessage; use crate::sub_lib::utils::{db_connection_launch_panic, handle_ui_crash_request}; use crate::sub_lib::wallet::Wallet; @@ -45,8 +47,8 @@ pub const CRASH_KEY: &str = "CONFIGURATOR"; pub struct Configurator { persistent_config: Box, - node_to_ui_sub: Option>, - new_password_subs: Option>>, + node_to_ui_sub_opt: Option>, + configuration_change_msg_sub_opt: Option>, crashable: bool, logger: Logger, } @@ -59,8 +61,9 @@ impl Handler for Configurator { type Result = (); fn handle(&mut self, msg: BindMessage, _ctx: &mut Self::Context) -> Self::Result { - self.node_to_ui_sub = Some(msg.peer_actors.ui_gateway.node_to_ui_message_sub.clone()); - self.new_password_subs = Some(vec![msg.peer_actors.neighborhood.new_password_sub]) + self.node_to_ui_sub_opt = Some(msg.peer_actors.ui_gateway.node_to_ui_message_sub.clone()); + self.configuration_change_msg_sub_opt = + Some(msg.peer_actors.neighborhood.configuration_change_msg_sub); } } @@ -68,6 +71,7 @@ impl Handler for Configurator { type Result = (); fn handle(&mut self, msg: NodeFromUiMessage, _ctx: &mut Self::Context) -> Self::Result { + // TODO: I wish if we would log the body and context_id of each request over here. Is there a security risk? if let Ok((body, context_id)) = UiChangePasswordRequest::fmb(msg.body.clone()) { let client_id = msg.client_id; self.call_handler(msg, |c| { @@ -107,8 +111,8 @@ impl Configurator { Box::new(PersistentConfigurationReal::new(Box::new(config_dao))); Configurator { persistent_config, - node_to_ui_sub: None, - new_password_subs: None, + node_to_ui_sub_opt: None, + configuration_change_msg_sub_opt: None, crashable, logger: Logger::new("Configurator"), } @@ -690,10 +694,18 @@ impl Configurator { msg: UiSetConfigurationRequest, context_id: u64, ) -> MessageBody { + let configuration_change_msg_sub_opt = self.configuration_change_msg_sub_opt.clone(); + let logger = &self.logger; + debug!( + logger, + "A request from UI received: {:?} from context id: {}", msg, context_id + ); match Self::unfriendly_handle_set_configuration( msg, context_id, &mut self.persistent_config, + configuration_change_msg_sub_opt, + logger, ) { Ok(message_body) => message_body, Err((code, msg)) => MessageBody { @@ -707,16 +719,25 @@ impl Configurator { fn unfriendly_handle_set_configuration( msg: UiSetConfigurationRequest, context_id: u64, - persist_config: &mut Box, + persistent_config: &mut Box, + configuration_change_msg_sub_opt: Option>, + logger: &Logger, ) -> Result { let password: Option = None; //prepared for an upgrade with parameters requiring the password match password { None => { if "gas-price" == &msg.name { - Self::set_gas_price(msg.value, persist_config)?; + Self::set_gas_price(msg.value, persistent_config)?; } else if "start-block" == &msg.name { - Self::set_start_block(msg.value, persist_config)?; + Self::set_start_block(msg.value, persistent_config)?; + } else if "min-hops" == &msg.name { + Self::set_min_hops( + msg.value, + persistent_config, + configuration_change_msg_sub_opt, + logger, + )?; } else { return Err(( UNRECOGNIZED_PARAMETER, @@ -746,6 +767,38 @@ impl Configurator { } } + fn set_min_hops( + string_number: String, + config: &mut Box, + configuration_change_msg_sub_opt: Option>, + logger: &Logger, + ) -> Result<(), (u64, String)> { + let min_hops = match Hops::from_str(&string_number) { + Ok(min_hops) => min_hops, + Err(e) => { + return Err((NON_PARSABLE_VALUE, format!("min hops: {:?}", e))); + } + }; + match config.set_min_hops(min_hops) { + Ok(_) => { + debug!( + logger, + "The value of min-hops has been changed to {}-hop inside the database", + min_hops + ); + configuration_change_msg_sub_opt + .as_ref() + .expect("Configurator is unbound") + .try_send(ConfigurationChangeMessage { + change: UpdateMinHops(min_hops), + }) + .expect("Configurator is unbound"); + Ok(()) + } + Err(e) => Err((CONFIGURATOR_WRITE_ERROR, format!("min hops: {:?}", e))), + } + } + fn set_start_block( string_number: String, config: &mut Box, @@ -762,7 +815,7 @@ impl Configurator { fn send_to_ui_gateway(&self, target: MessageTarget, body: MessageBody) { let msg = NodeToUiMessage { target, body }; - self.node_to_ui_sub + self.node_to_ui_sub_opt .as_ref() .expect("Configurator is unbound") .try_send(msg) @@ -770,15 +823,14 @@ impl Configurator { } fn send_password_changes(&self, new_password: String) { - let msg = NewPasswordMessage { new_password }; - self.new_password_subs + let msg = ConfigurationChangeMessage { + change: ConfigurationChange::UpdateNewPassword(new_password), + }; + self.configuration_change_msg_sub_opt .as_ref() .expect("Configurator is unbound") - .iter() - .for_each(|sub| { - sub.try_send(msg.clone()) - .expect("New password recipient is dead") - }); + .try_send(msg) + .expect("New password recipient is dead"); } fn call_handler MessageBody>( @@ -834,7 +886,7 @@ mod tests { use crate::sub_lib::accountant::{PaymentThresholds, ScanIntervals}; use crate::sub_lib::cryptde::PublicKey as PK; use crate::sub_lib::cryptde::{CryptDE, PlainData}; - use crate::sub_lib::neighborhood::{NodeDescriptor, RatePack}; + use crate::sub_lib::neighborhood::{ConfigurationChange, NodeDescriptor, RatePack}; use crate::sub_lib::node_addr::NodeAddr; use crate::sub_lib::wallet::Wallet; use crate::test_utils::unshared_test_utils::{ @@ -865,11 +917,13 @@ mod tests { ))); let (recorder, _, _) = make_recorder(); let recorder_addr = recorder.start(); + let (neighborhood, _, _) = make_recorder(); + let neighborhood_addr = neighborhood.start(); let mut subject = Configurator::new(data_dir, false); - subject.node_to_ui_sub = Some(recorder_addr.recipient()); - subject.new_password_subs = Some(vec![]); + subject.node_to_ui_sub_opt = Some(recorder_addr.recipient()); + subject.configuration_change_msg_sub_opt = Some(neighborhood_addr.recipient()); let _ = subject.handle_change_password( UiChangePasswordRequest { old_password_opt: None, @@ -1034,9 +1088,9 @@ mod tests { ); let neighborhood_recording = neighborhood_recording_arc.lock().unwrap(); assert_eq!( - neighborhood_recording.get_record::(0), - &NewPasswordMessage { - new_password: "new_password".to_string() + neighborhood_recording.get_record::(0), + &ConfigurationChangeMessage { + change: ConfigurationChange::UpdateNewPassword("new_password".to_string()) } ); assert_eq!(neighborhood_recording.len(), 1); @@ -1934,24 +1988,28 @@ mod tests { #[test] fn handle_set_configuration_works() { + init_test_logging(); + let test_name = "handle_set_configuration_works"; let set_start_block_params_arc = Arc::new(Mutex::new(vec![])); let (ui_gateway, _, ui_gateway_recording_arc) = make_recorder(); let persistent_config = PersistentConfigurationMock::new() .set_start_block_params(&set_start_block_params_arc) .set_start_block_result(Ok(())); - let subject = make_subject(Some(persistent_config)); + let mut subject = make_subject(Some(persistent_config)); + subject.logger = Logger::new(test_name); let subject_addr = subject.start(); let peer_actors = peer_actors_builder().ui_gateway(ui_gateway).build(); subject_addr.try_send(BindMessage { peer_actors }).unwrap(); + let msg = UiSetConfigurationRequest { + name: "start-block".to_string(), + value: "166666".to_string(), + }; + let context_id = 4444; subject_addr .try_send(NodeFromUiMessage { client_id: 1234, - body: UiSetConfigurationRequest { - name: "start-block".to_string(), - value: "166666".to_string(), - } - .tmb(4444), + body: msg.clone().tmb(context_id), }) .unwrap(); @@ -1964,6 +2022,10 @@ mod tests { assert_eq!(context_id, 4444); let check_start_block_params = set_start_block_params_arc.lock().unwrap(); assert_eq!(*check_start_block_params, vec![166666]); + TestLogHandler::new().exists_log_containing(&format!( + "DEBUG: {}: A request from UI received: {:?} from context id: {}", + test_name, msg, context_id + )); } #[test] @@ -2101,6 +2163,121 @@ mod tests { ); } + #[test] + fn handle_set_configuration_works_for_min_hops() { + init_test_logging(); + let test_name = "handle_set_configuration_works_for_min_hops"; + let new_min_hops = Hops::SixHops; + let set_min_hops_params_arc = Arc::new(Mutex::new(vec![])); + let persistent_config = PersistentConfigurationMock::new() + .set_min_hops_params(&set_min_hops_params_arc) + .set_min_hops_result(Ok(())); + let system = System::new("handle_set_configuration_works_for_min_hops"); + let (neighborhood, _, neighborhood_recording_arc) = make_recorder(); + let neighborhood_addr = neighborhood.start(); + let mut subject = make_subject(Some(persistent_config)); + subject.logger = Logger::new(test_name); + subject.configuration_change_msg_sub_opt = + Some(neighborhood_addr.recipient::()); + + let result = subject.handle_set_configuration( + UiSetConfigurationRequest { + name: "min-hops".to_string(), + value: new_min_hops.to_string(), + }, + 4000, + ); + + System::current().stop(); + system.run(); + let neighborhood_recording = neighborhood_recording_arc.lock().unwrap(); + let message_to_neighborhood = + neighborhood_recording.get_record::(0); + let set_min_hops_params = set_min_hops_params_arc.lock().unwrap(); + let min_hops_in_db = set_min_hops_params.get(0).unwrap(); + assert_eq!( + result, + MessageBody { + opcode: "setConfiguration".to_string(), + path: MessagePath::Conversation(4000), + payload: Ok(r#"{}"#.to_string()) + } + ); + assert_eq!( + message_to_neighborhood, + &ConfigurationChangeMessage { + change: ConfigurationChange::UpdateMinHops(new_min_hops) + } + ); + assert_eq!(*min_hops_in_db, new_min_hops); + TestLogHandler::new().exists_log_containing(&format!( + "DEBUG: {test_name}: The value of min-hops has been changed to {new_min_hops}-hop inside the database" + )); + } + + #[test] + fn handle_set_configuration_throws_err_for_invalid_min_hops() { + let mut subject = make_subject(None); + + let result = subject.handle_set_configuration( + UiSetConfigurationRequest { + name: "min-hops".to_string(), + value: "600".to_string(), + }, + 4000, + ); + + assert_eq!( + result, + MessageBody { + opcode: "setConfiguration".to_string(), + path: MessagePath::Conversation(4000), + payload: Err(( + NON_PARSABLE_VALUE, + "min hops: \"Invalid value for min hops provided\"".to_string() + )) + } + ); + } + + #[test] + fn handle_set_configuration_handles_failure_on_min_hops_database_issue() { + let persistent_config = PersistentConfigurationMock::new() + .set_min_hops_result(Err(PersistentConfigError::TransactionError)); + let system = + System::new("handle_set_configuration_handles_failure_on_min_hops_database_issue"); + let (neighborhood, _, neighborhood_recording_arc) = make_recorder(); + let configuration_change_msg_sub = neighborhood + .start() + .recipient::(); + let mut subject = make_subject(Some(persistent_config)); + subject.configuration_change_msg_sub_opt = Some(configuration_change_msg_sub); + + let result = subject.handle_set_configuration( + UiSetConfigurationRequest { + name: "min-hops".to_string(), + value: "4".to_string(), + }, + 4000, + ); + + System::current().stop(); + system.run(); + let recording = neighborhood_recording_arc.lock().unwrap(); + assert!(recording.is_empty()); + assert_eq!( + result, + MessageBody { + opcode: "setConfiguration".to_string(), + path: MessagePath::Conversation(4000), + payload: Err(( + CONFIGURATOR_WRITE_ERROR, + "min hops: TransactionError".to_string() + )) + } + ); + } + #[test] fn handle_set_configuration_complains_about_unexpected_parameter() { let persistent_config = PersistentConfigurationMock::new(); @@ -2547,8 +2724,8 @@ mod tests { fn from(persistent_config: Box) -> Self { Configurator { persistent_config, - node_to_ui_sub: None, - new_password_subs: None, + node_to_ui_sub_opt: None, + configuration_change_msg_sub_opt: None, crashable: false, logger: Logger::new("Configurator"), } diff --git a/node/src/proxy_server/mod.rs b/node/src/proxy_server/mod.rs index 3f2ed1357..4149a09e0 100644 --- a/node/src/proxy_server/mod.rs +++ b/node/src/proxy_server/mod.rs @@ -33,7 +33,6 @@ use crate::sub_lib::proxy_server::ClientRequestPayload_0v1; use crate::sub_lib::proxy_server::ProxyServerSubs; use crate::sub_lib::proxy_server::{AddReturnRouteMessage, AddRouteMessage}; use crate::sub_lib::route::Route; -use crate::sub_lib::set_consuming_wallet_message::SetConsumingWalletMessage; use crate::sub_lib::stream_handler_pool::TransmitDataMsg; use crate::sub_lib::stream_key::StreamKey; use crate::sub_lib::ttl_hashmap::TtlHashMap; @@ -108,21 +107,6 @@ impl Handler for ProxyServer { } } -//TODO comes across as basically dead code -// I think the idea was to supply the wallet if wallets hadn't been generated until recently, without the need to kill the Node -// I also found out that there is a test for this, but it changes nothing on it's normally unused -impl Handler for ProxyServer { - type Result = (); - - fn handle( - &mut self, - _msg: SetConsumingWalletMessage, - _ctx: &mut Self::Context, - ) -> Self::Result { - self.consuming_wallet_balance = Some(0); - } -} - impl Handler for ProxyServer { type Result = (); @@ -247,7 +231,6 @@ impl ProxyServer { add_return_route: recipient!(addr, AddReturnRouteMessage), add_route: recipient!(addr, AddRouteMessage), stream_shutdown_sub: recipient!(addr, StreamShutdownMsg), - set_consuming_wallet_sub: recipient!(addr, SetConsumingWalletMessage), node_from_ui: recipient!(addr, NodeFromUiMessage), } } @@ -1996,86 +1979,6 @@ mod tests { assert_eq!(record, &expected_pkg); } - #[test] - fn proxy_server_applies_late_wallet_information() { - let main_cryptde = main_cryptde(); - let alias_cryptde = alias_cryptde(); - let http_request = b"GET /index.html HTTP/1.1\r\nHost: nowhere.com\r\n\r\n"; - let hopper_mock = Recorder::new(); - let hopper_log_arc = hopper_mock.get_recording(); - let hopper_awaiter = hopper_mock.get_awaiter(); - let destination_key = PublicKey::from(&b"our destination"[..]); - let route_query_response = RouteQueryResponse { - route: Route { hops: vec![] }, - expected_services: ExpectedServices::RoundTrip( - vec![make_exit_service_from_key(destination_key.clone())], - vec![], - 1234, - ), - }; - let socket_addr = SocketAddr::from_str("1.2.3.4:5678").unwrap(); - let stream_key = make_meaningless_stream_key(); - let expected_data = http_request.to_vec(); - let msg_from_dispatcher = InboundClientData { - timestamp: SystemTime::now(), - peer_addr: socket_addr.clone(), - reception_port: Some(HTTP_PORT), - sequence_number: Some(0), - last_data: true, - is_clandestine: false, - data: expected_data.clone(), - }; - let expected_http_request = PlainData::new(http_request); - let route = route_query_response.route.clone(); - let expected_payload = ClientRequestPayload_0v1 { - stream_key: stream_key.clone(), - sequenced_packet: SequencedPacket { - data: expected_http_request.into(), - sequence_number: 0, - last_data: true, - }, - target_hostname: Some(String::from("nowhere.com")), - target_port: HTTP_PORT, - protocol: ProxyProtocol::HTTP, - originator_public_key: alias_cryptde.public_key().clone(), - }; - let expected_pkg = IncipientCoresPackage::new( - main_cryptde, - route, - expected_payload.into(), - &destination_key, - ) - .unwrap(); - thread::spawn(move || { - let stream_key_factory = StreamKeyFactoryMock::new(); // can't make any stream keys; shouldn't have to - let system = System::new("proxy_server_applies_late_wallet_information"); - let mut subject = ProxyServer::new(main_cryptde, alias_cryptde, true, None, false); - subject.stream_key_factory = Box::new(stream_key_factory); - subject.keys_and_addrs.insert(stream_key, socket_addr); - subject - .stream_key_routes - .insert(stream_key, route_query_response); - let subject_addr: Addr = subject.start(); - let mut peer_actors = peer_actors_builder().hopper(hopper_mock).build(); - peer_actors.proxy_server = ProxyServer::make_subs_from(&subject_addr); - subject_addr.try_send(BindMessage { peer_actors }).unwrap(); - - subject_addr - .try_send(SetConsumingWalletMessage { - wallet: make_wallet("Consuming wallet"), - }) - .unwrap(); - - subject_addr.try_send(msg_from_dispatcher).unwrap(); - system.run(); - }); - - hopper_awaiter.await_message_count(1); - let recording = hopper_log_arc.lock().unwrap(); - let record = recording.get_record::(0); - assert_eq!(record, &expected_pkg); - } - #[test] fn proxy_server_receives_http_request_from_dispatcher_then_sends_multihop_cores_package_to_hopper( ) { diff --git a/node/src/sub_lib/configurator.rs b/node/src/sub_lib/configurator.rs index 43f5c3319..c5612be3d 100644 --- a/node/src/sub_lib/configurator.rs +++ b/node/src/sub_lib/configurator.rs @@ -6,11 +6,6 @@ use masq_lib::ui_gateway::NodeFromUiMessage; use std::fmt; use std::fmt::{Debug, Formatter}; -#[derive(Debug, actix::Message, Clone, PartialEq, Eq)] -pub struct NewPasswordMessage { - pub new_password: String, -} - #[derive(Clone, PartialEq, Eq)] pub struct ConfiguratorSubs { pub bind: Recipient, diff --git a/node/src/sub_lib/mod.rs b/node/src/sub_lib/mod.rs index 2bbc553ab..51360357b 100644 --- a/node/src/sub_lib/mod.rs +++ b/node/src/sub_lib/mod.rs @@ -34,7 +34,6 @@ pub mod proxy_server; pub mod route; pub mod sequence_buffer; pub mod sequencer; -pub mod set_consuming_wallet_message; pub mod socket_server; pub mod stream_connector; pub mod stream_handler_pool; diff --git a/node/src/sub_lib/neighborhood.rs b/node/src/sub_lib/neighborhood.rs index f70ca1857..e01a35f12 100644 --- a/node/src/sub_lib/neighborhood.rs +++ b/node/src/sub_lib/neighborhood.rs @@ -4,7 +4,6 @@ use crate::neighborhood::gossip::Gossip_0v1; use crate::neighborhood::node_record::NodeRecord; use crate::neighborhood::overall_connection_status::ConnectionProgress; use crate::neighborhood::Neighborhood; -use crate::sub_lib::configurator::NewPasswordMessage; use crate::sub_lib::cryptde::{CryptDE, PublicKey}; use crate::sub_lib::cryptde_real::CryptDEReal; use crate::sub_lib::dispatcher::{Component, StreamShutdownMsg}; @@ -12,7 +11,6 @@ use crate::sub_lib::hopper::ExpiredCoresPackage; use crate::sub_lib::node_addr::NodeAddr; use crate::sub_lib::peer_actors::{BindMessage, NewPublicIp, StartMessage}; use crate::sub_lib::route::Route; -use crate::sub_lib::set_consuming_wallet_message::SetConsumingWalletMessage; use crate::sub_lib::stream_handler_pool::DispatcherNodeQueryResponse; use crate::sub_lib::stream_handler_pool::TransmitDataMsg; use crate::sub_lib::utils::{NotifyLaterHandle, NotifyLaterHandleReal}; @@ -370,7 +368,7 @@ impl Display for DescriptorParsingError<'_> { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq)] pub enum Hops { OneHop = 1, TwoHops = 2, @@ -423,10 +421,9 @@ pub struct NeighborhoodSubs { pub gossip_failure: Recipient>, pub dispatcher_node_query: Recipient, pub remove_neighbor: Recipient, + pub configuration_change_msg_sub: Recipient, pub stream_shutdown_sub: Recipient, - pub set_consuming_wallet_sub: Recipient, pub from_ui_message_sub: Recipient, - pub new_password_sub: Recipient, pub connection_progress_sub: Recipient, } @@ -555,6 +552,18 @@ pub enum NRMetadataChange { AddUnreachableHost { hostname: String }, } +#[derive(Clone, Debug, Message, PartialEq, Eq)] +pub struct ConfigurationChangeMessage { + pub change: ConfigurationChange, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ConfigurationChange { + UpdateNewPassword(String), + UpdateConsumingWallet(Wallet), + UpdateMinHops(Hops), +} + #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[allow(non_camel_case_types)] pub enum GossipFailure_0v1 { @@ -582,7 +591,7 @@ impl fmt::Display for GossipFailure_0v1 { pub struct NeighborhoodMetadata { pub connection_progress_peers: Vec, pub cpm_recipient: Recipient, - pub min_hops: Hops, + pub db_patch_size: Hops, } pub struct NeighborhoodTools { @@ -659,10 +668,9 @@ mod tests { gossip_failure: recipient!(recorder, ExpiredCoresPackage), dispatcher_node_query: recipient!(recorder, DispatcherNodeQueryMessage), remove_neighbor: recipient!(recorder, RemoveNeighborMessage), + configuration_change_msg_sub: recipient!(recorder, ConfigurationChangeMessage), stream_shutdown_sub: recipient!(recorder, StreamShutdownMsg), - set_consuming_wallet_sub: recipient!(recorder, SetConsumingWalletMessage), from_ui_message_sub: recipient!(recorder, NodeFromUiMessage), - new_password_sub: recipient!(recorder, NewPasswordMessage), connection_progress_sub: recipient!(recorder, ConnectionProgressMessage), }; diff --git a/node/src/sub_lib/proxy_server.rs b/node/src/sub_lib/proxy_server.rs index 5d3b242ae..8706d2943 100644 --- a/node/src/sub_lib/proxy_server.rs +++ b/node/src/sub_lib/proxy_server.rs @@ -8,7 +8,6 @@ use crate::sub_lib::neighborhood::{ExpectedService, RouteQueryResponse}; use crate::sub_lib::peer_actors::BindMessage; use crate::sub_lib::proxy_client::{ClientResponsePayload_0v1, DnsResolveFailure_0v1}; use crate::sub_lib::sequence_buffer::SequencedPacket; -use crate::sub_lib::set_consuming_wallet_message::SetConsumingWalletMessage; use crate::sub_lib::stream_key::StreamKey; use crate::sub_lib::versioned_data::VersionedData; use actix::Message; @@ -79,7 +78,6 @@ pub struct ProxyServerSubs { pub add_return_route: Recipient, pub add_route: Recipient, pub stream_shutdown_sub: Recipient, - pub set_consuming_wallet_sub: Recipient, pub node_from_ui: Recipient, } @@ -111,7 +109,6 @@ mod tests { add_return_route: recipient!(recorder, AddReturnRouteMessage), add_route: recipient!(recorder, AddRouteMessage), stream_shutdown_sub: recipient!(recorder, StreamShutdownMsg), - set_consuming_wallet_sub: recipient!(recorder, SetConsumingWalletMessage), node_from_ui: recipient!(recorder, NodeFromUiMessage), }; diff --git a/node/src/sub_lib/set_consuming_wallet_message.rs b/node/src/sub_lib/set_consuming_wallet_message.rs deleted file mode 100644 index 186f4be77..000000000 --- a/node/src/sub_lib/set_consuming_wallet_message.rs +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. - -use crate::sub_lib::wallet::Wallet; -use actix::Message; - -#[derive(Clone, PartialEq, Eq, Debug, Message)] -pub struct SetConsumingWalletMessage { - pub wallet: Wallet, -} diff --git a/node/src/test_utils/recorder.rs b/node/src/test_utils/recorder.rs index e63844706..1790daae2 100644 --- a/node/src/test_utils/recorder.rs +++ b/node/src/test_utils/recorder.rs @@ -17,15 +17,15 @@ use crate::sub_lib::accountant::ReportRoutingServiceProvidedMessage; use crate::sub_lib::accountant::ReportServicesConsumedMessage; use crate::sub_lib::blockchain_bridge::ReportAccountsPayable; use crate::sub_lib::blockchain_bridge::{BlockchainBridgeSubs, RequestBalancesToPayPayables}; -use crate::sub_lib::configurator::{ConfiguratorSubs, NewPasswordMessage}; use crate::sub_lib::dispatcher::InboundClientData; use crate::sub_lib::dispatcher::{DispatcherSubs, StreamShutdownMsg}; use crate::sub_lib::hopper::IncipientCoresPackage; 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::{ConfigurationChangeMessage, ConnectionProgressMessage}; +use crate::sub_lib::configurator::ConfiguratorSubs; use crate::sub_lib::neighborhood::NodeQueryResponseMetadata; use crate::sub_lib::neighborhood::NodeRecordMetadataMessage; use crate::sub_lib::neighborhood::RemoveNeighborMessage; @@ -40,7 +40,6 @@ use crate::sub_lib::proxy_server::ProxyServerSubs; use crate::sub_lib::proxy_server::{ AddReturnRouteMessage, AddRouteMessage, ClientRequestPayload_0v1, }; -use crate::sub_lib::set_consuming_wallet_message::SetConsumingWalletMessage; use crate::sub_lib::stream_handler_pool::DispatcherNodeQueryResponse; use crate::sub_lib::stream_handler_pool::TransmitDataMsg; use crate::sub_lib::ui_gateway::UiGatewaySubs; @@ -112,7 +111,6 @@ recorder_message_handler!(ExpiredCoresPackage); recorder_message_handler!(InboundClientData); recorder_message_handler!(InboundServerData); recorder_message_handler!(IncipientCoresPackage); -recorder_message_handler!(NewPasswordMessage); recorder_message_handler!(NewPublicIp); recorder_message_handler!(NodeFromUiMessage); recorder_message_handler!(NodeToUiMessage); @@ -128,8 +126,8 @@ recorder_message_handler!(ReportRoutingServiceProvidedMessage); recorder_message_handler!(ScanError); recorder_message_handler!(ConsumingWalletBalancesAndQualifiedPayables); recorder_message_handler!(SentPayables); -recorder_message_handler!(SetConsumingWalletMessage); recorder_message_handler!(RequestBalancesToPayPayables); +recorder_message_handler!(ConfigurationChangeMessage); recorder_message_handler!(StartMessage); recorder_message_handler!(StreamShutdownMsg); recorder_message_handler!(TransmitDataMsg); @@ -344,7 +342,6 @@ pub fn make_proxy_server_subs_from(addr: &Addr) -> ProxyServerSubs { add_return_route: recipient!(addr, AddReturnRouteMessage), add_route: recipient!(addr, AddRouteMessage), stream_shutdown_sub: recipient!(addr, StreamShutdownMsg), - set_consuming_wallet_sub: recipient!(addr, SetConsumingWalletMessage), node_from_ui: recipient!(addr, NodeFromUiMessage), } } @@ -391,10 +388,9 @@ pub fn make_neighborhood_subs_from(addr: &Addr) -> NeighborhoodSubs { gossip_failure: recipient!(addr, ExpiredCoresPackage), dispatcher_node_query: recipient!(addr, DispatcherNodeQueryMessage), remove_neighbor: recipient!(addr, RemoveNeighborMessage), + configuration_change_msg_sub: recipient!(addr, ConfigurationChangeMessage), stream_shutdown_sub: recipient!(addr, StreamShutdownMsg), - set_consuming_wallet_sub: recipient!(addr, SetConsumingWalletMessage), from_ui_message_sub: recipient!(addr, NodeFromUiMessage), - new_password_sub: recipient!(addr, NewPasswordMessage), connection_progress_sub: recipient!(addr, ConnectionProgressMessage), } }