Grin wallet check/repair (#2256)

* first pass at basic check_fix process

* rustfmt

* unlocks and tx log entry reversals in place

* log restore output at warn

* rename check_repair

* rename check_repair

* add command line functionality and sanity test

* rustfmt

* update wallet usage doc with check_repair

* doc update

* consistency in NotEnoughFunds output

* consistency in NotEnoughFunds output

wallet/src/command.rs

@@ -480,7 +480,7 @@ pub fn restore(
 		let result = api.restore();
 		match result {
 			Ok(_) => {
-				info!("Wallet restore complete",);
+				warn!("Wallet restore complete",);
 				Ok(())
 			}
 			Err(e) => {
@@ -492,3 +492,23 @@ pub fn restore(
 	})?;
 	Ok(())
 }
+
+pub fn check_repair(
+	wallet: Arc<Mutex<WalletInst<impl NodeClient + 'static, keychain::ExtKeychain>>>,
+) -> Result<(), Error> {
+	controller::owner_single_use(wallet.clone(), |api| {
+		let result = api.check_repair();
+		match result {
+			Ok(_) => {
+				warn!("Wallet check/repair complete",);
+				Ok(())
+			}
+			Err(e) => {
+				error!("Wallet check/repair failed: {}", e);
+				error!("Backtrace: {}", e.backtrace().unwrap());
+				Err(e)
+			}
+		}
+	})?;
+	Ok(())
+}

wallet/src/libwallet/api.rs

@@ -353,7 +353,7 @@ where
 
 		let res = Ok((
 			validated,
-			updater::retrieve_outputs(&mut *w, include_spent, tx_id, &parent_key_id)?,
+			updater::retrieve_outputs(&mut *w, include_spent, tx_id, Some(&parent_key_id))?,
 		));
 
 		w.close()?;
@@ -756,9 +756,19 @@ where
 	pub fn restore(&mut self) -> Result<(), Error> {
 		let mut w = self.wallet.lock();
 		w.open_with_credentials()?;
-		let res = w.restore();
+		w.restore()?;
 		w.close()?;
-		res
+		Ok(())
+	}
+
+	/// Attempt to check and fix the contents of the wallet
+	pub fn check_repair(&mut self) -> Result<(), Error> {
+		let mut w = self.wallet.lock();
+		w.open_with_credentials()?;
+		self.update_outputs(&mut w);
+		w.check_repair()?;
+		w.close()?;
+		Ok(())
 	}
 
 	/// Retrieve current height from node

wallet/src/libwallet/error.rs

@@ -34,37 +34,49 @@ pub enum ErrorKind {
 	/// Not enough funds
 	#[fail(
 		display = "Not enough funds. Required: {}, Available: {}",
-		needed, available
+		needed_disp, available_disp
 	)]
 	NotEnoughFunds {
 		/// available funds
 		available: u64,
+		/// Display friendly
+		available_disp: String,
 		/// Needed funds
 		needed: u64,
+		/// Display friendly
+		needed_disp: String,
 	},
 
 	/// Fee dispute
 	#[fail(
 		display = "Fee dispute: sender fee {}, recipient fee {}",
-		sender_fee, recipient_fee
+		sender_fee_disp, recipient_fee_disp
 	)]
 	FeeDispute {
 		/// sender fee
 		sender_fee: u64,
+		/// display friendly
+		sender_fee_disp: String,
 		/// recipient fee
 		recipient_fee: u64,
+		/// display friendly
+		recipient_fee_disp: String,
 	},
 
 	/// Fee Exceeds amount
 	#[fail(
 		display = "Fee exceeds amount: sender amount {}, recipient fee {}",
-		sender_amount, recipient_fee
+		sender_amount_disp, recipient_fee
 	)]
 	FeeExceedsAmount {
 		/// sender amount
 		sender_amount: u64,
+		/// display friendly
+		sender_amount_disp: String,
 		/// recipient fee
 		recipient_fee: u64,
+		/// display friendly
+		recipient_fee_disp: String,
 	},
 
 	/// LibTX Error

wallet/src/libwallet/internal/restore.rs

@@ -16,13 +16,14 @@
 use crate::core::global;
 use crate::core::libtx::proof;
 use crate::keychain::{ExtKeychain, Identifier, Keychain};
