zcash_client_sqlite: Use ZcashAddress for persistence of sent note …

…addresses

Prior to this change, the recipient of a sent transaction would always
be shown as the protocol-level address, instead of any unified address
intended as the recipient. Now, instead of reencoding the recipient
address, we use the original `ZcashAddress` value from the payment
request.

components/zcash_protocol/src/lib.rs

@@ -41,6 +41,12 @@ pub enum PoolType {
     Shielded(ShieldedProtocol),
 }
 
+impl PoolType {
+    pub const TRANSPARENT: PoolType = PoolType::Transparent;
+    pub const SAPLING: PoolType = PoolType::Shielded(ShieldedProtocol::Sapling);
+    pub const ORCHARD: PoolType = PoolType::Shielded(ShieldedProtocol::Orchard);
+}
+
 impl fmt::Display for PoolType {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {

zcash_client_backend/CHANGELOG.md

@@ -21,6 +21,7 @@ and this library adheres to Rust's notion of
 - `zcash_client_backend::fees::ChangeValue::orchard`
 - `zcash_client_backend::wallet`:
   - `Note::Orchard`
+- `zcash_client_backend::PoolType::{TRANSPARENT, SAPLING, ORCHARD}` constants
 
 ### Changed
 - `zcash_client_backend::data_api`:
@@ -43,6 +44,10 @@ and this library adheres to Rust's notion of
   CHANGELOG for details.
 - `zcash_client_backend::proto::proposal::Proposal::{from_standard_proposal, try_into_standard_proposal`
   each no longer require a `consensus::Parameters` argument.
+- `zcash_client_backend::wallet::Recipient` variants have changed. Instead of
+  wrapping protocol-address types, the `Recipient` type now wraps a
+  `zcash_address::ZcashAddress`. This simplifies the process of tracking the
+  original address to which value was sent.
 
 ## [0.11.0-pre-release] Unreleased
 

zcash_client_backend/src/data_api/wallet.rs

@@ -667,11 +667,11 @@ where
     let mut sapling_output_meta = vec![];
     let mut transparent_output_meta = vec![];
     for payment in proposal.transaction_request().payments() {
-        let recipient_address = payment
-            .recipient_address
+        let recipient_zaddr = payment.recipient_address.clone();
+        match recipient_zaddr
             .clone()
-            .convert_if_network::<Address>(params.network_type())?;
-        match recipient_address {
+            .convert_if_network::<Address>(params.network_type())?
+        {
             Address::Unified(ua) => {
                 let memo = payment
                     .memo
@@ -686,10 +686,7 @@ where
                         memo.clone(),
                     )?;
                     sapling_output_meta.push((
-                        Recipient::Unified(
-                            ua.clone(),
-                            PoolType::Shielded(ShieldedProtocol::Sapling),
-                        ),
+                        Recipient::External(recipient_zaddr, PoolType::SAPLING),
                         payment.amount,
                         Some(memo),
                     ));
@@ -711,15 +708,23 @@ where
                     .as_ref()
                     .map_or_else(MemoBytes::empty, |m| m.clone());
                 builder.add_sapling_output(external_ovk, addr, payment.amount, memo.clone())?;
-                sapling_output_meta.push((Recipient::Sapling(addr), payment.amount, Some(memo)));
+                sapling_output_meta.push((
+                    Recipient::External(recipient_zaddr, PoolType::SAPLING),
+                    payment.amount,
+                    Some(memo),
+                ));
             }
             Address::Transparent(to) => {
                 if payment.memo.is_some() {
                     return Err(Error::MemoForbidden);
                 } else {
                     builder.add_transparent_output(&to, payment.amount)?;
                 }
-                transparent_output_meta.push((to, payment.amount));
+                transparent_output_meta.push((
+                    Recipient::External(recipient_zaddr, PoolType::TRANSPARENT),
+                    to,
+                    payment.amount,
+                ));
             }
         }
     }
@@ -737,19 +742,14 @@ where
                     memo.clone(),
                 )?;
                 sapling_output_meta.push((
-                    Recipient::InternalAccount(
-                        account,
-                        PoolType::Shielded(ShieldedProtocol::Sapling),
-                    ),
+                    Recipient::Internal(account, PoolType::SAPLING),
                     change_value.value(),
                     Some(memo),
                 ))
             }
             ShieldedProtocol::Orchard => {
                 #[cfg(not(feature = "orchard"))]
-                return Err(Error::UnsupportedPoolType(PoolType::Shielded(
-                    ShieldedProtocol::Orchard,
-                )));
+                return Err(Error::UnsupportedPoolType(PoolType::ORCHARD));
 
                 #[cfg(feature = "orchard")]
                 unimplemented!("FIXME: implement Orchard change output creation.")
@@ -771,10 +771,7 @@ where
                     .output_index(i)
                     .expect("An output should exist in the transaction for each Sapling payment.");
 
-                let received_as = if let Recipient::InternalAccount(
-                    account,
-                    PoolType::Shielded(ShieldedProtocol::Sapling),
-                ) = recipient
+                let received_as = if let Recipient::Internal(account, PoolType::SAPLING) = recipient
                 {
                     build_result
                         .transaction()
@@ -794,28 +791,27 @@ where
                 SentTransactionOutput::from_parts(output_index, recipient, value, memo, received_as)
             });
 
