Wallet - only update outputs with unconfirmed transactions outstanding (#2322)

* Only update outputs with unconfirmed transactions outstanding

* add further check for Unspent output that have been spent to check_repair

* rename  to

* make updating all outputs optional API parameter. do full update before a wallet check, remove unspent->spent check from check process

* rustfmt

* typo

servers/tests/framework.rs

@@ -349,7 +349,8 @@ impl LocalServerContainer {
 			.unwrap_or_else(|e| panic!("Error creating wallet: {:?} Config: {:?}", e, config));
 		wallet.keychain = Some(keychain);
 		let parent_id = keychain::ExtKeychain::derive_key_id(2, 0, 0, 0, 0);
-		let _ = wallet::libwallet::internal::updater::refresh_outputs(&mut wallet, &parent_id);
+		let _ =
+			wallet::libwallet::internal::updater::refresh_outputs(&mut wallet, &parent_id, false);
 		wallet::libwallet::internal::updater::retrieve_info(&mut wallet, &parent_id, 1).unwrap()
 	}
 

src/bin/cmd/wallet_args.rs

@@ -587,7 +587,7 @@ pub fn wallet_command(
 			command::cancel(inst_wallet(), a)
 		}
 		("restore", Some(_)) => command::restore(inst_wallet()),
-		("check_repair", Some(_)) => command::check_repair(inst_wallet()),
+		("check", Some(_)) => command::check_repair(inst_wallet()),
 		_ => {
 			let msg = format!("Unknown wallet command, use 'grin help wallet' for details");
 			return Err(ErrorKind::ArgumentError(msg).into());

src/bin/cmd/wallet_tests.rs

@@ -462,7 +462,7 @@ mod wallet_tests {
 		];
 		execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
 
-		let arg_vec = vec!["grin", "wallet", "-p", "password", "check_repair"];
+		let arg_vec = vec!["grin", "wallet", "-p", "password", "check"];
 		execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
 
 		// txs and outputs (mostly spit out for a visual in test logs)

src/bin/grin.yml

@@ -294,6 +294,6 @@ subcommands:
                   takes_value: false
         - restore:
             about: Restores a wallet contents from a seed file
-        - check_repair:
+        - check:
             about: Checks a wallet's outputs against a live node, repairing and restoring missing outputs if required
  

wallet/src/command.rs

@@ -501,14 +501,16 @@ pub fn check_repair(
 	wallet: Arc<Mutex<WalletInst<impl NodeClient + 'static, keychain::ExtKeychain>>>,
 ) -> Result<(), Error> {
 	controller::owner_single_use(wallet.clone(), |api| {
+		warn!("Starting wallet check...",);
+		warn!("Updating all wallet outputs, please wait ...",);
 		let result = api.check_repair();
 		match result {
 			Ok(_) => {
-				warn!("Wallet check/repair complete",);
+				warn!("Wallet check complete",);
 				Ok(())
 			}
 			Err(e) => {
-				error!("Wallet check/repair failed: {}", e);
+				error!("Wallet check failed: {}", e);
 				error!("Backtrace: {}", e.backtrace().unwrap());
 				Err(e)
 			}

wallet/src/libwallet/api.rs

@@ -348,7 +348,7 @@ where
 
 		let mut validated = false;
 		if refresh_from_node {
-			validated = self.update_outputs(&mut w);
+			validated = self.update_outputs(&mut w, false);
 		}
 
 		let res = Ok((
@@ -426,7 +426,7 @@ where
 
 		let mut validated = false;
 		if refresh_from_node {
-			validated = self.update_outputs(&mut w);
+			validated = self.update_outputs(&mut w, false);
 		}
 
 		let res = Ok((
@@ -498,7 +498,7 @@ where
 
 		let mut validated = false;
 		if refresh_from_node {
-			validated = self.update_outputs(&mut w);
+			validated = self.update_outputs(&mut w, false);
 		}
 
 		let wallet_info = updater::retrieve_info(&mut *w, &parent_key_id, minimum_confirmations)?;
@@ -708,7 +708,7 @@ where
 		let mut w = self.wallet.lock();
 		w.open_with_credentials()?;
 		let parent_key_id = w.parent_key_id();
-		if !self.update_outputs(&mut w) {
+		if !self.update_outputs(&mut w, false) {
 			return Err(ErrorKind::TransactionCancellationError(
 				"Can't contact running Grin node. Not Cancelling.",
 			))?;
@@ -765,7 +765,7 @@ where
 	pub fn check_repair(&mut self) -> Result<(), Error> {
 		let mut w = self.wallet.lock();
 		w.open_with_credentials()?;
-		self.update_outputs(&mut w);
+		self.update_outputs(&mut w, true);
 		w.check_repair()?;
 		w.close()?;
 		Ok(())
@@ -792,9 +792,9 @@ where
 	}
 
 	/// Attempt to update outputs in wallet, return whether it was successful
-	fn update_outputs(&self, w: &mut W) -> bool {
+	fn update_outputs(&self, w: &mut W, update_all: bool) -> bool {
 		let parent_key_id = w.parent_key_id();
-		match updater::refresh_outputs(&mut *w, &parent_key_id) {
+		match updater::refresh_outputs(&mut *w, &parent_key_id, update_all) {
 			Ok(_) => true,
 			Err(_) => false,
 		}

wallet/src/libwallet/internal/restore.rs

@@ -240,10 +240,11 @@ where
 		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![];
+
+	// check all definitive outputs exist in the wallet outputs
 	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 {

wallet/src/libwallet/internal/tx.rs

@@ -85,7 +85,7 @@ where
 	// Get lock height
 	let current_height = wallet.w2n_client().get_chain_height()?;
 	// ensure outputs we're selecting are up to date
-	updater::refresh_outputs(wallet, parent_key_id)?;
+	updater::refresh_outputs(wallet, parent_key_id, false)?;
 
 	let lock_height = current_height;
 

wallet/src/libwallet/internal/updater.rs

@@ -120,6 +120,7 @@ where
 pub fn refresh_outputs<T: ?Sized, C, K>(
 	wallet: &mut T,
 	parent_key_id: &Identifier,
+	update_all: bool,
 ) -> Result<(), Error>
 where
 	T: WalletBackend<C, K>,
@@ -127,7 +128,7 @@ where
 	K: Keychain,
 {
 	let height = wallet.w2n_client().get_chain_height()?;
-	refresh_output_state(wallet, height, parent_key_id)?;
+	refresh_output_state(wallet, height, parent_key_id, update_all)?;
 	Ok(())
 }
 
@@ -136,6 +137,7 @@ where
 pub fn map_wallet_outputs<T: ?Sized, C, K>(
 	wallet: &mut T,
 	parent_key_id: &Identifier,
+	update_all: bool,
 ) -> Result<HashMap<pedersen::Commitment, Identifier>, Error>
 where
 	T: WalletBackend<C, K>,
@@ -144,9 +146,33 @@ where
 {
 	let mut wallet_outputs: HashMap<pedersen::Commitment, Identifier> = HashMap::new();
 	let keychain = wallet.keychain().clone();
-	let unspents = wallet
+	let unspents: Vec<OutputData> = wallet
 		.iter()
-		.filter(|x| x.root_key_id == *parent_key_id && x.status != OutputStatus::Spent);
+		.filter(|x| x.root_key_id == *parent_key_id && x.status != OutputStatus::Spent)
+		.collect();
+
+	// Only select outputs that are actually involved in an outstanding transaction
+	let unspents: Vec<OutputData> = match update_all {
+		false => unspents
+			.into_iter()
+			.filter(|x| match x.tx_log_entry.as_ref() {
+				Some(t) => {
+					let entries = retrieve_txs(wallet, Some(*t), None, Some(&parent_key_id));
+					match entries {
+						Err(_) => true,
+						Ok(e) => {
+							e.len() > 0
+								&& !e[0].confirmed && (e[0].tx_type == TxLogEntryType::TxReceived
+								|| e[0].tx_type == TxLogEntryType::TxSent)
+						}
+					}
+				}
+				None => true,
+			})
+			.collect(),
+		true => unspents,
+	};
+
 	for out in unspents {
 		let commit = keychain.commit(out.value, &out.key_id)?;
 		wallet_outputs.insert(commit, out.key_id.clone());
@@ -275,6 +301,7 @@ fn refresh_output_state<T: ?Sized, C, K>(
 	wallet: &mut T,
 	height: u64,
 	parent_key_id: &Identifier,
+	update_all: bool,
 ) -> Result<(), Error>
 where
 	T: WalletBackend<C, K>,
@@ -285,7 +312,7 @@ where
 
 	// build a local map of wallet outputs keyed by commit
 	// and a list of outputs we want to query the node for
-	let wallet_outputs = map_wallet_outputs(wallet, parent_key_id)?;
+	let wallet_outputs = map_wallet_outputs(wallet, parent_key_id, update_all)?;
 
 	let wallet_output_keys = wallet_outputs.keys().map(|commit| commit.clone()).collect();