Give zcash_client_sqlite its own account ID

zcash_client_backend/CHANGELOG.md

@@ -51,6 +51,7 @@ and this library adheres to Rust's notion of
     - `fn with_orchard_tree_mut`
     - `fn put_orchard_subtree_roots`
   - Added method `WalletRead::validate_seed`
+  - Removed `Error::AccountNotFound` variant.
 - `zcash_client_backend::fees`:
   - Arguments to `ChangeStrategy::compute_balance` have changed.
 - `zcash_client_backend::zip321::render::amount_str` now takes a

zcash_client_backend/src/data_api.rs

@@ -500,7 +500,7 @@ pub trait WalletRead {
     /// - `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.
+    ///   specified account or the account has no known ZIP-32 derivation.
     fn validate_seed(
         &self,
         account_id: Self::AccountId,
@@ -1103,6 +1103,8 @@ pub trait WalletWrite: WalletRead {
     /// funds have been received by the currently-available account (in order to enable automated
     /// account recovery).
     ///
+    /// Panics if the length of the seed is not between 32 and 252 bytes inclusive.
+    ///
     /// [ZIP 316]: https://zips.z.cash/zip-0316
     fn create_account(
         &mut self,

zcash_client_backend/src/data_api/error.rs

@@ -5,12 +5,9 @@ use std::fmt::{self, Debug, Display};
 
 use shardtree::error::ShardTreeError;
 use zcash_primitives::transaction::components::amount::NonNegativeAmount;
-use zcash_primitives::{
-    transaction::{
-        builder,
-        components::{amount::BalanceError, transparent},
-    },
-    zip32::AccountId,
+use zcash_primitives::transaction::{
+    builder,
+    components::{amount::BalanceError, transparent},
 };
 
 use crate::address::UnifiedAddress;
@@ -45,9 +42,6 @@ pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError> {
     /// No account could be found corresponding to a provided spending key.
     KeyNotRecognized,
 
-    /// No account with the given identifier was found in the wallet.
-    AccountNotFound(AccountId),
-
     /// Zcash amount computation encountered an overflow or underflow.
     BalanceError(BalanceError),
 
@@ -128,9 +122,6 @@ where
                     "Wallet does not contain an account corresponding to the provided spending key"
                 )
             }
-            Error::AccountNotFound(account) => {
-                write!(f, "Wallet does not contain account {}", u32::from(*account))
-            }
             Error::BalanceError(e) => write!(
                 f,
                 "The value lies outside the valid range of Zcash amounts: {:?}.",

zcash_client_backend/src/data_api/wallet.rs

@@ -36,21 +36,6 @@ use sapling::{
 };
 use std::num::NonZeroU32;
 
-use zcash_primitives::{
-    consensus::{self, BlockHeight, NetworkUpgrade},
-    memo::MemoBytes,
-    transaction::{
-        builder::{BuildConfig, BuildResult, Builder},
-        components::{
-            amount::{Amount, NonNegativeAmount},
-            sapling::zip212_enforcement,
-        },
-        fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule},
-        Transaction, TxId,
-    },
-    zip32::Scope,
-};
-
 use super::InputSource;
 use crate::{
     address::Address,
@@ -66,6 +51,20 @@ use crate::{
     zip321::{self, Payment},
     PoolType, ShieldedProtocol,
 };
+use zcash_primitives::{
+    consensus::{self, BlockHeight, NetworkUpgrade},
+    memo::MemoBytes,
+    transaction::{
+        builder::{BuildConfig, BuildResult, Builder},
+        components::{
+            amount::{Amount, NonNegativeAmount},
+            sapling::zip212_enforcement,
+        },
+        fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule},
+        Transaction, TxId,
+    },
+    zip32::Scope,
+};
 
 #[cfg(feature = "transparent-inputs")]
 use {

zcash_client_sqlite/CHANGELOG.md

@@ -10,11 +10,24 @@ and this library adheres to Rust's notion of
 ### Added
 - A new `orchard` feature flag has been added to make it possible to
   build client code without `orchard` dependendencies.
+- `zcash_client_sqlite::AccountId` 
 - `impl From<zcash_keys::keys::AddressGenerationError> for SqliteClientError`
 
 ### Changed
+- Many places that `AccountId` appeared in the API changed from
+  using `zcash_primitives::zip32::AccountId` to using an opaque `zcash_client_sqlite::AccountId`
+  type. This impacts the following APIs:
+  - `zcash_client_sqlite::error::SqliteClientError::AccountUnknown`
+  - Changes to the implementation of the `WalletWrite` trait:
+    - `create_account` function returns a unique identifier for the new account (as before),
+      except that this ID no longer happens to match the ZIP-32 account index.
+      To get the ZIP-32 account index, use the new `WalletRead::get_account` function.
+  - Two columns in the `transactions` view were renamed. They refer to the primary key field in the `accounts` table, which no longer equates to a ZIP-32 account index.
+    - `to_account` -> `to_account_id`
+    - `from_account` -> `from_account_id`
 - `zcash_client_sqlite::error::SqliteClientError` has changed variants:
   - Added `AddressGeneration`
+  - Added `UnknownZip32Derivation`
   - Removed `DiversifierIndexOutOfRange`
 - `zcash_client_sqlite::wallet::init::WalletMigrationError` has added variant
   `AddressGeneration`

zcash_client_sqlite/Cargo.toml

@@ -43,6 +43,7 @@ jubjub.workspace = true
 
 # - Secret management
 secrecy.workspace = true
+subtle.workspace = true
 
 # - Shielded protocols
 orchard = { workspace = true, optional = true }

zcash_client_sqlite/src/chain.rs

@@ -346,7 +346,6 @@ mod tests {
     use crate::{
         testing::{AddressType, TestBuilder},
         wallet::truncate_to_height,
-        AccountId,
     };
 
     #[test]
@@ -439,6 +438,7 @@ mod tests {
             .with_block_cache()
             .with_test_account(AccountBirthday::from_sapling_activation)
             .build();
+        let account = st.test_account().unwrap();
 
         let dfvk = st.test_account_sapling().unwrap();
 
@@ -455,38 +455,29 @@ mod tests {
         st.scan_cached_blocks(h, 2);
 
         // Account balance should reflect both received notes
-        assert_eq!(
-            st.get_total_balance(AccountId::ZERO),
-            (value + value2).unwrap()
-        );
+        assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap());
 
         // "Rewind" to height of last scanned block
         st.wallet_mut()
             .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h + 1))
             .unwrap();
 
         // Account balance should be unaltered
-        assert_eq!(
-            st.get_total_balance(AccountId::ZERO),
-            (value + value2).unwrap()
-        );
+        assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap());
 
         // Rewind so that one block is dropped
         st.wallet_mut()
             .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h))
             .unwrap();
 
         // Account balance should only contain the first received note
-        assert_eq!(st.get_total_balance(AccountId::ZERO), value);
+        assert_eq!(st.get_total_balance(account.0), value);
 
         // Scan the cache again
         st.scan_cached_blocks(h, 2);
 
         // Account balance should again reflect both received notes
-        assert_eq!(
-            st.get_total_balance(AccountId::ZERO),
-            (value + value2).unwrap()
-        );
+        assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap());
     }
 
     #[test]
