From ff1238c3c42aecb1e67a50ef6d199496c0634bc4 Mon Sep 17 00:00:00 2001 From: ardocrat Date: Thu, 4 Jun 2026 16:46:36 +0300 Subject: [PATCH] backend: do not panic on iter and directory creation --- libwallet/src/api_impl/owner.rs | 9 +++-- libwallet/src/backend.rs | 54 ++++++++++++----------------- libwallet/src/internal/keys.rs | 6 ++-- libwallet/src/internal/scan.rs | 2 +- libwallet/src/internal/selection.rs | 22 ++++++------ libwallet/src/internal/updater.rs | 24 ++++++------- 6 files changed, 56 insertions(+), 61 deletions(-) diff --git a/libwallet/src/api_impl/owner.rs b/libwallet/src/api_impl/owner.rs index 429ad91..95f4dc9 100644 --- a/libwallet/src/api_impl/owner.rs +++ b/libwallet/src/api_impl/owner.rs @@ -845,7 +845,7 @@ where { let mut uuid = None; if let Some(i) = tx_id { - let tx = w.tx_log_iter().find(|t| t.id == i); + let tx = w.tx_log_iter()?.find(|t| t.id == i); if let Some(t) = tx { uuid = t.tx_slate_id; } @@ -1010,7 +1010,12 @@ where }), Err(_) => { let outputs = retrieve_outputs(wallet_inst, keychain_mask, &None, true, false, None)?; - let height = outputs.1.iter().map(|m| m.output.height).max().unwrap_or_else(|| 0); + let height = outputs + .1 + .iter() + .map(|m| m.output.height) + .max() + .unwrap_or_else(|| 0); Ok(NodeHeightResult { height, header_hash: "".to_owned(), diff --git a/libwallet/src/backend.rs b/libwallet/src/backend.rs index fac5b3c..a9f37c9 100644 --- a/libwallet/src/backend.rs +++ b/libwallet/src/backend.rs @@ -122,11 +122,10 @@ where /// Create new wallet backend. pub fn new(data_file_dir: &str, n_client: C) -> Result { let db_path = Path::new(data_file_dir).join(DB_DIR); - fs::create_dir_all(&db_path).expect("Couldn't create wallet backend directory!"); + fs::create_dir_all(&db_path)?; let stored_tx_path = Path::new(data_file_dir).join(TX_SAVE_DIR); - fs::create_dir_all(&stored_tx_path) - .expect("Couldn't create wallet backend tx storage directory!"); + fs::create_dir_all(&stored_tx_path)?; let store = Store::new( db_path.to_str().unwrap(), @@ -279,7 +278,7 @@ where /// Set parent key id by stored account name. pub fn set_parent_key_id_by_name(&mut self, label: &str) -> Result<(), Error> { let label = label.to_owned(); - let res = self.acct_path_iter().find(|l| l.label == label); + let res = self.acct_path_iter()?.find(|l| l.label == label); if let Some(a) = res { self.set_parent_key_id(a.path); Ok(()) @@ -312,7 +311,7 @@ where } /// Iterate over all output data stored by the backend. - pub fn iter<'a>(&'a self) -> Box + 'a> { + pub fn iter(&self) -> Result, Error> { let protocol_version = self.db.protocol_version(); let prefix_iter = self.db.iter(Some(OUTPUT_PREFIX), move |_, mut v| { ser::deserialize( @@ -322,12 +321,11 @@ where ) .map_err(From::from) }); - let iter = prefix_iter - .expect("deserialize") + let iter = prefix_iter? .into_iter() .filter(|x| x.is_ok()) .map(|x| x.unwrap()); - Box::new(iter) + Ok(iter) } /// Get an (Optional) tx log entry by uuid. @@ -338,7 +336,7 @@ where } /// Iterate over all tx log data stored by the backend. - pub fn tx_log_iter<'a>(&'a self) -> Box + 'a> { + pub fn tx_log_iter(&self) -> Result, Error> { let protocol_version = self.db.protocol_version(); let prefix_iter = self.db.iter(Some(TX_LOG_ENTRY_PREFIX), move |_, mut v| { ser::deserialize( @@ -348,12 +346,11 @@ where ) .map_err(From::from) }); - let iter = prefix_iter - .expect("deserialize") + let iter = prefix_iter? .into_iter() .filter(|x| x.is_ok()) .map(|x| x.unwrap()); - Box::new(iter) + Ok(iter) } /// Retrieve the private context associated with a given slate id. @@ -381,7 +378,7 @@ where } /// Iterate over all stored account paths. - pub fn acct_path_iter<'a>(&'a self) -> Box + 'a> { + pub fn acct_path_iter(&self) -> Result, Error> { let protocol_version = self.db.protocol_version(); let prefix_iter = self .db @@ -393,12 +390,11 @@ where ) .map_err(From::from) }); - let iter = prefix_iter - .expect("deserialize") + let iter = prefix_iter? .into_iter() .filter(|x| x.is_ok()) .map(|x| x.unwrap()); - Box::new(iter) + Ok(iter) } /// Gets an account path for a given label. @@ -475,10 +471,7 @@ where } /// Next child ID when we want to create a new output, based on current parent. - pub fn next_child( - &mut self, - keychain_mask: Option<&SecretKey>, - ) -> Result { + pub fn next_child(&mut self, keychain_mask: Option<&SecretKey>) -> Result { let parent_key_id = self.parent_key_id.clone(); let mut deriv_idx = { let batch = self.db.batch()?; @@ -581,7 +574,7 @@ where } /// Iterate over all output data stored by the backend. - pub fn iter(&'a self) -> impl Iterator + 'a { + pub fn iter(&'a self) -> Result + 'a, Error> { let protocol_version = self.db.protocol_version(); let prefix_iter = self.db.iter(Some(OUTPUT_PREFIX), move |_, mut v| { ser::deserialize( @@ -591,12 +584,11 @@ where ) .map_err(From::from) }); - let iter = prefix_iter - .expect("deserialize") + let iter = prefix_iter? .into_iter() .filter(|x| x.is_ok()) .map(|x| x.unwrap()); - iter + Ok(iter) } /// Delete data about an output from the backend. @@ -666,7 +658,7 @@ where } /// Iterate over transactions data stored by the backend. - pub fn tx_log_iter(&'a self) -> impl Iterator + 'a { + pub fn tx_log_iter(&'a self) -> Result + 'a, Error> { let protocol_version = self.db.protocol_version(); let prefix_iter = self.db.iter(Some(TX_LOG_ENTRY_PREFIX), move |_, mut v| { ser::deserialize( @@ -676,12 +668,11 @@ where ) .map_err(From::from) }); - let iter = prefix_iter - .expect("deserialize") + let iter = prefix_iter? .into_iter() .filter(|x| x.is_ok()) .map(|x| x.unwrap()); - iter + Ok(iter) } /// Save a transaction log entry. @@ -714,7 +705,7 @@ where } /// Iterate over account names stored in backend. - pub fn acct_path_iter(&'a self) -> impl Iterator + 'a { + pub fn acct_path_iter(&'a self) -> Result + 'a, Error> { let protocol_version = self.db.protocol_version(); let prefix_iter = self .db @@ -726,12 +717,11 @@ where ) .map_err(From::from) }); - let iter = prefix_iter - .expect("deserialize") + let iter = prefix_iter? .into_iter() .filter(|x| x.is_ok()) .map(|x| x.unwrap()); - iter + Ok(iter) } /// Save an output as locked in the backend. diff --git a/libwallet/src/internal/keys.rs b/libwallet/src/internal/keys.rs index 47346a5..21c937a 100644 --- a/libwallet/src/internal/keys.rs +++ b/libwallet/src/internal/keys.rs @@ -54,7 +54,7 @@ where C: NodeClient, K: Keychain, { - Ok(wallet.acct_path_iter().collect()) + Ok(wallet.acct_path_iter()?.collect()) } /// Adds a new parent account path with a given label @@ -68,7 +68,7 @@ where K: Keychain, { let label = label.to_owned(); - if wallet.acct_path_iter().any(|l| l.label == label) { + if wallet.acct_path_iter()?.any(|l| l.label == label) { return Err(Error::AccountLabelAlreadyExists(label)); } @@ -76,7 +76,7 @@ where // so find the highest of those, then increment (to conform with external/internal // derivation chains in BIP32 spec) - let highest_entry = wallet.acct_path_iter().max_by(|a, b| { + let highest_entry = wallet.acct_path_iter()?.max_by(|a, b| { ::from(a.path.to_path().path[0]).cmp(&::from(b.path.to_path().path[0])) }); diff --git a/libwallet/src/internal/scan.rs b/libwallet/src/internal/scan.rs index b8755e6..85f0e9a 100644 --- a/libwallet/src/internal/scan.rs +++ b/libwallet/src/internal/scan.rs @@ -645,7 +645,7 @@ where // restore labels, account paths and child derivation indices wallet_lock!(wallet_inst, w); let label_base = "account"; - let accounts: Vec = w.acct_path_iter().map(|m| m.path).collect(); + let accounts: Vec = w.acct_path_iter()?.map(|m| m.path).collect(); let mut acct_index = accounts.len(); for (path, max_child_index) in found_parents.iter() { // Only restore paths that don't exist diff --git a/libwallet/src/internal/selection.rs b/libwallet/src/internal/selection.rs index 7105ba7..3b420bc 100644 --- a/libwallet/src/internal/selection.rs +++ b/libwallet/src/internal/selection.rs @@ -399,7 +399,7 @@ where max_outputs, selection_strategy_is_use_all, parent_key_id, - ); + )?; // sender is responsible for setting the fee on the partial tx // recipient should double-check the fee calculation and not blindly trust the @@ -464,7 +464,7 @@ where max_outputs, selection_strategy_is_use_all, parent_key_id, - ) + )? .1; fee = tx_fee(coins.len(), num_outputs, 1); total = coins.iter().map(|c| c.value).sum(); @@ -571,7 +571,7 @@ pub fn select_coins( max_outputs: usize, select_all: bool, parent_key_id: &Identifier, -) -> (usize, Vec) +) -> Result<(usize, Vec), Error> // max_outputs_available, Outputs where C: NodeClient, @@ -579,7 +579,7 @@ where { // first find all eligible outputs based on number of confirmations let mut eligible = wallet - .iter() + .iter()? .filter(|out| { out.root_key_id == *parent_key_id && out.eligible_to_spend(current_height, minimum_confirmations) @@ -603,7 +603,7 @@ where for window in eligible.windows(max_outputs) { let windowed_eligibles = window.to_vec(); if let Some(outputs) = select_from(amount, select_all, windowed_eligibles) { - return (max_available, outputs); + return Ok((max_available, outputs)); } } // Not exist in any window of which total amount >= amount. @@ -614,20 +614,20 @@ where "Extending maximum number of outputs. {} outputs selected.", outputs.len() ); - return (max_available, outputs); + return Ok((max_available, outputs)); } } else if let Some(outputs) = select_from(amount, select_all, eligible.clone()) { - return (max_available, outputs); + return Ok((max_available, outputs)); } // we failed to find a suitable set of outputs to spend, // so return the largest amount we can so we can provide guidance on what is // possible eligible.reverse(); - ( + Ok(( max_available, eligible.iter().take(max_outputs).cloned().collect(), - ) + )) } fn select_from(amount: u64, select_all: bool, outputs: Vec) -> Option> { @@ -684,7 +684,7 @@ where let mut parts = vec![]; for (id, _, value) in &context.get_inputs() { - let input = wallet.iter().find(|out| out.key_id == *id); + let input = wallet.iter()?.find(|out| out.key_id == *id); if let Some(i) = input { if i.is_coinbase { parts.push(build::coinbase_input(*value, i.key_id.clone())); @@ -694,7 +694,7 @@ where } } for (id, _, value) in &context.get_outputs() { - let output = wallet.iter().find(|out| out.key_id == *id); + let output = wallet.iter()?.find(|out| out.key_id == *id); if let Some(i) = output { parts.push(build::output(*value, i.key_id.clone())); } diff --git a/libwallet/src/internal/updater.rs b/libwallet/src/internal/updater.rs index 0c9f65c..f7b053d 100644 --- a/libwallet/src/internal/updater.rs +++ b/libwallet/src/internal/updater.rs @@ -52,7 +52,7 @@ where { // just read the wallet here, no need for a write lock let mut outputs = wallet - .iter() + .iter()? .filter(|out| show_spent || out.status != OutputStatus::Spent) .collect::>(); @@ -95,7 +95,7 @@ pub fn apply_advanced_tx_list_filtering( wallet: &mut WalletBackend, parent_key_id: Option<&Identifier>, query_args: &RetrieveTxQueryArgs, -) -> Vec +) -> Result, Error> where C: NodeClient, K: Keychain, @@ -103,7 +103,7 @@ where // Apply simple bool, GTE or LTE fields let txs_iter: Box> = Box::new( wallet - .tx_log_iter() + .tx_log_iter()? .filter(|tx_entry| match parent_key_id { Some(k) => tx_entry.parent_key_id == *k, None => true, @@ -322,7 +322,7 @@ where return_txs = return_txs.into_iter().take(l as usize).collect() } - return_txs + Ok(return_txs) } /// Retrieve all the transaction entries, or a particular entry @@ -343,10 +343,10 @@ where // Adding in new transaction list query logic. If `tx_id` or `tx_slate_id` // is provided, then `query_args` is ignored and old logic is followed. if query_args.is_some() && tx_id.is_none() && tx_slate_id.is_none() { - txs = apply_advanced_tx_list_filtering(wallet, parent_key_id, &query_args.unwrap()) + txs = apply_advanced_tx_list_filtering(wallet, parent_key_id, &query_args.unwrap())? } else { txs = wallet - .tx_log_iter() + .tx_log_iter()? .filter(|tx_entry| { let f_pk = match parent_key_id { Some(k) => tx_entry.parent_key_id == *k, @@ -409,7 +409,7 @@ where let mut wallet_outputs = HashMap::new(); let keychain = wallet.keychain(keychain_mask)?; let unspents: Vec = wallet - .iter() + .iter()? .filter(|x| x.root_key_id == *parent_key_id && x.status != OutputStatus::Spent) .collect(); @@ -547,7 +547,7 @@ where && (output.status == OutputStatus::Unconfirmed || output.status == OutputStatus::Reverted) { - let tx = batch.tx_log_iter().find(|t| { + let tx = batch.tx_log_iter()?.find(|t| { Some(t.id) == output.tx_log_entry && t.parent_key_id == *parent_key_id }); @@ -582,7 +582,7 @@ where } let mut txs_to_save = vec![]; - for mut tx in batch.tx_log_iter() { + for mut tx in batch.tx_log_iter()? { if reverted_kernels.contains(&tx.id) && tx.parent_key_id == *parent_key_id { tx.tx_type = TxLogEntryType::TxReverted; tx.reverted_after = tx.confirmation_ts.clone().and_then(|t| { @@ -672,7 +672,7 @@ where // Get corresponding kernels let kernels = wallet - .tx_log_iter() + .tx_log_iter()? .filter(|t| { ids.contains(&t.id) && t.parent_key_id == *parent_key_id @@ -707,7 +707,7 @@ where return Ok(()); } let mut ids_to_del = vec![]; - for out in wallet.iter() { + for out in wallet.iter()? { if out.status == OutputStatus::Unconfirmed && out.height > 0 && out.height < height - 50 @@ -737,7 +737,7 @@ where { let current_height = wallet.last_confirmed_height()?; let outputs = wallet - .iter() + .iter()? .filter(|out| out.root_key_id == *parent_key_id); let mut unspent_total = 0;