Merge pull request #1636 from zcash/wallet/imported_spending_key_meta…

…data

zcash_client_backend: Add optional derivation metadata when importing UFVKs with spending purpose.

zcash_client_backend/CHANGELOG.md

@@ -7,6 +7,9 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 
+### Added
+- `zcash_client_backend::data_api::AccountSource::key_derivation`
+
 ### Changed
 - `zcash_client_backend::data_api::WalletRead`:
   - The `create_account`, `import_account_hd`, and `import_account_ufvk`

zcash_client_backend/src/data_api.rs

@@ -331,13 +331,40 @@ impl AccountBalance {
     }
 }
 
+/// Source metadata for a ZIP 32-derived key.
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+pub struct Zip32Derivation {
+    seed_fingerprint: SeedFingerprint,
+    account_index: zip32::AccountId,
+}
+
+impl Zip32Derivation {
+    /// Constructs new derivation metadata from its constituent parts.
+    pub fn new(seed_fingerprint: SeedFingerprint, account_index: zip32::AccountId) -> Self {
+        Self {
+            seed_fingerprint,
+            account_index,
+        }
+    }
+
+    /// Returns the seed fingerprint.
+    pub fn seed_fingerprint(&self) -> &SeedFingerprint {
+        &self.seed_fingerprint
+    }
+
+    /// Returns the account-level index in the ZIP 32 derivation path.
+    pub fn account_index(&self) -> zip32::AccountId {
+        self.account_index
+    }
+}
+
 /// An enumeration used to control what information is tracked by the wallet for
 /// notes received by a given account.
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub enum AccountPurpose {
     /// For spending accounts, the wallet will track information needed to spend
     /// received notes.
-    Spending,
+    Spending { derivation: Option<Zip32Derivation> },
     /// For view-only accounts, the wallet will not track spend information.
     ViewOnly,
 }
