From cfb72b03b08bdb3112c06da303ebf352df388000 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Wed, 19 Jun 2024 15:28:59 +0000 Subject: [PATCH 1/3] get_spendable_note_and_values now handles and reports an error case tied through error handling --- zingolib/src/error.rs | 6 +++++ zingolib/src/wallet/transaction_record.rs | 21 +++++++++++++---- .../src/wallet/transaction_records_by_id.rs | 23 +++++++++++++++---- .../trait_inputsource.rs | 7 ++++++ 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/zingolib/src/error.rs b/zingolib/src/error.rs index 0d50a2eae..ceea2316a 100644 --- a/zingolib/src/error.rs +++ b/zingolib/src/error.rs @@ -35,6 +35,8 @@ pub enum ZingoLibError { /// TODO: Add Doc Comment Here! MissingOutputIndex(TxId), /// TODO: Add Doc Comment Here! + MissingOutputIndexInThisTx, + /// TODO: Add Doc Comment Here! CouldNotDecodeMemo(std::io::Error), } @@ -121,6 +123,10 @@ impl std::fmt::Display for ZingoLibError { f, "{txid} is missing output_index for note, cannot mark change" ), + MissingOutputIndexInThisTx => write!( + f, + "this tx is missing output_index for note, cannot mark change" + ), } } } diff --git a/zingolib/src/wallet/transaction_record.rs b/zingolib/src/wallet/transaction_record.rs index a3b0e8d3a..0a29c142e 100644 --- a/zingolib/src/wallet/transaction_record.rs +++ b/zingolib/src/wallet/transaction_record.rs @@ -2,7 +2,10 @@ //! conspicuously absent is the set of transparent inputs to the transaction. //! by its`nature this evolves through, different states of completeness. -use crate::wallet::notes::{interface::OutputConstructor, OutputId}; +use crate::{ + error::ZingoLibResult, + wallet::notes::{interface::OutputConstructor, OutputId}, +}; use std::io::{self, Read, Write}; use byteorder::{LittleEndian, ReadBytesExt as _, WriteBytesExt as _}; @@ -368,8 +371,9 @@ impl TransactionRecord { &self, sources: &[zcash_client_backend::ShieldedProtocol], exclude: &[NoteId], - ) -> Vec<(NoteId, u64)> { + ) -> ZingoLibResult> { let mut all = vec![]; + let mut missing_output_index = false; if sources.contains(&Sapling) { self.sapling_notes.iter().for_each(|zingo_sapling_note| { if zingo_sapling_note.is_unspent() { @@ -380,6 +384,7 @@ impl TransactionRecord { } } else { println!("note has no index"); + missing_output_index = true; } } }); @@ -394,11 +399,16 @@ impl TransactionRecord { } } else { println!("note has no index"); + missing_output_index = true; } } }); } - all + if missing_output_index { + Err(ZingoLibError::MissingOutputIndexInThisTx) + } else { + Ok(all) + } } } // read/write @@ -979,8 +989,9 @@ mod tests { fn select_spendable_note_ids_and_values() { let transaction_record = nine_note_transaction_record_default(); - let unspent_ids_and_values = - transaction_record.get_spendable_note_ids_and_values(&[Sapling, Orchard], &[]); + let unspent_ids_and_values = transaction_record + .get_spendable_note_ids_and_values(&[Sapling, Orchard], &[]) + .unwrap(); assert_eq!( unspent_ids_and_values, diff --git a/zingolib/src/wallet/transaction_records_by_id.rs b/zingolib/src/wallet/transaction_records_by_id.rs index a93c33fa5..ff20708d6 100644 --- a/zingolib/src/wallet/transaction_records_by_id.rs +++ b/zingolib/src/wallet/transaction_records_by_id.rs @@ -1,6 +1,7 @@ //! The lookup for transaction id indexed data. Currently this provides the //! transaction record. +use crate::error::{ZingoLibError, ZingoLibResult}; use crate::wallet::notes::interface::OutputConstructor; use crate::wallet::{ error::FeeError, @@ -668,19 +669,33 @@ impl TransactionRecordsById { sources: &[zcash_client_backend::ShieldedProtocol], anchor_height: zcash_primitives::consensus::BlockHeight, exclude: &[NoteId], - ) -> Vec<(NoteId, u64)> { - self.values() + ) -> Result, Vec> { + let mut missing_output_index = vec![]; + let ok = self + .values() .flat_map(|transaction_record| { if transaction_record .status .is_confirmed_before_or_at(&anchor_height) { - transaction_record.get_spendable_note_ids_and_values(sources, exclude) + if let Ok(lalala) = + transaction_record.get_spendable_note_ids_and_values(sources, exclude) + { + lalala + } else { + missing_output_index.push(transaction_record.txid); + vec![] + } } else { vec![] } }) - .collect() + .collect(); + if missing_output_index.is_empty() { + Ok(ok) + } else { + Err(missing_output_index) + } } } diff --git a/zingolib/src/wallet/transaction_records_by_id/trait_inputsource.rs b/zingolib/src/wallet/transaction_records_by_id/trait_inputsource.rs index 6ed989877..981e54562 100644 --- a/zingolib/src/wallet/transaction_records_by_id/trait_inputsource.rs +++ b/zingolib/src/wallet/transaction_records_by_id/trait_inputsource.rs @@ -12,6 +12,7 @@ use zcash_primitives::{ transaction::{ components::{amount::NonNegativeAmount, TxOut}, fees::zip317::MARGINAL_FEE, + TxId, }, }; @@ -36,6 +37,11 @@ pub enum InputSourceError { /// Value outside the valid range of zatoshis #[error("Value outside valid range of zatoshis. {0:?}")] InvalidValue(BalanceError), + /// Wallet data is out of date + #[error( + "Output index data is missing! Wallet data is from an ancient version, please rescan." + )] + MissingOutputIndexes(Vec), } // Calculate remaining difference between target and selected. @@ -191,6 +197,7 @@ impl InputSource for TransactionRecordsById { ) -> Result, Self::Error> { let mut unselected = self .get_spendable_note_ids_and_values(sources, anchor_height, exclude) + .map_err(InputSourceError::MissingOutputIndexes)? .into_iter() .map(|(id, value)| NonNegativeAmount::from_u64(value).map(|value| (id, value))) .collect::, _>>() From 9ef81479bfeb73ad72ba938d48c1c31647df6624 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Wed, 19 Jun 2024 16:28:53 +0000 Subject: [PATCH 2/3] cargo hooks, variable names, remove redundant enum --- zingolib/src/error.rs | 6 ------ zingolib/src/wallet/transaction_record.rs | 9 +++------ zingolib/src/wallet/transaction_records_by_id.rs | 5 ++--- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/zingolib/src/error.rs b/zingolib/src/error.rs index ceea2316a..0d50a2eae 100644 --- a/zingolib/src/error.rs +++ b/zingolib/src/error.rs @@ -35,8 +35,6 @@ pub enum ZingoLibError { /// TODO: Add Doc Comment Here! MissingOutputIndex(TxId), /// TODO: Add Doc Comment Here! - MissingOutputIndexInThisTx, - /// TODO: Add Doc Comment Here! CouldNotDecodeMemo(std::io::Error), } @@ -123,10 +121,6 @@ impl std::fmt::Display for ZingoLibError { f, "{txid} is missing output_index for note, cannot mark change" ), - MissingOutputIndexInThisTx => write!( - f, - "this tx is missing output_index for note, cannot mark change" - ), } } } diff --git a/zingolib/src/wallet/transaction_record.rs b/zingolib/src/wallet/transaction_record.rs index 0a29c142e..222375bf9 100644 --- a/zingolib/src/wallet/transaction_record.rs +++ b/zingolib/src/wallet/transaction_record.rs @@ -2,10 +2,7 @@ //! conspicuously absent is the set of transparent inputs to the transaction. //! by its`nature this evolves through, different states of completeness. -use crate::{ - error::ZingoLibResult, - wallet::notes::{interface::OutputConstructor, OutputId}, -}; +use crate::wallet::notes::{interface::OutputConstructor, OutputId}; use std::io::{self, Read, Write}; use byteorder::{LittleEndian, ReadBytesExt as _, WriteBytesExt as _}; @@ -371,7 +368,7 @@ impl TransactionRecord { &self, sources: &[zcash_client_backend::ShieldedProtocol], exclude: &[NoteId], - ) -> ZingoLibResult> { + ) -> Result, ()> { let mut all = vec![]; let mut missing_output_index = false; if sources.contains(&Sapling) { @@ -405,7 +402,7 @@ impl TransactionRecord { }); } if missing_output_index { - Err(ZingoLibError::MissingOutputIndexInThisTx) + Err(()) } else { Ok(all) } diff --git a/zingolib/src/wallet/transaction_records_by_id.rs b/zingolib/src/wallet/transaction_records_by_id.rs index ff20708d6..2179874a6 100644 --- a/zingolib/src/wallet/transaction_records_by_id.rs +++ b/zingolib/src/wallet/transaction_records_by_id.rs @@ -1,7 +1,6 @@ //! The lookup for transaction id indexed data. Currently this provides the //! transaction record. -use crate::error::{ZingoLibError, ZingoLibResult}; use crate::wallet::notes::interface::OutputConstructor; use crate::wallet::{ error::FeeError, @@ -678,10 +677,10 @@ impl TransactionRecordsById { .status .is_confirmed_before_or_at(&anchor_height) { - if let Ok(lalala) = + if let Ok(notes_from_tx) = transaction_record.get_spendable_note_ids_and_values(sources, exclude) { - lalala + notes_from_tx } else { missing_output_index.push(transaction_record.txid); vec![] From 1ea78b78d82ddadf4e411240a8a8cffb83d7dadb Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Wed, 19 Jun 2024 19:34:27 +0000 Subject: [PATCH 3/3] addressed review comment --- .../src/wallet/transaction_records_by_id/trait_inputsource.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/zingolib/src/wallet/transaction_records_by_id/trait_inputsource.rs b/zingolib/src/wallet/transaction_records_by_id/trait_inputsource.rs index 981e54562..7f5103333 100644 --- a/zingolib/src/wallet/transaction_records_by_id/trait_inputsource.rs +++ b/zingolib/src/wallet/transaction_records_by_id/trait_inputsource.rs @@ -38,9 +38,7 @@ pub enum InputSourceError { #[error("Value outside valid range of zatoshis. {0:?}")] InvalidValue(BalanceError), /// Wallet data is out of date - #[error( - "Output index data is missing! Wallet data is from an ancient version, please rescan." - )] + #[error("Output index data is missing! Wallet data is out of date, please rescan.")] MissingOutputIndexes(Vec), }