'Awaiting finalization' status and changes to 'wallet check' (mimblew…

…imble#8)

* add awaiting confirmation status and change check to not remove unconfirmed transactions by default

* test fixes

apiwallet/src/api.rs

@@ -842,11 +842,11 @@ where
 	}
 
 	/// Attempt to check and fix the contents of the wallet
-	pub fn check_repair(&mut self) -> Result<(), Error> {
+	pub fn check_repair(&mut self, delete_unconfirmed: bool) -> Result<(), Error> {
 		let mut w = self.wallet.lock();
 		w.open_with_credentials()?;
 		self.update_outputs(&mut w, true);
-		w.check_repair()?;
+		w.check_repair(delete_unconfirmed)?;
 		w.close()?;
 		Ok(())
 	}

libwallet/src/internal/restore.rs

@@ -267,7 +267,7 @@ where
 /// 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>
+pub fn check_repair<T, C, K>(wallet: &mut T, delete_unconfirmed: bool) -> Result<(), Error>
 where
 	T: WalletBackend<C, K>,
 	C: NodeClient,
@@ -335,37 +335,39 @@ where
 		restore_missing_output(wallet, m, &mut found_parents, &mut None)?;
 	}
 
-	// Unlock locked outputs
-	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()?;
-	}
+	if delete_unconfirmed {
+		// Unlock locked outputs
+		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()?;
+		}
 
-	let unconfirmed_outs: Vec<&(OutputData, pedersen::Commitment)> = wallet_outputs
-		.iter()
-		.filter(|o| o.0.status == OutputStatus::Unconfirmed)
-		.collect();
-	// Delete unconfirmed outputs
-	for m in unconfirmed_outs.into_iter() {
-		let o = m.0.clone();
-		warn!(
-			"Unconfirmed output for {} with ID {} ({:?}) not in UTXO set. \
-			 Deleting and cancelling associated transaction log entries.",
-			o.value, o.key_id, m.1,
-		);
-		cancel_tx_log_entry(wallet, &o)?;
-		let mut batch = wallet.batch()?;
-		batch.delete(&o.key_id, &o.mmr_index)?;
-		batch.commit()?;
+		let unconfirmed_outs: Vec<&(OutputData, pedersen::Commitment)> = wallet_outputs
+			.iter()
+			.filter(|o| o.0.status == OutputStatus::Unconfirmed)
+			.collect();
+		// Delete unconfirmed outputs
+		for m in unconfirmed_outs.into_iter() {
+			let o = m.0.clone();
+			warn!(
+				"Unconfirmed output for {} with ID {} ({:?}) not in UTXO set. \
+				 Deleting and cancelling associated transaction log entries.",
+				o.value, o.key_id, m.1,
+			);
+			cancel_tx_log_entry(wallet, &o)?;
+			let mut batch = wallet.batch()?;
+			batch.delete(&o.key_id, &o.mmr_index)?;
+			batch.commit()?;
+		}
 	}
 
 	// restore labels, account paths and child derivation indices

libwallet/src/internal/updater.rs

@@ -384,6 +384,7 @@ where
 
 	let mut unspent_total = 0;
 	let mut immature_total = 0;
+	let mut awaiting_finalization_total = 0;
 	let mut unconfirmed_total = 0;
 	let mut locked_total = 0;
 
@@ -403,9 +404,9 @@ where
 				// We ignore unconfirmed coinbase outputs completely.
 				if !out.is_coinbase {
 					if minimum_confirmations == 0 {
-						unspent_total += out.value;
-					} else {
 						unconfirmed_total += out.value;
+					} else {
+						awaiting_finalization_total += out.value;
 					}
 				}
 			}
@@ -420,6 +421,7 @@ where
 		last_confirmed_height: current_height,
 		minimum_confirmations,
 		total: unspent_total + unconfirmed_total + immature_total,
+		amount_awaiting_finalization: awaiting_finalization_total,
 		amount_awaiting_confirmation: unconfirmed_total,
 		amount_immature: immature_total,
 		amount_locked: locked_total,

libwallet/src/types.rs

@@ -130,7 +130,7 @@ where
 	fn restore(&mut self) -> Result<(), Error>;
 
 	/// Attempt to check and fix wallet state
