diff --git a/zcash_client_backend/src/data_api/scanning.rs b/zcash_client_backend/src/data_api/scanning.rs index bb91ba35a..dcd9e0ff6 100644 --- a/zcash_client_backend/src/data_api/scanning.rs +++ b/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, diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index da9657ca4..1a7f24cc9 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/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( 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), ), diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index 834960ec4..6fbaea7e2 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -56,7 +56,9 @@ pub(super) fn all_migrations( 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(), diff --git a/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs b/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs index 64e1c42fc..e35d01805 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/shardtree_support.rs +++ b/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

{ + pub(super) params: P, +} -impl schemer::Migration for Migration { +impl

schemer::Migration for Migration

{ fn id(&self) -> Uuid { MIGRATION_ID } @@ -57,7 +59,7 @@ impl schemer::Migration for Migration { } } -impl RusqliteMigration for Migration { +impl RusqliteMigration for Migration

{ 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()), )?; } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/v_sapling_shard_unscanned_ranges.rs b/zcash_client_sqlite/src/wallet/init/migrations/v_sapling_shard_unscanned_ranges.rs index a038057d0..770ecddc5 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/v_sapling_shard_unscanned_ranges.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/v_sapling_shard_unscanned_ranges.rs @@ -59,7 +59,7 @@ impl RusqliteMigration for Migration

{ 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()), diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index 83781837a..719d6d082 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -52,6 +52,7 @@ impl From for Dominance { pub(crate) fn parse_priority_code(code: i64) -> Option { 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 { 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 + ); + } }