fixes to wallet based on testing, ensure details file is being written properly for file wallet (#1204)

src/bin/grin.rs

@@ -739,4 +739,6 @@ fn wallet_command(wallet_args: &ArgMatches, global_config: GlobalConfig) {
 			}
 		});
 	}
+	// we need to give log output a chance to catch up before exiting
+	thread::sleep(Duration::from_millis(100));
 }

wallet/src/file_wallet.rs

@@ -14,7 +14,7 @@
 
 use std::collections::HashMap;
 use std::fs::{self, File};
-use std::io::{Read, Write};
+use std::io::Write;
 use std::path::{Path, MAIN_SEPARATOR};
 
 use serde_json;
@@ -44,14 +44,17 @@ const DET_BCK_FILE: &'static str = "wallet.detbck";
 const DAT_FILE: &'static str = "wallet.dat";
 const BCK_FILE: &'static str = "wallet.bck";
 const LOCK_FILE: &'static str = "wallet.lock";
-const SEED_FILE: &'static str = "wallet.seed";
 
 #[derive(Debug)]
 struct FileBatch<'a> {
 	/// List of outputs
 	outputs: &'a mut HashMap<String, OutputData>,
+	/// Wallet Details
+	details: &'a mut WalletDetails,
 	/// Data file path
 	data_file_path: String,
+	/// Details file path
+	details_file_path: String,
 	/// lock file path
 	lock_file_path: String,
 }
@@ -62,6 +65,10 @@ impl<'a> WalletOutputBatch for FileBatch<'a> {
 		Ok(())
 	}
 
+	fn details(&mut self) -> &mut WalletDetails {
+		&mut self.details
+	}
+
 	fn get(&self, id: &Identifier) -> Result<OutputData, libwallet::Error> {
 		self.outputs
 			.get(&id.to_hex())
@@ -69,6 +76,10 @@ impl<'a> WalletOutputBatch for FileBatch<'a> {
 			.ok_or(libwallet::ErrorKind::Backend("not found".to_string()).into())
 	}
 
+	fn iter<'b>(&'b self) -> Box<Iterator<Item = OutputData> + 'b> {
+		Box::new(self.outputs.values().cloned())
+	}
+
 	fn delete(&mut self, id: &Identifier) -> Result<(), libwallet::Error> {
 		let _ = self.outputs.remove(&id.to_hex());
 		Ok(())
@@ -86,17 +97,27 @@ impl<'a> WalletOutputBatch for FileBatch<'a> {
 	fn commit(&self) -> Result<(), libwallet::Error> {
 		let mut data_file = File::create(self.data_file_path.clone())
 			.context(libwallet::ErrorKind::CallbackImpl("Could not create"))?;
+		let mut details_file = File::create(self.details_file_path.clone())
+			.context(libwallet::ErrorKind::CallbackImpl("Could not create"))?;
 		let mut outputs = self.outputs.values().collect::<Vec<_>>();
 		outputs.sort();
 		let res_json = serde_json::to_vec_pretty(&outputs).context(
-			libwallet::ErrorKind::CallbackImpl("Error serializing wallet data"),
+			libwallet::ErrorKind::CallbackImpl("Error serializing wallet output data"),
+		)?;
+		let details_res_json = serde_json::to_vec_pretty(&self.details).context(
+			libwallet::ErrorKind::CallbackImpl("Error serializing wallet details data"),
 		)?;
 		data_file
 			.write_all(res_json.as_slice())
 			.context(libwallet::ErrorKind::CallbackImpl(
-				"Error writing wallet file",
-			))
-			.map_err(|e| e.into())
+				"Error writing wallet data file",
+			))?;
+		details_file
+			.write_all(details_res_json.as_slice())
+			.context(libwallet::ErrorKind::CallbackImpl(
+				"Error writing wallet details file",
+			))?;
+		Ok(())
 	}
 }
 
@@ -189,7 +210,9 @@ where
 
 		Ok(Box::new(FileBatch {
 			outputs: &mut self.outputs,
+			details: &mut self.details,
 			data_file_path: self.data_file_path.clone(),
+			details_file_path: self.details_file_path.clone(),
 			lock_file_path: self.lock_file_path.clone(),
 		}))
 	}
