diff --git a/zingo-testutils/src/data.rs b/zingo-testutils/src/data.rs index 548815b31..f55781b04 100644 --- a/zingo-testutils/src/data.rs +++ b/zingo-testutils/src/data.rs @@ -16,6 +16,7 @@ pub mod seeds { unfold vocal weird milk scale social vessel identify \ crowd hospital control album rib bulb path oven civil tank"; } +pub const BLOCK_REWARD: u64 = 625_000_000u64; pub const REGSAP_ADDR_FROM_ABANDONART: &str = "zregtestsapling1fmq2ufux3gm0v8qf7x585wj56le4wjfsqsj27zprjghntrerntggg507hxh2ydcdkn7sx8kya7p"; pub mod config_template_fillers { diff --git a/zingocli/tests/integration_tests.rs b/zingocli/tests/integration_tests.rs index 50ada2015..4f26ef9a3 100644 --- a/zingocli/tests/integration_tests.rs +++ b/zingocli/tests/integration_tests.rs @@ -5,7 +5,11 @@ use orchard::tree::MerkleHashOrchard; use shardtree::store::memory::MemoryShardStore; use shardtree::ShardTree; use std::{fs::File, path::Path, str::FromStr}; -use zingo_testutils::{self, build_fvk_client, data, BASE_HEIGHT}; +use zingo_testutils::{ + self, build_fvk_client, + data::{self, BLOCK_REWARD}, + BASE_HEIGHT, +}; use bip0039::Mnemonic; use data::seeds::HOSPITAL_MUSEUM_SEED; @@ -687,10 +691,11 @@ async fn send_mined_sapling_to_orchard() { // We send change to orchard now, so we should have the full value of the note // we spent, minus the transaction fee assert_eq!(balance.unverified_orchard_balance, Some(0)); - assert_eq!( - balance.verified_orchard_balance.unwrap(), - 625_000_000 - u64::from(MINIMUM_FEE) - ); + let expected_orchard = BLOCK_REWARD - u64::from(MINIMUM_FEE); + assert_eq!(balance.verified_orchard_balance.unwrap(), expected_orchard); + // Important to remember that the block reward includes not just the base 6.25 ZEC, + // but also all fees paid. + check_client_balances!(faucet, o: expected_orchard s: {(BLOCK_REWARD * 2) + u64::from(MINIMUM_FEE)} t: 0); } fn extract_value_as_u64(input: &JsonValue) -> u64 { @@ -961,7 +966,6 @@ async fn from_t_z_o_tz_to_zo_tzo_to_orchard() { async fn send_orchard_back_and_forth() { // setup let (regtest_manager, _cph, faucet, recipient) = scenarios::two_wallet_one_miner_fund().await; - let block_reward = 625_000_000u64; let faucet_to_recipient_amount = 20_000u64; let recipient_to_faucet_amount = 5_000u64; // check start state @@ -971,7 +975,7 @@ async fn send_orchard_back_and_forth() { wallet_height.as_fixed_point_u64(0).unwrap(), BASE_HEIGHT as u64 ); - let three_blocks_reward = block_reward.checked_mul(BASE_HEIGHT as u64).unwrap(); + let three_blocks_reward = BLOCK_REWARD.checked_mul(BASE_HEIGHT as u64).unwrap(); check_client_balances!(faucet, o: 0 s: three_blocks_reward t: 0); // post transfer to recipient, and verify @@ -983,7 +987,7 @@ async fn send_orchard_back_and_forth() { )]) .await .unwrap(); - let orch_change = block_reward - (faucet_to_recipient_amount + u64::from(MINIMUM_FEE)); + let orch_change = BLOCK_REWARD - (faucet_to_recipient_amount + u64::from(MINIMUM_FEE)); let reward_and_fee = three_blocks_reward + u64::from(MINIMUM_FEE); zingo_testutils::increase_height_and_wait_for_client(®test_manager, &recipient, 1) .await @@ -1019,7 +1023,7 @@ async fn send_orchard_back_and_forth() { let recipient_final_orch = faucet_to_recipient_amount - (u64::from(MINIMUM_FEE) + recipient_to_faucet_amount); let faucet_final_orch = orch_change + recipient_to_faucet_amount; - let faucet_final_block = 4 * block_reward + u64::from(MINIMUM_FEE) * 2; + let faucet_final_block = 4 * BLOCK_REWARD + u64::from(MINIMUM_FEE) * 2; check_client_balances!( faucet, o: faucet_final_orch s: faucet_final_block t: 0 diff --git a/zingolib/src/lightclient.rs b/zingolib/src/lightclient.rs index 76b8ba347..b763a0420 100644 --- a/zingolib/src/lightclient.rs +++ b/zingolib/src/lightclient.rs @@ -824,13 +824,14 @@ impl LightClient { .iter() { LightClient::tx_summary_matcher(&mut summaries, *txid, transaction_md); - let tx_fee = transaction_md.get_transaction_fee(); - let (block_height, datetime, price) = ( - transaction_md.block_height, - transaction_md.datetime, - transaction_md.price, - ); - if transaction_md.is_outgoing_transaction() { + if let Some(tx_fee) = transaction_md + .get_transaction_fee(&self.wallet.wallet_capability(), &self.config().chain) + { + let (block_height, datetime, price) = ( + transaction_md.block_height, + transaction_md.datetime, + transaction_md.price, + ); summaries.push(ValueTransfer { block_height, datetime, diff --git a/zingolib/src/wallet/data.rs b/zingolib/src/wallet/data.rs index 8f2d37fcb..7edcf72d8 100644 --- a/zingolib/src/wallet/data.rs +++ b/zingolib/src/wallet/data.rs @@ -14,10 +14,11 @@ use shardtree::ShardTree; use std::convert::TryFrom; use std::io::{self, Read, Write}; use std::usize; +use zcash_client_backend::address::RecipientAddress; use zcash_client_backend::serialization::shardtree::{read_shard, write_shard}; use zcash_encoding::{Optional, Vector}; use zcash_note_encryption::Domain; -use zcash_primitives::consensus::BlockHeight; +use zcash_primitives::consensus::{BlockHeight, Parameters}; use zcash_primitives::memo::MemoBytes; use zcash_primitives::merkle_tree::{read_commitment_tree, write_commitment_tree, HashSer}; use zcash_primitives::sapling::note_encryption::SaplingDomain; @@ -28,7 +29,7 @@ use zcash_primitives::{ }; use zingoconfig::{ChainType, MAX_REORG}; -use super::keys::unified::WalletCapability; +use super::keys::unified::{orchard_viewkey, sapling_viewkey, WalletCapability}; use super::traits::{self, DomainWalletExt, ReadableWriteable, ToBytes}; pub const COMMITMENT_TREE_LEVELS: u8 = 32; @@ -944,8 +945,27 @@ impl TransactionMetadata { } } - pub fn get_transaction_fee(&self) -> u64 { - self.total_value_spent() - (self.value_outgoing() + self.total_change_returned()) + /// Returns Some(fee) for outgoing transactions, and None for non-outgoing + pub fn get_transaction_fee( + &self, + wc: &WalletCapability, + params: &P, + ) -> Option { + if self.is_outgoing_transaction() { + println!( + "TXID {} spent {} and received {}", + self.txid, + self.total_value_spent(), + self.total_value_received() + ); + Some( + self.total_value_spent() + - self.total_value_received() + - self.value_outgoing(wc, params), + ) + } else { + None + } } // TODO: This is incorrect in the edge case where where we have a send-to-self with @@ -1145,9 +1165,54 @@ impl TransactionMetadata { self.value_spent_by_pool().iter().sum() } - pub fn value_outgoing(&self) -> u64 { + pub fn value_outgoing(&self, wc: &WalletCapability, params: &P) -> u64 { + let viewkey = wc.ufvk().unwrap(); self.outgoing_tx_data .iter() + // Filter out sends-to-self, by checking to see if are capable of generating the address + // we sent to from our viewing key(s) + .filter( + |outgoing| match RecipientAddress::decode(params, &outgoing.to_address) { + Some(RecipientAddress::Shielded(sapling_addr)) => sapling_viewkey(&viewkey) + .map_or(true, |sapling_viewkey| { + sapling_viewkey.decrypt_diversifier(&sapling_addr).is_none() + }), + //pubkey_to_address is deprecated, but as of yet there isn't a good alternative + #[allow(deprecated)] + Some(RecipientAddress::Transparent(taddr)) => wc + .transparent_child_keys() + .map_or(true, |child_pubkey_vec| { + child_pubkey_vec.iter().any(|(_diversifier_index, pubkey)| { + zcash_primitives::legacy::keys::pubkey_to_address(pubkey) != taddr + }) + }), + Some(RecipientAddress::Unified(ua)) => { + if let Some(orchard_addr) = ua.orchard() { + orchard_viewkey(&viewkey).map_or(true, |orchard_viewkey| { + orchard_viewkey.scope_for_address(orchard_addr).is_none() + }) + } else if let Some(sapling_addr) = ua.sapling() { + sapling_viewkey(&viewkey).map_or(true, |sapling_viewkey| { + sapling_viewkey.decrypt_diversifier(&sapling_addr).is_none() + }) + } else { + // If the UA we sent to has no sapling or orchard components, + // this means this is a send to a future pool that did not + // exist at the time of this code being written. + // This is probably the the least wrong thing to do here. + true + } + } + // This is situation that should never occur. + // If this is none, it means we can't parse the address we sent to + // This is either some future address type (exceedingly unlikely, as UAs are + // future-compatible), or we somehow sent to an invalid address + // (Maybe there's a way to burn funds by sending to a bad address?) + None => { + unreachable!("Sent to an address we can't parse: {}", outgoing.to_address) + } + }, + ) .fold(0, |running_total, tx_data| tx_data.value + running_total) } diff --git a/zingolib/src/wallet/keys/unified.rs b/zingolib/src/wallet/keys/unified.rs index 6a0319bbb..ead220636 100644 --- a/zingolib/src/wallet/keys/unified.rs +++ b/zingolib/src/wallet/keys/unified.rs @@ -70,7 +70,7 @@ pub struct WalletCapability { >, pub orchard: Capability, - transparent_child_keys: append_only_vec::AppendOnlyVec<(usize, secp256k1::SecretKey)>, + transparent_child_keys: append_only_vec::AppendOnlyVec<(usize, secp256k1::PublicKey)>, addresses: append_only_vec::AppendOnlyVec, // Not all diversifier indexes produce valid sapling addresses. // Because of this, the index isn't necessarily equal to addresses.len() @@ -147,11 +147,11 @@ impl WalletCapability { pub fn transparent_child_keys( &self, - ) -> Result<&AppendOnlyVec<(usize, secp256k1::SecretKey)>, String> { - if self.transparent.can_spend() { + ) -> Result<&AppendOnlyVec<(usize, secp256k1::PublicKey)>, String> { + if self.transparent.can_view() { Ok(&self.transparent_child_keys) } else { - Err("The wallet is not capable of spending transparent funds.".to_string()) + Err("The wallet is not capable of viewing transparent funds.".to_string()) } } @@ -245,7 +245,7 @@ impl WalletCapability { let secp = secp256k1::Secp256k1::new(); let child_pk = secp256k1::PublicKey::from_secret_key(&secp, &child_sk); self.transparent_child_keys - .push((self.addresses.len(), child_sk)); + .push((self.addresses.len(), child_pk)); Some(child_pk) } Capability::View(ext_pk) => { @@ -257,6 +257,8 @@ impl WalletCapability { } Ok(res) => res.public_key, }; + self.transparent_child_keys + .push((self.addresses.len(), child_pk)); Some(child_pk) } Capability::None => None, @@ -297,7 +299,7 @@ impl WalletCapability { &self, config: &ZingoConfig, ) -> Result, String> { - if self.transparent.can_spend() { + if let Capability::Spend(ref ext_sk) = self.transparent { Ok(self .addresses .iter() @@ -310,13 +312,19 @@ impl WalletCapability { ) }) .map(|(taddr, key)| { - let hash = match taddr { - TransparentAddress::PublicKey(hash) => hash, - TransparentAddress::Script(hash) => hash, - }; ( - hash.to_base58check(&config.base58_pubkey_address(), &[]), - key.1, + match taddr { + TransparentAddress::PublicKey(hash) => { + hash.to_base58check(&config.base58_pubkey_address(), &[]) + } + TransparentAddress::Script(hash) => { + hash.to_base58check(&config.base58_script_address(), &[]) + } + }, + ext_sk + .derive_private_key(KeyIndex::from_index(key.0 as u32).unwrap()) + .unwrap() + .private_key, ) }) .collect()) @@ -480,6 +488,38 @@ impl WalletCapability { } } +pub fn orchard_viewkey(ufvk: &Ufvk) -> Option { + ufvk.items().iter().find_map(|receiver| { + if let Fvk::Orchard(fvk_bytes) = receiver { + orchard::keys::FullViewingKey::from_bytes(fvk_bytes) + } else { + None + } + }) +} +pub fn sapling_viewkey( + ufvk: &Ufvk, +) -> Option { + ufvk.items().iter().find_map(|receiver| { + if let Fvk::Sapling(fvk_bytes) = receiver { + zcash_primitives::zip32::sapling::DiversifiableFullViewingKey::from_bytes(fvk_bytes) + } else { + None + } + }) +} + +pub fn transparent_viewkey(ufvk: &Ufvk) -> Option { + use zcash_primitives::legacy::keys::IncomingViewingKey as _; + ufvk.items().iter().find_map(|receiver| { + if let Fvk::P2pkh(fvk_bytes) = receiver { + zcash_primitives::legacy::keys::ExternalIvk::deserialize(fvk_bytes).ok() + } else { + None + } + }) +} + /// Reads a transparent ExtendedPrivKey from a buffer that has a 32 byte private key and 32 byte chain code. fn transparent_key_from_bytes(bytes: &[u8]) -> Result { let mut reader = std::io::Cursor::new(bytes); @@ -731,10 +771,14 @@ pub async fn get_transparent_secretkey_pubkey_taddr( let child_ext_pk = ext_pk.derive_public_key(KeyIndex::Normal(0)).ok(); (None, child_ext_pk.map(|x| x.public_key)) } - Capability::Spend(_) => { - let sk = wc.transparent_child_keys[0].1; - let secp = secp256k1::Secp256k1::new(); - let pk = secp256k1::PublicKey::from_secret_key(&secp, &sk); + Capability::Spend(ext_sk) => { + let pk = wc.transparent_child_keys[0].1; + let sk = ext_sk + .derive_private_key( + KeyIndex::from_index(wc.transparent_child_keys[0].0 as u32).unwrap(), + ) + .unwrap() + .private_key; (Some(sk), Some(pk)) } }; diff --git a/zingolib/src/wallet/transactions.rs b/zingolib/src/wallet/transactions.rs index 127e8f8ab..0850a23e0 100644 --- a/zingolib/src/wallet/transactions.rs +++ b/zingolib/src/wallet/transactions.rs @@ -41,13 +41,6 @@ impl TransactionMetadataSet { 22 } - pub fn get_fee_by_txid(&self, txid: &TxId) -> u64 { - self.current - .get(txid) - .expect("To have the requested txid") - .get_transaction_fee() - } - pub fn read_old( mut reader: R, wallet_capability: &WalletCapability, @@ -469,8 +462,12 @@ impl TransactionMetadataSet { fn mark_notes_as_change_for_pool(notes: &mut [Note]) { notes.iter_mut().for_each(|n| { *n.is_change_mut() = match n.memo() { - Some(Memo::Text(_)) => false, - Some(Memo::Empty | Memo::Arbitrary(_) | Memo::Future(_)) | None => true, + Some(Memo::Arbitrary(arb_memo)) + if zingo_memo::parse_zingo_memo(**arb_memo).is_ok() => + { + true + } + _ => false, } }); }