Require a source transparent address to shield transparent funds.

Previously, `shield_transparent_funds` was only shielding funds
associated with the legacy default transparent address. This meant
that transparent funds sent to unified addresses could not reliably
be shielded, as a unified address will frequently be constructed
using a diversifier index greater than zero.

This modifies the `get_transparent_receivers` method to return address
metadata containing the account ID and diversifier index used to derive
each address along with the receiver.

zcash_client_backend/CHANGELOG.md

@@ -25,6 +25,7 @@ and this library adheres to Rust's notion of
   in any release.
 - `zcash_client_backend::address`:
   - `RecipientAddress::Unified`
+  - `AddressMetadata`
 - `zcash_client_backend::data_api`:
   - `PoolType`
   - `Recipient`
@@ -102,9 +103,17 @@ and this library adheres to Rust's notion of
   `store_decrypted_tx`.
 - `data_api::ReceivedTransaction` has been renamed to `DecryptedTransaction`,
   and its `outputs` field has been renamed to `sapling_outputs`.
-- An `Error::MemoForbidden` error has been added to the
+- `data_api::error::Error` has the following additional cases:
-  `data_api::error::Error` enum to report the condition where a memo was
+  - `Error::MemoForbidden` to report the condition where a memo was
-  specified to be sent to a transparent recipient.
+    specified to be sent to a transparent recipient.
+  - `Error::TransparentInputsNotSupported` to represent the condition
+    where a transparent spend has been requested of a wallet compiled without
+    the `transparent-inputs` feature.
+  - `Error::AddressNotRecognized` to indicate that a transparent address from
+    which funds are being requested to be spent does not appear to be associated
+    with this wallet.
+  - `Error::ChildIndexOutOfRange` to indicate that a diversifier index for an
+    address is out of range for valid transparent child indices.
 - `zcash_client_backend::decrypt`:
   - `decrypt_transaction` now takes a `HashMap<_, UnifiedFullViewingKey>`
     instead of `HashMap<_, ExtendedFullViewingKey>`.

zcash_client_backend/src/address.rs

@@ -6,7 +6,34 @@ use zcash_address::{
     unified::{self, Container, Encoding},
     ConversionError, Network, ToAddress, TryFromRawAddress, ZcashAddress,
 };
-use zcash_primitives::{consensus, legacy::TransparentAddress, sapling::PaymentAddress};
+use zcash_primitives::{
+    consensus,
+    legacy::TransparentAddress,
+    sapling::PaymentAddress,
+    zip32::{AccountId, DiversifierIndex},
+};
+
+pub struct AddressMetadata {
+    account: AccountId,
+    diversifier_index: DiversifierIndex,
+}
+
+impl AddressMetadata {
+    pub fn new(account: AccountId, diversifier_index: DiversifierIndex) -> Self {
+        Self {
+            account,
+            diversifier_index,
+        }
+    }
+
+    pub fn account(&self) -> AccountId {
+        self.account
+    }
+
+    pub fn diversifier_index(&self) -> &DiversifierIndex {
+        &self.diversifier_index
+    }
+}
 
 /// A Unified Address.
 #[derive(Clone, Debug, PartialEq)]

zcash_client_backend/src/data_api.rs

@@ -1,7 +1,7 @@
 //! Interfaces for wallet data persistence & low-level wallet utilities.
 
 use std::cmp;
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::fmt::Debug;
 
 use secrecy::SecretVec;
