diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 875f2ee74..adbb6a941 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1270,6 +1270,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 be0476cd5..3bf871564 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/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index df9d7b3ff..1271f3dea 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -836,23 +836,199 @@ pub(crate) trait ScanProgress { #[derive(Debug)] pub(crate) struct SubtreeScanProgress; +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()?; + + 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 { + Ok(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); + + Ok((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); + + Ok((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. + Ok(None) + } +} + #[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, chain_tip_height: BlockHeight, ) -> Result { - let mut stmt_scanned_count_between = conn.prepare_cached(&format!( + 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}) @@ -875,7 +1051,7 @@ fn subtree_scan_progress( // the recover-until height. let recover = 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), @@ -887,15 +1063,15 @@ fn subtree_scan_progress( ) }) .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))); + // 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 = 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| { @@ -905,9 +1081,46 @@ fn subtree_scan_progress( )?; Ok(Progress { 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| { + 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() + .or_else(|| { + 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()) + .transpose() + }) + .transpose() + }; + // 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 = conn .query_row( &format!( "SELECT birthday_{table_prefix}_tree_size @@ -920,36 +1133,20 @@ fn subtree_scan_progress( .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() - }) + // If we don't have an explicit birthday tree size, find something nearby. + .or_else(|| get_tree_size_near(birthday_height).transpose()) .transpose()?; - // Get the note commitment tree size as of the end of the recover-until height. + // Get the note commitment tree size as of the start 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()) - }) + // 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), @@ -958,31 +1155,6 @@ 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. let tip_tree_size = match stmt_end_tree_size_at @@ -994,197 +1166,37 @@ 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) @@ -1209,9 +1221,7 @@ impl ScanProgress for SubtreeScanProgress { 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."), @@ -1236,9 +1246,7 @@ impl ScanProgress for SubtreeScanProgress { 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."), diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index af182003b..a77700f69 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -1292,20 +1292,16 @@ 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_eq!(summary.and_then(|s| s.scan_progress()), None); @@ -1347,7 +1343,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