Merge pull request #1480 from AloeareV/sort_value_transfers

Sort value transfers

libtonode-tests/tests/concrete.rs

@@ -1280,7 +1280,8 @@ mod slow {
                  recipient_address: "zregtestsapling1fmq2ufux3gm0v8qf7x585wj56le4wjfsqsj27zprjghntrerntggg507hxh2ydcdkn7sx8kya7p".to_string(),
                  value: first_send_to_sapling,
                  memo: Memo::Empty,
-                 recipient_ua: None
+                 recipient_ua: None,
+                 output_index: None,
              }])
             .build()
             .unwrap();
@@ -1310,6 +1311,7 @@ mod slow {
                 value: first_send_to_transparent,
                 memo: Memo::Empty,
                 recipient_ua: None,
+                output_index: None,
             }])
             .build()
             .unwrap();
@@ -1423,6 +1425,7 @@ mod slow {
                 value: second_send_to_transparent,
                 memo: Memo::Empty,
                 recipient_ua: None,
+                output_index: None,
             }])
             .build()
             .unwrap();
@@ -1460,7 +1463,8 @@ mod slow {
                  recipient_address: "zregtestsapling1fmq2ufux3gm0v8qf7x585wj56le4wjfsqsj27zprjghntrerntggg507hxh2ydcdkn7sx8kya7p".to_string(),
                  value: second_send_to_sapling,
                  memo: Memo::Empty,
-                 recipient_ua: None
+                 recipient_ua: None,
+                 output_index: None,
              }])
             .build()
             .unwrap();
@@ -1502,6 +1506,7 @@ mod slow {
                 value: external_transparent_3,
                 memo: Memo::Empty,
                 recipient_ua: None,
+                output_index: None,
             }])
             .build()
             .unwrap();

zingolib/src/lightclient/describe.rs

@@ -2,10 +2,7 @@
 use ::orchard::note_encryption::OrchardDomain;
 use json::{object, JsonValue};
 use sapling_crypto::note_encryption::SaplingDomain;
-use std::{
-    cmp::Ordering,
-    collections::{HashMap, HashSet},
-};
+use std::{cmp::Ordering, collections::HashMap};
 use tokio::runtime::Runtime;
 
 use zcash_client_backend::{encoding::encode_payment_address, PoolType, ShieldedProtocol};
@@ -282,7 +279,7 @@ impl LightClient {
             transaction_summary: &TransactionSummary,
         ) {
             let mut addresses =
-                HashSet::with_capacity(transaction_summary.outgoing_tx_data().len());
+                HashMap::with_capacity(transaction_summary.outgoing_tx_data().len());
             transaction_summary
                 .outgoing_tx_data()
                 .iter()
@@ -292,10 +289,12 @@ impl LightClient {
                     } else {
                         outgoing_tx_data.recipient_address.clone()
                     };
-                    // hash set is used to create unique list of addresses as duplicates are not inserted twice
-                    addresses.insert(address);
+                    // hash map is used to create unique list of addresses as duplicates are not inserted twice
+                    addresses.insert(address, outgoing_tx_data.output_index);
                 });
