diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 0f0e0d5167..07b8346c7b 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -103,6 +103,8 @@ and this library adheres to Rust's notion of feature flag. - `zcash_client_backend::proto`: - `ProposalDecodingError` has a new variant `TransparentMemo`. +- `zcash_client_backend::wallet::Recipient::InternalAccount` is now a structured + variant with an additional `external_address` field. - `zcash_client_backend::zip321::render::amount_str` now takes a `NonNegativeAmount` rather than a signed `Amount` as its argument. - `zcash_client_backend::zip321::parse::parse_amount` now parses a diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index aa73bd3f8c..922d2544c1 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -1049,10 +1049,11 @@ where memo.clone(), )?; sapling_output_meta.push(( - Recipient::InternalAccount( - account, - PoolType::Shielded(ShieldedProtocol::Sapling), - ), + Recipient::InternalAccount { + receiving_account: account, + external_address: None, + note: PoolType::Shielded(ShieldedProtocol::Sapling), + }, change_value.value(), Some(memo), )) @@ -1072,10 +1073,11 @@ where memo.clone(), )?; orchard_output_meta.push(( - Recipient::InternalAccount( - account, - PoolType::Shielded(ShieldedProtocol::Orchard), - ), + Recipient::InternalAccount { + receiving_account: account, + external_address: None, + note: PoolType::Shielded(ShieldedProtocol::Orchard), + }, change_value.value(), Some(memo), )) diff --git a/zcash_client_backend/src/decrypt.rs b/zcash_client_backend/src/decrypt.rs index 0db6c94813..51f9abcdbd 100644 --- a/zcash_client_backend/src/decrypt.rs +++ b/zcash_client_backend/src/decrypt.rs @@ -166,12 +166,8 @@ pub fn decrypt_transaction<'a, P: consensus::Parameters, AccountId: Copy>( .flat_map(|bundle| { ufvks .iter() - .flat_map(move |(account, ufvk)| { - ufvk.orchard() - .into_iter() - .map(|fvk| (account.to_owned(), fvk)) - }) - .flat_map(move |(account, fvk)| { + .flat_map(|(account, ufvk)| ufvk.orchard().into_iter().map(|fvk| (*account, fvk))) + .flat_map(|(account, fvk)| { let ivk_external = orchard::keys::PreparedIncomingViewingKey::new( &fvk.to_ivk(Scope::External), ); diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index e849d5d707..418bb3e3a1 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -2,6 +2,7 @@ //! light client. use incrementalmerkletree::Position; +use zcash_keys::address::Address; use zcash_note_encryption::EphemeralKeyBytes; use zcash_primitives::{ consensus::BlockHeight, @@ -70,7 +71,11 @@ pub enum Recipient { Transparent(TransparentAddress), Sapling(sapling::PaymentAddress), Unified(UnifiedAddress, PoolType), - InternalAccount(AccountId, N), + InternalAccount { + receiving_account: AccountId, + external_address: Option
, + note: N, + }, } impl Recipient { @@ -79,7 +84,15 @@ impl Recipient { Recipient::Transparent(t) => Recipient::Transparent(t), Recipient::Sapling(s) => Recipient::Sapling(s), Recipient::Unified(u, p) => Recipient::Unified(u, p), - Recipient::InternalAccount(a, n) => Recipient::InternalAccount(a, f(n)), + Recipient::InternalAccount { + receiving_account, + external_address, + note, + } => Recipient::InternalAccount { + receiving_account, + external_address, + note: f(note), + }, } } } @@ -90,7 +103,15 @@ impl Recipient> { Recipient::Transparent(t) => Some(Recipient::Transparent(t)), Recipient::Sapling(s) => Some(Recipient::Sapling(s)), Recipient::Unified(u, p) => Some(Recipient::Unified(u, p)), - Recipient::InternalAccount(a, n) => n.map(|n0| Recipient::InternalAccount(a, n0)), + Recipient::InternalAccount { + receiving_account, + external_address, + note, + } => note.map(|n0| Recipient::InternalAccount { + receiving_account, + external_address, + note: n0, + }), } } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 8b51b06c81..ca4245330a 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -65,6 +65,7 @@ use zcash_client_backend::{ wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput}, DecryptedOutput, PoolType, ShieldedProtocol, TransferType, }; +use zcash_keys::address::Address; use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight}, @@ -1049,19 +1050,39 @@ impl WalletWrite for WalletDb ) -> Result<(), Self::Error> { self.transactionally(|wdb| { let tx_ref = wallet::put_tx_data(wdb.conn.0, d_tx.tx(), None, None)?; + let funding_accounts = wallet::get_funding_accounts(wdb.conn.0, d_tx.tx())?; + let funding_account = funding_accounts.iter().next().copied(); + if funding_accounts.len() > 1 { + warn!( + "More than one wallet account detected as funding transaction {:?}, selecting {:?}", + d_tx.tx().txid(), + funding_account.unwrap() + ) + } - let mut spending_account_id: Option = None; for output in d_tx.sapling_outputs() { match output.transfer_type() { - TransferType::Outgoing | TransferType::WalletInternal => { - let recipient = if output.transfer_type() == TransferType::Outgoing { - //TODO: Recover the UA, if possible. - Recipient::Sapling(output.note().recipient()) - } else { - Recipient::InternalAccount( - *output.account(), - Note::Sapling(output.note().clone()), - ) + TransferType::Outgoing => { + //TODO: Recover the UA, if possible. + let recipient = Recipient::Sapling(output.note().recipient()); + wallet::put_sent_output( + wdb.conn.0, + &wdb.params, + *output.account(), + tx_ref, + output.index(), + &recipient, + output.note_value(), + Some(output.memo()), + )?; + } + TransferType::WalletInternal => { + wallet::sapling::put_received_note(wdb.conn.0, output, tx_ref, None)?; + + let recipient = Recipient::InternalAccount { + receiving_account: *output.account(), + external_address: None, + note: Note::Sapling(output.note().clone()), }; wallet::put_sent_output( @@ -1074,24 +1095,29 @@ impl WalletWrite for WalletDb output.note_value(), Some(output.memo()), )?; - - if matches!(recipient, Recipient::InternalAccount(_, _)) { - wallet::sapling::put_received_note(wdb.conn.0, output, tx_ref, None)?; - } } TransferType::Incoming => { - match spending_account_id { - Some(id) => { - if id != *output.account() { - panic!("Unable to determine a unique account identifier for z->t spend."); - } - } - None => { - spending_account_id = Some(*output.account()); - } - } - wallet::sapling::put_received_note(wdb.conn.0, output, tx_ref, None)?; + + if let Some(account_id) = funding_account { + let recipient = Recipient::InternalAccount { + receiving_account: *output.account(), + // TODO: recover the actual UA, if possible + external_address: Some(Address::Sapling(output.note().recipient())), + note: Note::Sapling(output.note().clone()), + }; + + wallet::put_sent_output( + wdb.conn.0, + &wdb.params, + account_id, + tx_ref, + output.index(), + &recipient, + output.note_value(), + Some(output.memo()), + )?; + } } } } @@ -1099,23 +1125,17 @@ impl WalletWrite for WalletDb #[cfg(feature = "orchard")] for output in d_tx.orchard_outputs() { match output.transfer_type() { - TransferType::Outgoing | TransferType::WalletInternal => { - let recipient = if output.transfer_type() == TransferType::Outgoing { - // TODO: Recover the actual UA, if possible. - Recipient::Unified( - UnifiedAddress::from_receivers( - Some(output.note().recipient()), - None, - None - ).expect("UA has an Orchard receiver by construction."), - PoolType::Shielded(ShieldedProtocol::Orchard) - ) - } else { - Recipient::InternalAccount( - *output.account(), - Note::Orchard(*output.note()), + TransferType::Outgoing => { + // TODO: Recover the actual UA, if possible. + let recipient = Recipient::Unified( + UnifiedAddress::from_receivers( + Some(output.note().recipient()), + None, + None, ) - }; + .expect("UA has an Orchard receiver by construction."), + PoolType::Shielded(ShieldedProtocol::Orchard), + ); wallet::put_sent_output( wdb.conn.0, @@ -1127,56 +1147,98 @@ impl WalletWrite for WalletDb output.note_value(), Some(output.memo()), )?; + } + TransferType::WalletInternal => { + wallet::orchard::put_received_note(wdb.conn.0, output, tx_ref, None)?; - if matches!(recipient, Recipient::InternalAccount(_, _)) { - wallet::orchard::put_received_note(wdb.conn.0, output, tx_ref, None)?; - } + let recipient = Recipient::InternalAccount { + receiving_account: *output.account(), + external_address: None, + note: Note::Orchard(*output.note()), + }; + + wallet::put_sent_output( + wdb.conn.0, + &wdb.params, + *output.account(), + tx_ref, + output.index(), + &recipient, + output.note_value(), + Some(output.memo()), + )?; } TransferType::Incoming => { - match spending_account_id { - Some(id) => { - if id != *output.account() { - panic!("Unable to determine a unique account identifier for z->t spend."); - } - } - None => { - spending_account_id = Some(*output.account()); - } - } - wallet::orchard::put_received_note(wdb.conn.0, output, tx_ref, None)?; + + if let Some(account_id) = funding_account { + // Even if the recipient address is external, record the send as internal. + let recipient = Recipient::InternalAccount { + receiving_account: *output.account(), + // TODO: recover the actual UA, if possible + external_address: Some(Address::Unified( + UnifiedAddress::from_receivers( + Some(output.note().recipient()), + None, + None, + ).expect("UA has an Orchard receiver by construction."))), + note: Note::Orchard(*output.note()), + }; + + wallet::put_sent_output( + wdb.conn.0, + &wdb.params, + account_id, + tx_ref, + output.index(), + &recipient, + output.note_value(), + Some(output.memo()), + )?; + } } } } // If any of the utxos spent in the transaction are ours, mark them as spent. #[cfg(feature = "transparent-inputs")] - for txin in d_tx.tx().transparent_bundle().iter().flat_map(|b| b.vin.iter()) { + for txin in d_tx + .tx() + .transparent_bundle() + .iter() + .flat_map(|b| b.vin.iter()) + { wallet::mark_transparent_utxo_spent(wdb.conn.0, tx_ref, &txin.prevout)?; } // If we have some transparent outputs: - if d_tx.tx().transparent_bundle().iter().any(|b| !b.vout.is_empty()) { + if d_tx + .tx() + .transparent_bundle() + .iter() + .any(|b| !b.vout.is_empty()) + { // If the transaction contains spends from our wallet, we will store z->t // transactions we observe in the same way they would be stored by // create_spend_to_address. - let sapling_from_account = wdb.get_sapling_nullifiers(NullifierQuery::All)?.into_iter().find( - |(_, nf)| - d_tx.tx().sapling_bundle().into_iter().flat_map(|b| b.shielded_spends().iter()) - .any(|input| nf == input.nullifier()) - ).map(|(account_id, _)| account_id); - - #[cfg(feature = "orchard")] - let orchard_from_account = wdb.get_orchard_nullifiers(NullifierQuery::All)?.into_iter().find( - |(_, nf)| - d_tx.tx().orchard_bundle().iter().flat_map(|b| b.actions().iter()) - .any(|input| nf == input.nullifier()) - ).map(|(account_id, _)| account_id); - #[cfg(not(feature = "orchard"))] - let orchard_from_account = None; + let funding_accounts = wallet::get_funding_accounts(wdb.conn.0, d_tx.tx())?; + let funding_account = funding_accounts.iter().next().copied(); + if let Some(account_id) = funding_account { + if funding_accounts.len() > 1 { + warn!( + "More than one wallet account detected as funding transaction {:?}, selecting {:?}", + d_tx.tx().txid(), + account_id + ) + } - if let Some(account_id) = orchard_from_account.or(sapling_from_account) { - for (output_index, txout) in d_tx.tx().transparent_bundle().iter().flat_map(|b| b.vout.iter()).enumerate() { + for (output_index, txout) in d_tx + .tx() + .transparent_bundle() + .iter() + .flat_map(|b| b.vout.iter()) + .enumerate() + { if let Some(address) = txout.recipient_address() { wallet::put_sent_output( wdb.conn.0, @@ -1186,7 +1248,7 @@ impl WalletWrite for WalletDb output_index, &Recipient::Transparent(address), txout.value, - None + None, )?; } } @@ -1252,13 +1314,17 @@ impl WalletWrite for WalletDb )?; match output.recipient() { - Recipient::InternalAccount(account, Note::Sapling(note)) => { + Recipient::InternalAccount { + receiving_account, + note: Note::Sapling(note), + .. + } => { wallet::sapling::put_received_note( wdb.conn.0, &DecryptedOutput::new( output.output_index(), note.clone(), - *account, + *receiving_account, output .memo() .map_or_else(MemoBytes::empty, |memo| memo.clone()), @@ -1269,13 +1335,17 @@ impl WalletWrite for WalletDb )?; } #[cfg(feature = "orchard")] - Recipient::InternalAccount(account, Note::Orchard(note)) => { + Recipient::InternalAccount { + receiving_account, + note: Note::Orchard(note), + .. + } => { wallet::orchard::put_received_note( wdb.conn.0, &DecryptedOutput::new( output.output_index(), *note, - *account, + *receiving_account, output .memo() .map_or_else(MemoBytes::empty, |memo| memo.clone()), diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index c0a37cc71b..3b4f9d066a 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -33,7 +33,10 @@ use zcash_client_backend::{ self, chain::{self, ChainState, CommitmentTreeRoot, ScanSummary}, error::Error, - wallet::input_selection::{GreedyInputSelector, GreedyInputSelectorError}, + wallet::{ + decrypt_and_store_transaction, + input_selection::{GreedyInputSelector, GreedyInputSelectorError}, + }, AccountBirthday, DecryptedTransaction, Ratio, WalletRead, WalletSummary, WalletWrite, }, decrypt_transaction, @@ -235,23 +238,25 @@ pub(crate) fn send_single_step_proposed_transfer() { assert!(found_tx_empty_memo); // Verify that the stored sent notes match what we're expecting - let mut stmt_sent_notes = st - .wallet() - .conn - .prepare( - "SELECT output_index - FROM sent_notes - JOIN transactions ON transactions.id_tx = sent_notes.tx - WHERE transactions.txid = ?", - ) - .unwrap(); + let sent_note_ids = { + let mut stmt_sent_notes = st + .wallet() + .conn + .prepare( + "SELECT output_index + FROM sent_notes + JOIN transactions ON transactions.id_tx = sent_notes.tx + WHERE transactions.txid = ?", + ) + .unwrap(); - let sent_note_ids = stmt_sent_notes - .query(rusqlite::params![sent_tx_id.as_ref()]) - .unwrap() - .mapped(|row| Ok(NoteId::new(sent_tx_id, T::SHIELDED_PROTOCOL, row.get(0)?))) - .collect::, _>>() - .unwrap(); + stmt_sent_notes + .query(rusqlite::params![sent_tx_id.as_ref()]) + .unwrap() + .mapped(|row| Ok(NoteId::new(sent_tx_id, T::SHIELDED_PROTOCOL, row.get(0)?))) + .collect::, _>>() + .unwrap() + }; assert_eq!(sent_note_ids.len(), 2); @@ -288,6 +293,11 @@ pub(crate) fn send_single_step_proposed_transfer() { let tx_history = st.get_tx_history().unwrap(); assert_eq!(tx_history.len(), 2); + + assert_matches!( + decrypt_and_store_transaction(&st.network(), st.wallet_mut(), &tx), + Ok(_) + ); } #[cfg(feature = "transparent-inputs")] diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 9fd7848d3c..e397c71211 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -70,7 +70,7 @@ use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; use zip32::fingerprint::SeedFingerprint; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::convert::TryFrom; use std::io::{self, Cursor}; use std::num::NonZeroU32; @@ -139,6 +139,8 @@ pub mod init; pub(crate) mod orchard; pub(crate) mod sapling; pub(crate) mod scanning; +#[cfg(feature = "transparent-inputs")] +pub(crate) mod transparent; pub(crate) const BLOCK_SAPLING_FRONTIER_ABSENT: &[u8] = &[0x0]; @@ -1499,6 +1501,40 @@ pub(crate) fn get_transaction( .transpose() } +pub(crate) fn get_funding_accounts( + conn: &rusqlite::Connection, + tx: &Transaction, +) -> Result, rusqlite::Error> { + let mut funding_accounts = HashSet::new(); + #[cfg(feature = "transparent-inputs")] + funding_accounts.extend(transparent::detect_spending_accounts( + conn, + tx.transparent_bundle() + .iter() + .flat_map(|bundle| bundle.vin.iter().map(|txin| &txin.prevout)), + )?); + + funding_accounts.extend(sapling::detect_spending_accounts( + conn, + tx.sapling_bundle().iter().flat_map(|bundle| { + bundle + .shielded_spends() + .iter() + .map(|spend| spend.nullifier()) + }), + )?); + + #[cfg(feature = "orchard")] + funding_accounts.extend(orchard::detect_spending_accounts( + conn, + tx.orchard_bundle() + .iter() + .flat_map(|bundle| bundle.actions().iter().map(|action| action.nullifier())), + )?); + + Ok(funding_accounts) +} + /// Returns the memo for a sent note, if the sent note is known to the wallet. pub(crate) fn get_sent_memo( conn: &rusqlite::Connection, @@ -1852,9 +1888,10 @@ pub(crate) fn get_tx_height( conn.query_row( "SELECT block FROM transactions WHERE txid = ?", [txid.as_ref().to_vec()], - |row| row.get(0).map(u32::into), + |row| Ok(row.get::<_, Option>(0)?.map(BlockHeight::from)), ) .optional() + .map(|opt| opt.flatten()) } /// Returns the block hash for the block at the specified height, @@ -2490,9 +2527,13 @@ fn recipient_params( PoolType::Shielded(ShieldedProtocol::Sapling), ), Recipient::Unified(addr, pool) => (Some(addr.encode(params)), None, *pool), - Recipient::InternalAccount(id, note) => ( - None, - Some(id.to_owned()), + Recipient::InternalAccount { + receiving_account, + external_address, + note, + } => ( + external_address.as_ref().map(|a| a.encode(params)), + Some(*receiving_account), PoolType::Shielded(note.protocol()), ), } @@ -2564,7 +2605,7 @@ pub(crate) fn put_sent_output( ON CONFLICT (tx, output_pool, output_index) DO UPDATE SET from_account_id = :from_account_id, to_address = :to_address, - to_account_id = :to_account_id, + to_account_id = IFNULL(to_account_id, :to_account_id), value = :value, memo = IFNULL(:memo, memo)", )?; diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 27c1559bc1..50833680ab 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -1,9 +1,11 @@ +use std::{collections::HashSet, rc::Rc}; + use incrementalmerkletree::Position; use orchard::{ keys::Diversifier, note::{Note, Nullifier, RandomSeed, Rho}, }; -use rusqlite::{named_params, Connection, Row, Transaction}; +use rusqlite::{named_params, types::Value, Connection, Row, Transaction}; use zcash_client_backend::{ data_api::NullifierQuery, @@ -335,6 +337,27 @@ pub(crate) fn get_orchard_nullifiers( Ok(res) } +pub(crate) fn detect_spending_accounts<'a>( + conn: &Connection, + nfs: impl Iterator, +) -> Result, rusqlite::Error> { + let mut account_q = conn.prepare_cached( + "SELECT rn.account_id + FROM orchard_received_notes rn + WHERE rn.nf IN rarray(:nf_ptr)", + )?; + + let nf_values: Vec = nfs.map(|nf| Value::Blob(nf.to_bytes().to_vec())).collect(); + let nf_ptr = Rc::new(nf_values); + let res = account_q + .query_and_then(named_params![":nf_ptr": &nf_ptr], |row| { + row.get::<_, u32>(0).map(AccountId) + })? + .collect::, _>>()?; + + Ok(res) +} + /// Marks a given nullifier as having been revealed in the construction /// of the specified transaction. /// diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 8a7c4ad8f7..072623fbee 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -1,8 +1,10 @@ //! Functions for Sapling support in the wallet. +use std::{collections::HashSet, rc::Rc}; + use group::ff::PrimeField; use incrementalmerkletree::Position; -use rusqlite::{named_params, Connection, Row, Transaction}; +use rusqlite::{named_params, types::Value, Connection, Row, Transaction}; use sapling::{self, Diversifier, Nullifier, Rseed}; use zcash_client_backend::{ @@ -269,6 +271,27 @@ pub(crate) fn get_sapling_nullifiers( Ok(res) } +pub(crate) fn detect_spending_accounts<'a>( + conn: &Connection, + nfs: impl Iterator, +) -> Result, rusqlite::Error> { + let mut account_q = conn.prepare_cached( + "SELECT rn.account_id + FROM sapling_received_notes rn + WHERE rn.nf IN rarray(:nf_ptr)", + )?; + + let nf_values: Vec = nfs.map(|nf| Value::Blob(nf.to_vec())).collect(); + let nf_ptr = Rc::new(nf_values); + let res = account_q + .query_and_then(named_params![":nf_ptr": &nf_ptr], |row| { + row.get::<_, u32>(0).map(AccountId) + })? + .collect::, _>>()?; + + Ok(res) +} + /// Marks a given nullifier as having been revealed in the construction /// of the specified transaction. /// diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs new file mode 100644 index 0000000000..63761a7100 --- /dev/null +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -0,0 +1,34 @@ +//! Functions for transparent input support in the wallet. +use std::collections::HashSet; + +use rusqlite::{named_params, Connection}; +use zcash_primitives::transaction::components::OutPoint; + +use crate::AccountId; + +pub(crate) fn detect_spending_accounts<'a>( + conn: &Connection, + spent: impl Iterator, +) -> Result, rusqlite::Error> { + let mut account_q = conn.prepare_cached( + "SELECT received_by_account_id + FROM utxos + WHERE prevout_txid = :prevout_txid + AND prevout_idx = :prevout_idx", + )?; + + let mut acc = HashSet::new(); + for prevout in spent { + for account in account_q.query_and_then( + named_params![ + ":prevout_txid": prevout.hash(), + ":prevout_idx": prevout.n() + ], + |row| row.get::<_, u32>(0).map(AccountId), + )? { + acc.insert(account?); + } + } + + Ok(acc) +}