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 23, 2024
1 parent ac77f50 commit 0259167
Show file tree
Hide file tree
Showing 22 changed files with 862 additions and 481 deletions.
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.

9 changes: 9 additions & 0 deletions zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ and this library adheres to Rust's notion of

### Changed
- Migrated to `nonempty 0.11`
- `zcash_client_backend::data_api::WalletRead::get_transparent_receivers` now
takes additional `include_change` and `include_ephemeral` arguments.
- `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 @@ -129,10 +129,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 @@ -1384,6 +1380,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 @@ -1408,7 +1406,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 @@ -1429,7 +1427,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 @@ -2375,9 +2376,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 @@ -2387,6 +2389,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 @@ -2611,6 +2611,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 @@ -2701,6 +2703,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 @@ -830,7 +838,10 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
.wallet()
.get_known_ephemeral_addresses(account_id, Some(5..100))
.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 @@ -892,7 +903,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
1 change: 1 addition & 0 deletions zcash_client_sqlite/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ features = [
rustdoc-args = ["--cfg", "docsrs"]

[dependencies]
transparent.workspace = true
zcash_address.workspace = true
zcash_client_backend = { workspace = true, features = ["unstable-serialization", "unstable-spanning-tree"] }
zcash_encoding.workspace = true
Expand Down
61 changes: 58 additions & 3 deletions zcash_client_sqlite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,42 @@ pub struct UtxoId(pub i64);
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
struct TxRef(pub i64);

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub(crate) struct GapLimits {
external: u32,
transparent_internal: u32,
ephemeral: u32,
}

impl GapLimits {
pub(crate) fn external(&self) -> u32 {
self.external
}

pub(crate) fn transparent_internal(&self) -> u32 {
self.transparent_internal
}

pub(crate) fn ephemeral(&self) -> u32 {
self.ephemeral
}
}

impl Default for GapLimits {
fn default() -> Self {
Self {
external: 20,
transparent_internal: 3,
ephemeral: 3,
}
}
}

/// A wrapper for the SQLite connection to the wallet database.
pub struct WalletDb<C, P> {
conn: C,
params: P,
gap_limits: GapLimits,
}

/// A wrapper for a SQLite transaction affecting the wallet database.
Expand All @@ -260,7 +292,11 @@ impl<P: consensus::Parameters + Clone> WalletDb<Connection, P> {
pub fn for_path<F: AsRef<Path>>(path: F, params: P) -> Result<Self, rusqlite::Error> {
Connection::open(path).and_then(move |conn| {
rusqlite::vtab::array::load_module(&conn)?;
Ok(WalletDb { conn, params })
Ok(WalletDb {
conn,
params,
gap_limits: GapLimits::default(),
})
})
}

Expand All @@ -272,6 +308,7 @@ impl<P: consensus::Parameters + Clone> WalletDb<Connection, P> {
let mut wdb = WalletDb {
conn: SqlTransaction(&tx),
params: self.params.clone(),
gap_limits: self.gap_limits,
};
let result = f(&mut wdb)?;
tx.commit()?;
Expand Down Expand Up @@ -619,8 +656,23 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
fn get_transparent_receivers(
&self,
account: Self::AccountId,
include_change: bool,
include_ephemeral: bool,
) -> Result<HashMap<TransparentAddress, Option<TransparentAddressMetadata>>, Self::Error> {
wallet::transparent::get_transparent_receivers(self.conn.borrow(), &self.params, account)
use wallet::KeyScope;

let key_scopes: &[KeyScope] = match (include_change, include_ephemeral) {
(true, true) => &[KeyScope::EXTERNAL, KeyScope::INTERNAL, KeyScope::Ephemeral],
(true, false) => &[KeyScope::EXTERNAL, KeyScope::INTERNAL],
(false, true) => &[KeyScope::EXTERNAL, KeyScope::Ephemeral],
(false, false) => &[KeyScope::EXTERNAL],
};
wallet::transparent::get_transparent_receivers(
self.conn.borrow(),
&self.params,
account,
key_scopes,
)
}

#[cfg(feature = "transparent-inputs")]
Expand Down Expand Up @@ -2236,7 +2288,10 @@ mod tests {
let ufvk = account.usk().to_unified_full_viewing_key();
let (taddr, _) = account.usk().default_transparent_address();

let receivers = st.wallet().get_transparent_receivers(account.id()).unwrap();
let receivers = st
.wallet()
.get_transparent_receivers(account.id(), false, false)
.unwrap();

// The receiver for the default UA should be in the set.
assert!(receivers.contains_key(
Expand Down
Loading

0 comments on commit 0259167

Please sign in to comment.