From b95caecc27a3a2bd5fe4e35c56c6393e98662db5 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Tue, 27 Nov 2018 13:36:49 +0000 Subject: [PATCH] Fixes to `grin wallet repost`, cancel tx (#2029) * fixes to wallet cancel, repost, ensure stored transaction is updated with final signatures * rustfmt * add tests for reposting * fixes to tests * repost --- src/bin/cmd/wallet.rs | 3 +- wallet/src/libwallet/api.rs | 18 +- wallet/src/libwallet/error.rs | 14 +- wallet/src/libwallet/internal/tx.rs | 27 ++- wallet/src/libwallet/internal/updater.rs | 4 +- wallet/tests/accounts.rs | 2 +- wallet/tests/repost.rs | 259 +++++++++++++++++++++++ wallet/tests/restore.rs | 8 +- wallet/tests/transaction.rs | 2 +- 9 files changed, 314 insertions(+), 23 deletions(-) create mode 100644 wallet/tests/repost.rs diff --git a/src/bin/cmd/wallet.rs b/src/bin/cmd/wallet.rs index f39a73025..7fa7b6b69 100644 --- a/src/bin/cmd/wallet.rs +++ b/src/bin/cmd/wallet.rs @@ -453,11 +453,12 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) -> i Ok(()) })?; } + api.tx_lock_outputs(&slate, lock_fn)?; api.finalize_tx(&mut slate)?; } else { adapter.send_tx_async(dest, &slate)?; + api.tx_lock_outputs(&slate, lock_fn)?; } - api.tx_lock_outputs(&slate, lock_fn)?; if adapter.supports_sync() { let result = api.post_tx(&slate, fluff); match result { diff --git a/wallet/src/libwallet/api.rs b/wallet/src/libwallet/api.rs index 72d220d6c..145f508db 100644 --- a/wallet/src/libwallet/api.rs +++ b/wallet/src/libwallet/api.rs @@ -222,9 +222,11 @@ where /// propagation. pub fn finalize_tx(&mut self, slate: &mut Slate) -> Result<(), Error> { let mut w = self.wallet.lock(); + let parent_key_id = w.parent_key_id(); w.open_with_credentials()?; let context = w.get_private_context(slate.id.as_bytes())?; tx::complete_tx(&mut *w, slate, &context)?; + tx::update_tx_hex(&mut *w, &parent_key_id, slate)?; { let mut batch = w.batch()?; batch.delete_private_context(slate.id.as_bytes())?; @@ -367,23 +369,13 @@ where ); return Err(ErrorKind::TransactionBuildingNotCompleted(tx_id))?; } - - let res = client.post_tx( + client.post_tx( &TxWrapper { tx_hex: tx_hex.unwrap(), }, fluff, - ); - if let Err(e) = res { - error!("api: repost_tx: failed with error: {}", e); - Err(e) - } else { - debug!( - "api: repost_tx: successfully posted tx at: {}, fluff? {}", - tx_id, fluff - ); - Ok(()) - } + )?; + Ok(()) } /// Attempt to restore contents of wallet diff --git a/wallet/src/libwallet/error.rs b/wallet/src/libwallet/error.rs index 1c5456b0b..403b8fd44 100644 --- a/wallet/src/libwallet/error.rs +++ b/wallet/src/libwallet/error.rs @@ -195,7 +195,19 @@ pub enum ErrorKind { impl Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - Display::fmt(&self.inner, f) + let cause = match self.cause() { + Some(c) => format!("{}", c), + None => String::from("Unknown"), + }; + let backtrace = match self.backtrace() { + Some(b) => format!("{}", b), + None => String::from("Unknown"), + }; + let output = format!( + "{} \n Cause: {} \n Backtrace: {}", + self.inner, cause, backtrace + ); + Display::fmt(&output, f) } } diff --git a/wallet/src/libwallet/internal/tx.rs b/wallet/src/libwallet/internal/tx.rs index d082f2972..0884452ed 100644 --- a/wallet/src/libwallet/internal/tx.rs +++ b/wallet/src/libwallet/internal/tx.rs @@ -15,11 +15,12 @@ //! Transaction building functions use std::sync::Arc; -use util::RwLock; +use util::{self, RwLock}; use uuid::Uuid; use core::core::verifier_cache::LruVerifierCache; use core::core::Transaction; +use core::ser; use keychain::{Identifier, Keychain}; use libtx::slate::Slate; use libtx::{build, tx_fee}; @@ -206,6 +207,30 @@ where Ok((tx.confirmed, tx.tx_hex)) } +/// Update the stored hex transaction (this update needs to happen when the TX is finalised) +pub fn update_tx_hex( + wallet: &mut T, + parent_key_id: &Identifier, + slate: &Slate, +) -> Result<(), Error> +where + T: WalletBackend, + C: NodeClient, + K: Keychain, +{ + let tx_hex = util::to_hex(ser::ser_vec(&slate.tx).unwrap()); + let tx_vec = updater::retrieve_txs(wallet, None, Some(slate.id), parent_key_id)?; + if tx_vec.len() != 1 { + return Err(ErrorKind::TransactionDoesntExist(slate.id.to_string()))?; + } + let mut tx = tx_vec[0].clone(); + tx.tx_hex = Some(tx_hex); + let batch = wallet.batch()?; + batch.save_tx_log_entry(tx, &parent_key_id)?; + batch.commit()?; + Ok(()) +} + /// Issue a burn tx pub fn issue_burn_tx( wallet: &mut T, diff --git a/wallet/src/libwallet/internal/updater.rs b/wallet/src/libwallet/internal/updater.rs index 756361b09..7c63b5fab 100644 --- a/wallet/src/libwallet/internal/updater.rs +++ b/wallet/src/libwallet/internal/updater.rs @@ -91,7 +91,9 @@ where { // just read the wallet here, no need for a write lock let mut txs = if let Some(id) = tx_id { - let tx = wallet.tx_log_iter().find(|t| t.id == id); + let tx = wallet + .tx_log_iter() + .find(|t| t.id == id && t.parent_key_id == *parent_key_id); if let Some(t) = tx { vec![t] } else { diff --git a/wallet/tests/accounts.rs b/wallet/tests/accounts.rs index 5348b9636..58cecded2 100644 --- a/wallet/tests/accounts.rs +++ b/wallet/tests/accounts.rs @@ -192,8 +192,8 @@ fn accounts_test_impl(test_dir: &str) -> Result<(), libwallet::Error> { true, // select all outputs )?; slate = client1.send_tx_slate_direct("wallet2", &slate)?; - api.finalize_tx(&mut slate)?; api.tx_lock_outputs(&slate, lock_fn)?; + api.finalize_tx(&mut slate)?; api.post_tx(&slate, false)?; Ok(()) })?; diff --git a/wallet/tests/repost.rs b/wallet/tests/repost.rs new file mode 100644 index 000000000..7288fa58d --- /dev/null +++ b/wallet/tests/repost.rs @@ -0,0 +1,259 @@ +// Copyright 2018 The Grin Developers +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Test a wallet repost command +extern crate grin_chain as chain; +extern crate grin_core as core; +extern crate grin_keychain as keychain; +extern crate grin_store as store; +extern crate grin_util as util; +extern crate grin_wallet as wallet; +extern crate rand; +#[macro_use] +extern crate log; +extern crate chrono; +extern crate serde; +extern crate uuid; + +mod common; +use common::testclient::{LocalWalletClient, WalletProxy}; + +use std::fs; +use std::thread; +use std::time::Duration; + +use core::global; +use core::global::ChainTypes; +use keychain::ExtKeychain; +use wallet::libtx::slate::Slate; +use wallet::{libwallet, FileWalletCommAdapter}; + +fn clean_output_dir(test_dir: &str) { + let _ = fs::remove_dir_all(test_dir); +} + +fn setup(test_dir: &str) { + util::init_test_logger(); + clean_output_dir(test_dir); + global::set_mining_mode(ChainTypes::AutomatedTesting); +} + +/// self send impl +fn file_repost_test_impl(test_dir: &str) -> Result<(), libwallet::Error> { + setup(test_dir); + // Create a new proxy to simulate server and wallet responses + let mut wallet_proxy: WalletProxy = WalletProxy::new(test_dir); + let chain = wallet_proxy.chain.clone(); + + let client1 = LocalWalletClient::new("wallet1", wallet_proxy.tx.clone()); + let wallet1 = common::create_wallet(&format!("{}/wallet1", test_dir), client1.clone()); + wallet_proxy.add_wallet("wallet1", client1.get_send_instance(), wallet1.clone()); + + let client2 = LocalWalletClient::new("wallet2", wallet_proxy.tx.clone()); + let wallet2 = common::create_wallet(&format!("{}/wallet2", test_dir), client2.clone()); + wallet_proxy.add_wallet("wallet2", client2.get_send_instance(), wallet2.clone()); + + // Set the wallet proxy listener running + thread::spawn(move || { + if let Err(e) = wallet_proxy.run() { + error!("Wallet Proxy error: {}", e); + } + }); + + // few values to keep things shorter + let reward = core::consensus::REWARD; + + // add some accounts + wallet::controller::owner_single_use(wallet1.clone(), |api| { + api.new_account_path("mining")?; + api.new_account_path("listener")?; + Ok(()) + })?; + + // add some accounts + wallet::controller::owner_single_use(wallet2.clone(), |api| { + api.new_account_path("account1")?; + api.new_account_path("account2")?; + Ok(()) + })?; + + // Get some mining done + { + let mut w = wallet1.lock(); + w.set_parent_key_id_by_name("mining")?; + } + let mut bh = 10u64; + let _ = common::award_blocks_to_wallet(&chain, wallet1.clone(), bh as usize); + + let send_file = format!("{}/part_tx_1.tx", test_dir); + let receive_file = format!("{}/part_tx_2.tx", test_dir); + + // Should have 5 in account1 (5 spendable), 5 in account (2 spendable) + wallet::controller::owner_single_use(wallet1.clone(), |api| { + let (wallet1_refreshed, wallet1_info) = api.retrieve_summary_info(true, 1)?; + assert!(wallet1_refreshed); + assert_eq!(wallet1_info.last_confirmed_height, bh); + assert_eq!(wallet1_info.total, bh * reward); + // send to send + let (mut slate, lock_fn) = api.initiate_tx( + Some("mining"), + reward * 2, // amount + 2, // minimum confirmations + 500, // max outputs + 1, // num change outputs + true, // select all outputs + )?; + // output tx file + let file_adapter = FileWalletCommAdapter::new(); + file_adapter.send_tx_async(&send_file, &mut slate)?; + api.tx_lock_outputs(&slate, lock_fn)?; + Ok(()) + })?; + + let _ = common::award_blocks_to_wallet(&chain, wallet1.clone(), 3); + bh += 3; + + // wallet 1 receives file to different account, completes + { + let mut w = wallet1.lock(); + w.set_parent_key_id_by_name("listener")?; + } + + wallet::controller::foreign_single_use(wallet1.clone(), |api| { + let adapter = FileWalletCommAdapter::new(); + let mut slate = adapter.receive_tx_async(&send_file)?; + api.receive_tx(&mut slate, None)?; + adapter.send_tx_async(&receive_file, &mut slate)?; + Ok(()) + })?; + + // wallet 1 receives file to different account, completes + { + let mut w = wallet1.lock(); + w.set_parent_key_id_by_name("mining")?; + } + + let mut slate = Slate::blank(2); + // wallet 1 finalize + wallet::controller::owner_single_use(wallet1.clone(), |api| { + let adapter = FileWalletCommAdapter::new(); + slate = adapter.receive_tx_async(&receive_file)?; + api.finalize_tx(&mut slate)?; + Ok(()) + })?; + + // Now repost from cached + wallet::controller::owner_single_use(wallet1.clone(), |api| { + let (_, txs) = api.retrieve_txs(true, None, Some(slate.id))?; + api.post_stored_tx(txs[0].id, false)?; + bh += 1; + Ok(()) + })?; + + let _ = common::award_blocks_to_wallet(&chain, wallet1.clone(), 3); + bh += 3; + + // update/test contents of both accounts + wallet::controller::owner_single_use(wallet1.clone(), |api| { + let (wallet1_refreshed, wallet1_info) = api.retrieve_summary_info(true, 1)?; + assert!(wallet1_refreshed); + assert_eq!(wallet1_info.last_confirmed_height, bh); + assert_eq!(wallet1_info.total, bh * reward - reward * 2); + Ok(()) + })?; + + { + let mut w = wallet1.lock(); + w.set_parent_key_id_by_name("listener")?; + } + + wallet::controller::owner_single_use(wallet1.clone(), |api| { + let (wallet2_refreshed, wallet2_info) = api.retrieve_summary_info(true, 1)?; + assert!(wallet2_refreshed); + assert_eq!(wallet2_info.last_confirmed_height, bh); + assert_eq!(wallet2_info.total, 2 * reward); + Ok(()) + })?; + + // as above, but syncronously + { + let mut w = wallet1.lock(); + w.set_parent_key_id_by_name("mining")?; + } + { + let mut w = wallet2.lock(); + w.set_parent_key_id_by_name("account1")?; + } + + let mut slate = Slate::blank(2); + let amount = 60_000_000_000; + + wallet::controller::owner_single_use(wallet1.clone(), |sender_api| { + // note this will increment the block count as part of the transaction "Posting" + let (slate_i, lock_fn) = sender_api.initiate_tx( + None, + amount * 2, // amount + 2, // minimum confirmations + 500, // max outputs + 1, // num change outputs + true, // select all outputs + )?; + slate = client1.send_tx_slate_direct("wallet2", &slate_i)?; + sender_api.tx_lock_outputs(&slate, lock_fn)?; + sender_api.finalize_tx(&mut slate)?; + Ok(()) + })?; + + let _ = common::award_blocks_to_wallet(&chain, wallet1.clone(), 3); + bh += 3; + + // Now repost from cached + wallet::controller::owner_single_use(wallet1.clone(), |api| { + let (_, txs) = api.retrieve_txs(true, None, Some(slate.id))?; + api.post_stored_tx(txs[0].id, false)?; + bh += 1; + Ok(()) + })?; + + let _ = common::award_blocks_to_wallet(&chain, wallet1.clone(), 3); + bh += 3; + // + // update/test contents of both accounts + wallet::controller::owner_single_use(wallet1.clone(), |api| { + let (wallet1_refreshed, wallet1_info) = api.retrieve_summary_info(true, 1)?; + assert!(wallet1_refreshed); + assert_eq!(wallet1_info.last_confirmed_height, bh); + assert_eq!(wallet1_info.total, bh * reward - reward * 4); + Ok(()) + })?; + + wallet::controller::owner_single_use(wallet2.clone(), |api| { + let (wallet2_refreshed, wallet2_info) = api.retrieve_summary_info(true, 1)?; + assert!(wallet2_refreshed); + assert_eq!(wallet2_info.last_confirmed_height, bh); + assert_eq!(wallet2_info.total, 2 * amount); + Ok(()) + })?; + + // let logging finish + thread::sleep(Duration::from_millis(200)); + Ok(()) +} + +#[test] +fn wallet_file_repost() { + let test_dir = "test_output/file_repost"; + if let Err(e) = file_repost_test_impl(test_dir) { + panic!("Libwallet Error: {} - {}", e, e.backtrace().unwrap()); + } +} diff --git a/wallet/tests/restore.rs b/wallet/tests/restore.rs index 693dafd5b..323de93f3 100644 --- a/wallet/tests/restore.rs +++ b/wallet/tests/restore.rs @@ -238,8 +238,8 @@ fn setup_restore(test_dir: &str) -> Result<(), libwallet::Error> { true, // select all outputs )?; slate = client1.send_tx_slate_direct("wallet2", &slate_i)?; - sender_api.finalize_tx(&mut slate)?; sender_api.tx_lock_outputs(&slate, lock_fn)?; + sender_api.finalize_tx(&mut slate)?; sender_api.post_tx(&slate, false)?; Ok(()) })?; @@ -259,8 +259,8 @@ fn setup_restore(test_dir: &str) -> Result<(), libwallet::Error> { true, // select all outputs )?; slate = client1.send_tx_slate_direct("wallet3", &slate_i)?; - sender_api.finalize_tx(&mut slate)?; sender_api.tx_lock_outputs(&slate, lock_fn)?; + sender_api.finalize_tx(&mut slate)?; sender_api.post_tx(&slate, false)?; Ok(()) })?; @@ -280,8 +280,8 @@ fn setup_restore(test_dir: &str) -> Result<(), libwallet::Error> { true, // select all outputs )?; slate = client3.send_tx_slate_direct("wallet2", &slate_i)?; - sender_api.finalize_tx(&mut slate)?; sender_api.tx_lock_outputs(&slate, lock_fn)?; + sender_api.finalize_tx(&mut slate)?; sender_api.post_tx(&slate, false)?; Ok(()) })?; @@ -307,8 +307,8 @@ fn setup_restore(test_dir: &str) -> Result<(), libwallet::Error> { true, // select all outputs )?; slate = client3.send_tx_slate_direct("wallet2", &slate_i)?; - sender_api.finalize_tx(&mut slate)?; sender_api.tx_lock_outputs(&slate, lock_fn)?; + sender_api.finalize_tx(&mut slate)?; sender_api.post_tx(&slate, false)?; Ok(()) })?; diff --git a/wallet/tests/transaction.rs b/wallet/tests/transaction.rs index a54e0fe8e..11ffa3d5b 100644 --- a/wallet/tests/transaction.rs +++ b/wallet/tests/transaction.rs @@ -112,8 +112,8 @@ fn basic_transaction_api(test_dir: &str) -> Result<(), libwallet::Error> { true, // select all outputs )?; slate = client1.send_tx_slate_direct("wallet2", &slate_i)?; - sender_api.finalize_tx(&mut slate)?; sender_api.tx_lock_outputs(&slate, lock_fn)?; + sender_api.finalize_tx(&mut slate)?; Ok(()) })?;