Merge pull request #903 from nuttycom/sbs/scan_queue_init

zcash_client_sqlite: Initialize the scan queue as part of `init_blocks_table`

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,27 @@ 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, ScanRange},
+        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::insert_queue_entries,
+};
 
 mod migrations;
 
@@ -309,15 +318,32 @@ 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) {
+            let scan_range_start = std::cmp::min(sapling_activation, height);
+            let scan_range_end = height + 1;
+            debug!(
+                "Setting ignored block range {}..{}",
+                scan_range_start, scan_range_end
+            );
+            insert_queue_entries(
+                wdb.conn.0,
+                Some(ScanRange::from_parts(
+                    scan_range_start..scan_range_end,
+                    ScanPriority::Ignored,
+                ))
+                .iter(),
+            )?;
+        }
+
         if let Some(nonempty_frontier) = block_end_tree.to_frontier().value() {
             debug!("Inserting frontier into ShardTree: {:?}", nonempty_frontier);
             let shard_store =
@@ -582,7 +608,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.rs

@@ -56,7 +56,9 @@ pub(super) fn all_migrations<P: consensus::Parameters + 'static>(
         Box::new(add_transaction_views::Migration),
         Box::new(v_transactions_net::Migration),
         Box::new(received_notes_nullable_nf::Migration),
-        Box::new(shardtree_support::Migration),
+        Box::new(shardtree_support::Migration {
+            params: params.clone(),
+        }),
         Box::new(nullifier_map::Migration),
         Box::new(sapling_memo_consistency::Migration {
             params: params.clone(),

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

@@ -17,7 +17,7 @@ use zcash_client_backend::data_api::{
     SAPLING_SHARD_HEIGHT,
 };
 use zcash_primitives::{
-    consensus::BlockHeight,
+    consensus::{self, BlockHeight, NetworkUpgrade},
     merkle_tree::{read_commitment_tree, read_incremental_witness},
     sapling,
 };
@@ -39,9 +39,11 @@ pub(super) const MIGRATION_ID: Uuid = Uuid::from_fields(
     b"\x8b\xe5\xf5\x12\xbc\xce\x6c\xbf",
 );
 
-pub(super) struct Migration;
+pub(super) struct Migration<P> {
+    pub(super) params: P,
+}
 
-impl schemer::Migration for Migration {
+impl<P> schemer::Migration for Migration<P> {
     fn id(&self) -> Uuid {
         MIGRATION_ID
     }
@@ -57,7 +59,7 @@ impl schemer::Migration for Migration {
     }
 }
 
-impl RusqliteMigration for Migration {
+impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
     type Error = WalletMigrationError;
 
     fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> {
@@ -262,13 +264,17 @@ impl RusqliteMigration for Migration {
         if let Some((start, end)) = block_height_extrema {
             // `ScanRange` uses an exclusive upper bound.
             let chain_end = end + 1;
+            let ignored_range =
+                self.params
+                    .activation_height(NetworkUpgrade::Sapling)
+                    .map(|sapling_activation| {
+                        let ignored_range_start = std::cmp::min(sapling_activation, start);
+                        ScanRange::from_parts(ignored_range_start..start, ScanPriority::Ignored)
+                    });
+            let scanned_range = ScanRange::from_parts(start..chain_end, ScanPriority::Scanned);
             insert_queue_entries(
                 transaction,
-                Some(ScanRange::from_parts(
-                    start..chain_end,
-                    ScanPriority::Scanned,
-                ))
-                .iter(),
+                ignored_range.iter().chain(Some(scanned_range).iter()),
             )?;
         }
 

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,103 @@ mod tests {
             ]
         );
     }
+
+    #[test]
+    fn init_blocks_table_creates_ignored_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 up to and including 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
+        );
+    }
 }