Add cancel_tx and retrieve by tx_slate_id in owner_api (#1963)

* Add retrieve by tx_id in owner_api
* Add cancel tx by tx_slate_id
* Add doc

doc/api/wallet_owner_api.md

@@ -189,8 +189,9 @@ Return whether the outputs were validated against a node and an array of TxLogEn
 
 * **URL**
 
-  */v1/wallet/owner/retrieve_txs
+  * /v1/wallet/owner/retrieve_txs
-  */v1/wallet/owner/retrieve_txs?refresh?id=x
+  * /v1/wallet/owner/retrieve_txs?refresh&id=x
+  * /v1/wallet/owner/retrieve_txs?tx_id=x
 
 * **Method:**
 
@@ -200,8 +201,9 @@ Return whether the outputs were validated against a node and an array of TxLogEn
 
   **Optional:**
 
-  `refresh` to refresh from node
+  * `refresh` to refresh from node
-  `tx_id=[number]` to retrieve only the specified output
+  * `id=[number]` to retrieve only the specified output by id
+  * `tx_id=[string]` to retrieve only the specified output by tx id
 
 * **Data Params**
 
@@ -521,7 +523,8 @@ Roll back a transaction and all associated outputs with a given transaction id T
 
 * **URL**
 
-  /v1/wallet/owner/cancel_tx?id=x
+  * /v1/wallet/owner/cancel_tx?id=x
+  * /v1/wallet/owner/cancel_tx?tx_id=x
 
 * **Method:**
 
@@ -530,7 +533,8 @@ Roll back a transaction and all associated outputs with a given transaction id T
 * **URL Params**
 
   **Required:**
-  `id=[number]`
+  * `id=[number]` the transaction id
+  * `tx_id=[string]`the transaction slate id
 
 * **Data Params**
 

src/bin/cmd/wallet.rs

@@ -532,7 +532,7 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) -> i
 					},
 				};
 				let (height, _) = api.node_height()?;
-				let (validated, txs) = api.retrieve_txs(true, tx_id)?;
+				let (validated, txs) = api.retrieve_txs(true, tx_id, None)?;
 				let include_status = !tx_id.is_some();
 				display::txs(
 					account,
@@ -611,22 +611,49 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) -> i
 				}
 			}
 			("cancel", Some(tx_args)) => {
-				let tx_id = tx_args
+				let mut tx_id_string = "";
-					.value_of("id")
+				let tx_id = match tx_args.value_of("id") {
-					.ok_or_else(|| {
+					None => None,
-						ErrorKind::GenericError("'id' argument (-i) is required.".to_string())
+					Some(tx) => match tx.parse() {
-					}).and_then(|v| {
+						Ok(t) => {
-						v.parse().map_err(|e| {
+							tx_id_string = tx;
-							ErrorKind::GenericError(format!(
+							Some(t)
+						}
+						Err(e) => {
+							return Err(ErrorKind::GenericError(format!(
 								"Could not parse id parameter. e={:?}",
-								e
+								e,
-							))
+							)).into());
-						})
+						}
-					})?;
+					},
-				let result = api.cancel_tx(tx_id);
+				};
+				let tx_slate_id = match tx_args.value_of("txid") {
+					None => None,
+					Some(tx) => match tx.parse() {
+						Ok(t) => {
+							tx_id_string = tx;
+							Some(t)
+						}
+						Err(e) => {
+							return Err(ErrorKind::GenericError(format!(
+								"Could not parse txid parameter. e={:?}",
+								e,
+							)).into());
+						}
+					},
+				};
+				if (tx_id.is_none() && tx_slate_id.is_none())
+					|| (tx_id.is_some() && tx_slate_id.is_some())
+				{
+					return Err(ErrorKind::GenericError(format!(
+						"'id' (-i) or 'txid' (-t) argument is required."
+					)).into());
+				}
+
+				let result = api.cancel_tx(tx_id, tx_slate_id);
 				match result {
 					Ok(_) => {
-						info!("Transaction {} Cancelled", tx_id);
+						info!("Transaction {} Cancelled", tx_id_string);
 						Ok(())
 					}
 					Err(e) => {

src/bin/grin.rs

@@ -321,6 +321,11 @@ fn real_main() -> i32 {
 				.help("The ID of the transaction to cancel")
 				.short("i")
 				.long("id")
+				.takes_value(true))
+			.arg(Arg::with_name("txid")
+				.help("The TxID of the transaction to cancel")
+				.short("t")
+				.long("txid")
 				.takes_value(true)))
 
 		.subcommand(SubCommand::with_name("info")

wallet/src/libwallet/api.rs

@@ -22,6 +22,7 @@ use std::io::{Read, Write};
 use std::marker::PhantomData;
 use std::sync::Arc;
 use util::Mutex;
+use uuid::Uuid;
 
 use serde_json as json;
 
@@ -106,6 +107,7 @@ where
 		&self,
 		refresh_from_node: bool,
 		tx_id: Option<u32>,
+		tx_slate_id: Option<Uuid>,
 	) -> Result<(bool, Vec<TxLogEntry>), Error> {
 		let mut w = self.wallet.lock();
 		w.open_with_credentials()?;
@@ -118,7 +120,7 @@ where
 
 		let res = Ok((
 			validated,
-			updater::retrieve_txs(&mut *w, tx_id, &parent_key_id)?,
+			updater::retrieve_txs(&mut *w, tx_id, tx_slate_id, &parent_key_id)?,
 		));
 
 		w.close()?;
@@ -324,7 +326,11 @@ where
 	/// output if you're recipient), and unlock all locked outputs associated
 	/// with the transaction used when a transaction is created but never
 	/// posted
-	pub fn cancel_tx(&mut self, tx_id: u32) -> Result<(), Error> {
+	pub fn cancel_tx(
+		&mut self,
+		tx_id: Option<u32>,
+		tx_slate_id: Option<Uuid>,
+	) -> Result<(), Error> {
 		let mut w = self.wallet.lock();
 		w.open_with_credentials()?;
 		let parent_key_id = w.parent_key_id();
@@ -333,7 +339,7 @@ where
 				"Can't contact running Grin node. Not Cancelling.",
 			))?;
 		}
-		tx::cancel_tx(&mut *w, &parent_key_id, tx_id)?;
+		tx::cancel_tx(&mut *w, &parent_key_id, tx_id, tx_slate_id)?;
 		w.close()?;
 		Ok(())
 	}

wallet/src/libwallet/controller.rs

@@ -207,7 +207,8 @@ where
 		req: &Request<Body>,
 		api: APIOwner<T, C, L, K>,
 	) -> Result<(bool, Vec<TxLogEntry>), Error> {
-		let mut id = None;
+		let mut tx_id = None;
+		let mut tx_slate_id = None;
 		let mut update_from_node = false;
 
 		let params = parse_params(req);
@@ -217,10 +218,15 @@ where
 		}
 		if let Some(ids) = params.get("id") {
 			for i in ids {
-				id = Some(i.parse().unwrap());
+				tx_id = Some(i.parse().unwrap());
 			}
 		}
