Merge pull request zingolabs#1351 from fluidvanadium/dry_pending

Simplify pending
juanky201271 · Sep 3, 2024 · f3c51ec · f3c51ec
2 parents df09a3d + 2fc9b22
commit f3c51ec
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 92 deletions.
diff --git 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)

diff --git 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<D>(
             &self,
             ivk: Ivk<D, External>,
@@ -446,33 +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
-                if let Some(height) = status.get_pending_height() {
-                    self.transaction_metadata_set
-                        .write()
-                        .await
-                        .transaction_records_by_id
-                        .add_pending_note::<D>(
-                            transaction.txid(),
-                            height,
-                            block_time,
-                            note.clone(),
-                            to,
-                            output_index,
-                            status.is_mempool(),
-                        );
+
+                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() {
+                    transaction_record.add_pending_note::<D>(note.clone(), to, output_index);
                 } else {
-                    self.transaction_metadata_set
-                        .write()
-                        .await
-                        .transaction_records_by_id
-                        .update_output_index::<D>(
-                            transaction.txid(),
-                            status,
-                            block_time,
-                            note.clone(),
-                            output_index,
-                        )
+                    let _note_does_not_exist_result =
+                        transaction_record.update_output_index::<D>(note.clone(), output_index);
                 }
+
                 let memo = memo_bytes
                     .clone()
                     .try_into()
@@ -489,11 +484,7 @@ mod decrypt_transaction {
                         }
                     }
                 }
-                self.transaction_metadata_set
-                    .write()
-                    .await
-                    .transaction_records_by_id
-                    .add_memo_to_note_metadata::<D::WalletNote>(&transaction.txid(), note, memo);
+                tx_map.add_memo_to_note_metadata::<D::WalletNote>(&transaction.txid(), note, memo);
             }
         }
         /// Transactions contain per-protocol "bundles" of components.

diff --git 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<f64>,
 }
 
+// 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,56 @@ 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
+    pub(crate) fn add_pending_note<D: DomainWalletExt>(
+        &mut self,
+        note: D::Note,
+        to: D::Recipient,
+        output_index: usize,
+    ) {
+        match D::WalletNote::get_record_outputs(self)
+            .iter_mut()
+            .find(|n| n.note() == &note)
+        {
+            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(_) => {}
+        }
+    }
+
+    /// returns Err(()) if note does not exist
+    pub(crate) fn update_output_index<D: DomainWalletExt>(
+        &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() == &note)
+        {
+            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
@@ -586,67 +586,7 @@ impl TransactionRecordsById {
             );
         }
     }
-    pub(crate) fn update_output_index<D: DomainWalletExt>(
-        &mut self,
-        txid: TxId,
-        status: ConfirmationStatus,
-        timestamp: Option<u32>,
-        note: D::Note,
-        output_index: usize,
-    ) {
-        let transaction_record =
-            self.create_modify_get_transaction_metadata(&txid, status, timestamp);
 
-        if let Some(n) = D::WalletNote::transaction_metadata_notes_mut(transaction_record)
-            .iter_mut()
-            .find(|n| n.note() == &note)
-        {
-            if n.output_index().is_none() {
-                *n.output_index_mut() = Some(output_index as u32)
-            }
-        }
-    }
-    pub(crate) fn add_pending_note<D: DomainWalletExt>(
-        &mut self,
-        txid: TxId,
-        height: BlockHeight,
-        timestamp: Option<u32>,
-        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);
-
-        match D::WalletNote::get_record_outputs(transaction_record)
-            .iter_mut()
-            .find(|n| n.note() == &note)
-        {
-            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(_) => {}
-        }
-    }
     #[allow(clippy::too_many_arguments)]
     pub(crate) fn add_new_note<D: DomainWalletExt>(
         &mut self,