From 1af90e34245fd9f13e0bd31afff0aa389c0e6828 Mon Sep 17 00:00:00 2001 From: Daira-Emma Hopwood Date: Sat, 18 May 2024 03:21:07 +0100 Subject: [PATCH] Most of the rest of the ZIP 320 implementation. Signed-off-by: Daira-Emma Hopwood --- zcash_client_backend/src/data_api.rs | 24 ++-- zcash_client_backend/src/data_api/wallet.rs | 35 +++--- zcash_client_sqlite/src/lib.rs | 61 +++++++---- .../src/wallet/address_tracker.rs | 103 +++++++++++++++--- 4 files changed, 167 insertions(+), 56 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 4403f9a06f..c8efc45d73 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1615,8 +1615,8 @@ pub trait WalletCommitmentTrees { /// An error related to tracking of ephemeral transparent addresses. #[derive(Debug, Clone, PartialEq, Eq)] pub enum AddressTrackingError { - /// Only ZIP 32 accounts are supported for ZIP 320 (TEX Addresses). - UnsupportedAccountType, + /// The account id could not be found. + AccountNotFound, /// The proposal cannot be constructed until transactions with previously reserved /// ephemeral address outputs have been mined. @@ -1629,9 +1629,9 @@ pub enum AddressTrackingError { impl Display for AddressTrackingError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - AddressTrackingError::UnsupportedAccountType => write!( + AddressTrackingError::AccountNotFound => write!( f, - "Only ZIP 32 accounts are supported for ZIP 320 (TEX Addresses)." + "The account id could not be found." ), AddressTrackingError::ReachedGapLimit => write!( f, @@ -1650,6 +1650,9 @@ impl Display for AddressTrackingError { /// This trait serves to allow the corresponding wallet functions to be abstracted /// away from any particular data storage substrate. pub trait WalletAddressTracking { + /// The type of the account identifier. + type AccountId: Copy + Debug + Eq + Hash; + /// Reserves the next available address. /// /// To ensure that sufficient information is stored on-chain to allow recovering @@ -1665,7 +1668,7 @@ pub trait WalletAddressTracking { /// [BIP 44]: https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#user-content-Address_gap_limit fn reserve_next_address( &self, - uivk: &UnifiedIncomingViewingKey, + account_id: Self::AccountId, ) -> Result; /// Frees previously reserved ephemeral transparent addresses. @@ -1679,12 +1682,14 @@ pub trait WalletAddressTracking { /// account. fn unreserve_addresses( &self, + account_id: Self::AccountId, address: &[TransparentAddress], ) -> Result<(), AddressTrackingError>; /// Mark addresses as having been used. fn mark_addresses_as_used( &self, + account_id: Self::AccountId, address: &[TransparentAddress], ) -> Result<(), AddressTrackingError>; @@ -1693,6 +1698,7 @@ pub trait WalletAddressTracking { /// index if necessary. fn mark_addresses_as_mined( &self, + account_id: Self::AccountId, addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError>; } @@ -1703,7 +1709,6 @@ pub mod testing { use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, store::memory::MemoryShardStore, ShardTree}; use std::{collections::HashMap, convert::Infallible, num::NonZeroU32}; - use zcash_keys::keys::UnifiedIncomingViewingKey; use zip32::fingerprint::SeedFingerprint; use zcash_primitives::{ @@ -2019,15 +2024,18 @@ pub mod testing { } impl WalletAddressTracking for MockWalletDb { + type AccountId = u32; + fn reserve_next_address( &self, - _uivk: &UnifiedIncomingViewingKey, + _account_id: Self::AccountId, ) -> Result { Err(AddressTrackingError::ReachedGapLimit) } fn unreserve_addresses( &self, + _account_id: Self::AccountId, _addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError> { Ok(()) @@ -2035,6 +2043,7 @@ pub mod testing { fn mark_addresses_as_used( &self, + _account_id: Self::AccountId, _addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError> { Ok(()) @@ -2042,6 +2051,7 @@ pub mod testing { fn mark_addresses_as_mined( &self, + _account_id: Self::AccountId, _addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError> { Ok(()) diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 31b7fdee3c..b59248c152 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -257,7 +257,8 @@ where Error = ::Error, AccountId = ::AccountId, >, - DbT: WalletCommitmentTrees + WalletAddressTracking, + DbT: WalletCommitmentTrees, + DbT: WalletAddressTracking::AccountId>, { let account = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) @@ -368,7 +369,8 @@ where Error = ::Error, AccountId = ::AccountId, >, - DbT: WalletCommitmentTrees + WalletAddressTracking, + DbT: WalletCommitmentTrees, + DbT: WalletAddressTracking::AccountId>, ParamsT: consensus::Parameters + Clone, InputsT: InputSelector, { @@ -601,7 +603,8 @@ pub fn create_proposed_transactions( >, > where - DbT: WalletWrite + WalletCommitmentTrees + WalletAddressTracking, + DbT: WalletWrite + WalletCommitmentTrees, + DbT: WalletAddressTracking::AccountId>, ParamsT: consensus::Parameters + Clone, FeeRuleT: FeeRule, { @@ -654,8 +657,8 @@ fn create_proposed_transaction( >, > where - DbT: WalletWrite + WalletCommitmentTrees + WalletAddressTracking, - + DbT: WalletWrite + WalletCommitmentTrees, + DbT: WalletAddressTracking::AccountId>, ParamsT: consensus::Parameters + Clone, FeeRuleT: FeeRule, { @@ -1096,8 +1099,9 @@ where // TODO: make sure that this is supposed to be an ephemeral output (by checking // for backlinks) rather than a non-ephemeral transparent change output. let ephemeral_addr = wallet_db - .reserve_next_address(&account.uivk()) + .reserve_next_address(account_id) .map_err(InputSelectorError::from)?; + ephemeral_addrs.push(ephemeral_addr); builder.add_transparent_output(&ephemeral_addr, change_value.value())?; transparent_output_meta.push((ephemeral_addr, change_value.value())) @@ -1221,11 +1225,13 @@ where }; let res = finish(); - match res { - Ok(_) => wallet_db.mark_addresses_as_used(&ephemeral_addrs), - Err(_) => wallet_db.unreserve_addresses(&ephemeral_addrs), + if !ephemeral_addrs.is_empty() { + match res { + Ok(_) => wallet_db.mark_addresses_as_used(account_id, &ephemeral_addrs), + Err(_) => wallet_db.unreserve_addresses(account_id, &ephemeral_addrs), + } + .map_err(InputSelectorError::from)?; } - .map_err(InputSelectorError::from)?; res } @@ -1287,10 +1293,11 @@ pub fn shield_transparent_funds( > where ParamsT: consensus::Parameters, - DbT: WalletWrite - + WalletCommitmentTrees - + WalletAddressTracking - + InputSource::Error>, + DbT: WalletRead, + DbT: WalletWrite, + DbT: WalletCommitmentTrees, + DbT: InputSource::Error>, + DbT: WalletAddressTracking::AccountId>, InputsT: ShieldingSelector, { let proposal = propose_shielding( diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index fe910164b4..5f35f4eeb0 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -42,7 +42,7 @@ use rusqlite::{self, Connection}; use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, ShardTree}; use std::{ - borrow::Borrow, collections::HashMap, convert::AsRef, fmt, num::NonZeroU32, ops::Range, + borrow::Borrow, collections::{HashMap, hash_map::Entry::{Occupied, Vacant}}, convert::AsRef, fmt, num::NonZeroU32, ops::Range, path::Path, rc::Rc, sync::Mutex, }; use subtle::ConditionallySelectable; @@ -66,7 +66,7 @@ use zcash_client_backend::{ wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput}, DecryptedOutput, PoolType, ShieldedProtocol, TransferType, }; -use zcash_keys::{address::Address, keys::UnifiedIncomingViewingKey}; +use zcash_keys::address::Address; use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight}, @@ -171,7 +171,7 @@ pub struct UtxoId(pub i64); pub struct WalletDb { conn: C, params: P, - address_tracker: Rc>, + address_trackers: Rc>>, } /// A wrapper for a SQLite transaction affecting the wallet database. @@ -188,11 +188,10 @@ impl WalletDb { pub fn for_path>(path: F, params: P) -> Result { Connection::open(path).and_then(move |conn| { rusqlite::vtab::array::load_module(&conn)?; - let tracker = AddressTracker::new(&conn, ¶ms); Ok(WalletDb { conn, params, - address_tracker: Rc::new(Mutex::new(tracker)), + address_trackers: Rc::new(Mutex::new(HashMap::new())), }) }) } @@ -205,7 +204,7 @@ impl WalletDb { let mut wdb = WalletDb { conn: SqlTransaction(&tx), params: self.params.clone(), - address_tracker: self.address_tracker.clone(), + address_trackers: self.address_trackers.clone(), }; let result = f(&mut wdb)?; tx.commit()?; @@ -1268,7 +1267,7 @@ impl WalletWrite for WalletDb )?; } } - wdb.mark_addresses_as_mined(&output_addrs).map_err(SqliteClientError::from)?; + wdb.mark_addresses_as_mined(account_id, &output_addrs).map_err(SqliteClientError::from)?; } } @@ -1582,48 +1581,70 @@ where C: Borrow, P: consensus::Parameters, { + /// Backend-specific account identifier. + type AccountId = ::AccountId; + fn reserve_next_address( &self, - uivk: &UnifiedIncomingViewingKey, + account_id: Self::AccountId, ) -> Result { - let mut tracker = self - .address_tracker + let mut trackers = self + .address_trackers .lock() .map_err(|e| AddressTrackingError::Internal(format!("{:?}", e)))?; - tracker.reserve_next_address(self, uivk) + + match trackers.entry(account_id) { + Occupied(e) => e.into_mut(), + Vacant(e) => e.insert(AddressTracker::new(self, account_id)?), + }.reserve_next_address(self) } fn unreserve_addresses( &self, + account_id: Self::AccountId, addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError> { - let mut tracker = self - .address_tracker + let mut trackers = self + .address_trackers .lock() .map_err(|e| AddressTrackingError::Internal(format!("{:?}", e)))?; - tracker.unreserve_addresses(self, addresses) + + match trackers.entry(account_id) { + Occupied(e) => e.into_mut(), + Vacant(e) => e.insert(AddressTracker::new(self, account_id)?), + }.unreserve_addresses(self, addresses) } fn mark_addresses_as_used( &self, + account_id: Self::AccountId, addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError> { - let mut tracker = self - .address_tracker + let mut trackers = self + .address_trackers .lock() .map_err(|e| AddressTrackingError::Internal(format!("{:?}", e)))?; - tracker.mark_addresses_as_used(self, addresses) + + match trackers.entry(account_id) { + Occupied(e) => e.into_mut(), + Vacant(e) => e.insert(AddressTracker::new(self, account_id)?), + }.mark_addresses_as_used(self, addresses) } fn mark_addresses_as_mined( &self, + account_id: Self::AccountId, addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError> { - let mut tracker = self - .address_tracker + let mut trackers = self + .address_trackers .lock() .map_err(|e| AddressTrackingError::Internal(format!("{:?}", e)))?; - tracker.mark_addresses_as_mined(self, addresses) + + match trackers.entry(account_id) { + Occupied(e) => e.into_mut(), + Vacant(e) => e.insert(AddressTracker::new(self, account_id)?), + }.mark_addresses_as_mined(self, addresses) } } diff --git a/zcash_client_sqlite/src/wallet/address_tracker.rs b/zcash_client_sqlite/src/wallet/address_tracker.rs index 97d2e2e93e..44d404c674 100644 --- a/zcash_client_sqlite/src/wallet/address_tracker.rs +++ b/zcash_client_sqlite/src/wallet/address_tracker.rs @@ -1,65 +1,138 @@ //! Allocator for ephemeral transparent addresses. -use std::borrow::Borrow; +use std::{borrow::Borrow, cmp::max}; use rusqlite::Connection; -use zcash_client_backend::data_api::AddressTrackingError; +use zcash_client_backend::data_api::{AddressTrackingError, WalletRead}; use zcash_primitives::{consensus::Parameters, legacy::TransparentAddress}; use super::UnifiedIncomingViewingKey; -use crate::WalletDb; +use crate::{Account, AccountId, WalletDb}; + +// Consider making this larger from the start than in Bitcoin, say 100. +const GAP_LIMIT: u32 = 20; pub(crate) struct AddressTracker { + /// The UIVK of the account for which this `AddressTracker` keeps track of ephemeral t-addresses. + uivk: UnifiedIncomingViewingKey, + + /// Invariant: `gap_set` holds the ephemeral t-addresses for `account` at indices + /// `first_unmined_index..(first_unmined_index + GAP_LIMIT)`. gap_set: Vec, + + /// Invariant: `first_unused_index` is in `first_unmined_index..(first_unmined_index + GAP_LIMIT)`. first_unused_index: u32, + + /// Invariant: `first_unreserved_index` is in `first_unused_index..(first_unmined_index + GAP_LIMIT)`. + first_unreserved_index: u32, + + /// Invariant: `first_unmined_index` is in `0..=(u32::MAX - GAP_LIMIT)`. first_unmined_index: u32, } impl AddressTracker { - pub(crate) fn new, P: Parameters>(_conn: C, _params: &P) -> Self { - AddressTracker { - gap_set: vec![], - first_unused_index: 0, - first_unmined_index: 0, - } + pub(crate) fn new, P: Parameters>( + wallet: &WalletDb, + account_id: AccountId, + ) -> Result { + // TODO: read (first_unused_index, first_unmined_index) from database. + let first_unused_index: u32 = 0; // TODO + let first_unmined_index: u32 = 0; // TODO + // TODO: fill in gap_set. + let account = wallet.get_account(account_id).unwrap_or_default().ok_or(AddressTrackingError::AccountNotFound)?; + Ok(AddressTracker { + uivk: account.uivk(), + gap_set: vec![], // TODO + first_unused_index, + first_unreserved_index: first_unused_index, + first_unmined_index, + }) } pub(crate) fn reserve_next_address, P: Parameters>( &mut self, _wallet: &WalletDb, - _uivk: &UnifiedIncomingViewingKey, ) -> Result { - Err(AddressTrackingError::ReachedGapLimit) + if self.first_unreserved_index >= self.first_unmined_index + GAP_LIMIT - 1 { + return Err(AddressTrackingError::ReachedGapLimit); + } + let ephemeral_addr = + self.gap_set[(self.first_unreserved_index - self.first_unmined_index) as usize]; + self.first_unreserved_index += 1; + Ok(ephemeral_addr) } + /// We can assume the particular pattern of use in `create_proposed_transaction`, i.e. + /// the array will be in the same order the addresses were reserved. pub(crate) fn unreserve_addresses, P: Parameters>( &mut self, _wallet: &WalletDb, addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError> { - for _addr in addresses.iter().rev() { - todo!() + for &addr in addresses.iter().rev() { + if self.first_unreserved_index > self.first_unmined_index + && self.gap_set + [(self.first_unreserved_index - self.first_unmined_index - 1) as usize] + == addr + { + self.first_unreserved_index -= 1; + } } Ok(()) } + /// We can assume the particular pattern of use in `create_proposed_transaction`, i.e. + /// the array will be in the same order the addresses were reserved. pub(crate) fn mark_addresses_as_used, P: Parameters>( &mut self, _wallet: &WalletDb, - _addresses: &[TransparentAddress], + used_addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError> { + for &addr in used_addresses { + if self.first_unused_index < self.first_unmined_index + GAP_LIMIT + && self.gap_set[(self.first_unused_index - self.first_unmined_index) as usize] + == addr + { + self.first_unused_index += 1; + } + } Ok(()) } /// Checks the set of ephemeral transparent addresses within the gap limit for the /// given mined t-addresses, in order to update the first unmined ephemeral t-address /// index if necessary. + /// These addresses could be in any order. pub(crate) fn mark_addresses_as_mined, P: Parameters>( &mut self, _wallet: &WalletDb, - _addresses: &[TransparentAddress], + mined_addresses: &[TransparentAddress], ) -> Result<(), AddressTrackingError> { + // Find the position of the *last* element of `gap_set`, if any, that matches some element of `mined_addresses`. + // TODO: This is O(n²) and could be Õ(n) (e.g. by converting `mined_addresses` to a HashSet). + if let Some(pos) = self + .gap_set + .iter() + .enumerate() + .rev() + .find_map(|(pos, &gap_addr)| { + mined_addresses + .iter() + .find(|&&mined_addr| mined_addr == gap_addr) + .map(|_| pos) + }) + { + if self.first_unmined_index + (pos as u32) + 1 > u32::MAX - GAP_LIMIT { + return Err(AddressTrackingError::ReachedGapLimit); + } + self.first_unmined_index += (pos as u32) + 1; + self.first_unreserved_index = + max(self.first_unreserved_index, self.first_unmined_index); + self.first_unused_index = max(self.first_unused_index, self.first_unmined_index); + self.gap_set.copy_within(pos..(GAP_LIMIT as usize), 0); + // TODO: re-fill the rest of `gap_set`. + } Ok(()) } }