diff --git a/libtonode-tests/tests/concrete.rs b/libtonode-tests/tests/concrete.rs index 797b795bc..934ef7ccb 100644 --- a/libtonode-tests/tests/concrete.rs +++ b/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(); diff --git a/zingolib/src/lightclient/describe.rs b/zingolib/src/lightclient/describe.rs index 09e7feb10..0c2fb6170 100644 --- a/zingolib/src/lightclient/describe.rs +++ b/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::>(); + addresses_vec.sort_by_key(|x| x.1); + addresses_vec.iter().for_each(|(address, _output_index)| { let outgoing_data_to_address: Vec = transaction_summary .outgoing_tx_data() .iter() diff --git a/zingolib/src/wallet/data.rs b/zingolib/src/wallet/data.rs index 01b5c1781..133305b44 100644 --- a/zingolib/src/wallet/data.rs +++ b/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, + /// This output's index in its containing transaction + pub output_index: Option, } 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(mut reader: R) -> io::Result { + let address_len = reader.read_u64::()?; + 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::()?; + + 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(mut reader: R) -> io::Result { + let _external_version = CompactSize::read(&mut reader)?; let address_len = reader.read_u64::()?; 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(&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::(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, memo: Option, recipient_ua: Option>, + output_index: Option>, } impl OutgoingTxDataBuilder { @@ -2056,6 +2099,7 @@ pub(crate) mod mocks { value: None, memo: None, recipient_ua: None, + output_index: None, } } @@ -2064,6 +2108,7 @@ pub(crate) mod mocks { build_method!(value, u64); build_method!(memo, Memo); build_method!(recipient_ua, Option); + build_method!(output_index, Option); pub(crate) fn build(&self) -> OutgoingTxData { OutgoingTxData { @@ -2071,6 +2116,7 @@ pub(crate) mod mocks { 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 } } diff --git a/zingolib/src/wallet/traits.rs b/zingolib/src/wallet/traits.rs index b76a6da11..bf99cd05b 100644 --- a/zingolib/src/wallet/traits.rs +++ b/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, diff --git a/zingolib/src/wallet/transaction_context.rs b/zingolib/src/wallet/transaction_context.rs index 802398f9c..b348c7414 100644 --- a/zingolib/src/wallet/transaction_context.rs +++ b/zingolib/src/wallet/transaction_context.rs @@ -555,7 +555,7 @@ 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. // - 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, @@ -563,7 +563,7 @@ mod decrypt_transaction { >( &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(¬e), 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), }); } } diff --git a/zingolib/src/wallet/transaction_record.rs b/zingolib/src/wallet/transaction_record.rs index ec2f7aa8e..d147f1c99 100644 --- a/zingolib/src/wallet/transaction_record.rs +++ b/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!