Merge pull request #1132 from zancas/fee_calc_from_foundational_truths_2

Fee calc from foundational truths 2

zingolib/src/wallet/data.rs

@@ -275,16 +275,17 @@ pub(crate) fn write_sapling_rseed<W: Write>(
     }
 }
 
-/// TODO: Add Doc Comment Here!
-#[derive(Debug)]
+/// Only for TransactionRecords *from* "this" capability
+#[derive(Clone, Debug)]
 pub struct OutgoingTxData {
     /// TODO: Add Doc Comment Here!
     pub to_address: String,
-    /// TODO: Add Doc Comment Here!
+    /// Amount to this receiver
     pub value: u64,
-    /// TODO: Add Doc Comment Here!
+    /// Note to the receiver, why not an option?
     pub memo: Memo,
-    /// TODO: Add Doc Comment Here!
+    /// What if it wasn't provided?  How does this relate to
+    /// to_address?
     pub recipient_ua: Option<String>,
 }
 

zingolib/src/wallet/error.rs

@@ -2,25 +2,31 @@
 
 use std::fmt;
 
+use crate::wallet::data::OutgoingTxData;
+
 /// Errors associated with calculating transaction fees
 #[derive(Debug)]
 pub enum FeeError {
-    /// Notes spent in a transaction not found in the wallet
-    SpendNotFound,
+    /// Sapling notes spent in a transaction not found in the wallet
+    SaplingSpendNotFound(sapling_crypto::Nullifier),
+    /// Orchard notes spent in a transaction not found in the wallet
+    OrchardSpendNotFound(orchard::note::Nullifier),
     /// Attempted to calculate a fee for a transaction received and not created by the wallet's spend capability
     ReceivedTransaction,
-    /// Total output value is larger than total spend value causing the unsigned integer to underflow
-    FeeUnderflow,
+    /// Outgoing tx data, but no spends found!
+    OutgoingWithoutSpends(Vec<OutgoingTxData>),
+    /// Total explicit receiver value is larger than input value causing the unsigned integer to underflow
+    FeeUnderflow((u64, u64)),
 }
 
 impl fmt::Display for FeeError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use FeeError::*;
-
         match self {
-            SpendNotFound => write!(f, "spend(s) for this transaction not found in wallet. check wallet is fully synced."),
-            ReceivedTransaction => write!(f, "no spends or outgoing transaction data found, indicating this transaction was received and not sent by this capability. check wallet is fully synced."),
-            FeeUnderflow => write!(f, "total output value is larger than total spend value indicating transparent spends not found in the wallet. check wallet is fully synced."),
+            FeeError::OrchardSpendNotFound(n) => write!(f, "Orchard nullifier(s) {:?} for this transaction not found in wallet. Is the wallet fully synced?", n),
+            FeeError::SaplingSpendNotFound(n) => write!(f, "Sapling nullifier(s) {:?} for this transaction not found in wallet. Is the wallet fully synced?", n),
+            FeeError::ReceivedTransaction => write!(f, "No inputs or outgoing transaction data found, indicating this transaction was received and not sent by this capability"),
+            FeeError::FeeUnderflow((total_in, total_out)) => write!(f, "Output value: {} is larger than total input value: {} Is the wallet fully synced?", total_out, total_in),
+            FeeError::OutgoingWithoutSpends(ov) =>  write!(f, "No inputs funded this transaction, but it has outgoing data! Is the wallet fully synced? {:?}", ov),
         }
     }
 }

zingolib/src/wallet/transaction_record.rs

@@ -732,6 +732,49 @@ pub mod mocks {
             10_000, 20_000, 30_000, 40_000, 50_000, 60_000, 70_000, 80_000, 90_000,
         )
     }