-		api.retrieve_txs(update_from_node, id)
+		if let Some(tx_slate_ids) = params.get("tx_id") {
+			for i in tx_slate_ids {
+				tx_slate_id = Some(i.parse().unwrap());
+			}
+		}
+		api.retrieve_txs(update_from_node, tx_id, tx_slate_id)
 	}
 
 	fn dump_stored_tx(
@@ -345,7 +351,7 @@ where
 		let params = parse_params(&req);
 		if let Some(id_string) = params.get("id") {
 			Box::new(match id_string[0].parse() {
-				Ok(id) => match api.cancel_tx(id) {
+				Ok(id) => match api.cancel_tx(Some(id), None) {
 					Ok(_) => ok(()),
 					Err(e) => {
 						error!("cancel_tx: failed with error: {}", e);
@@ -359,9 +365,25 @@ where
 					).into())
 				}
 			})
+		} else if let Some(tx_id_string) = params.get("tx_id") {
+			Box::new(match tx_id_string[0].parse() {
+				Ok(tx_id) => match api.cancel_tx(None, Some(tx_id)) {
+					Ok(_) => ok(()),
+					Err(e) => {
+						error!("cancel_tx: failed with error: {}", e);
+						err(e)
+					}
+				},
+				Err(e) => {
+					error!("cancel_tx: could not parse tx_id: {}", e);
+					err(ErrorKind::TransactionCancellationError(
+						"cancel_tx: cannot cancel transaction. Could not parse tx_id in request.",
+					).into())
+				}
+			})
 		} else {
 			Box::new(err(ErrorKind::TransactionCancellationError(
-				"cancel_tx: Cannot cancel transaction. Missing id param in request.",
+				"cancel_tx: Cannot cancel transaction. Missing id or tx_id param in request.",
 			).into()))
 		}
 	}

wallet/src/libwallet/error.rs

