From 9a497f143939ac475e6c45d478bb1d49a9c53268 Mon Sep 17 00:00:00 2001 From: Yeastplume Date: Mon, 14 Jan 2019 15:43:10 +0000 Subject: [PATCH] Wallet Performance enhancements - cache commit + transaction fetching logic (#2375) * add optional cached commit to output data, to speed up queries * rustfmt * clean up transaction retrieval logic * rustfmt * small logic error * rename cache method * rustfmt --- config/src/comments.rs | 9 +++ wallet/src/libwallet/api.rs | 4 +- wallet/src/libwallet/internal/restore.rs | 3 + wallet/src/libwallet/internal/selection.rs | 16 ++++- wallet/src/libwallet/internal/tx.rs | 6 +- wallet/src/libwallet/internal/updater.rs | 74 +++++++++++++--------- wallet/src/libwallet/types.rs | 9 +++ wallet/src/lmdb_wallet.rs | 15 +++++ wallet/src/types.rs | 4 ++ 9 files changed, 104 insertions(+), 36 deletions(-) diff --git a/config/src/comments.rs b/config/src/comments.rs index 5b339d591..67507d812 100644 --- a/config/src/comments.rs +++ b/config/src/comments.rs @@ -411,6 +411,15 @@ fn comments() -> HashMap { "data_file_dir".to_string(), " #where to find wallet files (seed, data, etc) +" + .to_string(), + ); + retval.insert( + "no_commit_cache".to_string(), + " +#If true, don't store calculated commits in the database +#better privacy, but at a performance cost of having to +#re-calculate commits every time they're used " .to_string(), ); diff --git a/wallet/src/libwallet/api.rs b/wallet/src/libwallet/api.rs index 91a7b79d5..9d247a148 100644 --- a/wallet/src/libwallet/api.rs +++ b/wallet/src/libwallet/api.rs @@ -431,7 +431,7 @@ where let res = Ok(( validated, - updater::retrieve_txs(&mut *w, tx_id, tx_slate_id, Some(&parent_key_id))?, + updater::retrieve_txs(&mut *w, tx_id, tx_slate_id, Some(&parent_key_id), false)?, )); w.close()?; @@ -867,7 +867,7 @@ where None => w.parent_key_id(), }; // Don't do this multiple times - let tx = updater::retrieve_txs(&mut *w, None, Some(slate.id), Some(&parent_key_id))?; + let tx = updater::retrieve_txs(&mut *w, None, Some(slate.id), Some(&parent_key_id), false)?; for t in &tx { if t.tx_type == TxLogEntryType::TxReceived { return Err(ErrorKind::TransactionAlreadyReceived(slate.id.to_string()).into()); diff --git a/wallet/src/libwallet/internal/restore.rs b/wallet/src/libwallet/internal/restore.rs index fe25a88ec..af3fe2caa 100644 --- a/wallet/src/libwallet/internal/restore.rs +++ b/wallet/src/libwallet/internal/restore.rs @@ -142,6 +142,7 @@ where C: NodeClient, K: Keychain, { + let commit = wallet.calc_commit_for_cache(output.value, &output.key_id)?; let mut batch = wallet.batch()?; let parent_key_id = output.key_id.parent_path(); @@ -167,6 +168,7 @@ where key_id: output.key_id, n_child: output.n_child, mmr_index: Some(output.mmr_index), + commit: commit, value: output.value, status: OutputStatus::Unspent, height: output.height, @@ -198,6 +200,7 @@ where output.tx_log_entry.clone(), None, Some(&parent_key_id), + false, )?; if entries.len() > 0 { let mut entry = entries[0].clone(); diff --git a/wallet/src/libwallet/internal/selection.rs b/wallet/src/libwallet/internal/selection.rs index a7740b26f..fcc6ed4ca 100644 --- a/wallet/src/libwallet/internal/selection.rs +++ b/wallet/src/libwallet/internal/selection.rs @@ -21,6 +21,8 @@ use crate::libwallet::error::{Error, ErrorKind}; use crate::libwallet::internal::keys; use crate::libwallet::types::*; +use std::collections::HashMap; + /// Initialize a transaction on the sender side, returns a corresponding /// libwallet transaction slate with the appropriate inputs selected, /// and saves the private wallet identifiers of our selected outputs @@ -85,9 +87,15 @@ where context.add_input(&input.key_id, &input.mmr_index); } - // Store change output(s) - for (_, id, mmr_index) in &change_amounts_derivations { + let mut commits: HashMap> = HashMap::new(); + + // Store change output(s) and cached commits + for (change_amount, id, mmr_index) in &change_amounts_derivations { context.add_output(&id, &mmr_index); + commits.insert( + id.clone(), + wallet.calc_commit_for_cache(*change_amount, &id)?, + ); } let lock_inputs = context.get_inputs().clone(); @@ -119,10 +127,12 @@ where for (change_amount, id, _) in &change_amounts_derivations { t.num_outputs += 1; t.amount_credited += change_amount; + let commit = commits.get(&id).unwrap().clone(); batch.save(OutputData { root_key_id: parent_key_id.clone(), key_id: id.clone(), n_child: id.to_path().last_path_index(), + commit: commit, mmr_index: None, value: change_amount.clone(), status: OutputStatus::Unconfirmed, @@ -189,6 +199,7 @@ where // Create closure that adds the output to recipient's wallet // (up to the caller to decide when to do) let wallet_add_fn = move |wallet: &mut T| { + let commit = wallet.calc_commit_for_cache(amount, &key_id_inner)?; let mut batch = wallet.batch()?; let log_id = batch.next_tx_log_id(&parent_key_id)?; let mut t = TxLogEntry::new(parent_key_id.clone(), TxLogEntryType::TxReceived, log_id); @@ -200,6 +211,7 @@ where key_id: key_id_inner.clone(), mmr_index: None, n_child: key_id_inner.to_path().last_path_index(), + commit: commit, value: amount, status: OutputStatus::Unconfirmed, height: height, diff --git a/wallet/src/libwallet/internal/tx.rs b/wallet/src/libwallet/internal/tx.rs index a18d6b0a1..c68dd3084 100644 --- a/wallet/src/libwallet/internal/tx.rs +++ b/wallet/src/libwallet/internal/tx.rs @@ -161,7 +161,7 @@ where } 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, Some(&parent_key_id))?; + let tx_vec = updater::retrieve_txs(wallet, tx_id, tx_slate_id, Some(&parent_key_id), false)?; if tx_vec.len() != 1 { return Err(ErrorKind::TransactionDoesntExist(tx_id_string))?; } @@ -191,7 +191,7 @@ where C: NodeClient, K: Keychain, { - let tx_vec = updater::retrieve_txs(wallet, Some(tx_id), None, Some(parent_key_id))?; + let tx_vec = updater::retrieve_txs(wallet, Some(tx_id), None, Some(parent_key_id), false)?; if tx_vec.len() != 1 { return Err(ErrorKind::TransactionDoesntExist(tx_id.to_string()))?; } @@ -207,7 +207,7 @@ where K: Keychain, { // finalize command - let tx_vec = updater::retrieve_txs(wallet, None, Some(slate.id), None)?; + let tx_vec = updater::retrieve_txs(wallet, None, Some(slate.id), None, false)?; let mut tx = None; // don't want to assume this is the right tx, in case of self-sending for t in tx_vec { diff --git a/wallet/src/libwallet/internal/updater.rs b/wallet/src/libwallet/internal/updater.rs index 3b4fd185f..678fa9632 100644 --- a/wallet/src/libwallet/internal/updater.rs +++ b/wallet/src/libwallet/internal/updater.rs @@ -74,7 +74,10 @@ where let res = outputs .into_iter() .map(|out| { - let commit = keychain.commit(out.value, &out.key_id).unwrap(); + let commit = match out.commit.clone() { + Some(c) => pedersen::Commitment::from_vec(util::from_hex(c).unwrap()), + None => keychain.commit(out.value, &out.key_id).unwrap(), + }; (out, commit) }) .collect(); @@ -88,30 +91,39 @@ pub fn retrieve_txs( tx_id: Option, tx_slate_id: Option, parent_key_id: Option<&Identifier>, + outstanding_only: bool, ) -> Result, Error> where T: WalletBackend, C: NodeClient, K: Keychain, { - // just read the wallet here, no need for a write lock - let mut txs = if let Some(id) = tx_id { - wallet.tx_log_iter().filter(|t| t.id == id).collect() - } else if tx_slate_id.is_some() { - wallet - .tx_log_iter() - .filter(|t| t.tx_slate_id == tx_slate_id) - .collect() - } else { - wallet.tx_log_iter().collect::>() - }; - if let Some(k) = parent_key_id { - txs = txs - .iter() - .filter(|t| t.parent_key_id == *k) - .map(|t| t.clone()) - .collect(); - } + let mut txs: Vec = wallet + .tx_log_iter() + .filter(|tx_entry| { + let f_pk = match parent_key_id { + Some(k) => tx_entry.parent_key_id == *k, + None => true, + }; + let f_tx_id = match tx_id { + Some(i) => tx_entry.id == i, + None => true, + }; + let f_txs = match tx_slate_id { + Some(t) => tx_entry.tx_slate_id == Some(t), + None => true, + }; + let f_outstanding = match outstanding_only { + true => { + !tx_entry.confirmed + && (tx_entry.tx_type == TxLogEntryType::TxReceived + || tx_entry.tx_type == TxLogEntryType::TxSent) + } + false => true, + }; + f_pk && f_tx_id && f_txs && f_outstanding + }) + .collect(); txs.sort_by_key(|tx| tx.creation_ts); Ok(txs) } @@ -153,20 +165,18 @@ where .filter(|x| x.root_key_id == *parent_key_id && x.status != OutputStatus::Spent) .collect(); + let tx_entries = retrieve_txs(wallet, None, None, Some(&parent_key_id), true)?; + // Only select outputs that are actually involved in an outstanding transaction let unspents: Vec = match update_all { false => unspents .into_iter() .filter(|x| match x.tx_log_entry.as_ref() { Some(t) => { - let entries = retrieve_txs(wallet, Some(*t), None, Some(&parent_key_id)); - match entries { - Err(_) => true, - Ok(e) => { - e.len() > 0 - && !e[0].confirmed && (e[0].tx_type == TxLogEntryType::TxReceived - || e[0].tx_type == TxLogEntryType::TxSent) - } + if let Some(_) = tx_entries.iter().find(|&te| te.id == *t) { + true + } else { + false } } None => true, @@ -176,7 +186,10 @@ where }; for out in unspents { - let commit = keychain.commit(out.value, &out.key_id)?; + let commit = match out.commit.clone() { + Some(c) => pedersen::Commitment::from_vec(util::from_hex(c).unwrap()), + None => keychain.commit(out.value, &out.key_id).unwrap(), + }; wallet_outputs.insert(commit, (out.key_id.clone(), out.mmr_index)); } Ok(wallet_outputs) @@ -466,13 +479,16 @@ where { // Now acquire the wallet lock and write the new output. + let amount = reward(block_fees.fees); + let commit = wallet.calc_commit_for_cache(amount, &key_id)?; let mut batch = wallet.batch()?; batch.save(OutputData { root_key_id: parent_key_id, key_id: key_id.clone(), n_child: key_id.to_path().last_path_index(), mmr_index: None, - value: reward(block_fees.fees), + commit: commit, + value: amount, status: OutputStatus::Unconfirmed, height: height, lock_height: lock_height, diff --git a/wallet/src/libwallet/types.rs b/wallet/src/libwallet/types.rs index a1337e2c2..cf2a35278 100644 --- a/wallet/src/libwallet/types.rs +++ b/wallet/src/libwallet/types.rs @@ -67,6 +67,13 @@ where /// Return the client being used to communicate with the node fn w2n_client(&mut self) -> &mut C; + /// return the commit for caching if allowed, none otherwise + fn calc_commit_for_cache( + &mut self, + amount: u64, + id: &Identifier, + ) -> Result, Error>; + /// Set parent key id by stored account name fn set_parent_key_id_by_name(&mut self, label: &str) -> Result<(), Error>; @@ -241,6 +248,8 @@ pub struct OutputData { pub key_id: Identifier, /// How many derivations down from the root key pub n_child: u32, + /// The actual commit, optionally stored + pub commit: Option, /// PMMR Index, used on restore in case of duplicate wallets using the same /// key_id (2 wallets using same seed, for instance pub mmr_index: Option, diff --git a/wallet/src/lmdb_wallet.rs b/wallet/src/lmdb_wallet.rs index 5f60cc909..b58368377 100644 --- a/wallet/src/lmdb_wallet.rs +++ b/wallet/src/lmdb_wallet.rs @@ -199,6 +199,21 @@ where &mut self.w2n_client } + /// return the version of the commit for caching + fn calc_commit_for_cache( + &mut self, + amount: u64, + id: &Identifier, + ) -> Result, Error> { + if self.config.no_commit_cache == Some(true) { + Ok(None) + } else { + Ok(Some(util::to_hex( + self.keychain().commit(amount, &id)?.0.to_vec(), + ))) + } + } + /// Set parent path by account name fn set_parent_key_id_by_name(&mut self, label: &str) -> Result<(), Error> { let label = label.to_owned(); diff --git a/wallet/src/types.rs b/wallet/src/types.rs index 65ca4c7a2..f47add9d4 100644 --- a/wallet/src/types.rs +++ b/wallet/src/types.rs @@ -52,6 +52,9 @@ pub struct WalletConfig { pub owner_api_include_foreign: Option, // The directory in which wallet files are stored pub data_file_dir: String, + /// If Some(true), don't cache commits alongside output data + /// speed improvement, but your commits are in the database + pub no_commit_cache: Option, /// TLS certificate file pub tls_certificate_file: Option, /// TLS certificate private key file @@ -74,6 +77,7 @@ impl Default for WalletConfig { check_node_api_http_addr: "http://127.0.0.1:3413".to_string(), owner_api_include_foreign: Some(false), data_file_dir: ".".to_string(), + no_commit_cache: Some(false), tls_certificate_file: None, tls_certificate_key: None, dark_background_color_scheme: Some(true),