From 0993033b9917aa68e43da5d8ebf71f1cd84f53eb Mon Sep 17 00:00:00 2001 From: Syther007 Date: Wed, 5 Apr 2023 22:17:06 +1200 Subject: [PATCH 1/7] GH-662: Established a single function to test panic_on_migration --- node/src/accountant/mod.rs | 26 +++++++++-- node/src/actor_system_factory.rs | 44 +++++++++++++++--- node/src/blockchain/blockchain_bridge.rs | 35 ++++++++++----- node/src/bootstrapper.rs | 52 +++++++++++++++++----- node/src/daemon/setup_reporter.rs | 4 +- node/src/database/db_initializer.rs | 8 +--- node/src/node_configurator/configurator.rs | 36 +++++++++++---- node/src/node_configurator/mod.rs | 10 +---- node/src/sub_lib/utils.rs | 6 +++ node/src/test_utils/database_utils.rs | 4 +- node/src/test_utils/mod.rs | 46 ++++++++++++++++++- 11 files changed, 216 insertions(+), 55 deletions(-) diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index 0a10b4ce6..2c01d25f1 100644 --- a/node/src/accountant/mod.rs +++ b/node/src/accountant/mod.rs @@ -949,7 +949,7 @@ mod tests { use actix::{Arbiter, System}; use ethereum_types::{BigEndianHash, U64}; use ethsign_crypto::Keccak256; - use itertools::Itertools; + use itertools::{Either, Itertools}; use log::Level; use masq_lib::constants::{ REQUEST_WITH_MUTUALLY_EXCLUSIVE_PARAMS, REQUEST_WITH_NO_VALUES, SCAN_ERROR, @@ -967,7 +967,7 @@ mod tests { use crate::accountant::dao_utils::from_time_t; use crate::accountant::dao_utils::{to_time_t, CustomQuery}; - use crate::accountant::payable_dao::{PayableAccount, PayableDaoError}; + use crate::accountant::payable_dao::{PayableAccount, PayableDaoError, PayableDaoFactory}; use crate::accountant::pending_payable_dao::PendingPayableDaoError; use crate::accountant::receivable_dao::ReceivableAccount; use crate::accountant::scanners::{BeginScanError, NullScanner, ScannerMock}; @@ -1002,10 +1002,12 @@ mod tests { use crate::test_utils::unshared_test_utils::notify_handlers::NotifyLaterHandleMock; use crate::test_utils::unshared_test_utils::system_killer_actor::SystemKillerActor; use crate::test_utils::unshared_test_utils::{ - make_bc_with_defaults, prove_that_crash_request_handler_is_hooked_up, AssertionsMessage, + assert_on_initialization_with_panic_on_migration, make_bc_with_defaults, + prove_that_crash_request_handler_is_hooked_up, AssertionsMessage, }; use crate::test_utils::{make_paying_wallet, make_wallet}; use masq_lib::messages::TopRecordsOrdering::{Age, Balance}; + use masq_lib::test_utils::utils::ensure_node_home_directory_exists; use masq_lib::ui_gateway::MessagePath::Conversation; use web3::types::{TransactionReceipt, H256}; @@ -4248,4 +4250,22 @@ mod tests { } } } + + #[test] + fn make_dao_factory_uses_panic_on_migration() { + let data_dir = ensure_node_home_directory_exists( + "accountant", + "make_dao_factory_uses_panic_on_migration", + ); + + let act = |data_dir: &Path| { + let factory = Accountant::dao_factory(data_dir); + factory.make(); + }; + + assert_on_initialization_with_panic_on_migration( + &data_dir, + &act + ); + } } diff --git a/node/src/actor_system_factory.rs b/node/src/actor_system_factory.rs index f03f57df2..e700c761f 100644 --- a/node/src/actor_system_factory.rs +++ b/node/src/actor_system_factory.rs @@ -49,6 +49,7 @@ use masq_lib::logger::Logger; use masq_lib::ui_gateway::{NodeFromUiMessage, NodeToUiMessage}; use masq_lib::utils::{exit_process, AutomapProtocol}; use std::net::{IpAddr, Ipv4Addr}; +use std::path::Path; pub trait ActorSystemFactory { fn make_and_start_actors( @@ -453,11 +454,7 @@ impl ActorFactory for ActorFactoryReal { let pending_payable_dao_factory = Box::new(Accountant::dao_factory(data_directory)); let receivable_dao_factory = Box::new(Accountant::dao_factory(data_directory)); let banned_dao_factory = Box::new(Accountant::dao_factory(data_directory)); - banned_cache_loader.load(connection_or_panic( - db_initializer, - data_directory, - DbInitializationConfig::panic_on_migration(), - )); + Self::load_banned_cache(db_initializer, banned_cache_loader, data_directory); let arbiter = Arbiter::builder().stop_system_on_panic(true); let addr: Addr = arbiter.start(move |_| { Accountant::new( @@ -543,6 +540,20 @@ impl ActorFactory for ActorFactoryReal { } } +impl ActorFactoryReal { + fn load_banned_cache( + db_initializer: &dyn DbInitializer, + banned_cache_loader: &dyn BannedCacheLoader, + data_directory: &Path, + ) { + banned_cache_loader.load(connection_or_panic( + db_initializer, + data_directory, + DbInitializationConfig::panic_on_migration(), + )); + } +} + fn is_crashable(config: &BootstrapperConfig) -> bool { config.crash_point == CrashPoint::Message } @@ -639,6 +650,7 @@ mod tests { }; use crate::test_utils::recorder::{make_recorder, Recorder}; use crate::test_utils::unshared_test_utils::arbitrary_id_stamp::ArbitraryIdStamp; + use crate::test_utils::unshared_test_utils::assert_on_initialization_with_panic_on_migration; use crate::test_utils::unshared_test_utils::system_killer_actor::SystemKillerActor; use crate::test_utils::{alias_cryptde, rate_pack}; use crate::test_utils::{main_cryptde, make_cryptde_pair}; @@ -650,6 +662,7 @@ mod tests { parameterizable_automap_control, TransactorMock, PUBLIC_IP, ROUTER_IP, }; use crossbeam_channel::{bounded, unbounded, Sender}; + use itertools::Either; use log::LevelFilter; use masq_lib::constants::DEFAULT_CHAIN; use masq_lib::crash_point::CrashPoint; @@ -2081,6 +2094,27 @@ mod tests { assert_eq!(msg, &msg_of_irrelevant_choice); } + #[test] + fn load_banned_cache_implements_panic_on_migration() { + let data_dir = ensure_node_home_directory_exists( + "actor_system_factory", + "load_banned_cache_implements_panic_on_migration", + ); + + let act = |data_dir: &Path| { + ActorFactoryReal::load_banned_cache( + &DbInitializerReal::default(), + &BannedCacheLoaderMock::default(), + &data_dir, + ); + }; + + assert_on_initialization_with_panic_on_migration( + &data_dir, + &act + ); + } + fn public_key_for_dyn_cryptde_being_null(cryptde: &dyn CryptDE) -> &PublicKey { let null_cryptde = <&CryptDENull>::from(cryptde); null_cryptde.public_key() diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index 4ffc97bef..01a70b598 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -19,7 +19,7 @@ use crate::sub_lib::blockchain_bridge::BlockchainBridgeSubs; use crate::sub_lib::blockchain_bridge::ReportAccountsPayable; use crate::sub_lib::peer_actors::BindMessage; use crate::sub_lib::set_consuming_wallet_message::SetConsumingWalletMessage; -use crate::sub_lib::utils::handle_ui_crash_request; +use crate::sub_lib::utils::{db_connection_launch_panic, handle_ui_crash_request}; use crate::sub_lib::wallet::Wallet; use actix::Actor; use actix::Context; @@ -210,12 +210,7 @@ impl BlockchainBridge { &data_directory, DbInitializationConfig::panic_on_migration(), ) - .unwrap_or_else(|_| { - panic!( - "Failed to connect to database at {:?}", - &data_directory.join(DATABASE_FILE) - ) - }), + .unwrap_or_else(|err| db_connection_launch_panic(err, &data_directory) ), )); ( blockchain_interface, @@ -438,9 +433,7 @@ mod tests { use crate::test_utils::recorder::{make_recorder, peer_actors_builder}; use crate::test_utils::recorder_stop_conditions::StopCondition; use crate::test_utils::recorder_stop_conditions::StopConditions; - use crate::test_utils::unshared_test_utils::{ - configure_default_persistent_config, prove_that_crash_request_handler_is_hooked_up, ZERO, - }; + use crate::test_utils::unshared_test_utils::{assert_on_initialization_with_panic_on_migration, configure_default_persistent_config, prove_that_crash_request_handler_is_hooked_up, ZERO}; use crate::test_utils::{make_paying_wallet, make_wallet}; use actix::System; use ethereum_types::{BigEndianHash, U64}; @@ -451,9 +444,14 @@ mod tests { use masq_lib::test_utils::logging::TestLogHandler; use rustc_hex::FromHex; use std::any::TypeId; + use std::path::Path; use std::sync::{Arc, Mutex}; use std::time::SystemTime; + use itertools::Either; use web3::types::{TransactionReceipt, H160, H256, U256}; + use masq_lib::test_utils::utils::ensure_node_home_directory_exists; + use crate::database::db_initializer::DbInitializerReal; + use crate::node_configurator::configurator::Configurator; #[test] fn constants_have_correct_values() { @@ -1446,4 +1444,21 @@ mod tests { prove_that_crash_request_handler_is_hooked_up(subject, CRASH_KEY); } + + #[test] + fn make_connections_implements_panic_on_migration() { + let data_dir = ensure_node_home_directory_exists( + "blockchain_bridge", + "make_connections_with_panic_on_migration", + ); + + let act = |data_dir: &Path| { + BlockchainBridge::make_connections(Some("http://127.0.0.1".to_string()), &DbInitializerReal::default(), data_dir.to_path_buf(), Chain::PolyMumbai ); + }; + + assert_on_initialization_with_panic_on_migration( + &data_dir, + &act + ); + } } diff --git a/node/src/bootstrapper.rs b/node/src/bootstrapper.rs index 2103cdf04..1497c2d45 100644 --- a/node/src/bootstrapper.rs +++ b/node/src/bootstrapper.rs @@ -57,6 +57,7 @@ use tokio::prelude::stream::futures_unordered::FuturesUnordered; use tokio::prelude::Async; use tokio::prelude::Future; use tokio::prelude::Stream; +use crate::stream_handler_pool::StreamHandlerPoolSubs; static mut MAIN_CRYPTDE_BOX_OPT: Option> = None; static mut ALIAS_CRYPTDE_BOX_OPT: Option> = None; @@ -515,15 +516,7 @@ impl ConfiguredByPrivilege for Bootstrapper { if node_addr.ip_addr() == Ipv4Addr::new(0, 0, 0, 0) => {} // node_addr still coming _ => Bootstrapper::report_local_descriptor(cryptdes.main, &self.config.node_descriptor), // here or not coming } - let stream_handler_pool_subs = self.actor_system_factory.make_and_start_actors( - self.config.clone(), - Box::new(ActorFactoryReal {}), - initialize_database( - &self.config.data_directory, - DbInitializationConfig::panic_on_migration(), - ), - ); - + let stream_handler_pool_subs = self.start_actors_and_return_shp_subs(); self.listener_handlers .iter_mut() .for_each(|f| f.bind_subs(stream_handler_pool_subs.add_sub.clone())); @@ -612,6 +605,17 @@ impl Bootstrapper { } } + fn start_actors_and_return_shp_subs(&self) -> StreamHandlerPoolSubs { + self.actor_system_factory.make_and_start_actors( + self.config.clone(), + Box::new(ActorFactoryReal {}), + initialize_database( + &self.config.data_directory, + DbInitializationConfig::panic_on_migration(), + ), + ) + } + pub fn report_local_descriptor(cryptde: &dyn CryptDE, descriptor: &NodeDescriptor) { let descriptor_msg = format!( "MASQ Node local descriptor: {}", @@ -738,7 +742,7 @@ mod tests { use crate::test_utils::recorder::Recording; use crate::test_utils::tokio_wrapper_mocks::ReadHalfWrapperMock; use crate::test_utils::tokio_wrapper_mocks::WriteHalfWrapperMock; - use crate::test_utils::unshared_test_utils::make_simplified_multi_config; + use crate::test_utils::unshared_test_utils::{assert_on_initialization_with_panic_on_migration, make_simplified_multi_config}; use crate::test_utils::{assert_contains, rate_pack}; use crate::test_utils::{main_cryptde, make_wallet}; use actix::Recipient; @@ -762,10 +766,11 @@ mod tests { use std::io::ErrorKind; use std::marker::Sync; use std::net::{IpAddr, SocketAddr}; - use std::path::PathBuf; + use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::{Arc, Mutex}; use std::thread; + use itertools::Either; use tokio; use tokio::executor::current_thread::CurrentThread; use tokio::prelude::stream::FuturesUnordered; @@ -1383,6 +1388,31 @@ mod tests { assert_eq!(config.blockchain_bridge_config.gas_price, 11); } + #[test] + fn initialize_as_unprivileged_implements_panic_on_migration_for_make_and_start_actors() { + let _lock = INITIALIZATION.lock(); + let data_dir = ensure_node_home_directory_exists( + "bootstrapper", + "initialize_as_unprivileged_implements_panic_on_migration_for_make_and_start_actors", + ); + + let act = |data_dir: &Path| { + let mut config = BootstrapperConfig::new(); + config.data_directory = data_dir.to_path_buf(); + let mut subject = BootstrapperBuilder::new() + .config(config) + .build(); + subject.start_actors_and_return_shp_subs(); + }; + + assert_on_initialization_with_panic_on_migration( + &data_dir, + &act + ); + } + + + #[test] fn initialize_with_clandestine_port_produces_expected_clandestine_discriminator_factories_vector( ) { diff --git a/node/src/daemon/setup_reporter.rs b/node/src/daemon/setup_reporter.rs index c1ee05c65..298d0d166 100644 --- a/node/src/daemon/setup_reporter.rs +++ b/node/src/daemon/setup_reporter.rs @@ -462,7 +462,9 @@ impl SetupReporterReal { } Err(InitializationError::Nonexistent | InitializationError::SuppressedMigration) => { // When the Daemon runs for the first time, the database will not yet have been - // created. If the database is old, it should not be used by the Daemon. + // created. If the database is old, it should not be used by the Daemon (see more + // information at ConfigDaoNull). + let parse_args_configuration = UnprivilegedParseArgsConfigurationDaoNull {}; let mut persistent_config = PersistentConfigurationReal::new(Box::new(ConfigDaoNull::default())); diff --git a/node/src/database/db_initializer.rs b/node/src/database/db_initializer.rs index d84abc803..65dfa8503 100644 --- a/node/src/database/db_initializer.rs +++ b/node/src/database/db_initializer.rs @@ -20,6 +20,7 @@ use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use std::path::Path; use std::{fs, vec}; use tokio::net::TcpListener; +use crate::sub_lib::utils::db_connection_launch_panic; pub const DATABASE_FILE: &str = "node-data.db"; pub const CURRENT_SCHEMA_VERSION: usize = 7; @@ -487,12 +488,7 @@ pub fn connection_or_panic( ) -> Box { db_initializer .initialize(path, init_config) - .unwrap_or_else(|_| { - panic!( - "Failed to connect to database at {:?}", - path.join(DATABASE_FILE) - ) - }) + .unwrap_or_else(|err| db_connection_launch_panic(err, path)) } #[derive(Clone)] diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index ce38d3b45..0c7303f67 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -27,7 +27,7 @@ use crate::db_config::persistent_configuration::{ }; use crate::sub_lib::configurator::NewPasswordMessage; use crate::sub_lib::peer_actors::BindMessage; -use crate::sub_lib::utils::handle_ui_crash_request; +use crate::sub_lib::utils::{db_connection_launch_panic, handle_ui_crash_request}; use crate::sub_lib::wallet::Wallet; use crate::test_utils::main_cryptde; use bip39::{Language, Mnemonic, MnemonicType, Seed}; @@ -101,7 +101,7 @@ impl Configurator { &data_directory, DbInitializationConfig::panic_on_migration(), ) - .expect("Couldn't initialize database"); + .unwrap_or_else(|err| db_connection_launch_panic(err, &data_directory)); let config_dao = ConfigDaoReal::new(conn); let persistent_config: Box = Box::new(PersistentConfigurationReal::new(Box::new(config_dao))); @@ -814,6 +814,7 @@ mod tests { UiWalletAddressesResponse, }; use masq_lib::ui_gateway::{MessagePath, MessageTarget}; + use std::path::Path; use std::str::FromStr; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -837,9 +838,11 @@ mod tests { use crate::sub_lib::node_addr::NodeAddr; use crate::sub_lib::wallet::Wallet; use crate::test_utils::unshared_test_utils::{ - configure_default_persistent_config, prove_that_crash_request_handler_is_hooked_up, ZERO, + assert_on_initialization_with_panic_on_migration, configure_default_persistent_config, + prove_that_crash_request_handler_is_hooked_up, ZERO, }; use bip39::{Language, Mnemonic}; + use itertools::Either; use masq_lib::blockchains::chains::Chain; use masq_lib::constants::MISSING_DATA; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; @@ -863,11 +866,12 @@ mod tests { ))); let (recorder, _, _) = make_recorder(); let recorder_addr = recorder.start(); - let mut subject = Configurator::new(data_dir, false); - subject.node_to_ui_sub = Some(recorder_addr.recipient()); - subject.new_password_subs = Some(vec![]); - let _ = subject.handle_change_password( + let mut configurator = Configurator::new(data_dir, false); + + configurator.node_to_ui_sub = Some(recorder_addr.recipient()); + configurator.new_password_subs = Some(vec![]); + let _ = configurator.handle_change_password( UiChangePasswordRequest { old_password_opt: None, new_password: "password".to_string(), @@ -875,13 +879,29 @@ mod tests { 0, 0, ); - assert_eq!( verifier.check_password(Some("password".to_string())), Ok(true) ) } + #[test] + fn constructor_panics_on_database_migration() { + let data_dir = ensure_node_home_directory_exists( + "configurator", + "constructor_panics_on_database_migration", + ); + + let act = |data_dir: &Path| { + Configurator::new(data_dir.to_path_buf(), false); + }; + + assert_on_initialization_with_panic_on_migration( + &data_dir, + &act + ); + } + #[test] fn ignores_unexpected_message() { let system = System::new("test"); diff --git a/node/src/node_configurator/mod.rs b/node/src/node_configurator/mod.rs index ad232115c..9b3391da2 100644 --- a/node/src/node_configurator/mod.rs +++ b/node/src/node_configurator/mod.rs @@ -11,7 +11,7 @@ use crate::database::db_initializer::{DbInitializer, DbInitializerReal, DATABASE use crate::db_config::persistent_configuration::{ PersistentConfiguration, PersistentConfigurationReal, }; -use crate::sub_lib::utils::make_new_multi_config; +use crate::sub_lib::utils::{db_connection_launch_panic, make_new_multi_config}; use clap::{value_t, App}; use dirs::{data_local_dir, home_dir}; use masq_lib::blockchains::chains::Chain; @@ -69,13 +69,7 @@ pub fn initialize_database( ) -> Box { let conn = DbInitializerReal::default() .initialize(data_directory, migrator_config) - .unwrap_or_else(|e| { - panic!( - "Can't initialize database at {:?}: {:?}", - data_directory.join(DATABASE_FILE), - e - ) - }); + .unwrap_or_else(|e| db_connection_launch_panic(e, &data_directory)); Box::new(PersistentConfigurationReal::from(conn)) } diff --git a/node/src/sub_lib/utils.rs b/node/src/sub_lib/utils.rs index 74758bd0b..289ec2070 100644 --- a/node/src/sub_lib/utils.rs +++ b/node/src/sub_lib/utils.rs @@ -12,7 +12,9 @@ use masq_lib::utils::type_name_of; use std::any::Any; use std::io::ErrorKind; use std::marker::PhantomData; +use std::path::Path; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use crate::database::db_initializer::{DATABASE_FILE, InitializationError}; static DEAD_STREAM_ERRORS: [ErrorKind; 5] = [ ErrorKind::BrokenPipe, @@ -237,6 +239,10 @@ where implement_as_any!(); } +pub fn db_connection_launch_panic(err: InitializationError, data_directory: &Path) -> ! { todo! (Wright a test to test the whole message including the path); + panic!("Couldn't initialize database due to \"{:?}\" at {:?}",err, data_directory.join(DATABASE_FILE) ) +} + #[cfg(test)] mod tests { use super::*; diff --git a/node/src/test_utils/database_utils.rs b/node/src/test_utils/database_utils.rs index 43594962c..ef6ed10c2 100644 --- a/node/src/test_utils/database_utils.rs +++ b/node/src/test_utils/database_utils.rs @@ -15,10 +15,10 @@ use std::env::current_dir; use std::fs::{remove_file, File}; use std::io::Read; use std::iter::once; -use std::path::PathBuf; +use std::path::Path; use std::sync::{Arc, Mutex}; -pub fn bring_db_0_back_to_life_and_return_connection(db_path: &PathBuf) -> Connection { +pub fn bring_db_0_back_to_life_and_return_connection(db_path: &Path) -> Connection { match remove_file(db_path) { Err(e) if e.kind() == std::io::ErrorKind::NotFound => (), Err(e) => panic!("Unexpected but serious error: {}", e), diff --git a/node/src/test_utils/mod.rs b/node/src/test_utils/mod.rs index 62f4dd702..46f2092d6 100644 --- a/node/src/test_utils/mod.rs +++ b/node/src/test_utils/mod.rs @@ -545,6 +545,7 @@ pub mod unshared_test_utils { use crate::apps::app_node; use crate::bootstrapper::BootstrapperConfig; use crate::daemon::{ChannelFactory, DaemonBindMessage}; + use crate::database::db_initializer::DATABASE_FILE; use crate::db_config::config_dao_null::ConfigDaoNull; use crate::db_config::persistent_configuration::PersistentConfigurationReal; use crate::node_test_utils::DirsWrapperMock; @@ -553,6 +554,7 @@ pub mod unshared_test_utils { use crate::sub_lib::utils::{ NLSpawnHandleHolder, NLSpawnHandleHolderReal, NotifyHandle, NotifyLaterHandle, }; + use crate::test_utils::database_utils::bring_db_0_back_to_life_and_return_connection; use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock; use crate::test_utils::recorder::{make_recorder, Recorder, Recording}; use crate::test_utils::recorder_stop_conditions::{StopCondition, StopConditions}; @@ -560,6 +562,7 @@ pub mod unshared_test_utils { use actix::{Actor, Addr, AsyncContext, Context, Handler, Recipient, System}; use actix::{Message, SpawnHandle}; use crossbeam_channel::{unbounded, Receiver, Sender}; + use itertools::Either; use lazy_static::lazy_static; use masq_lib::messages::{ToMessageBody, UiCrashRequest}; use masq_lib::multi_config::MultiConfig; @@ -571,7 +574,8 @@ pub mod unshared_test_utils { use std::cell::RefCell; use std::collections::HashMap; use std::num::ParseIntError; - use std::path::PathBuf; + use std::panic::{catch_unwind, AssertUnwindSafe}; + use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use std::time::Duration; use std::vec; @@ -581,6 +585,46 @@ pub mod unshared_test_utils { pub assertions: Box, } + pub fn assert_on_initialization_with_panic_on_migration( + data_dir: &Path, + act: &A, + ) where + A: Fn(&Path) + ?Sized, + { + fn assert_closure( + act: &A, + expected_panic_message: Either<(&str, &str), &str>, + data_dir: &Path, + ) where + A: Fn(&Path) + ?Sized, + S: AsRef + 'static, + { + let caught_panic_err = catch_unwind(AssertUnwindSafe(|| act(data_dir))); + + let caught_panic = caught_panic_err.unwrap_err(); + let panic_message = caught_panic.downcast_ref::().unwrap(); + let panic_message_str: &str = panic_message.as_ref(); + match expected_panic_message { + Either::Left((message_start,message_end)) => { + assert!(panic_message_str.contains(message_start), "We expected this message {} to start with {}",panic_message_str,message_start); + assert!(panic_message_str.ends_with(message_end), "We expected this message {} to end with {}",panic_message_str,message_end); + }, + Either::Right(message) => assert_eq!(panic_message_str, message), + } + } + let database_file_path =data_dir.join(DATABASE_FILE); + + assert_closure::(&act, Either::Left(("Couldn't initialize database due to \"Nonexistent\" at \"generated", &format!("{}\"",DATABASE_FILE))), data_dir); + + bring_db_0_back_to_life_and_return_connection(&database_file_path); + + assert_closure::<&str, _>( + &act, + Either::Right("Broken code: Migrating database at inappropriate place"), + data_dir, + ); + } + pub fn make_simplified_multi_config<'a, const T: usize>(args: [&str; T]) -> MultiConfig<'a> { let mut app_args = vec!["MASQNode".to_string()]; app_args.append(&mut array_of_borrows_to_vec(&args)); From 8e6947a733f2f7262421669c6a8e9ff5767a056d Mon Sep 17 00:00:00 2001 From: Syther007 Date: Mon, 10 Apr 2023 20:46:23 +1200 Subject: [PATCH 2/7] GH-662: Tested a frew more occurrences --- node/src/accountant/mod.rs | 5 +--- node/src/actor_system_factory.rs | 5 +--- node/src/blockchain/blockchain_bridge.rs | 27 ++++++++++------- node/src/bootstrapper.rs | 19 +++++------- node/src/database/db_initializer.rs | 2 +- node/src/node_configurator/configurator.rs | 5 +--- node/src/sub_lib/utils.rs | 35 ++++++++++++++++++++-- node/src/test_utils/mod.rs | 35 +++++++++++++++------- 8 files changed, 84 insertions(+), 49 deletions(-) diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index 2c01d25f1..199094756 100644 --- a/node/src/accountant/mod.rs +++ b/node/src/accountant/mod.rs @@ -4263,9 +4263,6 @@ mod tests { factory.make(); }; - assert_on_initialization_with_panic_on_migration( - &data_dir, - &act - ); + assert_on_initialization_with_panic_on_migration(&data_dir, &act); } } diff --git a/node/src/actor_system_factory.rs b/node/src/actor_system_factory.rs index e700c761f..27b0d5b48 100644 --- a/node/src/actor_system_factory.rs +++ b/node/src/actor_system_factory.rs @@ -2109,10 +2109,7 @@ mod tests { ); }; - assert_on_initialization_with_panic_on_migration( - &data_dir, - &act - ); + assert_on_initialization_with_panic_on_migration(&data_dir, &act); } fn public_key_for_dyn_cryptde_being_null(cryptde: &dyn CryptDE) -> &PublicKey { diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index 01a70b598..462f59888 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -210,7 +210,7 @@ impl BlockchainBridge { &data_directory, DbInitializationConfig::panic_on_migration(), ) - .unwrap_or_else(|err| db_connection_launch_panic(err, &data_directory) ), + .unwrap_or_else(|err| db_connection_launch_panic(err, &data_directory)), )); ( blockchain_interface, @@ -426,32 +426,35 @@ mod tests { use crate::blockchain::test_utils::BlockchainInterfaceMock; use crate::blockchain::tool_wrappers::SendTransactionToolsWrapperNull; use crate::database::db_initializer::test_utils::DbInitializerMock; + use crate::database::db_initializer::DbInitializerReal; use crate::db_config::persistent_configuration::PersistentConfigError; use crate::match_every_type_id; + use crate::node_configurator::configurator::Configurator; use crate::node_test_utils::check_timestamp; use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock; use crate::test_utils::recorder::{make_recorder, peer_actors_builder}; use crate::test_utils::recorder_stop_conditions::StopCondition; use crate::test_utils::recorder_stop_conditions::StopConditions; - use crate::test_utils::unshared_test_utils::{assert_on_initialization_with_panic_on_migration, configure_default_persistent_config, prove_that_crash_request_handler_is_hooked_up, ZERO}; + use crate::test_utils::unshared_test_utils::{ + assert_on_initialization_with_panic_on_migration, configure_default_persistent_config, + prove_that_crash_request_handler_is_hooked_up, ZERO, + }; use crate::test_utils::{make_paying_wallet, make_wallet}; use actix::System; use ethereum_types::{BigEndianHash, U64}; use ethsign_crypto::Keccak256; + use itertools::Either; use masq_lib::constants::DEFAULT_CHAIN; use masq_lib::messages::ScanType; use masq_lib::test_utils::logging::init_test_logging; use masq_lib::test_utils::logging::TestLogHandler; + use masq_lib::test_utils::utils::ensure_node_home_directory_exists; use rustc_hex::FromHex; use std::any::TypeId; use std::path::Path; use std::sync::{Arc, Mutex}; use std::time::SystemTime; - use itertools::Either; use web3::types::{TransactionReceipt, H160, H256, U256}; - use masq_lib::test_utils::utils::ensure_node_home_directory_exists; - use crate::database::db_initializer::DbInitializerReal; - use crate::node_configurator::configurator::Configurator; #[test] fn constants_have_correct_values() { @@ -1453,12 +1456,14 @@ mod tests { ); let act = |data_dir: &Path| { - BlockchainBridge::make_connections(Some("http://127.0.0.1".to_string()), &DbInitializerReal::default(), data_dir.to_path_buf(), Chain::PolyMumbai ); + BlockchainBridge::make_connections( + Some("http://127.0.0.1".to_string()), + &DbInitializerReal::default(), + data_dir.to_path_buf(), + Chain::PolyMumbai, + ); }; - assert_on_initialization_with_panic_on_migration( - &data_dir, - &act - ); + assert_on_initialization_with_panic_on_migration(&data_dir, &act); } } diff --git a/node/src/bootstrapper.rs b/node/src/bootstrapper.rs index 1497c2d45..1f6600ddc 100644 --- a/node/src/bootstrapper.rs +++ b/node/src/bootstrapper.rs @@ -21,6 +21,7 @@ use crate::node_configurator::node_configurator_standard::{ use crate::node_configurator::{initialize_database, DirsWrapper, NodeConfigurator}; use crate::privilege_drop::{IdWrapper, IdWrapperReal}; use crate::server_initializer::LoggerInitializerWrapper; +use crate::stream_handler_pool::StreamHandlerPoolSubs; use crate::sub_lib::accountant; use crate::sub_lib::accountant::{PaymentThresholds, ScanIntervals}; use crate::sub_lib::blockchain_bridge::BlockchainBridgeConfig; @@ -57,7 +58,6 @@ use tokio::prelude::stream::futures_unordered::FuturesUnordered; use tokio::prelude::Async; use tokio::prelude::Future; use tokio::prelude::Stream; -use crate::stream_handler_pool::StreamHandlerPoolSubs; static mut MAIN_CRYPTDE_BOX_OPT: Option> = None; static mut ALIAS_CRYPTDE_BOX_OPT: Option> = None; @@ -742,13 +742,16 @@ mod tests { use crate::test_utils::recorder::Recording; use crate::test_utils::tokio_wrapper_mocks::ReadHalfWrapperMock; use crate::test_utils::tokio_wrapper_mocks::WriteHalfWrapperMock; - use crate::test_utils::unshared_test_utils::{assert_on_initialization_with_panic_on_migration, make_simplified_multi_config}; + use crate::test_utils::unshared_test_utils::{ + assert_on_initialization_with_panic_on_migration, make_simplified_multi_config, + }; use crate::test_utils::{assert_contains, rate_pack}; use crate::test_utils::{main_cryptde, make_wallet}; use actix::Recipient; use actix::System; use crossbeam_channel::unbounded; use futures::Future; + use itertools::Either; use lazy_static::lazy_static; use log::LevelFilter; use log::LevelFilter::Off; @@ -770,7 +773,6 @@ mod tests { use std::str::FromStr; use std::sync::{Arc, Mutex}; use std::thread; - use itertools::Either; use tokio; use tokio::executor::current_thread::CurrentThread; use tokio::prelude::stream::FuturesUnordered; @@ -1399,20 +1401,13 @@ mod tests { let act = |data_dir: &Path| { let mut config = BootstrapperConfig::new(); config.data_directory = data_dir.to_path_buf(); - let mut subject = BootstrapperBuilder::new() - .config(config) - .build(); + let mut subject = BootstrapperBuilder::new().config(config).build(); subject.start_actors_and_return_shp_subs(); }; - assert_on_initialization_with_panic_on_migration( - &data_dir, - &act - ); + assert_on_initialization_with_panic_on_migration(&data_dir, &act); } - - #[test] fn initialize_with_clandestine_port_produces_expected_clandestine_discriminator_factories_vector( ) { diff --git a/node/src/database/db_initializer.rs b/node/src/database/db_initializer.rs index 65dfa8503..c8e617e63 100644 --- a/node/src/database/db_initializer.rs +++ b/node/src/database/db_initializer.rs @@ -4,6 +4,7 @@ use crate::database::db_migrations::{DbMigrator, DbMigratorReal}; use crate::db_config::secure_config_layer::EXAMPLE_ENCRYPTED; use crate::sub_lib::accountant::{DEFAULT_PAYMENT_THRESHOLDS, DEFAULT_SCAN_INTERVALS}; use crate::sub_lib::neighborhood::DEFAULT_RATE_PACK; +use crate::sub_lib::utils::db_connection_launch_panic; use masq_lib::blockchains::chains::Chain; use masq_lib::constants::{ DEFAULT_GAS_PRICE, HIGHEST_RANDOM_CLANDESTINE_PORT, LOWEST_USABLE_INSECURE_PORT, @@ -20,7 +21,6 @@ use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use std::path::Path; use std::{fs, vec}; use tokio::net::TcpListener; -use crate::sub_lib::utils::db_connection_launch_panic; pub const DATABASE_FILE: &str = "node-data.db"; pub const CURRENT_SCHEMA_VERSION: usize = 7; diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index 0c7303f67..0c53ac78d 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -896,10 +896,7 @@ mod tests { Configurator::new(data_dir.to_path_buf(), false); }; - assert_on_initialization_with_panic_on_migration( - &data_dir, - &act - ); + assert_on_initialization_with_panic_on_migration(&data_dir, &act); } #[test] diff --git a/node/src/sub_lib/utils.rs b/node/src/sub_lib/utils.rs index 289ec2070..d1237da37 100644 --- a/node/src/sub_lib/utils.rs +++ b/node/src/sub_lib/utils.rs @@ -1,5 +1,6 @@ // Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. +use crate::database::db_initializer::{InitializationError, DATABASE_FILE}; use actix::{Actor, AsyncContext, Context, Handler, Message, SpawnHandle}; use clap::App; use masq_lib::logger::Logger; @@ -14,7 +15,6 @@ use std::io::ErrorKind; use std::marker::PhantomData; use std::path::Path; use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use crate::database::db_initializer::{DATABASE_FILE, InitializationError}; static DEAD_STREAM_ERRORS: [ErrorKind; 5] = [ ErrorKind::BrokenPipe, @@ -239,8 +239,12 @@ where implement_as_any!(); } -pub fn db_connection_launch_panic(err: InitializationError, data_directory: &Path) -> ! { todo! (Wright a test to test the whole message including the path); - panic!("Couldn't initialize database due to \"{:?}\" at {:?}",err, data_directory.join(DATABASE_FILE) ) +pub fn db_connection_launch_panic(err: InitializationError, data_directory: &Path) -> ! { + panic!( + "Couldn't initialize database due to \"{:?}\" at {:?}", + err, + data_directory.join(DATABASE_FILE) + ) } #[cfg(test)] @@ -249,11 +253,13 @@ mod tests { use crate::apps::app_node; use actix::{Handler, System}; use crossbeam_channel::{unbounded, Sender}; + use futures::Future; use log::Level; use masq_lib::messages::ToMessageBody; use masq_lib::multi_config::CommandLineVcl; use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; use std::ops::Sub; + use std::panic::{catch_unwind, AssertUnwindSafe}; #[test] fn indicates_dead_stream_identifies_dead_stream_errors() { @@ -528,4 +534,27 @@ mod tests { assert!(notify_exec_duration < Duration::from_millis(DELAYED)); assert!(notify_later_exec_duration >= Duration::from_millis(DELAYED)); } + + #[test] + fn db_connection_initialization_panic_message_contains_full_path() { + let path = Path::new("first_directory").join("second_directory"); + + let caught_panic_err = catch_unwind(AssertUnwindSafe(|| { + db_connection_launch_panic( + InitializationError::SqliteError(rusqlite::Error::ExecuteReturnedResults), + &path, + ); + })); + + let caught_panic = caught_panic_err.unwrap_err(); + let panic_message = caught_panic.downcast_ref::().unwrap(); + assert_eq!( + panic_message, + &format!( + "Couldn't initialize database due to \"{:?}\" at {:?}", + InitializationError::SqliteError(rusqlite::Error::ExecuteReturnedResults), + path.join(DATABASE_FILE) + ) + ); + } } diff --git a/node/src/test_utils/mod.rs b/node/src/test_utils/mod.rs index 46f2092d6..170fd76d3 100644 --- a/node/src/test_utils/mod.rs +++ b/node/src/test_utils/mod.rs @@ -585,10 +585,8 @@ pub mod unshared_test_utils { pub assertions: Box, } - pub fn assert_on_initialization_with_panic_on_migration( - data_dir: &Path, - act: &A, - ) where + pub fn assert_on_initialization_with_panic_on_migration(data_dir: &Path, act: &A) + where A: Fn(&Path) + ?Sized, { fn assert_closure( @@ -605,16 +603,33 @@ pub mod unshared_test_utils { let panic_message = caught_panic.downcast_ref::().unwrap(); let panic_message_str: &str = panic_message.as_ref(); match expected_panic_message { - Either::Left((message_start,message_end)) => { - assert!(panic_message_str.contains(message_start), "We expected this message {} to start with {}",panic_message_str,message_start); - assert!(panic_message_str.ends_with(message_end), "We expected this message {} to end with {}",panic_message_str,message_end); - }, + Either::Left((message_start, message_end)) => { + assert!( + panic_message_str.contains(message_start), + "We expected this message {} to start with {}", + panic_message_str, + message_start + ); + assert!( + panic_message_str.ends_with(message_end), + "We expected this message {} to end with {}", + panic_message_str, + message_end + ); + } Either::Right(message) => assert_eq!(panic_message_str, message), } } - let database_file_path =data_dir.join(DATABASE_FILE); + let database_file_path = data_dir.join(DATABASE_FILE); - assert_closure::(&act, Either::Left(("Couldn't initialize database due to \"Nonexistent\" at \"generated", &format!("{}\"",DATABASE_FILE))), data_dir); + assert_closure::( + &act, + Either::Left(( + "Couldn't initialize database due to \"Nonexistent\" at \"generated", + &format!("{}\"", DATABASE_FILE), + )), + data_dir, + ); bring_db_0_back_to_life_and_return_connection(&database_file_path); From 269263097b9f0c238f86446ed925b6732c7d29cd Mon Sep 17 00:00:00 2001 From: Syther007 Date: Thu, 13 Apr 2023 22:01:43 +1200 Subject: [PATCH 3/7] GH-662: Finished last test --- node/src/bootstrapper.rs | 26 ++++++++++++++++++++++++-- node/src/neighborhood/mod.rs | 31 +++++++++++++++++++++++++------ node/src/test_utils/mod.rs | 2 -- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/node/src/bootstrapper.rs b/node/src/bootstrapper.rs index 1f6600ddc..cf82e0828 100644 --- a/node/src/bootstrapper.rs +++ b/node/src/bootstrapper.rs @@ -58,6 +58,7 @@ use tokio::prelude::stream::futures_unordered::FuturesUnordered; use tokio::prelude::Async; use tokio::prelude::Future; use tokio::prelude::Stream; +use crate::sub_lib::utils::db_connection_launch_panic; static mut MAIN_CRYPTDE_BOX_OPT: Option> = None; static mut ALIAS_CRYPTDE_BOX_OPT: Option> = None; @@ -634,7 +635,7 @@ impl Bootstrapper { &self.config.data_directory, DbInitializationConfig::panic_on_migration(), ) - .expect("Cannot initialize database"); + .unwrap_or_else(|err| db_connection_launch_panic(err, &self.config.data_directory)); let config_dao = ConfigDaoReal::new(conn); let mut persistent_config = PersistentConfigurationReal::new(Box::new(config_dao)); let clandestine_port = self.establish_clandestine_port(&mut persistent_config); @@ -732,7 +733,7 @@ mod tests { use crate::sub_lib::cryptde::PublicKey; use crate::sub_lib::cryptde::{CryptDE, PlainData}; use crate::sub_lib::cryptde_null::CryptDENull; - use crate::sub_lib::neighborhood::{NeighborhoodConfig, NeighborhoodMode, NodeDescriptor}; + use crate::sub_lib::neighborhood::{DEFAULT_RATE_PACK, NeighborhoodConfig, NeighborhoodMode, NodeDescriptor, RatePack}; use crate::sub_lib::node_addr::NodeAddr; use crate::sub_lib::socket_server::ConfiguredByPrivilege; use crate::sub_lib::stream_connector::ConnectionInfo; @@ -2047,6 +2048,27 @@ mod tests { .is_none()); } + #[test] + fn set_up_clandestine_port_panics_on_migration() { + let data_dir = ensure_node_home_directory_exists( + "bootstrapper", + "set_up_clandestine_port_panics_on_migration", + ); + + let act = |data_dir: &Path| { + let mut config = BootstrapperConfig::new(); + config.data_directory = data_dir.to_path_buf(); + config.neighborhood_config = NeighborhoodConfig { + mode: NeighborhoodMode::Standard(NodeAddr::default(), vec![], DEFAULT_RATE_PACK), + }; + let mut subject = BootstrapperBuilder::new().config(config).build(); + subject.set_up_clandestine_port(); + }; + + assert_on_initialization_with_panic_on_migration(&data_dir, &act); + } + + #[test] #[should_panic( expected = "Database is corrupt: error setting clandestine port: TransactionError" diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 82b4f7a31..0ab8bf3ac 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -64,7 +64,7 @@ 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::{handle_ui_crash_request, NODE_MAILBOX_CAPACITY}; +use crate::sub_lib::utils::{db_connection_launch_panic, handle_ui_crash_request, NODE_MAILBOX_CAPACITY}; use crate::sub_lib::versioned_data::VersionedData; use crate::sub_lib::wallet::Wallet; use gossip_acceptor::GossipAcceptor; @@ -564,7 +564,7 @@ impl Neighborhood { &self.data_directory, DbInitializationConfig::panic_on_migration(), ) - .expect("Neighborhood could not connect to database"); + .unwrap_or_else(|err| db_connection_launch_panic(err, &self.data_directory)); self.persistent_config_opt = Some(Box::new(PersistentConfigurationReal::from(conn))); } } @@ -1585,12 +1585,14 @@ mod tests { use std::cell::RefCell; use std::convert::TryInto; use std::net::{IpAddr, SocketAddr}; + use std::path::Path; use std::str::FromStr; use std::sync::{Arc, Mutex}; use std::thread; use std::time::Duration; use std::time::Instant; use tokio::prelude::Future; + use masq_lib::blockchains::chains::Chain::PolyMumbai; use masq_lib::constants::{DEFAULT_CHAIN, TLS_PORT}; use masq_lib::messages::{ToMessageBody, UiConnectionChangeBroadcast, UiConnectionStage}; @@ -1631,10 +1633,7 @@ mod tests { use crate::test_utils::recorder::peer_actors_builder; use crate::test_utils::recorder::Recorder; use crate::test_utils::recorder::Recording; - use crate::test_utils::unshared_test_utils::{ - make_node_to_ui_recipient, make_recipient_and_recording_arc, - prove_that_crash_request_handler_is_hooked_up, AssertionsMessage, - }; + use crate::test_utils::unshared_test_utils::{make_node_to_ui_recipient, make_recipient_and_recording_arc, prove_that_crash_request_handler_is_hooked_up, AssertionsMessage, assert_on_initialization_with_panic_on_migration}; use crate::test_utils::vec_to_set; use crate::test_utils::{main_cryptde, make_paying_wallet}; @@ -1647,6 +1646,7 @@ mod tests { }; use crate::test_utils::unshared_test_utils::notify_handlers::NotifyLaterHandleMock; use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; + use crate::accountant::test_utils::bc_from_earning_wallet; impl Handler> for Neighborhood { type Result = (); @@ -5687,6 +5687,25 @@ mod tests { // No panic; therefore no attempt was made to persist: test passes! } + #[test] + fn make_connect_database_implements_panic_on_migration() { + let data_dir = ensure_node_home_directory_exists( + "neighborhood", + "make_connect_database_implements_panic_on_migration", + ); + + let act = |data_dir: &Path| { + let mut subject = Neighborhood::new( + main_cryptde(), + &bc_from_earning_wallet(make_wallet("earning_wallet")), + ); + subject.data_directory = data_dir.to_path_buf(); + subject.connect_database(); + }; + + assert_on_initialization_with_panic_on_migration(&data_dir, &act); + } + fn make_standard_subject() -> Neighborhood { let root_node = make_global_cryptde_node_record(9999, true); let neighbor_node = make_node_record(9998, true); diff --git a/node/src/test_utils/mod.rs b/node/src/test_utils/mod.rs index 170fd76d3..a296392e5 100644 --- a/node/src/test_utils/mod.rs +++ b/node/src/test_utils/mod.rs @@ -621,7 +621,6 @@ pub mod unshared_test_utils { } } let database_file_path = data_dir.join(DATABASE_FILE); - assert_closure::( &act, Either::Left(( @@ -632,7 +631,6 @@ pub mod unshared_test_utils { ); bring_db_0_back_to_life_and_return_connection(&database_file_path); - assert_closure::<&str, _>( &act, Either::Right("Broken code: Migrating database at inappropriate place"), From 16ba99087f0fef7a56bb2bb2de0a620219abc31c Mon Sep 17 00:00:00 2001 From: Syther007 Date: Tue, 18 Apr 2023 21:09:30 +1200 Subject: [PATCH 4/7] GH-662: Fixed all warning messages & formatting --- node/src/accountant/mod.rs | 2 +- node/src/actor_system_factory.rs | 1 - node/src/blockchain/blockchain_bridge.rs | 4 +--- node/src/bootstrapper.rs | 14 ++++++++------ node/src/neighborhood/mod.rs | 13 +++++++++---- node/src/node_configurator/configurator.rs | 1 - node/src/node_configurator/mod.rs | 4 ++-- node/src/sub_lib/utils.rs | 1 - 8 files changed, 21 insertions(+), 19 deletions(-) diff --git a/node/src/accountant/mod.rs b/node/src/accountant/mod.rs index 199094756..2a175e3b2 100644 --- a/node/src/accountant/mod.rs +++ b/node/src/accountant/mod.rs @@ -949,7 +949,7 @@ mod tests { use actix::{Arbiter, System}; use ethereum_types::{BigEndianHash, U64}; use ethsign_crypto::Keccak256; - use itertools::{Either, Itertools}; + use itertools::Itertools; use log::Level; use masq_lib::constants::{ REQUEST_WITH_MUTUALLY_EXCLUSIVE_PARAMS, REQUEST_WITH_NO_VALUES, SCAN_ERROR, diff --git a/node/src/actor_system_factory.rs b/node/src/actor_system_factory.rs index 27b0d5b48..d4060bf9b 100644 --- a/node/src/actor_system_factory.rs +++ b/node/src/actor_system_factory.rs @@ -662,7 +662,6 @@ mod tests { parameterizable_automap_control, TransactorMock, PUBLIC_IP, ROUTER_IP, }; use crossbeam_channel::{bounded, unbounded, Sender}; - use itertools::Either; use log::LevelFilter; use masq_lib::constants::DEFAULT_CHAIN; use masq_lib::crash_point::CrashPoint; diff --git a/node/src/blockchain/blockchain_bridge.rs b/node/src/blockchain/blockchain_bridge.rs index 462f59888..4fff8e4ae 100644 --- a/node/src/blockchain/blockchain_bridge.rs +++ b/node/src/blockchain/blockchain_bridge.rs @@ -10,7 +10,7 @@ use crate::blockchain::blockchain_interface::{ BlockchainInterfaceNonClandestine, BlockchainResult, BlockchainTxnInputs, }; use crate::database::db_initializer::DbInitializationConfig; -use crate::database::db_initializer::{DbInitializer, DATABASE_FILE}; +use crate::database::db_initializer::DbInitializer; use crate::db_config::config_dao::ConfigDaoReal; use crate::db_config::persistent_configuration::{ PersistentConfiguration, PersistentConfigurationReal, @@ -429,7 +429,6 @@ mod tests { use crate::database::db_initializer::DbInitializerReal; use crate::db_config::persistent_configuration::PersistentConfigError; use crate::match_every_type_id; - use crate::node_configurator::configurator::Configurator; use crate::node_test_utils::check_timestamp; use crate::test_utils::persistent_configuration_mock::PersistentConfigurationMock; use crate::test_utils::recorder::{make_recorder, peer_actors_builder}; @@ -443,7 +442,6 @@ mod tests { use actix::System; use ethereum_types::{BigEndianHash, U64}; use ethsign_crypto::Keccak256; - use itertools::Either; use masq_lib::constants::DEFAULT_CHAIN; use masq_lib::messages::ScanType; use masq_lib::test_utils::logging::init_test_logging; diff --git a/node/src/bootstrapper.rs b/node/src/bootstrapper.rs index cf82e0828..50eb9a6a5 100644 --- a/node/src/bootstrapper.rs +++ b/node/src/bootstrapper.rs @@ -33,6 +33,7 @@ use crate::sub_lib::neighborhood::{NeighborhoodConfig, NeighborhoodMode}; use crate::sub_lib::node_addr::NodeAddr; use crate::sub_lib::socket_server::ConfiguredByPrivilege; use crate::sub_lib::ui_gateway::UiGatewayConfig; +use crate::sub_lib::utils::db_connection_launch_panic; use crate::sub_lib::wallet::Wallet; use futures::try_ready; use itertools::Itertools; @@ -58,7 +59,6 @@ use tokio::prelude::stream::futures_unordered::FuturesUnordered; use tokio::prelude::Async; use tokio::prelude::Future; use tokio::prelude::Stream; -use crate::sub_lib::utils::db_connection_launch_panic; static mut MAIN_CRYPTDE_BOX_OPT: Option> = None; static mut ALIAS_CRYPTDE_BOX_OPT: Option> = None; @@ -635,7 +635,9 @@ impl Bootstrapper { &self.config.data_directory, DbInitializationConfig::panic_on_migration(), ) - .unwrap_or_else(|err| db_connection_launch_panic(err, &self.config.data_directory)); + .unwrap_or_else(|err| { + db_connection_launch_panic(err, &self.config.data_directory) + }); let config_dao = ConfigDaoReal::new(conn); let mut persistent_config = PersistentConfigurationReal::new(Box::new(config_dao)); let clandestine_port = self.establish_clandestine_port(&mut persistent_config); @@ -733,7 +735,9 @@ mod tests { use crate::sub_lib::cryptde::PublicKey; use crate::sub_lib::cryptde::{CryptDE, PlainData}; use crate::sub_lib::cryptde_null::CryptDENull; - use crate::sub_lib::neighborhood::{DEFAULT_RATE_PACK, NeighborhoodConfig, NeighborhoodMode, NodeDescriptor, RatePack}; + use crate::sub_lib::neighborhood::{ + NeighborhoodConfig, NeighborhoodMode, NodeDescriptor, DEFAULT_RATE_PACK, + }; use crate::sub_lib::node_addr::NodeAddr; use crate::sub_lib::socket_server::ConfiguredByPrivilege; use crate::sub_lib::stream_connector::ConnectionInfo; @@ -752,7 +756,6 @@ mod tests { use actix::System; use crossbeam_channel::unbounded; use futures::Future; - use itertools::Either; use lazy_static::lazy_static; use log::LevelFilter; use log::LevelFilter::Off; @@ -1402,7 +1405,7 @@ mod tests { let act = |data_dir: &Path| { let mut config = BootstrapperConfig::new(); config.data_directory = data_dir.to_path_buf(); - let mut subject = BootstrapperBuilder::new().config(config).build(); + let subject = BootstrapperBuilder::new().config(config).build(); subject.start_actors_and_return_shp_subs(); }; @@ -2068,7 +2071,6 @@ mod tests { assert_on_initialization_with_panic_on_migration(&data_dir, &act); } - #[test] #[should_panic( expected = "Database is corrupt: error setting clandestine port: TransactionError" diff --git a/node/src/neighborhood/mod.rs b/node/src/neighborhood/mod.rs index 0ab8bf3ac..45685587b 100644 --- a/node/src/neighborhood/mod.rs +++ b/node/src/neighborhood/mod.rs @@ -64,7 +64,9 @@ 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}; +use crate::sub_lib::utils::{ + db_connection_launch_panic, handle_ui_crash_request, NODE_MAILBOX_CAPACITY, +}; use crate::sub_lib::versioned_data::VersionedData; use crate::sub_lib::wallet::Wallet; use gossip_acceptor::GossipAcceptor; @@ -1592,7 +1594,6 @@ mod tests { use std::time::Duration; use std::time::Instant; use tokio::prelude::Future; - use masq_lib::blockchains::chains::Chain::PolyMumbai; use masq_lib::constants::{DEFAULT_CHAIN, TLS_PORT}; use masq_lib::messages::{ToMessageBody, UiConnectionChangeBroadcast, UiConnectionStage}; @@ -1633,11 +1634,16 @@ mod tests { use crate::test_utils::recorder::peer_actors_builder; use crate::test_utils::recorder::Recorder; use crate::test_utils::recorder::Recording; - use crate::test_utils::unshared_test_utils::{make_node_to_ui_recipient, make_recipient_and_recording_arc, prove_that_crash_request_handler_is_hooked_up, AssertionsMessage, assert_on_initialization_with_panic_on_migration}; + use crate::test_utils::unshared_test_utils::{ + assert_on_initialization_with_panic_on_migration, make_node_to_ui_recipient, + make_recipient_and_recording_arc, prove_that_crash_request_handler_is_hooked_up, + AssertionsMessage, + }; use crate::test_utils::vec_to_set; use crate::test_utils::{main_cryptde, make_paying_wallet}; use super::*; + use crate::accountant::test_utils::bc_from_earning_wallet; use crate::neighborhood::overall_connection_status::ConnectionStageErrors::{ NoGossipResponseReceived, PassLoopFound, TcpConnectionFailed, }; @@ -1646,7 +1652,6 @@ mod tests { }; use crate::test_utils::unshared_test_utils::notify_handlers::NotifyLaterHandleMock; use masq_lib::test_utils::logging::{init_test_logging, TestLogHandler}; - use crate::accountant::test_utils::bc_from_earning_wallet; impl Handler> for Neighborhood { type Result = (); diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index 0c53ac78d..0223bebd3 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -842,7 +842,6 @@ mod tests { prove_that_crash_request_handler_is_hooked_up, ZERO, }; use bip39::{Language, Mnemonic}; - use itertools::Either; use masq_lib::blockchains::chains::Chain; use masq_lib::constants::MISSING_DATA; use masq_lib::test_utils::utils::ensure_node_home_directory_exists; diff --git a/node/src/node_configurator/mod.rs b/node/src/node_configurator/mod.rs index 9b3391da2..294854a40 100644 --- a/node/src/node_configurator/mod.rs +++ b/node/src/node_configurator/mod.rs @@ -7,7 +7,7 @@ pub mod unprivileged_parse_args_configuration; use crate::bootstrapper::RealUser; use crate::database::db_initializer::DbInitializationConfig; -use crate::database::db_initializer::{DbInitializer, DbInitializerReal, DATABASE_FILE}; +use crate::database::db_initializer::{DbInitializer, DbInitializerReal}; use crate::db_config::persistent_configuration::{ PersistentConfiguration, PersistentConfigurationReal, }; @@ -69,7 +69,7 @@ pub fn initialize_database( ) -> Box { let conn = DbInitializerReal::default() .initialize(data_directory, migrator_config) - .unwrap_or_else(|e| db_connection_launch_panic(e, &data_directory)); + .unwrap_or_else(|e| db_connection_launch_panic(e, data_directory)); Box::new(PersistentConfigurationReal::from(conn)) } diff --git a/node/src/sub_lib/utils.rs b/node/src/sub_lib/utils.rs index d1237da37..65767bce1 100644 --- a/node/src/sub_lib/utils.rs +++ b/node/src/sub_lib/utils.rs @@ -253,7 +253,6 @@ mod tests { use crate::apps::app_node; use actix::{Handler, System}; use crossbeam_channel::{unbounded, Sender}; - use futures::Future; use log::Level; use masq_lib::messages::ToMessageBody; use masq_lib::multi_config::CommandLineVcl; From 5d1e009164da562213d0410d9a903cefe0d63e82 Mon Sep 17 00:00:00 2001 From: Syther007 Date: Wed, 19 Apr 2023 23:17:43 +1200 Subject: [PATCH 5/7] GH-622: Decreased last_remapped duration for maybe_remap_handles_remapping_error --- automap/src/comm_layer/pmp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/automap/src/comm_layer/pmp.rs b/automap/src/comm_layer/pmp.rs index b5833a9df..db6f63adc 100644 --- a/automap/src/comm_layer/pmp.rs +++ b/automap/src/comm_layer/pmp.rs @@ -1442,7 +1442,7 @@ mod tests { next_lifetime: Duration::from_secs(600), remap_interval: Duration::from_secs(0), }; - let mut last_remapped = Instant::now().sub(Duration::from_secs(3600)); + let mut last_remapped = Instant::now().sub(Duration::from_secs(60)); let logger = Logger::new("maybe_remap_handles_remapping_error"); let transactor = PmpTransactor::new(); let mut subject = From 79f5ccf293ee801b342935ff6a0df74df7e1f20c Mon Sep 17 00:00:00 2001 From: Syther007 Date: Wed, 19 Apr 2023 23:21:16 +1200 Subject: [PATCH 6/7] Revert "GH-622: Decreased last_remapped duration for maybe_remap_handles_remapping_error" This reverts commit 5d1e009164da562213d0410d9a903cefe0d63e82. --- automap/src/comm_layer/pmp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/automap/src/comm_layer/pmp.rs b/automap/src/comm_layer/pmp.rs index db6f63adc..b5833a9df 100644 --- a/automap/src/comm_layer/pmp.rs +++ b/automap/src/comm_layer/pmp.rs @@ -1442,7 +1442,7 @@ mod tests { next_lifetime: Duration::from_secs(600), remap_interval: Duration::from_secs(0), }; - let mut last_remapped = Instant::now().sub(Duration::from_secs(60)); + let mut last_remapped = Instant::now().sub(Duration::from_secs(3600)); let logger = Logger::new("maybe_remap_handles_remapping_error"); let transactor = PmpTransactor::new(); let mut subject = From fede0bc3b38ace0c3e3738db2275efd87e2fcf05 Mon Sep 17 00:00:00 2001 From: Syther007 Date: Fri, 21 Apr 2023 22:20:09 +1200 Subject: [PATCH 7/7] GH-662: Amended Subjects name --- node/src/node_configurator/configurator.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/node/src/node_configurator/configurator.rs b/node/src/node_configurator/configurator.rs index 0223bebd3..7a9da5f62 100644 --- a/node/src/node_configurator/configurator.rs +++ b/node/src/node_configurator/configurator.rs @@ -866,11 +866,11 @@ mod tests { let (recorder, _, _) = make_recorder(); let recorder_addr = recorder.start(); - let mut configurator = Configurator::new(data_dir, false); + let mut subject = Configurator::new(data_dir, false); - configurator.node_to_ui_sub = Some(recorder_addr.recipient()); - configurator.new_password_subs = Some(vec![]); - let _ = configurator.handle_change_password( + subject.node_to_ui_sub = Some(recorder_addr.recipient()); + subject.new_password_subs = Some(vec![]); + let _ = subject.handle_change_password( UiChangePasswordRequest { old_password_opt: None, new_password: "password".to_string(),