Fix for many wallet config errors (#731)

* Added an extra warning message when failing to reach the node, work in progress on changing default dir for node api ssecret 'foreign_api_secret'

* In the middle of debugging hell, default overwrite any updates to the config made earlier, its never written to a file when running init

* More debugging, next trace the overwrite of the config

* Added get get_node_path function, seperating wallet dir and node_dir -compiling but some  warnings

* Tested wallet config creation fix for Issues: #728 #3394 Pull 3420, partial fix for 3002

* Removed some debugging code

* Fixed passing config to create_config function, some more test needed before pulling to main

* All test succeed: a) normal, b) top-dir, c) here, d) with default config template in working dir

* Cleaning up

* Fixed a single test for config that needed adjusting

Cargo.lock

@@ -1,6 +1,6 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
-version = 3
+version = 4
 
 [[package]]
 name = "addr2line"

config/src/config.rs

@@ -14,7 +14,6 @@
 
 //! Configuration file management
 
-use dirs;
 use rand::distributions::{Alphanumeric, Distribution};
 use rand::thread_rng;
 use std::env;
@@ -35,7 +34,8 @@ use crate::util::logger::LoggingConfig;
 /// Wallet configuration file name
 pub const WALLET_CONFIG_FILE_NAME: &str = "grin-wallet.toml";
 const WALLET_LOG_FILE_NAME: &str = "grin-wallet.log";
-const GRIN_HOME: &str = ".grin";
+/// .grin folder, usually in home/.grin
+pub const GRIN_HOME: &str = ".grin";
 /// Wallet data directory
 pub const GRIN_WALLET_DIR: &str = "wallet_data";
 /// Node API secret
@@ -43,7 +43,8 @@ pub const API_SECRET_FILE_NAME: &str = ".foreign_api_secret";
 /// Owner API secret
 pub const OWNER_API_SECRET_FILE_NAME: &str = ".owner_api_secret";
 