@@ -347,8 +374,7 @@ pub enum AccountPurpose {
 pub enum AccountSource {
     /// An account derived from a known seed.
     Derived {
-        seed_fingerprint: SeedFingerprint,
-        account_index: zip32::AccountId,
+        derivation: Zip32Derivation,
         key_source: Option<String>,
     },
 
@@ -359,6 +385,28 @@ pub enum AccountSource {
     },
 }
 
+impl AccountSource {
+    /// Returns the key derivation metadata for the account source, if any is available.
+    pub fn key_derivation(&self) -> Option<&Zip32Derivation> {
+        match self {
+            AccountSource::Derived { derivation, .. } => Some(derivation),
+            AccountSource::Imported {
+                purpose: AccountPurpose::Spending { derivation },
+                ..
+            } => derivation.as_ref(),
+            _ => None,
+        }
+    }
+
+    /// Returns the application-level key source identifier.
+    pub fn key_source(&self) -> Option<&str> {
+        match self {
+            AccountSource::Derived { key_source, .. } => key_source.as_ref().map(|s| s.as_str()),
+            AccountSource::Imported { key_source, .. } => key_source.as_ref().map(|s| s.as_str()),
+        }
+    }
+}
+
 /// A set of capabilities that a client account must provide.
 pub trait Account {
     type AccountId: Copy;
@@ -376,8 +424,10 @@ pub trait Account {
     /// Returns whether the account is a spending account or a view-only account.
     fn purpose(&self) -> AccountPurpose {
         match self.source() {
-            AccountSource::Derived { .. } => AccountPurpose::Spending,
-            AccountSource::Imported { purpose, .. } => *purpose,
+            AccountSource::Derived { derivation, .. } => AccountPurpose::Spending {
+                derivation: Some(derivation.clone()),
+            },
+            AccountSource::Imported { purpose, .. } => purpose.clone(),
         }
     }
 

zcash_client_sqlite/src/lib.rs

@@ -54,7 +54,7 @@ use zcash_client_backend::{
         Account, AccountBirthday, AccountMeta, AccountPurpose, AccountSource, BlockMetadata,
         DecryptedTransaction, InputSource, NoteFilter, NullifierQuery, ScannedBlock, SeedRelevance,
         SentTransaction, SpendableNotes, TransactionDataRequest, WalletCommitmentTrees, WalletRead,
-        WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
+        WalletSummary, WalletWrite, Zip32Derivation, SAPLING_SHARD_HEIGHT,
     },
     keys::{
         AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey,
@@ -442,17 +442,12 @@ 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 AccountSource::Derived {
-                seed_fingerprint,
-                account_index,
-                ..
-            } = account.source()
-            {
+            if let AccountSource::Derived { derivation, .. } = account.source() {
                 wallet::seed_matches_derived_account(
                     &self.params,
                     seed,
-                    seed_fingerprint,
-                    *account_index,
+                    derivation.seed_fingerprint(),
+                    derivation.account_index(),
                     &account.uivk(),
                 )
             } else {
@@ -480,19 +475,14 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
             // way we could determine that is by brute-forcing the ZIP 32 account
             // index space, which we're not going to do. The method name indicates to
             // the caller that we only check derived accounts.
-            if let AccountSource::Derived {
-                seed_fingerprint,
-                account_index,
-                ..
-            } = account.source()
-            {
+            if let AccountSource::Derived { derivation, .. } = account.source() {
                 has_derived = true;
 
                 if wallet::seed_matches_derived_account(
                     &self.params,
                     seed,
-                    seed_fingerprint,
-                    *account_index,
+                    derivation.seed_fingerprint(),
+                    derivation.account_index(),
                     &account.uivk(),
                 )? {
                     // The seed is relevant to this account.
@@ -873,8 +863,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                 &wdb.params,
                 account_name,
                 &AccountSource::Derived {
-                    seed_fingerprint,
-                    account_index: zip32_account_index,
+                    derivation: Zip32Derivation::new(seed_fingerprint, zip32_account_index),
                     key_source: key_source.map(|s| s.to_owned()),
                 },
                 wallet::ViewingKey::Full(Box::new(ufvk)),
@@ -911,8 +900,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                 &wdb.params,
                 account_name,
                 &AccountSource::Derived {
-                    seed_fingerprint,
-                    account_index,
+                    derivation: Zip32Derivation::new(seed_fingerprint, account_index),
                     key_source: key_source.map(|s| s.to_owned()),
                 },
                 wallet::ViewingKey::Full(Box::new(ufvk)),
@@ -2023,7 +2011,7 @@ mod tests {
             .build();
         assert_matches!(
             st.test_account().unwrap().account().source(),
-            AccountSource::Derived { account_index, .. } if *account_index == zip32::AccountId::ZERO);
+            AccountSource::Derived { derivation, .. } if derivation.account_index() == zip32::AccountId::ZERO);
     }
 
     #[test]
@@ -2046,7 +2034,7 @@ mod tests {
             .unwrap();
         assert_matches!(
             first.0.source(),
-            AccountSource::Derived { account_index, .. } if *account_index == zip32_index_1);
+            AccountSource::Derived { derivation, .. } if derivation.account_index() == zip32_index_1);
 
         let zip32_index_2 = zip32_index_1.next().unwrap();
         let second = st
@@ -2055,7 +2043,7 @@ mod tests {
             .unwrap();
         assert_matches!(
             second.0.source(),
-            AccountSource::Derived { account_index, .. } if *account_index == zip32_index_2);
+            AccountSource::Derived { derivation, .. } if derivation.account_index() == zip32_index_2);
     }
 
     fn check_collisions<C, DbT: WalletTest + WalletWrite, P: consensus::Parameters>(
@@ -2068,7 +2056,7 @@ mod tests {
     {
         assert_matches!(
             st.wallet_mut()
-                .import_account_ufvk("", ufvk, birthday, AccountPurpose::Spending, None),
+                .import_account_ufvk("", ufvk, birthday, AccountPurpose::Spending { derivation: None }, None),
             Err(e) if is_account_collision(&e)
         );
 
@@ -2089,7 +2077,7 @@ mod tests {
                     "",
                     &subset_ufvk,
                     birthday,
-                    AccountPurpose::Spending,
+                    AccountPurpose::Spending { derivation: None },
                     None,
                 ),
                 Err(e) if is_account_collision(&e)
@@ -2113,7 +2101,7 @@ mod tests {
                     "",
                     &subset_ufvk,
                     birthday,
-                    AccountPurpose::Spending,
+                    AccountPurpose::Spending { derivation: None },
                     None,
                 ),
                 Err(e) if is_account_collision(&e)
@@ -2172,7 +2160,13 @@ mod tests {
 
         let account = st
             .wallet_mut()
-            .import_account_ufvk("", &ufvk, &birthday, AccountPurpose::Spending, None)
+            .import_account_ufvk(
+                "",
+                &ufvk,
+                &birthday,
+                AccountPurpose::Spending { derivation: None },
+                None,
+            )
             .unwrap();
         assert_eq!(
             ufvk.encode(st.network()),
@@ -2182,7 +2176,7 @@ mod tests {
         assert_matches!(
             account.source(),
             AccountSource::Imported {
-                purpose: AccountPurpose::Spending,
+                purpose: AccountPurpose::Spending { .. },
                 ..
             }
         );

zcash_client_sqlite/src/wallet.rs

@@ -72,7 +72,9 @@ use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
 use uuid::Uuid;
 use zcash_client_backend::data_api::{
     AccountPurpose, DecryptedTransaction, Progress, TransactionDataRequest, TransactionStatus,
+    Zip32Derivation,
 };
+
 use zip32::fingerprint::SeedFingerprint;
 
 use std::collections::{HashMap, HashSet};
@@ -150,28 +152,36 @@ fn parse_account_source(
     spending_key_available: bool,
     key_source: Option<String>,
 ) -> Result<AccountSource, SqliteClientError> {
-    match (account_kind, hd_seed_fingerprint, hd_account_index) {
-        (0, Some(seed_fp), Some(account_index)) => Ok(AccountSource::Derived {
-            seed_fingerprint: SeedFingerprint::from_bytes(seed_fp),
-            account_index: zip32::AccountId::try_from(account_index).map_err(|_| {
-                SqliteClientError::CorruptedData(
-                    "ZIP-32 account ID from wallet DB is out of range.".to_string(),
-                )
-            })?,
+    let derivation = hd_seed_fingerprint
+        .zip(hd_account_index)
+        .map(|(seed_fp, idx)| {
+            zip32::AccountId::try_from(idx)
+                .map_err(|_| {
+                    SqliteClientError::CorruptedData(
+                        "ZIP-32 account ID from wallet DB is out of range.".to_string(),
+                    )
+                })
+                .map(|idx| Zip32Derivation::new(SeedFingerprint::from_bytes(seed_fp), idx))
+        })
+        .transpose()?;
+
+    match (account_kind, derivation) {
+        (0, Some(derivation)) => Ok(AccountSource::Derived {
+            derivation,
             key_source,
         }),
-        (1, None, None) => Ok(AccountSource::Imported {
+        (1, derivation) => Ok(AccountSource::Imported {
             purpose: if spending_key_available {
-                AccountPurpose::Spending
+                AccountPurpose::Spending { derivation }
             } else {
                 AccountPurpose::ViewOnly
             },
             key_source,
         }),
-        (0, None, None) | (1, Some(_), Some(_)) => Err(SqliteClientError::CorruptedData(
+        (0, None) => Err(SqliteClientError::CorruptedData(
             "Wallet DB account_kind constraint violated".to_string(),
         )),
-        (_, _, _) => Err(SqliteClientError::CorruptedData(
+        (_, _) => Err(SqliteClientError::CorruptedData(
             "Unrecognized account_kind".to_string(),
         )),
     }
@@ -378,21 +388,19 @@ pub(crate) fn add_account<P: consensus::Parameters>(
 
     let account_uuid = AccountUuid(Uuid::new_v4());
 
-    let (hd_seed_fingerprint, hd_account_index, spending_key_available, key_source) = match kind {
+    let (derivation, spending_key_available, key_source) = match kind {
         AccountSource::Derived {
-            seed_fingerprint,
-            account_index,
+            derivation,
             key_source,
-        } => (
-            Some(seed_fingerprint),
-            Some(account_index),
-            true,
+        } => (Some(derivation), true, key_source),
+        AccountSource::Imported {
+            purpose: AccountPurpose::Spending { derivation },
             key_source,
-        ),
+        } => (derivation.as_ref(), true, key_source),
         AccountSource::Imported {
-            purpose,
+            purpose: AccountPurpose::ViewOnly,
             key_source,
-        } => (None, None, *purpose == AccountPurpose::Spending, key_source),
+        } => (None, false, key_source),
     };
 
     #[cfg(feature = "orchard")]
@@ -449,8 +457,8 @@ pub(crate) fn add_account<P: consensus::Parameters>(
                 ":account_name": account_name,
                 ":uuid": account_uuid.0,
                 ":account_kind": account_kind_code(kind),
-                ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.to_bytes()),
-                ":hd_account_index": hd_account_index.map(|i| u32::from(*i)),
+                ":hd_seed_fingerprint": derivation.map(|d| d.seed_fingerprint().to_bytes()),
+                ":hd_account_index": derivation.map(|d| u32::from(d.account_index())),
                 ":key_source": key_source,
                 ":ufvk": ufvk_encoded,
                 ":uivk": viewing_key.uivk().encode(params),
@@ -832,8 +840,7 @@ pub(crate) fn get_derived_account<P: consensus::Parameters>(
                 name: account_name,
                 uuid: account_uuid,
                 kind: AccountSource::Derived {
-                    seed_fingerprint: *seed_fp,
-                    account_index,
+                    derivation: Zip32Derivation::new(*seed_fp, account_index),
                     key_source,
                 },
                 viewing_key: ViewingKey::Full(Box::new(ufvk)),
@@ -3761,7 +3768,7 @@ mod tests {
         let expected_account_index = zip32::AccountId::try_from(0).unwrap();
         assert_matches!(
             account_parameters.kind,
-            AccountSource::Derived{account_index, ..} if account_index == expected_account_index
+            AccountSource::Derived{derivation, ..} if derivation.account_index() == expected_account_index
         );
     }