From 94b47194d3a0356b8469d162ea03304ba1240d74 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 8 Mar 2024 21:21:58 -0700 Subject: [PATCH] zcash_client_backend: Permit spending pool selection. --- zcash_client_backend/CHANGELOG.md | 15 +- zcash_client_backend/src/data_api.rs | 79 +++++++++-- zcash_client_backend/src/data_api/wallet.rs | 20 +-- .../src/data_api/wallet/input_selection.rs | 19 +-- zcash_client_sqlite/src/chain.rs | 34 ++--- zcash_client_sqlite/src/lib.rs | 65 +++++---- zcash_client_sqlite/src/testing.rs | 27 ++-- zcash_client_sqlite/src/wallet.rs | 130 +++++++++++++----- zcash_client_sqlite/src/wallet/init.rs | 3 + .../init/migrations/full_account_ids.rs | 50 +++++-- zcash_client_sqlite/src/wallet/sapling.rs | 61 ++++---- zcash_keys/CHANGELOG.md | 4 +- zcash_keys/src/keys.rs | 27 ++++ 13 files changed, 373 insertions(+), 161 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 28c500e3de..501fbf09eb 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -15,6 +15,7 @@ and this library adheres to Rust's notion of - `zcash_client_backend::data_api`: - `AccountBalance::with_orchard_balance_mut` - `AccountBirthday::orchard_frontier` + - `AccountSources` - `BlockMetadata::orchard_tree_size` - `DecryptedTransaction::{new, tx(), orchard_outputs()}` - `ScannedBlock::orchard` @@ -49,10 +50,14 @@ and this library adheres to Rust's notion of - Arguments to `ScannedBlock::from_parts` have changed. - Changes to the `WalletRead` trait: - Added `get_orchard_nullifiers` + - `get_account_for_ufvk` now returns an `AccountSources` instead of a bare + `AccountId` - Changes to the `InputSource` trait: - - `select_spendable_notes` now takes its `target_value` argument as a - `NonNegativeAmount`. Also, the values of the returned map are also - `NonNegativeAmount`s instead of `Amount`s. + - `select_spendable_notes` has changed: + - It now takes its `target_value` argument as a `NonNegativeAmount`. + - Instead of an `AccountId`, it takes an `AccountSources` argument. The + separate `sources` argument has been removed. + - The values of the returned map are `NonNegativeAmount`s instead of `Amount`s. - Fields of `DecryptedTransaction` are now private. Use `DecryptedTransaction::new` and the newly provided accessors instead. - Fields of `SentTransaction` are now private. Use `SentTransaction::new` @@ -64,6 +69,10 @@ and this library adheres to Rust's notion of - `fn put_orchard_subtree_roots` - Added method `WalletRead::validate_seed` - Removed `Error::AccountNotFound` variant. + - `wallet::input_selection::InputSelector::propose_transaction` now takes an + `AccountSources` rather than a bare `AccountId`. + - `wallet::{propose_transfer, propose_standard_transfer_to_address}` now + each take an `AccountSources` instead of a bare `AccountId`. - `zcash_client_backend::decrypt`: - Fields of `DecryptedOutput` are now private. Use `DecryptedOutput::new` and the newly provided accessors instead. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 362c2bf113..750f73203c 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -308,6 +308,68 @@ impl AccountBalance { } } +/// A type that describes what FVK components are known to the wallet for an account. +#[derive(Clone, Copy, Debug)] +pub struct AccountSources { + account_id: AccountId, + use_orchard: bool, + use_sapling: bool, + use_transparent: bool, +} + +impl AccountSources { + /// Constructs AccountSources from its constituent parts + pub fn new( + account_id: AccountId, + use_orchard: bool, + use_sapling: bool, + use_transparent: bool, + ) -> Self { + Self { + account_id, + use_orchard, + use_sapling, + use_transparent, + } + } + + /// Returns the id for the account to which this metadata applies. + pub fn account_id(&self) -> AccountId { + self.account_id + } + + /// Returns whether the account has an Orchard balance and spendability determination + /// capability. + pub fn use_orchard(&self) -> bool { + self.use_orchard + } + + /// Returns whether the account has an Sapling balance and spendability determination + /// capability. + pub fn use_sapling(&self) -> bool { + self.use_sapling + } + + /// Returns whether the account has a Transparent balance determination capability. + pub fn use_transparent(&self) -> bool { + self.use_transparent + } + + /// Restricts the sources to be used to those for which the given [`UnifiedSpendingKey`] + /// provides a spending capability. + pub fn filter_with_usk(&mut self, usk: &UnifiedSpendingKey) { + self.use_orchard &= usk.has_orchard(); + self.use_sapling &= usk.has_sapling(); + self.use_transparent &= usk.has_transparent(); + } + + /// Returns the [`UnifiedAddressRequest`] that will produce a [`UnifiedAddress`] having + /// receivers corresponding to the spending capabilities described by this value. + pub fn to_unified_address_request(&self) -> Option { + UnifiedAddressRequest::new(self.use_orchard, self.use_sapling, self.use_transparent) + } +} + /// A polymorphic ratio type, usually used for rational numbers. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct Ratio { @@ -444,9 +506,8 @@ pub trait InputSource { /// be included. fn select_spendable_notes( &self, - account: Self::AccountId, + inputs_from: AccountSources, target_value: NonNegativeAmount, - sources: &[ShieldedProtocol], anchor_height: BlockHeight, exclude: &[Self::NoteRef], ) -> Result>, Self::Error>; @@ -606,7 +667,7 @@ pub trait WalletRead { fn get_account_for_ufvk( &self, ufvk: &UnifiedFullViewingKey, - ) -> Result, Self::Error>; + ) -> Result>, Self::Error>; /// Returns the wallet balances and sync status for an account given the specified minimum /// number of confirmations, or `Ok(None)` if the wallet has no balance data available. @@ -1372,9 +1433,10 @@ pub mod testing { }; use super::{ - chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata, - DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SentTransaction, - WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, AccountSources, + BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, + SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, + SAPLING_SHARD_HEIGHT, }; #[cfg(feature = "transparent-inputs")] @@ -1425,9 +1487,8 @@ pub mod testing { fn select_spendable_notes( &self, - _account: Self::AccountId, + _inputs_from: AccountSources, _target_value: NonNegativeAmount, - _sources: &[ShieldedProtocol], _anchor_height: BlockHeight, _exclude: &[Self::NoteRef], ) -> Result>, Self::Error> { @@ -1523,7 +1584,7 @@ pub mod testing { fn get_account_for_ufvk( &self, _ufvk: &UnifiedFullViewingKey, - ) -> Result, Self::Error> { + ) -> Result>, Self::Error> { Ok(None) } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index c2bc244626..2d8606452d 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -41,7 +41,7 @@ use sapling::{ }; use std::num::NonZeroU32; -use super::InputSource; +use super::{AccountSources, InputSource}; use crate::{ address::Address, data_api::{ @@ -405,7 +405,7 @@ where pub fn propose_transfer( wallet_db: &mut DbT, params: &ParamsT, - spend_from_account: ::AccountId, + sources: AccountSources<::AccountId>, input_selector: &InputsT, request: zip321::TransactionRequest, min_confirmations: NonZeroU32, @@ -435,7 +435,7 @@ where wallet_db, target_height, anchor_height, - spend_from_account, + sources, request, ) .map_err(Error::from) @@ -453,9 +453,10 @@ where /// * `wallet_db`: A read/write reference to the wallet database. /// * `params`: Consensus parameters. /// * `fee_rule`: The fee rule to use in creating the transaction. -/// * `spend_from_account`: The unified account that controls the funds that will be spent -/// in the resulting transaction. This procedure will return an error if the -/// account ID does not correspond to an account known to the wallet. +/// * `sources`: Metadata that describes the unified account and the pools from which +/// funds may be spent in the resulting transaction. This procedure will return an +/// error if the contained account ID does not correspond to an account known to +/// the wallet. /// * `min_confirmations`: The minimum number of confirmations that a previously /// received note must have in the blockchain in order to be considered for being /// spent. A value of 10 confirmations is recommended and 0-conf transactions are @@ -472,7 +473,7 @@ pub fn propose_standard_transfer_to_address( wallet_db: &mut DbT, params: &ParamsT, fee_rule: StandardFeeRule, - spend_from_account: ::AccountId, + sources: AccountSources<::AccountId>, min_confirmations: NonZeroU32, to: &Address, amount: NonNegativeAmount, @@ -520,7 +521,7 @@ where propose_transfer( wallet_db, params, - spend_from_account, + sources, &input_selector, request, min_confirmations, @@ -694,7 +695,8 @@ where let account = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) .map_err(Error::DataSource)? - .ok_or(Error::KeyNotRecognized)?; + .ok_or(Error::KeyNotRecognized)? + .account_id(); let (sapling_anchor, sapling_inputs) = if proposal_step.involves(PoolType::Shielded(ShieldedProtocol::Sapling)) { diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index 1627698bab..a327216d7e 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -21,7 +21,7 @@ use zcash_primitives::{ use crate::{ address::{Address, UnifiedAddress}, - data_api::InputSource, + data_api::{AccountSources, InputSource}, fees::{sapling, ChangeError, ChangeStrategy, DustOutputPolicy}, proposal::{Proposal, ProposalError, ShieldedInputs}, wallet::{Note, ReceivedNote, WalletTransparentOutput}, @@ -148,7 +148,7 @@ pub trait InputSelector { wallet_db: &Self::InputSource, target_height: BlockHeight, anchor_height: BlockHeight, - account: ::AccountId, + sources: AccountSources<::AccountId>, transaction_request: TransactionRequest, ) -> Result< Proposal::NoteRef>, @@ -328,7 +328,7 @@ where wallet_db: &Self::InputSource, target_height: BlockHeight, anchor_height: BlockHeight, - account: ::AccountId, + sources: AccountSources<::AccountId>, transaction_request: TransactionRequest, ) -> Result< Proposal, @@ -454,19 +454,8 @@ where Err(other) => return Err(other.into()), } - #[cfg(not(feature = "orchard"))] - let selectable_pools = &[ShieldedProtocol::Sapling]; - #[cfg(feature = "orchard")] - let selectable_pools = &[ShieldedProtocol::Sapling, ShieldedProtocol::Orchard]; - shielded_inputs = wallet_db - .select_spendable_notes( - account, - amount_required, - selectable_pools, - anchor_height, - &exclude, - ) + .select_spendable_notes(sources, amount_required, anchor_height, &exclude) .map_err(InputSelectorError::DataSource)?; let new_available = shielded_inputs diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 7585720383..b9cb51af74 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -438,7 +438,7 @@ mod tests { .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let account = st.test_account().unwrap(); + let account = st.test_account().unwrap().0.account_id(); let dfvk = st.test_account_sapling().unwrap(); @@ -455,7 +455,7 @@ mod tests { st.scan_cached_blocks(h, 2); // Account balance should reflect both received notes - assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); + assert_eq!(st.get_total_balance(account), (value + value2).unwrap()); // "Rewind" to height of last scanned block st.wallet_mut() @@ -463,7 +463,7 @@ mod tests { .unwrap(); // Account balance should be unaltered - assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); + assert_eq!(st.get_total_balance(account), (value + value2).unwrap()); // Rewind so that one block is dropped st.wallet_mut() @@ -471,13 +471,13 @@ mod tests { .unwrap(); // Account balance should only contain the first received note - assert_eq!(st.get_total_balance(account.0), value); + assert_eq!(st.get_total_balance(account), 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(account.0), (value + value2).unwrap()); + assert_eq!(st.get_total_balance(account), (value + value2).unwrap()); } #[test] @@ -486,7 +486,7 @@ mod tests { .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let account = st.test_account().unwrap(); + let account = st.test_account().unwrap().0.account_id(); let (_, usk, _) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); @@ -495,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(account.0), value); + assert_eq!(st.get_total_balance(account), value); // Create blocks to reach SAPLING_ACTIVATION_HEIGHT + 2 let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); @@ -507,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(account.0), + st.get_total_balance(account), NonNegativeAmount::const_from_u64(150_000) ); @@ -543,7 +543,7 @@ mod tests { .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let account = st.test_account().unwrap(); + let account = st.test_account().unwrap().0.account_id(); let dfvk = st.test_account_sapling().unwrap(); @@ -561,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(account.0), value); + assert_eq!(st.get_total_balance(account), value); // Create a second fake CompactBlock sending more value to the address let value2 = NonNegativeAmount::const_from_u64(7); @@ -574,7 +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(account.0), (value + value2).unwrap()); + assert_eq!(st.get_total_balance(account), (value + value2).unwrap()); } #[test] @@ -583,7 +583,7 @@ mod tests { .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let account = st.test_account().unwrap(); + let account = st.test_account().unwrap().0.account_id(); let dfvk = st.test_account_sapling().unwrap(); // Wallet summary is not yet available @@ -598,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(account.0), value); + assert_eq!(st.get_total_balance(account), value); // Create a second fake CompactBlock spending value from the address let extsk2 = ExtendedSpendingKey::master(&[0]); @@ -610,7 +610,7 @@ mod tests { st.scan_cached_blocks(spent_height, 1); // Account balance should equal the change - assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap()); + assert_eq!(st.get_total_balance(account), (value - value2).unwrap()); } #[test] @@ -619,7 +619,7 @@ mod tests { .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let account = st.test_account().unwrap(); + let account = st.test_account().unwrap().0.account_id(); let dfvk = st.test_account_sapling().unwrap(); @@ -641,12 +641,12 @@ mod tests { st.scan_cached_blocks(spent_height, 1); // Account balance should equal the change - assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap()); + assert_eq!(st.get_total_balance(account), (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(account.0), (value - value2).unwrap()); + assert_eq!(st.get_total_balance(account), (value - value2).unwrap()); } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index b04ea3e5a7..59b397cdec 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -60,9 +60,9 @@ use zcash_client_backend::{ self, chain::{BlockSource, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, - AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, - ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, - WalletWrite, SAPLING_SHARD_HEIGHT, + AccountBirthday, AccountSources, BlockMetadata, DecryptedTransaction, InputSource, + NullifierQuery, ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, + WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }, keys::{ AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey, @@ -212,20 +212,23 @@ impl, P: consensus::Parameters> InputSource for fn select_spendable_notes( &self, - account: AccountId, + sources: AccountSources, target_value: NonNegativeAmount, - _sources: &[ShieldedProtocol], anchor_height: BlockHeight, exclude: &[Self::NoteRef], ) -> Result>, Self::Error> { - wallet::sapling::select_spendable_sapling_notes( - self.conn.borrow(), - &self.params, - account, - target_value, - anchor_height, - exclude, - ) + if sources.use_sapling() { + wallet::sapling::select_spendable_sapling_notes( + self.conn.borrow(), + &self.params, + sources.account_id(), + target_value, + anchor_height, + exclude, + ) + } else { + Ok(vec![]) + } } #[cfg(feature = "transparent-inputs")] @@ -373,8 +376,8 @@ impl, P: consensus::Parameters> WalletRead for W fn get_account_for_ufvk( &self, ufvk: &UnifiedFullViewingKey, - ) -> Result, Self::Error> { - wallet::get_account_for_ufvk(self.conn.borrow(), &self.params, ufvk) + ) -> Result>, Self::Error> { + wallet::get_account_for_ufvk(self.conn.borrow(), ufvk) } fn get_wallet_summary( @@ -1316,7 +1319,7 @@ 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, AccountId}; #[cfg(feature = "unstable")] use { @@ -1331,7 +1334,7 @@ mod tests { .build(); assert!({ - let account = st.test_account().unwrap().0; + let account = st.test_account().unwrap().0.account_id(); st.wallet() .validate_seed(account, st.test_seed().unwrap()) .unwrap() @@ -1347,7 +1350,7 @@ mod tests { // check that passing an invalid seed results in a failure assert!({ - let account = st.test_account().unwrap().0; + let account = st.test_account().unwrap().0.account_id(); !st.wallet() .validate_seed(account, &SecretVec::new(vec![1u8; 32])) .unwrap() @@ -1359,20 +1362,29 @@ mod tests { let mut st = TestBuilder::new() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let account = st.test_account().unwrap(); + let sources = st.test_account().unwrap().0; - let current_addr = st.wallet().get_current_address(account.0).unwrap(); + let current_addr = st + .wallet() + .get_current_address(sources.account_id()) + .unwrap(); assert!(current_addr.is_some()); // TODO: Add Orchard let addr2 = st .wallet_mut() - .get_next_available_address(account.0, DEFAULT_UA_REQUEST) + .get_next_available_address( + sources.account_id(), + sources.to_unified_address_request().unwrap(), + ) .unwrap(); assert!(addr2.is_some()); assert_ne!(current_addr, addr2); - let addr2_cur = st.wallet().get_current_address(account.0).unwrap(); + let addr2_cur = st + .wallet() + .get_current_address(sources.account_id()) + .unwrap(); assert_eq!(addr2, addr2_cur); } @@ -1384,17 +1396,20 @@ mod tests { .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let account = st.test_account().unwrap(); + let sources = st.test_account().unwrap().0; let (_, usk, _) = st.test_account().unwrap(); let ufvk = usk.to_unified_full_viewing_key(); let (taddr, _) = usk.default_transparent_address(); - let receivers = st.wallet().get_transparent_receivers(account.0).unwrap(); + let receivers = st + .wallet() + .get_transparent_receivers(sources.account_id()) + .unwrap(); // The receiver for the default UA should be in the set. assert!(receivers.contains_key( - ufvk.default_address(DEFAULT_UA_REQUEST) + ufvk.default_address(sources.to_unified_address_request().unwrap()) .expect("A valid default address exists for the UFVK") .0 .transparent() diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index a37ff5c2ca..4db472c2f4 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -44,6 +44,7 @@ use zcash_client_backend::{ zip321, }; use zcash_client_backend::{ + data_api::AccountSources, fees::{standard, DustOutputPolicy}, ShieldedProtocol, }; @@ -438,10 +439,20 @@ impl TestState { } /// 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() - .map(|(_, a, k, b)| (*a, k.clone(), b.clone())) + pub(crate) fn test_account( + &self, + ) -> Option<( + AccountSources, + UnifiedSpendingKey, + AccountBirthday, + )> { + self.test_account.as_ref().map(|(_, a, k, b)| { + ( + AccountSources::new(*a, k.has_orchard(), k.has_sapling(), k.has_transparent()), + k.clone(), + b.clone(), + ) + }) } /// Exposes the test account's Sapling DFVK, if enabled via [`TestBuilder::with_test_account`]. @@ -533,7 +544,7 @@ impl TestState { #[allow(clippy::type_complexity)] pub(crate) fn propose_transfer( &mut self, - spend_from_account: AccountId, + account_sources: AccountSources, input_selector: &InputsT, request: zip321::TransactionRequest, min_confirmations: NonZeroU32, @@ -553,7 +564,7 @@ impl TestState { propose_transfer::<_, _, _, Infallible>( &mut self.db_data, ¶ms, - spend_from_account, + account_sources, input_selector, request, min_confirmations, @@ -565,7 +576,7 @@ impl TestState { #[allow(clippy::too_many_arguments)] pub(crate) fn propose_standard_transfer( &mut self, - spend_from_account: AccountId, + account_sources: AccountSources, fee_rule: StandardFeeRule, min_confirmations: NonZeroU32, to: &Address, @@ -587,7 +598,7 @@ impl TestState { &mut self.db_data, ¶ms, fee_rule, - spend_from_account, + account_sources, min_confirmations, to, amount, diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 0a8a02a4e3..21881aa7e5 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -75,6 +75,7 @@ use std::num::NonZeroU32; use std::ops::RangeInclusive; use tracing::debug; use zcash_address::unified::{Encoding, Ivk, Uivk}; +use zcash_client_backend::data_api::AccountSources; use zcash_keys::keys::{AddressGenerationError, HdSeedFingerprint, UnifiedAddressRequest}; use zcash_client_backend::{ @@ -216,7 +217,7 @@ impl Account { /// Returns the default Unified Address for the account, /// along with the diversifier index that generated it. /// - /// The diversifier index may be non-zero if the Unified Address includes a Sapling + /// The diversifier index may be non-zero if the Unified Address includes a Sapling /// receiver, and there was no valid Sapling receiver at diversifier index zero. pub fn default_address( &self, @@ -289,11 +290,11 @@ struct AccountSqlValues<'a> { account_type: u32, hd_seed_fingerprint: Option<&'a [u8]>, hd_account_index: Option, - ufvk: Option, + ufvk: Option<&'a UnifiedFullViewingKey>, uivk: String, } -/// Returns (account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk) for a given account. +/// Returns (account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk) for a given account. fn get_sql_values_for_account_parameters<'a, P: consensus::Parameters>( account: &'a Account, params: &P, @@ -303,14 +304,14 @@ fn get_sql_values_for_account_parameters<'a, P: consensus::Parameters>( account_type: AccountType::Zip32.into(), hd_seed_fingerprint: Some(hdaccount.hd_seed_fingerprint().as_bytes()), hd_account_index: Some(hdaccount.account_index().into()), - ufvk: Some(hdaccount.ufvk().encode(params)), + ufvk: Some(hdaccount.ufvk()), uivk: ufvk_to_uivk(hdaccount.ufvk(), params)?, }, Account::Imported(ImportedAccount::Full(ufvk)) => AccountSqlValues { account_type: AccountType::Imported.into(), hd_seed_fingerprint: None, hd_account_index: None, - ufvk: Some(ufvk.encode(params)), + ufvk: Some(ufvk), uivk: ufvk_to_uivk(ufvk, params)?, }, Account::Imported(ImportedAccount::Incoming(uivk)) => AccountSqlValues { @@ -355,21 +356,49 @@ pub(crate) fn add_account( birthday: AccountBirthday, ) -> Result { let args = get_sql_values_for_account_parameters(&account, params)?; - let account_id: AccountId = conn.query_row(r#" - INSERT INTO accounts (account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk, birthday_height, recover_until_height) - VALUES (:account_type, :hd_seed_fingerprint, :hd_account_index, :ufvk, :uivk, :birthday_height, :recover_until_height) + + let orchard_item = args + .ufvk + .and_then(|ufvk| ufvk.orchard().map(|k| k.to_bytes())); + let sapling_item = args + .ufvk + .and_then(|ufvk| ufvk.sapling().map(|k| k.to_bytes())); + #[cfg(feature = "transparent-inputs")] + let transparent_item = args + .ufvk + .and_then(|ufvk| ufvk.transparent().map(|k| k.serialize())); + #[cfg(not(feature = "transparent-inputs"))] + let transparent_item: Option> = None; + + let account_id: AccountId = conn.query_row( + r#" + INSERT INTO accounts ( + account_type, hd_seed_fingerprint, hd_account_index, + ufvk, uivk, + orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache, + birthday_height, recover_until_height + ) + VALUES ( + :account_type, :hd_seed_fingerprint, :hd_account_index, + :ufvk, :uivk, + :orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache, + :birthday_height, :recover_until_height + ) RETURNING id; "#, named_params![ ":account_type": args.account_type, ":hd_seed_fingerprint": args.hd_seed_fingerprint, ":hd_account_index": args.hd_account_index, - ":ufvk": args.ufvk, + ":ufvk": args.ufvk.map(|ufvk| ufvk.encode(params)), ":uivk": args.uivk, + ":orchard_fvk_item_cache": orchard_item, + ":sapling_fvk_item_cache": sapling_item, + ":p2pkh_fvk_item_cache": transparent_item, ":birthday_height": u32::from(birthday.height()), ":recover_until_height": birthday.recover_until().map(u32::from) ], - |row| Ok(AccountId(row.get(0)?)) + |row| Ok(AccountId(row.get(0)?)), )?; // If a birthday frontier is available, insert it into the note commitment tree. If the @@ -665,21 +694,49 @@ pub(crate) fn get_unified_full_viewing_keys( /// Returns the account id corresponding to a given [`UnifiedFullViewingKey`], /// if any. -pub(crate) fn get_account_for_ufvk( +pub(crate) fn get_account_for_ufvk( conn: &rusqlite::Connection, - params: &P, ufvk: &UnifiedFullViewingKey, -) -> Result, SqliteClientError> { - conn.query_row( - "SELECT id FROM accounts WHERE ufvk = ?", - [&ufvk.encode(params)], - |row| { - let acct = row.get(0)?; - Ok(AccountId(acct)) - }, - ) - .optional() - .map_err(SqliteClientError::from) +) -> Result>, SqliteClientError> { + #[cfg(feature = "transparent-inputs")] + let transparent_item = ufvk.transparent().map(|k| k.serialize()); + #[cfg(not(feature = "transparent-inputs"))] + let transparent_item: Option> = None; + + let mut stmt = conn.prepare( + "SELECT id, + (orchard_fvk_item_cache == :orchard_fvk_item_cache) AS has_orchard, + (sapling_fvk_item_cache == :sapling_fvk_item_cache) AS has_sapling, + (p2pkh_fvk_item_cache == :p2pkh_fvk_item_cache) AS has_transparent + FROM accounts + WHERE has_orchard OR has_sapling OR has_transparent", + )?; + + let accounts = stmt + .query_and_then::<_, rusqlite::Error, _, _>( + named_params![ + ":orchard_fvk_item_cache": ufvk.orchard().map(|k| k.to_bytes()), + ":sapling_fvk_item_cache": ufvk.sapling().map(|k| k.to_bytes()), + ":p2pkh_fvk_item_cache": transparent_item, + ], + |row| { + Ok(AccountSources::new( + row.get::<_, u32>(0).map(AccountId)?, + row.get::<_, Option>(1)?.unwrap_or(false), + row.get::<_, Option>(2)?.unwrap_or(false), + row.get::<_, Option>(3)?.unwrap_or(false), + )) + }, + )? + .collect::, _>>()?; + + if accounts.len() > 1 { + Err(SqliteClientError::CorruptedData( + "Mutiple account records correspond to a single UFVK".to_owned(), + )) + } else { + Ok(accounts.first().copied()) + } } pub(crate) trait ScanProgress { @@ -2341,7 +2398,7 @@ mod tests { let st = TestBuilder::new() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let account = st.test_account().unwrap(); + let account = st.test_account().unwrap().0.account_id(); // The account should have no summary information assert_eq!(st.get_wallet_summary(0), None); @@ -2355,11 +2412,11 @@ mod tests { ); // The default address is set for the test account - assert_matches!(st.wallet().get_current_address(account.0), Ok(Some(_))); + assert_matches!(st.wallet().get_current_address(account), Ok(Some(_))); // No default address is set for an un-initialized account assert_matches!( - st.wallet().get_current_address(AccountId(account.0 .0 + 1)), + st.wallet().get_current_address(AccountId(account.0 + 1)), Ok(None) ); } @@ -2373,10 +2430,10 @@ mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account_id, _, _) = st.test_account().unwrap(); + let (sources, _, _) = st.test_account().unwrap(); let uaddr = st .wallet() - .get_current_address(account_id) + .get_current_address(sources.account_id()) .unwrap() .unwrap(); let taddr = uaddr.transparent().unwrap(); @@ -2384,7 +2441,7 @@ mod tests { let height_1 = BlockHeight::from_u32(12345); let bal_absent = st .wallet() - .get_transparent_balances(account_id, height_1) + .get_transparent_balances(sources.account_id(), height_1) .unwrap(); assert!(bal_absent.is_empty()); @@ -2436,7 +2493,7 @@ mod tests { ); assert_matches!( - st.wallet().get_transparent_balances(account_id, height_2), + st.wallet().get_transparent_balances(sources.account_id(), height_2), Ok(h) if h.get(taddr) == Some(&value) ); @@ -2461,7 +2518,7 @@ mod tests { let st = TestBuilder::new() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let account_id = st.test_account().unwrap().0; + let account_id = st.test_account().unwrap().0.account_id(); let account_parameters = get_account(st.wallet(), account_id).unwrap().unwrap(); let expected_account_index = zip32::AccountId::try_from(0).unwrap(); @@ -2481,10 +2538,10 @@ mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account_id, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); let uaddr = st .wallet() - .get_current_address(account_id) + .get_current_address(sources.account_id()) .unwrap() .unwrap(); let taddr = uaddr.transparent().unwrap(); @@ -2506,14 +2563,17 @@ mod tests { .get_wallet_summary(min_confirmations) .unwrap() .unwrap(); - let balance = summary.account_balances().get(&account_id).unwrap(); + let balance = summary + .account_balances() + .get(&sources.account_id()) + .unwrap(); assert_eq!(balance.unshielded(), expected); // Check the older APIs for consistency. let max_height = st.wallet().chain_height().unwrap().unwrap() + 1 - min_confirmations; assert_eq!( st.wallet() - .get_transparent_balances(account_id, max_height) + .get_transparent_balances(sources.account_id(), max_height) .unwrap() .get(taddr) .cloned() diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index bb59d303d9..6445ca5f28 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -230,6 +230,9 @@ mod tests { hd_account_index INTEGER, ufvk TEXT, uivk TEXT NOT NULL, + orchard_fvk_item_cache BLOB, + sapling_fvk_item_cache BLOB, + p2pkh_fvk_item_cache BLOB, 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 AND ufvk IS NOT NULL) OR (account_type = 1 AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL) ) diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index be084c6767..5f44ef037a 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -58,6 +58,9 @@ impl RusqliteMigration for Migration

{ hd_account_index INTEGER, ufvk TEXT, uivk TEXT NOT NULL, + orchard_fvk_item_cache BLOB, + sapling_fvk_item_cache BLOB, + p2pkh_fvk_item_cache BLOB, birthday_height INTEGER NOT NULL, recover_until_height INTEGER, CHECK ( @@ -116,19 +119,40 @@ impl RusqliteMigration for Migration

{ let uivk = ufvk_to_uivk(&ufvk_parsed, &self.params) .map_err(|e| WalletMigrationError::CorruptedData(e.to_string()))?; - transaction.execute(r#" - INSERT INTO accounts_new (id, account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk, birthday_height, recover_until_height) - VALUES (:account_id, :account_type, :seed_id, :account_index, :ufvk, :uivk, :birthday_height, :recover_until_height); - "#, named_params![ - ":account_id": account_id, - ":account_type": account_type, - ":seed_id": seed_id.as_bytes(), - ":account_index": account_index, - ":ufvk": ufvk, - ":uivk": uivk, - ":birthday_height": birthday_height, - ":recover_until_height": recover_until_height, - ])?; + #[cfg(feature = "transparent-inputs")] + let transparent_item = ufvk_parsed.transparent().map(|k| k.serialize()); + #[cfg(not(feature = "transparent-inputs"))] + let transparent_item: Option> = None; + + transaction.execute( + r#" + INSERT INTO accounts_new ( + id, account_type, hd_seed_fingerprint, hd_account_index, + ufvk, uivk, + orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache, + birthday_height, recover_until_height + ) + VALUES ( + :account_id, :account_type, :seed_id, :account_index, + :ufvk, :uivk, + :orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache, + :birthday_height, :recover_until_height + ); + "#, + named_params![ + ":account_id": account_id, + ":account_type": account_type, + ":seed_id": seed_id.as_bytes(), + ":account_index": account_index, + ":ufvk": ufvk, + ":uivk": uivk, + ":orchard_fvk_item_cache": ufvk_parsed.orchard().map(|k| k.to_bytes()), + ":sapling_fvk_item_cache": ufvk_parsed.sapling().map(|k| k.to_bytes()), + ":p2pkh_fvk_item_cache": transparent_item, + ":birthday_height": birthday_height, + ":recover_until_height": recover_until_height, + ], + )?; } } else { return Err(WalletMigrationError::SeedRequired); diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index bcf78d29a1..2533e46075 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -542,7 +542,8 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); + let account = sources.account_id(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note @@ -590,7 +591,7 @@ pub(crate) mod tests { let proposal = st .propose_transfer( - account, + sources, input_selector, request, NonZeroU32::new(1).unwrap(), @@ -697,7 +698,8 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); + let account = sources.account_id(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note @@ -740,7 +742,7 @@ pub(crate) mod tests { ); let proposal0 = st .propose_transfer( - account, + sources, &input_selector, request0, NonZeroU32::new(1).unwrap(), @@ -904,7 +906,8 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); + let account = sources.account_id(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note @@ -952,7 +955,7 @@ pub(crate) mod tests { let to = extsk2.default_address().1.into(); assert_matches!( st.propose_standard_transfer::( - account, + sources, StandardFeeRule::Zip317, NonZeroU32::new(2).unwrap(), &to, @@ -982,7 +985,7 @@ pub(crate) mod tests { // Spend still fails assert_matches!( st.propose_standard_transfer::( - account, + sources, StandardFeeRule::Zip317, NonZeroU32::new(10).unwrap(), &to, @@ -1017,7 +1020,7 @@ pub(crate) mod tests { let min_confirmations = NonZeroU32::new(10).unwrap(); let proposal = st .propose_standard_transfer::( - account, + sources, StandardFeeRule::Zip317, min_confirmations, &to, @@ -1052,7 +1055,8 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); + let account = sources.account_id(); let dfvk = st.test_account_sapling().unwrap(); // TODO: This test was originally written to use the pre-zip-313 fee rule @@ -1075,7 +1079,7 @@ pub(crate) mod tests { let min_confirmations = NonZeroU32::new(1).unwrap(); let proposal = st .propose_standard_transfer::( - account, + sources, fee_rule, min_confirmations, &to, @@ -1095,7 +1099,7 @@ pub(crate) mod tests { // A second proposal fails because there are no usable notes assert_matches!( st.propose_standard_transfer::( - account, + sources, fee_rule, NonZeroU32::new(1).unwrap(), &to, @@ -1125,7 +1129,7 @@ pub(crate) mod tests { // Second proposal still fails assert_matches!( st.propose_standard_transfer::( - account, + sources, fee_rule, NonZeroU32::new(1).unwrap(), &to, @@ -1158,7 +1162,7 @@ pub(crate) mod tests { let min_confirmations = NonZeroU32::new(1).unwrap(); let proposal = st .propose_standard_transfer::( - account, + sources, fee_rule, min_confirmations, &to, @@ -1191,7 +1195,8 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); + let account = sources.account_id(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note @@ -1226,7 +1231,7 @@ pub(crate) mod tests { > { let min_confirmations = NonZeroU32::new(1).unwrap(); let proposal = st.propose_standard_transfer( - account, + sources, fee_rule, min_confirmations, &to, @@ -1307,7 +1312,8 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); + let account = sources.account_id(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note @@ -1329,7 +1335,7 @@ pub(crate) mod tests { let min_confirmations = NonZeroU32::new(1).unwrap(); let proposal = st .propose_standard_transfer::( - account, + sources, fee_rule, min_confirmations, &to, @@ -1354,7 +1360,8 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); + let account = sources.account_id(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet in a single note owned by the internal spending key @@ -1392,7 +1399,7 @@ pub(crate) mod tests { let min_confirmations = NonZeroU32::new(1).unwrap(); let proposal = st .propose_standard_transfer::( - account, + sources, fee_rule, min_confirmations, &to, @@ -1533,7 +1540,8 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); + let account = sources.account_id(); let dfvk = st.test_account_sapling().unwrap(); // Add funds to the wallet @@ -1628,7 +1636,8 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account_id, usk, _) = st.test_account().unwrap(); + let (sources, usk, _) = st.test_account().unwrap(); + let account_id = sources.account_id(); let dfvk = st.test_account_sapling().unwrap(); let uaddr = st @@ -1742,11 +1751,11 @@ pub(crate) mod tests { st.scan_cached_blocks(birthday.height() + 5, 20); // Verify that the received note is not considered spendable - let account = st.test_account().unwrap(); + let account = st.test_account().unwrap().0.account_id(); let spendable = select_spendable_sapling_notes( &st.wallet().conn, &st.wallet().params, - account.0, + account, NonNegativeAmount::const_from_u64(300000), received_tx_height + 10, &[], @@ -1762,7 +1771,7 @@ pub(crate) mod tests { let spendable = select_spendable_sapling_notes( &st.wallet().conn, &st.wallet().params, - account.0, + account, NonNegativeAmount::const_from_u64(300000), received_tx_height + 10, &[], @@ -1779,7 +1788,7 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (account, usk, birthday) = st.test_account().unwrap(); + let (sources, usk, birthday) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); // Generate a block with funds belonging to our wallet. @@ -1816,7 +1825,7 @@ pub(crate) mod tests { let spendable = select_spendable_sapling_notes( &st.wallet().conn, &st.wallet().params, - account, + sources.account_id(), NonNegativeAmount::const_from_u64(300000), birthday.height() + 5, &[], diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 79654d4c95..38cc1b0a1f 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -7,7 +7,9 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Added -- `zcash_keys::keys::HdSeedFingerprint` +- `zcash_keys::keys`: + - `HdSeedFingerprint` + - `UnifiedSpendingKey::{has_orchard, has_sapling, has_transparent}` - `zcash_keys::address::Address::has_receiver` - `impl Display for zcash_keys::keys::AddressGenerationError` - `impl std::error::Error for zcash_keys::keys::AddressGenerationError` diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 27dfb8393b..779cbe4f27 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -269,18 +269,45 @@ impl UnifiedSpendingKey { &self.transparent } + /// Returns whether the key provides transparent P2PKH spending capability. + pub fn has_transparent(&self) -> bool { + #[cfg(feature = "transparent-inputs")] + return true; + + #[cfg(not(feature = "transparent-inputs"))] + return false; + } + /// Returns the Sapling extended spending key component of this unified spending key. #[cfg(feature = "sapling")] pub fn sapling(&self) -> &sapling::ExtendedSpendingKey { &self.sapling } + /// Returns whether the key provides Sapling spending capability. + pub fn has_sapling(&self) -> bool { + #[cfg(feature = "sapling")] + return true; + + #[cfg(not(feature = "sapling"))] + return false; + } + /// Returns the Orchard spending key component of this unified spending key. #[cfg(feature = "orchard")] pub fn orchard(&self) -> &orchard::keys::SpendingKey { &self.orchard } + /// Returns whether the key provides Orchard spending capability. + pub fn has_orchard(&self) -> bool { + #[cfg(feature = "orchard")] + return true; + + #[cfg(not(feature = "orchard"))] + return false; + } + /// Returns a binary encoding of this key suitable for decoding with [`decode`]. /// /// The encoded form of a unified spending key is only intended for use