+    #[test]
+    fn check_nullifier_indices() {
+        let sap_null_one = SaplingNullifierBuilder::new()
+            .assign_unique_nullifier()
+            .clone();
+        let sap_null_two = SaplingNullifierBuilder::new()
+            .assign_unique_nullifier()
+            .clone();
+        let orch_null_one = OrchardNullifierBuilder::new()
+            .assign_unique_nullifier()
+            .clone();
+        let orch_null_two = OrchardNullifierBuilder::new()
+            .assign_unique_nullifier()
+            .clone();
+        let sent_transaction_record = TransactionRecordBuilder::default()
+            .status(ConfirmationStatus::Confirmed(15.into()))
+            .spent_sapling_nullifiers(sap_null_one.clone())
+            .spent_sapling_nullifiers(sap_null_two.clone())
+            .spent_orchard_nullifiers(orch_null_one.clone())
+            .spent_orchard_nullifiers(orch_null_two.clone())
+            .transparent_outputs(TransparentOutputBuilder::default())
+            .sapling_notes(SaplingNoteBuilder::default())
+            .orchard_notes(OrchardNoteBuilder::default())
+            .total_transparent_value_spent(30_000)
+            .outgoing_tx_data(OutgoingTxDataBuilder::default())
+            .build();
+        assert_eq!(
+            sent_transaction_record.spent_sapling_nullifiers[0],
+            sap_null_one.build()
+        );
+        assert_eq!(
+            sent_transaction_record.spent_sapling_nullifiers[1],
+            sap_null_two.build()
+        );
+        assert_eq!(
+            sent_transaction_record.spent_orchard_nullifiers[0],
+            orch_null_one.build()
+        );
+        assert_eq!(
+            sent_transaction_record.spent_orchard_nullifiers[1],
+            orch_null_two.build()
+        );
+    }
 }
 
 #[cfg(test)]

zingolib/src/wallet/transaction_records_by_id.rs

@@ -211,95 +211,124 @@ impl TransactionRecordsById {
         });
     }
 
