Documentation updates, fixes, and cleanups

Co-authored-by: Daira Hopwood <[email protected]>

zcash_client_backend/src/data_api.rs

@@ -89,6 +89,8 @@ pub trait WalletRead {
     /// tree size information for each block; or else the scan is likely to fail if notes belonging
     /// to the wallet are detected.
     ///
+    /// The returned range(s) may include block heights beyond the current chain tip.
+    ///
     /// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock
     fn suggest_scan_ranges(&self) -> Result<Vec<ScanRange>, Self::Error>;
 

zcash_client_backend/src/data_api/chain.rs

@@ -104,8 +104,7 @@
 //!                 }
 //!             }
 //!
-//!             // Truncation will have updated the suggested scan ranges, so we now
-//!             // re_request
+//!             // In case we updated the suggested scan ranges, now re-request.
 //!             scan_ranges = wallet_db.suggest_scan_ranges().map_err(Error::Wallet)?;
 //!         }
 //!         _ => {
@@ -126,7 +125,7 @@
 //!     // separate thread.
 //!     unimplemented!();
 //!
-//!     // Scan the downloaded blocks,
+//!     // Scan the downloaded blocks.
 //!     let scan_result = scan_cached_blocks(
 //!         &network,
 //!         &block_source,

zcash_client_backend/src/data_api/scanning.rs

@@ -10,13 +10,14 @@ pub enum ScanPriority {
     Scanned,
     /// Block ranges to be scanned to advance the fully-scanned height.
     Historic,
-    /// Block ranges adjacent to wallet open heights.
+    /// Block ranges adjacent to heights at which the user opened the wallet.
     OpenAdjacent,
     /// Blocks that must be scanned to complete note commitment tree shards adjacent to found notes.
     FoundNote,
     /// Blocks that must be scanned to complete the latest note commitment tree shard.
     ChainTip,
-    /// A previously-scanned range that must be verified has highest priority.
+    /// A previously scanned range that must be verified to check it is still in the
+    /// main chain, has highest priority.
     Verify,
 }
 

zcash_client_sqlite/src/chain.rs

@@ -26,10 +26,6 @@ pub mod migrations;
 /// Starting at `from_height`, the `with_row` callback is invoked with each block retrieved from
 /// the backing store. If the `limit` value provided is `None`, all blocks are traversed up to the
 /// maximum height.
-///
-/// # Panics
-///
-/// Panics if the provided `limit` value exceeds the range of a u32
 pub(crate) fn blockdb_with_blocks<F, DbErrT>(
     block_source: &BlockDb,
     from_height: Option<BlockHeight>,
@@ -200,10 +196,6 @@ pub(crate) fn blockmetadb_find_block(
 /// Starting at `from_height`, the `with_row` callback is invoked with each block retrieved from
 /// the backing store. If the `limit` value provided is `None`, all blocks are traversed up to the
 /// maximum height for which metadata is available.
-///
-/// # Panics
-///
-/// Panics if the provided `limit` value exceeds the range of a u32
 #[cfg(feature = "unstable")]
 pub(crate) fn fsblockdb_with_blocks<F, DbErrT>(
     cache: &FsBlockDb,

zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs

@@ -203,7 +203,8 @@ impl RusqliteMigration for Migration {
             }
         }
 
-        // Establish the scan queue & wallet history table
+        // Establish the scan queue & wallet history table.
+        // block_range_end is exclusive.
         transaction.execute_batch(
             "CREATE TABLE scan_queue (
                 block_range_start INTEGER NOT NULL,

zcash_client_sqlite/src/wallet/scanning.rs

@@ -107,7 +107,8 @@ pub(crate) fn suggest_scan_ranges(
 
 // This implements the dominance rule for range priority. If the inserted range's priority is
 // `Verify`, this replaces any existing priority. Otherwise, if the current priority is
-// `Scanned`, this overwrites any priority
+// `Scanned`, it remains as `Scanned`; and if the new priority is `Scanned`, it
+// overrides any existing priority.
 fn update_priority(current: ScanPriority, inserted: ScanPriority) -> ScanPriority {
     match (current, inserted) {
         (_, ScanPriority::Verify) => ScanPriority::Verify,
@@ -129,14 +130,25 @@ fn dominance(current: &ScanPriority, inserted: &ScanPriority, insert: Insert) ->
     }
 }
 
+/// In the comments for each alternative, `()` represents the left range and `[]` represents the right range.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 enum RangeOrdering {
+    /// `(   ) [   ]`
     LeftFirstDisjoint,
+    /// `( [   ) ]`
     LeftFirstOverlap,
+    /// `[ (   ) ]`
     LeftContained,
+    /// ```
+    /// (   )
+    /// [   ]
+    /// ```
     Equal,
+    /// `( [   ] )`
     RightContained,
+    /// `[ (   ] )`
     RightFirstOverlap,
+    /// `[   ] (   )`
     RightFirstDisjoint,
 }
 
@@ -317,7 +329,7 @@ impl SpanningTree {
             SpanningTree::Parent { span, left, right } => {
                 // TODO: this algorithm always preserves the existing partition point, and does not
                 // do any rebalancing or unification of ranges within the tree; `into_vec`
-                // performes such unification and the tree being unbalanced should be fine given
+                // performs such unification, and the tree being unbalanced should be fine given
                 // the relatively small number of ranges we should ordinarily be concerned with.
                 use RangeOrdering::*;
                 match RangeOrdering::cmp(&span, to_insert.block_range()) {
@@ -425,7 +437,7 @@ pub(crate) fn insert_queue_entries<'a>(
         trace!("Inserting queue entry {}", entry);
         if !entry.is_empty() {
             stmt.execute(named_params![
-                ":block_range_start": u32::from(entry.block_range().start) ,
+                ":block_range_start": u32::from(entry.block_range().start),
                 ":block_range_end": u32::from(entry.block_range().end),
                 ":priority": priority_code(&entry.priority())
             ])?;
@@ -633,7 +645,7 @@ pub(crate) fn update_chain_tip<P: consensus::Parameters>(
         .map(|h| ScanRange::from_parts(h..chain_end, ScanPriority::ChainTip));
 
     // Create scanning ranges to either validate potentially invalid blocks at the wallet's view
-    // of the chain tip,
+    // of the chain tip, or connect the prior tip to the new tip.
     let tip_entry = block_height_extrema(conn)?.map(|(_, prior_tip)| {
         // If we don't have shard metadata, this means we're doing linear scanning, so create a
         // scan range from the prior tip to the current tip with `Historic` priority.
@@ -1049,7 +1061,7 @@ mod tests {
             ]
         );
 
-        // simulate the wallet going offline for a bit, update the chain tip to 30 blocks in the
+        // simulate the wallet going offline for a bit, update the chain tip to 20 blocks in the
         // future
         assert_matches!(
             db_data.update_chain_tip(sapling_activation_height() + 340),