From 650edd0fc71ed762494d0195fe61bddae475382a Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 22 Mar 2023 16:43:11 -0600 Subject: [PATCH 1/2] Make `bridgetree` polymorphic in checkpoint identifier type. --- bridgetree/src/lib.rs | 86 +++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 45 deletions(-) diff --git a/bridgetree/src/lib.rs b/bridgetree/src/lib.rs index 3b39bc3..49f8bbb 100644 --- a/bridgetree/src/lib.rs +++ b/bridgetree/src/lib.rs @@ -613,9 +613,9 @@ impl<'a, H: Hashable + Ord + Clone + 'a> MerkleBridge { /// bridges; instead, we use [`Checkpoint`] values to be able to rapidly restore the cache to its /// previous state. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct Checkpoint { +pub struct Checkpoint { /// The unique identifier for this checkpoint. - id: usize, + id: C, /// The number of bridges that will be retained in a rewind. bridges_len: usize, /// A set of the positions that have been marked during the period that this @@ -627,10 +627,10 @@ pub struct Checkpoint { forgotten: BTreeSet, } -impl Checkpoint { +impl Checkpoint { /// Creates a new checkpoint from its constituent parts. pub fn from_parts( - id: usize, + id: C, bridges_len: usize, marked: BTreeSet, forgotten: BTreeSet, @@ -644,7 +644,7 @@ impl Checkpoint { } /// Creates a new empty checkpoint for the specified [`BridgeTree`] state. - pub fn at_length(bridges_len: usize, id: usize) -> Self { + pub fn at_length(bridges_len: usize, id: C) -> Self { Checkpoint { id, bridges_len, @@ -655,8 +655,8 @@ impl Checkpoint { /// The unique identifier for the checkpoint, which is simply an automatically incrementing /// index over all checkpoints that have ever been created in the history of the tree. - pub fn id(&self) -> usize { - self.id + pub fn id(&self) -> &C { + &self.id } /// Returns the length of the [`BridgeTree::prior_bridges`] vector of the [`BridgeTree`] to @@ -714,7 +714,7 @@ impl Checkpoint { /// A sparse representation of a Merkle tree with linear appending of leaves that contains enough /// information to produce a witness for any `mark`ed leaf. #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct BridgeTree { +pub struct BridgeTree { /// The ordered list of Merkle bridges representing the history /// of the tree. There will be one bridge for each saved leaf. prior_bridges: Vec>, @@ -727,14 +727,14 @@ pub struct BridgeTree { /// This deque must be maintained to have a minimum size of 1 and a maximum /// size of `max_checkpoints` in order to correctly maintain mark & rewind /// semantics. - checkpoints: VecDeque, + checkpoints: VecDeque>, /// The maximum number of checkpoints to retain. If this number is /// exceeded, the oldest checkpoint will be dropped when creating /// a new checkpoint. max_checkpoints: usize, } -impl Debug for BridgeTree { +impl Debug for BridgeTree { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { write!( f, @@ -756,12 +756,12 @@ pub enum BridgeTreeError { CheckpointMismatch, } -impl BridgeTree { +impl BridgeTree { /// Construct an empty BridgeTree value with the specified maximum number of checkpoints. /// /// Panics if `max_checkpoints < 1` because mark/rewind logic depends upon the presence /// of checkpoints to function. - pub fn new(max_checkpoints: usize, initial_checkpoint_id: usize) -> Self { + pub fn new(max_checkpoints: usize, initial_checkpoint_id: C) -> Self { assert!(max_checkpoints >= 1); Self { prior_bridges: vec![], @@ -806,7 +806,7 @@ impl BridgeTree { } /// Returns the checkpoints to which this tree may be rewound. - pub fn checkpoints(&self) -> &VecDeque { + pub fn checkpoints(&self) -> &VecDeque> { &self.checkpoints } @@ -823,7 +823,7 @@ impl BridgeTree { } } -impl BridgeTree { +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 { @@ -847,7 +847,7 @@ impl BridgeTree { prior_bridges: Vec>, current_bridge: Option>, saved: BTreeMap, - checkpoints: VecDeque, + checkpoints: VecDeque>, max_checkpoints: usize, ) -> Result { Self::check_consistency_internal( @@ -880,7 +880,7 @@ impl BridgeTree { prior_bridges: &[MerkleBridge], current_bridge: &Option>, saved: &BTreeMap, - checkpoints: &VecDeque, + checkpoints: &VecDeque>, max_checkpoints: usize, ) -> Result<(), BridgeTreeError> { // check that saved values correspond to bridges @@ -1049,8 +1049,8 @@ impl BridgeTree { /// It is valid to have multiple checkpoints for the same tree state, and each `rewind` call /// will remove a single checkpoint. Successive checkpoint identifiers must always be provided /// in increasing order. - pub fn checkpoint(&mut self, id: usize) -> bool { - if Some(id) > self.checkpoints.back().map(|c| c.id) { + pub fn checkpoint(&mut self, id: C) -> bool { + if Some(&id) > self.checkpoints.back().map(|c| &c.id) { match self.current_bridge.take() { Some(cur_b) => { // Do not create a duplicate bridge @@ -1120,9 +1120,9 @@ impl BridgeTree { checkpoint_depth: usize, ) -> Result, WitnessingError> { #[derive(Debug)] - enum AuthBase<'a> { + enum AuthBase<'a, C> { Current, - Checkpoint(usize, &'a Checkpoint), + Checkpoint(usize, &'a Checkpoint), } // Find the earliest checkpoint having a matching root, or the current @@ -1283,24 +1283,16 @@ mod tests { use super::*; use incrementalmerkletree::{ testing::{ - apply_operation, arb_operation, check_checkpoint_rewind, check_operations, + self, apply_operation, arb_operation, check_checkpoint_rewind, check_operations, check_remove_mark, check_rewind_remove_mark, check_root_hashes, check_witnesses, - complete_tree::CompleteTree, CombinedTree, Frontier, SipHashable, Tree, + complete_tree::CompleteTree, CombinedTree, SipHashable, }, Hashable, }; - impl Frontier for super::Frontier { - fn append(&mut self, value: H) -> bool { - super::Frontier::append(self, value) - } - - fn root(&self) -> H { - super::Frontier::root(self) - } - } - - impl Tree for BridgeTree { + impl testing::Tree + for BridgeTree + { fn append(&mut self, value: H, retention: Retention) -> bool { let appended = BridgeTree::append(self, value); if appended { @@ -1448,7 +1440,7 @@ mod tests { #[test] fn tree_depth() { - let mut tree = BridgeTree::::new(100, 0); + let mut tree = BridgeTree::::new(100, 0); for c in 'a'..'i' { assert!(tree.append(c.to_string())) } @@ -1456,7 +1448,7 @@ mod tests { } fn check_garbage_collect( - mut tree: BridgeTree, + mut tree: BridgeTree, ) { // Add checkpoints until we're sure everything that can be gc'ed will be gc'ed for i in 0..tree.max_checkpoints { @@ -1474,13 +1466,13 @@ mod tests { fn arb_bridgetree( item_gen: G, max_count: usize, - ) -> impl Strategy> + ) -> impl Strategy> where G::Value: Hashable + Ord + Clone + Debug + 'static, { proptest::collection::vec(arb_operation(item_gen, 0..max_count), 0..max_count).prop_map( |ops| { - let mut tree: BridgeTree = BridgeTree::new(10, 0); + let mut tree: BridgeTree = BridgeTree::new(10, 0); for (i, op) in ops.into_iter().enumerate() { apply_operation(&mut tree, op.map_checkpoint_id(|_| i)); } @@ -1516,29 +1508,33 @@ mod tests { #[test] fn root_hashes() { - check_root_hashes(|max_checkpoints| BridgeTree::::new(max_checkpoints, 0)); + check_root_hashes(|max_checkpoints| { + BridgeTree::::new(max_checkpoints, 0) + }); } #[test] fn witness() { - check_witnesses(|max_checkpoints| BridgeTree::::new(max_checkpoints, 0)); + check_witnesses(|max_checkpoints| BridgeTree::::new(max_checkpoints, 0)); } #[test] fn checkpoint_rewind() { - check_checkpoint_rewind(|max_checkpoints| BridgeTree::::new(max_checkpoints, 0)); + check_checkpoint_rewind(|max_checkpoints| { + BridgeTree::::new(max_checkpoints, 0) + }); } #[test] fn rewind_remove_mark() { check_rewind_remove_mark(|max_checkpoints| { - BridgeTree::::new(max_checkpoints, 0) + BridgeTree::::new(max_checkpoints, 0) }); } #[test] fn garbage_collect() { - let mut tree: BridgeTree = BridgeTree::new(1000, 0); + let mut tree: BridgeTree = BridgeTree::new(1000, 0); let empty_root = tree.root(0); tree.append("a".to_string()); for i in 0..100 { @@ -1549,7 +1545,7 @@ mod tests { tree.rewind(); assert!(tree.root(0) != empty_root); - let mut t = BridgeTree::::new(10, 0); + let mut t = BridgeTree::::new(10, 0); let mut to_unmark = vec![]; let mut has_witness = vec![]; for i in 0usize..100 { @@ -1593,10 +1589,10 @@ mod tests { // Combined tree tests fn new_combined_tree( max_checkpoints: usize, - ) -> CombinedTree, BridgeTree> { + ) -> CombinedTree, BridgeTree> { CombinedTree::new( CompleteTree::::new(max_checkpoints, 0), - BridgeTree::::new(max_checkpoints, 0), + BridgeTree::::new(max_checkpoints, 0), ) } From f7931ec31f7a1ed1ab13c519f9b90d0291bbdfdf Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 24 Mar 2023 08:38:11 -0600 Subject: [PATCH 2/2] Apply suggestions from code review. --- bridgetree/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/bridgetree/src/lib.rs b/bridgetree/src/lib.rs index 49f8bbb..926dfab 100644 --- a/bridgetree/src/lib.rs +++ b/bridgetree/src/lib.rs @@ -477,7 +477,7 @@ impl MerkleBridge { } } -impl<'a, H: Hashable + Ord + Clone + 'a> MerkleBridge { +impl<'a, H: Hashable + Clone + Ord + 'a> MerkleBridge { /// Constructs a new bridge to follow this one. If `mark_current_leaf` is true, the successor /// will track the information necessary to create a witness for the leaf most /// recently appended to this bridge's frontier. @@ -653,8 +653,7 @@ impl Checkpoint { } } - /// The unique identifier for the checkpoint, which is simply an automatically incrementing - /// index over all checkpoints that have ever been created in the history of the tree. + /// The unique identifier for the checkpoint. pub fn id(&self) -> &C { &self.id } @@ -823,7 +822,7 @@ impl BridgeTree { } } -impl BridgeTree { +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 { @@ -1290,7 +1289,7 @@ mod tests { Hashable, }; - impl testing::Tree + impl testing::Tree for BridgeTree { fn append(&mut self, value: H, retention: Retention) -> bool { @@ -1468,7 +1467,7 @@ mod tests { max_count: usize, ) -> impl Strategy> where - G::Value: Hashable + Ord + Clone + Debug + 'static, + G::Value: Hashable + Clone + Ord + Debug + 'static, { proptest::collection::vec(arb_operation(item_gen, 0..max_count), 0..max_count).prop_map( |ops| { @@ -1587,7 +1586,7 @@ mod tests { } // Combined tree tests - fn new_combined_tree( + fn new_combined_tree( max_checkpoints: usize, ) -> CombinedTree, BridgeTree> { CombinedTree::new(