Merge pull request #412 from antiochp/local_chain_type

local vs global chain_type (for better test isolation)

api/src/foreign.rs

@@ -102,6 +102,7 @@ where
 	/// ```
 	/// use grin_wallet_util::grin_keychain as keychain;
 	/// use grin_wallet_util::grin_util as util;
+	/// use grin_wallet_util::grin_core;
 	/// use grin_wallet_api as api;
 	/// use grin_wallet_config as config;
 	/// use grin_wallet_impls as impls;
@@ -113,11 +114,15 @@ where
 	/// use std::sync::Arc;
 	/// use util::{Mutex, ZeroingString};
 	///
+	/// use grin_core::global;
+	///
 	/// use api::Foreign;
 	/// use config::WalletConfig;
 	/// use impls::{DefaultWalletImpl, DefaultLCProvider, HTTPNodeClient};
 	/// use libwallet::WalletInst;
 	///
+	/// global::set_local_chain_type(global::ChainTypes::AutomatedTesting);
+	///
 	/// let mut wallet_config = WalletConfig::default();
 	/// # let dir = tempdir().map_err(|e| format!("{:#?}", e)).unwrap();
 	/// # let dir = dir
@@ -406,9 +411,11 @@ macro_rules! doctest_helper_setup_doc_env_foreign {
 		use grin_wallet_config as config;
 		use grin_wallet_impls as impls;
 		use grin_wallet_libwallet as libwallet;
+		use grin_wallet_util::grin_core;
 		use grin_wallet_util::grin_keychain as keychain;
 		use grin_wallet_util::grin_util as util;
 
+		use grin_core::global;
 		use keychain::ExtKeychain;
 		use tempfile::tempdir;
 
@@ -423,7 +430,10 @@ macro_rules! doctest_helper_setup_doc_env_foreign {
 		// don't run on windows CI, which gives very inconsistent results
 		if cfg!(windows) {
 			return;
-		}
+			}
+
+		// Set our local chain_type for testing.
+		global::set_local_chain_type(global::ChainTypes::AutomatedTesting);
 
 		let dir = tempdir().map_err(|e| format!("{:#?}", e)).unwrap();
 		let dir = dir

api/src/foreign_rpc.rs

@@ -360,7 +360,7 @@ pub fn run_doctest_foreign(
 
 	util::init_test_logger();
 	let _ = fs::remove_dir_all(test_dir);
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let mut wallet_proxy: WalletProxy<
 		DefaultLCProvider<LocalWalletClient, ExtKeychain>,

api/src/owner.rs

@@ -109,11 +109,13 @@ where
 	/// ```
 	/// use grin_wallet_util::grin_keychain as keychain;
 	/// use grin_wallet_util::grin_util as util;
+	/// use grin_wallet_util::grin_core;
 	/// use grin_wallet_api as api;
 	/// use grin_wallet_config as config;
 	/// use grin_wallet_impls as impls;
 	/// use grin_wallet_libwallet as libwallet;
 	///
+	/// use grin_core::global;
 	/// use keychain::ExtKeychain;
 	/// use tempfile::tempdir;
 	///
@@ -125,6 +127,8 @@ where
 	/// use impls::{DefaultWalletImpl, DefaultLCProvider, HTTPNodeClient};
 	/// use libwallet::WalletInst;
 	///
+	/// global::set_local_chain_type(global::ChainTypes::AutomatedTesting);
+	///
 	/// let mut wallet_config = WalletConfig::default();
 	/// # let dir = tempdir().map_err(|e| format!("{:#?}", e)).unwrap();
 	/// # let dir = dir
@@ -2113,6 +2117,8 @@ macro_rules! doctest_helper_setup_doc_env {
 		use grin_wallet_util::grin_keychain as keychain;
 		use grin_wallet_util::grin_util as util;
 
+		use grin_core::global;
+
 		use keychain::ExtKeychain;
 		use tempfile::tempdir;
 
@@ -2129,7 +2135,10 @@ macro_rules! doctest_helper_setup_doc_env {
 		// don't run on windows CI, which gives very inconsistent results
 		if cfg!(windows) {
 			return;
-		}
+			}
+
+		// Set our local chain_type for testing.
+		global::set_local_chain_type(global::ChainTypes::AutomatedTesting);
 
 		let dir = tempdir().map_err(|e| format!("{:#?}", e)).unwrap();
 		let dir = dir

api/src/owner_rpc.rs

@@ -1937,7 +1937,7 @@ pub fn run_doctest_owner(
 
 	util::init_test_logger();
 	let _ = fs::remove_dir_all(test_dir);
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let mut wallet_proxy: WalletProxy<
 		DefaultLCProvider<LocalWalletClient, ExtKeychain>,

controller/src/command.rs

@@ -54,7 +54,6 @@ pub struct GlobalArgs {
 	pub api_secret: Option<String>,
 	pub node_api_secret: Option<String>,
 	pub show_spent: bool,
-	pub chain_type: global::ChainTypes,
 	pub password: Option<ZeroingString>,
 	pub tls_conf: Option<TLSConfig>,
 }
@@ -71,23 +70,20 @@ pub struct InitArgs {
 
 pub fn init<L, C, K>(
 	owner_api: &mut Owner<L, C, K>,
-	g_args: &GlobalArgs,
+	_g_args: &GlobalArgs,
 	args: InitArgs,
 ) -> Result<(), Error>
 where
 	L: WalletLCProvider<'static, C, K> + 'static,
 	C: NodeClient + 'static,
 	K: keychain::Keychain + 'static,
 {
+	// Assume global chain type has already been initialized.
+	let chain_type = global::get_chain_type();
+
 	let mut w_lock = owner_api.wallet_inst.lock();
 	let p = w_lock.lc_provider()?;
-	p.create_config(
-		&g_args.chain_type,
-		WALLET_CONFIG_FILE_NAME,
-		None,
-		None,
-		None,
-	)?;
+	p.create_config(&chain_type, WALLET_CONFIG_FILE_NAME, None, None, None)?;
 	p.create_wallet(
 		None,
 		args.recovery_phrase,

controller/tests/common/mod.rs

@@ -84,7 +84,15 @@ pub fn clean_output_dir(test_dir: &str) {
 pub fn setup(test_dir: &str) {
 	util::init_test_logger();
 	clean_output_dir(test_dir);
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
+}
+
+/// Some tests require the global chain_type to be configured due to threads being spawned internally.
+/// It is recommended to avoid relying on this if at all possible as global chain_type
+/// leaks across multiple tests and will likely have unintended consequences.
+#[allow(dead_code)]
+pub fn setup_global_chain_type() {
+	global::init_global_chain_type(global::ChainTypes::AutomatedTesting);
 }
 
 pub fn create_wallet_proxy(

controller/tests/updater_thread.rs

@@ -29,7 +29,7 @@ use std::time::Duration;
 
 #[macro_use]
 mod common;
-use common::{clean_output_dir, create_wallet_proxy, setup};
+use common::{clean_output_dir, create_wallet_proxy, setup, setup_global_chain_type};
 
 /// updater thread test impl
 fn updater_thread_test_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
@@ -110,6 +110,10 @@ fn updater_thread_test_impl(test_dir: &'static str) -> Result<(), libwallet::Err
 
 #[test]
 fn updater_thread() {
+	// The "updater" kicks off a new thread so we need to ensure the global chain_type
+	// is set for this to work correctly.
+	setup_global_chain_type();
+
 	let test_dir = "test_output/updater_thread";
 	setup(test_dir);
 	if let Err(e) = updater_thread_test_impl(test_dir) {

impls/src/lifecycle/default.rs

@@ -78,7 +78,7 @@ where
 		logging_config: Option<LoggingConfig>,
 		tor_config: Option<TorConfig>,
 	) -> Result<(), Error> {
-		let mut default_config = GlobalWalletConfig::for_chain(chain_type);
+		let mut default_config = GlobalWalletConfig::for_chain(&chain_type);
 		let logging = match logging_config {
 			Some(l) => Some(l),
 			None => match default_config.members.as_ref() {

impls/src/test_framework/testclient.rs

@@ -21,7 +21,7 @@ use crate::chain::types::NoopAdapter;
 use crate::chain::Chain;
 use crate::core::core::verifier_cache::LruVerifierCache;
 use crate::core::core::{Transaction, TxKernel};
-use crate::core::global::{set_mining_mode, ChainTypes};
+use crate::core::global::{set_local_chain_type, ChainTypes};
 use crate::core::pow;
 use crate::keychain::Keychain;
 use crate::libwallet;
@@ -93,7 +93,7 @@ where
 {
 	/// Create a new client that will communicate with the given grin node
 	pub fn new(chain_dir: &str) -> Self {
-		set_mining_mode(ChainTypes::AutomatedTesting);
+		set_local_chain_type(ChainTypes::AutomatedTesting);
 		let genesis_block = pow::mine_genesis_block().unwrap();
 		let verifier_cache = Arc::new(RwLock::new(LruVerifierCache::new()));
 		let dir_name = format!("{}/.grin", chain_dir);
@@ -136,6 +136,10 @@ where
 	/// Run the incoming message queue and respond more or less
 	/// synchronously
 	pub fn run(&mut self) -> Result<(), libwallet::Error> {
+		// We run the wallet_proxy within a spawned thread in tests.
+		// We set the local chain_type here within the thread.
+		set_local_chain_type(ChainTypes::AutomatedTesting);
+
 		self.running.store(true, Ordering::Relaxed);
 		loop {
 			thread::sleep(Duration::from_millis(10));

integration/tests/api.rs

@@ -33,7 +33,7 @@ fn simple_server_wallet() {
 	init_test_logger();
 	info!("starting simple_server_wallet");
 	let _test_name_dir = "test_servers";
-	core::global::set_mining_mode(core::global::ChainTypes::AutomatedTesting);
+	core::global::set_local_chain_type(core::global::ChainTypes::AutomatedTesting);
 
 	// Run a separate coinbase wallet for coinbase transactions
 	let coinbase_dir = "coinbase_wallet_api";
@@ -142,7 +142,7 @@ fn simple_server_wallet() {
 fn test_p2p() {
 	init_test_logger();
 	info!("starting test_p2p");
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let _test_name_dir = "test_servers";
 

integration/tests/dandelion.rs

@@ -32,7 +32,7 @@ use std::{thread, time};
 #[ignore]
 fn test_dandelion_timeout() {
 	let test_name_dir = "test_dandelion_timeout";
-	core::global::set_mining_mode(core::global::ChainTypes::AutomatedTesting);
+	core::global::set_local_chain_type(core::global::ChainTypes::AutomatedTesting);
 	framework::clean_all_output(test_name_dir);
 	let mut log_config = util::LoggingConfig::default();
 	//log_config.stdout_log_level = util::LogLevel::Trace;

integration/tests/simulnet.rs

@@ -52,7 +52,7 @@ use crate::framework::{
 #[test]
 fn basic_genesis_mine() {
 	util::init_test_logger();
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let test_name_dir = "genesis_mine";
 	framework::clean_all_output(test_name_dir);
@@ -84,7 +84,7 @@ fn basic_genesis_mine() {
 #[test]
 fn simulate_seeding() {
 	util::init_test_logger();
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let test_name_dir = "simulate_seeding";
 	framework::clean_all_output(test_name_dir);
@@ -151,7 +151,7 @@ fn simulate_seeding() {
 #[ignore]
 #[test]
 fn simulate_parallel_mining() {
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let test_name_dir = "simulate_parallel_mining";
 	// framework::clean_all_output(test_name_dir);
@@ -208,7 +208,7 @@ fn simulate_block_propagation() {
 
 	// we actually set the chain_type in the ServerConfig below
 	// TODO - avoid needing to set it in two places?
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let test_name_dir = "grin-prop";
 	framework::clean_all_output(test_name_dir);
@@ -268,7 +268,7 @@ fn simulate_full_sync() {
 	util::init_test_logger();
 
 	// we actually set the chain_type in the ServerConfig below
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let test_name_dir = "grin-sync";
 	framework::clean_all_output(test_name_dir);
@@ -324,7 +324,7 @@ fn simulate_fast_sync() {
 	util::init_test_logger();
 
 	// we actually set the chain_type in the ServerConfig below
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let test_name_dir = "grin-fast";
 	framework::clean_all_output(test_name_dir);
@@ -417,7 +417,7 @@ fn simulate_long_fork() {
 	println!("starting simulate_long_fork");
 
 	// we actually set the chain_type in the ServerConfig below
-	global::set_mining_mode(ChainTypes::AutomatedTesting);
+	global::set_local_chain_type(ChainTypes::AutomatedTesting);
 
 	let test_name_dir = "grin-long-fork";
 	framework::clean_all_output(test_name_dir);
@@ -903,7 +903,7 @@ pub fn create_wallet(
 #[test]
 fn replicate_tx_fluff_failure() {
 	util::init_test_logger();
-	global::set_mining_mode(ChainTypes::UserTesting);
+	global::set_local_chain_type(ChainTypes::UserTesting);
 	framework::clean_all_output("tx_fluff");
 
 	// Create Wallet 1 (Mining Input) and start it listening