diff --git a/doc/api/wallet_owner_api.md b/doc/api/wallet_owner_api.md index 85dd7abef..3da387570 100644 --- a/doc/api/wallet_owner_api.md +++ b/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?refresh?id=x + * /v1/wallet/owner/retrieve_txs + * /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 - `tx_id=[number]` to retrieve only the specified output + * `refresh` to refresh from node + * `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** diff --git a/src/bin/cmd/wallet.rs b/src/bin/cmd/wallet.rs index fb375f02e..7b5909a67 100644 --- a/src/bin/cmd/wallet.rs +++ b/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 - .value_of("id") - .ok_or_else(|| { - ErrorKind::GenericError("'id' argument (-i) is required.".to_string()) - }).and_then(|v| { - v.parse().map_err(|e| { - ErrorKind::GenericError(format!( + let mut tx_id_string = ""; + let tx_id = match tx_args.value_of("id") { + 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 id parameter. e={:?}", - e - )) - }) - })?; - let result = api.cancel_tx(tx_id); + e, + )).into()); + } + }, + }; + 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) => { diff --git a/src/bin/grin.rs b/src/bin/grin.rs index 8679f7f07..704fda17d 100644 --- a/src/bin/grin.rs +++ b/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") diff --git a/wallet/src/libwallet/api.rs b/wallet/src/libwallet/api.rs index 8ff9ab30f..cb9778f35 100644 --- a/wallet/src/libwallet/api.rs +++ b/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, + tx_slate_id: Option, ) -> Result<(bool, Vec), 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, + tx_slate_id: Option, + ) -> 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(()) } diff --git a/wallet/src/libwallet/controller.rs b/wallet/src/libwallet/controller.rs index 9305a0d52..da7eb7754 100644 --- a/wallet/src/libwallet/controller.rs +++ b/wallet/src/libwallet/controller.rs @@ -207,7 +207,8 @@ where req: &Request, api: APIOwner, ) -> Result<(bool, Vec), 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())) } } diff --git a/wallet/src/libwallet/error.rs b/wallet/src/libwallet/error.rs index d7dc888eb..7a84a2a55 100644 --- a/wallet/src/libwallet/error.rs +++ b/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)] diff --git a/wallet/src/libwallet/internal/tx.rs b/wallet/src/libwallet/internal/tx.rs index b1b8bd7bb..82aca05d0 100644 --- a/wallet/src/libwallet/internal/tx.rs +++ b/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( wallet: &mut T, parent_key_id: &Identifier, - tx_id: u32, + tx_id: Option, + tx_slate_id: Option, ) -> Result<(), Error> where T: WalletBackend, @@ -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)) diff --git a/wallet/src/libwallet/internal/updater.rs b/wallet/src/libwallet/internal/updater.rs index cc6f8abc1..4f0abb0f4 100644 --- a/wallet/src/libwallet/internal/updater.rs +++ b/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( wallet: &mut T, tx_id: Option, + tx_slate_id: Option, parent_key_id: &Identifier, ) -> Result, 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() diff --git a/wallet/tests/accounts.rs b/wallet/tests/accounts.rs index 4b2f7b982..9646b2617 100644 --- a/wallet/tests/accounts.rs +++ b/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(()) })?; diff --git a/wallet/tests/restore.rs b/wallet/tests/restore.rs index 3a441c33a..b9b36a994 100644 --- a/wallet/tests/restore.rs +++ b/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(()) })?; diff --git a/wallet/tests/transaction.rs b/wallet/tests/transaction.rs index 3b2968c4f..1c73d172d 100644 --- a/wallet/tests/transaction.rs +++ b/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(())