@@ -495,6 +486,7 @@ mod tests {
             .with_block_cache()
             .with_test_account(AccountBirthday::from_sapling_activation)
             .build();
+        let account = st.test_account().unwrap();
 
         let (_, usk, _) = st.test_account().unwrap();
         let dfvk = st.test_account_sapling().unwrap();
@@ -503,7 +495,7 @@ mod tests {
         let value = NonNegativeAmount::const_from_u64(50000);
         let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value);
         st.scan_cached_blocks(h1, 1);
-        assert_eq!(st.get_total_balance(AccountId::ZERO), value);
+        assert_eq!(st.get_total_balance(account.0), value);
 
         // Create blocks to reach SAPLING_ACTIVATION_HEIGHT + 2
         let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value);
@@ -515,7 +507,7 @@ mod tests {
         // Now scan the block of height SAPLING_ACTIVATION_HEIGHT + 1
         st.scan_cached_blocks(h2, 1);
         assert_eq!(
-            st.get_total_balance(AccountId::ZERO),
+            st.get_total_balance(account.0),
             NonNegativeAmount::const_from_u64(150_000)
         );
 
@@ -551,6 +543,7 @@ mod tests {
             .with_block_cache()
             .with_test_account(AccountBirthday::from_sapling_activation)
             .build();
+        let account = st.test_account().unwrap();
 
         let dfvk = st.test_account_sapling().unwrap();
 
@@ -568,7 +561,7 @@ mod tests {
         assert_eq!(summary.received_sapling_note_count(), 1);
 
         // Account balance should reflect the received note
-        assert_eq!(st.get_total_balance(AccountId::ZERO), value);
+        assert_eq!(st.get_total_balance(account.0), value);
 
         // Create a second fake CompactBlock sending more value to the address
         let value2 = NonNegativeAmount::const_from_u64(7);
@@ -581,10 +574,7 @@ mod tests {
         assert_eq!(summary.received_sapling_note_count(), 1);
 
         // Account balance should reflect both received notes
-        assert_eq!(
-            st.get_total_balance(AccountId::ZERO),
-            (value + value2).unwrap()
-        );
+        assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap());
     }
 
     #[test]
@@ -593,6 +583,7 @@ mod tests {
             .with_block_cache()
             .with_test_account(AccountBirthday::from_sapling_activation)
             .build();
+        let account = st.test_account().unwrap();
         let dfvk = st.test_account_sapling().unwrap();
 
         // Wallet summary is not yet available
@@ -607,7 +598,7 @@ mod tests {
         st.scan_cached_blocks(received_height, 1);
 
         // Account balance should reflect the received note
-        assert_eq!(st.get_total_balance(AccountId::ZERO), value);
+        assert_eq!(st.get_total_balance(account.0), value);
 
         // Create a second fake CompactBlock spending value from the address
         let extsk2 = ExtendedSpendingKey::master(&[0]);
@@ -619,10 +610,7 @@ mod tests {
         st.scan_cached_blocks(spent_height, 1);
 
         // Account balance should equal the change
-        assert_eq!(
-            st.get_total_balance(AccountId::ZERO),
-            (value - value2).unwrap()
-        );
+        assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap());
     }
 
     #[test]
@@ -631,6 +619,7 @@ mod tests {
             .with_block_cache()
             .with_test_account(AccountBirthday::from_sapling_activation)
             .build();
+        let account = st.test_account().unwrap();
 
         let dfvk = st.test_account_sapling().unwrap();
 
@@ -652,18 +641,12 @@ mod tests {
         st.scan_cached_blocks(spent_height, 1);
 
         // Account balance should equal the change
-        assert_eq!(
-            st.get_total_balance(AccountId::ZERO),
-            (value - value2).unwrap()
-        );
+        assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap());
 
         // Now scan the block in which we received the note that was spent.
         st.scan_cached_blocks(received_height, 1);
 
         // Account balance should be the same.
-        assert_eq!(
-            st.get_total_balance(AccountId::ZERO),
-            (value - value2).unwrap()
-        );
+        assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap());
     }
 }

zcash_client_sqlite/src/error.rs

@@ -9,9 +9,8 @@
     PoolType,
 };
 use zcash_keys::keys::AddressGenerationError;
