Use private fields in HdSeedAccount

And a few other PR comment fixes.

zcash_client_backend/src/data_api/wallet.rs

@@ -90,11 +90,32 @@ use input_selection::{
 
 /// Describes the key inputs and UFVK for an account that was derived from a ZIP-32 HD seed and account index.
 #[derive(Debug, Clone)]
-pub struct HdSeedAccount(
-    pub HdSeedFingerprint,
-    pub zip32::AccountId,
-    pub UnifiedFullViewingKey,
-);
+pub struct HdSeedAccount(HdSeedFingerprint, zip32::AccountId, UnifiedFullViewingKey);
+
+impl HdSeedAccount {
+    pub fn new(
+        hd_seed_fingerprint: HdSeedFingerprint,
+        account_index: zip32::AccountId,
+        ufvk: UnifiedFullViewingKey,
+    ) -> Self {
+        Self(hd_seed_fingerprint, account_index, ufvk)
+    }
+
+    /// Returns the HD seed fingerprint for this account.
+    pub fn hd_seed_fingerprint(&self) -> &HdSeedFingerprint {
+        &self.0
+    }
+
+    /// Returns the ZIP-32 account index for this account.
+    pub fn account_index(&self) -> zip32::AccountId {
+        self.1
+    }
+
+    /// Returns the Unified Full Viewing Key for this account.
+    pub fn ufvk(&self) -> &UnifiedFullViewingKey {
+        &self.2
+    }
+}
 
 /// Represents an arbitrary account for which the seed and ZIP-32 account ID are not known
 /// and may not have been involved in creating this account.

zcash_client_sqlite/src/lib.rs

@@ -267,21 +267,21 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
         seed: &SecretVec<u8>,
     ) -> Result<bool, Self::Error> {
         if let Some(account) = self.get_account(account_id)? {
-            if let Account::Zip32(HdSeedAccount(_, account_index, ufvk)) = account {
+            if let Account::Zip32(hdaccount) = account {
                 let usk = UnifiedSpendingKey::from_seed(
                     &self.params,
                     &seed.expose_secret()[..],
-                    account_index,
+                    hdaccount.account_index(),
                 )
-                .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?;
+                .map_err(|_| SqliteClientError::KeyDerivationError(hdaccount.account_index()))?;
 
                 // Keys are not comparable with `Eq`, but addresses are, so we derive what should
                 // be equivalent addresses for each key and use those to check for key equality.
                 UnifiedAddressRequest::all().map_or(Ok(false), |ua_request| {
                     Ok(usk
                         .to_unified_full_viewing_key()
                         .default_address(ua_request)?
-                        == ufvk.default_address(ua_request)?)
+                        == hdaccount.ufvk().default_address(ua_request)?)
                 })
             } else {
                 Err(SqliteClientError::UnknownZip32Derivation)
@@ -460,7 +460,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                     .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?;
             let ufvk = usk.to_unified_full_viewing_key();
 
-            let account = Account::Zip32(HdSeedAccount(seed_id, account_index, ufvk));
+            let account = Account::Zip32(HdSeedAccount::new(seed_id, account_index, ufvk));
             let account_id = wallet::add_account(wdb.conn.0, &wdb.params, account, birthday)?;
 
             Ok((account_id, usk))

zcash_client_sqlite/src/wallet.rs

@@ -220,11 +220,11 @@ fn get_sql_values_for_account_parameters<'a, P: consensus::Parameters>(
     params: &P,
 ) -> (u32, Option<&'a [u8]>, Option<u32>, String) {
     match account {
-        Account::Zip32(HdSeedAccount(fingerprint, account_index, ufvk)) => (
+        Account::Zip32(hdaccount) => (
             AccountType::Zip32.into(),
-            Some(fingerprint.as_bytes()),
-            Some((*account_index).into()),
-            ufvk.encode(params),
+            Some(hdaccount.hd_seed_fingerprint().as_bytes()),
+            Some(hdaccount.account_index().into()),
+            hdaccount.ufvk().encode(params),
         ),
         Account::Imported(ImportedAccount::Full(ufvk)) => (
             AccountType::ImportedUfvk.into(),
@@ -1064,7 +1064,7 @@ pub(crate) fn get_account<P: Parameters>(
                 .map_err(SqliteClientError::CorruptedData)?;
 
             match account_type {
-                AccountType::Zip32 => Ok(Some(Account::Zip32(HdSeedAccount(
+                AccountType::Zip32 => Ok(Some(Account::Zip32(HdSeedAccount::new(
                     HdSeedFingerprint::from_bytes(row.get(2)?),
                     zip32::AccountId::try_from(row.get::<_, u32>(3)?).map_err(|_| {
                         SqliteClientError::CorruptedData(
@@ -2155,10 +2155,7 @@ mod tests {
     use std::num::NonZeroU32;
 
     use sapling::zip32::ExtendedSpendingKey;
-    use zcash_client_backend::data_api::{
-        wallet::{Account, HdSeedAccount},
-        AccountBirthday, WalletRead,
-    };
+    use zcash_client_backend::data_api::{wallet::Account, AccountBirthday, WalletRead};
     use zcash_primitives::{block::BlockHash, transaction::components::amount::NonNegativeAmount};
 
     use crate::{
@@ -2315,7 +2312,7 @@ mod tests {
         let expected_account_index = zip32::AccountId::try_from(0).unwrap();
         assert_matches!(
             account_parameters,
-            Account::Zip32(HdSeedAccount(_,actual_account_index,_)) if actual_account_index == expected_account_index
+            Account::Zip32(hdaccount) if hdaccount.account_index() == expected_account_index
         );
     }
 

zcash_client_sqlite/src/wallet/init.rs

@@ -225,7 +225,7 @@ mod tests {
                 uvk TEXT NOT NULL,
                 birthday_height INTEGER NOT NULL,
                 recover_until_height INTEGER,
-                CHECK ( (account_type = 0 AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL) OR (account_type = 1 AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL) )
+                CHECK ( (account_type = 0 AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL) OR (account_type != 0 AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL) )
             )"#,
             r#"CREATE TABLE "addresses" (
                 account_id INTEGER NOT NULL,
@@ -331,6 +331,7 @@ mod tests {
                     (to_address IS NOT NULL) != (to_account_id IS NOT NULL)
                 )
             )"#,
+            // Internal table created by SQLite when we started using `AUTOINCREMENT`.
             "CREATE TABLE sqlite_sequence(name,seq)",
             "CREATE TABLE transactions (
                 id_tx INTEGER PRIMARY KEY,
@@ -1085,10 +1086,7 @@ mod tests {
     #[test]
     #[cfg(feature = "transparent-inputs")]
     fn account_produces_expected_ua_sequence() {
-        use zcash_client_backend::data_api::{
-            wallet::{Account, HdSeedAccount},
-            AccountBirthday, WalletRead,
-        };
+        use zcash_client_backend::data_api::{wallet::Account, AccountBirthday, WalletRead};
 
         let network = Network::MainNetwork;
         let data_file = NamedTempFile::new().unwrap();
@@ -1105,11 +1103,7 @@ mod tests {
             .unwrap();
         assert_matches!(
             db_data.get_account(account_id),
-            Ok(Some(Account::Zip32(HdSeedAccount(
-                _,
-                zip32::AccountId::ZERO,
-                _
-            ))))
+            Ok(Some(Account::Zip32(hdaccount))) if hdaccount.account_index() == zip32::AccountId::ZERO
         );
 
         for tv in &test_vectors::UNIFIED[..3] {

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

@@ -14,14 +14,15 @@ use {
     crate::error::SqliteClientError,
     rusqlite::{named_params, OptionalExtension},
     std::collections::HashMap,
-    zcash_client_backend::encoding::AddressCodec,
-    zcash_client_backend::keys::UnifiedFullViewingKey,
+    zcash_client_backend::{
+        encoding::AddressCodec, keys::UnifiedFullViewingKey, wallet::TransparentAddressMetadata,
+    },
     zcash_keys::address::Address,
     zcash_primitives::legacy::{
         keys::{IncomingViewingKey, NonHardenedChildIndex},
         TransparentAddress,
     },
-    zip32::{AccountId, DiversifierIndex},
+    zip32::{AccountId, DiversifierIndex, Scope},
 };
 
 /// This migration adds an account identifier column to the UTXOs table.
@@ -134,8 +135,8 @@ fn get_transparent_receivers<P: consensus::Parameters>(
     conn: &rusqlite::Connection,
     params: &P,
     account: AccountId,
-) -> Result<HashMap<TransparentAddress, NonHardenedChildIndex>, SqliteClientError> {
-    let mut ret = HashMap::new();
+) -> Result<HashMap<TransparentAddress, Option<TransparentAddressMetadata>>, SqliteClientError> {
+    let mut ret: HashMap<TransparentAddress, Option<TransparentAddressMetadata>> = HashMap::new();
 
     // Get all UAs derived
     let mut ua_query = conn
@@ -165,17 +166,37 @@ fn get_transparent_receivers<P: consensus::Parameters>(
             })?;
 
         if let Some(taddr) = ua.transparent() {
-            let di_short = DiversifierIndex::from(di).try_into();
-            if let Ok(di_short) = di_short {
-                if let Some(index) = NonHardenedChildIndex::from_index(di_short) {
-                    ret.insert(*taddr, index);
-                }
-            }
+            let index = NonHardenedChildIndex::from_index(
+                DiversifierIndex::from(di).try_into().map_err(|_| {
+                    SqliteClientError::CorruptedData(
+                        "Unable to get diversifier for transparent address.".to_string(),
+                    )
+                })?,
+            )
+            .ok_or_else(|| {
+                SqliteClientError::CorruptedData(
+                    "Unexpected hardened index for transparent address.".to_string(),
+                )
+            })?;
+
+            ret.insert(
+                *taddr,
+                Some(TransparentAddressMetadata::new(
+                    Scope::External.into(),
+                    index,
+                )),
+            );
         }
     }
 
     if let Some((taddr, child_index)) = get_legacy_transparent_address(params, conn, account)? {
-        ret.insert(taddr, child_index);
+        ret.insert(
+            taddr,
+            Some(TransparentAddressMetadata::new(
+                Scope::External.into(),
+                child_index,
+            )),
+        );
     }
 
     Ok(ret)

zcash_keys/src/keys.rs

@@ -92,7 +92,7 @@ impl HdSeedFingerprint {
         let len = seed.expose_secret().len();
         let len = match len {
             32..=252 => [u8::try_from(len).unwrap()],
-            _ => panic!("ZIP 32 seeds MUST be at most 252 bytes"),
+            _ => panic!("ZIP 32 seeds MUST be at least 32 bytes and at most 252 bytes"),
         };
         const PERSONALIZATION: &[u8] = b"Zcash_HD_Seed_FP";
         let hash = blake2bParams::new()