From 6db4355fc46838b0c0bb91dd33ab7a67ad2c723e Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 7 Jul 2023 18:36:37 -0600 Subject: [PATCH] Fix an error in dominance calculation. --- zcash_client_sqlite/src/wallet/scanning.rs | 94 +++++++++++++++++----- 1 file changed, 72 insertions(+), 22 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index edc9dfacc..5313a330a 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -1,7 +1,7 @@ use rusqlite::{self, named_params, types::Value, OptionalExtension}; use std::cmp::{max, min, Ordering}; use std::collections::BTreeSet; -use std::ops::Range; +use std::ops::{Not, Range}; use std::rc::Rc; use zcash_client_backend::data_api::scanning::{ScanPriority, ScanRange}; @@ -15,13 +15,39 @@ use crate::{PRUNING_DEPTH, VALIDATION_DEPTH}; use super::block_height_extrema; -#[derive(Copy, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Copy)] +enum Insert { + Left, + Right, +} + +impl Not for Insert { + type Output = Self; + + fn not(self) -> Self::Output { + match self { + Insert::Left => Insert::Right, + Insert::Right => Insert::Left, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] enum Dominance { Left, Right, Equal, } +impl From for Dominance { + fn from(value: Insert) -> Self { + match value { + Insert::Left => Dominance::Left, + Insert::Right => Dominance::Right, + } + } +} + pub(crate) fn parse_priority_code(code: i64) -> Option { use ScanPriority::*; match code { @@ -91,14 +117,14 @@ fn update_priority(current: ScanPriority, inserted: ScanPriority) -> ScanPriorit } } -fn dominance(current: &ScanPriority, inserted: &ScanPriority) -> Dominance { +fn dominance(current: &ScanPriority, inserted: &ScanPriority, insert: Insert) -> Dominance { match (current, inserted) { - (_, ScanPriority::Verify) | (_, ScanPriority::Scanned) => Dominance::Right, - (ScanPriority::Scanned, _) => Dominance::Left, + (_, ScanPriority::Verify | ScanPriority::Scanned) => Dominance::from(insert), + (ScanPriority::Scanned, _) => Dominance::from(!insert), (a, b) => match a.cmp(b) { - Ordering::Less => Dominance::Right, + Ordering::Less => Dominance::from(insert), Ordering::Equal => Dominance::Equal, - Ordering::Greater => Dominance::Left, + Ordering::Greater => Dominance::from(!insert), }, } } @@ -183,11 +209,6 @@ fn join_nonoverlapping(left: ScanRange, right: ScanRange) -> Joined { } fn insert(current: ScanRange, to_insert: ScanRange) -> Joined { - enum Insert { - Left, - Right, - } - fn join_overlapping(left: ScanRange, right: ScanRange, insert: Insert) -> Joined { assert!( left.block_range().start <= right.block_range().start @@ -196,8 +217,8 @@ fn insert(current: ScanRange, to_insert: ScanRange) -> Joined { // recompute the range dominance based upon the queue entry priorities let dominance = match insert { - Insert::Left => dominance(&right.priority(), &left.priority()), - Insert::Right => dominance(&left.priority(), &right.priority()), + Insert::Left => dominance(&right.priority(), &left.priority(), insert), + Insert::Right => dominance(&left.priority(), &right.priority(), insert), }; match dominance { @@ -535,13 +556,15 @@ pub(crate) fn replace_queue_entries( // Update the tree that we read from the database, or if we didn't find any ranges // start with the scanned range. - let mut to_create = match existing_ranges { - Some(cur) => entries.next().map(|entry| cur.insert(entry)), - None => entries.next().map(SpanningTree::Leaf), + let mut to_create = match (existing_ranges, entries.next()) { + (Some(cur), Some(entry)) => Some(cur.insert(entry)), + (None, Some(entry)) => Some(SpanningTree::Leaf(entry)), + (Some(cur), None) => Some(cur), + (None, None) => None, }; for entry in entries { - to_create = to_create.map(|cur| cur.insert(entry)) + to_create = to_create.map(|cur| cur.insert(entry)); } (to_create, to_delete_ends) @@ -841,7 +864,6 @@ mod tests { use ScanPriority::*; let t = spanning_tree(&[(0..3, Verify), (2..8, Scanned), (6..10, Verify)]).unwrap(); - assert_eq!( t.into_vec(), vec![ @@ -852,7 +874,6 @@ mod tests { ); let t = spanning_tree(&[(0..3, Verify), (2..8, Historic), (6..10, Verify)]).unwrap(); - assert_eq!( t.into_vec(), vec![ @@ -863,7 +884,6 @@ mod tests { ); let t = spanning_tree(&[(0..3, Scanned), (2..8, Verify), (6..10, Scanned)]).unwrap(); - assert_eq!( t.into_vec(), vec![ @@ -874,7 +894,6 @@ mod tests { ); let t = spanning_tree(&[(0..3, Scanned), (2..8, Historic), (6..10, Scanned)]).unwrap(); - assert_eq!( t.into_vec(), vec![ @@ -883,6 +902,37 @@ mod tests { scan_range(6..10, Scanned), ] ); + + // a `ChainTip` insertion should not overwrite a scanned range. + let mut t = spanning_tree(&[(0..3, ChainTip), (3..5, Scanned), (5..7, ChainTip)]).unwrap(); + t = t.insert(scan_range(0..7, ChainTip)); + assert_eq!( + t.into_vec(), + vec![ + scan_range(0..3, ChainTip), + scan_range(3..5, Scanned), + scan_range(5..7, ChainTip), + ] + ); + + let mut t = + spanning_tree(&[(280300..280310, FoundNote), (280310..280320, Scanned)]).unwrap(); + assert_eq!( + t.clone().into_vec(), + vec![ + scan_range(280300..280310, FoundNote), + scan_range(280310..280320, Scanned) + ] + ); + t = t.insert(scan_range(280300..280340, ChainTip)); + assert_eq!( + t.into_vec(), + vec![ + scan_range(280300..280310, ChainTip), + scan_range(280310..280320, Scanned), + scan_range(280320..280340, ChainTip) + ] + ); } #[test]