Merge pull request #1541 from zcash/1070-zcb-wallet-summary-progress

Improve progress representation in `WalletSummary`

Author: nuttycom

zcash_client_backend/CHANGELOG.md

@@ -7,10 +7,18 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 
+### Added
+- `zcash_client_backend::data_api`:
+  - `WalletSummary::recovery_progress`
+
 ### Changed
 - The `Account` trait now uses an associated type for its `AccountId`
   type instead of a type parameter. This change allows for the simplification
   of some type signatures.
+- `zcash_client_backend::data_api`:
+  - `WalletSummary::scan_progress` now only reports progress for scanning blocks
+    "near" the chain tip. Progress for scanning earlier blocks is now reported
+    via `WalletSummary::recovery_progress`.
 - `zcash_client_backend::sync::run`:
   - Transparent outputs are now refreshed in addition to shielded notes.
 

zcash_client_backend/src/data_api.rs

@@ -471,6 +471,7 @@ pub struct WalletSummary<AccountId: Eq + Hash> {
     chain_tip_height: BlockHeight,
     fully_scanned_height: BlockHeight,
     scan_progress: Option<Ratio<u64>>,
+    recovery_progress: Option<Ratio<u64>>,
     next_sapling_subtree_index: u64,
     #[cfg(feature = "orchard")]
     next_orchard_subtree_index: u64,
@@ -483,6 +484,7 @@ impl<AccountId: Eq + Hash> WalletSummary<AccountId> {
         chain_tip_height: BlockHeight,
         fully_scanned_height: BlockHeight,
         scan_progress: Option<Ratio<u64>>,
+        recovery_progress: Option<Ratio<u64>>,
         next_sapling_subtree_index: u64,
         #[cfg(feature = "orchard")] next_orchard_subtree_index: u64,
     ) -> Self {
@@ -491,6 +493,7 @@ impl<AccountId: Eq + Hash> WalletSummary<AccountId> {
             chain_tip_height,
             fully_scanned_height,
             scan_progress,
+            recovery_progress,
             next_sapling_subtree_index,
             #[cfg(feature = "orchard")]
             next_orchard_subtree_index,
@@ -513,16 +516,47 @@ impl<AccountId: Eq + Hash> WalletSummary<AccountId> {
         self.fully_scanned_height
     }
 
-    /// Returns the progress of scanning shielded outputs, in terms of the ratio between notes
-    /// scanned and the total number of notes added to the chain since the wallet birthday.
+    /// Returns the progress of scanning the chain to bring the wallet up to date.
     ///
-    /// This ratio should only be used to compute progress percentages, and the numerator and
-    /// denominator should not be treated as authoritative note counts. Returns `None` if the
-    /// wallet is unable to determine the size of the note commitment tree.
+    /// This progress metric is intended as an indicator of how close the wallet is to
+    /// 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.
+    ///
+    /// 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.
+    ///
+    /// Returns `None` if the wallet is unable to determine the size of the note
+    /// commitment tree.
     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.
+    ///
+    /// Returns `None` if the wallet is unable to determine the size of the note
+    /// commitment tree.
+    pub fn recovery_progress(&self) -> Option<Ratio<u64>> {
+        self.recovery_progress
+    }
+
     /// Returns the Sapling subtree index that should start the next range of subtree
     /// roots passed to [`WalletCommitmentTrees::put_sapling_subtree_roots`].
     pub fn next_sapling_subtree_index(&self) -> u64 {

zcash_client_sqlite/src/lib.rs

@@ -74,7 +74,7 @@ use zip32::fingerprint::SeedFingerprint;
 
 use crate::{error::SqliteClientError, wallet::commitment_tree::SqliteShardStore};
 
-#[cfg(any(feature = "test-dependencies", not(feature = "orchard")))]
+#[cfg(any(test, feature = "test-dependencies", not(feature = "orchard")))]
 use zcash_protocol::PoolType;
 
 #[cfg(feature = "orchard")]

zcash_client_sqlite/src/testing/pool.rs

@@ -686,8 +686,21 @@ pub(crate) fn spend_fails_on_unverified_notes<T: ShieldedPoolTester>() {
         NonNegativeAmount::ZERO
     );
 
+    // The account is configured without a recover-until height, so is by definition
+    // fully recovered, and we count 1 per pool for both numerator and denominator.
+    let fully_recovered = {
+        let n = 1;
+        #[cfg(feature = "orchard")]
+        let n = n * 2;
+        Some(Ratio::new(n, n))
+    };
+
     // Wallet is fully scanned
     let summary = st.get_wallet_summary(1);
+    assert_eq!(
+        summary.as_ref().and_then(|s| s.recovery_progress()),
+        fully_recovered,
+    );
     assert_eq!(
         summary.and_then(|s| s.scan_progress()),
         Some(Ratio::new(1, 1))
@@ -705,6 +718,10 @@ pub(crate) 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()),
+        fully_recovered
+    );
     assert_eq!(
         summary.and_then(|s| s.scan_progress()),
         Some(Ratio::new(2, 2))