-    /// Calculate the fee for a transaction in the wallet
-    ///
-    /// # Error
-    ///
-    /// Returns [`crate::wallet::error::FeeError::ReceivedTransaction`] if no spends or outgoing_tx_data were found
-    /// in the wallet for this transaction, indicating this transaction was not created by this spend capability.
-    /// Returns [`crate::wallet::error::FeeError::SpendNotFound`] if any shielded spends in the transaction are not
-    /// found in the wallet, indicating that all shielded spends have not yet been synced. Also returns this error
-    /// if the transaction record contains outgoing_tx_data but no spends are found.
-    /// If a transparent spend has not yet been synced, the fee will be incorrect and return
-    /// [`crate::wallet::error::FeeError::FeeUnderflow`] if an underflow occurs.
-    /// The tracking of transparent spends will be improved on the next internal wallet version.
-    pub fn calculate_transaction_fee(
+    fn get_sapling_notes_spent_in_tx(
         &self,
-        transaction_record: &TransactionRecord,
-    ) -> Result<u64, FeeError> {
-        let sapling_spends = transaction_record
+        query: &TransactionRecord,
+    ) -> Result<Vec<&SaplingNote>, FeeError> {
+        query
             .spent_sapling_nullifiers()
             .iter()
             .map(|nullifier| {
                 self.values()
-                    .flat_map(|transaction_record| transaction_record.sapling_notes())
+                    .flat_map(|wallet_transaction_record| wallet_transaction_record.sapling_notes())
                     .find(|&note| {
                         if let Some(nf) = note.nullifier() {
                             nf == *nullifier
                         } else {
                             false
                         }
                     })
-                    .ok_or(FeeError::SpendNotFound)
+                    .ok_or(FeeError::SaplingSpendNotFound(*nullifier))
             })
-            .collect::<Result<Vec<&SaplingNote>, FeeError>>()?;
-        let sapling_spend_value: u64 = sapling_spends.iter().map(|&note| note.value()).sum();
-
-        let orchard_spends = transaction_record
+            .collect::<Result<Vec<&SaplingNote>, FeeError>>()
+    }
+    fn get_orchard_notes_spent_in_tx(
+        &self,
+        query: &TransactionRecord,
+    ) -> Result<Vec<&OrchardNote>, FeeError> {
+        query
             .spent_orchard_nullifiers()
             .iter()
             .map(|nullifier| {
                 self.values()
-                    .flat_map(|transaction_record| transaction_record.orchard_notes())
+                    .flat_map(|wallet_transaction_record| wallet_transaction_record.orchard_notes())
                     .find(|&note| {
                         if let Some(nf) = note.nullifier() {
                             nf == *nullifier
                         } else {
                             false
                         }
                     })
-                    .ok_or(FeeError::SpendNotFound)
+                    .ok_or(FeeError::OrchardSpendNotFound(*nullifier))
             })
-            .collect::<Result<Vec<&OrchardNote>, FeeError>>()?;
-        let orchard_spend_value: u64 = orchard_spends.iter().map(|&note| note.value()).sum();
-
-        let total_spend_value = transaction_record.total_transparent_value_spent
-            + sapling_spend_value
-            + orchard_spend_value;
-
-        if total_spend_value == 0 {
-            if transaction_record.value_outgoing() == 0 {
-                return Err(FeeError::ReceivedTransaction);
-            } else {
-                return Err(FeeError::SpendNotFound);
-            }
-        }
-
-        let transparent_output_value: u64 = transaction_record
+            .collect::<Result<Vec<&OrchardNote>, FeeError>>()
+    }
+    fn total_value_input_to_transaction(
+        &self,
+        query_record: &TransactionRecord,
+    ) -> Result<u64, FeeError> {
+        let sapling_spend_value: u64 = self
+            .get_sapling_notes_spent_in_tx(query_record)?
+            .iter()
+            .map(|&note| note.value())
+            .sum();
+        let orchard_spend_value: u64 = self
+            .get_orchard_notes_spent_in_tx(query_record)?
+            .iter()
+            .map(|&note| note.value())
+            .sum();
+        Ok(query_record.total_transparent_value_spent + sapling_spend_value + orchard_spend_value)
+    }
+    fn total_value_output_to_explicit_receivers(&self, query_record: &TransactionRecord) -> u64 {
+        let transparent_output_value: u64 = query_record
             .transparent_outputs()
             .iter()
             .map(|note| note.value())
             .sum();
-        let sapling_output_value: u64 = transaction_record
+        let sapling_output_value: u64 = query_record
             .sapling_notes()
             .iter()
             .map(|note| note.value())
             .sum();
-        let orchard_output_value: u64 = transaction_record
+        let orchard_output_value: u64 = query_record
             .orchard_notes()
             .iter()
             .map(|note| note.value())
             .sum();
-
-        let total_output_value = transaction_record.value_outgoing()
-            + transparent_output_value
+        transparent_output_value
             + sapling_output_value
-            + orchard_output_value;
+            + orchard_output_value
+            + query_record.value_outgoing()
+    }
+    /// Calculate the fee for a transaction in the wallet
+    ///
+    /// # Error
+    ///
+    /// Returns [`crate::wallet::error::FeeError::ReceivedTransaction`] if no spends or outgoing_tx_data were found
+    /// in the wallet for this transaction, indicating this transaction was not created by this spend capability.
+    /// Returns
+    /// [`crate::wallet::error::FeeError::SaplingSpendNotFound`]
+    /// OR
+    /// [`crate::wallet::error::FeeError::OrchardSpendNotFound`]
+    /// if any shielded spends in the transaction are not
+    /// found in the wallet, indicating that all shielded spends have not yet been synced.
+    /// Also returns this error
+    /// if the transaction record contains outgoing_tx_data but no spends are found.
+    /// If a transparent spend has not yet been synced, the fee will be incorrect and return
+    /// [`crate::wallet::error::FeeError::FeeUnderflow`] if an underflow occurs.
+    /// The tracking of transparent spends will be improved on the next internal wallet version.
+    pub fn calculate_transaction_fee(
+        &self,
+        query_record: &TransactionRecord,
+    ) -> Result<u64, FeeError> {
+        let input_value = dbg!(self.total_value_input_to_transaction(query_record)?);
 
-        if total_spend_value >= total_output_value {
-            Ok(total_spend_value - total_output_value)
+        if input_value == 0 {
+            if query_record.value_outgoing() == 0 {
+                return Err(FeeError::ReceivedTransaction);
+            } else {
+                return Err(FeeError::OutgoingWithoutSpends(
+                    query_record.outgoing_tx_data.to_vec(),
+                ));
+            }
+        }
+
+        let explicit_output_value = self.total_value_output_to_explicit_receivers(query_record);
+
+        if input_value >= explicit_output_value {
+            Ok(input_value - explicit_output_value)
         } else {
-            Err(FeeError::FeeUnderflow)
+            Err(FeeError::FeeUnderflow((explicit_output_value, input_value)))
         }
     }
 
@@ -632,7 +661,10 @@ mod tests {
 
     use sapling_crypto::note_encryption::SaplingDomain;
     use zcash_client_backend::{wallet::ReceivedNote, ShieldedProtocol};
-    use zcash_primitives::{consensus::BlockHeight, transaction::TxId};
+    use zcash_primitives::{
+        consensus::BlockHeight,
+        transaction::{fees::zip317::MARGINAL_FEE, TxId},
+    };
     use zingo_status::confirmation_status::ConfirmationStatus::Confirmed;
 
     #[test]
@@ -888,7 +920,7 @@ mod tests {
 
             let fee = transaction_records_by_id
                 .calculate_transaction_fee(transaction_records_by_id.get(&sent_txid).unwrap());
-            assert!(matches!(fee, Err(FeeError::SpendNotFound)));
+            assert!(matches!(fee, Err(FeeError::OrchardSpendNotFound(_))));
         }
         #[test]
         fn received_transaction() {
@@ -923,7 +955,7 @@ mod tests {
 
             let fee = transaction_records_by_id
                 .calculate_transaction_fee(transaction_records_by_id.get(&sent_txid).unwrap());
-            assert!(matches!(fee, Err(FeeError::SpendNotFound)));
+            assert!(matches!(fee, Err(FeeError::OutgoingWithoutSpends(_))));
         }
         #[test]
         fn transparent_spends_not_fully_synced() {
@@ -947,7 +979,7 @@ mod tests {
 
             let fee = transaction_records_by_id
                 .calculate_transaction_fee(transaction_records_by_id.get(&sent_txid).unwrap());
-            assert!(matches!(fee, Err(FeeError::FeeUnderflow)));
+            assert!(matches!(fee, Err(FeeError::FeeUnderflow((_, _)))));
         }
     }
 
@@ -997,4 +1029,72 @@ mod tests {
             )
         }
     }
+    #[ignore = "This work-in-progress is based on incorrect assumptions about the order of transactions."]
+    #[test]
+    fn single_sapling_send() {
+        let sent_transaction_record = TransactionRecordBuilder::default()
+            .status(Confirmed(15.into()))
+            .spent_sapling_nullifiers(
+                SaplingNullifierBuilder::new()
+                    .assign_unique_nullifier()
+                    .clone(),
+            )
+            .build();
+        let first_sapling_nullifier = sent_transaction_record.spent_sapling_nullifiers[0];
+        let sent_txid = sent_transaction_record.txid;
+        let first_received_transaction_record = TransactionRecordBuilder::default()
+            .randomize_txid()
+            .status(Confirmed(5.into()))
+            .sapling_notes(spent_sapling_note_builder(
+                175_000,
+                (sent_txid, 15),
+                &first_sapling_nullifier,
+            ))
+            .set_output_indexes()
+            .build();
+        let received_txid = first_received_transaction_record.txid;
+        let mut transaction_records_by_id = TransactionRecordsById::default();
+        transaction_records_by_id.insert_transaction_record(sent_transaction_record);
+        transaction_records_by_id.insert_transaction_record(first_received_transaction_record);
+
+        let recovered_send_record = transaction_records_by_id.get(&sent_txid).unwrap();
+        dbg!(&recovered_send_record.spent_sapling_nullifiers);
+        dbg!(recovered_send_record.spent_sapling_nullifiers());
+        assert_eq!(
+            transaction_records_by_id
+                .get(&sent_txid)
+                .unwrap()
+                .spent_sapling_nullifiers()
+                .len(),
+            1
+        );
+        assert_eq!(
+            transaction_records_by_id
+                .get(&sent_txid)
+                .unwrap()
+                .spent_orchard_nullifiers()
+                .len(),
+            0
+        );
+        assert_eq!(
+            transaction_records_by_id
+                .get(&received_txid)
+                .unwrap()
+                .sapling_notes()
+                .len(),
+            1
+        );
+        assert_eq!(
+            transaction_records_by_id
+                .get(&received_txid)
+                .unwrap()
+                .orchard_notes()
+                .len(),
+            0
+        );
+        let fee = transaction_records_by_id
+            .calculate_transaction_fee(transaction_records_by_id.get(&sent_txid).unwrap())
+            .unwrap();
+        assert_eq!(fee, u64::from(MARGINAL_FEE) * 2);
+    }
 }