diff --git a/proptest-regressions/bridgetree.txt b/proptest-regressions/bridgetree.txt index 7b93f4b7..afa608a0 100644 --- a/proptest-regressions/bridgetree.txt +++ b/proptest-regressions/bridgetree.txt @@ -4,4 +4,5 @@ # # It is recommended to check this file in to source control so that # everyone who runs the test benefits from these saved cases. -cc ed8f0851cbfcde17e671dca49e83151f01d277c881abdb09c738d00ead7fc74b # shrinks to tree = BridgeTree { depth: 8, bridges: [MerkleBridge { prior_position: None, auth_fragments: {}, frontier: NonEmptyFrontier { position: Position(1), leaf: Right("a", "a"), ommers: [] } }, MerkleBridge { prior_position: Some(Position(1)), auth_fragments: {Position(1): AuthFragment { position: Position(1), altitudes_observed: 1, values: ["ek"] }}, frontier: NonEmptyFrontier { position: Position(3), leaf: Right("e", "k"), ommers: ["aa"] } }], saved: {(Position(1), "a"): 0}, checkpoints: [], max_checkpoints: 10 } +cc a64e0d40c9287f2975a27447d266e4dd7228e44de99d000df39c4610598f486a # shrinks to tree = BridgeTree { depth: 8, prior_bridges: [MerkleBridge { prior_position: None, auth_fragments: {}, frontier: NonEmptyFrontier { position: Position(0), leaf: Left("a"), ommers: [] } }, MerkleBridge { prior_position: Some(Position(0)), auth_fragments: {}, frontier: NonEmptyFrontier { position: Position(1), leaf: Right("a", "a"), ommers: [] } }], current_bridge: Some(MerkleBridge { prior_position: Some(Position(1)), auth_fragments: {Position(1): AuthFragment { position: Position(1), altitudes_observed: 0, values: [] }}, frontier: NonEmptyFrontier { position: Position(1), leaf: Right("a", "a"), ommers: [] } }), saved: {Position(1): 1}, checkpoints: [Checkpoint { bridges_len: 1, is_witnessed: false, forgotten: {} }], max_checkpoints: 10 } +cc 736aee7c92f418b3b7391b0ae253ca4dc18f9b6cc625c0c34f0e568d26421e92 # shrinks to tree = BridgeTree { depth: 8, prior_bridges: [], current_bridge: None, saved: {}, checkpoints: [], max_checkpoints: 10 } diff --git a/src/bridgetree.rs b/src/bridgetree.rs index 69200a68..37907f83 100644 --- a/src/bridgetree.rs +++ b/src/bridgetree.rs @@ -702,7 +702,8 @@ impl Debug for BridgeTree #[derive(Debug, Clone, PartialEq, Eq)] pub enum BridgeTreeError { IncorrectIncompleteIndex, - InvalidWitnessIndex, + InvalidWitnessIndex(usize), + PositionMismatch { expected: Position, found: Position }, InvalidSavePoints, ContinuityError, CheckpointMismatch, @@ -762,31 +763,25 @@ impl BridgeTree { } } - /// Construct a new BridgeTree that will start recording changes from the state of - /// the specified frontier. - pub fn from_frontier(max_checkpoints: usize, frontier: NonEmptyFrontier) -> Self { - Self { - prior_bridges: vec![], - current_bridge: Some(MerkleBridge::from_parts(None, BTreeMap::new(), frontier)), - saved: BTreeMap::new(), - checkpoints: vec![], - max_checkpoints, - } - } - - pub fn from_parts( - prior_bridges: Vec>, - current_bridge: Option>, - saved: BTreeMap, - checkpoints: Vec, + fn check_consistency_internal( + prior_bridges: &[MerkleBridge], + current_bridge: &Option>, + saved: &BTreeMap, + checkpoints: &[Checkpoint], max_checkpoints: usize, - ) -> Result { + ) -> Result<(), BridgeTreeError> { // check that saved values correspond to bridges - if saved - .iter() - .any(|(pos, i)| i >= &prior_bridges.len() || &prior_bridges[*i].position() != pos) - { - return Err(BridgeTreeError::InvalidWitnessIndex); + for (pos, i) in saved { + if i >= &prior_bridges.len() { + return Err(BridgeTreeError::InvalidWitnessIndex(*i)); + } + let found = prior_bridges[*i].position(); + if &found != pos { + return Err(BridgeTreeError::PositionMismatch { + expected: *pos, + found, + }); + } } if checkpoints.len() > max_checkpoints @@ -813,6 +808,35 @@ impl BridgeTree { return Err(BridgeTreeError::ContinuityError); } + Ok(()) + } + + /// Construct a new BridgeTree that will start recording changes from the state of + /// the specified frontier. + pub fn from_frontier(max_checkpoints: usize, frontier: NonEmptyFrontier) -> Self { + Self { + prior_bridges: vec![], + current_bridge: Some(MerkleBridge::from_parts(None, BTreeMap::new(), frontier)), + saved: BTreeMap::new(), + checkpoints: vec![], + max_checkpoints, + } + } + + pub fn from_parts( + prior_bridges: Vec>, + current_bridge: Option>, + saved: BTreeMap, + checkpoints: Vec, + max_checkpoints: usize, + ) -> Result { + Self::check_consistency_internal( + &prior_bridges, + ¤t_bridge, + &saved, + &checkpoints, + max_checkpoints, + )?; Ok(BridgeTree { prior_bridges, current_bridge, @@ -822,64 +846,14 @@ impl BridgeTree { }) } - pub fn garbage_collect(&mut self) { - // Only garbage collect once we have more bridges than the maximum number of - // checkpoints; we cannot remove information that we might need to restore in - // a rewind. - if self.checkpoints.len() == self.max_checkpoints { - let gc_len = self.checkpoints.first().unwrap().bridges_len; - // Get a list of the leaf positions that we need to retain. This consists of - // all the saved leaves, plus all the leaves that have been forgotten since - // the most distant checkpoint to which we could rewind. - let remember: BTreeSet = self - .saved - .keys() - .chain(self.checkpoints.iter().flat_map(|c| c.forgotten.keys())) - .cloned() - .collect(); - - let mut cur: Option> = None; - let mut merged = 0; - let mut prune_fragment_positions: BTreeSet = BTreeSet::new(); - for (i, next_bridge) in std::mem::take(&mut self.prior_bridges) - .into_iter() - .enumerate() - { - if let Some(cur_bridge) = cur { - let pos = cur_bridge.position(); - let mut new_cur = if remember.contains(&pos) || i > gc_len { - // We need to remember cur_bridge; update its save index & put next_bridge - // on the chopping block - if let Some(idx) = self.saved.get_mut(&pos) { - *idx -= merged; - } - - self.prior_bridges.push(cur_bridge); - next_bridge - } else { - // We can fuse these bridges together because we don't need to - // remember next_bridge. - merged += 1; - prune_fragment_positions.insert(cur_bridge.frontier.position()); - cur_bridge.fuse(&next_bridge).unwrap() - }; - - new_cur.prune_auth_fragments(&prune_fragment_positions); - cur = Some(new_cur); - } else { - // this case will only occur for the first bridge - cur = Some(next_bridge); - } - } - - if let Some(last_bridge) = cur { - self.prior_bridges.push(last_bridge); - } - - for c in self.checkpoints.iter_mut() { - c.rewrite_indices(|idx| idx - merged); - } - } + pub fn check_consistency(&self) -> Result<(), BridgeTreeError> { + Self::check_consistency_internal( + &self.prior_bridges, + &self.current_bridge, + &self.saved, + &self.checkpoints, + self.max_checkpoints, + ) } } @@ -913,7 +887,7 @@ impl crate::Frontier for BridgeTr } } -impl Tree for BridgeTree { +impl Tree for BridgeTree { fn current_position(&self) -> Option { self.current_bridge.as_ref().map(|b| b.position()) } @@ -962,6 +936,10 @@ impl Tree for BridgeTree BTreeSet { + self.saved.keys().cloned().collect() + } + fn authentication_path(&self, position: Position) -> Option> { self.saved.get(&position).and_then(|idx| { let frontier = &self.prior_bridges[*idx].frontier; @@ -1082,8 +1060,8 @@ impl Tree for BridgeTree { // drop witnessed values at and above the checkpoint height; // we will re-witness if necessary. - self.saved.retain(|_, i| *i + 1 < c.bridges_len); self.saved.append(&mut c.forgotten); + self.saved.retain(|_, i| *i + 1 < c.bridges_len); self.prior_bridges.truncate(c.bridges_len); self.current_bridge = self.prior_bridges.last().map(|b| b.successor(false)); if c.is_witnessed { @@ -1094,6 +1072,73 @@ impl Tree for BridgeTree false, } } + + fn garbage_collect(&mut self) { + // Only garbage collect once we have more bridges than the maximum number of + // checkpoints; we cannot remove information that we might need to restore in + // a rewind. + if self.checkpoints.len() == self.max_checkpoints { + let gc_len = self.checkpoints.first().unwrap().bridges_len; + // Get a list of the leaf positions that we need to retain. This consists of + // all the saved leaves, plus all the leaves that have been forgotten since + // the most distant checkpoint to which we could rewind. + let remember: BTreeSet = self + .saved + .keys() + .chain(self.checkpoints.iter().flat_map(|c| c.forgotten.keys())) + .cloned() + .collect(); + + let mut cur: Option> = None; + let mut merged = 0; + let mut prune_fragment_positions: BTreeSet = BTreeSet::new(); + for (i, next_bridge) in std::mem::take(&mut self.prior_bridges) + .into_iter() + .enumerate() + { + if let Some(cur_bridge) = cur { + let pos = cur_bridge.position(); + let mut new_cur = if remember.contains(&pos) || i > gc_len { + // We need to remember cur_bridge; update its save index & put next_bridge + // on the chopping block + if let Some(idx) = self.saved.get_mut(&pos) { + *idx -= merged; + } + + self.prior_bridges.push(cur_bridge); + next_bridge + } else { + // We can fuse these bridges together because we don't need to + // remember next_bridge. + merged += 1; + prune_fragment_positions.insert(cur_bridge.frontier.position()); + cur_bridge.fuse(&next_bridge).unwrap() + }; + + new_cur.prune_auth_fragments(&prune_fragment_positions); + cur = Some(new_cur); + } else { + // this case will only occur for the first bridge + cur = Some(next_bridge); + } + } + + // unwrap is safe because we know that prior_bridges was nonempty. + if let Some(last_bridge) = cur { + if let Some(idx) = self.saved.get_mut(&last_bridge.position()) { + *idx -= merged; + } + self.prior_bridges.push(last_bridge); + } + + for c in self.checkpoints.iter_mut() { + c.rewrite_indices(|idx| idx - merged); + } + } + if let Err(e) = self.check_consistency() { + panic!("Consistency check failed with {:?} for tree {:?}", e, self); + } + } } #[cfg(test)] diff --git a/src/lib.rs b/src/lib.rs index 8bae6ee1..14b97f35 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,6 +28,7 @@ pub mod bridgetree; mod sample; use serde::{Deserialize, Serialize}; +use std::collections::BTreeSet; use std::convert::TryFrom; use std::num::TryFromIntError; use std::ops::{Add, AddAssign, Sub}; @@ -248,6 +249,9 @@ pub trait Tree: Frontier { /// marked, or None if the tree is empty. fn witness(&mut self) -> Option; + /// Return a set of all the positions for which we have witnessed. + fn witnessed_positions(&self) -> BTreeSet; + /// Obtains an authentication path to the value at the specified position. /// Returns `None` if there is no available authentication path to that /// value. @@ -269,11 +273,19 @@ pub trait Tree: Frontier { /// at that tree state have been removed using `rewind`. This function /// return false and leave the tree unmodified if no checkpoints exist. fn rewind(&mut self) -> bool; + + /// Remove state from the tree that no longer needs to be maintained + /// because it is associated with checkpoints or witnesses that + /// have been removed from the tree at positions deeper than those + /// reachable by calls to `rewind`. It is always safe to implement + /// this as a no-op operation + fn garbage_collect(&mut self); } #[cfg(test)] pub(crate) mod tests { #![allow(deprecated)] + use std::collections::BTreeSet; use std::fmt::Debug; use std::hash::Hasher; use std::hash::SipHasher; @@ -786,6 +798,16 @@ pub(crate) mod tests { let a = self.inefficient.witness(); let b = self.efficient.witness(); assert_eq!(a, b); + let apos = self.inefficient.witnessed_positions(); + let bpos = self.efficient.witnessed_positions(); + assert_eq!(apos, bpos); + a + } + + fn witnessed_positions(&self) -> BTreeSet { + let a = self.inefficient.witnessed_positions(); + let b = self.efficient.witnessed_positions(); + assert_eq!(a, b); a } @@ -814,6 +836,11 @@ pub(crate) mod tests { assert_eq!(a, b); a } + + fn garbage_collect(&mut self) { + self.inefficient.garbage_collect(); + self.efficient.garbage_collect(); + } } #[derive(Clone, Debug)] @@ -823,10 +850,12 @@ pub(crate) mod tests { CurrentLeaf, Witness, WitnessedLeaf(Position), + WitnessedPositions, Unwitness(Position), Checkpoint, Rewind, Authpath(Position), + GarbageCollect, } use Operation::*; @@ -845,6 +874,7 @@ pub(crate) mod tests { None } WitnessedLeaf(_) => None, + WitnessedPositions => None, Unwitness(p) => { assert!(tree.remove_witness(*p), "remove witness failed"); None @@ -858,6 +888,7 @@ pub(crate) mod tests { None } Authpath(p) => tree.authentication_path(*p).map(|xs| (*p, xs)), + GarbageCollect => None, } } @@ -957,9 +988,13 @@ pub(crate) mod tests { { prop_oneof![ item_gen.prop_map(Operation::Append), - Just(Operation::CurrentLeaf), - Just(Operation::CurrentPosition), Just(Operation::Witness), + prop_oneof![ + Just(Operation::CurrentLeaf), + Just(Operation::CurrentPosition), + Just(Operation::WitnessedPositions), + ], + Just(Operation::GarbageCollect), pos_gen .clone() .prop_map(|i| Operation::WitnessedLeaf(Position::from(i))), @@ -993,6 +1028,8 @@ pub(crate) mod tests { CurrentLeaf => {} Authpath(_) => {} WitnessedLeaf(_) => {} + WitnessedPositions => {} + GarbageCollect => {} } } @@ -1043,6 +1080,7 @@ pub(crate) mod tests { Unwitness(position) => { tree.remove_witness(position); } + WitnessedPositions => {} Checkpoint => { tree_checkpoints.push(tree_size); tree.checkpoint(); @@ -1071,6 +1109,7 @@ pub(crate) mod tests { ); } } + GarbageCollect => {} } } diff --git a/src/sample.rs b/src/sample.rs index 4804bc0e..9570427e 100644 --- a/src/sample.rs +++ b/src/sample.rs @@ -165,6 +165,10 @@ impl Tree for CompleteTree { self.tree_state.witness() } + fn witnessed_positions(&self) -> BTreeSet { + self.tree_state.witnesses.clone() + } + fn authentication_path(&self, position: Position) -> Option> { self.tree_state.authentication_path(position) } @@ -188,6 +192,10 @@ impl Tree for CompleteTree { false } } + + fn garbage_collect(&mut self) { + // Garbage collection of the sample tree is a no-op. + } } pub(crate) fn lazy_root(mut leaves: Vec) -> H {