-            addresses.iter().for_each(|address| {
+            let mut addresses_vec = addresses.into_iter().collect::<Vec<_>>();
+            addresses_vec.sort_by_key(|x| x.1);
+            addresses_vec.iter().for_each(|(address, _output_index)| {
                 let outgoing_data_to_address: Vec<OutgoingTxData> = transaction_summary
                     .outgoing_tx_data()
                     .iter()

zingolib/src/wallet/data.rs

@@ -15,7 +15,7 @@ use zcash_client_backend::{
     proto::compact_formats::CompactBlock, wallet::TransparentAddressMetadata,
 };
 
-use zcash_encoding::{Optional, Vector};
+use zcash_encoding::{CompactSize, Optional, Vector};
 
 use zcash_primitives::merkle_tree::{read_commitment_tree, write_commitment_tree};
 use zcash_primitives::{legacy::TransparentAddress, memo::MemoBytes};
@@ -291,6 +291,8 @@ pub struct OutgoingTxData {
     /// What if it wasn't provided?  How does this relate to
     /// recipient_address?
     pub recipient_ua: Option<String>,
+    /// This output's index in its containing transaction
+    pub output_index: Option<u64>,
 }
 impl std::fmt::Display for OutgoingTxData {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -343,8 +345,42 @@ impl PartialEq for OutgoingTxData {
     }
 }
 impl OutgoingTxData {
-    /// TODO: Add Doc Comment Here!
+    const VERSION: u64 = 0;
+    /// Before version 0, OutgoingTxData didn't have a version field
+    pub fn read_old<R: Read>(mut reader: R) -> io::Result<Self> {
+        let address_len = reader.read_u64::<LittleEndian>()?;
+        let mut address_bytes = vec![0; address_len as usize];
+        reader.read_exact(&mut address_bytes)?;
+        let address = String::from_utf8(address_bytes).unwrap();
+
+        let value = reader.read_u64::<LittleEndian>()?;
+
+        let mut memo_bytes = [0u8; 512];
+        reader.read_exact(&mut memo_bytes)?;
+        let memo = match MemoBytes::from_bytes(&memo_bytes) {
+            Ok(mb) => match Memo::try_from(mb.clone()) {
+                Ok(m) => Ok(m),
+                Err(_) => Ok(Memo::Future(mb)),
+            },
+            Err(e) => Err(io::Error::new(
+                io::ErrorKind::InvalidInput,
+                format!("Couldn't create memo: {}", e),
+            )),
+        }?;
+
+        Ok(OutgoingTxData {
+            recipient_address: address,
+            value,
+            memo,
+            recipient_ua: None,
+            output_index: None,
+        })
+    }
+
+    /// Read an OutgoingTxData from its serialized
+    /// representation
     pub fn read<R: Read>(mut reader: R) -> io::Result<Self> {
+        let _external_version = CompactSize::read(&mut reader)?;
         let address_len = reader.read_u64::<LittleEndian>()?;
         let mut address_bytes = vec![0; address_len as usize];
         reader.read_exact(&mut address_bytes)?;
@@ -364,17 +400,20 @@ impl OutgoingTxData {
                 format!("Couldn't create memo: {}", e),
             )),
         }?;
+        let output_index = Optional::read(&mut reader, CompactSize::read)?;
 
         Ok(OutgoingTxData {
             recipient_address: address,
             value,
             memo,
             recipient_ua: None,
+            output_index,
         })
     }
 
     /// TODO: Add Doc Comment Here!
     pub fn write<W: Write>(&self, mut writer: W) -> io::Result<()> {
+        CompactSize::write(&mut writer, Self::VERSION as usize)?;
         // Strings are written as len + utf8
         match &self.recipient_ua {
             None => {
@@ -387,7 +426,10 @@ impl OutgoingTxData {
             }
         }
         writer.write_u64::<LittleEndian>(self.value)?;
-        writer.write_all(self.memo.encode().as_array())
+        writer.write_all(self.memo.encode().as_array())?;
+        Optional::write(&mut writer, self.output_index, |w, output_index| {
+            CompactSize::write(w, output_index as usize)
+        })
     }
 }
 
@@ -2047,6 +2089,7 @@ pub(crate) mod mocks {
         value: Option<u64>,
         memo: Option<Memo>,
         recipient_ua: Option<Option<String>>,
+        output_index: Option<Option<u64>>,
     }
 
     impl OutgoingTxDataBuilder {
@@ -2056,6 +2099,7 @@ pub(crate) mod mocks {
                 value: None,
                 memo: None,
                 recipient_ua: None,
+                output_index: None,
             }
         }
 
@@ -2064,13 +2108,15 @@ pub(crate) mod mocks {
         build_method!(value, u64);
         build_method!(memo, Memo);
         build_method!(recipient_ua, Option<String>);
+        build_method!(output_index, Option<u64>);
 
         pub(crate) fn build(&self) -> OutgoingTxData {
             OutgoingTxData {
                 recipient_address: self.recipient_address.clone().unwrap(),
                 value: self.value.unwrap(),
                 memo: self.memo.clone().unwrap(),
                 recipient_ua: self.recipient_ua.clone().unwrap(),
+                output_index: self.output_index.unwrap(),
             }
         }
     }
@@ -2082,7 +2128,9 @@ pub(crate) mod mocks {
                 .recipient_address("default_address".to_string())
                 .value(50_000)
                 .memo(Memo::default())
-                .recipient_ua(None);
+                .recipient_ua(None)
+                .output_index(None);
+
             builder
         }
     }

zingolib/src/wallet/traits.rs

@@ -537,6 +537,10 @@ pub trait DomainWalletExt:
     /// TODO: Add Doc Comment Here!
     fn get_tree(tree_state: &TreeState) -> &String;
 
+    /// Counts the number of outputs in earlier pools, to allow for
+    // the output index to be a single source for output order
+    fn output_index_offset(transaction: &Transaction) -> u64;
+
     /// TODO: Add Doc Comment Here!
     fn ua_from_contained_receiver<'a>(
         unified_spend_auth: &'a WalletCapability,
@@ -596,6 +600,13 @@ impl DomainWalletExt for SaplingDomain {
         &tree_state.sapling_tree
     }
 
+    fn output_index_offset(transaction: &Transaction) -> u64 {
+        transaction
+            .transparent_bundle()
+            .map(|tbundle| tbundle.vout.len() as u64)
+            .unwrap_or(0)
+    }
+
     fn ua_from_contained_receiver<'a>(
         unified_spend_auth: &'a WalletCapability,
         receiver: &Self::Recipient,
@@ -660,6 +671,14 @@ impl DomainWalletExt for OrchardDomain {
         &tree_state.orchard_tree
     }
 
+    fn output_index_offset(transaction: &Transaction) -> u64 {
+        SaplingDomain::output_index_offset(transaction)
+            + transaction
+                .sapling_bundle()
+                .map(|sbundle| sbundle.shielded_outputs().len() as u64)
+                .unwrap_or(0)
+    }
+
     fn ua_from_contained_receiver<'a>(
         unified_spend_capability: &'a WalletCapability,
         receiver: &Self::Recipient,

zingolib/src/wallet/transaction_context.rs

@@ -555,15 +555,15 @@ mod decrypt_transaction {
             //     1. There's more than one way to be "spent".
             //     2. It's possible for a "nullifier" to be in the wallet's spent list, but never in the global ledger.
             //     <https://github.com/zingolabs/zingolib/issues/65>
-            for (_domain, output) in domain_tagged_outputs {
+            for (i, (_domain, output)) in domain_tagged_outputs.iter().enumerate() {
                 outgoing_metadatas.extend(
                     match try_output_recovery_with_ovk::<
                         D,
                         <FnGenBundle<D> as zingo_traits::Bundle<D>>::Output,
                     >(
                         &output.domain(status.get_height(), self.config.chain),
                         &ovk.ovk,
-                        &output,
+                        output,
                         &output.value_commitment(),
                         &output.out_ciphertext(),
                     ) {
@@ -611,6 +611,9 @@ mod decrypt_transaction {
                                             value: D::WalletNote::value_from_note(&note),
                                             memo,
                                             recipient_ua: None,
+                                            output_index: Some(
+                                                D::output_index_offset(transaction) + i as u64,
+                                            ),
                                         })
                                     }
                                 }
@@ -642,7 +645,7 @@ mod decrypt_transaction {
                     .transaction_kind(transaction_record, &self.config.chain)
                 {
                     if let Some(t_bundle) = transaction.transparent_bundle() {
-                        for vout in &t_bundle.vout {
+                        for (i, vout) in t_bundle.vout.iter().enumerate() {
                             if let Some(taddr) = vout.recipient_address().map(|raw_taddr| {
                                 match sent_to_tex {
                                     false => address_from_pubkeyhash(&self.config, raw_taddr),
@@ -661,6 +664,7 @@ mod decrypt_transaction {
                                         value: u64::from(vout.value),
                                         memo: Memo::Empty,
                                         recipient_ua: None,
+                                        output_index: Some(i as u64),
                                     });
                                 }
                             }

zingolib/src/wallet/transaction_record.rs

@@ -445,10 +445,10 @@ impl TransactionRecord {
             0
         };
 
-        // Outgoing metadata was only added in version 2
-        let outgoing_metadata =
-            zcash_encoding::Vector::read(&mut reader, |r| OutgoingTxData::read(r))?;
-
+        let outgoing_metadata = match version {
+            ..24 => zcash_encoding::Vector::read(&mut reader, |r| OutgoingTxData::read_old(r))?,
+            24.. => zcash_encoding::Vector::read(&mut reader, |r| OutgoingTxData::read(r))?,
+        };
         let _full_tx_scanned = reader.read_u8()? > 0;
 
         let zec_price = if version <= 4 {
@@ -496,7 +496,7 @@ impl TransactionRecord {
 
     /// TODO: Add Doc Comment Here!
     pub fn serialized_version() -> u64 {
-        23
+        24
     }
 
     /// TODO: Add Doc Comment Here!