Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add transparent address gap limit handling & general address rotation functionality. #1673

Merged
merged 31 commits into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6556505
zcash_client_sqlite: Add handling for the transparent address gap limit.
nuttycom Dec 2, 2024
f859883
zcash_client_sqlite: Add a test for external transparent address gap …
nuttycom Dec 30, 2024
4d69e06
Apply suggestions from code review
nuttycom Jan 29, 2025
4c0b9e6
zcash_client_backend: Update default gap limit for external to 10 add…
nuttycom Feb 3, 2025
b0cb1a3
zcash_client_backend: Add `WalletRead::utxo_query_height`
nuttycom Feb 5, 2025
7d3107f
Move transparent gap limit tests to `zcash_client_backend`
nuttycom Feb 6, 2025
b579c61
zcash_client_sqlite: Add birthday height to `Account` type.
nuttycom Feb 6, 2025
1e396ca
zcash_client_sqlite: Implement `get_address_at_index`
nuttycom Feb 7, 2025
7d845dd
zcash_client_sqlite: Use `ephemeral_addresses.used_in_tx` to set addr…
nuttycom Feb 7, 2025
8083782
Address comments from code review.
nuttycom Feb 6, 2025
295a422
zcash_client_sqlite: Set `exposed_at_height` for all existing addresses.
nuttycom Feb 10, 2025
71e1ca6
Address comments from code review.
nuttycom Feb 10, 2025
a16c8a7
zcash_client_sqlite: Add `Clock` type for capability-oriented access …
nuttycom Feb 25, 2025
4e715c3
zcash_client_sqlite: Add `Clock` capability to `WalletDb`
nuttycom Feb 25, 2025
d9776a4
zcash_client_backend: Update address rotation to take gap limits into…
nuttycom Feb 25, 2025
e58be3e
Fix transparent-inputs feature flagging error.
nuttycom Feb 26, 2025
e225a1d
zcash_client_sqlite: Narrow usage of `check_address_reuse`
nuttycom Feb 27, 2025
b582920
zcash_client_backend: Error on unknown account in `get_last_generated…
nuttycom Feb 27, 2025
25be66a
Apply suggestions from code review.
nuttycom Feb 27, 2025
bb26078
zcash_client_sqlite: Remove unused clocks
nuttycom Feb 28, 2025
5583608
Address comments from code review.
nuttycom Feb 28, 2025
0dcb2b2
zcash_client_sqlite: Remove `KeyScope` from the public API.
nuttycom Mar 1, 2025
96f5426
zcash_client_sqlite: Move encoding types & functions into the `encodi…
nuttycom Mar 1, 2025
c1f622a
zcash_keys: Replace the `UnifiedAddressRequest` struct with an enum.
nuttycom Mar 1, 2025
1199e60
Fix clippy complaints.
nuttycom Mar 1, 2025
d69e1a8
Address further comments from code review.
nuttycom Mar 1, 2025
08c7a90
zcash_client_backend: Document transparent recovery hazards of `get_a…
nuttycom Mar 3, 2025
6793a7d
zcash_client_sqlite: Update default ephemeral gap limit to 5
nuttycom Mar 3, 2025
fd78d1e
Documentation fixes and improvements.
daira Mar 4, 2025
cffda28
Fix incorrect arguments to `new_data_store` when the "transparent-inp…
daira Mar 4, 2025
19e0e39
Fix conditional imports.
daira Mar 4, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,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"] }
Expand Down
10 changes: 10 additions & 0 deletions components/zcash_protocol/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,16 @@ pub trait Parameters: Clone {
}
}

impl<P: Parameters> Parameters for &P {
fn network_type(&self) -> NetworkType {
(*self).network_type()
}

fn activation_height(&self, nu: NetworkUpgrade) -> Option<BlockHeight> {
(*self).activation_height(nu)
}
}

impl<P: Parameters> NetworkConstants for P {
fn coin_type(&self) -> u32 {
self.network_type().coin_type()
Expand Down
37 changes: 34 additions & 3 deletions zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,40 @@ and this library adheres to Rust's notion of

## [Unreleased]

### Added
- `zcash_client_backend::data_api::testing`:
- `struct transparent::GapLimits`
- `transparent::gap_limits` high-level test for gap limit handling

### 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<zcash_transparent::keys::NonHardenedChildIndex>` as its argument
instead of a `Range<u32>`
- 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 `WalletRead::get_last_generated_address_matching`
- `zcash_client_backend::data_api::WalletWrite`:
- has added method `get_address_for_index`. Please note the WARNINGS section
in the documentation for use of this method.
- `get_next_available_address` now returns the diversifier index at which the
address was generated in addition to the address. In addition, the
`UnifiedAddressRequest` argument is now non-optional; use
`UnifiedAddressRequest::AllAvailableKeys` to indicate that all available
keys should be used to generate receivers instead of `None`.
- Arguments to `get_address_for_index` have changed: the
`UnifiedAddressRequest` argument is now non-optional; use
`UnifiedAddressRequest::AllAvailableKeys` to indicate that all available
keys should be used to generate receivers instead of `None`.

