From bc5bc3c40c7382c2790c7840547b601441d69d66 Mon Sep 17 00:00:00 2001
From: ryan kurte <ryan@kurte.nz>
Date: Wed, 26 Jul 2023 11:23:10 +1200
Subject: [PATCH] add swap elision to report, update docs

---
 transaction/summary/src/error.rs    |   2 +
 transaction/summary/src/report.rs   | 105 +++++++++++++++++++++++++---
 transaction/summary/src/verifier.rs |   5 +-
 3 files changed, 99 insertions(+), 13 deletions(-)

diff --git a/transaction/summary/src/error.rs b/transaction/summary/src/error.rs
index 120071e7cc..06ab2c55f7 100644
--- a/transaction/summary/src/error.rs
+++ b/transaction/summary/src/error.rs
@@ -38,6 +38,8 @@ pub enum Error {
     Amount(AmountError),
     /// ZipExact error: {0}
     ZipExact(ZipExactError),
+    /// Missing address for own output
+    MissingOutputAddress,
 }
 
 impl From<BlockVersionError> for Error {
diff --git a/transaction/summary/src/report.rs b/transaction/summary/src/report.rs
index 2f9e5a106f..66fc4d0e39 100644
--- a/transaction/summary/src/report.rs
+++ b/transaction/summary/src/report.rs
@@ -26,7 +26,7 @@ pub enum TransactionEntity {
     /// Outputs to other accounts (hash {0})
     OtherAddress(ShortAddressHash),
 
-    /// Swap counterparty
+    /// Outputs to swap counterparty
     Swap,
 }
 
@@ -117,10 +117,12 @@ pub struct TxSummaryUnblindingReport<
     /// Total balance change for our account for each type of token in the
     /// transaction.
     ///
-    /// totals = our inputs - sum(change outputs)
+    /// totals = inputs - sum(change outputs)
     ///
-    /// Note that swap inputs are elided as these are not inputs
-    /// owned by us (ie. are not spent from our account)
+    /// Note that owned and swap inputs are split as most
+    /// applications are concerned only with the _cost_ of
+    /// the transaction to the user.
+    /// See [elide_swap_totals] for more detail.
     pub totals: Vec<(TokenId, TotalKind, i64), TOTALS>,
 
     /// The network fee that we pay to execute the transaction
@@ -132,9 +134,10 @@ pub struct TxSummaryUnblindingReport<
 
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
 pub enum TotalKind {
-    /// Input owned by our account
+    /// Input owned by our account (less change), outgoing from our account
     Ours,
-    /// Input owned by SCI counterparty
+    /// Input owned by SCI counterparty (less change for partial swaps) incoming
+    /// to our account
     Sci,
 }
 
@@ -323,11 +326,32 @@ impl<const RECORDS: usize, const TOTALS: usize> TxSummaryUnblindingReport<RECORD
     pub fn sort(&mut self) {
         // TODO: should we remove zeroed balances / totals?
 
-        self.outputs[..].sort_by_key(|(_, t, _)| *t);
-        self.outputs[..].sort_by_key(|(e, _, _)| e.clone());
+        self.outputs[..].sort_by_key(|(e, t, _)| (e.clone(), *t));
+        self.totals[..].sort_by_key(|(t, k, _)| (*k, *t));
+    }
+
+    /// Elide SCI inputs and change outputs from a finalised report
+    ///
+    /// This transforms the report to only include outputs from _our_
+    /// account in the totals for display to users.
+    pub fn elide_swap_totals(&mut self) {
+        // Find SCI inputs in totals
+        for (token_id, kind, _) in &self.totals[..] {
+            if *kind != TotalKind::Sci {
+                continue;
+            }
+
+            // Remove corresponding change outputs for partial swap
+            // (change outputs are in the same token type as the swap,
+            // where the outputs to fulfil the swap are not)
+            self.outputs.retain(|(entity, token, _amount)| {
+                *entity != TransactionEntity::Swap || token != token_id
+            });
+        }
 
-        self.totals[..].sort_by_key(|(t, _, _)| *t);
-        self.totals[..].sort_by_key(|(_, k, _)| *k);
+        // Remove SCI inputs from totals
+        self.totals
+            .retain(|(_token_id, kind, _)| *kind != TotalKind::Sci)
     }
 }
 
@@ -443,4 +467,65 @@ mod tests {
             ]
         );
     }
+
+    #[test]
+    fn test_flatten_sci() {
+        let mut report = TxSummaryUnblindingReport::<16>::new();
+
+        let addr = TransactionEntity::OurAddress(ShortAddressHash::from(random::<[u8; 16]>()));
+
+        // Add SCI outputs
+        report
+            .output_add(TransactionEntity::Swap, Amount::new(10, TokenId::from(1)))
+            .unwrap();
+        report
+            .output_add(TransactionEntity::Swap, Amount::new(50, TokenId::from(2)))
+            .unwrap();
+
+        // Add output to us
+        report
+            .output_add(addr.clone(), Amount::new(50, TokenId::from(2)))
+            .unwrap();
+
+        // Add input from SCI
+        report.sci_add(Amount::new(100, TokenId::from(2))).unwrap();
+
+        // Add owned input
+        report.input_add(Amount::new(10, TokenId::from(1))).unwrap();
+
+        // Finalise report
+        report.finalize().unwrap();
+
+        // Check full outputs / totals
+        assert_eq!(
+            &report.outputs[..],
+            &[
+                (addr.clone(), TokenId::from(2), 50),
+                (TransactionEntity::Swap, TokenId::from(1), 10),
+                (TransactionEntity::Swap, TokenId::from(2), 50),
+            ]
+        );
+        assert_eq!(
+            &report.totals[..],
+            &[
+                (TokenId::from(1), TotalKind::Ours, 10),
+                (TokenId::from(2), TotalKind::Sci, 50),
+            ]
+        );
+
+        // Trim swap information (removes swap partial returns and totals)
+        report.elide_swap_totals();
+
+        assert_eq!(
+            &report.outputs[..],
+            &[
+                (addr.clone(), TokenId::from(2), 50),
+                (TransactionEntity::Swap, TokenId::from(1), 10),
+            ]
+        );
+        assert_eq!(
+            &report.totals[..],
+            &[(TokenId::from(1), TotalKind::Ours, 10),]
+        );
+    }
 }
diff --git a/transaction/summary/src/verifier.rs b/transaction/summary/src/verifier.rs
index 9eb40b2bcd..c9a5ddf96d 100644
--- a/transaction/summary/src/verifier.rs
+++ b/transaction/summary/src/verifier.rs
@@ -149,9 +149,8 @@ impl TxSummaryStreamingVerifierCtx {
                         .output_add(TransactionEntity::OurAddress(address_hash.clone()), amount)?;
                 }
             } else {
-                // TODO: If we _don't_ have address information but it's to our own address...
-                // what then? is this even possible??!
-                panic!("what's the right thing to do here..?");
+                // If we _don't_ have address information but it's to our own address...
+                return Err(Error::MissingOutputAddress);
             }
 
         // If we didn't match the output, and we have address information, this