Skip to content

Commit

Permalink
WIP: Add handling for transparent gap limit.
Browse files Browse the repository at this point in the history
  • Loading branch information
nuttycom committed Dec 24, 2024
1 parent 45b520d commit 783ea54
Show file tree
Hide file tree
Showing 24 changed files with 1,422 additions and 703 deletions.
16 changes: 13 additions & 3 deletions zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,19 @@ and this library adheres to Rust's notion of
- The `Recipient::External` variant is now a structured variant.
- The `Recipient::EphemeralTransparent` variant is now only available if
`zcash_client_backend` is built using the `transparent-inputs` feature flag.
- `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::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>`
- `zcash_client_backend::data_api::WalletWrite` has an added method
`get_address_for_index`

### 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.16.0] - 2024-12-16

Expand Down
40 changes: 30 additions & 10 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ use incrementalmerkletree::{frontier::Frontier, Retention};
use nonempty::NonEmpty;
use secrecy::SecretVec;
use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
use zip32::fingerprint::SeedFingerprint;
use zip32::{fingerprint::SeedFingerprint, DiversifierIndex};

use self::{
chain::{ChainState, CommitmentTreeRoot},
Expand Down Expand Up @@ -131,10 +131,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 @@ -1386,6 +1382,8 @@ pub trait WalletRead {
fn get_transparent_receivers(
&self,
_account: Self::AccountId,
_include_change: bool,
_include_ephemeral: bool,
) -> Result<HashMap<TransparentAddress, Option<TransparentAddressMetadata>>, Self::Error> {
Ok(HashMap::new())
}
Expand All @@ -1410,7 +1408,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, true)?.get(address) {
/// result.clone()
/// } else {
/// self.get_known_ephemeral_addresses(account, None)?
Expand All @@ -1431,7 +1429,10 @@ 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, true)?
.get(address)
{
result.clone()
} else {
self.get_known_ephemeral_addresses(account, None)?
Expand Down Expand Up @@ -2377,9 +2378,10 @@ 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. If the `request`
/// parameter is `None`, an address should be generated using all of the available receivers
/// for the account's UFVK.
///
/// Returns `Ok(None)` if the account identifier does not correspond to a known
/// account.
Expand All @@ -2389,6 +2391,24 @@ pub trait WalletWrite: WalletRead {
request: Option<UnifiedAddressRequest>,
) -> Result<Option<UnifiedAddress>, 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
/// be generated using all of the available receivers for the account's UFVK.
///
/// In the case that the diversifier index is outside of the range of valid transparent address
/// indexes, no transparent receiver should be generated in the resulting unified address. If a
/// transparent receiver is specifically requested for such a diversifier index,
/// implementations of this method should return an error.
///
/// 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.
fn get_address_for_index(
&mut self,
account: Self::AccountId,
diversifier_index: DiversifierIndex,
request: Option<UnifiedAddressRequest>,
) -> Result<Option<UnifiedAddress>, Self::Error>;

/// Updates the wallet's view of the blockchain.
///
/// This method is used to provide the wallet with information about the state of the
Expand Down
11 changes: 11 additions & 0 deletions zcash_client_backend/src/data_api/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2613,6 +2613,8 @@ impl WalletRead for MockWalletDb {
fn get_transparent_receivers(
&self,
_account: Self::AccountId,
_include_change: bool,
_include_ephemeral: bool,
) -> Result<HashMap<TransparentAddress, Option<TransparentAddressMetadata>>, Self::Error> {
Ok(HashMap::new())
}
Expand Down Expand Up @@ -2703,6 +2705,15 @@ impl WalletWrite for MockWalletDb {
Ok(None)
}

fn get_address_for_index(
&mut self,
_account: Self::AccountId,
_diversifier_index: DiversifierIndex,
_request: Option<UnifiedAddressRequest>,
) -> Result<Option<UnifiedAddress>, Self::Error> {
Ok(None)
}

#[allow(clippy::type_complexity)]
fn put_blocks(
&mut self,
Expand Down
24 changes: 19 additions & 5 deletions zcash_client_backend/src/data_api/testing/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ use crate::PoolType;
#[cfg(feature = "pczt")]
use pczt::roles::{prover::Prover, signer::Signer};

/// 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.
#[cfg(feature = "transparent-inputs")]
pub const EXTERNAL_ADDR_GAP_LIMIT: u32 = 20;

/// Trait that exposes the pool-specific types and operations necessary to run the
/// single-shielded-pool tests on a given pool.
///
Expand Down Expand Up @@ -511,7 +516,7 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
{
use zcash_primitives::transaction::components::transparent::builder::TransparentSigningSet;

use crate::data_api::{OutputOfSentTx, GAP_LIMIT};
use crate::data_api::OutputOfSentTx;

let mut st = TestBuilder::new()
.with_data_store_factory(ds_factory)
Expand Down Expand Up @@ -708,7 +713,7 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
.wallet()
.get_known_ephemeral_addresses(account_id, None)
.unwrap();
assert_eq!(known_addrs.len(), (GAP_LIMIT as usize) + 2);
assert_eq!(known_addrs.len(), (EXTERNAL_ADDR_GAP_LIMIT as usize) + 2);

// Check that the addresses are all distinct.
let known_set: HashSet<_> = known_addrs.iter().map(|(addr, _)| addr).collect();
Expand Down Expand Up @@ -802,7 +807,10 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
.wallet()
.get_known_ephemeral_addresses(account_id, None)
.unwrap();
assert_eq!(new_known_addrs.len(), (GAP_LIMIT as usize) + 11);
assert_eq!(
new_known_addrs.len(),
(EXTERNAL_ADDR_GAP_LIMIT as usize) + 11
);
assert!(new_known_addrs.starts_with(&known_addrs));

let reservation_should_succeed = |st: &mut TestState<_, DSF::DataStore, _>, n| {
Expand Down Expand Up @@ -836,7 +844,10 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
),
)
.unwrap();
assert_eq!(newer_known_addrs.len(), (GAP_LIMIT as usize) + 12 - 5);
assert_eq!(
newer_known_addrs.len(),
(EXTERNAL_ADDR_GAP_LIMIT as usize) + 12 - 5
);
assert!(newer_known_addrs.starts_with(&new_known_addrs[5..]));

// None of the five transactions created above (two from each proposal and the
Expand Down Expand Up @@ -898,7 +909,10 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
.wallet()
.get_known_ephemeral_addresses(account_id, None)
.unwrap();
assert_eq!(newest_known_addrs.len(), (GAP_LIMIT as usize) + 31);
assert_eq!(
newest_known_addrs.len(),
(EXTERNAL_ADDR_GAP_LIMIT as usize) + 31
);
assert!(newest_known_addrs.starts_with(&known_addrs));
assert!(newest_known_addrs[5..].starts_with(&newer_known_addrs));
}
Expand Down
5 changes: 3 additions & 2 deletions zcash_client_backend/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ where
"Refreshing UTXOs for {:?} from height {}",
account_id, start_height,
);
refresh_utxos(params, client, db_data, account_id, start_height).await?;
refresh_utxos(params, client, db_data, account_id, start_height, false).await?;
}

// 5) Get the suggested scan ranges from the wallet database
Expand Down Expand Up @@ -498,6 +498,7 @@ async fn refresh_utxos<P, ChT, DbT, CaErr, TrErr>(
db_data: &mut DbT,
account_id: DbT::AccountId,
start_height: BlockHeight,
include_ephemeral: bool,
) -> Result<(), Error<CaErr, <DbT as WalletRead>::Error, TrErr>>
where
P: Parameters + Send + 'static,
Expand All @@ -510,7 +511,7 @@ where
{
let request = service::GetAddressUtxosArg {
addresses: db_data
.get_transparent_receivers(account_id)
.get_transparent_receivers(account_id, true, include_ephemeral)
.map_err(Error::Wallet)?
.into_keys()
.map(|addr| addr.encode(params))
Expand Down
3 changes: 3 additions & 0 deletions zcash_client_sqlite/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ and this library adheres to Rust's notion of

### Changed
- Migrated to `nonempty 0.11`
- `zcash_client_sqlite::error::SqliteClientError` variants have changed:
- The `EphemeralAddressReuse` variant has been removed and replaced
by a new generalized `AddressReuse` error variant.

## [0.14.0] - 2024-12-16

Expand Down
33 changes: 17 additions & 16 deletions zcash_client_sqlite/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ use std::fmt;
use shardtree::error::ShardTreeError;
use zcash_address::ParseError;
use zcash_client_backend::data_api::NoteFilter;
use zcash_client_backend::PoolType;
use zcash_keys::keys::AddressGenerationError;
use zcash_primitives::zip32;
use zcash_primitives::{consensus::BlockHeight, transaction::components::amount::BalanceError};
use zcash_protocol::{PoolType, TxId};
use zip32;

use crate::{wallet::commitment_tree, AccountUuid};

#[cfg(feature = "transparent-inputs")]
use {
::transparent::address::TransparentAddress,
zcash_client_backend::encoding::TransparentCodecError,
zcash_primitives::{legacy::TransparentAddress, transaction::TxId},
};

/// The primary error type for the SQLite wallet backend.
Expand Down Expand Up @@ -121,16 +121,16 @@ pub enum SqliteClientError {
NoteFilterInvalid(NoteFilter),

/// The proposal cannot be constructed until transactions with previously reserved
/// ephemeral address outputs have been mined. The parameters are the account UUID and
/// the index that could not safely be reserved.
/// ephemeral address outputs have been mined. The error contains the index that could not
/// safely be reserved.
#[cfg(feature = "transparent-inputs")]
ReachedGapLimit(AccountUuid, u32),
ReachedGapLimit(u32),

/// An ephemeral address would be reused. The parameters are the address in string
/// form, and the txid of the earliest transaction in which it is known to have been
/// used.
#[cfg(feature = "transparent-inputs")]
EphemeralAddressReuse(String, TxId),
/// 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 of
/// the transaction in which it is known to have been used.
AddressReuse(String, TxId),
}

impl error::Error for SqliteClientError {
Expand Down Expand Up @@ -187,12 +187,13 @@ impl fmt::Display for SqliteClientError {
SqliteClientError::BalanceError(e) => write!(f, "Balance error: {}", e),
SqliteClientError::NoteFilterInvalid(s) => write!(f, "Could not evaluate filter query: {:?}", s),
#[cfg(feature = "transparent-inputs")]
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.",
SqliteClientError::ReachedGapLimit(bad_index) => write!(f,
"The proposal cannot be constructed until transactions with outputs to previously reserved ephemeral addresses have been mined. \
The ephemeral address at index {bad_index} could not be safely reserved.",
),
#[cfg(feature = "transparent-inputs")]
SqliteClientError::EphemeralAddressReuse(address_str, txid) => write!(f, "The ephemeral address {address_str} previously used in txid {txid} would be reused."),
SqliteClientError::AddressReuse(address_str, txid) => {
write!(f, "The address {address_str} previously used in txid {txid} would be reused.")
}
}
}
}
Expand Down
Loading

0 comments on commit 783ea54

Please sign in to comment.