Lots more fixes

zcash_client_backend/src/data_api/chain.rs

@@ -161,6 +161,8 @@ use crate::{
 pub mod error;
 use error::Error;
 
+use super::WalletRead;
+
 /// A struct containing metadata about a subtree root of the note commitment tree.
 ///
 /// This stores the block height at which the leaf that completed the subtree was
@@ -277,6 +279,7 @@ where
     ParamsT: consensus::Parameters + Send + 'static,
     BlockSourceT: BlockSource,
     DbT: WalletWrite,
+    <DbT as WalletRead>::AccountPK: 'static,
 {
     // Fetch the UnifiedFullViewingKeys we are tracking
     let ufvks = data_db

zcash_client_backend/src/scanning.rs

@@ -255,7 +255,7 @@ pub fn scan_block<P: consensus::Parameters + Send + 'static, K: ScanningKey, Acc
     prior_block_metadata: Option<&BlockMetadata>,
 ) -> Result<ScannedBlock<K::Nf, K::Scope, AccountPK>, ScanError>
 where
-    AccountPK: Copy + Clone + Send + PartialEq + Eq + std::hash::Hash,
+    AccountPK: Copy + Clone + Send + PartialEq + Eq + std::hash::Hash + 'static,
 {
     scan_block_with_runner::<_, _, (), AccountPK>(
         params,
@@ -268,10 +268,14 @@ where
 }
 
 type TaggedBatch<S, AccountPK> =
-
     Batch<(AccountId<AccountPK>, S), SaplingDomain, CompactOutputDescription, CompactDecryptor>;
-type TaggedBatchRunner<S, T, AccountPK> =
-    BatchRunner<(AccountId<AccountPK>, S), SaplingDomain, CompactOutputDescription, CompactDecryptor, T>;
+type TaggedBatchRunner<S, T, AccountPK> = BatchRunner<
+    (AccountId<AccountPK>, S),
+    SaplingDomain,
+    CompactOutputDescription,
+    CompactDecryptor,
+    T,
+>;
 
 #[tracing::instrument(skip_all, fields(height = block.height))]
 pub(crate) fn add_block_to_runner<P, S, T, AccountPK>(
@@ -282,7 +286,7 @@ pub(crate) fn add_block_to_runner<P, S, T, AccountPK>(
     P: consensus::Parameters + Send + 'static,
     S: Clone + Send + 'static,
     T: Tasks<TaggedBatch<S, AccountPK>>,
-    AccountPK: Copy + Clone + Send + PartialEq + Eq + std::hash::Hash,
+    AccountPK: Copy + Clone + Send + PartialEq + Eq + std::hash::Hash + 'static,
 {
     let block_hash = block.hash();
     let block_height = block.height();
@@ -345,7 +349,7 @@ pub(crate) fn scan_block_with_runner<
     mut batch_runner: Option<&mut TaggedBatchRunner<K::Scope, T, AccountPK>>,
 ) -> Result<ScannedBlock<K::Nf, K::Scope, AccountPK>, ScanError>
 where
-    AccountPK: std::cmp::Eq + std::hash::Hash + Clone + Send + Copy,
+    AccountPK: std::cmp::Eq + std::hash::Hash + Clone + Send + Copy + 'static,
 {
     if let Some(scan_error) = check_hash_continuity(&block, prior_block_metadata) {
         return Err(scan_error);

zcash_client_sqlite/src/error.rs

@@ -4,8 +4,9 @@ use std::error;
 use std::fmt;
 
 use shardtree::error::ShardTreeError;
+use zcash_client_backend::data_api::AccountParameters;
+use zcash_client_backend::data_api::HDSeedAccount;
 use zcash_client_backend::{
-    data_api::AccountId,
     encoding::{Bech32DecodeError, TransparentCodecError},
     PoolType,
 };
@@ -71,11 +72,11 @@ pub enum SqliteClientError {
     DiversifierIndexOutOfRange,
 
     /// The account for which information was requested does not belong to the wallet.
-    AccountUnknown(AccountId),
+    AccountUnknown(AccountParameters), // TODO: AccountId<AccountPK> would probably be a bit better here, but would make the error type generic
 
     /// An error occurred deriving a spending key from a seed and an account
     /// identifier.
-    KeyDerivationError(AccountId), // Is there a way to constrain this to AccountId::Zip32?
+    KeyDerivationError(HDSeedAccount),
 
     /// A caller attempted to initialize the accounts table with a discontinuous
     /// set of account identifiers.

zcash_client_sqlite/src/lib.rs

@@ -184,14 +184,17 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> InputSource for
         wallet::sapling::get_spendable_sapling_note(self.conn.borrow(), &self.params, txid, index)
     }
 
-    fn select_spendable_notes(
+    fn select_spendable_notes<AccountPK>(
         &self,
-        account: AccountId,
+        account: AccountId<AccountPK>,
         target_value: Amount,
         _sources: &[ShieldedProtocol],
         anchor_height: BlockHeight,
         exclude: &[Self::NoteRef],
-    ) -> Result<Vec<ReceivedNote<Self::NoteRef, Note>>, Self::Error> {
+    ) -> Result<Vec<ReceivedNote<Self::NoteRef, Note>>, Self::Error>
+    where
+        AccountPK: Copy + Clone + Send + PartialEq + Eq + std::hash::Hash,
+    {
         wallet::sapling::select_spendable_sapling_notes(
             self.conn.borrow(),
             &self.params,
@@ -282,35 +285,38 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
         wallet::wallet_birthday(self.conn.borrow()).map_err(SqliteClientError::from)
     }
 
-    fn get_account_birthday(&self, account: AccountId) -> Result<BlockHeight, Self::Error> {
+    fn get_account_birthday(
+        &self,
+        account: AccountId<Self::AccountPK>,
+    ) -> Result<BlockHeight, Self::Error> {
         wallet::account_birthday(self.conn.borrow(), account).map_err(SqliteClientError::from)
     }
 
     fn get_current_address(
         &self,
-        account: AccountId,
+        account: AccountId<Self::AccountPK>,
     ) -> Result<Option<UnifiedAddress>, Self::Error> {
         wallet::get_current_address(self.conn.borrow(), &self.params, account)
             .map(|res| res.map(|(addr, _)| addr))
     }
 
     fn get_unified_full_viewing_keys(
         &self,
-    ) -> Result<HashMap<AccountId, UnifiedFullViewingKey>, Self::Error> {
+    ) -> Result<HashMap<AccountId<Self::AccountPK>, UnifiedFullViewingKey>, Self::Error> {
         wallet::get_unified_full_viewing_keys(self.conn.borrow(), &self.params)
     }
 
     fn get_account_for_ufvk(
         &self,
         ufvk: &UnifiedFullViewingKey,
-    ) -> Result<Option<AccountId>, Self::Error> {
+    ) -> Result<Option<AccountId<Self::AccountPK>>, Self::Error> {
         wallet::get_account_for_ufvk(self.conn.borrow(), &self.params, ufvk)
     }
 
     fn get_wallet_summary(
         &self,
         min_confirmations: u32,
-    ) -> Result<Option<WalletSummary>, Self::Error> {
+    ) -> Result<Option<WalletSummary<Self::AccountPK>>, Self::Error> {
         // This will return a runtime error if we call `get_wallet_summary` from two
         // threads at the same time, as transactions cannot nest.
         wallet::get_wallet_summary(
@@ -337,7 +343,7 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
     fn get_sapling_nullifiers(
         &self,
         query: NullifierQuery,
-    ) -> Result<Vec<(AccountId, sapling::Nullifier)>, Self::Error> {
+    ) -> Result<Vec<(AccountId<Self::AccountPK>, sapling::Nullifier)>, Self::Error> {
         match query {
             NullifierQuery::Unspent => wallet::sapling::get_sapling_nullifiers(self.conn.borrow()),
             NullifierQuery::All => wallet::sapling::get_all_sapling_nullifiers(self.conn.borrow()),
@@ -346,7 +352,7 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
 
     fn get_transparent_receivers(
         &self,
-        _account: AccountId,
+        _account: AccountId<Self::AccountPK>,
     ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error> {
         #[cfg(feature = "transparent-inputs")]
         return wallet::get_transparent_receivers(self.conn.borrow(), &self.params, _account);
@@ -359,7 +365,7 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
 
     fn get_transparent_balances(
         &self,
-        _account: AccountId,
+        _account: AccountId<Self::AccountPK>,
         _max_height: BlockHeight,
     ) -> Result<HashMap<TransparentAddress, Amount>, Self::Error> {
         #[cfg(feature = "transparent-inputs")]
@@ -380,11 +386,11 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
     fn get_orchard_nullifiers(
         &self,
         _query: NullifierQuery,
-    ) -> Result<Vec<(AccountId, orchard::note::Nullifier)>, Self::Error> {
+    ) -> Result<Vec<(AccountId<Self::AccountPK>, orchard::note::Nullifier)>, Self::Error> {
         todo!()
     }
 
-    fn get_account_ids(&self) -> Result<Vec<AccountId>, Self::Error> {
+    fn get_account_ids(&self) -> Result<Vec<AccountId<Self::AccountPK>>, Self::Error> {
         wallet::get_account_ids(self.conn.borrow())
     }
 }
@@ -396,7 +402,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
         &mut self,
         seed: &SecretVec<u8>,
         birthday: AccountBirthday,
-    ) -> Result<(AccountId, UnifiedSpendingKey), Self::Error> {
+    ) -> Result<(AccountId<Self::AccountPK>, UnifiedSpendingKey), Self::Error> {
         let seed_id = HDSeedFingerprint::from_seed(seed);
         self.transactionally(|wdb| {
             let account_id = wallet::get_max_account_id(wdb.conn.0, &seed_id)?
@@ -420,7 +426,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
 
     fn get_next_available_address(
         &mut self,
-        account: AccountId,
+        account: AccountId<Self::AccountPK>,
         request: UnifiedAddressRequest,
     ) -> Result<Option<UnifiedAddress>, Self::Error> {
         self.transactionally(
@@ -460,7 +466,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
     #[allow(clippy::type_complexity)]
     fn put_blocks(
         &mut self,
-        blocks: Vec<ScannedBlock<sapling::Nullifier, Scope>>,
+        blocks: Vec<ScannedBlock<sapling::Nullifier, Scope, Self::AccountPK>>,
     ) -> Result<(), Self::Error> {
         self.transactionally(|wdb| {
             let start_positions = blocks.first().map(|block| {
@@ -599,11 +605,14 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
         Ok(())
     }
 
-    fn store_decrypted_tx(&mut self, d_tx: DecryptedTransaction) -> Result<(), Self::Error> {
+    fn store_decrypted_tx(
+        &mut self,
+        d_tx: DecryptedTransaction<Self::AccountPK>,
+    ) -> Result<(), Self::Error> {
         self.transactionally(|wdb| {
             let tx_ref = wallet::put_tx_data(wdb.conn.0, d_tx.tx, None, None)?;
 
-            let mut spending_account_id: Option<AccountId> = None;
+            let mut spending_account_id: Option<AccountId<Self::AccountPK>> = None;
             for output in d_tx.sapling_outputs {
                 match output.transfer_type {
                     TransferType::Outgoing | TransferType::WalletInternal => {
@@ -690,7 +699,10 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
         })
     }
 
-    fn store_sent_tx(&mut self, sent_tx: &SentTransaction) -> Result<(), Self::Error> {
+    fn store_sent_tx(
+        &mut self,
+        sent_tx: &SentTransaction<Self::AccountPK>,
+    ) -> Result<(), Self::Error> {
         self.transactionally(|wdb| {
             let tx_ref = wallet::put_tx_data(
                 wdb.conn.0,
@@ -1132,7 +1144,7 @@ extern crate assert_matches;
 mod tests {
     use zcash_client_backend::data_api::{AccountBirthday, WalletRead, WalletWrite};
 
-    use crate::{testing::TestBuilder, AccountId, DEFAULT_UA_REQUEST};
+    use crate::{testing::TestBuilder, DEFAULT_UA_REQUEST};
 
     #[cfg(feature = "unstable")]
     use {

zcash_client_sqlite/src/testing.rs

@@ -91,6 +91,8 @@ pub(crate) struct TestBuilder<Cache> {
     test_account_birthday: Option<AccountBirthday>,
 }
 
+type AccountPK = ();
+
 impl TestBuilder<()> {
     /// Constructs a new test.
     pub(crate) fn new() -> Self {
@@ -160,7 +162,7 @@ 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<(AccountId<AccountPK>, UnifiedSpendingKey, AccountBirthday)>,
 }
 
 impl<Cache: TestCache> TestState<Cache>
@@ -418,7 +420,9 @@ impl<Cache> TestState<Cache> {
     }
 
     /// Exposes the test account, if enabled via [`TestBuilder::with_test_account`].
-    pub(crate) fn test_account(&self) -> Option<(AccountId, UnifiedSpendingKey, AccountBirthday)> {
+    pub(crate) fn test_account(
+        &self,
+    ) -> Option<(AccountId<AccountPK>, UnifiedSpendingKey, AccountBirthday)> {
         self.test_account.as_ref().cloned()
     }
 
@@ -508,7 +512,7 @@ impl<Cache> TestState<Cache> {
     #[allow(clippy::type_complexity)]
     pub(crate) fn propose_transfer<InputsT>(
         &mut self,
-        spend_from_account: AccountId,
+        spend_from_account: AccountId<AccountPK>,
         input_selector: &InputsT,
         request: zip321::TransactionRequest,
         min_confirmations: NonZeroU32,
@@ -540,7 +544,7 @@ impl<Cache> TestState<Cache> {
     #[allow(clippy::too_many_arguments)]
     pub(crate) fn propose_standard_transfer<CommitmentTreeErrT>(
         &mut self,
-        spend_from_account: AccountId,
+        spend_from_account: AccountId<AccountPK>,
         fee_rule: StandardFeeRule,
         min_confirmations: NonZeroU32,
         to: &Address,
@@ -679,21 +683,21 @@ impl<Cache> TestState<Cache> {
 
     fn with_account_balance<T, F: FnOnce(&AccountBalance) -> T>(
         &self,
-        account: AccountId,
+        account: AccountId<AccountPK>,
         min_confirmations: u32,
         f: F,
     ) -> T {
         let binding = self.get_wallet_summary(min_confirmations).unwrap();
         f(binding.account_balances().get(&account).unwrap())
     }
 
-    pub(crate) fn get_total_balance(&self, account: AccountId) -> NonNegativeAmount {
+    pub(crate) fn get_total_balance(&self, account: AccountId<AccountPK>) -> NonNegativeAmount {
         self.with_account_balance(account, 0, |balance| balance.total())
     }
 
     pub(crate) fn get_spendable_balance(
         &self,
-        account: AccountId,
+        account: AccountId<AccountPK>,
         min_confirmations: u32,
     ) -> NonNegativeAmount {
         self.with_account_balance(account, min_confirmations, |balance| {
@@ -703,7 +707,7 @@ impl<Cache> TestState<Cache> {
 
     pub(crate) fn get_pending_shielded_balance(
         &self,
-        account: AccountId,
+        account: AccountId<AccountPK>,
         min_confirmations: u32,
     ) -> NonNegativeAmount {
         self.with_account_balance(account, min_confirmations, |balance| {
@@ -716,15 +720,18 @@ impl<Cache> TestState<Cache> {
     #[allow(dead_code)]
     pub(crate) fn get_pending_change(
         &self,
-        account: AccountId,
+        account: AccountId<AccountPK>,
         min_confirmations: u32,
     ) -> NonNegativeAmount {
         self.with_account_balance(account, min_confirmations, |balance| {
             balance.sapling_balance().change_pending_confirmation()
         })
     }
 
-    pub(crate) fn get_wallet_summary(&self, min_confirmations: u32) -> Option<WalletSummary> {
+    pub(crate) fn get_wallet_summary(
+        &self,
+        min_confirmations: u32,
+    ) -> Option<WalletSummary<AccountPK>> {
         get_wallet_summary(
             &self.wallet().conn.unchecked_transaction().unwrap(),
             &self.wallet().params,