@@ -199,19 +222,23 @@ where
 		&'a mut self,
 		root_key_id: keychain::Identifier,
 	) -> Result<u32, libwallet::Error> {
+		let mut batch = self.batch()?;
+		{
 			let mut max_n = 0;
-		for out in self.outputs.values() {
+			for out in batch.iter() {
 				if max_n < out.n_child && out.root_key_id == root_key_id {
 					max_n = out.n_child;
 				}
 			}
-
-		if self.details.last_child_index <= max_n {
-			self.details.last_child_index = max_n + 1;
+			let details = batch.details();
+			if details.last_child_index <= max_n {
+				details.last_child_index = max_n + 1;
 			} else {
-			self.details.last_child_index += 1;
+				details.last_child_index += 1;
 			}
-		Ok(self.details.last_child_index)
+		}
+		batch.commit()?;
+		Ok(batch.details().last_child_index)
 	}
 
 	/// Select spendable coins from the wallet.
@@ -304,18 +331,30 @@ impl<K> WalletClient for FileWallet<K> {
 			Ok(r) => Ok(r),
 			Err(e) => {
 				let message = format!("{}", e.cause().unwrap());
+				error!(
+					LOGGER,
+					"Create Coinbase: Communication error: {},{}",
+					e.cause().unwrap(),
+					e.backtrace().unwrap()
+				);
 				Err(libwallet::ErrorKind::WalletComms(message))?
 			}
 		}
 	}
 
 	/// Send a transaction slate to another listening wallet and return result
-	fn send_tx_slate(&self, slate: &Slate) -> Result<Slate, libwallet::Error> {
-		let res = client::send_tx_slate(self.node_url(), slate);
+	fn send_tx_slate(&self, addr: &str, slate: &Slate) -> Result<Slate, libwallet::Error> {
+		let res = client::send_tx_slate(addr, slate);
 		match res {
 			Ok(r) => Ok(r),
 			Err(e) => {
 				let message = format!("{}", e.cause().unwrap());
+				error!(
+					LOGGER,
+					"Send TX Slate: Communication error: {},{}",
+					e.cause().unwrap(),
+					e.backtrace().unwrap()
+				);
 				Err(libwallet::ErrorKind::WalletComms(message))?
 			}
 		}

wallet/src/libwallet/api.rs

@@ -103,7 +103,7 @@ where
 			selection_strategy_is_use_all,
 		)?;
 
-		let mut slate = match self.wallet.send_tx_slate(&slate) {
+		let mut slate = match self.wallet.send_tx_slate(dest, &slate) {
 			Ok(s) => s,
 			Err(e) => {
 				error!(

wallet/src/libwallet/controller.rs

@@ -31,8 +31,9 @@ use failure::Fail;
 use keychain::Keychain;
 use libtx::slate::Slate;
 use libwallet::api::{APIForeign, APIOwner};
-use libwallet::types::{BlockFees, CbData, OutputData, SendTXArgs, WalletBackend, WalletClient,
-                       WalletInfo};
+use libwallet::types::{
+	BlockFees, CbData, OutputData, SendTXArgs, WalletBackend, WalletClient, WalletInfo,
+};
 use libwallet::{Error, ErrorKind};
 
 use util::LOGGER;
@@ -379,6 +380,7 @@ where
 	T: WalletBackend<K> + WalletClient,
 	K: Keychain,
 {
+	/// create a new api handler
 	pub fn new(wallet: Arc<Mutex<T>>) -> ForeignAPIHandler<T, K> {
 		ForeignAPIHandler {
 			wallet,

wallet/src/libwallet/error.rs

@@ -48,8 +48,11 @@ pub enum ErrorKind {
 	},
 
 	/// Fee Exceeds amount
-	#[fail(display = "Fee exceeds amount: sender amount {}, recipient fee {}", sender_amount,
-	       recipient_fee)]
+	#[fail(
+		display = "Fee exceeds amount: sender amount {}, recipient fee {}",
+		sender_amount,
+		recipient_fee
+	)]
 	FeeExceedsAmount {
 		/// sender amount
 		sender_amount: u64,

wallet/src/libwallet/internal/selection.rs

@@ -15,7 +15,7 @@
 //! Selection of inputs for building transactions
 
 use keychain::{Identifier, Keychain};
-use libtx::{build, tx_fee, slate::Slate};
+use libtx::{build, slate::Slate, tx_fee};
 use libwallet::error::{Error, ErrorKind};
 use libwallet::internal::{keys, sigcontext};
 use libwallet::types::*;

wallet/src/libwallet/internal/tx.rs

@@ -32,7 +32,7 @@ where
 {
 	// create an output using the amount in the slate
 	let (_, mut context, receiver_create_fn) =
-		selection::build_recipient_output_with_slate(wallet, slate).unwrap();
+		selection::build_recipient_output_with_slate(wallet, slate)?;
 
 	// fill public keys
 	let _ = slate.fill_round_1(

wallet/src/libwallet/internal/updater.rs

@@ -16,7 +16,6 @@
 //! the wallet storage and update them.
 
 use failure::ResultExt;
-use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 
 use core::consensus::reward;
@@ -185,13 +184,15 @@ where
 					Some(_) => output.mark_unspent(),
 					None => output.mark_spent(),
 				};
-				batch.save(output);
+				batch.save(output)?;
 			}
 		}
+		{
+			let details = batch.details();
+			details.last_confirmed_height = height;
+		}
 		batch.commit()?;
 	}
-	let details = wallet.details();
-	details.last_confirmed_height = height;
 	Ok(())
 }
 
@@ -328,6 +329,7 @@ where
 	{
 		// Now acquire the wallet lock and write the new output.
 		let mut batch = wallet.batch()?;
+		{
 			batch.save(OutputData {
 				root_key_id: root_key_id.clone(),
 				key_id: key_id.clone(),
@@ -339,7 +341,8 @@ where
 				is_coinbase: true,
 				block: None,
 				merkle_proof: None,
-		});
+			})?;
+		}
 		batch.commit()?;
 	}
 

wallet/src/libwallet/types.rs

@@ -88,9 +88,15 @@ pub trait WalletOutputBatch {
 	/// Add or update data about an output to the backend
 	fn save(&mut self, out: OutputData) -> Result<(), Error>;
 
+	/// Get wallet details
+	fn details(&mut self) -> &mut WalletDetails;
+
 	/// Gets output data by id
 	fn get(&self, id: &Identifier) -> Result<OutputData, Error>;
 
+	/// Iterate over all output data in batch
+	fn iter<'b>(&'b self) -> Box<Iterator<Item = OutputData> + 'b>;
+
 	/// Delete data about an output to the backend
 	fn delete(&mut self, id: &Identifier) -> Result<(), Error>;
 
@@ -112,7 +118,7 @@ pub trait WalletClient {
 
 	/// Send a transaction slate to another listening wallet and return result
 	/// TODO: Probably need a slate wrapper type
-	fn send_tx_slate(&self, slate: &Slate) -> Result<Slate, Error>;
+	fn send_tx_slate(&self, addr: &str, slate: &Slate) -> Result<Slate, Error>;
 
 	/// Posts a transaction to a grin node
 	fn post_tx(&self, tx: &TxWrapper, fluff: bool) -> Result<(), Error>;

wallet/src/lmdb_wallet.rs

@@ -160,11 +160,19 @@ impl<'a, K> WalletOutputBatch for Batch<'a, K> {
 		Ok(())
 	}
 
+	fn details(&mut self) -> &mut WalletDetails {
+		unimplemented!()
+	}
+
 	fn get(&self, id: &Identifier) -> Result<OutputData, Error> {
 		let key = to_key(OUTPUT_PREFIX, &mut id.to_bytes().to_vec());
 		option_to_not_found(self.db.borrow().as_ref().unwrap().get_ser(&key)).map_err(|e| e.into())
 	}
 
+	fn iter<'b>(&'b self) -> Box<Iterator<Item = OutputData> + 'b> {
+		unimplemented!();
+	}
+
 	fn delete(&mut self, id: &Identifier) -> Result<(), Error> {
 		let key = to_key(OUTPUT_PREFIX, &mut id.to_bytes().to_vec());
 		self.db.borrow().as_ref().unwrap().delete(&key)?;
@@ -197,8 +205,8 @@ impl<K> WalletClient for LMDBBackend<K> {
 	}
 
 	/// Send a transaction slate to another listening wallet and return result
-	fn send_tx_slate(&self, slate: &Slate) -> Result<Slate, Error> {
-		let res = client::send_tx_slate(self.node_url(), slate)
+	fn send_tx_slate(&self, addr: &str, slate: &Slate) -> Result<Slate, Error> {
+		let res = client::send_tx_slate(addr, slate)
 			.context(ErrorKind::WalletComms(format!("Sending transaction")))?;
 		Ok(res)
 	}