@@ -17,7 +17,7 @@ use zcash_primitives::{
 };
 
 use crate::{
-    address::UnifiedAddress,
+    address::{AddressMetadata, UnifiedAddress},
     decrypt::DecryptedOutput,
     keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
     proto::compact_formats::CompactBlock,
@@ -207,7 +207,7 @@ pub trait WalletRead {
     fn get_transparent_receivers(
         &self,
         account: AccountId,
-    ) -> Result<HashSet<TransparentAddress>, Self::Error>;
+    ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error>;
 
     /// Returns a list of unspent transparent UTXOs that appear in the chain at heights up to and
     /// including `max_height`.
@@ -400,9 +400,6 @@ pub mod testing {
     use secrecy::{ExposeSecret, SecretVec};
     use std::collections::HashMap;
 
-    #[cfg(feature = "transparent-inputs")]
-    use std::collections::HashSet;
-
     use zcash_primitives::{
         block::BlockHash,
         consensus::{BlockHeight, Network},
@@ -415,7 +412,7 @@ pub mod testing {
     };
 
     use crate::{
-        address::UnifiedAddress,
+        address::{AddressMetadata, UnifiedAddress},
         keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
         proto::compact_formats::CompactBlock,
         wallet::{SpendableNote, WalletTransparentOutput},
@@ -555,8 +552,8 @@ pub mod testing {
         fn get_transparent_receivers(
             &self,
             _account: AccountId,
-        ) -> Result<HashSet<TransparentAddress>, Self::Error> {
+        ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error> {
-            Ok(HashSet::new())
+            Ok(HashMap::new())
         }
 
         fn get_unspent_transparent_outputs(

zcash_client_backend/src/data_api/error.rs

@@ -10,6 +10,9 @@ use zcash_primitives::{
     zip32::AccountId,
 };
 
+#[cfg(feature = "transparent-inputs")]
+use zcash_primitives::{legacy::TransparentAddress, zip32::DiversifierIndex};
+
 #[derive(Debug)]
 pub enum ChainInvalid {
     /// The hash of the parent block given by a proposed new chain tip does
@@ -83,6 +86,12 @@ pub enum Error<NoteId> {
     /// support
     #[cfg(not(feature = "transparent-inputs"))]
     TransparentInputsNotSupported,
+
+    #[cfg(feature = "transparent-inputs")]
+    AddressNotRecognized(TransparentAddress),
+
+    #[cfg(feature = "transparent-inputs")]
+    ChildIndexOutOfRange(DiversifierIndex),
 }
 
 impl ChainInvalid {
@@ -143,6 +152,14 @@ impl<N: fmt::Display> fmt::Display for Error<N> {
             Error::TransparentInputsNotSupported => {
                 write!(f, "This wallet does not support spending or manipulating transparent UTXOs.")
             }
+            #[cfg(feature = "transparent-inputs")]
+            Error::AddressNotRecognized(_) => {
+                write!(f, "The specified transparent address was not recognized as belonging to the wallet.")
+            }
+            #[cfg(feature = "transparent-inputs")]
+            Error::ChildIndexOutOfRange(i) => {
+                write!(f, "The diversifier index {:?} is out of range for transparent addresses.", i)
+            }
         }
     }
 }

zcash_client_backend/src/data_api/wallet.rs

@@ -23,7 +23,7 @@ use crate::{
 };
 
 #[cfg(feature = "transparent-inputs")]
-use zcash_primitives::{legacy::keys::IncomingViewingKey, sapling::keys::OutgoingViewingKey};
+use zcash_primitives::{legacy::TransparentAddress, sapling::keys::OutgoingViewingKey};
 
 /// Scans a [`Transaction`] for any information that can be decrypted by the accounts in
 /// the wallet, and saves it to the wallet.
@@ -440,6 +440,7 @@ pub fn shield_transparent_funds<E, N, P, D, R, U>(
     params: &P,
     prover: impl TxProver,
     usk: &UnifiedSpendingKey,
+    from_addr: &TransparentAddress,
     memo: &MemoBytes,
     min_confirmations: u32,
 ) -> Result<D::TxRef, E>
@@ -465,14 +466,8 @@ where
     let account_pubkey = usk.transparent().to_account_pubkey();
     let ovk = OutgoingViewingKey(account_pubkey.internal_ovk().as_bytes());
 
-    // derive the t-address for the extpubkey at the minimum valid child index
-    let (taddr, child_index) = account_pubkey
-        .derive_external_ivk()
-        .unwrap()
-        .default_address();
-
     // get UTXOs from DB
-    let utxos = wallet_db.get_unspent_transparent_outputs(&taddr, latest_anchor)?;
+    let utxos = wallet_db.get_unspent_transparent_outputs(from_addr, latest_anchor)?;
     let total_amount = utxos
         .iter()
         .map(|utxo| utxo.txout().value)
@@ -488,10 +483,20 @@ where
 
     let mut builder = Builder::new_with_fee(params.clone(), latest_scanned_height, fee);
 
+    let diversifier_index = *wallet_db
+        .get_transparent_receivers(account)?
+        .get(from_addr)
+        .ok_or(Error::AddressNotRecognized(*from_addr))?
+        .diversifier_index();
+
+    let child_index = u32::try_from(diversifier_index)
+        .map_err(|_| Error::ChildIndexOutOfRange(diversifier_index))?;
+
     let secret_key = usk
         .transparent()
         .derive_external_secret_key(child_index)
         .unwrap();
+
     for utxo in &utxos {
         builder
             .add_transparent_input(secret_key, utxo.outpoint().clone(), utxo.txout().clone())

zcash_client_sqlite/src/error.rs

@@ -4,7 +4,6 @@ use std::error;
 use std::fmt;
 
 use zcash_client_backend::{
-    address::RecipientAddress,
     data_api,
     encoding::{Bech32DecodeError, TransparentCodecError},
 };
@@ -12,6 +11,9 @@ use zcash_primitives::{consensus::BlockHeight, zip32::AccountId};
 
 use crate::{NoteId, PRUNING_HEIGHT};
 
+#[cfg(feature = "transparent-inputs")]
+use zcash_primitives::legacy::TransparentAddress;
+
 #[cfg(feature = "unstable")]
 use zcash_primitives::transaction::TxId;
 
@@ -44,6 +46,7 @@ pub enum SqliteClientError {
     /// Base58 decoding error
     Base58(bs58::decode::Error),
 
+    ///
     #[cfg(feature = "transparent-inputs")]
     HdwalletError(hdwallet::error::Error),
 
@@ -84,7 +87,8 @@ pub enum SqliteClientError {
 
     /// The address associated with a record being inserted was not recognized as
     /// belonging to the wallet
-    AddressNotRecognized(RecipientAddress),
+    #[cfg(feature = "transparent-inputs")]
+    AddressNotRecognized(TransparentAddress),
 }
 
 impl error::Error for SqliteClientError {
@@ -127,6 +131,7 @@ impl fmt::Display for SqliteClientError {
             SqliteClientError::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {:?}", acct_id),
             SqliteClientError::AccountIdDiscontinuity => write!(f, "Wallet account identifiers must be sequential."),
             SqliteClientError::AccountIdOutOfRange => write!(f, "Wallet account identifiers must be less than 0x7FFFFFFF."),
+            #[cfg(feature = "transparent-inputs")]
             SqliteClientError::AddressNotRecognized(_) => write!(f, "The address associated with a received txo is not identifiable as belonging to the wallet."),
         }
     }

zcash_client_sqlite/src/lib.rs

@@ -34,7 +34,7 @@
 
 use rusqlite::Connection;
 use secrecy::{ExposeSecret, SecretVec};
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::fmt;
 use std::path::Path;
 
@@ -50,7 +50,7 @@ use zcash_primitives::{
 };
 
 use zcash_client_backend::{
-    address::UnifiedAddress,
+    address::{AddressMetadata, UnifiedAddress},
     data_api::{
         BlockSource, DecryptedTransaction, PoolType, PrunedBlock, Recipient, SentTransaction,
         WalletRead, WalletWrite,
@@ -248,7 +248,7 @@ impl<P: consensus::Parameters> WalletRead for WalletDb<P> {
     fn get_transparent_receivers(
         &self,
         _account: AccountId,
-    ) -> Result<HashSet<TransparentAddress>, Self::Error> {
+    ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error> {
         #[cfg(feature = "transparent-inputs")]
         return wallet::get_transparent_receivers(&self.params, &self.conn, _account);
 
@@ -379,7 +379,7 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> {
     fn get_transparent_receivers(
         &self,
         account: AccountId,
-    ) -> Result<HashSet<TransparentAddress>, Self::Error> {
+    ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error> {
         self.wallet_db.get_transparent_receivers(account)
     }
 
@@ -1187,10 +1187,10 @@ mod tests {
         let receivers = db_data.get_transparent_receivers(0.into()).unwrap();
 
         // The receiver for the default UA should be in the set.
-        assert!(receivers.contains(ufvk.default_address().0.transparent().unwrap()));
+        assert!(receivers.contains_key(ufvk.default_address().0.transparent().unwrap()));
 
         // The default t-addr should be in the set.
-        assert!(receivers.contains(&taddr));
+        assert!(receivers.contains_key(&taddr));
     }
 
     #[cfg(feature = "unstable")]

zcash_client_sqlite/src/wallet.rs

@@ -42,8 +42,9 @@ use crate::{
 use {
     crate::UtxoId,
     rusqlite::{params, Connection},
-    std::collections::HashSet,
+    zcash_client_backend::{
-    zcash_client_backend::{encoding::AddressCodec, wallet::WalletTransparentOutput},
+        address::AddressMetadata, encoding::AddressCodec, wallet::WalletTransparentOutput,
+    },
     zcash_primitives::{
         legacy::{keys::IncomingViewingKey, Script, TransparentAddress},
         transaction::components::{OutPoint, TxOut},
@@ -236,7 +237,7 @@ pub(crate) fn get_current_address<P: consensus::Parameters>(
     addr.map(|(addr_str, di_vec)| {
         let mut di_be: [u8; 11] = di_vec.try_into().map_err(|_| {
             SqliteClientError::CorruptedData(
-                "Diverisifier index is not an 11-byte value".to_owned(),
+                "Diversifier index is not an 11-byte value".to_owned(),
             )
         })?;
         di_be.reverse();
@@ -262,15 +263,24 @@ pub(crate) fn get_transparent_receivers<P: consensus::Parameters>(
     params: &P,
     conn: &Connection,
     account: AccountId,
-) -> Result<HashSet<TransparentAddress>, SqliteClientError> {
+) -> Result<HashMap<TransparentAddress, AddressMetadata>, SqliteClientError> {
-    let mut ret = HashSet::new();
+    let mut ret = HashMap::new();
 
     // Get all UAs derived
-    let mut ua_query = conn.prepare("SELECT address FROM addresses WHERE account = :account")?;
+    let mut ua_query =
+        conn.prepare("SELECT address, diversifier_index_be FROM addresses WHERE account = :account")?;
     let mut rows = ua_query.query(named_params![":account": &u32::from(account)])?;
 
     while let Some(row) = rows.next()? {
         let ua_str: String = row.get(0)?;
+        let di_vec: Vec<u8> = row.get(1)?;
+        let mut di_be: [u8; 11] = di_vec.try_into().map_err(|_| {
+            SqliteClientError::CorruptedData(
+                "Diverisifier index is not an 11-byte value".to_owned(),
+            )
+        })?;
+        di_be.reverse();
+
         let ua = RecipientAddress::decode(params, &ua_str)
             .ok_or_else(|| {
                 SqliteClientError::CorruptedData("Not a valid Zcash recipient address".to_owned())
@@ -282,13 +292,18 @@ pub(crate) fn get_transparent_receivers<P: consensus::Parameters>(
                     ua_str,
                 ))),
             })?;
+
         if let Some(taddr) = ua.transparent() {
-            ret.insert(*taddr);
+            ret.insert(
+                *taddr,
+                AddressMetadata::new(account, DiversifierIndex(di_be)),
+            );
         }
     }
 
-    if let Some(taddr) = get_legacy_transparent_address(params, conn, account)? {
+    if let Some((taddr, diversifier_index)) = get_legacy_transparent_address(params, conn, account)?
-        ret.insert(taddr);
+    {
+        ret.insert(taddr, AddressMetadata::new(account, diversifier_index));
     }
 
     Ok(ret)
@@ -299,7 +314,7 @@ pub(crate) fn get_legacy_transparent_address<P: consensus::Parameters>(
     params: &P,
     conn: &Connection,
     account: AccountId,
-) -> Result<Option<TransparentAddress>, SqliteClientError> {
+) -> Result<Option<(TransparentAddress, DiversifierIndex)>, SqliteClientError> {
     // Get the UFVK for the account.
     let ufvk_str: Option<String> = conn
         .query_row(
@@ -317,7 +332,10 @@ pub(crate) fn get_legacy_transparent_address<P: consensus::Parameters>(
         ufvk.transparent()
             .map(|tfvk| {
                 tfvk.derive_external_ivk()
-                    .map(|tivk| tivk.default_address().0)
+                    .map(|tivk| {
+                        let (taddr, child_index) = tivk.default_address();
+                        (taddr, DiversifierIndex::from(child_index))
+                    })
                     .map_err(SqliteClientError::HdwalletError)
             })
             .transpose()
@@ -1094,8 +1112,11 @@ pub(crate) fn put_received_transparent_utxo<'a, P: consensus::Parameters>(
             // so then insert/update it directly.
             let account = AccountId::from(0u32);
             get_legacy_transparent_address(&stmts.wallet_db.params, &stmts.wallet_db.conn, account)
-                .and_then(|taddr| {
+                .and_then(|legacy_taddr| {
-                    if taddr.as_ref() == Some(output.recipient_address()) {
+                    if legacy_taddr
+                        .iter()
+                        .any(|(taddr, _)| taddr == output.recipient_address())
+                    {
                         stmts
                             .stmt_update_legacy_transparent_utxo(output, account)
                             .transpose()
@@ -1104,7 +1125,7 @@ pub(crate) fn put_received_transparent_utxo<'a, P: consensus::Parameters>(
                             })
                     } else {
                         Err(SqliteClientError::AddressNotRecognized(
-                            RecipientAddress::Transparent(*output.recipient_address()),
+                            *output.recipient_address(),
                         ))
                     }
                 })

zcash_client_sqlite/src/wallet/init/migrations/add_utxo_account.rs

@@ -79,7 +79,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                             )),
                         })?;
 
-                for taddr in taddrs {
+                for (taddr, _) in taddrs {
                     stmt_update_utxo_account.execute(named_params![
                         ":account": &account,
                         ":address": &taddr.encode(&self._params),

zcash_primitives/CHANGELOG.md

@@ -7,6 +7,10 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 
+### Added
+- Added in `zcash_primitives::zip32`
+  - An implementation of `TryFrom<DiversifierIndex>` for `u32`
+
 ## [0.8.1] - 2022-10-19
 ### Added
 - `zcash_primitives::legacy`:

zcash_primitives/src/zip32.rs

@@ -105,6 +105,16 @@ impl From<u64> for DiversifierIndex {
     }
 }
 
+impl TryFrom<DiversifierIndex> for u32 {
+    type Error = std::num::TryFromIntError;
+
+    fn try_from(di: DiversifierIndex) -> Result<u32, Self::Error> {
+        let mut u128_bytes = [0u8; 16];
+        u128_bytes[0..11].copy_from_slice(&di.0[..]);
+        u128::from_le_bytes(u128_bytes).try_into()
+    }
+}
+
 impl DiversifierIndex {
     pub fn new() -> Self {
         DiversifierIndex([0; 11])
@@ -144,3 +154,26 @@ pub enum Scope {
 }
 
 memuse::impl_no_dynamic_usage!(Scope);
+
+#[cfg(test)]
+mod tests {
+    use super::DiversifierIndex;
+
+    #[test]
+    fn diversifier_index_to_u32() {
+        let two = DiversifierIndex([
+            0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        ]);
+        assert_eq!(u32::try_from(two), Ok(2));
+
+        let max_u32 = DiversifierIndex([
+            0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        ]);
+        assert_eq!(u32::try_from(max_u32), Ok(u32::MAX));
+
+        let too_big = DiversifierIndex([
+            0xff, 0xff, 0xff, 0xff, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        ]);
+        assert!(matches!(u32::try_from(too_big), Err(_)));
+    }
+}