From 5ac61b0bc8b5126f48e4f1eae0f67082128cb29e Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Wed, 27 Jun 2018 16:57:40 +0100 Subject: [PATCH] fixes to wallet based on testing, ensure details file is being written properly for file wallet (#1204) --- src/bin/grin.rs | 2 + wallet/src/file_wallet.rs | 77 ++++++++++++++++------ wallet/src/libwallet/api.rs | 2 +- wallet/src/libwallet/controller.rs | 6 +- wallet/src/libwallet/error.rs | 7 +- wallet/src/libwallet/internal/selection.rs | 2 +- wallet/src/libwallet/internal/tx.rs | 2 +- wallet/src/libwallet/internal/updater.rs | 35 +++++----- wallet/src/libwallet/types.rs | 8 ++- wallet/src/lmdb_wallet.rs | 12 +++- 10 files changed, 108 insertions(+), 45 deletions(-) diff --git a/src/bin/grin.rs b/src/bin/grin.rs index 1d34f3f0b..5de5623df 100644 --- a/src/bin/grin.rs +++ b/src/bin/grin.rs @@ -739,4 +739,6 @@ fn wallet_command(wallet_args: &ArgMatches, global_config: GlobalConfig) { } }); } + // we need to give log output a chance to catch up before exiting + thread::sleep(Duration::from_millis(100)); } diff --git a/wallet/src/file_wallet.rs b/wallet/src/file_wallet.rs index e734a6070..b439f8838 100644 --- a/wallet/src/file_wallet.rs +++ b/wallet/src/file_wallet.rs @@ -14,7 +14,7 @@ use std::collections::HashMap; use std::fs::{self, File}; -use std::io::{Read, Write}; +use std::io::Write; use std::path::{Path, MAIN_SEPARATOR}; use serde_json; @@ -44,14 +44,17 @@ const DET_BCK_FILE: &'static str = "wallet.detbck"; const DAT_FILE: &'static str = "wallet.dat"; const BCK_FILE: &'static str = "wallet.bck"; const LOCK_FILE: &'static str = "wallet.lock"; -const SEED_FILE: &'static str = "wallet.seed"; #[derive(Debug)] struct FileBatch<'a> { /// List of outputs outputs: &'a mut HashMap, + /// Wallet Details + details: &'a mut WalletDetails, /// Data file path data_file_path: String, + /// Details file path + details_file_path: String, /// lock file path lock_file_path: String, } @@ -62,6 +65,10 @@ impl<'a> WalletOutputBatch for FileBatch<'a> { Ok(()) } + fn details(&mut self) -> &mut WalletDetails { + &mut self.details + } + fn get(&self, id: &Identifier) -> Result { self.outputs .get(&id.to_hex()) @@ -69,6 +76,10 @@ impl<'a> WalletOutputBatch for FileBatch<'a> { .ok_or(libwallet::ErrorKind::Backend("not found".to_string()).into()) } + fn iter<'b>(&'b self) -> Box + 'b> { + Box::new(self.outputs.values().cloned()) + } + fn delete(&mut self, id: &Identifier) -> Result<(), libwallet::Error> { let _ = self.outputs.remove(&id.to_hex()); Ok(()) @@ -86,17 +97,27 @@ impl<'a> WalletOutputBatch for FileBatch<'a> { fn commit(&self) -> Result<(), libwallet::Error> { let mut data_file = File::create(self.data_file_path.clone()) .context(libwallet::ErrorKind::CallbackImpl("Could not create"))?; + let mut details_file = File::create(self.details_file_path.clone()) + .context(libwallet::ErrorKind::CallbackImpl("Could not create"))?; let mut outputs = self.outputs.values().collect::>(); outputs.sort(); let res_json = serde_json::to_vec_pretty(&outputs).context( - libwallet::ErrorKind::CallbackImpl("Error serializing wallet data"), + libwallet::ErrorKind::CallbackImpl("Error serializing wallet output data"), + )?; + let details_res_json = serde_json::to_vec_pretty(&self.details).context( + libwallet::ErrorKind::CallbackImpl("Error serializing wallet details data"), )?; data_file .write_all(res_json.as_slice()) .context(libwallet::ErrorKind::CallbackImpl( - "Error writing wallet file", - )) - .map_err(|e| e.into()) + "Error writing wallet data file", + ))?; + details_file + .write_all(details_res_json.as_slice()) + .context(libwallet::ErrorKind::CallbackImpl( + "Error writing wallet details file", + ))?; + Ok(()) } } @@ -189,7 +210,9 @@ where Ok(Box::new(FileBatch { outputs: &mut self.outputs, + details: &mut self.details, data_file_path: self.data_file_path.clone(), + details_file_path: self.details_file_path.clone(), lock_file_path: self.lock_file_path.clone(), })) } @@ -199,19 +222,23 @@ where &'a mut self, root_key_id: keychain::Identifier, ) -> Result { - let mut max_n = 0; - for out in self.outputs.values() { - if max_n < out.n_child && out.root_key_id == root_key_id { - max_n = out.n_child; + let mut batch = self.batch()?; + { + let mut max_n = 0; + for out in batch.iter() { + if max_n < out.n_child && out.root_key_id == root_key_id { + max_n = out.n_child; + } + } + let details = batch.details(); + if details.last_child_index <= max_n { + details.last_child_index = max_n + 1; + } else { + details.last_child_index += 1; } } - - if self.details.last_child_index <= max_n { - self.details.last_child_index = max_n + 1; - } else { - self.details.last_child_index += 1; - } - Ok(self.details.last_child_index) + batch.commit()?; + Ok(batch.details().last_child_index) } /// Select spendable coins from the wallet. @@ -304,18 +331,30 @@ impl WalletClient for FileWallet { Ok(r) => Ok(r), Err(e) => { let message = format!("{}", e.cause().unwrap()); + error!( + LOGGER, + "Create Coinbase: Communication error: {},{}", + e.cause().unwrap(), + e.backtrace().unwrap() + ); Err(libwallet::ErrorKind::WalletComms(message))? } } } /// Send a transaction slate to another listening wallet and return result - fn send_tx_slate(&self, slate: &Slate) -> Result { - let res = client::send_tx_slate(self.node_url(), slate); + fn send_tx_slate(&self, addr: &str, slate: &Slate) -> Result { + let res = client::send_tx_slate(addr, slate); match res { Ok(r) => Ok(r), Err(e) => { let message = format!("{}", e.cause().unwrap()); + error!( + LOGGER, + "Send TX Slate: Communication error: {},{}", + e.cause().unwrap(), + e.backtrace().unwrap() + ); Err(libwallet::ErrorKind::WalletComms(message))? } } diff --git a/wallet/src/libwallet/api.rs b/wallet/src/libwallet/api.rs index 09745c4d5..c7809f6c6 100644 --- a/wallet/src/libwallet/api.rs +++ b/wallet/src/libwallet/api.rs @@ -103,7 +103,7 @@ where selection_strategy_is_use_all, )?; - let mut slate = match self.wallet.send_tx_slate(&slate) { + let mut slate = match self.wallet.send_tx_slate(dest, &slate) { Ok(s) => s, Err(e) => { error!( diff --git a/wallet/src/libwallet/controller.rs b/wallet/src/libwallet/controller.rs index de3c656df..52f6e500c 100644 --- a/wallet/src/libwallet/controller.rs +++ b/wallet/src/libwallet/controller.rs @@ -31,8 +31,9 @@ use failure::Fail; use keychain::Keychain; use libtx::slate::Slate; use libwallet::api::{APIForeign, APIOwner}; -use libwallet::types::{BlockFees, CbData, OutputData, SendTXArgs, WalletBackend, WalletClient, - WalletInfo}; +use libwallet::types::{ + BlockFees, CbData, OutputData, SendTXArgs, WalletBackend, WalletClient, WalletInfo, +}; use libwallet::{Error, ErrorKind}; use util::LOGGER; @@ -379,6 +380,7 @@ where T: WalletBackend + WalletClient, K: Keychain, { + /// create a new api handler pub fn new(wallet: Arc>) -> ForeignAPIHandler { ForeignAPIHandler { wallet, diff --git a/wallet/src/libwallet/error.rs b/wallet/src/libwallet/error.rs index 0f7dd771e..12c4a31b3 100644 --- a/wallet/src/libwallet/error.rs +++ b/wallet/src/libwallet/error.rs @@ -48,8 +48,11 @@ pub enum ErrorKind { }, /// Fee Exceeds amount - #[fail(display = "Fee exceeds amount: sender amount {}, recipient fee {}", sender_amount, - recipient_fee)] + #[fail( + display = "Fee exceeds amount: sender amount {}, recipient fee {}", + sender_amount, + recipient_fee + )] FeeExceedsAmount { /// sender amount sender_amount: u64, diff --git a/wallet/src/libwallet/internal/selection.rs b/wallet/src/libwallet/internal/selection.rs index f15b85db2..30de86a51 100644 --- a/wallet/src/libwallet/internal/selection.rs +++ b/wallet/src/libwallet/internal/selection.rs @@ -15,7 +15,7 @@ //! Selection of inputs for building transactions use keychain::{Identifier, Keychain}; -use libtx::{build, tx_fee, slate::Slate}; +use libtx::{build, slate::Slate, tx_fee}; use libwallet::error::{Error, ErrorKind}; use libwallet::internal::{keys, sigcontext}; use libwallet::types::*; diff --git a/wallet/src/libwallet/internal/tx.rs b/wallet/src/libwallet/internal/tx.rs index db30da5a5..6893341d8 100644 --- a/wallet/src/libwallet/internal/tx.rs +++ b/wallet/src/libwallet/internal/tx.rs @@ -32,7 +32,7 @@ where { // create an output using the amount in the slate let (_, mut context, receiver_create_fn) = - selection::build_recipient_output_with_slate(wallet, slate).unwrap(); + selection::build_recipient_output_with_slate(wallet, slate)?; // fill public keys let _ = slate.fill_round_1( diff --git a/wallet/src/libwallet/internal/updater.rs b/wallet/src/libwallet/internal/updater.rs index 81563012b..5b355721b 100644 --- a/wallet/src/libwallet/internal/updater.rs +++ b/wallet/src/libwallet/internal/updater.rs @@ -16,7 +16,6 @@ //! the wallet storage and update them. use failure::ResultExt; -use std::collections::hash_map::Entry; use std::collections::HashMap; use core::consensus::reward; @@ -185,13 +184,15 @@ where Some(_) => output.mark_unspent(), None => output.mark_spent(), }; - batch.save(output); + batch.save(output)?; } } + { + let details = batch.details(); + details.last_confirmed_height = height; + } batch.commit()?; } - let details = wallet.details(); - details.last_confirmed_height = height; Ok(()) } @@ -328,18 +329,20 @@ where { // Now acquire the wallet lock and write the new output. let mut batch = wallet.batch()?; - batch.save(OutputData { - root_key_id: root_key_id.clone(), - key_id: key_id.clone(), - n_child: derivation, - value: reward(block_fees.fees), - status: OutputStatus::Unconfirmed, - height: height, - lock_height: lock_height, - is_coinbase: true, - block: None, - merkle_proof: None, - }); + { + batch.save(OutputData { + root_key_id: root_key_id.clone(), + key_id: key_id.clone(), + n_child: derivation, + value: reward(block_fees.fees), + status: OutputStatus::Unconfirmed, + height: height, + lock_height: lock_height, + is_coinbase: true, + block: None, + merkle_proof: None, + })?; + } batch.commit()?; } diff --git a/wallet/src/libwallet/types.rs b/wallet/src/libwallet/types.rs index 2f83c587d..1bdaac243 100644 --- a/wallet/src/libwallet/types.rs +++ b/wallet/src/libwallet/types.rs @@ -88,9 +88,15 @@ pub trait WalletOutputBatch { /// Add or update data about an output to the backend fn save(&mut self, out: OutputData) -> Result<(), Error>; + /// Get wallet details + fn details(&mut self) -> &mut WalletDetails; + /// Gets output data by id fn get(&self, id: &Identifier) -> Result; + /// Iterate over all output data in batch + fn iter<'b>(&'b self) -> Box + 'b>; + /// Delete data about an output to the backend fn delete(&mut self, id: &Identifier) -> Result<(), Error>; @@ -112,7 +118,7 @@ pub trait WalletClient { /// Send a transaction slate to another listening wallet and return result /// TODO: Probably need a slate wrapper type - fn send_tx_slate(&self, slate: &Slate) -> Result; + fn send_tx_slate(&self, addr: &str, slate: &Slate) -> Result; /// Posts a transaction to a grin node fn post_tx(&self, tx: &TxWrapper, fluff: bool) -> Result<(), Error>; diff --git a/wallet/src/lmdb_wallet.rs b/wallet/src/lmdb_wallet.rs index dbde27256..b24d12f12 100644 --- a/wallet/src/lmdb_wallet.rs +++ b/wallet/src/lmdb_wallet.rs @@ -160,11 +160,19 @@ impl<'a, K> WalletOutputBatch for Batch<'a, K> { Ok(()) } + fn details(&mut self) -> &mut WalletDetails { + unimplemented!() + } + fn get(&self, id: &Identifier) -> Result { let key = to_key(OUTPUT_PREFIX, &mut id.to_bytes().to_vec()); option_to_not_found(self.db.borrow().as_ref().unwrap().get_ser(&key)).map_err(|e| e.into()) } + fn iter<'b>(&'b self) -> Box + 'b> { + unimplemented!(); + } + fn delete(&mut self, id: &Identifier) -> Result<(), Error> { let key = to_key(OUTPUT_PREFIX, &mut id.to_bytes().to_vec()); self.db.borrow().as_ref().unwrap().delete(&key)?; @@ -197,8 +205,8 @@ impl WalletClient for LMDBBackend { } /// Send a transaction slate to another listening wallet and return result - fn send_tx_slate(&self, slate: &Slate) -> Result { - let res = client::send_tx_slate(self.node_url(), slate) + fn send_tx_slate(&self, addr: &str, slate: &Slate) -> Result { + let res = client::send_tx_slate(addr, slate) .context(ErrorKind::WalletComms(format!("Sending transaction")))?; Ok(res) }