@@ -150,11 +150,11 @@ pub enum ErrorKind {
 
 	/// Transaction doesn't exist
 	#[fail(display = "Transaction {} doesn't exist", _0)]
-	TransactionDoesntExist(u32),
+	TransactionDoesntExist(String),
 
 	/// Transaction already rolled back
 	#[fail(display = "Transaction {} cannot be cancelled", _0)]
-	TransactionNotCancellable(u32),
+	TransactionNotCancellable(String),
 
 	/// Cancellation error
 	#[fail(display = "Cancellation Error: {}", _0)]

wallet/src/libwallet/internal/tx.rs

@@ -16,6 +16,7 @@
 
 use std::sync::Arc;
 use util::RwLock;
+use uuid::Uuid;
 
 use core::core::verifier_cache::LruVerifierCache;
 use core::core::Transaction;
@@ -158,7 +159,8 @@ where
 pub fn cancel_tx<T: ?Sized, C, L, K>(
 	wallet: &mut T,
 	parent_key_id: &Identifier,
-	tx_id: u32,
+	tx_id: Option<u32>,
+	tx_slate_id: Option<Uuid>,
 ) -> Result<(), Error>
 where
 	T: WalletBackend<C, L, K>,
@@ -166,19 +168,25 @@ where
 	L: WalletToWalletClient,
 	K: Keychain,
 {
-	let tx_vec = updater::retrieve_txs(wallet, Some(tx_id), &parent_key_id)?;
+	let mut tx_id_string = String::new();
+	if let Some(tx_id) = tx_id {
+		tx_id_string = tx_id.to_string();
+	} else if let Some(tx_slate_id) = tx_slate_id {
+		tx_id_string = tx_slate_id.to_string();
+	}
+	let tx_vec = updater::retrieve_txs(wallet, tx_id, tx_slate_id, &parent_key_id)?;
 	if tx_vec.len() != 1 {
-		return Err(ErrorKind::TransactionDoesntExist(tx_id))?;
+		return Err(ErrorKind::TransactionDoesntExist(tx_id_string))?;
 	}
 	let tx = tx_vec[0].clone();
 	if tx.tx_type != TxLogEntryType::TxSent && tx.tx_type != TxLogEntryType::TxReceived {
-		return Err(ErrorKind::TransactionNotCancellable(tx_id))?;
+		return Err(ErrorKind::TransactionNotCancellable(tx_id_string))?;
 	}
 	if tx.confirmed == true {
-		return Err(ErrorKind::TransactionNotCancellable(tx_id))?;
+		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), &parent_key_id)?;
 	let outputs = res.iter().map(|(out, _)| out).cloned().collect();
 	updater::cancel_tx_and_outputs(wallet, tx, outputs, parent_key_id)?;
 	Ok(())
@@ -197,9 +205,9 @@ where
 	L: WalletToWalletClient,
 	K: Keychain,
 {
-	let tx_vec = updater::retrieve_txs(wallet, Some(tx_id), parent_key_id)?;
+	let tx_vec = updater::retrieve_txs(wallet, Some(tx_id), None, parent_key_id)?;
 	if tx_vec.len() != 1 {
-		return Err(ErrorKind::TransactionDoesntExist(tx_id))?;
+		return Err(ErrorKind::TransactionDoesntExist(tx_id.to_string()))?;
 	}
 	let tx = tx_vec[0].clone();
 	Ok((tx.confirmed, tx.tx_hex))

wallet/src/libwallet/internal/updater.rs

@@ -17,6 +17,7 @@
 
 use failure::ResultExt;
 use std::collections::HashMap;
+use uuid::Uuid;
 
 use core::consensus::reward;
 use core::core::{Output, TxKernel};
@@ -81,6 +82,7 @@ where
 pub fn retrieve_txs<T: ?Sized, C, L, K>(
 	wallet: &mut T,
 	tx_id: Option<u32>,
+	tx_slate_id: Option<Uuid>,
 	parent_key_id: &Identifier,
 ) -> Result<Vec<TxLogEntry>, Error>
 where
@@ -97,6 +99,13 @@ where
 		} else {
 			vec![]
 		}
+	} else if tx_slate_id.is_some() {
+		let tx = wallet.tx_log_iter().find(|t| t.tx_slate_id == tx_slate_id);
+		if let Some(t) = tx {
+			vec![t]
+		} else {
+			vec![]
+		}
 	} else {
 		wallet
 			.tx_log_iter()

wallet/tests/accounts.rs

@@ -143,7 +143,7 @@ fn accounts_test_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 		assert_eq!(wallet1_info.total, 5 * reward);
 		assert_eq!(wallet1_info.amount_currently_spendable, (5 - cm) * reward);
 		// check tx log as well
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		assert_eq!(txs.len(), 5);
 		Ok(())
 	})?;
