From acfc88d9a2c29c1b98ce9886f6d5b27871cfb1f6 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Thu, 29 Aug 2024 23:49:25 +0000 Subject: [PATCH 1/7] simplified add_pending_note_parameters --- zingolib/src/wallet/transaction_context.rs | 7 ++--- .../src/wallet/transaction_records_by_id.rs | 27 +++++++++++-------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/zingolib/src/wallet/transaction_context.rs b/zingolib/src/wallet/transaction_context.rs index de526a7e6..599c4822b 100644 --- a/zingolib/src/wallet/transaction_context.rs +++ b/zingolib/src/wallet/transaction_context.rs @@ -414,6 +414,8 @@ mod decrypt_transaction { .await; } + /// account here is a verb meaning record note data + /// and perform some other appropriate actions async fn account_for_shielded_receipts( &self, ivk: Ivk, @@ -446,19 +448,18 @@ mod decrypt_transaction { let memo_bytes = MemoBytes::from_bytes(&memo_bytes.to_bytes()).unwrap(); // if status is pending add the whole pending note // otherwise, just update the output index - if let Some(height) = status.get_pending_height() { + if status.is_pending() { self.transaction_metadata_set .write() .await .transaction_records_by_id .add_pending_note::( transaction.txid(), - height, block_time, note.clone(), to, output_index, - status.is_mempool(), + status, ); } else { self.transaction_metadata_set diff --git a/zingolib/src/wallet/transaction_records_by_id.rs b/zingolib/src/wallet/transaction_records_by_id.rs index 6aa83d028..52ac1545b 100644 --- a/zingolib/src/wallet/transaction_records_by_id.rs +++ b/zingolib/src/wallet/transaction_records_by_id.rs @@ -606,23 +606,27 @@ impl TransactionRecordsById { } } } + + /// adds a note. however, does not fully commit to adding a note, because this note isnt chained into block + /// if the transaction is not already recorded, return Err(()) pub(crate) fn add_pending_note( &mut self, txid: TxId, - height: BlockHeight, - timestamp: Option, note: D::Note, to: D::Recipient, output_index: usize, - in_mempool: bool, - ) { - let status = if in_mempool { - ConfirmationStatus::Mempool(height) - } else { - ConfirmationStatus::Transmitted(height) - }; - let transaction_record = - self.create_modify_get_transaction_metadata(&txid, status, timestamp); + status: ConfirmationStatus, + ) -> Result<(), ()> { + if status.is_confirmed() { + // this could be a complex error variant... but i suspect it will disappear before i get a chance to write it + return Err(()); + } + + let has_transaction = self.get_mut(&txid); + let transaction_record = has_transaction.ok_or(())?; + + // let transaction_record = + // self.create_modify_get_transaction_metadata(&txid, status, timestamp); match D::WalletNote::get_record_outputs(transaction_record) .iter_mut() @@ -646,6 +650,7 @@ impl TransactionRecordsById { } Some(_) => {} } + Ok(()) } #[allow(clippy::too_many_arguments)] pub(crate) fn add_new_note( From 12ba778b4cb4348ad09e85e69af9729e864958f8 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Fri, 30 Aug 2024 00:18:52 +0000 Subject: [PATCH 2/7] moved create_modify_get_transaction_metadata out of one arm of conditional --- zingolib/src/wallet/transaction_context.rs | 25 ++++++++++++------- .../src/wallet/transaction_records_by_id.rs | 9 ------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/zingolib/src/wallet/transaction_context.rs b/zingolib/src/wallet/transaction_context.rs index 599c4822b..0c3e78fc1 100644 --- a/zingolib/src/wallet/transaction_context.rs +++ b/zingolib/src/wallet/transaction_context.rs @@ -448,19 +448,26 @@ mod decrypt_transaction { let memo_bytes = MemoBytes::from_bytes(&memo_bytes.to_bytes()).unwrap(); // if status is pending add the whole pending note // otherwise, just update the output index + + let tx_map = &mut self + .transaction_metadata_set + .write() + .await + .transaction_records_by_id; + + let _transaction_record = tx_map.create_modify_get_transaction_metadata( + &transaction.txid(), + status, + block_time, + ); + if status.is_pending() { - self.transaction_metadata_set + let _todo_error_stack = self + .transaction_metadata_set .write() .await .transaction_records_by_id - .add_pending_note::( - transaction.txid(), - block_time, - note.clone(), - to, - output_index, - status, - ); + .add_pending_note::(transaction.txid(), note.clone(), to, output_index); } else { self.transaction_metadata_set .write() diff --git a/zingolib/src/wallet/transaction_records_by_id.rs b/zingolib/src/wallet/transaction_records_by_id.rs index 52ac1545b..cd60cc553 100644 --- a/zingolib/src/wallet/transaction_records_by_id.rs +++ b/zingolib/src/wallet/transaction_records_by_id.rs @@ -615,19 +615,10 @@ impl TransactionRecordsById { note: D::Note, to: D::Recipient, output_index: usize, - status: ConfirmationStatus, ) -> Result<(), ()> { - if status.is_confirmed() { - // this could be a complex error variant... but i suspect it will disappear before i get a chance to write it - return Err(()); - } - let has_transaction = self.get_mut(&txid); let transaction_record = has_transaction.ok_or(())?; - // let transaction_record = - // self.create_modify_get_transaction_metadata(&txid, status, timestamp); - match D::WalletNote::get_record_outputs(transaction_record) .iter_mut() .find(|n| n.note() == ¬e) From 69425dc689e674eb48146b18cbb4562b54b858a1 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Fri, 30 Aug 2024 00:28:17 +0000 Subject: [PATCH 3/7] simplified --- zingolib/src/wallet/transaction_context.rs | 31 +++++++++---------- .../src/wallet/transaction_records_by_id.rs | 13 +++++--- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/zingolib/src/wallet/transaction_context.rs b/zingolib/src/wallet/transaction_context.rs index 0c3e78fc1..d29d597f8 100644 --- a/zingolib/src/wallet/transaction_context.rs +++ b/zingolib/src/wallet/transaction_context.rs @@ -461,26 +461,23 @@ mod decrypt_transaction { block_time, ); + // now that the transaction exists, add_pending_note or update_output_index will succeed _todo_error_stack is not to be handled. + if status.is_pending() { - let _todo_error_stack = self - .transaction_metadata_set - .write() - .await - .transaction_records_by_id - .add_pending_note::(transaction.txid(), note.clone(), to, output_index); + let _todo_error_stack = tx_map.add_pending_note::( + transaction.txid(), + note.clone(), + to, + output_index, + ); } else { - self.transaction_metadata_set - .write() - .await - .transaction_records_by_id - .update_output_index::( - transaction.txid(), - status, - block_time, - note.clone(), - output_index, - ) + let _todo_error_stack = tx_map.update_output_index::( + transaction.txid(), + note.clone(), + output_index, + ); } + let memo = memo_bytes .clone() .try_into() diff --git a/zingolib/src/wallet/transaction_records_by_id.rs b/zingolib/src/wallet/transaction_records_by_id.rs index cd60cc553..ce4a06500 100644 --- a/zingolib/src/wallet/transaction_records_by_id.rs +++ b/zingolib/src/wallet/transaction_records_by_id.rs @@ -586,16 +586,16 @@ impl TransactionRecordsById { ); } } + + /// returns Err(()) if transaction or note does not exist pub(crate) fn update_output_index( &mut self, txid: TxId, - status: ConfirmationStatus, - timestamp: Option, note: D::Note, output_index: usize, - ) { - let transaction_record = - self.create_modify_get_transaction_metadata(&txid, status, timestamp); + ) -> Result<(), ()> { + let has_transaction = self.get_mut(&txid); + let transaction_record = has_transaction.ok_or(())?; if let Some(n) = D::WalletNote::transaction_metadata_notes_mut(transaction_record) .iter_mut() @@ -604,6 +604,9 @@ impl TransactionRecordsById { if n.output_index().is_none() { *n.output_index_mut() = Some(output_index as u32) } + Ok(()) + } else { + Err(()) } } From b225a4fbb2aa907a45726d7375ee80f3976f5b96 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Fri, 30 Aug 2024 00:34:09 +0000 Subject: [PATCH 4/7] moved add_pending_note to transaction_record --- zingolib/src/wallet/transaction_context.rs | 9 ++--- zingolib/src/wallet/transaction_record.rs | 35 +++++++++++++++++- .../src/wallet/transaction_records_by_id.rs | 36 ------------------- 3 files changed, 36 insertions(+), 44 deletions(-) diff --git a/zingolib/src/wallet/transaction_context.rs b/zingolib/src/wallet/transaction_context.rs index d29d597f8..3b31b2dd8 100644 --- a/zingolib/src/wallet/transaction_context.rs +++ b/zingolib/src/wallet/transaction_context.rs @@ -455,7 +455,7 @@ mod decrypt_transaction { .await .transaction_records_by_id; - let _transaction_record = tx_map.create_modify_get_transaction_metadata( + let transaction_record = tx_map.create_modify_get_transaction_metadata( &transaction.txid(), status, block_time, @@ -464,12 +464,7 @@ mod decrypt_transaction { // now that the transaction exists, add_pending_note or update_output_index will succeed _todo_error_stack is not to be handled. if status.is_pending() { - let _todo_error_stack = tx_map.add_pending_note::( - transaction.txid(), - note.clone(), - to, - output_index, - ); + transaction_record.add_pending_note::(note.clone(), to, output_index); } else { let _todo_error_stack = tx_map.update_output_index::( transaction.txid(), diff --git a/zingolib/src/wallet/transaction_record.rs b/zingolib/src/wallet/transaction_record.rs index bb0d723e7..79f674a72 100644 --- a/zingolib/src/wallet/transaction_record.rs +++ b/zingolib/src/wallet/transaction_record.rs @@ -15,6 +15,7 @@ use zcash_client_backend::{ }; use zcash_primitives::{consensus::BlockHeight, transaction::TxId}; +use crate::wallet::traits::Recipient as _; use crate::{ error::ZingoLibError, wallet::{ @@ -72,6 +73,8 @@ pub struct TransactionRecord { pub price: Option, } +// much data assignment of this struct is done through the pub fields as of january 2024. Todo: should have private fields and public methods. + // set impl TransactionRecord { /// TODO: Add Doc Comment Here! @@ -110,7 +113,37 @@ impl TransactionRecord { } } } - // much data assignment of this struct is done through the pub fields as of january 2024. Todo: should have private fields and public methods. + /// adds a note. however, does not fully commit to adding a note, because this note isnt chained into block + /// if the transaction is not already recorded, return Err(()) + pub(crate) fn add_pending_note( + &mut self, + note: D::Note, + to: D::Recipient, + output_index: usize, + ) { + match D::WalletNote::get_record_outputs(self) + .iter_mut() + .find(|n| n.note() == ¬e) + { + None => { + let nd = D::WalletNote::from_parts( + to.diversifier(), + note, + None, + None, + None, + None, + // if this is change, we'll mark it later in check_notes_mark_change + false, + false, + Some(output_index as u32), + ); + + D::WalletNote::transaction_metadata_notes_mut(self).push(nd); + } + Some(_) => {} + } + } } //get impl TransactionRecord { diff --git a/zingolib/src/wallet/transaction_records_by_id.rs b/zingolib/src/wallet/transaction_records_by_id.rs index ce4a06500..0e2452f21 100644 --- a/zingolib/src/wallet/transaction_records_by_id.rs +++ b/zingolib/src/wallet/transaction_records_by_id.rs @@ -610,42 +610,6 @@ impl TransactionRecordsById { } } - /// adds a note. however, does not fully commit to adding a note, because this note isnt chained into block - /// if the transaction is not already recorded, return Err(()) - pub(crate) fn add_pending_note( - &mut self, - txid: TxId, - note: D::Note, - to: D::Recipient, - output_index: usize, - ) -> Result<(), ()> { - let has_transaction = self.get_mut(&txid); - let transaction_record = has_transaction.ok_or(())?; - - match D::WalletNote::get_record_outputs(transaction_record) - .iter_mut() - .find(|n| n.note() == ¬e) - { - None => { - let nd = D::WalletNote::from_parts( - to.diversifier(), - note, - None, - None, - None, - None, - // if this is change, we'll mark it later in check_notes_mark_change - false, - false, - Some(output_index as u32), - ); - - D::WalletNote::transaction_metadata_notes_mut(transaction_record).push(nd); - } - Some(_) => {} - } - Ok(()) - } #[allow(clippy::too_many_arguments)] pub(crate) fn add_new_note( &mut self, From f39d867e6d2d7341c1d623ce412a316263a825b8 Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Fri, 30 Aug 2024 02:13:45 +0000 Subject: [PATCH 5/7] moved update_output_index to transaction_record --- zingolib/src/wallet/transaction_context.rs | 9 ++------ zingolib/src/wallet/transaction_record.rs | 21 ++++++++++++++++- .../src/wallet/transaction_records_by_id.rs | 23 ------------------- 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/zingolib/src/wallet/transaction_context.rs b/zingolib/src/wallet/transaction_context.rs index 3b31b2dd8..f8100ed86 100644 --- a/zingolib/src/wallet/transaction_context.rs +++ b/zingolib/src/wallet/transaction_context.rs @@ -461,16 +461,11 @@ mod decrypt_transaction { block_time, ); - // now that the transaction exists, add_pending_note or update_output_index will succeed _todo_error_stack is not to be handled. - if status.is_pending() { transaction_record.add_pending_note::(note.clone(), to, output_index); } else { - let _todo_error_stack = tx_map.update_output_index::( - transaction.txid(), - note.clone(), - output_index, - ); + let _note_does_not_exist_result = + transaction_record.update_output_index::(note.clone(), output_index); } let memo = memo_bytes diff --git a/zingolib/src/wallet/transaction_record.rs b/zingolib/src/wallet/transaction_record.rs index 79f674a72..735b45543 100644 --- a/zingolib/src/wallet/transaction_record.rs +++ b/zingolib/src/wallet/transaction_record.rs @@ -113,8 +113,8 @@ impl TransactionRecord { } } } + /// adds a note. however, does not fully commit to adding a note, because this note isnt chained into block - /// if the transaction is not already recorded, return Err(()) pub(crate) fn add_pending_note( &mut self, note: D::Note, @@ -144,6 +144,25 @@ impl TransactionRecord { Some(_) => {} } } + + /// returns Err(()) if note does not exist + pub(crate) fn update_output_index( + &mut self, + note: D::Note, + output_index: usize, + ) -> Result<(), ()> { + if let Some(n) = D::WalletNote::transaction_metadata_notes_mut(self) + .iter_mut() + .find(|n| n.note() == ¬e) + { + if n.output_index().is_none() { + *n.output_index_mut() = Some(output_index as u32) + } + Ok(()) + } else { + Err(()) + } + } } //get impl TransactionRecord { diff --git a/zingolib/src/wallet/transaction_records_by_id.rs b/zingolib/src/wallet/transaction_records_by_id.rs index 0e2452f21..edbe1c584 100644 --- a/zingolib/src/wallet/transaction_records_by_id.rs +++ b/zingolib/src/wallet/transaction_records_by_id.rs @@ -587,29 +587,6 @@ impl TransactionRecordsById { } } - /// returns Err(()) if transaction or note does not exist - pub(crate) fn update_output_index( - &mut self, - txid: TxId, - note: D::Note, - output_index: usize, - ) -> Result<(), ()> { - let has_transaction = self.get_mut(&txid); - let transaction_record = has_transaction.ok_or(())?; - - if let Some(n) = D::WalletNote::transaction_metadata_notes_mut(transaction_record) - .iter_mut() - .find(|n| n.note() == ¬e) - { - if n.output_index().is_none() { - *n.output_index_mut() = Some(output_index as u32) - } - Ok(()) - } else { - Err(()) - } - } - #[allow(clippy::too_many_arguments)] pub(crate) fn add_new_note( &mut self, From ad6cc01bfebf94d0c16512a4986d4bf873b1305e Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Mon, 2 Sep 2024 15:52:17 +0000 Subject: [PATCH 6/7] re-used write lock --- zingolib/src/wallet/transaction_context.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/zingolib/src/wallet/transaction_context.rs b/zingolib/src/wallet/transaction_context.rs index f8100ed86..890740bcd 100644 --- a/zingolib/src/wallet/transaction_context.rs +++ b/zingolib/src/wallet/transaction_context.rs @@ -484,11 +484,7 @@ mod decrypt_transaction { } } } - self.transaction_metadata_set - .write() - .await - .transaction_records_by_id - .add_memo_to_note_metadata::(&transaction.txid(), note, memo); + tx_map.add_memo_to_note_metadata::(&transaction.txid(), note, memo); } } /// Transactions contain per-protocol "bundles" of components. From 2fc9b22652d4b7475be2b9ef93d4930a404f92dc Mon Sep 17 00:00:00 2001 From: fluidvanadium Date: Mon, 2 Sep 2024 20:54:16 +0000 Subject: [PATCH 7/7] fix non-CI test --- libtonode-tests/tests/concrete.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libtonode-tests/tests/concrete.rs b/libtonode-tests/tests/concrete.rs index 7e68f6796..5f85df3c1 100644 --- a/libtonode-tests/tests/concrete.rs +++ b/libtonode-tests/tests/concrete.rs @@ -1208,7 +1208,7 @@ mod slow { let first_send_to_transparent = 20_000; let summary_external_transparent = TransactionSummaryBuilder::new() .blockheight(BlockHeight::from_u32(7)) - .status(ConfirmationStatus::Mempool(BlockHeight::from_u32(7))) + .status(ConfirmationStatus::Transmitted(BlockHeight::from_u32(7))) .datetime(0) .txid(utils::conversion::txid_from_hex_encoded_str(TEST_TXID).unwrap()) .value(first_send_to_transparent)