-use zcash_primitives::{
-    consensus::BlockHeight, transaction::components::amount::BalanceError, zip32::AccountId,
-};
+use zcash_primitives::zip32;
+use zcash_primitives::{consensus::BlockHeight, transaction::components::amount::BalanceError};
 
 use crate::wallet::commitment_tree;
 use crate::PRUNING_DEPTH;
@@ -73,11 +72,13 @@
     AddressGeneration(AddressGenerationError),
 
     /// The account for which information was requested does not belong to the wallet.
-    AccountUnknown(AccountId),
+    AccountUnknown,
+
+    /// The account was imported, and ZIP-32 derivation information is not known for it.
+    UnknownZip32Derivation,
 
-    /// An error occurred deriving a spending key from a seed and an account
-    /// identifier.
-    KeyDerivationError(AccountId),
+    /// An error occurred deriving a spending key from a seed and a ZIP-32 account index.
+    KeyDerivationError(zip32::AccountId),
 
     /// A caller attempted to initialize the accounts table with a discontinuous
     /// set of account identifiers.
@@ -149,8 +150,8 @@
             SqliteClientError::BlockConflict(h) => write!(f, "A block hash conflict occurred at height {}; rewind required.", u32::from(*h)),
             SqliteClientError::NonSequentialBlocks => write!(f, "`put_blocks` requires that the provided block range be sequential"),
             SqliteClientError::AddressGeneration(e) => write!(f, "{}", e),
-            SqliteClientError::AccountUnknown(acct_id) => write!(f, "Account {} does not belong to this wallet.", u32::from(*acct_id)),
-
+            SqliteClientError::AccountUnknown => write!(f, "The account with the given ID does not belong to this wallet."),
+            SqliteClientError::UnknownZip32Derivation => write!(f, "ZIP-32 derivation information is not known for this account."),
             SqliteClientError::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {}", u32::from(*acct_id)),
             SqliteClientError::AccountIdDiscontinuity => write!(f, "Wallet account identifiers must be sequential."),
             SqliteClientError::AccountIdOutOfRange => write!(f, "Wallet account identifiers must be less than 0x7FFFFFFF."),