From e164b593297f4804dd9644ac0351bf404a77b8e6 Mon Sep 17 00:00:00 2001 From: Daira-Emma Hopwood Date: Mon, 24 Jun 2024 10:49:47 +0100 Subject: [PATCH] Move most ephemeral address index handling into helper functions in `zcash_client_sqlite::wallet::transparent::ephemeral`. Also report the account id and index for `SqliteClientError::ReachedGapLimit`. Signed-off-by: Daira-Emma Hopwood --- zcash_client_backend/src/wallet.rs | 1 + zcash_client_sqlite/src/error.rs | 11 +- zcash_client_sqlite/src/wallet/init.rs | 2 +- zcash_client_sqlite/src/wallet/transparent.rs | 129 ++++++------------ .../src/wallet/transparent/ephemeral.rs | 98 +++++++++++++ 5 files changed, 148 insertions(+), 93 deletions(-) create mode 100644 zcash_client_sqlite/src/wallet/transparent/ephemeral.rs diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 18540c824..f6897fac0 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -633,6 +633,7 @@ impl OvkPolicy { } /// Metadata related to the ZIP 32 derivation of a transparent address. +/// This is implicitly scoped to an account. #[derive(Debug, Clone, PartialEq, Eq)] #[cfg(feature = "transparent-inputs")] pub struct TransparentAddressMetadata { diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index aad3df1b7..5e4fc1c97 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -15,6 +15,7 @@ use crate::PRUNING_DEPTH; #[cfg(feature = "transparent-inputs")] use { + crate::AccountId, zcash_client_backend::encoding::TransparentCodecError, zcash_primitives::{legacy::TransparentAddress, transaction::TxId}, }; @@ -114,9 +115,10 @@ pub enum SqliteClientError { BalanceError(BalanceError), /// The proposal cannot be constructed until transactions with previously reserved - /// ephemeral address outputs have been mined. + /// ephemeral address outputs have been mined. The parameters are the account id and + /// the index that could not safely be reserved. #[cfg(feature = "transparent-inputs")] - ReachedGapLimit, + ReachedGapLimit(AccountId, u32), /// An ephemeral address would be reused, or incorrectly used as an external address. /// The parameters are the address in string form, and if it is known to have been @@ -175,7 +177,10 @@ impl fmt::Display for SqliteClientError { SqliteClientError::UnsupportedPoolType(t) => write!(f, "Pool type is not currently supported: {}", t), SqliteClientError::BalanceError(e) => write!(f, "Balance error: {}", e), #[cfg(feature = "transparent-inputs")] - SqliteClientError::ReachedGapLimit => write!(f, "The proposal cannot be constructed until transactions with previously reserved ephemeral address outputs have been mined."), + SqliteClientError::ReachedGapLimit(account_id, bad_index) => write!(f, + "The proposal cannot be constructed until transactions with previously reserved ephemeral address outputs have been mined. \ + The ephemeral address in account {account_id:?} at index {bad_index} could not be safely reserved.", + ), #[cfg(feature = "transparent-inputs")] SqliteClientError::EphemeralAddressReuse(address_str, Some(txid)) => write!(f, "The ephemeral address {address_str} previously used in txid {txid} would be reused."), #[cfg(feature = "transparent-inputs")] diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index cfe6f7320..b14ecd9e5 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -177,7 +177,7 @@ fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> Wallet unreachable!("we don't call methods that require a known chain height") } #[cfg(feature = "transparent-inputs")] - SqliteClientError::ReachedGapLimit => { + SqliteClientError::ReachedGapLimit(_, _) => { unreachable!("we don't do ephemeral address tracking") } #[cfg(feature = "transparent-inputs")] diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs index cdd632d1f..82e43cede 100644 --- a/zcash_client_sqlite/src/wallet/transparent.rs +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -1,19 +1,24 @@ //! Functions for transparent input support in the wallet. +use std::cmp::max; +use std::collections::{HashMap, HashSet}; + use rusqlite::OptionalExtension; use rusqlite::{named_params, Connection, Row}; -use std::collections::HashMap; -use std::collections::HashSet; -use zcash_client_backend::data_api::AccountBalance; -use zcash_keys::address::Address; -use zcash_keys::keys::AddressGenerationError; use zip32::{DiversifierIndex, Scope}; use zcash_address::unified::{Encoding, Ivk, Uivk}; -use zcash_client_backend::wallet::{TransparentAddressMetadata, WalletTransparentOutput}; -use zcash_keys::encoding::{encode_transparent_address_p, AddressCodec}; +use zcash_client_backend::{ + data_api::AccountBalance, + keys::AddressGenerationError, + wallet::{TransparentAddressMetadata, WalletTransparentOutput}, +}; +use zcash_keys::{ + address::Address, + encoding::{encode_transparent_address_p, AddressCodec}, +}; use zcash_primitives::{ legacy::{ - keys::{EphemeralIvk, IncomingViewingKey, NonHardenedChildIndex, TransparentKeyScope}, + keys::{EphemeralIvk, IncomingViewingKey, NonHardenedChildIndex}, Script, TransparentAddress, }, transaction::{ @@ -28,6 +33,8 @@ use crate::{SqlTransaction, WalletDb}; use super::{chain_tip_height, get_account, get_account_ids}; +mod ephemeral; + pub(crate) fn detect_spending_accounts<'a>( conn: &Connection, spent: impl Iterator, @@ -494,7 +501,7 @@ pub(crate) fn find_account_for_transparent_output( return Ok(Some(account_id)); } - // Note that this does not search ephemeral addresses that have not yet been reserved. + // Search ephemeral addresses that have already been reserved. if let Some(account_id) = conn .query_row( "SELECT account_id FROM ephemeral_addresses WHERE address = :address", @@ -741,14 +748,6 @@ pub(crate) fn get_ephemeral_ivk( .derive_ephemeral_ivk()?) } -// Same as Bitcoin. -const GAP_LIMIT: i32 = 20; - -const EPHEMERAL_SCOPE: TransparentKeyScope = match TransparentKeyScope::custom(2) { - Some(s) => s, - None => unreachable!(), -}; - /// Returns a vector with all ephemeral transparent addresses potentially belonging to this wallet. /// If `for_detection` is true, this includes addresses for an additional GAP_LIMIT indices. pub(crate) fn get_reserved_ephemeral_addresses( @@ -771,30 +770,19 @@ pub(crate) fn get_reserved_ephemeral_addresses( first_unused_index = i32::try_from(raw_index) .map_err(|e| SqliteClientError::CorruptedData(e.to_string()))? .checked_add(1); - let address_index = NonHardenedChildIndex::from_index(raw_index).expect("just checked"); - result.insert( - TransparentAddress::decode(params, &addr_str)?, - Some(TransparentAddressMetadata::new( - EPHEMERAL_SCOPE, - address_index, - )), - ); + let address_index = NonHardenedChildIndex::from_index(raw_index).unwrap(); + let address = TransparentAddress::decode(params, &addr_str)?; + result.insert(address, Some(ephemeral::metadata(address_index))); } if for_detection { if let Some(first) = first_unused_index { let ephemeral_ivk = get_ephemeral_ivk(conn, params, account_id)?; - for index in first..=first.saturating_add(GAP_LIMIT - 1) { - let address_index = - NonHardenedChildIndex::from_index(index as u32).expect("valid index"); - result.insert( - ephemeral_ivk.derive_ephemeral_address(address_index)?, - Some(TransparentAddressMetadata::new( - EPHEMERAL_SCOPE, - address_index, - )), - ); + for raw_index in ephemeral::range_after(first, ephemeral::GAP_LIMIT) { + let address_index = NonHardenedChildIndex::from_index(raw_index).unwrap(); + let address = ephemeral_ivk.derive_ephemeral_address(address_index)?; + result.insert(address, Some(ephemeral::metadata(address_index))); } } } @@ -827,52 +815,18 @@ pub(crate) fn reserve_next_n_ephemeral_addresses( assert!(n > 0); let ephemeral_ivk = get_ephemeral_ivk(wdb.conn.0, &wdb.params, account_id)?; + let last_reserved_index = ephemeral::last_reserved_index(wdb.conn.0, account_id)?; + let last_safe_index = ephemeral::last_safe_index(wdb.conn.0, account_id)?; + let allocation = ephemeral::range_after(last_reserved_index, n); - // The inner join with `transactions` excludes addresses for which - // `mined_in_tx` is NULL. The query also excludes addresses observed - // to have been mined in a transaction that we currently see as unmined. - // This is conservative in terms of avoiding violation of the gap - // invariant: it can only cause us to get to the end of the gap (and - // start reporting `ReachedGapLimit` errors) sooner. - let last_gap_index: i32 = wdb - .conn - .0 - .query_row( - "SELECT address_index FROM ephemeral_addresses - JOIN transactions t ON t.id_tx = mined_in_tx - WHERE account_id = :account_id AND t.mined_height IS NOT NULL - ORDER BY address_index DESC LIMIT 1", - named_params![":account_id": account_id.0], - |row| row.get::<_, u32>(0), - ) - .optional()? - .map_or(Ok(-1i32), |i| { - i32::try_from(i).map_err(|e| SqliteClientError::CorruptedData(e.to_string())) - })? - .saturating_add(GAP_LIMIT); - - let (first_index, last_index) = wdb - .conn - .0 - .query_row( - "SELECT address_index FROM ephemeral_addresses - WHERE account_id = :account_id - ORDER BY address_index DESC LIMIT 1", - named_params![":account_id": account_id.0], - |row| row.get::<_, u32>(0), - ) - .optional()? - .map_or(Ok(-1i32), |i| { - i32::try_from(i).map_err(|e| SqliteClientError::CorruptedData(e.to_string())) - }) - .map(|i: i32| i.checked_add(1).zip(i.checked_add(n.try_into().ok()?)))? - .ok_or(SqliteClientError::AddressGeneration( + if allocation.clone().count() < n.try_into().unwrap() { + return Err(SqliteClientError::AddressGeneration( AddressGenerationError::DiversifierSpaceExhausted, - ))?; - - assert!(last_index >= first_index); - if last_index > last_gap_index { - return Err(SqliteClientError::ReachedGapLimit); + )); + } + if *allocation.end() > last_safe_index { + let unsafe_index = max(*allocation.start(), last_safe_index.saturating_add(1)); + return Err(SqliteClientError::ReachedGapLimit(account_id, unsafe_index)); } // used_in_tx and mined_in_tx are initially NULL @@ -881,20 +835,17 @@ pub(crate) fn reserve_next_n_ephemeral_addresses( VALUES (:account_id, :address_index, :address)", )?; - (first_index..=last_index) - .map(|address_index| { - let child = NonHardenedChildIndex::from_index(address_index as u32) - .expect("valid by construction"); - let address = ephemeral_ivk.derive_ephemeral_address(child)?; + allocation + .map(|raw_index| { + let address_index = NonHardenedChildIndex::from_index(raw_index).unwrap(); + let address = ephemeral_ivk.derive_ephemeral_address(address_index)?; + stmt_insert_ephemeral_address.execute(named_params![ ":account_id": account_id.0, - ":address_index": address_index, + ":address_index": raw_index, ":address": encode_transparent_address_p(&wdb.params, &address) ])?; - Ok(( - address, - TransparentAddressMetadata::new(EPHEMERAL_SCOPE, child), - )) + Ok((address, ephemeral::metadata(address_index))) }) .collect() } diff --git a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs new file mode 100644 index 000000000..7930d5bf3 --- /dev/null +++ b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs @@ -0,0 +1,98 @@ +//! Functions for wallet support of ephemeral transparent addresses. +use std::ops::RangeInclusive; + +use rusqlite::{named_params, OptionalExtension}; + +use zcash_client_backend::wallet::TransparentAddressMetadata; +use zcash_primitives::legacy::keys::{NonHardenedChildIndex, TransparentKeyScope}; + +use crate::{error::SqliteClientError, AccountId}; + +/// The number of ephemeral addresses that can be safely reserved without observing any +/// of them to be mined. This is the same as the gap limit in Bitcoin. +pub(crate) const GAP_LIMIT: i32 = 20; + +// The custom scope used for derivation of ephemeral addresses. +// TODO: consider moving this to `zcash_primitives::legacy::keys`, or else +// provide a way to derive `ivk`s for custom scopes in general there, so that +// the constant isn't duplicated. +pub(crate) const EPHEMERAL_SCOPE: TransparentKeyScope = match TransparentKeyScope::custom(2) { + Some(s) => s, + None => unreachable!(), +}; + +// Returns `TransparentAddressMetadata` in the ephemeral scope for the +// given address index. +pub(crate) fn metadata(address_index: NonHardenedChildIndex) -> TransparentAddressMetadata { + TransparentAddressMetadata::new(EPHEMERAL_SCOPE, address_index) +} + +/// Returns the last reserved ephemeral address index in the given account, +/// or -1 if the account has no reserved ephemeral addresses. +pub(crate) fn last_reserved_index( + conn: &rusqlite::Connection, + account_id: AccountId, +) -> Result { + match conn + .query_row( + "SELECT address_index FROM ephemeral_addresses + WHERE account_id = :account_id + ORDER BY address_index DESC + LIMIT 1", + named_params![":account_id": account_id.0], + |row| row.get::<_, i32>(0), + ) + .optional()? + { + Some(i) if i < 0 => Err(SqliteClientError::CorruptedData( + "negative index".to_owned(), + )), + Some(i) => Ok(i), + None => Ok(-1), + } +} + +/// Returns the last ephemeral address index in the given account that +/// would not violate the gap invariant if used. +pub(crate) fn last_safe_index( + conn: &rusqlite::Connection, + account_id: AccountId, +) -> Result { + // The inner join with `transactions` excludes addresses for which + // `mined_in_tx` is NULL. The query also excludes addresses observed + // to have been mined in a transaction that we currently see as unmined. + // This is conservative in terms of avoiding violation of the gap + // invariant: it can only cause us to get to the end of the gap sooner. + let last_mined_index: i32 = match conn + .query_row( + "SELECT address_index FROM ephemeral_addresses + JOIN transactions t ON t.id_tx = mined_in_tx + WHERE account_id = :account_id AND t.mined_height IS NOT NULL + ORDER BY address_index DESC + LIMIT 1", + named_params![":account_id": account_id.0], + |row| row.get::<_, i32>(0), + ) + .optional()? + { + Some(i) if i < 0 => Err(SqliteClientError::CorruptedData( + "negative index".to_owned(), + )), + Some(i) => Ok(i), + None => Ok(-1), + }?; + Ok(u32::try_from(last_mined_index.saturating_add(GAP_LIMIT)).unwrap()) +} + +/// Utility function to return an `InclusiveRange` that starts at `i + 1` +/// and is of length up to `n`. The range is truncated if necessary to end at +/// the maximum valid address index, `i32::MAX`. +/// +/// Precondition: `i >= -1 and n > 0` +pub(crate) fn range_after(i: i32, n: i32) -> RangeInclusive { + assert!(i >= -1); + assert!(n > 0); + let first = u32::try_from(i64::from(i) + 1).unwrap(); + let last = u32::try_from(i.saturating_add(n)).unwrap(); + first..=last +}