Merge pull request #1213 from nuttycom/wallet/key_validation

zcash_client_backend: Add `WalletRead::validate_seed`

Cargo.lock

@@ -3085,6 +3085,7 @@ dependencies = [
  "zcash_note_encryption",
  "zcash_primitives",
  "zcash_proofs",
+ "zip32",
 ]
 
 [[package]]

zcash_client_backend/CHANGELOG.md

@@ -50,6 +50,7 @@ and this library adheres to Rust's notion of
     - `type OrchardShardStore`
     - `fn with_orchard_tree_mut`
     - `fn put_orchard_subtree_roots`
+  - Added method `WalletRead::validate_seed`
 - `zcash_client_backend::fees`:
   - Arguments to `ChangeStrategy::compute_balance` have changed.
 

zcash_client_backend/src/data_api.rs

@@ -492,6 +492,21 @@ pub trait WalletRead {
     /// will be interpreted as belonging to that account.
     type AccountId: Copy + Debug + Eq + Hash;
 
+    /// Verifies that the given seed corresponds to the viewing key for the specified account.
+    ///
+    /// Returns:
+    /// - `Ok(true)` if the viewing key for the specified account can be derived from the
+    ///   provided seed.
+    /// - `Ok(false)` if the derived seed does not match, or the specified account is not
+    ///   present in the database.
+    /// - `Err(_)` if a Unified Spending Key cannot be derived from the seed for the
+    ///   specified account.
+    fn validate_seed(
+        &self,
+        account_id: Self::AccountId,
+        seed: &SecretVec<u8>,
+    ) -> Result<bool, Self::Error>;
+
     /// Returns the height of the chain as known to the wallet as of the most recent call to
     /// [`WalletWrite::update_chain_tip`].
     ///
@@ -1317,14 +1332,15 @@ pub mod testing {
         type Error = ();
         type AccountId = u32;
 
-        fn chain_height(&self) -> Result<Option<BlockHeight>, Self::Error> {
+        fn validate_seed(
-            Ok(None)
+            &self,
+            _account_id: Self::AccountId,
+            _seed: &SecretVec<u8>,
+        ) -> Result<bool, Self::Error> {
+            Ok(false)
         }
 
-        fn get_target_and_anchor_heights(
+        fn chain_height(&self) -> Result<Option<BlockHeight>, Self::Error> {
-            &self,
-            _min_confirmations: NonZeroU32,
-        ) -> Result<Option<(BlockHeight, BlockHeight)>, Self::Error> {
             Ok(None)
         }
 
@@ -1347,6 +1363,13 @@ pub mod testing {
             Ok(vec![])
         }
 
+        fn get_target_and_anchor_heights(
+            &self,
+            _min_confirmations: NonZeroU32,
+        ) -> Result<Option<(BlockHeight, BlockHeight)>, Self::Error> {
+            Ok(None)
+        }
+
         fn get_min_unspent_height(&self) -> Result<Option<BlockHeight>, Self::Error> {
             Ok(None)
         }

zcash_client_sqlite/Cargo.toml

@@ -24,6 +24,7 @@ zcash_client_backend = { workspace = true, features = ["unstable-serialization",
 zcash_encoding.workspace = true
 zcash_keys = { workspace = true, features = ["orchard", "sapling"] }
 zcash_primitives.workspace = true
+zip32.workspace = true
 
 # Dependencies exposed in a public API:
 # (Breaking upgrades to these require a breaking upgrade to this crate.)

zcash_client_sqlite/src/lib.rs

@@ -245,6 +245,32 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
     type Error = SqliteClientError;
     type AccountId = AccountId;
 
+    fn validate_seed(
+        &self,
+        account_id: Self::AccountId,
+        seed: &SecretVec<u8>,
+    ) -> Result<bool, Self::Error> {
+        self.get_unified_full_viewing_keys().and_then(|keys| {
+            keys.get(&account_id).map_or(Ok(false), |ufvk| {
+                let usk = UnifiedSpendingKey::from_seed(
+                    &self.params,
+                    &seed.expose_secret()[..],
+                    account_id,
+                )
+                .map_err(|_| SqliteClientError::KeyDerivationError(account_id))?;
+
+                // 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))
+                })
+            })
+        })
+    }
+
     fn chain_height(&self) -> Result<Option<BlockHeight>, Self::Error> {
         wallet::scan_queue_extrema(self.conn.borrow())
             .map(|h| h.map(|range| *range.end()))
@@ -1204,9 +1230,11 @@ extern crate assert_matches;
 
 #[cfg(test)]
 mod tests {
+    use secrecy::SecretVec;
     use zcash_client_backend::data_api::{AccountBirthday, WalletRead, WalletWrite};
 
-    use crate::{testing::TestBuilder, AccountId, DEFAULT_UA_REQUEST};
+    use crate::{testing::TestBuilder, DEFAULT_UA_REQUEST};
+    use zip32::AccountId;
 
     #[cfg(feature = "unstable")]
     use {
@@ -1217,6 +1245,36 @@ mod tests {
         },
     };
 
+    #[test]
+    fn validate_seed() {
+        let st = TestBuilder::new()
+            .with_test_account(AccountBirthday::from_sapling_activation)
+            .build();
+
+        assert!({
+            let account = st.test_account().unwrap().0;
+            st.wallet()
+                .validate_seed(account, st.test_seed().unwrap())
+                .unwrap()
+        });
+
+        // check that passing an invalid account results in a failure
+        assert!({
+            let account = AccountId::try_from(1u32).unwrap();
+            !st.wallet()
+                .validate_seed(account, st.test_seed().unwrap())
+                .unwrap()
+        });
+
+        // check that passing an invalid seed results in a failure
+        assert!({
+            let account = st.test_account().unwrap().0;
+            !st.wallet()
+                .validate_seed(account, &SecretVec::new(vec![1u8; 32]))
+                .unwrap()
+        });
+    }
+
     #[test]
     pub(crate) fn get_next_available_address() {
         let mut st = TestBuilder::new()

zcash_client_sqlite/src/testing.rs

@@ -9,7 +9,7 @@ use nonempty::NonEmpty;
 use prost::Message;
 use rand_core::{OsRng, RngCore};
 use rusqlite::{params, Connection};
-use secrecy::Secret;
+use secrecy::{Secret, SecretVec};
 use tempfile::NamedTempFile;
 
 #[cfg(feature = "unstable")]
@@ -142,7 +142,7 @@ impl<Cache> TestBuilder<Cache> {
         let test_account = if let Some(birthday) = self.test_account_birthday {
             let seed = Secret::new(vec![0u8; 32]);
             let (account, usk) = db_data.create_account(&seed, birthday.clone()).unwrap();
-            Some((account, usk, birthday))
+            Some((seed, account, usk, birthday))
         } else {
             None
         };
@@ -163,7 +163,12 @@ pub(crate) struct TestState<Cache> {
     latest_cached_block: Option<(BlockHeight, BlockHash, u32)>,
     _data_file: NamedTempFile,
     db_data: WalletDb<Connection, Network>,
-    test_account: Option<(AccountId, UnifiedSpendingKey, AccountBirthday)>,
+    test_account: Option<(
+        SecretVec<u8>,
+        AccountId,
+        UnifiedSpendingKey,
+        AccountBirthday,
+    )>,
 }
 
 impl<Cache: TestCache> TestState<Cache>
@@ -420,16 +425,23 @@ impl<Cache> TestState<Cache> {
             .expect("Sapling activation height must be known.")
     }
 
+    /// Exposes the test seed, if enabled via [`TestBuilder::with_test_account`].
+    pub(crate) fn test_seed(&self) -> Option<&SecretVec<u8>> {
+        self.test_account.as_ref().map(|(seed, _, _, _)| seed)
+    }
+
     /// Exposes the test account, if enabled via [`TestBuilder::with_test_account`].
     pub(crate) fn test_account(&self) -> Option<(AccountId, UnifiedSpendingKey, AccountBirthday)> {
-        self.test_account.as_ref().cloned()
+        self.test_account
+            .as_ref()
+            .map(|(_, a, k, b)| (*a, k.clone(), b.clone()))
     }
 
     /// Exposes the test account's Sapling DFVK, if enabled via [`TestBuilder::with_test_account`].
     pub(crate) fn test_account_sapling(&self) -> Option<DiversifiableFullViewingKey> {
         self.test_account
             .as_ref()
-            .and_then(|(_, usk, _)| usk.to_unified_full_viewing_key().sapling().cloned())
+            .and_then(|(_, _, usk, _)| usk.to_unified_full_viewing_key().sapling().cloned())
     }
 
     /// Invokes [`create_spend_to_address`] with the given arguments.

zcash_keys/CHANGELOG.md

@@ -6,6 +6,9 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 
+### Added
+- `zcash_keys::keys::UnifiedAddressRequest::all`
+
 ## [0.1.0] - 2024-03-01
 The entries below are relative to the `zcash_client_backend` crate as of
 `zcash_client_backend 0.10.0`.

zcash_keys/src/keys.rs

@@ -450,6 +450,9 @@ pub struct UnifiedAddressRequest {
 }
 
 impl UnifiedAddressRequest {
+    /// Construct a new unified address request from its constituent parts.
+    ///
+    /// Returns `None` if the resulting unified address would not include at least one shielded receiver.
     pub fn new(has_orchard: bool, has_sapling: bool, has_p2pkh: bool) -> Option<Self> {
         let has_shielded_receiver = has_orchard || has_sapling;
 
@@ -464,6 +467,24 @@ impl UnifiedAddressRequest {
         }
     }
 
+    /// Constructs a new unified address request that includes a request for a receiver of each
+    /// type that is supported given the active feature flags.
+    pub fn all() -> Option<Self> {
+        let _has_orchard = false;
+        #[cfg(feature = "orchard")]
+        let _has_orchard = true;
+
+        let _has_sapling = false;
+        #[cfg(feature = "sapling")]
+        let _has_sapling = true;
+
+        let _has_p2pkh = false;
+        #[cfg(feature = "transparent-inputs")]
+        let _has_p2pkh = true;
+
+        Self::new(_has_orchard, _has_sapling, _has_p2pkh)
+    }
+
     /// Construct a new unified address request from its constituent parts.
     ///
     /// Panics: at least one of `has_orchard` or `has_sapling` must be `true`.