zec
/
librustzcash
mirror of https://github.com/zcash/librustzcash.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #1562 from nuttycom/fix/scan_progress

This commit is contained in:
Kris Nuttycombe 2024-10-17 12:04:43 -06:00 committed by GitHub
parent 4bccae7571 634e43e9ed
commit 3d6b6e8f09
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 84 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
zcash_client_backend/CHANGELOG.md
View File

@ -7,6 +7,16 @@ and this library adheres to Rust's notion of
## [Unreleased]
### Added
- `zcash_client_backend::data_api`:
- `Progress`
- `WalletSummary::progress`
### Removed
- `zcash_client_backend::data_api`:
- `WalletSummary::scan_progress` and `WalletSummary::recovery_progress` have
been removed. Use `WalletSummary::progress` instead.
## [0.14.0] - 2024-10-04
### Added

102
zcash_client_backend/src/data_api.rs
View File

@ -67,7 +67,6 @@ use incrementalmerkletree::{frontier::Frontier, Retention};
use nonempty::NonEmpty;
use secrecy::SecretVec;
use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
use zcash_keys::address::Address;
use zip32::fingerprint::SeedFingerprint;
use self::{
@ -107,7 +106,7 @@ use {
use ambassador::delegatable_trait;
#[cfg(any(test, feature = "test-dependencies"))]
use zcash_primitives::consensus::NetworkUpgrade;
use {zcash_keys::address::Address, zcash_primitives::consensus::NetworkUpgrade};
pub mod chain;
pub mod error;
@ -462,6 +461,51 @@ impl<T> Ratio<T> {
}
}
/// A type representing the progress the wallet has made toward detecting all of the funds
/// belonging to the wallet.
///
/// The window over which progress is computed spans from the wallet's birthday to the current
/// chain tip. It is divided into two regions, the "Scan Window" which covers the region from the
/// wallet recovery height to the current chain tip, and the "Recovery Window" which covers the
/// range from the wallet birthday to the wallet recovery height. If no wallet recovery height is
/// available, the scan window will cover the entire range from the wallet birthday to the chain
/// tip.
///
/// Progress for both scanning and recovery is represented in terms of the ratio between notes
/// scanned and the total number of notes added to the chain in the relevant window. This ratio
/// should only be used to compute progress percentages for display, and the numerator and
/// denominator should not be treated as authoritative note counts. In the case that there are no
/// notes in a given block range, the denominator of these values will be zero, so callers should always
/// use checked division when converting the resulting values to percentages.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct Progress {
scan: Ratio<u64>,
recovery: Option<Ratio<u64>>,
}
impl Progress {
/// Constructs a new progress value from its constituent parts.
pub fn new(scan: Ratio<u64>, recovery: Option<Ratio<u64>>) -> Self {
Self { scan, recovery }
}
/// Returns the progress the wallet has made in scanning blocks for shielded notes belonging to
/// the wallet between the wallet recovery height (or the wallet birthday if no recovery height
/// is set) and the chain tip.
pub fn scan(&self) -> Ratio<u64> {
self.scan
}
/// Returns the progress the wallet has made in scanning blocks for shielded notes belonging to
/// the wallet between the wallet birthday and the block height at which recovery from seed was
/// initiated.
///
/// Returns `None` if no recovery height is set for the wallet.
pub fn recovery(&self) -> Option<Ratio<u64>> {
self.recovery
}
}
/// A type representing the potentially-spendable value of unspent outputs in the wallet.
///
/// The balances reported using this data structure may overestimate the total spendable value of
@ -475,8 +519,7 @@ pub struct WalletSummary<AccountId: Eq + Hash> {
account_balances: HashMap<AccountId, AccountBalance>,
chain_tip_height: BlockHeight,
fully_scanned_height: BlockHeight,
scan_progress: Option<Ratio<u64>>,
recovery_progress: Option<Ratio<u64>>,
progress: Progress,
next_sapling_subtree_index: u64,
#[cfg(feature = "orchard")]
next_orchard_subtree_index: u64,
@ -488,8 +531,7 @@ impl<AccountId: Eq + Hash> WalletSummary<AccountId> {
account_balances: HashMap<AccountId, AccountBalance>,
chain_tip_height: BlockHeight,
fully_scanned_height: BlockHeight,
scan_progress: Option<Ratio<u64>>,
recovery_progress: Option<Ratio<u64>>,
progress: Progress,
next_sapling_subtree_index: u64,
#[cfg(feature = "orchard")] next_orchard_subtree_index: u64,
) -> Self {
@ -497,8 +539,7 @@ impl<AccountId: Eq + Hash> WalletSummary<AccountId> {
account_balances,
chain_tip_height,
fully_scanned_height,
scan_progress,
recovery_progress,
progress,
next_sapling_subtree_index,
#[cfg(feature = "orchard")]
next_orchard_subtree_index,
@ -527,42 +568,17 @@ impl<AccountId: Eq + Hash> WalletSummary<AccountId> {
/// general usability, including the ability to spend existing funds that were
/// previously spendable.
///
/// The window over which progress is computed spans from the wallet's recovery height
/// to the current chain tip. This may be adjusted in future updates to better match
/// the intended semantics.
/// The window over which progress is computed spans from the wallet's birthday to the current
/// chain tip. It is divided into two segments: a "recovery" segment, between the wallet
/// birthday and the recovery height (currently the height at which recovery from seed was
/// initiated, but how this boundary is computed may change in the future), and a "scan"
/// segment, between the recovery height and the current chain tip.
///
/// Progress is represented in terms of the ratio between notes scanned and the total
/// number of notes added to the chain in the relevant window. This ratio should only
/// be used to compute progress percentages, and the numerator and denominator should
/// not be treated as authoritative note counts. The denominator of this ratio is
/// guaranteed to be nonzero.
///
/// Returns `None` if the wallet has insufficient information to be able to determine
/// scan progress.
pub fn scan_progress(&self) -> Option<Ratio<u64>> {
self.scan_progress
}
/// Returns the progress of recovering the wallet from seed.
///
/// This progress metric is intended as an indicator of how close the wallet is to
/// having a complete history.
///
/// The window over which progress is computed spans from the wallet birthday to the
/// wallet's recovery height. This may be adjusted in future updates to better match
/// the intended semantics.
///
/// Progress is represented in terms of the ratio between notes scanned and the total
/// number of notes added to the chain in the relevant window. This ratio should only
/// be used to compute progress percentages, and the numerator and denominator should
/// not be treated as authoritative note counts. Note that both the numerator and the
/// denominator of this ratio may be zero in the case that there is no recovery range
/// that need be scanned.
///
/// Returns `None` if the wallet has insufficient information to be able to determine
/// progress in scanning between the wallet birthday and the wallet recovery height.
pub fn recovery_progress(&self) -> Option<Ratio<u64>> {
self.recovery_progress
/// When converting the ratios returned here to percentages, checked division must be used in
/// order to avoid divide-by-zero errors. A zero denominator in a returned ratio indicates that
/// there are no shielded notes to be scanned in the associated block range.
pub fn progress(&self) -> Progress {
self.progress
}
/// Returns the Sapling subtree index that should start the next range of subtree

14
zcash_client_backend/src/data_api/testing/pool.rs
View File

@ -889,13 +889,10 @@ pub fn spend_fails_on_unverified_notes<T: ShieldedPoolTester>(
// Wallet is fully scanned
let summary = st.get_wallet_summary(1);
assert_eq!(
summary.as_ref().and_then(|s| s.recovery_progress()),
summary.as_ref().and_then(|s| s.progress().recovery()),
no_recovery,
);
assert_eq!(
summary.and_then(|s| s.scan_progress()),
Some(Ratio::new(1, 1))
);
assert_eq!(summary.map(|s| s.progress().scan()), Some(Ratio::new(1, 1)));
// Add more funds to the wallet in a second note
let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value);
@ -910,13 +907,10 @@ pub fn spend_fails_on_unverified_notes<T: ShieldedPoolTester>(
// Wallet is still fully scanned
let summary = st.get_wallet_summary(1);
assert_eq!(
summary.as_ref().and_then(|s| s.recovery_progress()),
summary.as_ref().and_then(|s| s.progress().recovery()),
no_recovery
);
assert_eq!(
summary.and_then(|s| s.scan_progress()),
Some(Ratio::new(2, 2))
);
assert_eq!(summary.map(|s| s.progress().scan()), Some(Ratio::new(2, 2)));
// Spend fails because there are insufficient verified notes
let extsk2 = T::sk(&[0xf5; 32]);

25
zcash_client_sqlite/src/wallet.rs
View File

@ -69,7 +69,7 @@ use rusqlite::{self, named_params, params, OptionalExtension};
use secrecy::{ExposeSecret, SecretVec};
use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
use zcash_client_backend::data_api::{
AccountPurpose, DecryptedTransaction, TransactionDataRequest, TransactionStatus,
AccountPurpose, DecryptedTransaction, Progress, TransactionDataRequest, TransactionStatus,
};
use zip32::fingerprint::SeedFingerprint;
@ -804,26 +804,6 @@ pub(crate) fn get_derived_account<P: consensus::Parameters>(
accounts.next().transpose()
}
#[derive(Debug)]
pub(crate) struct Progress {
scan: Ratio<u64>,
recovery: Option<Ratio<u64>>,
}
impl Progress {
pub(crate) fn new(scan: Ratio<u64>, recovery: Option<Ratio<u64>>) -> Self {
Self { scan, recovery }
}
pub(crate) fn scan(&self) -> Ratio<u64> {
self.scan
}
pub(crate) fn recovery(&self) -> Option<Ratio<u64>> {
self.recovery
}
}
pub(crate) trait ProgressEstimator {
fn sapling_scan_progress<P: consensus::Parameters>(
&self,
@ -1598,8 +1578,7 @@ pub(crate) fn get_wallet_summary<P: consensus::Parameters>(
account_balances,
chain_tip_height,
fully_scanned_height.unwrap_or(birthday_height - 1),
Some(progress.scan),
progress.recovery,
progress,
next_sapling_subtree_index,
#[cfg(feature = "orchard")]
next_orchard_subtree_index,

16
zcash_client_sqlite/src/wallet/scanning.rs
View File

@ -1297,15 +1297,17 @@ pub(crate) mod tests {
// resulting ratio (the number of notes in the recovery range) is zero.
let no_recovery = Some(Ratio::new(0, 0));
// We have scan ranges and a subtree, but have scanned no blocks.
// We have scan ranges and a subtree, but have scanned no blocks. Given the number of
// blocks scanned in the previous subtree, we estimate the number of notes in the current
// subtree
let summary = st.get_wallet_summary(1);
assert_eq!(
summary.as_ref().and_then(|s| s.recovery_progress()),
summary.as_ref().and_then(|s| s.progress().recovery()),
no_recovery,
);
assert_matches!(
summary.and_then(|s| s.scan_progress()),
Some(progress) if progress.numerator() == &0
summary.map(|s| s.progress().scan()),
Some(ratio) if *ratio.numerator() == 0
);
// Set up prior chain state. This simulates us having imported a wallet
@ -1345,7 +1347,7 @@ pub(crate) mod tests {
assert_eq!(summary.as_ref().map(|s| T::next_subtree_index(s)), Some(0));
assert_eq!(
summary.as_ref().and_then(|s| s.recovery_progress()),
summary.as_ref().and_then(|s| s.progress().recovery()),
no_recovery
);
@ -1357,7 +1359,7 @@ pub(crate) mod tests {
let expected_denom = expected_denom * 2;
let expected_denom = expected_denom + 1;
assert_eq!(
summary.and_then(|s| s.scan_progress()),
summary.map(|s| s.progress().scan()),
Some(Ratio::new(1, u64::from(expected_denom)))
);
@ -1450,7 +1452,7 @@ pub(crate) mod tests {
/ (max_scanned - (birthday.height() - 10)));
let summary = st.get_wallet_summary(1);
assert_eq!(
summary.and_then(|s| s.scan_progress()),
summary.map(|s| s.progress().scan()),
Some(Ratio::new(1, u64::from(expected_denom)))
);
}