@@ -162,7 +162,7 @@ fn accounts_test_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 		assert_eq!(wallet1_info.total, 7 * reward);
 		assert_eq!(wallet1_info.amount_currently_spendable, 7 * reward);
 		// check tx log as well
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		assert_eq!(txs.len(), 7);
 		Ok(())
 	})?;
@@ -181,7 +181,7 @@ fn accounts_test_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 		assert_eq!(wallet1_info.total, 0,);
 		assert_eq!(wallet1_info.amount_currently_spendable, 0,);
 		// check tx log as well
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		assert_eq!(txs.len(), 0);
 		Ok(())
 	})?;
@@ -209,7 +209,7 @@ fn accounts_test_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 		let (wallet1_refreshed, wallet1_info) = api.retrieve_summary_info(true)?;
 		assert!(wallet1_refreshed);
 		assert_eq!(wallet1_info.last_confirmed_height, 13);
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		assert_eq!(txs.len(), 9);
 		Ok(())
 	})?;
@@ -224,7 +224,7 @@ fn accounts_test_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 		assert_eq!(wallet1_info.last_confirmed_height, 12);
 		let (_, wallet1_info) = api.retrieve_summary_info(true)?;
 		assert_eq!(wallet1_info.last_confirmed_height, 13);
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		println!("{:?}", txs);
 		assert_eq!(txs.len(), 5);
 		Ok(())
@@ -235,7 +235,7 @@ fn accounts_test_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 		let (wallet2_refreshed, wallet2_info) = api.retrieve_summary_info(true)?;
 		assert!(wallet2_refreshed);
 		assert_eq!(wallet2_info.last_confirmed_height, 13);
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		assert_eq!(txs.len(), 1);
 		Ok(())
 	})?;
@@ -253,7 +253,7 @@ fn accounts_test_impl(test_dir: &str) -> Result<(), libwallet::Error> {
 		assert_eq!(wallet2_info.total, 0,);
 		assert_eq!(wallet2_info.amount_currently_spendable, 0,);
 		// check tx log as well
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		assert_eq!(txs.len(), 0);
 		Ok(())
 	})?;

wallet/tests/restore.rs

