From 3ab66d3d296d617902fba06e4b408090401e629d 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

---
 Cargo.lock                          | 135 +++++++++++++---------------
 transaction/summary/src/error.rs    |   2 +
 transaction/summary/src/report.rs   | 105 +++++++++++++++++++---
 transaction/summary/src/verifier.rs |   5 +-
 4 files changed, 162 insertions(+), 85 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index c0de86af8c..d1e7c89c47 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -680,38 +680,30 @@ dependencies = [
 
 [[package]]
 name = "clap"
-version = "4.1.14"
+version = "4.1.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "906f7fe1da4185b7a282b2bc90172a496f9def1aca4545fe7526810741591e14"
+checksum = "42dfd32784433290c51d92c438bb72ea5063797fc3cc9a21a8c4346bebbb2098"
 dependencies = [
- "clap_builder",
+ "bitflags 2.0.2",
  "clap_derive",
- "once_cell",
-]
-
-[[package]]
-name = "clap_builder"
-version = "4.1.14"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "351f9ad9688141ed83dfd8f5fb998a06225ef444b48ff4dc43de6d409b7fd10b"
-dependencies = [
- "bitflags 1.3.2",
- "clap_lex 0.4.1",
+ "clap_lex 0.3.0",
  "is-terminal",
+ "once_cell",
  "strsim 0.10.0",
  "termcolor",
 ]
 
 [[package]]
 name = "clap_derive"
-version = "4.1.14"
+version = "4.1.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "81d7dc0031c3a59a04fc2ba395c8e2dd463cba1859275f065d225f6122221b45"
+checksum = "fddf67631444a3a3e3e5ac51c36a5e01335302de677bd78759eaa90ab1f46644"
 dependencies = [
  "heck",
+ "proc-macro-error",
  "proc-macro2",
  "quote",
- "syn 2.0.12",
+ "syn 1.0.109",
 ]
 
 [[package]]
@@ -725,9 +717,12 @@ dependencies = [
 
 [[package]]
 name = "clap_lex"
-version = "0.4.1"
+version = "0.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8a2dd5a6fe8c6e3502f568a6353e5273bbb15193ad9a89e457b9970798efbea1"
+checksum = "0d4198f73e42b4936b35b5bb248d81d2b595ecb170da0bac7655c54eedfa8da8"
+dependencies = [
+ "os_str_bytes",
+]
 
 [[package]]
 name = "clear_on_drop"
@@ -1184,9 +1179,9 @@ dependencies = [
 
 [[package]]
 name = "dirs-sys"
-version = "0.3.7"
+version = "0.3.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1b1d1d91c932ef41c0f2663aa8b0ca0342d444d842c06914aa0a7e352d0bada6"
+checksum = "03d86534ed367a67548dc68113a0f5db55432fdfbb6e6f9d77704397d95d5780"
 dependencies = [
  "libc",
  "redox_users",
@@ -1594,7 +1589,7 @@ checksum = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574"
 name = "go-grpc-gateway-testing"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "futures",
  "grpcio",
@@ -2301,7 +2296,7 @@ dependencies = [
 name = "mc-admin-http-gateway"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "grpcio",
  "mc-common",
  "mc-util-grpc",
@@ -2848,7 +2843,7 @@ dependencies = [
 name = "mc-consensus-mint-client"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "grpcio",
  "hex",
@@ -2920,7 +2915,7 @@ dependencies = [
 name = "mc-consensus-scp-play"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "mc-common",
  "mc-consensus-scp",
  "mc-transaction-core",
@@ -2950,7 +2945,7 @@ version = "5.0.8"
 dependencies = [
  "base64 0.21.0",
  "chrono",
- "clap 4.1.14",
+ "clap 4.1.11",
  "curve25519-dalek",
  "displaydoc",
  "fs_extra",
@@ -3014,7 +3009,7 @@ name = "mc-consensus-service-config"
 version = "5.0.8"
 dependencies = [
  "base64 0.21.0",
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "hex",
  "mc-attest-core",
@@ -3040,7 +3035,7 @@ dependencies = [
 name = "mc-consensus-tool"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "grpcio",
  "mc-common",
  "mc-connection",
@@ -3055,7 +3050,7 @@ name = "mc-core"
 version = "5.0.8"
 dependencies = [
  "anyhow",
- "clap 4.1.14",
+ "clap 4.1.11",
  "curve25519-dalek",
  "ed25519-dalek",
  "generic-array",
@@ -3346,7 +3341,7 @@ name = "mc-crypto-x509-test-vectors"
 version = "5.0.8"
 dependencies = [
  "cargo-emit",
- "clap 4.1.14",
+ "clap 4.1.11",
  "mc-crypto-keys",
  "mc-util-build-script",
  "pem",
@@ -3412,7 +3407,7 @@ dependencies = [
 name = "mc-fog-distribution"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "crossbeam-channel",
  "curve25519-dalek",
  "grpcio",
@@ -3470,7 +3465,7 @@ name = "mc-fog-ingest-client"
 version = "5.0.8"
 dependencies = [
  "assert_cmd",
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "grpcio",
  "hex",
@@ -3625,7 +3620,7 @@ dependencies = [
 name = "mc-fog-ingest-server"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "dirs",
  "displaydoc",
  "futures",
@@ -3847,7 +3842,7 @@ name = "mc-fog-ledger-server"
 version = "5.0.8"
 dependencies = [
  "aes-gcm",
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "futures",
  "grpcio",
@@ -3930,7 +3925,7 @@ dependencies = [
 name = "mc-fog-load-testing"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "grpcio",
  "mc-account-keys",
  "mc-blockchain-test-utils",
@@ -4002,7 +3997,7 @@ dependencies = [
 name = "mc-fog-overseer-server"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "grpcio",
  "lazy_static",
@@ -4085,7 +4080,7 @@ name = "mc-fog-report-cli"
 version = "5.0.8"
 dependencies = [
  "base64 0.21.0",
- "clap 4.1.14",
+ "clap 4.1.11",
  "grpcio",
  "hex",
  "mc-account-keys",
@@ -4140,7 +4135,7 @@ dependencies = [
 name = "mc-fog-report-server"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "futures",
  "grpcio",
@@ -4208,7 +4203,7 @@ name = "mc-fog-sample-paykit"
 version = "5.0.8"
 dependencies = [
  "cargo-emit",
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "futures",
  "grpcio",
@@ -4306,7 +4301,7 @@ name = "mc-fog-sql-recovery-db"
 version = "5.0.8"
 dependencies = [
  "chrono",
- "clap 4.1.14",
+ "clap 4.1.11",
  "diesel",
  "diesel-derive-enum",
  "diesel_migrations",
@@ -4339,7 +4334,7 @@ name = "mc-fog-sql-recovery-db-cleanup"
 version = "5.0.8"
 dependencies = [
  "chrono",
- "clap 4.1.14",
+ "clap 4.1.11",
  "mc-common",
  "mc-fog-recovery-db-iface",
  "mc-fog-sql-recovery-db",
@@ -4350,7 +4345,7 @@ dependencies = [
 name = "mc-fog-test-client"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "grpcio",
  "hex_fmt",
@@ -4384,7 +4379,7 @@ dependencies = [
 name = "mc-fog-test-infra"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "digest 0.10.6",
  "hex",
  "mc-account-keys",
@@ -4577,7 +4572,7 @@ dependencies = [
 name = "mc-fog-view-load-test"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "grpcio",
  "mc-account-keys",
  "mc-attest-verifier",
@@ -4619,7 +4614,7 @@ dependencies = [
 name = "mc-fog-view-server"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "futures",
  "grpcio",
@@ -4730,7 +4725,7 @@ dependencies = [
 name = "mc-ledger-distribution"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "dirs",
  "displaydoc",
  "mc-api",
@@ -4753,7 +4748,7 @@ dependencies = [
 name = "mc-ledger-from-archive"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "mc-api",
  "mc-common",
  "mc-ledger-db",
@@ -4764,7 +4759,7 @@ dependencies = [
 name = "mc-ledger-migration"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "lmdb-rkv",
  "mc-common",
  "mc-ledger-db",
@@ -4812,7 +4807,7 @@ name = "mc-mobilecoind"
 version = "5.0.8"
 dependencies = [
  "aes-gcm",
- "clap 4.1.14",
+ "clap 4.1.11",
  "crossbeam-channel",
  "displaydoc",
  "grpcio",
@@ -4900,7 +4895,7 @@ name = "mc-mobilecoind-dev-faucet"
 version = "5.0.8"
 dependencies = [
  "async-channel",
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "grpcio",
  "hex",
@@ -4931,7 +4926,7 @@ dependencies = [
 name = "mc-mobilecoind-json"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "grpcio",
  "hex",
  "mc-api",
@@ -5111,7 +5106,7 @@ dependencies = [
 name = "mc-sgx-css-dump"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "hex_fmt",
  "mc-sgx-css",
 ]
@@ -5397,7 +5392,7 @@ name = "mc-transaction-signer"
 version = "5.0.8"
 dependencies = [
  "anyhow",
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "hex",
  "log",
@@ -5426,19 +5421,15 @@ name = "mc-transaction-summary"
 version = "5.0.8"
 dependencies = [
  "displaydoc",
- "heapless",
  "mc-account-keys",
  "mc-core",
  "mc-crypto-digestible",
  "mc-crypto-keys",
  "mc-crypto-ring-signature",
- "mc-transaction-core",
  "mc-transaction-types",
- "mc-util-from-random",
  "mc-util-vec-map",
  "mc-util-zip-exact",
  "prost",
- "rand",
  "serde",
  "subtle",
  "zeroize",
@@ -5467,7 +5458,7 @@ dependencies = [
 name = "mc-util-b58-decoder"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "hex",
  "mc-account-keys",
  "mc-api",
@@ -5531,7 +5522,7 @@ dependencies = [
 name = "mc-util-cli"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "mc-util-build-info",
 ]
 
@@ -5539,7 +5530,7 @@ dependencies = [
 name = "mc-util-dump-ledger"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "mc-blockchain-types",
  "mc-common",
@@ -5574,7 +5565,7 @@ dependencies = [
 name = "mc-util-generate-sample-ledger"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "mc-account-keys",
  "mc-blockchain-test-utils",
  "mc-common",
@@ -5594,7 +5585,7 @@ name = "mc-util-grpc"
 version = "5.0.8"
 dependencies = [
  "base64 0.21.0",
- "clap 4.1.14",
+ "clap 4.1.11",
  "cookie 0.17.0",
  "displaydoc",
  "futures",
@@ -5627,7 +5618,7 @@ dependencies = [
 name = "mc-util-grpc-admin-tool"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "grpcio",
  "mc-common",
  "mc-util-grpc",
@@ -5638,7 +5629,7 @@ dependencies = [
 name = "mc-util-grpc-token-generator"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "mc-common",
  "mc-util-grpc",
  "mc-util-parse",
@@ -5654,7 +5645,7 @@ name = "mc-util-keyfile"
 version = "5.0.8"
 dependencies = [
  "base64 0.21.0",
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "hex",
  "mc-account-keys",
@@ -5742,7 +5733,7 @@ dependencies = [
 name = "mc-util-seeded-ed25519-key-gen"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "mc-crypto-keys",
  "mc-util-from-random",
  "mc-util-parse",
@@ -5778,7 +5769,7 @@ dependencies = [
 name = "mc-util-test-helper"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "itertools",
  "lazy_static",
  "mc-account-keys",
@@ -5860,7 +5851,7 @@ dependencies = [
 name = "mc-watcher"
 version = "5.0.8"
 dependencies = [
- "clap 4.1.14",
+ "clap 4.1.11",
  "displaydoc",
  "futures",
  "grpcio",
@@ -7460,9 +7451,9 @@ dependencies = [
 
 [[package]]
 name = "serde"
-version = "1.0.164"
+version = "1.0.159"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9e8c8cf938e98f769bc164923b06dce91cea1751522f46f8466461af04c9027d"
+checksum = "3c04e8343c3daeec41f58990b9d77068df31209f2af111e059e9fe9646693065"
 dependencies = [
  "serde_derive",
 ]
@@ -7488,9 +7479,9 @@ dependencies = [
 
 [[package]]
 name = "serde_derive"
-version = "1.0.164"
+version = "1.0.159"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d9735b638ccc51c28bf6914d90a2e9725b377144fc612c49a611fddd1b631d68"
+checksum = "4c614d17805b093df4b147b51339e7e44bf05ef59fba1e45d83500bcfb4d8585"
 dependencies = [
  "proc-macro2",
  "quote",
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