From f9fc8bbc74cf70d4314fb732c43b0a3fcf40534d Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 25 Feb 2025 11:22:21 -0700 Subject: [PATCH] zcash_client_backend: Update address rotation to take gap limits into account. --- Cargo.lock | 1 + Cargo.toml | 1 + zcash_client_backend/CHANGELOG.md | 22 +- zcash_client_backend/src/data_api.rs | 19 +- zcash_client_backend/src/data_api/testing.rs | 5 +- .../src/data_api/testing/pool.rs | 2 +- .../src/data_api/testing/transparent.rs | 6 +- zcash_client_sqlite/CHANGELOG.md | 1 + zcash_client_sqlite/Cargo.toml | 1 + zcash_client_sqlite/src/error.rs | 14 + zcash_client_sqlite/src/lib.rs | 70 ++--- zcash_client_sqlite/src/util.rs | 2 +- zcash_client_sqlite/src/wallet.rs | 244 ++++++++++++++++-- zcash_client_sqlite/src/wallet/encoding.rs | 145 +++++++++++ zcash_client_sqlite/src/wallet/init.rs | 18 +- .../transparent_gap_limit_handling.rs | 115 ++++++--- zcash_client_sqlite/src/wallet/orchard.rs | 1 + zcash_client_sqlite/src/wallet/sapling.rs | 1 + zcash_client_sqlite/src/wallet/transparent.rs | 61 ++++- zcash_keys/CHANGELOG.md | 3 + zcash_keys/src/keys.rs | 15 ++ 21 files changed, 608 insertions(+), 139 deletions(-) create mode 100644 zcash_client_sqlite/src/wallet/encoding.rs diff --git a/Cargo.lock b/Cargo.lock index adb1ae46fd..9ab3b11828 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6224,6 +6224,7 @@ dependencies = [ "ambassador", "assert_matches", "bip32", + "bitflags 2.6.0", "bls12_381", "bs58", "byteorder", diff --git a/Cargo.toml b/Cargo.toml index a4fa020953..73e091c6e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ document-features = "0.2" # Encodings base64 = "0.22" bech32 = { version = "0.11", default-features = false, features = ["alloc"] } +bitflags = "2" bs58 = { version = "0.5", default-features = false, features = ["alloc", "check"] } byteorder = "1" hex = { version = "0.4", default-features = false, features = ["alloc"] } diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index da5c9b8ca4..62442afe4f 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -14,15 +14,19 @@ and this library adheres to Rust's notion of ### Changed - `zcash_client_backend::data_api::WalletRead`: - - `get_transparent_receivers` now takes additional `include_change` and - `include_ephemeral` arguments. - - `get_known_ephemeral_addresses` now takes a - `Range` as its argument - instead of a `Range` - - Has added method `utxo_query_height` when the `transparent-inputs` feature - flag is active. -- `zcash_client_backend::data_api::WalletWrite` has an added method - `get_address_for_index` + - `get_transparent_receivers` now takes additional `include_change` and + `include_ephemeral` arguments. + - `get_known_ephemeral_addresses` now takes a + `Range` as its argument + instead of a `Range` + - Has added method `utxo_query_height` when the `transparent-inputs` feature + flag is active. + - has removed method `get_current_address`. It has been replaced by + added method `WalletWrite::get_last_generated_address` +- `zcash_client_backend::data_api::WalletWrite`: + - has added method `get_address_for_index` + - `get_next_available_address` now returns the diversifier index at which the + address was generated in addition to the address. ### Removed - `zcash_client_backend::data_api::GAP_LIMIT` gap limits are now configured diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 702492064e..3699884f58 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1225,14 +1225,19 @@ pub trait WalletRead { ufvk: &UnifiedFullViewingKey, ) -> Result, Self::Error>; - /// Returns the most recently generated unified address for the specified account, if the - /// account identifier specified refers to a valid account for this wallet. + /// Returns the most recently generated unified address for the specified account that conforms + /// to the specified address request, if the account identifier specified refers to a valid + /// account for this wallet. + /// + /// If the `request` parameter is `None`, this will be interpreted as a request for an address + /// having a receiver corresponding to each data item in the account's UIVK. /// /// This will return `Ok(None)` if the account identifier does not correspond to a known - /// account. - fn get_current_address( + /// account, or if no previously generated address conforms to the specified request. + fn get_last_generated_address( &self, account: Self::AccountId, + request: Option, ) -> Result, Self::Error>; /// Returns the birthday height for the given account, or an error if the account is not known @@ -2380,7 +2385,7 @@ pub trait WalletWrite: WalletRead { &mut self, account: Self::AccountId, request: Option, - ) -> Result, Self::Error>; + ) -> Result, Self::Error>; /// Generates, persists, and marks as exposed a diversified address for the specified account /// at the provided diversifier index. If the `request` parameter is `None`, an address should @@ -2392,7 +2397,9 @@ pub trait WalletWrite: WalletRead { /// [`ReceiverRequirement::Require`]'ed receiver. /// /// Address generation should fail if a transparent receiver would be generated that violates - /// the backend's internally configured gap limit for HD-seed-based recovery. + /// the backend's internally configured gap limit for HD-seed-based recovery, or if an address + /// has already been exposed for the given diversifier index and the given request produced an + /// address having different receivers than what was originally exposed. /// /// [`ReceiverRequirement::Require`]: zcash_keys::keys::ReceiverRequirement::Require fn get_address_for_index( diff --git a/zcash_client_backend/src/data_api/testing.rs b/zcash_client_backend/src/data_api/testing.rs index 2a036f2210..7108547d31 100644 --- a/zcash_client_backend/src/data_api/testing.rs +++ b/zcash_client_backend/src/data_api/testing.rs @@ -2523,9 +2523,10 @@ impl WalletRead for MockWalletDb { Ok(None) } - fn get_current_address( + fn get_last_generated_address( &self, _account: Self::AccountId, + _request: Option, ) -> Result, Self::Error> { Ok(None) } @@ -2709,7 +2710,7 @@ impl WalletWrite for MockWalletDb { &mut self, _account: Self::AccountId, _request: Option, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { Ok(None) } diff --git a/zcash_client_backend/src/data_api/testing/pool.rs b/zcash_client_backend/src/data_api/testing/pool.rs index 26c600bdcb..95e08582da 100644 --- a/zcash_client_backend/src/data_api/testing/pool.rs +++ b/zcash_client_backend/src/data_api/testing/pool.rs @@ -1766,7 +1766,7 @@ where let uaddr = st .wallet() - .get_current_address(account.id()) + .get_last_generated_address(account.id(), None) .unwrap() .unwrap(); let taddr = uaddr.transparent().unwrap(); diff --git a/zcash_client_backend/src/data_api/testing/transparent.rs b/zcash_client_backend/src/data_api/testing/transparent.rs index b72486f531..5d79401b21 100644 --- a/zcash_client_backend/src/data_api/testing/transparent.rs +++ b/zcash_client_backend/src/data_api/testing/transparent.rs @@ -102,7 +102,7 @@ where let account_id = st.test_account().unwrap().id(); let uaddr = st .wallet() - .get_current_address(account_id) + .get_last_generated_address(account_id, None) .unwrap() .unwrap(); let taddr = uaddr.transparent().unwrap(); @@ -193,7 +193,7 @@ where let account = st.test_account().cloned().unwrap(); let uaddr = st .wallet() - .get_current_address(account.id()) + .get_last_generated_address(account.id(), None) .unwrap() .unwrap(); let taddr = uaddr.transparent().unwrap(); @@ -301,7 +301,7 @@ where let account = st.test_account().cloned().unwrap(); let uaddr = st .wallet() - .get_current_address(account.id()) + .get_last_generated_address(account.id(), None) .unwrap() .unwrap(); let taddr = uaddr.transparent().unwrap(); diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 63e23a947b..174076e3ad 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -27,6 +27,7 @@ and this library adheres to Rust's notion of for the account that reached the limit in its payload. In addition to the transparent address index, it also contains the key scope involved when the error was encountered. + - A new `DiversifierIndexReuse` variant has been added. - Each row returned from the `v_received_outputs` view now exposes an internal identifier for the address that received that output. This should be ignored by external consumers of this view. diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index c1721a4392..cbc68494fc 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -45,6 +45,7 @@ bs58.workspace = true tracing.workspace = true # - Serialization +bitflags.workspace = true byteorder.workspace = true nonempty.workspace = true prost.workspace = true diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index e2a987c683..f0da1bfb04 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -8,8 +8,10 @@ use shardtree::error::ShardTreeError; use zcash_address::ParseError; use zcash_client_backend::data_api::NoteFilter; +use zcash_keys::address::UnifiedAddress; use zcash_keys::keys::AddressGenerationError; use zcash_protocol::{consensus::BlockHeight, value::BalanceError, PoolType, TxId}; +use zip32::DiversifierIndex; use crate::{wallet::commitment_tree, AccountUuid}; @@ -126,6 +128,11 @@ pub enum SqliteClientError { #[cfg(feature = "transparent-inputs")] ReachedGapLimit(KeyScope, u32), + /// The backend encountered an attempt to reuse a diversifier index to generate an address + /// having different receivers from an address that had previously been exposed for that + /// diversifier index. Returns the previously exposed address. + DiversifierIndexReuse(DiversifierIndex, Box), + /// The wallet attempted to create a transaction that would use of one of the wallet's /// previously-used addresses, potentially creating a problem with on-chain transaction /// linkability. The returned value contains the string encoding of the address and the txid(s) @@ -196,6 +203,13 @@ impl fmt::Display for SqliteClientError { KeyScope::Ephemeral => "ephemeral transparent", } ), + SqliteClientError::DiversifierIndexReuse(i, _) => { + write!( + f, + "An address has already been exposed for diversifier index {}", + u128::from(*i) + ) + } SqliteClientError::AddressReuse(address_str, txids) => { write!(f, "The address {address_str} previously used in txid(s) {:?} would be reused.", txids) } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 20b93ba33a..6adabc7f2f 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -67,10 +67,7 @@ use zcash_client_backend::{ }; use zcash_keys::{ address::UnifiedAddress, - keys::{ - AddressGenerationError, ReceiverRequirement, UnifiedAddressRequest, UnifiedFullViewingKey, - UnifiedSpendingKey, - }, + keys::{ReceiverRequirement, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey}, }; use zcash_primitives::{ block::BlockHash, @@ -86,7 +83,6 @@ use zip32::{fingerprint::SeedFingerprint, DiversifierIndex}; use crate::{error::SqliteClientError, wallet::commitment_tree::SqliteShardStore}; use wallet::{ - chain_tip_height, commitment_tree::{self, put_shard_roots}, common::spendable_notes_meta, upsert_address, SubtreeProgressEstimator, @@ -670,11 +666,12 @@ impl, P: consensus::Parameters, CL> WalletRead wallet::get_account_for_ufvk(self.conn.borrow(), &self.params, ufvk) } - fn get_current_address( + fn get_last_generated_address( &self, account: Self::AccountId, + request: Option, ) -> Result, Self::Error> { - wallet::get_current_address(self.conn.borrow(), &self.params, account) + wallet::get_last_generated_address(self.conn.borrow(), &self.params, account, request) .map(|res| res.map(|(addr, _)| addr)) } @@ -1129,39 +1126,18 @@ impl, P: consensus::Parameters, CL: Clock> Wa &mut self, account_uuid: Self::AccountId, request: Option, - ) -> Result, Self::Error> { - self.transactionally( - |wdb| match wdb.get_unified_full_viewing_keys()?.get(&account_uuid) { - Some(ufvk) => { - let search_from = - match wallet::get_current_address(wdb.conn.0, &wdb.params, account_uuid)? { - Some((_, mut last_diversifier_index)) => { - last_diversifier_index.increment().map_err(|_| { - AddressGenerationError::DiversifierSpaceExhausted - })?; - last_diversifier_index - } - None => DiversifierIndex::default(), - }; - - let (addr, diversifier_index) = ufvk.find_address(search_from, request)?; - let account_id = wallet::get_account_ref(wdb.conn.0, account_uuid)?; - let chain_tip_height = chain_tip_height(wdb.conn.0)? - .ok_or(SqliteClientError::ChainHeightUnknown)?; - wallet::upsert_address( - wdb.conn.0, - &wdb.params, - account_id, - diversifier_index, - &addr, - Some(chain_tip_height), - )?; - - Ok(Some(addr)) - } - None => Ok(None), - }, - ) + ) -> Result, Self::Error> { + self.transactionally(|wdb| { + wallet::get_next_available_address( + wdb.conn.0, + &wdb.params, + &wdb.clock, // TODO: make an abstract Clock a field of `WalletDb` + account_uuid, + request, + #[cfg(feature = "transparent-inputs")] + &wdb.gap_limits, + ) + }) } fn get_address_for_index( @@ -1180,6 +1156,7 @@ impl, P: consensus::Parameters, CL: Clock> Wa diversifier_index, &address, Some(chain_tip_height.unwrap_or(account.birthday())), + true, )?; Ok(Some(address)) @@ -2283,17 +2260,24 @@ mod tests { .update_chain_tip(account.birthday().height()) .unwrap(); - let current_addr = st.wallet().get_current_address(account.id()).unwrap(); + let current_addr = st + .wallet() + .get_last_generated_address(account.id(), None) + .unwrap(); assert!(current_addr.is_some()); let addr2 = st .wallet_mut() .get_next_available_address(account.id(), None) - .unwrap(); + .unwrap() + .map(|(a, _)| a); assert!(addr2.is_some()); assert_ne!(current_addr, addr2); - let addr2_cur = st.wallet().get_current_address(account.id()).unwrap(); + let addr2_cur = st + .wallet() + .get_last_generated_address(account.id(), None) + .unwrap(); assert_eq!(addr2, addr2_cur); } diff --git a/zcash_client_sqlite/src/util.rs b/zcash_client_sqlite/src/util.rs index eb1701d374..2d31c17396 100644 --- a/zcash_client_sqlite/src/util.rs +++ b/zcash_client_sqlite/src/util.rs @@ -51,7 +51,7 @@ pub mod testing { /// that instant. pub fn tick(&self, delta: Duration) { let mut w = self.now.write().unwrap(); - *w = *w + delta; + *w += delta; } } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index e9b28feef6..5680f7c93f 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -70,8 +70,10 @@ use std::{ io::{self, Cursor}, num::NonZeroU32, ops::RangeInclusive, + time::SystemTime, }; +use encoding::ReceiverFlags; use incrementalmerkletree::{Marking, Retention}; use rusqlite::{self, named_params, params, Connection, OptionalExtension}; use secrecy::{ExposeSecret, SecretVec}; @@ -95,7 +97,7 @@ use zcash_keys::{ address::{Address, Receiver, UnifiedAddress}, encoding::AddressCodec, keys::{ - AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, + AddressGenerationError, ReceiverRequirement, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedIncomingViewingKey, UnifiedSpendingKey, }, }; @@ -105,7 +107,7 @@ use zcash_primitives::{ transaction::{Transaction, TransactionData}, }; use zcash_protocol::{ - consensus::{self, BlockHeight, BranchId, NetworkConstants as _, NetworkUpgrade, Parameters}, + consensus::{self, BlockHeight, BranchId, NetworkUpgrade, Parameters}, memo::{Memo, MemoBytes}, value::{ZatBalance, Zatoshis}, PoolType, ShieldedProtocol, TxId, @@ -115,6 +117,7 @@ use zip32::{fingerprint::SeedFingerprint, DiversifierIndex}; use self::scanning::{parse_priority_code, priority_code, replace_queue_entries}; use crate::{ error::SqliteClientError, + util::Clock, wallet::commitment_tree::{get_max_checkpointed_height, SqliteShardStore}, AccountRef, AccountUuid, AddressRef, SqlTransaction, TransferType, TxRef, WalletCommitmentTrees, WalletDb, PRUNING_DEPTH, SAPLING_TABLES_PREFIX, VERIFY_LOOKAHEAD, @@ -136,6 +139,7 @@ use {crate::ORCHARD_TABLES_PREFIX, zcash_client_backend::data_api::ORCHARD_SHARD pub mod commitment_tree; pub(crate) mod common; mod db; +pub(crate) mod encoding; pub mod init; #[cfg(feature = "orchard")] pub(crate) mod orchard; @@ -146,6 +150,15 @@ pub(crate) mod transparent; pub(crate) const BLOCK_SAPLING_FRONTIER_ABSENT: &[u8] = &[0x0]; +/// A constant for use in converting Unix timestamps to shielded-only diversifier indices. The +/// value here is intended to be added to the current time, in seconds since the epoch, to obtain +/// an index that is greater than or equal to 2^31. While it would be possible to use indices in +/// the range 2^31..2^32, we wish to avoid any confusion with indices in the non-hardened BIP 32 +/// child index derivation space. +/// +/// 2^32 - (date --date "Oct 28, 2016 07:56 UTC" +%s) +const MIN_SHIELDED_DIVERSIFIER_OFFSET: u64 = 2817325936; + fn parse_account_source( account_kind: u32, hd_seed_fingerprint: Option<[u8; 32]>, @@ -704,6 +717,7 @@ pub(crate) fn add_account( d_idx, &address, Some(birthday.height()), + false, )?; // Pre-generate transparent addresses up to the gap limits for the external, internal, @@ -716,29 +730,146 @@ pub(crate) fn add_account( Ok(account) } -pub(crate) fn get_current_address( +pub(crate) fn get_next_available_address( + conn: &rusqlite::Transaction, + params: &P, + clock: &C, + account_uuid: AccountUuid, + request: Option, + #[cfg(feature = "transparent-inputs")] gap_limits: &GapLimits, +) -> Result, SqliteClientError> { + let account: Account = match get_account(conn, params, account_uuid)? { + Some(account) => account, + None => { + return Ok(None); + } + }; + + let request = request.unwrap_or_else(|| { + account + .uivk() + .to_address_request() + .expect("uivk can produce a valid address") + }); + + let (addr, diversifier_index) = if request.p2pkh() == ReceiverRequirement::Require { + #[cfg(not(feature = "transparent-inputs"))] + { + return Err(SqliteClientError::AddressGeneration( + AddressGenerationError::ReceiverTypeNotSupported(Typecode::P2pkh), + )); + } + + // If a p2pkh receiver is required, return the first un-exposed address from within the + // transparent gap limit. + #[cfg(feature = "transparent-inputs")] + { + // First, ensure that we have pre-generated as many addresses as we can. + transparent::generate_gap_addresses( + conn, + params, + account.internal_id(), + KeyScope::EXTERNAL, + gap_limits, + None, + )?; + + // Select indicies from the transparent gap limit that are available for use as + // diversifier indices. + let (gap_start, addrs) = transparent::select_addrs_to_reserve( + conn, + params, + account.internal_id(), + KeyScope::EXTERNAL, + gap_limits.external(), + gap_limits + .external() + .try_into() + .expect("gap limit fits in usize"), + )?; + + // Find the first index that generates an address conforming to the request. + addrs + .iter() + .find_map(|(_, _, meta)| { + let j = DiversifierIndex::from(meta.address_index()); + account + .uivk() + .address(j, Some(request)) + .ok() + .map(|ua| (ua, j)) + }) + .ok_or(SqliteClientError::ReachedGapLimit( + KeyScope::EXTERNAL, + gap_start.index() + gap_limits.external(), + ))? + } + } else { + let j = DiversifierIndex::from( + clock + .now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("system time is valid") + .as_secs() + .saturating_add(MIN_SHIELDED_DIVERSIFIER_OFFSET), + ); + + account.uivk().find_address(j, Some(request))? + }; + + let chain_tip_height = chain_tip_height(conn)?.ok_or(SqliteClientError::ChainHeightUnknown)?; + upsert_address( + conn, + params, + account.internal_id(), + diversifier_index, + &addr, + Some(chain_tip_height), + false, + )?; + + Ok(Some((addr, diversifier_index))) +} + +pub(crate) fn get_last_generated_address( conn: &rusqlite::Connection, params: &P, account_uuid: AccountUuid, + request: Option, ) -> Result, SqliteClientError> { - let ua_prefix = params.network_type().hrp_unified_address(); - // This returns the most recently generated address. + let account: Account = match get_account(conn, params, account_uuid)? { + Some(account) => account, + None => { + return Ok(None); + } + }; + let request = request + .map_or_else(|| account.uivk().to_address_request(), Ok) + .map_err(|_| { + SqliteClientError::BadAccountData( + "Could not generate UnifiedAddressRequest for UIVK".to_string(), + ) + })?; + let require_flags = ReceiverFlags::required(request); + let omit_flags = ReceiverFlags::omitted(request); + // This returns the most recently exposed external-scope address (the request that was exposed + // at the greatest block height) that conforms to the specified request. let addr: Option<(String, Vec)> = conn .query_row( - &format!( - "SELECT address, diversifier_index_be - FROM addresses - JOIN accounts ON addresses.account_id = accounts.id - WHERE accounts.uuid = :account_uuid - AND key_scope = :key_scope - AND address LIKE '{ua_prefix}%' - AND exposed_at_height IS NOT NULL - ORDER BY diversifier_index_be DESC - LIMIT 1" - ), + "SELECT address, diversifier_index_be + FROM addresses + WHERE account_id = :account_id + AND key_scope = :key_scope + AND (receiver_flags & :require_flags) = :require_flags + AND (receiver_flags & :omit_flags) = 0 + AND exposed_at_height IS NOT NULL + ORDER BY exposed_at_height DESC + LIMIT 1", named_params![ - ":account_uuid": account_uuid.0, - ":key_scope": KeyScope::EXTERNAL.encode() + ":account_id": account.internal_id().0, + ":key_scope": KeyScope::EXTERNAL.encode(), + ":require_flags": require_flags.bits(), + ":omit_flags": omit_flags.bits(), ], |row| Ok((row.get(0)?, row.get(1)?)), ) @@ -765,6 +896,17 @@ pub(crate) fn get_current_address( /// Adds the given external address and diversifier index to the addresses table. /// /// Returns the primary key identifier for the newly-inserted address. +/// +/// ## Parameters +/// - `account_id`: The account that the address was generated for. +/// - `diversifier_index`: The diversifier index used to generate the address. +/// - `address`: The unified address itself. +/// - `exposed_at_height`: The block height at the earliest time that the address may have been +/// exposed to a user, assuming a single generator of addresses. +/// - `force_update_address`: If this argument is set to `true`, an address has already been +/// inserted for the given account an diversifier index, and the `exposed_at_height` column +/// is currently `NULL` (i.e. the address at this diversifier index has not yet been exposed) +/// then the value of the `address` column will be replaced with the provided address. pub(crate) fn upsert_address( conn: &rusqlite::Connection, params: &P, @@ -772,7 +914,50 @@ pub(crate) fn upsert_address( diversifier_index: DiversifierIndex, address: &UnifiedAddress, exposed_at_height: Option, + force_update_address: bool, ) -> Result { + // the diversifier index is stored in big-endian order to allow sorting + let di_be = encode_diversifier_index_be(diversifier_index); + + // If a force-update was requested, check whether an address has previously been exposed for + // this diversifier index. If so, and if that address differs from the given address, return an + // error. + if force_update_address { + let previously_exposed_as = conn + .query_row( + "SELECT address, exposed_at_height + FROM addresses + WHERE account_id = :account_id + AND diversifier_index_be = :diversifier_index_be + AND key_scope = :key_scope", + named_params![ + ":account_id": account_id.0, + ":diversifier_index_be": di_be, + ":key_scope": KeyScope::EXTERNAL.encode(), + ], + |row| { + let address = row.get::<_, String>("address")?; + let exposed_at = row.get::<_, Option>("exposed_at_height")?; + Ok(exposed_at.map(|_| address)) + }, + ) + .optional()? + .flatten() + .map(|addr_str| UnifiedAddress::decode(params, &addr_str)) + .transpose() + .map_err(SqliteClientError::CorruptedData)?; + + match previously_exposed_as { + Some(addr) if &addr != address => { + return Err(SqliteClientError::DiversifierIndexReuse( + diversifier_index, + Box::new(addr), + )); + } + _ => (), + } + } + let mut stmt = conn.prepare_cached( "INSERT INTO addresses ( account_id, @@ -794,10 +979,15 @@ pub(crate) fn upsert_address( ) ON CONFLICT (account_id, diversifier_index_be, key_scope) DO UPDATE SET exposed_at_height = COALESCE( - MIN(exposed_at_height, :exposed_at_height), - exposed_at_height, - :exposed_at_height - ) + MIN(exposed_at_height, :exposed_at_height), + exposed_at_height, + :exposed_at_height + ), + address = IIF( + exposed_at_height IS NULL AND :force_update_address, + :address, + address + ) RETURNING id", )?; @@ -812,7 +1002,7 @@ pub(crate) fn upsert_address( named_params![ ":account_id": account_id.0, // the diversifier index is stored in big-endian order to allow sorting - ":diversifier_index_be": encode_diversifier_index_be(diversifier_index), + ":diversifier_index_be": &di_be, ":key_scope": KeyScope::EXTERNAL.encode(), ":address": &address.encode(params), ":transparent_child_index": transparent_child_index, @@ -4009,11 +4199,15 @@ mod tests { ); // The default address is set for the test account - assert_matches!(st.wallet().get_current_address(account.id()), Ok(Some(_))); + assert_matches!( + st.wallet().get_last_generated_address(account.id(), None), + Ok(Some(_)) + ); // No default address is set for an un-initialized account assert_matches!( - st.wallet().get_current_address(AccountUuid(Uuid::nil())), + st.wallet() + .get_last_generated_address(AccountUuid(Uuid::nil()), None), Ok(None) ); } diff --git a/zcash_client_sqlite/src/wallet/encoding.rs b/zcash_client_sqlite/src/wallet/encoding.rs new file mode 100644 index 0000000000..db4fa6e00a --- /dev/null +++ b/zcash_client_sqlite/src/wallet/encoding.rs @@ -0,0 +1,145 @@ +//! Functions and types related to encoding and decoding wallet data for storage in the wallet +//! database. + +use bitflags::bitflags; +use transparent::address::TransparentAddress::*; +use zcash_address::{ + unified::{Container, Receiver}, + ConversionError, TryFromAddress, +}; +use zcash_keys::{ + address::Address, + keys::{ReceiverRequirement, UnifiedAddressRequest}, +}; +use zcash_protocol::consensus::NetworkType; + +bitflags! { + /// A set of flags describing the type(s) of outputs that a Zcash address can receive. + #[derive(Clone, Copy, Debug, PartialEq, Eq)] + pub(crate) struct ReceiverFlags: i64 { + /// The address did not contain any recognized receiver types. + const UNKNOWN = 0b00000000; + /// The associated address can receive transparent p2pkh outputs. + const P2PKH = 0b00000001; + /// The associated address can receive transparent p2sh outputs. + const P2SH = 0b00000010; + /// The associated address can receive Sapling outputs. + const SAPLING = 0b00000100; + /// The associated address can receive Orchard outputs. + const ORCHARD = 0b00001000; + } +} + +impl ReceiverFlags { + pub(crate) fn required(request: UnifiedAddressRequest) -> Self { + let mut flags = ReceiverFlags::UNKNOWN; + if matches!(request.orchard(), ReceiverRequirement::Require) { + flags |= ReceiverFlags::ORCHARD; + } + if matches!(request.sapling(), ReceiverRequirement::Require) { + flags |= ReceiverFlags::SAPLING; + } + if matches!(request.p2pkh(), ReceiverRequirement::Require) { + flags |= ReceiverFlags::P2PKH; + } + flags + } + + pub(crate) fn omitted(request: UnifiedAddressRequest) -> Self { + let mut flags = ReceiverFlags::UNKNOWN; + if matches!(request.orchard(), ReceiverRequirement::Omit) { + flags |= ReceiverFlags::ORCHARD; + } + if matches!(request.sapling(), ReceiverRequirement::Omit) { + flags |= ReceiverFlags::SAPLING; + } + if matches!(request.p2pkh(), ReceiverRequirement::Omit) { + flags |= ReceiverFlags::P2PKH; + } + flags + } +} + +/// Computes the [`ReceiverFlags`] describing the types of outputs that the provided +/// [`Address`] can receive. +impl From
for ReceiverFlags { + fn from(address: Address) -> Self { + match address { + Address::Sapling(_) => ReceiverFlags::SAPLING, + Address::Transparent(addr) => match addr { + PublicKeyHash(_) => ReceiverFlags::P2PKH, + ScriptHash(_) => ReceiverFlags::P2SH, + }, + Address::Unified(ua) => { + let mut flags = ReceiverFlags::UNKNOWN; + match ua.transparent() { + Some(PublicKeyHash(_)) => { + flags |= ReceiverFlags::P2PKH; + } + Some(ScriptHash(_)) => { + flags |= ReceiverFlags::P2SH; + } + _ => {} + } + if ua.has_sapling() { + flags |= ReceiverFlags::SAPLING; + } + if ua.has_orchard() { + flags |= ReceiverFlags::ORCHARD; + } + flags + } + Address::Tex(_) => ReceiverFlags::P2PKH, + } + } +} + +impl TryFromAddress for ReceiverFlags { + type Error = (); + + fn try_from_sapling( + _net: NetworkType, + _data: [u8; 43], + ) -> Result> { + Ok(ReceiverFlags::SAPLING) + } + + fn try_from_unified( + _net: NetworkType, + data: zcash_address::unified::Address, + ) -> Result> { + let mut result = ReceiverFlags::UNKNOWN; + for i in data.items() { + match i { + Receiver::Orchard(_) => result |= ReceiverFlags::ORCHARD, + Receiver::Sapling(_) => result |= ReceiverFlags::SAPLING, + Receiver::P2pkh(_) => result |= ReceiverFlags::P2PKH, + Receiver::P2sh(_) => result |= ReceiverFlags::P2SH, + Receiver::Unknown { .. } => {} + } + } + + Ok(result) + } + + fn try_from_transparent_p2pkh( + _net: NetworkType, + _data: [u8; 20], + ) -> Result> { + Ok(ReceiverFlags::P2PKH) + } + + fn try_from_transparent_p2sh( + _net: NetworkType, + _data: [u8; 20], + ) -> Result> { + Ok(ReceiverFlags::P2SH) + } + + fn try_from_tex( + _net: NetworkType, + _data: [u8; 20], + ) -> Result> { + Ok(ReceiverFlags::P2PKH) + } +} diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 46bdd388d6..96a4d448fb 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -224,6 +224,12 @@ fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> Wallet SqliteClientError::ReachedGapLimit(..) => { unreachable!("we don't do ephemeral address tracking") } + SqliteClientError::DiversifierIndexReuse(i, _) => { + WalletMigrationError::CorruptedData(format!( + "invalid attempt to overwrite address at diversifier index {}", + u128::from(i) + )) + } SqliteClientError::AddressReuse(_, _) => { unreachable!("we don't create transactions in migrations") } @@ -1129,10 +1135,14 @@ mod tests { if let Some(Address::Unified(tvua)) = Address::decode(&Network::MainNetwork, tv.unified_addr) { - let (ua, di) = - wallet::get_current_address(&db_data.conn, &db_data.params, account_id) - .unwrap() - .expect("create_account generated the first address"); + let (ua, di) = wallet::get_last_generated_address( + &db_data.conn, + &db_data.params, + account_id, + None, + ) + .unwrap() + .expect("create_account generated the first address"); assert_eq!(DiversifierIndex::from(tv.diversifier_index), di); assert_eq!(tvua.transparent(), ua.transparent()); assert_eq!(tvua.sapling(), ua.sapling()); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/transparent_gap_limit_handling.rs b/zcash_client_sqlite/src/wallet/init/migrations/transparent_gap_limit_handling.rs index 75018ce171..113e672556 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/transparent_gap_limit_handling.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/transparent_gap_limit_handling.rs @@ -7,12 +7,13 @@ use uuid::Uuid; use rusqlite::{named_params, Transaction}; use schemerz_rusqlite::RusqliteMigration; +use zcash_address::ZcashAddress; use zcash_keys::keys::UnifiedIncomingViewingKey; use zcash_protocol::consensus::{self, BlockHeight}; use super::add_account_uuids; use crate::{ - wallet::{self, init::WalletMigrationError, KeyScope}, + wallet::{self, encoding::ReceiverFlags, init::WalletMigrationError, KeyScope}, AccountRef, }; @@ -73,13 +74,12 @@ impl RusqliteMigration for Migration

{ ALTER TABLE addresses ADD COLUMN key_scope INTEGER NOT NULL DEFAULT {external_scope_code}; ALTER TABLE addresses ADD COLUMN transparent_child_index INTEGER; ALTER TABLE addresses ADD COLUMN exposed_at_height INTEGER; + ALTER TABLE addresses ADD COLUMN receiver_flags INTEGER; "# ))?; - #[cfg(feature = "transparent-inputs")] let mut account_ids = HashSet::new(); - #[cfg(feature = "transparent-inputs")] { // If the diversifier index is in the valid range of non-hardened child indices, set // `transparent_child_index` so that we can use it for gap limit handling. @@ -87,7 +87,12 @@ impl RusqliteMigration for Migration

{ // diversifier_index_be) constraint. let mut di_query = conn.prepare( r#" - SELECT account_id, accounts.uivk AS uivk, diversifier_index_be, accounts.birthday_height + SELECT + account_id, + address, + accounts.uivk AS uivk, + diversifier_index_be, + accounts.birthday_height FROM addresses JOIN accounts ON accounts.id = account_id "#, @@ -97,44 +102,84 @@ impl RusqliteMigration for Migration

{ let account_id: i64 = row.get("account_id")?; account_ids.insert(account_id); - let uivk = decode_uivk(row.get("uivk")?)?; + let addr_str: String = row.get("address")?; + let address = ZcashAddress::try_from_encoded(&addr_str).map_err(|e| { + WalletMigrationError::CorruptedData(format!( + "Encoded address {} is not a valid zcash address: {}", + addr_str, e + )) + })?; + let receiver_flags = address.convert::().map_err(|_| { + WalletMigrationError::CorruptedData("Unexpected address type".to_string()) + })?; let di_be: Vec = row.get("diversifier_index_be")?; - let diversifier_index = decode_diversifier_index_be(&di_be)?; let account_birthday: i64 = row.get("birthday_height")?; - let transparent_external = NonHardenedChildIndex::try_from(diversifier_index) - .ok() - .and_then(|idx| { - uivk.transparent() - .as_ref() - .and_then(|external_ivk| external_ivk.derive_address(idx).ok()) - .map(|t_addr| (idx, t_addr)) - }); - - // Add transparent address index metadata and the transparent address corresponding - // to the index to the addresses table. We unconditionally set the cached - // transparent receiver address in order to simplify gap limit handling; even if a - // unified address is generated without a transparent receiver, we still assume - // that a transparent-only wallet for which we have imported the seed may have - // generated an address at that index. - if let Some((idx, t_addr)) = transparent_external { + let update_without_taddr = || { conn.execute( r#" UPDATE addresses - SET transparent_child_index = :transparent_child_index, - cached_transparent_receiver_address = :t_addr, - exposed_at_height = :account_birthday + SET exposed_at_height = :account_birthday, + receiver_flags = :receiver_flags WHERE account_id = :account_id AND diversifier_index_be = :diversifier_index_be "#, named_params! { ":account_id": account_id, ":diversifier_index_be": &di_be[..], - ":transparent_child_index": idx.index(), - ":t_addr": t_addr.encode(&self.params), ":account_birthday": account_birthday, + ":receiver_flags": receiver_flags.bits() }, - )?; + ) + }; + + #[cfg(feature = "transparent-inputs")] + { + let uivk = decode_uivk(row.get("uivk")?)?; + let diversifier_index = decode_diversifier_index_be(&di_be)?; + let transparent_external = NonHardenedChildIndex::try_from(diversifier_index) + .ok() + .and_then(|idx| { + uivk.transparent() + .as_ref() + .and_then(|external_ivk| external_ivk.derive_address(idx).ok()) + .map(|t_addr| (idx, t_addr.encode(&self.params))) + }); + + // Add transparent address index metadata and the transparent address corresponding + // to the index to the addresses table. We unconditionally set the cached + // transparent receiver address in order to simplify gap limit handling; even if a + // unified address is generated without a transparent receiver, we still assume + // that a transparent-only wallet for which we have imported the seed may have + // generated an address at that index. + if let Some((idx, t_addr)) = transparent_external { + conn.execute( + r#" + UPDATE addresses + SET transparent_child_index = :transparent_child_index, + cached_transparent_receiver_address = :t_addr, + exposed_at_height = :account_birthday, + receiver_flags = :receiver_flags + WHERE account_id = :account_id + AND diversifier_index_be = :diversifier_index_be + "#, + named_params! { + ":account_id": account_id, + ":diversifier_index_be": &di_be[..], + ":transparent_child_index": idx.index(), + ":t_addr": t_addr, + ":account_birthday": account_birthday, + ":receiver_flags": receiver_flags.bits() + }, + )?; + } else { + update_without_taddr()?; + } + } + + #[cfg(not(feature = "transparent-inputs"))] + { + update_without_taddr()?; } } } @@ -155,6 +200,7 @@ impl RusqliteMigration for Migration

{ transparent_child_index INTEGER, cached_transparent_receiver_address TEXT, exposed_at_height INTEGER, + receiver_flags INTEGER NOT NULL, FOREIGN KEY (account_id) REFERENCES accounts(id), CONSTRAINT diversification UNIQUE (account_id, key_scope, diversifier_index_be), CONSTRAINT transparent_index_consistency CHECK ( @@ -165,12 +211,12 @@ impl RusqliteMigration for Migration

{ INSERT INTO addresses_new ( account_id, key_scope, diversifier_index_be, address, transparent_child_index, cached_transparent_receiver_address, - exposed_at_height + exposed_at_height, receiver_flags ) SELECT account_id, key_scope, diversifier_index_be, address, transparent_child_index, cached_transparent_receiver_address, - exposed_at_height + exposed_at_height, receiver_flags FROM addresses; "#)?; @@ -182,11 +228,11 @@ impl RusqliteMigration for Migration

{ INSERT INTO addresses_new ( account_id, key_scope, diversifier_index_be, address, transparent_child_index, cached_transparent_receiver_address, - exposed_at_height + exposed_at_height, receiver_flags ) VALUES ( :account_id, :key_scope, :diversifier_index_be, :address, :transparent_child_index, :cached_transparent_receiver_address, - :exposed_at_height + :exposed_at_height, :receiver_flags ) "#, )?; @@ -227,7 +273,8 @@ impl RusqliteMigration for Migration

{ ":address": address, ":transparent_child_index": transparent_child_index, ":cached_transparent_receiver_address": address, - ":exposed_at_height": exposed_at_height + ":exposed_at_height": exposed_at_height, + ":receiver_flags": ReceiverFlags::P2PKH.bits() })?; account_ids.insert(account_id); @@ -328,6 +375,7 @@ impl RusqliteMigration for Migration

{ di, &ua, mined_height, + false, )?; conn.execute( @@ -391,6 +439,7 @@ impl RusqliteMigration for Migration

{ di, &ua, mined_height, + false, )?; conn.execute( diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 57605a9480..d9910b20fc 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -264,6 +264,7 @@ pub(crate) fn ensure_address< diversifier_index, &ua, exposure_height, + false, ) .map(Some) } else { diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 614c39c177..84fc7c0db0 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -367,6 +367,7 @@ pub(crate) fn ensure_address< diversifier_index, &ua, exposure_height, + false, ) .map(Some) } else { diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs index ab4dfff371..7198b877ad 100644 --- a/zcash_client_sqlite/src/wallet/transparent.rs +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -254,30 +254,34 @@ pub(crate) fn decode_transparent_child_index( }) } -/// Returns a vector with the next `n` previously unreserved transparent addresses for -/// the given account. These addresses must have been previously generated using -/// `generate_gap_addresses`. +/// Returns the current gap start, along with a vector with at most the next `n` previously +/// unreserved transparent addresses for the given account. These addresses must have been +/// previously generated using `generate_gap_addresses`. +/// +/// WARNING: the addresses returned by this method have not been marked as exposed; it is the +/// responsibility of the caller to correctly update the `exposed_at_height` value for each +/// returned address before such an address is exposed to a user. /// /// # Errors /// /// * `SqliteClientError::AccountUnknown`, if there is no account with the given id. -/// * `SqliteClientError::ReachedGapLimit`, if it is not possible to reserve `n` addresses -/// within the gap limit after the last address in this account that is known to have an -/// output in a mined transaction. /// * `SqliteClientError::AddressGeneration(AddressGenerationError::DiversifierSpaceExhausted)`, /// if the limit on transparent address indices has been reached. -pub(crate) fn reserve_next_n_addresses( +#[allow(clippy::type_complexity)] +pub(crate) fn select_addrs_to_reserve( conn: &rusqlite::Transaction, params: &P, account_id: AccountRef, key_scope: KeyScope, gap_limit: u32, n: usize, -) -> Result, SqliteClientError> { - if n == 0 { - return Ok(vec![]); - } - +) -> Result< + ( + NonHardenedChildIndex, + Vec<(AddressRef, TransparentAddress, TransparentAddressMetadata)>, + ), + SqliteClientError, +> { let gap_start = find_gap_start(conn, account_id, key_scope, gap_limit)?.ok_or( SqliteClientError::AddressGeneration(AddressGenerationError::DiversifierSpaceExhausted), )?; @@ -328,6 +332,39 @@ pub(crate) fn reserve_next_n_addresses( .filter_map(|r| r.transpose()) .collect::, _>>()?; + Ok((gap_start, addresses_to_reserve)) +} + +/// Returns a vector with the next `n` previously unreserved transparent addresses for the given +/// account, having marked each address as having been exposed at the current chain-tip height. +/// These addresses must have been previously generated using `generate_gap_addresses`. +/// +/// # Errors +/// +/// * [`SqliteClientError::AccountUnknown`], if there is no account with the given id. +/// * [`SqliteClientError::ReachedGapLimit`], if it is not possible to reserve `n` addresses +/// within the gap limit after the last address in this account that is known to have an +/// output in a mined transaction. +/// * [`SqliteClientError::AddressGeneration(AddressGenerationError::DiversifierSpaceExhausted)`] +/// if the limit on transparent address indices has been reached. +/// +/// [`SqliteClientError::AddressGeneration(AddressGenerationError::DiversifierSpaceExhausted)`]: +/// SqliteClientError::AddressGeneration +pub(crate) fn reserve_next_n_addresses( + conn: &rusqlite::Transaction, + params: &P, + account_id: AccountRef, + key_scope: KeyScope, + gap_limit: u32, + n: usize, +) -> Result, SqliteClientError> { + if n == 0 { + return Ok(vec![]); + } + + let (gap_start, addresses_to_reserve) = + select_addrs_to_reserve(conn, params, account_id, key_scope, gap_limit, n)?; + if addresses_to_reserve.len() < n { return Err(SqliteClientError::ReachedGapLimit( key_scope, diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index f8b289bba5..500fbfe59d 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -6,6 +6,9 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Added +- `zcash_keys::keys::UnifiedAddressRequest::{sapling, orchard, p2pkh}` methods + ## [0.7.0] - 2025-02-21 ### Added diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index dc6a04daa0..4c740229b5 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -655,6 +655,21 @@ impl UnifiedAddressRequest { p2pkh, } } + + /// Returns the [`ReceiverRequirement`] for inclusion of an Orchard receiver. + pub fn orchard(&self) -> ReceiverRequirement { + self.orchard + } + + /// Returns the [`ReceiverRequirement`] for inclusion of a Sapling receiver. + pub fn sapling(&self) -> ReceiverRequirement { + self.sapling + } + + /// Returns the [`ReceiverRequirement`] for inclusion of a P2pkh receiver. + pub fn p2pkh(&self) -> ReceiverRequirement { + self.p2pkh + } } #[cfg(feature = "transparent-inputs")]