diff --git a/Cargo.lock b/Cargo.lock index 6e19d119cf..6d81ef639d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5901,7 +5901,7 @@ dependencies = [ [[package]] name = "zcash_client_sqlite" -version = "0.12.0" +version = "0.12.1" dependencies = [ "ambassador", "assert_matches", diff --git a/supply-chain/audits.toml b/supply-chain/audits.toml index 2a60ed2f0e..1c94159afd 100644 --- a/supply-chain/audits.toml +++ b/supply-chain/audits.toml @@ -271,6 +271,12 @@ who = "Daira-Emma Hopwood " criteria = "safe-to-run" delta = "0.9.9 -> 0.9.10" +[[audits.pczt]] +who = "Kris Nuttycombe " +criteria = "safe-to-deploy" +version = "0.0.0" +notes = "Initial empty crate release." + [[audits.pin-project-internal]] who = "Daira-Emma Hopwood " criteria = "safe-to-deploy" diff --git a/supply-chain/config.toml b/supply-chain/config.toml index 40f21f7503..3a91af1c79 100644 --- a/supply-chain/config.toml +++ b/supply-chain/config.toml @@ -31,6 +31,9 @@ audit-as-crates-io = true [policy.f4jumble] audit-as-crates-io = true +[policy.pczt] +audit-as-crates-io = true + [policy.zcash] audit-as-crates-io = true diff --git a/supply-chain/imports.lock b/supply-chain/imports.lock index 4713d88f43..02f393f119 100644 --- a/supply-chain/imports.lock +++ b/supply-chain/imports.lock @@ -245,8 +245,8 @@ user-login = "nuttycom" user-name = "Kris Nuttycombe" [[publisher.zcash_client_sqlite]] -version = "0.12.0" -when = "2024-10-04" +version = "0.12.1" +when = "2024-10-11" user-id = 169181 user-login = "nuttycom" user-name = "Kris Nuttycombe" diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 875f2ee747..ce743fc53c 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -534,10 +534,11 @@ impl WalletSummary { /// Progress is represented in terms of the ratio between notes scanned and the total /// number of notes added to the chain in the relevant window. This ratio should only /// be used to compute progress percentages, and the numerator and denominator should - /// not be treated as authoritative note counts. + /// not be treated as authoritative note counts. The denominator of this ratio is + /// guaranteed to be nonzero. /// - /// Returns `None` if the wallet is unable to determine the size of the note - /// commitment tree. + /// Returns `None` if the wallet has insufficient information to be able to determine + /// scan progress. pub fn scan_progress(&self) -> Option> { self.scan_progress } @@ -554,10 +555,12 @@ impl WalletSummary { /// Progress is represented in terms of the ratio between notes scanned and the total /// number of notes added to the chain in the relevant window. This ratio should only /// be used to compute progress percentages, and the numerator and denominator should - /// not be treated as authoritative note counts. + /// not be treated as authoritative note counts. Note that both the numerator and the + /// denominator of this ratio may be zero in the case that there is no recovery range + /// that need be scanned. /// - /// Returns `None` if the wallet is unable to determine the size of the note - /// commitment tree. + /// Returns `None` if the wallet has insufficient information to be able to determine + /// progress in scanning between the wallet birthday and the wallet recovery height. pub fn recovery_progress(&self) -> Option> { self.recovery_progress } @@ -1270,6 +1273,7 @@ pub trait WalletTest: InputSource + WalletRead { /// /// This type is opaque, and exists for use by tests defined in this crate. #[cfg(any(test, feature = "test-dependencies"))] +#[allow(dead_code)] #[derive(Clone, Debug)] pub struct OutputOfSentTx { value: NonNegativeAmount, diff --git a/zcash_client_backend/src/data_api/testing/pool.rs b/zcash_client_backend/src/data_api/testing/pool.rs index be0476cd56..3bf8715640 100644 --- a/zcash_client_backend/src/data_api/testing/pool.rs +++ b/zcash_client_backend/src/data_api/testing/pool.rs @@ -881,20 +881,16 @@ pub fn spend_fails_on_unverified_notes( NonNegativeAmount::ZERO ); - // The account is configured without a recover-until height, so is by definition - // fully recovered, and we count 1 per pool for both numerator and denominator. - let fully_recovered = { - let n = 1; - #[cfg(feature = "orchard")] - let n = n * 2; - Some(Ratio::new(n, n)) - }; + // If none of the wallet's accounts have a recover-until height, then there + // is no recovery phase for the wallet, and therefore the denominator in the + // resulting ratio (the number of notes in the recovery range) is zero. + let no_recovery = Some(Ratio::new(0, 0)); // Wallet is fully scanned let summary = st.get_wallet_summary(1); assert_eq!( summary.as_ref().and_then(|s| s.recovery_progress()), - fully_recovered, + no_recovery, ); assert_eq!( summary.and_then(|s| s.scan_progress()), @@ -915,7 +911,7 @@ pub fn spend_fails_on_unverified_notes( let summary = st.get_wallet_summary(1); assert_eq!( summary.as_ref().and_then(|s| s.recovery_progress()), - fully_recovered + no_recovery ); assert_eq!( summary.and_then(|s| s.scan_progress()), diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 88350e2c0f..c013d0a028 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -11,6 +11,16 @@ and this library adheres to Rust's notion of - Exposed `AccountId::from_u32` and `AccountId::as_u32` conversions under the `unstable` feature flag. +## [0.12.1] - 2024-10-10 + +### Fixed +- An error in scan progress computation was fixed. As part of this fix, wallet + summary information is now only returned in the case that some note + commitment tree size information can be determined, either from subtree root + download or from downloaded block data. NOTE: The recovery progress ratio may + be present as `0:0` in the case that the recovery range contains no notes; + this was not adequately documented in the previous release. + ## [0.12.0] - 2024-10-04 ### Added diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 38a251775e..e1db2d2130 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "zcash_client_sqlite" description = "An SQLite-based Zcash light client" -version = "0.12.0" +version = "0.12.1" authors = [ "Jack Grigg ", "Kris Nuttycombe " diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index cdc7dc57d5..0453989dd0 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -128,7 +128,7 @@ pub mod error; pub mod wallet; use wallet::{ commitment_tree::{self, put_shard_roots}, - SubtreeScanProgress, + SubtreeProgressEstimator, }; #[cfg(test)] @@ -481,7 +481,7 @@ impl, P: consensus::Parameters> WalletRead for W &self.conn.borrow().unchecked_transaction()?, &self.params, min_confirmations, - &SubtreeScanProgress, + &SubtreeProgressEstimator, ) } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index df9d7b3ff3..388433b4f5 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -806,20 +806,34 @@ pub(crate) fn get_derived_account( #[derive(Debug)] pub(crate) struct Progress { - scan: Option>, - recover: Option>, + scan: Ratio, + recovery: Option>, } -pub(crate) trait ScanProgress { +impl Progress { + pub(crate) fn new(scan: Ratio, recovery: Option>) -> Self { + Self { scan, recovery } + } + + pub(crate) fn scan(&self) -> Ratio { + self.scan + } + + pub(crate) fn recovery(&self) -> Option> { + self.recovery + } +} + +pub(crate) trait ProgressEstimator { fn sapling_scan_progress( &self, conn: &rusqlite::Connection, params: &P, birthday_height: BlockHeight, recover_until_height: Option, - fully_scanned_height: BlockHeight, + fully_scanned_height: Option, chain_tip_height: BlockHeight, - ) -> Result; + ) -> Result, SqliteClientError>; #[cfg(feature = "orchard")] fn orchard_scan_progress( @@ -828,31 +842,221 @@ pub(crate) trait ScanProgress { params: &P, birthday_height: BlockHeight, recover_until_height: Option, - fully_scanned_height: BlockHeight, + fully_scanned_height: Option, chain_tip_height: BlockHeight, - ) -> Result; + ) -> Result, SqliteClientError>; } #[derive(Debug)] -pub(crate) struct SubtreeScanProgress; +pub(crate) struct SubtreeProgressEstimator; + +fn table_constants( + shielded_protocol: ShieldedProtocol, +) -> Result<(&'static str, &'static str, u8), SqliteClientError> { + match shielded_protocol { + ShieldedProtocol::Sapling => Ok(( + SAPLING_TABLES_PREFIX, + "sapling_output_count", + SAPLING_SHARD_HEIGHT, + )), + #[cfg(feature = "orchard")] + ShieldedProtocol::Orchard => Ok(( + ORCHARD_TABLES_PREFIX, + "orchard_action_count", + ORCHARD_SHARD_HEIGHT, + )), + #[cfg(not(feature = "orchard"))] + ShieldedProtocol::Orchard => Err(SqliteClientError::UnsupportedPoolType(PoolType::ORCHARD)), + } +} + +fn estimate_tree_size( + conn: &rusqlite::Connection, + params: &P, + shielded_protocol: ShieldedProtocol, + pool_activation_height: BlockHeight, + chain_tip_height: BlockHeight, +) -> Result, SqliteClientError> { + let (table_prefix, _, shard_height) = table_constants(shielded_protocol)?; + + // Estimate the size of the tree by linear extrapolation from available + // data closest to the chain tip. + // + // - If we have scanned blocks within the incomplete subtree, and we know + // the tree size for the end of the most recent scanned range, then we + // extrapolate from the start of the incomplete subtree: + // + // subtree + // / \ + // / \ + // / \ + // / \ + // |<--------->| | + // | scanned | tip + // last_scanned + // + // + // subtree + // / \ + // / \ + // / \ + // / \ + // |<------->| | + // | scanned | tip + // last_scanned + // + // - If we don't have scanned blocks within the incomplete subtree, or we + // don't know the tree size, then we extrapolate from the block-width of + // the last complete subtree. + // + // This avoids having a sharp discontinuity in the progress percentages + // shown to users, and gets more accurate the closer to the chain tip we + // have scanned. + // + // TODO: it would be nice to be able to reliably have the size of the + // commitment tree at the chain tip without having to have scanned that + // block. + + // Get the tree size at the last scanned height, if known. + let last_scanned = block_max_scanned(conn, params)?.and_then(|last_scanned| { + match shielded_protocol { + ShieldedProtocol::Sapling => last_scanned.sapling_tree_size(), + #[cfg(feature = "orchard")] + ShieldedProtocol::Orchard => last_scanned.orchard_tree_size(), + #[cfg(not(feature = "orchard"))] + ShieldedProtocol::Orchard => None, + } + .map(|tree_size| (last_scanned.block_height(), u64::from(tree_size))) + }); + + // Get the last completed subtree. + let last_completed_subtree = conn + .query_row( + &format!( + "SELECT shard_index, subtree_end_height + FROM {table_prefix}_tree_shards + WHERE subtree_end_height IS NOT NULL + ORDER BY shard_index DESC + LIMIT 1" + ), + [], + |row| { + Ok(( + incrementalmerkletree::Address::from_parts( + incrementalmerkletree::Level::new(shard_height), + row.get(0)?, + ), + BlockHeight::from_u32(row.get(1)?), + )) + }, + ) + // `None` if we have no subtree roots yet. + .optional()?; + + let result = if let Some((last_completed_subtree, last_completed_subtree_end)) = + last_completed_subtree + { + // If we know the tree size at the last scanned height, and that + // height is within the incomplete subtree, extrapolate. + let tip_tree_size = last_scanned.and_then(|(last_scanned, last_scanned_tree_size)| { + (last_scanned > last_completed_subtree_end) + .then(|| { + let scanned_notes = last_scanned_tree_size + - u64::from(last_completed_subtree.position_range_end()); + let scanned_range = u64::from(last_scanned - last_completed_subtree_end); + let unscanned_range = u64::from(chain_tip_height - last_scanned); + + (scanned_notes * unscanned_range) + .checked_div(scanned_range) + .map(|extrapolated_unscanned_notes| { + last_scanned_tree_size + extrapolated_unscanned_notes + }) + }) + .flatten() + }); + + if let Some(tree_size) = tip_tree_size { + Some(tree_size) + } else if let Some(second_to_last_completed_subtree_end) = last_completed_subtree + .index() + .checked_sub(1) + .and_then(|subtree_index| { + conn.query_row( + &format!( + "SELECT subtree_end_height + FROM {table_prefix}_tree_shards + WHERE shard_index = :shard_index" + ), + named_params! {":shard_index": subtree_index}, + |row| Ok(row.get::<_, Option<_>>(0)?.map(BlockHeight::from_u32)), + ) + .transpose() + }) + .transpose()? + { + let notes_in_complete_subtrees = u64::from(last_completed_subtree.position_range_end()); + + let subtree_notes = 1 << shard_height; + let subtree_range = + u64::from(last_completed_subtree_end - second_to_last_completed_subtree_end); + let unscanned_range = u64::from(chain_tip_height - last_completed_subtree_end); + + (subtree_notes * unscanned_range) + .checked_div(subtree_range) + .map(|extrapolated_incomplete_subtree_notes| { + notes_in_complete_subtrees + extrapolated_incomplete_subtree_notes + }) + } else { + // There's only one completed subtree; its start height must + // be the activation height for this shielded protocol. + let subtree_notes = 1 << shard_height; + + let subtree_range = u64::from(last_completed_subtree_end - pool_activation_height); + let unscanned_range = u64::from(chain_tip_height - last_completed_subtree_end); + + (subtree_notes * unscanned_range) + .checked_div(subtree_range) + .map(|extrapolated_incomplete_subtree_notes| { + subtree_notes + extrapolated_incomplete_subtree_notes + }) + } + } else { + // If there are no completed subtrees, but we have scanned some blocks, we can still + // interpolate based upon the tree size as of the last scanned block. Here, since we + // don't have any subtree data to draw on, we will interpolate based on the number of + // blocks since the pool activation height + last_scanned.and_then(|(last_scanned_height, last_scanned_tree_size)| { + let subtree_range = u64::from(last_scanned_height - pool_activation_height); + let unscanned_range = u64::from(chain_tip_height - last_scanned_height); + + (last_scanned_tree_size * unscanned_range) + .checked_div(subtree_range) + .map(|extrapolated_incomplete_subtree_notes| { + last_scanned_tree_size + extrapolated_incomplete_subtree_notes + }) + }) + }; + + Ok(result) +} #[allow(clippy::too_many_arguments)] fn subtree_scan_progress( conn: &rusqlite::Connection, params: &P, - table_prefix: &'static str, - output_count_col: &'static str, - shard_height: u8, + shielded_protocol: ShieldedProtocol, pool_activation_height: BlockHeight, birthday_height: BlockHeight, recover_until_height: Option, - fully_scanned_height: BlockHeight, + fully_scanned_height: Option, chain_tip_height: BlockHeight, -) -> Result { - let mut stmt_scanned_count_between = conn.prepare_cached(&format!( +) -> Result, SqliteClientError> { + let (table_prefix, output_count_col, shard_height) = table_constants(shielded_protocol)?; + + let mut stmt_scanned_count_until = conn.prepare_cached(&format!( "SELECT SUM({output_count_col}) FROM blocks - WHERE :start_height <= height AND height <= :end_height", + WHERE :start_height <= height AND height < :end_height", ))?; let mut stmt_scanned_count_from = conn.prepare_cached(&format!( "SELECT SUM({output_count_col}) @@ -870,32 +1074,32 @@ fn subtree_scan_progress( WHERE height = :height", ))?; - if fully_scanned_height == chain_tip_height { + if fully_scanned_height == Some(chain_tip_height) { // Compute the total blocks scanned since the wallet birthday on either side of // the recover-until height. - let recover = recover_until_height - .map(|end_height| { - stmt_scanned_count_between.query_row( - named_params! { - ":start_height": u32::from(birthday_height), - ":end_height": u32::from(end_height), - }, - |row| { - let recovered = row.get::<_, Option>(0)?; - Ok(recovered.map(|n| Ratio::new(n, n))) - }, - ) - }) - .transpose()? - // If none of the wallet's accounts have a recover-until height, then we can't - // (yet) distinguish general scanning from recovery, so treat the wallet as - // fully recovered. - .unwrap_or_else(|| Some(Ratio::new(1, 1))); + let recover = match recover_until_height { + Some(end_height) => stmt_scanned_count_until.query_row( + named_params! { + ":start_height": u32::from(birthday_height), + ":end_height": u32::from(end_height), + }, + |row| { + let recovered = row.get::<_, Option>(0)?; + Ok(recovered.map(|n| Ratio::new(n, n))) + }, + )?, + None => { + // If none of the wallet's accounts have a recover-until height, then there + // is no recovery phase for the wallet, and therefore the denominator in the + // resulting ratio (the number of notes in the recovery range) is zero. + Some(Ratio::new(0, 0)) + } + }; + let scan = stmt_scanned_count_from.query_row( named_params! { ":start_height": u32::from( - recover_until_height.map(|h| h + 1) - .unwrap_or(birthday_height) + recover_until_height.unwrap_or(birthday_height) ), }, |row| { @@ -903,53 +1107,81 @@ fn subtree_scan_progress( Ok(scanned.map(|n| Ratio::new(n, n))) }, )?; - Ok(Progress { scan, recover }) + + Ok(scan.map(|scan| Progress::new(scan, recover))) } else { + // In case we didn't have information about the tree size at the recover-until + // height, get the tree size from a nearby subtree. It's fine for this to be + // approximate; it just shifts the boundary between scan and recover progress. + let mut get_tree_size_near = |as_of: BlockHeight| { + let size_from_blocks = stmt_start_tree_size + .query_row(named_params![":start_height": u32::from(as_of)], |row| { + row.get::<_, Option>(0) + }) + .optional()? + .flatten(); + + let size_from_subtree_roots = || { + conn.query_row( + &format!( + "SELECT MIN(shard_index) + FROM {table_prefix}_tree_shards + WHERE subtree_end_height >= :start_height + OR subtree_end_height IS NULL", + ), + named_params! { + ":start_height": u32::from(as_of), + }, + |row| { + let min_tree_size = row + .get::<_, Option>(0)? + .map(|min_idx| min_idx << shard_height); + Ok(min_tree_size) + }, + ) + .optional() + .map(|opt| opt.flatten()) + }; + + match size_from_blocks { + Some(size) => Ok(Some(size)), + None => size_from_subtree_roots(), + } + }; + // Get the starting note commitment tree size from the wallet birthday, or failing that // from the blocks table. - let start_size = conn + let birthday_size = match conn .query_row( &format!( "SELECT birthday_{table_prefix}_tree_size - FROM accounts - WHERE birthday_height = :birthday_height", + FROM accounts + WHERE birthday_height = :birthday_height", ), named_params![":birthday_height": u32::from(birthday_height)], |row| row.get::<_, Option>(0), ) .optional()? .flatten() - .map(Ok) - .or_else(|| { - stmt_start_tree_size - .query_row( - named_params![":start_height": u32::from(birthday_height)], - |row| row.get::<_, Option>(0), - ) - .optional() - .map(|opt| opt.flatten()) - .transpose() - }) - .transpose()?; + { + Some(tree_size) => Some(tree_size), + // If we don't have an explicit birthday tree size, find something nearby. + None => get_tree_size_near(birthday_height)?, + }; - // Get the note commitment tree size as of the end of the recover-until height. - let recover_until_size = recover_until_height - .map(|end_height| { - stmt_start_tree_size - .query_row( - named_params![":start_height": u32::from(end_height + 1)], - |row| row.get::<_, Option>(0), - ) - .optional() - .map(|opt| opt.flatten()) - }) + // Get the note commitment tree size as of the start of the recover-until height. + // The outer option indicates whether or not we have recover-until height information; + // the inner option indicates whether or not we were able to obtain a tree size given + // the recover-until height. + let recover_until_size: Option> = recover_until_height + // Find a tree size near to the recover-until height + .map(get_tree_size_near) .transpose()?; - // Count the total outputs scanned so far on the birthday side of the - // recover-until height. + // Count the total outputs scanned so far on the birthday side of the recover-until height. let recovered_count = recover_until_height .map(|end_height| { - stmt_scanned_count_between.query_row( + stmt_scanned_count_until.query_row( named_params! { ":start_height": u32::from(birthday_height), ":end_height": u32::from(end_height), @@ -959,32 +1191,8 @@ fn subtree_scan_progress( }) .transpose()?; - // In case we didn't have information about the tree size at the recover-until - // height, get the tree size from a nearby subtree. It's fine for this to be - // approximate; it just shifts the boundary between scan and recover progress. - let min_tree_size = conn - .query_row( - &format!( - "SELECT MIN(shard_index) - FROM {table_prefix}_tree_shards - WHERE subtree_end_height > :start_height - OR subtree_end_height IS NULL", - ), - named_params! { - ":start_height": u32::from(recover_until_height.unwrap_or(birthday_height) + 1), - }, - |row| { - let min_tree_size = row - .get::<_, Option>(0)? - .map(|min_idx| min_idx << shard_height); - Ok(min_tree_size) - }, - ) - .optional()? - .flatten(); - - // If we've scanned the block at the chain tip, we know how many notes are - // currently in the tree. + // If we've scanned the block at the chain tip, we know how many notes are currently in the + // tree. let tip_tree_size = match stmt_end_tree_size_at .query_row( named_params! {":height": u32::from(chain_tip_height)}, @@ -994,208 +1202,48 @@ fn subtree_scan_progress( .flatten() { Some(tree_size) => Some(tree_size), - None => { - // Estimate the size of the tree by linear extrapolation from available - // data closest to the chain tip. - // - // - If we have scanned blocks within the incomplete subtree, and we know - // the tree size for the end of the most recent scanned range, then we - // extrapolate from the start of the incomplete subtree: - // - // subtree - // / \ - // / \ - // / \ - // / \ - // |<--------->| | - // | scanned | tip - // last_scanned - // - // - // subtree - // / \ - // / \ - // / \ - // / \ - // |<------->| | - // | scanned | tip - // last_scanned - // - // - If we don't have scanned blocks within the incomplete subtree, or we - // don't know the tree size, then we extrapolate from the block-width of - // the last complete subtree. - // - // This avoids having a sharp discontinuity in the progress percentages - // shown to users, and gets more accurate the closer to the chain tip we - // have scanned. - // - // TODO: it would be nice to be able to reliably have the size of the - // commitment tree at the chain tip without having to have scanned that - // block. - - // Get the tree size at the last scanned height, if known. - let last_scanned = block_max_scanned(conn, params)?.and_then(|last_scanned| { - match table_prefix { - SAPLING_TABLES_PREFIX => last_scanned.sapling_tree_size(), - #[cfg(feature = "orchard")] - ORCHARD_TABLES_PREFIX => last_scanned.orchard_tree_size(), - _ => unreachable!(), - } - .map(|tree_size| (last_scanned.block_height(), u64::from(tree_size))) - }); - - // Get the last completed subtree. - let last_completed_subtree = conn - .query_row( - &format!( - "SELECT shard_index, subtree_end_height - FROM {table_prefix}_tree_shards - WHERE subtree_end_height IS NOT NULL - ORDER BY shard_index DESC - LIMIT 1" - ), - [], - |row| { - Ok(( - incrementalmerkletree::Address::from_parts( - incrementalmerkletree::Level::new(shard_height), - row.get(0)?, - ), - BlockHeight::from_u32(row.get(1)?), - )) - }, - ) - // `None` if we have no subtree roots yet. - .optional()?; - - if let Some((last_completed_subtree, last_completed_subtree_end)) = - last_completed_subtree - { - // If we know the tree size at the last scanned height, and that - // height is within the incomplete subtree, extrapolate. - let tip_tree_size = - last_scanned.and_then(|(last_scanned, last_scanned_tree_size)| { - (last_scanned > last_completed_subtree_end) - .then(|| { - let scanned_notes = last_scanned_tree_size - - u64::from(last_completed_subtree.position_range_end()); - let scanned_range = - u64::from(last_scanned - last_completed_subtree_end); - let unscanned_range = - u64::from(chain_tip_height - last_scanned); - - (scanned_notes * unscanned_range) - .checked_div(scanned_range) - .map(|extrapolated_unscanned_notes| { - last_scanned_tree_size + extrapolated_unscanned_notes - }) - }) - .flatten() - }); - - if let Some(tree_size) = tip_tree_size { - Some(tree_size) - } else if let Some(second_to_last_completed_subtree_end) = - last_completed_subtree - .index() - .checked_sub(1) - .and_then(|subtree_index| { - conn.query_row( - &format!( - "SELECT subtree_end_height - FROM {table_prefix}_tree_shards - WHERE shard_index = :shard_index" - ), - named_params! {":shard_index": subtree_index}, - |row| { - Ok(row.get::<_, Option<_>>(0)?.map(BlockHeight::from_u32)) - }, - ) - .transpose() - }) - .transpose()? - { - let notes_in_complete_subtrees = - u64::from(last_completed_subtree.position_range_end()); - - let subtree_notes = 1 << shard_height; - let subtree_range = u64::from( - last_completed_subtree_end - second_to_last_completed_subtree_end, - ); - let unscanned_range = - u64::from(chain_tip_height - last_completed_subtree_end); - - (subtree_notes * unscanned_range) - .checked_div(subtree_range) - .map(|extrapolated_incomplete_subtree_notes| { - notes_in_complete_subtrees + extrapolated_incomplete_subtree_notes - }) - } else { - // There's only one completed subtree; its start height must - // be the activation height for this shielded protocol. - let subtree_notes = 1 << shard_height; - - let subtree_range = - u64::from(last_completed_subtree_end - pool_activation_height); - let unscanned_range = - u64::from(chain_tip_height - last_completed_subtree_end); - - (subtree_notes * unscanned_range) - .checked_div(subtree_range) - .map(|extrapolated_incomplete_subtree_notes| { - subtree_notes + extrapolated_incomplete_subtree_notes - }) - } - } else { - // We don't have subtree information, so give up. We'll get it soon. - None - } - } + None => estimate_tree_size( + conn, + params, + shielded_protocol, + pool_activation_height, + chain_tip_height, + )?, }; let recover = recovered_count .zip(recover_until_size) .map(|(recovered, end_size)| { - start_size - .or(min_tree_size) - .zip(end_size) - .map(|(start_size, end_size)| { - Ratio::new(recovered.unwrap_or(0), end_size - start_size) - }) + birthday_size.zip(end_size).map(|(start_size, end_size)| { + Ratio::new(recovered.unwrap_or(0), end_size - start_size) + }) }) - // If none of the wallet's accounts have a recover-until height, then we can't - // (yet) distinguish general scanning from recovery, so treat the wallet as - // fully recovered. - .unwrap_or_else(|| Some(Ratio::new(1, 1))); - - let scan = if recover_until_height.map_or(false, |h| h == chain_tip_height) { - // The wallet was likely just created for a recovery from seed, or with an - // imported viewing key. In this state, it is fully synced as there is nothing - // else for us to scan beyond `recover_until_height`; ensure we show 100% - // instead of 0%. - Some(Ratio::new(1, 1)) - } else { + // If none of the wallet's accounts have a recover-until height, then there + // is no recovery phase for the wallet, and therefore the denominator in the + // resulting ratio (the number of notes in the recovery range) is zero. + .unwrap_or_else(|| Some(Ratio::new(0, 0))); + + let scan = { // Count the total outputs scanned so far on the chain tip side of the // recover-until height. let scanned_count = stmt_scanned_count_from.query_row( - named_params![":start_height": u32::from(recover_until_height.unwrap_or(birthday_height) + 1)], + named_params![":start_height": u32::from(recover_until_height.unwrap_or(birthday_height))], |row| row.get::<_, Option>(0), )?; recover_until_size - .unwrap_or(start_size) - .or(min_tree_size) + .unwrap_or(birthday_size) .zip(tip_tree_size) .map(|(start_size, tip_tree_size)| { Ratio::new(scanned_count.unwrap_or(0), tip_tree_size - start_size) }) }; - Ok(Progress { scan, recover }) + Ok(scan.map(|scan| Progress::new(scan, recover))) } } -impl ScanProgress for SubtreeScanProgress { +impl ProgressEstimator for SubtreeProgressEstimator { #[tracing::instrument(skip(conn, params))] fn sapling_scan_progress( &self, @@ -1203,15 +1251,13 @@ impl ScanProgress for SubtreeScanProgress { params: &P, birthday_height: BlockHeight, recover_until_height: Option, - fully_scanned_height: BlockHeight, + fully_scanned_height: Option, chain_tip_height: BlockHeight, - ) -> Result { + ) -> Result, SqliteClientError> { subtree_scan_progress( conn, params, - SAPLING_TABLES_PREFIX, - "sapling_output_count", - SAPLING_SHARD_HEIGHT, + ShieldedProtocol::Sapling, params .activation_height(NetworkUpgrade::Sapling) .expect("Sapling activation height must be available."), @@ -1230,15 +1276,13 @@ impl ScanProgress for SubtreeScanProgress { params: &P, birthday_height: BlockHeight, recover_until_height: Option, - fully_scanned_height: BlockHeight, + fully_scanned_height: Option, chain_tip_height: BlockHeight, - ) -> Result { + ) -> Result, SqliteClientError> { subtree_scan_progress( conn, params, - ORCHARD_TABLES_PREFIX, - "orchard_action_count", - ORCHARD_SHARD_HEIGHT, + ShieldedProtocol::Orchard, params .activation_height(NetworkUpgrade::Nu5) .expect("NU5 activation height must be available."), @@ -1262,7 +1306,7 @@ pub(crate) fn get_wallet_summary( tx: &rusqlite::Transaction, params: &P, min_confirmations: u32, - progress: &impl ScanProgress, + progress: &impl ProgressEstimator, ) -> Result>, SqliteClientError> { let chain_tip_height = match chain_tip_height(tx)? { Some(h) => h, @@ -1271,12 +1315,16 @@ pub(crate) fn get_wallet_summary( } }; - let birthday_height = - wallet_birthday(tx)?.expect("If a scan range exists, we know the wallet birthday."); + let birthday_height = match wallet_birthday(tx)? { + Some(h) => h, + None => { + return Ok(None); + } + }; + let recover_until_height = recover_until_height(tx)?; - let fully_scanned_height = - block_fully_scanned(tx, params)?.map_or(birthday_height - 1, |m| m.block_height()); + let fully_scanned_height = block_fully_scanned(tx, params)?.map(|m| m.block_height()); let summary_height = (chain_tip_height + 1).saturating_sub(std::cmp::max(min_confirmations, 1)); let sapling_progress = progress.sapling_scan_progress( @@ -1298,34 +1346,37 @@ pub(crate) fn get_wallet_summary( chain_tip_height, )?; #[cfg(not(feature = "orchard"))] - let orchard_progress: Progress = Progress { - scan: None, - recover: None, - }; + let orchard_progress: Option = None; // Treat Sapling and Orchard outputs as having the same cost to scan. - let scan_progress = sapling_progress - .scan - .zip(orchard_progress.scan) + let progress = sapling_progress + .as_ref() + .zip(orchard_progress.as_ref()) .map(|(s, o)| { - Ratio::new( - s.numerator() + o.numerator(), - s.denominator() + o.denominator(), - ) - }) - .or(sapling_progress.scan) - .or(orchard_progress.scan); - let recover_progress = sapling_progress - .recover - .zip(orchard_progress.recover) - .map(|(s, o)| { - Ratio::new( - s.numerator() + o.numerator(), - s.denominator() + o.denominator(), + Progress::new( + Ratio::new( + s.scan().numerator() + o.scan().numerator(), + s.scan().denominator() + o.scan().denominator(), + ), + s.recovery() + .zip(o.recovery()) + .map(|(s, o)| { + Ratio::new( + s.numerator() + o.numerator(), + s.denominator() + o.denominator(), + ) + }) + .or_else(|| s.recovery()) + .or_else(|| o.recovery()), ) }) - .or(sapling_progress.recover) - .or(orchard_progress.recover); + .or(sapling_progress) + .or(orchard_progress); + + let progress = match progress { + Some(p) => p, + None => return Ok(None), + }; let mut stmt_accounts = tx.prepare_cached("SELECT id FROM accounts")?; let mut account_balances = stmt_accounts @@ -1546,9 +1597,9 @@ pub(crate) fn get_wallet_summary( let summary = WalletSummary::new( account_balances, chain_tip_height, - fully_scanned_height, - scan_progress, - recover_progress, + fully_scanned_height.unwrap_or(birthday_height - 1), + Some(progress.scan), + progress.recovery, next_sapling_subtree_index, #[cfg(feature = "orchard")] next_orchard_subtree_index, diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index af182003bc..594c9c32ca 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -1292,22 +1292,21 @@ pub(crate) mod tests { let birthday = account.birthday(); let sap_active = st.sapling_activation_height(); - // The account is configured without a recover-until height, so is by definition - // fully recovered, and we count 1 per pool for both numerator and denominator. - let fully_recovered = { - let n = 1; - #[cfg(feature = "orchard")] - let n = n * 2; - Some(Ratio::new(n, n)) - }; + // If none of the wallet's accounts have a recover-until height, then there + // is no recovery phase for the wallet, and therefore the denominator in the + // resulting ratio (the number of notes in the recovery range) is zero. + let no_recovery = Some(Ratio::new(0, 0)); // We have scan ranges and a subtree, but have scanned no blocks. let summary = st.get_wallet_summary(1); assert_eq!( summary.as_ref().and_then(|s| s.recovery_progress()), - fully_recovered, + no_recovery, + ); + assert_matches!( + summary.and_then(|s| s.scan_progress()), + Some(progress) if progress.numerator() == &0 ); - assert_eq!(summary.and_then(|s| s.scan_progress()), None); // Set up prior chain state. This simulates us having imported a wallet // with a birthday 520 blocks below the chain tip. @@ -1347,7 +1346,7 @@ pub(crate) mod tests { assert_eq!( summary.as_ref().and_then(|s| s.recovery_progress()), - fully_recovered, + no_recovery ); // Progress denominator depends on which pools are enabled (which changes the