From 151e6e526e52f96f2c686516daf8cc7cecf18c1a Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 25 Mar 2024 07:45:42 -0600 Subject: [PATCH] zcash_client_backend: Track external addresses in inter-account transactions. Previously, if the funding account for a received transaction output was determined to be an account known to the wallet, the output was recorded as though it were sent to an internal (change) address of the wallet. --- zcash_client_backend/CHANGELOG.md | 2 + zcash_client_backend/src/data_api/wallet.rs | 18 +++--- zcash_client_backend/src/wallet.rs | 27 ++++++++- zcash_client_sqlite/src/lib.rs | 65 +++++++++++++-------- zcash_client_sqlite/src/wallet.rs | 10 +++- 5 files changed, 85 insertions(+), 37 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 0f0e0d516..07b8346c7 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 aa73bd3f8..922d2544c 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/wallet.rs b/zcash_client_backend/src/wallet.rs index e849d5d70..418bb3e3a 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 cc9d0b9ad..9b1d02693 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}, @@ -1066,10 +1067,11 @@ impl WalletWrite for WalletDb //TODO: Recover the UA, if possible. Recipient::Sapling(output.note().recipient()) } else { - Recipient::InternalAccount( - *output.account(), - Note::Sapling(output.note().clone()), - ) + Recipient::InternalAccount { + receiving_account: *output.account(), + external_address: None, + note: Note::Sapling(output.note().clone()), + } }; wallet::put_sent_output( @@ -1083,7 +1085,7 @@ impl WalletWrite for WalletDb Some(output.memo()), )?; - if matches!(recipient, Recipient::InternalAccount(_, _)) { + if matches!(recipient, Recipient::InternalAccount { .. }) { wallet::sapling::put_received_note(wdb.conn.0, output, tx_ref, None)?; } } @@ -1091,11 +1093,12 @@ impl WalletWrite for WalletDb wallet::sapling::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( - *output.account(), - Note::Sapling(output.note().clone()), - ); + 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, @@ -1128,10 +1131,11 @@ impl WalletWrite for WalletDb PoolType::Shielded(ShieldedProtocol::Orchard), ) } else { - Recipient::InternalAccount( - *output.account(), - Note::Orchard(*output.note()), - ) + Recipient::InternalAccount { + receiving_account: *output.account(), + external_address: None, + note: Note::Orchard(*output.note()), + } }; wallet::put_sent_output( @@ -1145,7 +1149,7 @@ impl WalletWrite for WalletDb Some(output.memo()), )?; - if matches!(recipient, Recipient::InternalAccount(_, _)) { + if matches!(recipient, Recipient::InternalAccount { .. }) { wallet::orchard::put_received_note(wdb.conn.0, output, tx_ref, None)?; } } @@ -1154,10 +1158,17 @@ impl WalletWrite for WalletDb if let Some(account_id) = funding_account { // Even if the recipient address is external, record the send as internal. - let recipient = Recipient::InternalAccount( - *output.account(), - Note::Orchard(*output.note()), - ); + 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, @@ -1288,13 +1299,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()), @@ -1305,13 +1320,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/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 5356b2704..efbaad5b3 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -2527,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()), ), }