-fn get_grin_path(
+/// Function to get wallet dir and create dirs if not existing
+pub fn get_wallet_path(
 	chain_type: &global::ChainTypes,
 	create_path: bool,
 ) -> Result<PathBuf, ConfigError> {
@@ -68,6 +69,49 @@ fn get_grin_path(
 	}
 }
 
+/// Smart function to find the most likely .api_secret location for the node
+pub fn get_node_path(
+	data_path: Option<PathBuf>,
+	chain_type: &global::ChainTypes,
+) -> Result<PathBuf, ConfigError> {
+	let node_path = match data_path {
+		// 1) A If top dir provided and api_secret exist, return top dir
+		Some(path) => {
+			let mut node_path = path;
+			node_path.push(GRIN_HOME);
+			node_path.push(chain_type.shortname());
+			node_path.push(API_SECRET_FILE_NAME);
+			if node_path.exists() {
+				node_path.pop();
+				Ok(node_path)
+
+			// 1) B If top dir exists, but no config there, return home dir
+			} else {
+				let mut node_path = match dirs::home_dir() {
+					Some(p) => p,
+					None => PathBuf::new(),
+				};
+				node_path.push(GRIN_HOME);
+				node_path.push(chain_type.shortname());
+				Ok(node_path)
+			}
+		}
+
+		// 2) If there is no top_dir provided, always return home dir
+		None => {
+			let mut node_path = match dirs::home_dir() {
+				Some(p) => p,
+				None => PathBuf::new(),
+			};
+			node_path.push(GRIN_HOME);
+			node_path.push(chain_type.shortname());
+			Ok(node_path)
+		}
+	};
+	node_path
+}
+
+/// Checks if config in current working dir
 fn check_config_current_dir(path: &str) -> Option<PathBuf> {
 	let p = env::current_dir();
 	let mut c = match p {
@@ -122,7 +166,7 @@ fn check_api_secret_file(
 ) -> Result<(), ConfigError> {
 	let grin_path = match data_path {
 		Some(p) => p,
-		None => get_grin_path(chain_type, false)?,
+		None => get_node_path(data_path, chain_type)?,
 	};
 	let mut api_secret_path = grin_path;
 	api_secret_path.push(file_name);
@@ -134,6 +178,7 @@ fn check_api_secret_file(
 }
 
 /// Handles setup and detection of paths for wallet
+/// Use config file in a) current dir as template, b) in top path, or c) .grin home
 pub fn initial_setup_wallet(
 	chain_type: &global::ChainTypes,
 	data_path: Option<PathBuf>,
@@ -144,33 +189,45 @@ pub fn initial_setup_wallet(
 			fs::create_dir_all(p)?;
 		}
 	}
-	// Use config file if current directory if it exists, .grin home otherwise
+
+	// Get wallet data_dir path, create it if it does not exist
+	let wallet_path = match data_path {
+		Some(p) => {
+			let mut abs_wallet_path = std::env::current_dir()?;
+			abs_wallet_path.push(p);
+			abs_wallet_path
+		}
+		None => get_wallet_path(chain_type, create_path)?,
+	};
+
+	// Get path to the node dir(s), first try top dir, if no node api_secret return home./grin
+	let node_path = get_node_path(Some(wallet_path.clone()), chain_type)?;
+
+	// Get path to the newwly to be created config file
+	let mut config_path = wallet_path.clone();
+	config_path.push(WALLET_CONFIG_FILE_NAME);
+
+	// Check if config exists in working dir, if so, use it as template for newly created config
 	let (path, config) = if let Some(p) = check_config_current_dir(WALLET_CONFIG_FILE_NAME) {
 		let mut path = p.clone();
 		path.pop();
-		(path, GlobalWalletConfig::new(p.to_str().unwrap())?)
+		let mut config = GlobalWalletConfig::new(p.to_str().unwrap())?;
+		// Use template config, update data_dir, network and api secrets
+		config.config_file_path = Some(config_path);
+		config.update_paths(&wallet_path, &node_path);
+		(path, config)
 	} else {
-		// Check if grin dir exists
+		// Return defaults config updated with node and wallet dir and chain dir
-		let grin_path = match data_path {
-			Some(p) => p,
-			None => get_grin_path(chain_type, create_path)?,
-		};
-
-		// Get path to default config file
-		let mut config_path = grin_path.clone();
-		config_path.push(WALLET_CONFIG_FILE_NAME);
-
-		// Return defaults if file doesn't exist
 		match config_path.clone().exists() {
 			false => {
 				let mut default_config = GlobalWalletConfig::for_chain(chain_type);
 				default_config.config_file_path = Some(config_path);
-				// update paths relative to current dir
+				// Update paths relative to current dir
-				default_config.update_paths(&grin_path);
+				default_config.update_paths(&wallet_path, &node_path);
-				(grin_path, default_config)
+				(wallet_path, default_config)
 			}
 			true => {
-				let mut path = config_path.clone();
+				let mut path = wallet_path.clone();
 				path.pop();
 				(
 					path,
@@ -179,7 +236,7 @@ pub fn initial_setup_wallet(
 			}
 		}
 	};
-
+	// Check API secrets, if ok, return config
 	check_api_secret_file(chain_type, Some(path.clone()), OWNER_API_SECRET_FILE_NAME)?;
 	check_api_secret_file(chain_type, Some(path), API_SECRET_FILE_NAME)?;
 	Ok(config)
@@ -268,7 +325,7 @@ impl GlobalWalletConfig {
 	}
 
 	/// Update paths
-	pub fn update_paths(&mut self, wallet_home: &PathBuf) {
+	pub fn update_paths(&mut self, wallet_home: &PathBuf, node_home: &PathBuf) {
 		let mut wallet_path = wallet_home.clone();
 		wallet_path.push(GRIN_WALLET_DIR);
 		self.members.as_mut().unwrap().wallet.data_file_dir =
@@ -277,7 +334,7 @@ impl GlobalWalletConfig {
 		secret_path.push(OWNER_API_SECRET_FILE_NAME);
 		self.members.as_mut().unwrap().wallet.api_secret_path =
 			Some(secret_path.to_str().unwrap().to_owned());
-		let mut node_secret_path = wallet_home.clone();
+		let mut node_secret_path = node_home.clone();
 		node_secret_path.push(API_SECRET_FILE_NAME);
 		self.members.as_mut().unwrap().wallet.node_api_secret_path =
 			Some(node_secret_path.to_str().unwrap().to_owned());

controller/src/command.rs

@@ -70,6 +70,7 @@ pub struct InitArgs {
 	pub restore: bool,
 }
 
+/// Write config (default if None), initiate the wallet
 pub fn init<L, C, K>(
 	owner_api: &mut Owner<L, C, K>,
 	_g_args: &GlobalArgs,
@@ -81,12 +82,16 @@ where
 	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(&chain_type, WALLET_CONFIG_FILE_NAME, None, None, None)?;
+	p.create_config(
+		&chain_type,
+		WALLET_CONFIG_FILE_NAME,
+		Some(args.config),
+		None,
+		None,
+	)?;
 	p.create_wallet(
 		None,
 		args.recovery_phrase,

impls/src/lifecycle/default.rs

@@ -87,11 +87,12 @@ where
 				None => None,
 			},
 		};
-		let wallet = match wallet_config {
+		// Check if config was provided, if not load default and set update to "true"
-			Some(w) => w,
+		let (wallet, update) = match wallet_config {
+			Some(w) => (w, false),
 			None => match default_config.members.as_ref() {
-				Some(m) => m.clone().wallet,
+				Some(m) => (m.clone().wallet, true),
-				None => WalletConfig::default(),
+				None => (WalletConfig::default(), true),
 			},
 		};
 		let tor = match tor_config {
@@ -136,11 +137,16 @@ where
 		if config_file_name.exists() {
 			return Ok(());
 		}
+		// default settings are updated if no config was provided, no support for top_dir/here
+		let mut abs_path_node = std::env::current_dir()?;
+		abs_path_node.push(self.data_dir.clone());
+		let mut absolute_path_wallet = std::env::current_dir()?;
+		absolute_path_wallet.push(self.data_dir.clone());
 
-		let mut abs_path = std::env::current_dir()?;
+		// if no config provided, update defaults
-		abs_path.push(self.data_dir.clone());
+		if update == true {
-
+			default_config.update_paths(&abs_path_node, &absolute_path_wallet);
-		default_config.update_paths(&abs_path);
+		};
 		let res =
 			default_config.write_to_file(config_file_name.to_str().unwrap(), false, None, None);
 		if let Err(e) = res {

impls/src/node_clients/http.rs

@@ -128,7 +128,8 @@ impl NodeClient for HTTPNodeClient {
 						verified: Some(false),
 					});
 				} else {
-					error!("Unable to contact Node to get version info: {}", e);
+					error!("Unable to contact Node to get version info: {}, check your node is running", e);
+					warn!("Warning: a) Node is offline, or b) 'node_api_secret_path' in 'grin-wallet.toml' is set incorrectly");
 					return None;
 				}
 			}

src/bin/grin-wallet.rs

@@ -87,7 +87,7 @@ fn real_main() -> i32 {
 		let res = args.value_of("top_level_dir");
 		match res {
 			Some(d) => {
-				current_dir = Some(PathBuf::from(d));
+				current_dir = Some(PathBuf::from(d.replace("/", "\\")));
 			}
 			None => {
 				warn!("Argument --top_level_dir needs a value. Defaulting to current directory")
@@ -138,7 +138,6 @@ fn real_main() -> i32 {
 		"Using wallet configuration file at {}",
 		config.config_file_path.as_ref().unwrap().to_str().unwrap()
 	);
-
 	log_build_info();
 
 	global::init_global_chain_type(
@@ -152,11 +151,9 @@ fn real_main() -> i32 {
 			.unwrap()
 			.clone(),
 	);
-
 	global::init_global_accept_fee_base(config.members.as_ref().unwrap().wallet.accept_fee_base());
 
 	let wallet_config = config.clone().members.unwrap().wallet;
 	let node_client = HTTPNodeClient::new(&wallet_config.check_node_api_http_addr, None).unwrap();
-
 	cmd::wallet_command(&args, config, node_client)
 }

tests/common/mod.rs

@@ -165,7 +165,7 @@ pub fn config_command_wallet(
 				.to_owned(),
 		))?;
 	}
-	default_config.update_paths(&current_dir);
+	default_config.update_paths(&current_dir, &current_dir);
 	default_config
 		.write_to_file(config_file_name.to_str().unwrap(), false, None, None)
 		.unwrap_or_else(|e| {