-    let transparent_outputs = transparent_output_meta.into_iter().map(|(addr, value)| {
-        let script = addr.script();
-        let output_index = build_result
-            .transaction()
-            .transparent_bundle()
-            .and_then(|b| {
-                b.vout
-                    .iter()
-                    .enumerate()
-                    .find(|(_, tx_out)| tx_out.script_pubkey == script)
-            })
-            .map(|(index, _)| index)
-            .expect("An output should exist in the transaction for each transparent payment.");
-
-        SentTransactionOutput::from_parts(
-            output_index,
-            Recipient::Transparent(addr),
-            value,
-            None,
-            None,
-        )
-    });
+    let transparent_outputs =
+        transparent_output_meta
+            .into_iter()
+            .map(|(recipient, addr, value)| {
+                let script = addr.script();
+                let output_index = build_result
+                    .transaction()
+                    .transparent_bundle()
+                    .and_then(|b| {
+                        b.vout
+                            .iter()
+                            .enumerate()
+                            .find(|(_, tx_out)| tx_out.script_pubkey == script)
+                    })
+                    .map(|(index, _)| index)
+                    .expect(
+                        "An output should exist in the transaction for each transparent payment.",
+                    );
+
+                SentTransactionOutput::from_parts(output_index, recipient, value, None, None)
+            });
 
     wallet_db
         .store_sent_tx(&SentTransaction {

zcash_client_backend/src/wallet.rs

@@ -2,6 +2,7 @@
 //! light client.
 
 use incrementalmerkletree::Position;
+use zcash_address::ZcashAddress;
 use zcash_note_encryption::EphemeralKeyBytes;
 use zcash_primitives::{
     consensus::BlockHeight,
@@ -17,7 +18,7 @@ use zcash_primitives::{
     zip32::{AccountId, Scope},
 };
 
-use crate::{address::UnifiedAddress, fees::sapling as sapling_fees, PoolType, ShieldedProtocol};
+use crate::{fees::sapling as sapling_fees, PoolType, ShieldedProtocol};
 
 #[cfg(feature = "orchard")]
 use crate::fees::orchard as orchard_fees;
@@ -63,10 +64,8 @@ impl NoteId {
 /// output.
 #[derive(Debug, Clone)]
 pub enum Recipient {
-    Transparent(TransparentAddress),
-    Sapling(sapling::PaymentAddress),
-    Unified(UnifiedAddress, PoolType),
-    InternalAccount(AccountId, PoolType),
+    External(ZcashAddress, PoolType),
+    Internal(AccountId, PoolType),
 }
 
 /// A subset of a [`Transaction`] relevant to wallets and light clients.

zcash_client_sqlite/CHANGELOG.md

@@ -15,6 +15,8 @@ and this library adheres to Rust's notion of
 - `zcash_client_sqlite::error::SqliteClientError` has new error variants:
   - `SqliteClientError::UnsupportedPoolType`
   - `SqliteClientError::BalanceError`
+  - The `Bech32DecodeError` variant has been replaced with a more general
+    `DecodingError` type.
 
 ## [0.8.1] - 2023-10-18
 

zcash_client_sqlite/src/error.rs

@@ -4,11 +4,9 @@ use std::error;
 use std::fmt;
 
 use shardtree::error::ShardTreeError;
+use zcash_address::ParseError;
 use zcash_client_backend::keys::AddressGenerationError;
-use zcash_client_backend::{
-    encoding::{Bech32DecodeError, TransparentCodecError},
-    PoolType,
-};
+use zcash_client_backend::PoolType;
 use zcash_primitives::{
     consensus::BlockHeight, transaction::components::amount::BalanceError, zip32::AccountId,
 };
@@ -17,7 +15,10 @@ use crate::wallet::commitment_tree;
 use crate::PRUNING_DEPTH;
 
 #[cfg(feature = "transparent-inputs")]
-use zcash_primitives::legacy::TransparentAddress;
+use {
+    zcash_client_backend::encoding::TransparentCodecError,
+    zcash_primitives::legacy::TransparentAddress,
+};
 
 /// The primary error type for the SQLite wallet backend.
 #[derive(Debug)]
@@ -38,15 +39,16 @@ pub enum SqliteClientError {
     /// Illegal attempt to reinitialize an already-initialized wallet database.
     TableNotEmpty,
 
-    /// A Bech32-encoded key or address decoding error
-    Bech32DecodeError(Bech32DecodeError),
+    /// A Zcash key or address decoding error
+    DecodingError(ParseError),
 
     /// An error produced in legacy transparent address derivation
     #[cfg(feature = "transparent-inputs")]
     HdwalletError(hdwallet::error::Error),
 
     /// An error encountered in decoding a transparent address from its
     /// serialized form.
+    #[cfg(feature = "transparent-inputs")]
     TransparentAddress(TransparentCodecError),
 
     /// Wrapper for rusqlite errors.
@@ -116,7 +118,6 @@ impl error::Error for SqliteClientError {
     fn source(&self) -> Option<&(dyn error::Error + 'static)> {
         match &self {
             SqliteClientError::InvalidMemo(e) => Some(e),
-            SqliteClientError::Bech32DecodeError(Bech32DecodeError::Bech32Error(e)) => Some(e),
             SqliteClientError::DbError(e) => Some(e),
             SqliteClientError::Io(e) => Some(e),
             SqliteClientError::BalanceError(e) => Some(e),
@@ -138,9 +139,10 @@ impl fmt::Display for SqliteClientError {
                 write!(f, "The note ID associated with an inserted witness must correspond to a received note."),
             SqliteClientError::RequestedRewindInvalid(h, r) =>
                 write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet; the requested height was {}.", PRUNING_DEPTH, h, r),
-            SqliteClientError::Bech32DecodeError(e) => write!(f, "{}", e),
+            SqliteClientError::DecodingError(e) => write!(f, "{}", e),
             #[cfg(feature = "transparent-inputs")]
             SqliteClientError::HdwalletError(e) => write!(f, "{:?}", e),
+            #[cfg(feature = "transparent-inputs")]
             SqliteClientError::TransparentAddress(e) => write!(f, "{}", e),
             SqliteClientError::TableNotEmpty => write!(f, "Table is not empty"),
             SqliteClientError::DbError(e) => write!(f, "{}", e),
@@ -176,10 +178,9 @@ impl From<std::io::Error> for SqliteClientError {
         SqliteClientError::Io(e)
     }
 }
-
-impl From<Bech32DecodeError> for SqliteClientError {
-    fn from(e: Bech32DecodeError) -> Self {
-        SqliteClientError::Bech32DecodeError(e)
+impl From<ParseError> for SqliteClientError {
+    fn from(e: ParseError) -> Self {
+        SqliteClientError::DecodingError(e)
     }
 }
 
@@ -196,6 +197,7 @@ impl From<hdwallet::error::Error> for SqliteClientError {
     }
 }
 
+#[cfg(feature = "transparent-inputs")]
 impl From<TransparentCodecError> for SqliteClientError {
     fn from(e: TransparentCodecError) -> Self {
         SqliteClientError::TransparentAddress(e)

zcash_client_sqlite/src/lib.rs

@@ -45,6 +45,7 @@ use std::{
 
 use incrementalmerkletree::Position;
 use shardtree::{error::ShardTreeError, ShardTree};
+use zcash_keys::address::Address;
 use zcash_primitives::{
     block::BlockHash,
     consensus::{self, BlockHeight},
@@ -93,7 +94,7 @@ pub mod error;
 pub mod wallet;
 use wallet::{
     commitment_tree::{self, put_shard_roots},
-    SubtreeScanProgress,
+    Receiver, SubtreeScanProgress,
 };
 
 #[cfg(test)]
@@ -597,17 +598,20 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                 match output.transfer_type {
                     TransferType::Outgoing | TransferType::WalletInternal => {
                         let recipient = if output.transfer_type == TransferType::Outgoing {
-                            Recipient::Sapling(output.note.recipient())
+                            let receiver = Receiver::Sapling(output.note.recipient());
+                            let wallet_address = wallet::select_receiving_address(&wdb.params, wdb.conn.0,output.account, &receiver)?.unwrap_or_else(||
+                                Address::Sapling(output.note.recipient()).to_zcash_address(&wdb.params)
+                            );
+                            Recipient::External(wallet_address, PoolType::SAPLING)
                         } else {
-                            Recipient::InternalAccount(
+                            Recipient::Internal(
                                 output.account,
                                 PoolType::Shielded(ShieldedProtocol::Sapling)
                             )
                         };
 
                         wallet::put_sent_output(
                             wdb.conn.0,
-                            &wdb.params,
                             output.account,
                             tx_ref,
                             output.index,
@@ -620,7 +624,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                             Some(&output.memo),
                         )?;
 
-                        if matches!(recipient, Recipient::InternalAccount(_, _)) {
+                        if matches!(recipient, Recipient::Internal(_, _)) {
                             wallet::sapling::put_received_note(wdb.conn.0, output, tx_ref, None)?;
                         }
                     }
@@ -660,13 +664,13 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                 ) {
                     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() {
+                            let recipient_zaddr = Address::Transparent(address).to_zcash_address(&wdb.params);
                             wallet::put_sent_output(
                                 wdb.conn.0,
-                                &wdb.params,
                                 *account_id,
                                 tx_ref,
                                 output_index,
-                                &Recipient::Transparent(address),
+                                &Recipient::External(recipient_zaddr, PoolType::TRANSPARENT),
                                 txout.value,
                                 None
                             )?;
@@ -712,13 +716,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
             }
 
             for output in &sent_tx.outputs {
-                wallet::insert_sent_output(
-                    wdb.conn.0,
-                    &wdb.params,
-                    tx_ref,
-                    sent_tx.account,
-                    output,
-                )?;
+                wallet::insert_sent_output(wdb.conn.0, tx_ref, sent_tx.account, output)?;
 
                 if let Some((account, note)) = output.sapling_change_to() {
                     wallet::sapling::put_received_note(