Apply suggestions from code review

Co-authored-by: str4d <[email protected]>

zcash_client_backend/src/data_api.rs

@@ -487,7 +487,7 @@ pub struct AccountBirthday {
     recover_until: Option<BlockHeight>,
 }
 
-/// Errors that can occur in the construction of an [`AccountBirthday`] from a [`TreeState`]
+/// Errors that can occur in the construction of an [`AccountBirthday`] from a [`TreeState`].
 pub enum BirthdayError {
     HeightInvalid(TryFromIntError),
     Decode(io::Error),
@@ -597,12 +597,12 @@ pub trait WalletWrite: WalletRead {
     /// Returns the account identifier for the newly-created wallet database entry, along with the
     /// associated [`UnifiedSpendingKey`].
     ///
-    /// If a birthday height is having a height is below the current chain tip, this operation will
+    /// If `birthday.height()` is below the current chain tip, this operation will
     /// trigger a re-scan of the blocks at and above the provided height. The birthday height is
     /// defined as the minimum block height that will be scanned for funds belonging to the wallet.
     ///
     /// For new wallets, callers should construct the [`AccountBirthday`] using
-    /// [`AccountBirthday::from_treestate`] for the block at height `chain_tip_height - PRUNING_DEPTH`.
+    /// [`AccountBirthday::from_treestate`] for the block at height `chain_tip_height - 100`.
     /// Setting the birthday height to a tree state below the pruning depth ensures that reorgs
     /// cannot cause funds intended for the wallet to be missed; otherwise, if the chain tip height
     /// were used for the wallet birthday, a transaction targeted at a height greater than the

zcash_client_sqlite/src/wallet.rs

@@ -207,7 +207,7 @@ pub(crate) fn add_account<P: consensus::Parameters>(
         .activation_height(NetworkUpgrade::Sapling)
         .expect("Sapling activation height must be available.");
 
-    // Add the ignored range up to and including the birthday height.
+    // Add the ignored range up to the birthday height.
     if sapling_activation_height < birthday.height() {
         let ignored_range = sapling_activation_height..birthday.height();
 
@@ -223,8 +223,8 @@ pub(crate) fn add_account<P: consensus::Parameters>(
         )?;
     };
 
-    // Rewrite the scan ranges above the birthday height so that we'll ensure we re-scan to find
-    // any notes that might belong to the newly added account.
+    // Rewrite the scan ranges starting from the birthday height so that we'll ensure we
+    // re-scan to find any notes that might belong to the newly added account.
     if let Some(t) = chain_tip {
         let rescan_range = birthday.height()..(t + 1);
 
@@ -676,10 +676,10 @@ pub(crate) fn get_sent_memo(
 }
 
 /// Returns the minimum birthday height for accounts in the wallet.
-///
-/// TODO ORCHARD: we should consider whether we want to permit protocol-restricted accounts; if so,
-/// we would then want this method to take a protocol identifier to be able to learn the wallet's
-/// "Orchard birthday" which might be different from the overall wallet birthday.
+//
+// TODO ORCHARD: we should consider whether we want to permit protocol-restricted accounts; if so,
+// we would then want this method to take a protocol identifier to be able to learn the wallet's
+// "Orchard birthday" which might be different from the overall wallet birthday.
 pub(crate) fn wallet_birthday(
     conn: &rusqlite::Connection,
 ) -> Result<Option<BlockHeight>, rusqlite::Error> {

zcash_client_sqlite/src/wallet/sapling.rs

@@ -170,7 +170,7 @@ pub(crate) fn get_spendable_sapling_notes(
             AND sapling_received_notes.commitment_tree_position < unscanned.end_position_exclusive
             -- exclude unscanned ranges that start above the anchor height (they don't affect spendability)
             AND unscanned.block_range_start <= :anchor_height
-            -- exclude unscanned ranges that end below or on the wallet birthday
+            -- exclude unscanned ranges that end below the wallet birthday
             AND unscanned.block_range_end > :wallet_birthday
          )",
     )?;
@@ -256,7 +256,7 @@ pub(crate) fn select_spendable_sapling_notes(
                 AND sapling_received_notes.commitment_tree_position < unscanned.end_position_exclusive
                 -- exclude unscanned ranges that start above the anchor height (they don't affect spendability)
                 AND unscanned.block_range_start <= :anchor_height
-                -- exclude unscanned ranges that end below or on the wallet birthday
+                -- exclude unscanned ranges that end below the wallet birthday
                 AND unscanned.block_range_end > :wallet_birthday
              )
          )

zcash_client_sqlite/src/wallet/scanning.rs

@@ -1347,7 +1347,7 @@ mod tests {
         let sap_active = st.sapling_activation_height();
 
         let expected = vec![
-            // The range up to and including the wallet's birthday height is ignored.
+            // The range up to the wallet's birthday height is ignored.
             scan_range(u32::from(sap_active)..u32::from(birthday.height()), Ignored),
         ];
         let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
@@ -1394,8 +1394,8 @@ mod tests {
 
         // Verify that the suggested scan ranges match what is expected
         let expected = vec![
-            // The birthday height was "last scanned" (as the wallet birthday) so we verify 10
-            // blocks starting at that height.
+            // The birthday height is the "first to be scanned" (as the wallet birthday),
+            // so we verify 10 blocks starting at that height.
             scan_range(
                 u32::from(birthday.height())..u32::from(birthday.height() + 10),
                 Verify,