-use crate::libwallet::internal::keys;
+use crate::libwallet::internal::{keys, updater};
 use crate::libwallet::types::*;
 use crate::libwallet::Error;
 use crate::util::secp::{key::SecretKey, pedersen};
 use std::collections::HashMap;
 
 /// Utility struct for return values from below
+#[derive(Clone)]
 struct OutputResult {
 	///
 	pub commit: pedersen::Commitment,
@@ -53,7 +54,7 @@ where
 {
 	let mut wallet_outputs: Vec<OutputResult> = Vec::new();
 
-	info!(
+	warn!(
 		"Scanning {} outputs in the current Grin utxo set",
 		outputs.len(),
 	);
@@ -97,6 +98,212 @@ where
 	Ok(wallet_outputs)
 }
 
+fn collect_chain_outputs<T, C, K>(wallet: &mut T) -> Result<Vec<OutputResult>, Error>
+where
+	T: WalletBackend<C, K>,
+	C: NodeClient,
+	K: Keychain,
+{
+	let batch_size = 1000;
+	let mut start_index = 1;
+	let mut result_vec: Vec<OutputResult> = vec![];
+	loop {
+		let (highest_index, last_retrieved_index, outputs) = wallet
+			.w2n_client()
+			.get_outputs_by_pmmr_index(start_index, batch_size)?;
+		warn!(
+			"Checking {} outputs, up to index {}. (Highest index: {})",
+			outputs.len(),
+			highest_index,
+			last_retrieved_index,
+		);
+
+		result_vec.append(&mut identify_utxo_outputs(wallet, outputs.clone())?);
+
+		if highest_index == last_retrieved_index {
+			break;
+		}
+		start_index = last_retrieved_index + 1;
+	}
+	Ok(result_vec)
+}
+
+///
+fn restore_missing_output<T, C, K>(
+	wallet: &mut T,
+	output: OutputResult,
+	found_parents: &mut HashMap<Identifier, u32>,
+) -> Result<(), Error>
+where
+	T: WalletBackend<C, K>,
+	C: NodeClient,
+	K: Keychain,
+{
+	let mut batch = wallet.batch()?;
+
+	let parent_key_id = output.key_id.parent_path();
+	if !found_parents.contains_key(&parent_key_id) {
+		found_parents.insert(parent_key_id.clone(), 0);
+	}
+
+	let log_id = batch.next_tx_log_id(&parent_key_id)?;
+	let entry_type = match output.is_coinbase {
+		true => TxLogEntryType::ConfirmedCoinbase,
+		false => TxLogEntryType::TxReceived,
+	};
+
+	let mut t = TxLogEntry::new(parent_key_id.clone(), entry_type, log_id);
+	t.confirmed = true;
+	t.amount_credited = output.value;
+	t.num_outputs = 1;
+	t.update_confirmation_ts();
+	batch.save_tx_log_entry(t, &parent_key_id)?;
+
+	let _ = batch.save(OutputData {
+		root_key_id: parent_key_id.clone(),
+		key_id: output.key_id,
+		n_child: output.n_child,
+		value: output.value,
+		status: OutputStatus::Unspent,
+		height: output.height,
+		lock_height: output.lock_height,
+		is_coinbase: output.is_coinbase,
+		tx_log_entry: Some(log_id),
+	});
+
+	let max_child_index = found_parents.get(&parent_key_id).unwrap().clone();
+	if output.n_child >= max_child_index {
+		found_parents.insert(parent_key_id.clone(), output.n_child);
+	}
+
+	batch.commit()?;
+	Ok(())
+}
+
+///
+fn cancel_tx_log_entry<T, C, K>(wallet: &mut T, output: &OutputData) -> Result<(), Error>
+where
+	T: WalletBackend<C, K>,
+	C: NodeClient,
+	K: Keychain,
+{
+	let parent_key_id = output.key_id.parent_path();
+	let updated_tx_entry = if output.tx_log_entry.is_some() {
+		let entries = updater::retrieve_txs(
+			wallet,
+			output.tx_log_entry.clone(),
+			None,
+			Some(&parent_key_id),
+		)?;
+		if entries.len() > 0 {
+			let mut entry = entries[0].clone();
+			match entry.tx_type {
+				TxLogEntryType::TxSent => entry.tx_type = TxLogEntryType::TxSentCancelled,
+				TxLogEntryType::TxReceived => entry.tx_type = TxLogEntryType::TxReceivedCancelled,
+				_ => {}
+			}
+			Some(entry)
+		} else {
+			None
+		}
+	} else {
+		None
+	};
+	let mut batch = wallet.batch()?;
+	if let Some(t) = updated_tx_entry {
+		batch.save_tx_log_entry(t, &parent_key_id)?;
+	}
+	batch.commit()?;
+	Ok(())
+}
+
+/// Check / repair wallet contents
+/// assume wallet contents have been freshly updated with contents
+/// of latest block
+pub fn check_repair<T, C, K>(wallet: &mut T) -> Result<(), Error>
+where
+	T: WalletBackend<C, K>,
+	C: NodeClient,
+	K: Keychain,
+{
+	// First, get a definitive list of outputs we own from the chain
+	warn!("Starting wallet check.");
+	let chain_outs = collect_chain_outputs(wallet)?;
+	warn!(
+		"Identified {} wallet_outputs as belonging to this wallet",
+		chain_outs.len(),
+	);
+
+	// Now, get all outputs owned by this wallet (regardless of account)
+	let wallet_outputs = {
+		let res = updater::retrieve_outputs(&mut *wallet, true, None, None)?;
+		res
+	};
+
+	// check all definitive outputs exist in the wallet outputs
+	let mut missing_outs = vec![];
+	let mut accidental_spend_outs = vec![];
+	let mut locked_outs = vec![];
+	for deffo in chain_outs.into_iter() {
+		let matched_out = wallet_outputs.iter().find(|wo| wo.0.key_id == deffo.key_id);
+		match matched_out {
+			Some(s) => {
+				if s.0.status == OutputStatus::Spent {
+					accidental_spend_outs.push((s.0.clone(), deffo.clone()));
+				}
+				if s.0.status == OutputStatus::Locked {
+					locked_outs.push((s.0.clone(), deffo));
+				}
+			}
+			None => missing_outs.push(deffo),
+		}
+	}
+
+	// mark problem spent outputs as unspent (confirmed against a short-lived fork, for example)
+	for m in accidental_spend_outs.into_iter() {
+		let mut o = m.0;
+		warn!(
+			"Output for {} with ID {} ({:?}) marked as spent but exists in UTXO set. \
+			 Marking unspent and cancelling any associated transaction log entries.",
+			o.value, o.key_id, m.1.commit,
+		);
+		o.status = OutputStatus::Unspent;
+		// any transactions associated with this should be cancelled
+		cancel_tx_log_entry(wallet, &o)?;
+		let mut batch = wallet.batch()?;
+		batch.save(o)?;
+		batch.commit()?;
+	}
+
+	let mut found_parents: HashMap<Identifier, u32> = HashMap::new();
+
+	// Restore missing outputs, adding transaction for it back to the log
+	for m in missing_outs.into_iter() {
+		warn!(
+			"Confirmed output for {} with ID {} ({:?}) exists in UTXO set but not in wallet. \
+			 Restoring.",
+			m.value, m.key_id, m.commit,
+		);
+		restore_missing_output(wallet, m, &mut found_parents)?;
+	}
+
+	for m in locked_outs.into_iter() {
+		let mut o = m.0;
+		warn!(
+			"Confirmed output for {} with ID {} ({:?}) exists in UTXO set and is locked. \
+			 Unlocking and cancelling associated transaction log entries.",
+			o.value, o.key_id, m.1.commit,
+		);
+		o.status = OutputStatus::Unspent;
+		cancel_tx_log_entry(wallet, &o)?;
+		let mut batch = wallet.batch()?;
+		batch.save(o)?;
+		batch.commit()?;
+	}
+
+	Ok(())
+}
+
 /// Restore a wallet
 pub fn restore<T, C, K>(wallet: &mut T) -> Result<(), Error>
 where
@@ -111,78 +318,22 @@ where
 		return Ok(());
 	}
 
-	info!("Starting restore.");
+	warn!("Starting restore.");
 
-	let batch_size = 1000;
-	let mut start_index = 1;
-	let mut result_vec: Vec<OutputResult> = vec![];
-	loop {
-		let (highest_index, last_retrieved_index, outputs) = wallet
-			.w2n_client()
-			.get_outputs_by_pmmr_index(start_index, batch_size)?;
-		info!(
-			"Retrieved {} outputs, up to index {}. (Highest index: {})",
-			outputs.len(),
-			highest_index,
-			last_retrieved_index,
-		);
+	let result_vec = collect_chain_outputs(wallet)?;
 
-		result_vec.append(&mut identify_utxo_outputs(wallet, outputs.clone())?);
-
-		if highest_index == last_retrieved_index {
-			break;
-		}
-		start_index = last_retrieved_index + 1;
-	}
-
-	info!(
+	warn!(
 		"Identified {} wallet_outputs as belonging to this wallet",
 		result_vec.len(),
 	);
 
 	let mut found_parents: HashMap<Identifier, u32> = HashMap::new();
 	// Now save what we have
-	{
-		let mut batch = wallet.batch()?;
 
-		for output in result_vec {
-			let parent_key_id = output.key_id.parent_path();
-			if !found_parents.contains_key(&parent_key_id) {
-				found_parents.insert(parent_key_id.clone(), 0);
-			}
-
-			let log_id = batch.next_tx_log_id(&parent_key_id)?;
-			let entry_type = match output.is_coinbase {
-				true => TxLogEntryType::ConfirmedCoinbase,
-				false => TxLogEntryType::TxReceived,
-			};
-
-			let mut t = TxLogEntry::new(parent_key_id.clone(), entry_type, log_id);
-			t.confirmed = true;
-			t.amount_credited = output.value;
-			t.num_outputs = 1;
-			t.update_confirmation_ts();
-			batch.save_tx_log_entry(t, &parent_key_id)?;
-
-			let _ = batch.save(OutputData {
-				root_key_id: parent_key_id.clone(),
-				key_id: output.key_id,
-				n_child: output.n_child,
-				value: output.value,
-				status: OutputStatus::Unspent,
-				height: output.height,
-				lock_height: output.lock_height,
-				is_coinbase: output.is_coinbase,
-				tx_log_entry: Some(log_id),
-			});
-
-			let max_child_index = found_parents.get(&parent_key_id).unwrap().clone();
-			if output.n_child >= max_child_index {
-				found_parents.insert(parent_key_id.clone(), output.n_child);
-			};
-		}
-		batch.commit()?;
+	for output in result_vec {
+		restore_missing_output(wallet, output, &mut found_parents)?;
 	}
+
 	// restore labels, account paths and child derivation indices
 	let label_base = "account";
 	let mut index = 1;

wallet/src/libwallet/internal/selection.rs

@@ -14,7 +14,7 @@
 
 //! Selection of inputs for building transactions
 
-use crate::core::core::Transaction;
+use crate::core::core::{amount_to_hr_string, Transaction};
 use crate::core::libtx::{build, slate::Slate, tx_fee};
 use crate::keychain::{Identifier, Keychain};
 use crate::libwallet::error::{Error, ErrorKind};
@@ -266,7 +266,9 @@ where
 	if total == 0 {
 		return Err(ErrorKind::NotEnoughFunds {
 			available: 0,
+			available_disp: amount_to_hr_string(0, false),
 			needed: amount_with_fee as u64,
+			needed_disp: amount_to_hr_string(amount_with_fee as u64, false),
 		})?;
 	}
 
@@ -274,7 +276,9 @@ where
 	if total < amount_with_fee && coins.len() == max_outputs {
 		return Err(ErrorKind::NotEnoughFunds {
 			available: total,
+			available_disp: amount_to_hr_string(total, false),
 			needed: amount_with_fee as u64,
+			needed_disp: amount_to_hr_string(amount_with_fee as u64, false),
 		})?;
 	}
 
@@ -292,7 +296,9 @@ where
 			if coins.len() == max_outputs {
 				return Err(ErrorKind::NotEnoughFunds {
 					available: total as u64,
+					available_disp: amount_to_hr_string(total, false),
 					needed: amount_with_fee as u64,
+					needed_disp: amount_to_hr_string(amount_with_fee as u64, false),
 				})?;
 			}
 

wallet/src/libwallet/internal/tx.rs

@@ -173,7 +173,7 @@ where
 		return Err(ErrorKind::TransactionNotCancellable(tx_id_string))?;
 	}
 	// get outputs associated with tx
-	let res = updater::retrieve_outputs(wallet, false, Some(tx.id), &parent_key_id)?;
+	let res = updater::retrieve_outputs(wallet, false, Some(tx.id), Some(&parent_key_id))?;
 	let outputs = res.iter().map(|(out, _)| out).cloned().collect();
 	updater::cancel_tx_and_outputs(wallet, tx, outputs, parent_key_id)?;
 	Ok(())

wallet/src/libwallet/internal/updater.rs

@@ -39,7 +39,7 @@ pub fn retrieve_outputs<T: ?Sized, C, K>(
 	wallet: &mut T,
 	show_spent: bool,
 	tx_id: Option<u32>,
-	parent_key_id: &Identifier,
+	parent_key_id: Option<&Identifier>,
 ) -> Result<Vec<(OutputData, pedersen::Commitment)>, Error>
 where
 	T: WalletBackend<C, K>,
@@ -49,7 +49,6 @@ where
 	// just read the wallet here, no need for a write lock
 	let mut outputs = wallet
 		.iter()
-		.filter(|out| out.root_key_id == *parent_key_id)
 		.filter(|out| {
 			if show_spent {
 				true
@@ -63,10 +62,18 @@ where
 	if let Some(id) = tx_id {
 		outputs = outputs
 			.into_iter()
-			.filter(|out| out.tx_log_entry == Some(id) && out.root_key_id == *parent_key_id)
+			.filter(|out| out.tx_log_entry == Some(id))
 			.collect::<Vec<_>>();
 	}
 
+	if let Some(k) = parent_key_id {
+		outputs = outputs
+			.iter()
+			.filter(|o| o.root_key_id == *k)
+			.map(|o| o.clone())
+			.collect();
+	}
+
 	outputs.sort_by_key(|out| out.n_child);
 
 	let res = outputs

wallet/src/libwallet/types.rs

@@ -118,6 +118,9 @@ where
 
 	/// Attempt to restore the contents of a wallet from seed
 	fn restore(&mut self) -> Result<(), Error>;
+
+	/// Attempt to check and fix wallet state
+	fn check_repair(&mut self) -> Result<(), Error>;
 }
 
 /// Batch trait to update the output data backend atomically. Trying to use a
@@ -562,7 +565,7 @@ impl fmt::Display for TxLogEntryType {
 			TxLogEntryType::TxReceived => write!(f, "Received Tx"),
 			TxLogEntryType::TxSent => write!(f, "Sent Tx"),
 			TxLogEntryType::TxReceivedCancelled => write!(f, "Received Tx\n- Cancelled"),
-			TxLogEntryType::TxSentCancelled => write!(f, "Send Tx\n- Cancelled"),
+			TxLogEntryType::TxSentCancelled => write!(f, "Sent Tx\n- Cancelled"),
 		}
 	}
 }
@@ -637,6 +640,14 @@ impl TxLogEntry {
 		}
 	}
 
+	/// Given a vec of TX log entries, return credited + debited sums
+	pub fn sum_confirmed(txs: &Vec<TxLogEntry>) -> (u64, u64) {
+		txs.iter().fold((0, 0), |acc, tx| match tx.confirmed {
+			true => (acc.0 + tx.amount_credited, acc.1 + tx.amount_debited),
+			false => acc,
+		})
+	}
+
 	/// Update confirmation TS with now
 	pub fn update_confirmation_ts(&mut self) {
 		self.confirmation_ts = Some(Utc::now());

wallet/src/lmdb_wallet.rs

@@ -369,6 +369,11 @@ where
 		internal::restore::restore(self).context(ErrorKind::Restore)?;
 		Ok(())
 	}
+
+	fn check_repair(&mut self) -> Result<(), Error> {
+		internal::restore::check_repair(self).context(ErrorKind::Restore)?;
+		Ok(())
+	}
 }
 
 /// An atomic batch in which all changes can be committed all at once or