-	fn check_repair(&mut self) -> Result<(), Error>;
+	fn check_repair(&mut self, delete_unconfirmed: bool) -> Result<(), Error>;
 }
 
 /// Batch trait to update the output data backend atomically. Trying to use a
@@ -546,6 +546,8 @@ pub struct WalletInfo {
 	pub minimum_confirmations: u64,
 	/// total amount in the wallet
 	pub total: u64,
+	/// amount awaiting finalization
+	pub amount_awaiting_finalization: u64,
 	/// amount awaiting confirmation
 	pub amount_awaiting_confirmation: u64,
 	/// coinbases waiting for lock height

refwallet/src/command.rs

@@ -530,13 +530,19 @@ pub fn restore(
 	Ok(())
 }
 
+/// wallet check
+pub struct CheckArgs {
+	pub delete_unconfirmed: bool,
+}
+
 pub fn check_repair(
 	wallet: Arc<Mutex<WalletInst<impl NodeClient + 'static, keychain::ExtKeychain>>>,
+	args: CheckArgs,
 ) -> 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();
+		let result = api.check_repair(args.delete_unconfirmed);
 		match result {
 			Ok(_) => {
 				warn!("Wallet check complete",);

refwallet/src/display.rs

@@ -270,7 +270,7 @@ pub fn info(
 
 	if dark_background_color_scheme {
 		table.add_row(row![
-			bFG->"Total",
+			bFG->"Confirmed Total",
 			FG->amount_to_hr_string(wallet_info.total, false)
 		]);
 		// Only dispay "Immature Coinbase" if we have related outputs in the wallet.
@@ -285,6 +285,10 @@ pub fn info(
 			bFY->format!("Awaiting Confirmation (< {})", wallet_info.minimum_confirmations),
 			FY->amount_to_hr_string(wallet_info.amount_awaiting_confirmation, false)
 		]);
+		table.add_row(row![
+			bFB->format!("Awaiting Finalization"),
+			FB->amount_to_hr_string(wallet_info.amount_awaiting_finalization, false)
+		]);
 		table.add_row(row![
 			Fr->"Locked by previous transaction",
 			Fr->amount_to_hr_string(wallet_info.amount_locked, false)

refwallet/src/lmdb_wallet.rs

@@ -350,8 +350,8 @@ where
 		Ok(())
 	}
 
-	fn check_repair(&mut self) -> Result<(), Error> {
-		internal::restore::check_repair(self).context(ErrorKind::Restore)?;
+	fn check_repair(&mut self, delete_unconfirmed: bool) -> Result<(), Error> {
+		internal::restore::check_repair(self, delete_unconfirmed).context(ErrorKind::Restore)?;
 		Ok(())
 	}
 }

refwallet/tests/check.rs

@@ -140,7 +140,7 @@ fn check_repair_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 
 	// this should restore our missing outputs
 	wallet::controller::owner_single_use(wallet1.clone(), |api| {
-		api.check_repair()?;
+		api.check_repair(true)?;
 		Ok(())
 	})?;
 
@@ -181,7 +181,7 @@ fn check_repair_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 
 	// unlock/restore
 	wallet::controller::owner_single_use(wallet1.clone(), |api| {
-		api.check_repair()?;
+		api.check_repair(true)?;
 		Ok(())
 	})?;
 
@@ -327,7 +327,7 @@ fn two_wallets_one_seed_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 
 	// 0) Check repair when all is okay should leave wallet contents alone
 	wallet::controller::owner_single_use(wallet1.clone(), |api| {
-		api.check_repair()?;
+		api.check_repair(true)?;
 		let info = test_framework::wallet_info(api)?;
 		assert_eq!(info.amount_currently_spendable, base_amount * 6);
 		assert_eq!(info.total, base_amount * 6);
@@ -381,7 +381,7 @@ fn two_wallets_one_seed_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 
 	// 2) check_repair should recover them into a single wallet
 	wallet::controller::owner_single_use(wallet1.clone(), |api| {
-		api.check_repair()?;
+		api.check_repair(true)?;
 		Ok(())
 	})?;
 
@@ -447,7 +447,7 @@ fn two_wallets_one_seed_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 	})?;
 
 	wallet::controller::owner_single_use(wallet6.clone(), |api| {
-		api.check_repair()?;
+		api.check_repair(true)?;
 		Ok(())
 	})?;
 
@@ -538,7 +538,7 @@ fn two_wallets_one_seed_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 		let outputs = api.retrieve_outputs(true, false, None)?.1;
 		assert_eq!(outputs.len(), 3);
 		assert_eq!(info.amount_currently_spendable, base_amount * 15);
-		api.check_repair()?;
+		api.check_repair(true)?;
 		let info = test_framework::wallet_info(api)?;
 		let outputs = api.retrieve_outputs(true, false, None)?.1;
 		assert_eq!(outputs.len(), 6);
@@ -556,7 +556,7 @@ fn two_wallets_one_seed_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 
 	// 7) Ensure check_repair creates missing accounts
 	wallet::controller::owner_single_use(wallet10.clone(), |api| {
-		api.check_repair()?;
+		api.check_repair(true)?;
 		api.set_active_account("account_1")?;
 		let info = test_framework::wallet_info(api)?;
 		let outputs = api.retrieve_outputs(true, false, None)?.1;

refwallet/tests/transaction.rs

@@ -420,11 +420,12 @@ fn tx_rollback(test_dir: &str) -> Result<(), libwallet::Error> {
 		let (refreshed, wallet2_info) = api.retrieve_summary_info(true, 1)?;
 		assert!(refreshed);
 		assert_eq!(wallet2_info.amount_currently_spendable, 0,);
-		assert_eq!(wallet2_info.total, amount);
+		assert_eq!(wallet2_info.amount_awaiting_finalization, amount);
 		Ok(())
 	})?;
 
-	// wallet 1 is bold and doesn't ever post the transaction mine a few more blocks
+	// wallet 1 is bold and doesn't ever post the transaction 
+	// mine a few more blocks
 	let _ = test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), 5);
 
 	// Wallet 1 decides to roll back instead

src/bin/cmd/wallet_args.rs

@@ -452,6 +452,13 @@ pub fn parse_info_args(args: &ArgMatches) -> Result<command::InfoArgs, ParseErro
 	})
 }
 
+pub fn parse_check_args(args: &ArgMatches) -> Result<command::CheckArgs, ParseError> {
+	let delete_unconfirmed = args.is_present("delete_unconfirmed");
+	Ok(command::CheckArgs {
+		delete_unconfirmed: delete_unconfirmed,
+	})
+}
+
 pub fn parse_txs_args(args: &ArgMatches) -> Result<command::TxsArgs, ParseError> {
 	let tx_id = match args.value_of("id") {
 		None => None,
@@ -621,7 +628,10 @@ pub fn wallet_command(
 			command::cancel(inst_wallet(), a)
 		}
 		("restore", Some(_)) => command::restore(inst_wallet()),
-		("check", Some(_)) => command::check_repair(inst_wallet()),
+		("check", Some(args)) => {
+			let a = arg_parse!(parse_check_args(&args));
+			command::check_repair(inst_wallet(), a)
+		}
 		_ => {
 			let msg = format!("Unknown wallet command, use 'grin help wallet' for details");
 			return Err(ErrorKind::ArgumentError(msg).into());

src/bin/cmd/wallet_tests.rs

@@ -456,7 +456,7 @@ mod wallet_tests {
 		];
 		execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
 
-		let arg_vec = vec!["grin-wallet", "-p", "password", "check"];
+		let arg_vec = vec!["grin-wallet", "-p", "password", "check", "-d"];
 		execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?;
 
 		// Another file exchange, cancel this time

src/bin/grin-wallet.yml

@@ -236,3 +236,9 @@ subcommands:
       about: Restores a wallet contents from a seed file
   - check:
       about: Checks a wallet's outputs against a live node, repairing and restoring missing outputs if required
+      args:
+        - delete_unconfirmed:
+            help: Delete any unconfirmed outputsm unlock any locked outputs and delete associated transactions while doing the check.
+            short: d
+            long: delete_unconfirmed
+            takes_value: false