Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-662: Write tests for usages of 'panic_on_migration()' #267

Merged
merged 9 commits into from
May 3, 2023
21 changes: 19 additions & 2 deletions node/src/accountant/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,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};
Expand Down Expand Up @@ -1043,10 +1043,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};

Expand Down Expand Up @@ -4338,4 +4340,19 @@ 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);
}
}
40 changes: 35 additions & 5 deletions node/src/actor_system_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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<Accountant> = arbiter.start(move |_| {
Accountant::new(
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -2081,6 +2093,24 @@ 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()
Expand Down
36 changes: 27 additions & 9 deletions node/src/blockchain/blockchain_bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,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,
Expand All @@ -20,7 +20,7 @@ use crate::sub_lib::blockchain_bridge::{BlockchainBridgeSubs, RequestBalancesToP
use crate::sub_lib::blockchain_bridge::{ConsumingWalletBalances, 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;
Expand Down Expand Up @@ -230,12 +230,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,
Expand Down Expand Up @@ -508,6 +503,7 @@ 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_test_utils::check_timestamp;
Expand All @@ -517,7 +513,8 @@ mod tests {
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,
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;
Expand All @@ -527,8 +524,10 @@ mod tests {
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::{Duration, SystemTime};
use web3::types::{TransactionReceipt, H160, H256, U256};
Expand Down Expand Up @@ -1713,4 +1712,23 @@ 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);
}
}
75 changes: 62 additions & 13 deletions node/src/bootstrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -32,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;
Expand Down Expand Up @@ -515,15 +517,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()));
Expand Down Expand Up @@ -612,6 +606,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: {}",
Expand All @@ -630,7 +635,9 @@ 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);
Expand Down Expand Up @@ -728,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::{NeighborhoodConfig, NeighborhoodMode, NodeDescriptor};
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;
Expand All @@ -738,7 +747,9 @@ 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;
Expand All @@ -762,7 +773,7 @@ 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;
Expand Down Expand Up @@ -1383,6 +1394,24 @@ 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 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(
) {
Expand Down Expand Up @@ -2022,6 +2051,26 @@ 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"
Expand Down
4 changes: 3 additions & 1 deletion node/src/daemon/setup_reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,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()));
Expand Down
8 changes: 2 additions & 6 deletions node/src/database/db_initializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::database::db_migrations::db_migrator::{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,
Expand Down Expand Up @@ -488,12 +489,7 @@ pub fn connection_or_panic(
) -> Box<dyn ConnectionWrapper> {
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)]
Expand Down
Loading