@@ -138,14 +138,14 @@ fn compare_wallet_restore(
 	// Overall wallet info should be the same
 	wallet::controller::owner_single_use(wallet_source.clone(), |api| {
 		src_info = Some(api.retrieve_summary_info(true)?.1);
-		src_txs = Some(api.retrieve_txs(true, None)?.1);
+		src_txs = Some(api.retrieve_txs(true, None, None)?.1);
 		src_accts = Some(api.accounts()?);
 		Ok(())
 	})?;
 
 	wallet::controller::owner_single_use(wallet_dest.clone(), |api| {
 		dest_info = Some(api.retrieve_summary_info(true)?.1);
-		dest_txs = Some(api.retrieve_txs(true, None)?.1);
+		dest_txs = Some(api.retrieve_txs(true, None, None)?.1);
 		dest_accts = Some(api.accounts()?);
 		Ok(())
 	})?;

wallet/tests/transaction.rs

@@ -127,7 +127,7 @@ fn basic_transaction_api(test_dir: &str) -> Result<(), libwallet::Error> {
 	// Check transaction log for wallet 1
 	wallet::controller::owner_single_use(wallet1.clone(), |api| {
 		let (_, wallet1_info) = api.retrieve_summary_info(true)?;
-		let (refreshed, txs) = api.retrieve_txs(true, None)?;
+		let (refreshed, txs) = api.retrieve_txs(true, None, None)?;
 		assert!(refreshed);
 		let fee = wallet::libtx::tx_fee(
 			wallet1_info.last_confirmed_height as usize - cm as usize,
@@ -148,7 +148,7 @@ fn basic_transaction_api(test_dir: &str) -> Result<(), libwallet::Error> {
 
 	// Check transaction log for wallet 2
 	wallet::controller::owner_single_use(wallet2.clone(), |api| {
-		let (refreshed, txs) = api.retrieve_txs(true, None)?;
+		let (refreshed, txs) = api.retrieve_txs(true, None, None)?;
 		assert!(refreshed);
 		// we should have a transaction entry for this slate
 		let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id));
@@ -194,7 +194,7 @@ fn basic_transaction_api(test_dir: &str) -> Result<(), libwallet::Error> {
 		assert_eq!(wallet1_info.amount_immature, cm * reward + fee);
 
 		// check tx log entry is confirmed
-		let (refreshed, txs) = api.retrieve_txs(true, None)?;
+		let (refreshed, txs) = api.retrieve_txs(true, None, None)?;
 		assert!(refreshed);
 		let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id));
 		assert!(tx.is_some());
@@ -230,7 +230,7 @@ fn basic_transaction_api(test_dir: &str) -> Result<(), libwallet::Error> {
 		assert_eq!(wallet2_info.amount_currently_spendable, amount);
 
 		// check tx log entry is confirmed
-		let (refreshed, txs) = api.retrieve_txs(true, None)?;
+		let (refreshed, txs) = api.retrieve_txs(true, None, None)?;
 		assert!(refreshed);
 		let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id));
 		assert!(tx.is_some());
@@ -258,7 +258,7 @@ fn basic_transaction_api(test_dir: &str) -> Result<(), libwallet::Error> {
 	wallet::controller::owner_single_use(wallet1.clone(), |sender_api| {
 		let (refreshed, _wallet1_info) = sender_api.retrieve_summary_info(true)?;
 		assert!(refreshed);
-		let (_, txs) = sender_api.retrieve_txs(true, None)?;
+		let (_, txs) = sender_api.retrieve_txs(true, None, None)?;
 
 		// find the transaction
 		let tx = txs
@@ -285,7 +285,7 @@ fn basic_transaction_api(test_dir: &str) -> Result<(), libwallet::Error> {
 		assert_eq!(wallet2_info.amount_currently_spendable, amount * 3);
 
 		// check tx log entry is confirmed
-		let (refreshed, txs) = api.retrieve_txs(true, None)?;
+		let (refreshed, txs) = api.retrieve_txs(true, None, None)?;
 		assert!(refreshed);
 		let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id));
 		assert!(tx.is_some());
@@ -364,7 +364,7 @@ fn tx_rollback(test_dir: &str) -> Result<(), libwallet::Error> {
 			wallet1_info.last_confirmed_height
 		);
 		assert!(refreshed);
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		// we should have a transaction entry for this slate
 		let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id));
 		assert!(tx.is_some());
@@ -389,7 +389,7 @@ fn tx_rollback(test_dir: &str) -> Result<(), libwallet::Error> {
 
 	// Check transaction log for wallet 2
 	wallet::controller::owner_single_use(wallet2.clone(), |api| {
-		let (refreshed, txs) = api.retrieve_txs(true, None)?;
+		let (refreshed, txs) = api.retrieve_txs(true, None, None)?;
 		assert!(refreshed);
 		let mut unconfirmed_count = 0;
 		let tx = txs.iter().find(|t| t.tx_slate_id == Some(slate.id));
@@ -416,14 +416,14 @@ fn tx_rollback(test_dir: &str) -> Result<(), libwallet::Error> {
 	// Wallet 1 decides to roll back instead
 	wallet::controller::owner_single_use(wallet1.clone(), |api| {
 		// can't roll back coinbase
-		let res = api.cancel_tx(1);
+		let res = api.cancel_tx(Some(1), None);
 		assert!(res.is_err());
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		let tx = txs
 			.iter()
 			.find(|t| t.tx_slate_id == Some(slate.id))
 			.unwrap();
-		api.cancel_tx(tx.id)?;
+		api.cancel_tx(Some(tx.id), None)?;
 		let (refreshed, wallet1_info) = api.retrieve_summary_info(true)?;
 		assert!(refreshed);
 		println!(
@@ -437,7 +437,7 @@ fn tx_rollback(test_dir: &str) -> Result<(), libwallet::Error> {
 			(wallet1_info.last_confirmed_height - cm) * reward
 		);
 		// can't roll back again
-		let res = api.cancel_tx(tx.id);
+		let res = api.cancel_tx(Some(tx.id), None);
 		assert!(res.is_err());
 
 		Ok(())
@@ -445,19 +445,19 @@ fn tx_rollback(test_dir: &str) -> Result<(), libwallet::Error> {
 
 	// Wallet 2 rolls back
 	wallet::controller::owner_single_use(wallet2.clone(), |api| {
-		let (_, txs) = api.retrieve_txs(true, None)?;
+		let (_, txs) = api.retrieve_txs(true, None, None)?;
 		let tx = txs
 			.iter()
 			.find(|t| t.tx_slate_id == Some(slate.id))
 			.unwrap();
-		api.cancel_tx(tx.id)?;
+		api.cancel_tx(Some(tx.id), None)?;
 		let (refreshed, wallet2_info) = api.retrieve_summary_info(true)?;
 		assert!(refreshed);
 		// check all eligible inputs should be now be spendable
 		assert_eq!(wallet2_info.amount_currently_spendable, 0,);
 		assert_eq!(wallet2_info.total, 0,);
 		// can't roll back again
-		let res = api.cancel_tx(tx.id);
+		let res = api.cancel_tx(Some(tx.id), None);
 		assert!(res.is_err());
 
 		Ok(())