zcash_client_sqlite: Initialize the scan queue as part of `init_block…

…s_table`

Fixes zcash#902
Fixes zcash#898

zcash_client_backend/src/data_api/scanning.rs

@@ -6,7 +6,9 @@ use zcash_primitives::consensus::BlockHeight;
 /// Scanning range priority levels.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
 pub enum ScanPriority {
-    /// Block ranges that have already been scanned have lowest priority.
+    /// Block ranges that are ignored have lowest priority.
+    Ignored,
+    /// Block ranges that have already been scanned will not be re-scanned.
     Scanned,
     /// Block ranges to be scanned to advance the fully-scanned height.
     Historic,

zcash_client_sqlite/src/wallet/init.rs

@@ -4,7 +4,7 @@ use incrementalmerkletree::Retention;
 use std::{collections::HashMap, fmt};
 use tracing::debug;
 
-use rusqlite::{self, types::ToSql};
+use rusqlite::{self, named_params};
 use schemer::{Migrator, MigratorError};
 use schemer_rusqlite::RusqliteAdapter;
 use secrecy::SecretVec;
@@ -13,18 +13,24 @@ use uuid::Uuid;
 
 use zcash_primitives::{
     block::BlockHash,
-    consensus::{self, BlockHeight},
+    consensus::{self, BlockHeight, NetworkUpgrade},
     merkle_tree::read_commitment_tree,
     sapling,
     transaction::components::amount::BalanceError,
     zip32::AccountId,
 };
 
-use zcash_client_backend::{data_api::SAPLING_SHARD_HEIGHT, keys::UnifiedFullViewingKey};
+use zcash_client_backend::{
+    data_api::{scanning::ScanPriority, SAPLING_SHARD_HEIGHT},
+    keys::UnifiedFullViewingKey,
+};
 
 use crate::{error::SqliteClientError, wallet, WalletDb, PRUNING_DEPTH, SAPLING_TABLES_PREFIX};
 
-use super::commitment_tree::{self, SqliteShardStore};
+use super::{
+    commitment_tree::{self, SqliteShardStore},
+    scanning::priority_code,
+};
 
 mod migrations;
 
@@ -309,15 +315,27 @@ pub fn init_blocks_table<P: consensus::Parameters>(
 
         wdb.conn.0.execute(
             "INSERT INTO blocks (height, hash, time, sapling_tree)
-            VALUES (?, ?, ?, ?)",
-            [
-                u32::from(height).to_sql()?,
-                hash.0.to_sql()?,
-                time.to_sql()?,
-                sapling_tree.to_sql()?,
+             VALUES (:height, :hash, :time, :sapling_tree)",
+            named_params![
+                ":height": u32::from(height),
+                ":hash": hash.0,
+                ":time": time,
+                ":sapling_tree": sapling_tree,
             ],
         )?;
 
+        if let Some(sapling_activation) = wdb.params.activation_height(NetworkUpgrade::Sapling) {
+            wdb.conn.0.execute(
+                "INSERT INTO scan_queue (block_range_start, block_range_end, priority)
+                 VALUES (:block_range_start, :block_range_end, :priority)",
+                named_params![
+                   ":block_range_start": u32::from(std::cmp::min(sapling_activation, height)),
+                   ":block_range_end": u32::from(height + 1),
+                   ":priority": priority_code(&ScanPriority::Ignored)
+                ],
+            )?;
+        }
+
         if let Some(nonempty_frontier) = block_end_tree.to_frontier().value() {
             debug!("Inserting frontier into ShardTree: {:?}", nonempty_frontier);
             let shard_store =
@@ -582,7 +600,7 @@ mod tests {
                         scan_queue.block_range_start <= prev_shard.subtree_end_height
                         AND (scan_queue.block_range_end - 1) >= shard.subtree_end_height
                     )
-                WHERE scan_queue.priority != {}",
+                WHERE scan_queue.priority > {}",
                 u32::from(tests::network().activation_height(NetworkUpgrade::Sapling).unwrap()),
                 priority_code(&ScanPriority::Scanned),
             ),

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

@@ -59,7 +59,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                         scan_queue.block_range_start <= prev_shard.subtree_end_height
                         AND (scan_queue.block_range_end - 1) >= shard.subtree_end_height
                     )
-                WHERE scan_queue.priority != {}",
+                WHERE scan_queue.priority > {}",
                 SAPLING_SHARD_HEIGHT,
                 SAPLING_SHARD_HEIGHT,
                 u32::from(self.params.activation_height(consensus::NetworkUpgrade::Sapling).unwrap()),

zcash_client_sqlite/src/wallet/scanning.rs

@@ -52,6 +52,7 @@ impl From<Insert> for Dominance {
 pub(crate) fn parse_priority_code(code: i64) -> Option<ScanPriority> {
     use ScanPriority::*;
     match code {
+        0 => Some(Ignored),
         10 => Some(Scanned),
         20 => Some(Historic),
         30 => Some(OpenAdjacent),
@@ -65,6 +66,7 @@ pub(crate) fn parse_priority_code(code: i64) -> Option<ScanPriority> {
 pub(crate) fn priority_code(priority: &ScanPriority) -> i64 {
     use ScanPriority::*;
     match priority {
+        Ignored => 0,
         Scanned => 10,
         Historic => 20,
         OpenAdjacent => 30,
@@ -745,7 +747,10 @@ mod tests {
         WalletCommitmentTrees, WalletRead, WalletWrite,
     };
     use zcash_primitives::{
-        block::BlockHash, consensus::BlockHeight, sapling::Node, transaction::components::Amount,
+        block::BlockHash,
+        consensus::{BlockHeight, NetworkUpgrade, Parameters},
+        sapling::Node,
+        transaction::components::Amount,
     };
 
     use crate::{
@@ -754,7 +759,10 @@ mod tests {
             self, fake_compact_block, init_test_accounts_table, insert_into_cache,
             sapling_activation_height, AddressType,
         },
-        wallet::{init::init_wallet_db, scanning::suggest_scan_ranges},
+        wallet::{
+            init::{init_blocks_table, init_wallet_db},
+            scanning::suggest_scan_ranges,
+        },
         BlockDb, WalletDb,
     };
 
@@ -1212,4 +1220,102 @@ mod tests {
             ]
         );
     }
+
+    #[test]
+    fn init_blocks_table_creates_scanned_range() {
+        use ScanPriority::*;
+
+        let data_file = NamedTempFile::new().unwrap();
+        let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap();
+        init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap();
+
+        let sap_active = db_data
+            .params
+            .activation_height(NetworkUpgrade::Sapling)
+            .unwrap();
+        // Initialise the blocks table. We use Canopy activation as an arbitrary birthday height
+        // that's greater than Sapling activation.
+        let birthday_height = db_data
+            .params
+            .activation_height(NetworkUpgrade::Canopy)
+            .unwrap();
+        init_blocks_table(
+            &mut db_data,
+            birthday_height,
+            BlockHash([1; 32]),
+            1,
+            &[0x0, 0x0, 0x0],
+        )
+        .unwrap();
+
+        let expected = vec![
+            // The range below the wallet's birthday height is ignored
+            scan_range(
+                u32::from(sap_active)..u32::from(birthday_height + 1),
+                Ignored,
+            ),
+        ];
+        assert_matches!(
+            suggest_scan_ranges(&db_data.conn, Ignored),
+            Ok(scan_ranges) if scan_ranges == expected
+        );
+
+        // Set up some shard history
+        db_data
+            .put_sapling_subtree_roots(
+                0,
+                &[
+                    // Add the end of a commitment tree below the wallet birthday. We currently
+                    // need to scan from this height up to the tip to make notes spendable, though
+                    // this should not be necessary as we have added a frontier that should
+                    // complete the left-hand side of the required shard; this can be fixed once we
+                    // have proper account birthdays.
+                    CommitmentTreeRoot::from_parts(
+                        birthday_height - 1000,
+                        // fake a hash, the value doesn't matter
+                        Node::empty_leaf(),
+                    ),
+                ],
+            )
+            .unwrap();
+
+        // Update the chain tip
+        let tip_height = db_data
+            .params
+            .activation_height(NetworkUpgrade::Nu5)
+            .unwrap();
+        db_data.update_chain_tip(tip_height).unwrap();
+
+        // 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.
+            scan_range(
+                u32::from(birthday_height)..u32::from(birthday_height + 10),
+                Verify,
+            ),
+            // The remainder of the shard after the verify segment is required in order to make
+            // notes spendable, so it has priority `ChainTip`
+            scan_range(
+                u32::from(birthday_height + 10)..u32::from(tip_height + 1),
+                ChainTip,
+            ),
+            // The remainder of the shard prior to the birthday height must be scanned because the
+            // wallet doesn't know that it already has enough data from the initial frontier to
+            // avoid having to scan this range.
+            scan_range(
+                u32::from(birthday_height - 1000)..u32::from(birthday_height),
+                ChainTip,
+            ),
+            // The range below the wallet's birthday height is ignored
+            scan_range(
+                u32::from(sap_active)..u32::from(birthday_height - 1000),
+                Ignored,
+            ),
+        ];
+
+        assert_matches!(
+            suggest_scan_ranges(&db_data.conn, Ignored),
+            Ok(scan_ranges) if scan_ranges == expected        );
+    }
 }