### Removed
- `zcash_client_backend::data_api::GAP_LIMIT` gap limits are now configured
based upon the key scope that they're associated with; there is no longer a
globally applicable gap limit.

## [0.17.0] - 2025-02-21

### Added
Expand All @@ -32,9 +66,6 @@ and this library adheres to Rust's notion of
`map_internal_account_note` and `map_ephemeral_transparent_outpoint` and
`internal_account_note_transpose_option` methods have consequently been
removed.
- `zcash_client_backend::data_api::WalletRead::get_known_ephemeral_addresses`
now takes a `Range<zcash_transparent::keys::NonHardenedChildIndex>` as its
argument instead of a `Range<u32>`
- `zcash_client_backend::data_api::testing::TransactionSummary::from_parts`
has been modified; it now requires additional `total_spent` and `total_received`
arguments.
Expand Down
105 changes: 72 additions & 33 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ use zcash_protocol::{
value::{BalanceError, Zatoshis},
ShieldedProtocol, TxId,
};
use zip32::fingerprint::SeedFingerprint;
use zip32::{fingerprint::SeedFingerprint, DiversifierIndex};

use self::{
chain::{ChainState, CommitmentTreeRoot},
Expand Down Expand Up @@ -127,10 +127,6 @@ pub const SAPLING_SHARD_HEIGHT: u8 = sapling::NOTE_COMMITMENT_TREE_DEPTH / 2;
#[cfg(feature = "orchard")]
pub const ORCHARD_SHARD_HEIGHT: u8 = { orchard::NOTE_COMMITMENT_TREE_DEPTH as u8 } / 2;

/// 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 const GAP_LIMIT: u32 = 20;

/// An enumeration of constraints that can be applied when querying for nullifiers for notes
/// belonging to the wallet.
pub enum NullifierQuery {
Expand Down Expand Up @@ -1229,14 +1225,16 @@ pub trait WalletRead {
ufvk: &UnifiedFullViewingKey,
) -> Result<Option<Self::Account>, 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 filter, if the account identifier specified refers to a valid
/// account for this wallet.
///
/// This will return `Ok(None)` if the account identifier does not correspond to a known
/// account.
fn get_current_address(
/// This will return `Ok(None)` if no previously generated address conforms to the specified
/// request.
fn get_last_generated_address_matching(
&self,
account: Self::AccountId,
address_filter: UnifiedAddressRequest,
) -> Result<Option<UnifiedAddress>, Self::Error>;

/// Returns the birthday height for the given account, or an error if the account is not known
Expand Down Expand Up @@ -1369,6 +1367,7 @@ pub trait WalletRead {
fn get_transparent_receivers(
&self,
_account: Self::AccountId,
_include_change: bool,
) -> Result<HashMap<TransparentAddress, Option<TransparentAddressMetadata>>, Self::Error> {
Ok(HashMap::new())
}
Expand All @@ -1393,7 +1392,7 @@ pub trait WalletRead {
/// This is equivalent to (but may be implemented more efficiently than):
/// ```compile_fail
/// Ok(
/// if let Some(result) = self.get_transparent_receivers(account)?.get(address) {
/// if let Some(result) = self.get_transparent_receivers(account, true)?.get(address) {
/// result.clone()
/// } else {
/// self.get_known_ephemeral_addresses(account, None)?
Expand All @@ -1414,7 +1413,7 @@ pub trait WalletRead {
) -> Result<Option<TransparentAddressMetadata>, Self::Error> {
// This should be overridden.
Ok(
if let Some(result) = self.get_transparent_receivers(account)?.get(address) {
if let Some(result) = self.get_transparent_receivers(account, true)?.get(address) {
result.clone()
} else {
self.get_known_ephemeral_addresses(account, None)?
Expand All @@ -1425,10 +1424,22 @@ pub trait WalletRead {
)
}

/// Returns the maximum block height at which a transparent output belonging to the wallet has
/// been observed.
///
/// We must start looking for UTXOs for addresses within the current gap limit as of the block
/// height at which they might have first been revealed. This would have occurred when the gap
/// advanced as a consequence of a transaction being mined. The address at the start of the current
/// gap was potentially first revealed after the address at index `gap_start - (gap_limit + 1)`
/// received an output in a mined transaction; therefore, we take that height to be where we
/// should start searching for UTXOs.
#[cfg(feature = "transparent-inputs")]
fn utxo_query_height(&self, account: Self::AccountId) -> Result<BlockHeight, Self::Error>;

/// Returns a vector of ephemeral transparent addresses associated with the given
/// account controlled by this wallet, along with their metadata. The result includes
/// reserved addresses, and addresses for [`GAP_LIMIT`] additional indices (capped to
/// the maximum index).
/// reserved addresses, and addresses for the backend's configured gap limit worth
/// of additional indices (capped to the maximum index).
///
/// If `index_range` is some `Range`, it limits the result to addresses with indices
/// in that range. An `index_range` of `None` is defined to be equivalent to
Expand Down Expand Up @@ -1974,8 +1985,8 @@ impl<AccountId> SentTransactionOutput<AccountId> {
/// ### Fields:
/// * `output_index` - the index of the output or action in the sent transaction
/// * `recipient` - the recipient of the output, either a Zcash address or a
/// wallet-internal account and the note belonging to the wallet created by
/// the output
/// wallet-internal account and the note belonging to the wallet created by
/// the output
/// * `value` - the value of the output, in zatoshis
/// * `memo` - the memo that was sent with this output
pub fn from_parts(
Expand Down Expand Up @@ -2047,14 +2058,14 @@ impl AccountBirthday {
/// Constructs a new [`AccountBirthday`] from its constituent parts.
///
/// * `prior_chain_state`: The chain state prior to the birthday height of the account. The
/// birthday height is defined as the height of the first block to be scanned in wallet
/// recovery.
/// birthday height is defined as the height of the first block to be scanned in wallet
/// recovery.
/// * `recover_until`: An optional height at which the wallet should exit "recovery mode". In
/// order to avoid confusing shifts in wallet balance and spendability that may temporarily be
/// visible to a user during the process of recovering from seed, wallets may optionally set a
/// "recover until" height. The wallet is considered to be in "recovery mode" until there
/// exist no unscanned ranges between the wallet's birthday height and the provided
/// `recover_until` height, exclusive.
/// order to avoid confusing shifts in wallet balance and spendability that may temporarily be
/// visible to a user during the process of recovering from seed, wallets may optionally set a
/// "recover until" height. The wallet is considered to be in "recovery mode" until there
/// exist no unscanned ranges between the wallet's birthday height and the provided
/// `recover_until` height, exclusive.
///
/// This API is intended primarily to be used in testing contexts; under normal circumstances,
/// [`AccountBirthday::from_treestate`] should be used instead.
Expand All @@ -2069,13 +2080,13 @@ impl AccountBirthday {
/// Constructs a new [`AccountBirthday`] from a [`TreeState`] returned from `lightwalletd`.
///
/// * `treestate`: The tree state corresponding to the last block prior to the wallet's
/// birthday height.
/// birthday height.
/// * `recover_until`: An optional height at which the wallet should exit "recovery mode". In
/// order to avoid confusing shifts in wallet balance and spendability that may temporarily be
/// visible to a user during the process of recovering from seed, wallets may optionally set a
/// "recover until" height. The wallet is considered to be in "recovery mode" until there
/// exist no unscanned ranges between the wallet's birthday height and the provided
/// `recover_until` height, exclusive.
/// order to avoid confusing shifts in wallet balance and spendability that may temporarily be
/// visible to a user during the process of recovering from seed, wallets may optionally set a
/// "recover until" height. The wallet is considered to be in "recovery mode" until there
/// exist no unscanned ranges between the wallet's birthday height and the provided
/// `recover_until` height, exclusive.
pub fn from_treestate(
treestate: TreeState,
recover_until: Option<BlockHeight>,
Expand Down Expand Up @@ -2360,16 +2371,44 @@ pub trait WalletWrite: WalletRead {
key_source: Option<&str>,
) -> Result<Self::Account, Self::Error>;

/// Generates and persists the next available diversified address for the specified account,
/// given the current addresses known to the wallet. If the `request` parameter is `None`,
/// an address should be generated using all of the available receivers for the account's UFVK.
/// Generates, persists, and marks as exposed the next available diversified address for the
/// specified account, given the current addresses known to the wallet.
///
/// Returns `Ok(None)` if the account identifier does not correspond to a known
/// account.
fn get_next_available_address(
&mut self,
account: Self::AccountId,
request: Option<UnifiedAddressRequest>,
request: UnifiedAddressRequest,
) -> Result<Option<(UnifiedAddress, DiversifierIndex)>, Self::Error>;

/// Generates, persists, and marks as exposed a diversified address for the specified account
/// at the provided diversifier index.
///
/// Returns `Ok(None)` in the case that it is not possible to generate an address conforming
/// to the provided request at the specified diversifier index. Such a result might arise from
/// either a key not being available or the diversifier index not being valid for a
/// [`ReceiverRequirement::Require`]'ed receiver.
///
/// Address generation should fail 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.
///
/// # WARNINGS
/// If an address generated using this method has a transparent receiver and the
/// chosen diversifier index would be outside the wallet's internally-configured gap limit,
/// funds sent to these address are **likely to not be discovered on recovery from seed**. It
/// up to the caller of this method to either ensure that they only request transparent
/// receivers with indices within the range of a reasonable gap limit, or that they ensure that
/// their wallet provides backup facilities that can be used to ensure that funds sent to such
/// addresses are recoverable after a loss of wallet data.
///
/// [`ReceiverRequirement::Require`]: zcash_keys::keys::ReceiverRequirement::Require
fn get_address_for_index(
&mut self,
account: Self::AccountId,
diversifier_index: DiversifierIndex,
request: UnifiedAddressRequest,
) -> Result<Option<UnifiedAddress>, Self::Error>;

/// Updates the wallet's view of the blockchain.
Expand Down
Loading
Loading