From e01885f6e2e2d63f990a3ad9785789b15855cd0f Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 2 Mar 2022 10:04:11 -0700 Subject: [PATCH] Rewinds are no longer blocked by existing witnesses. In practice, when performing a rewind we don't want to have to do the additional bookkeeping keeping the leaf position and value around just to be able to remove the witnesses. After this change, `rewind` will always succeed unless there are no checkpoints. --- proptest-regressions/lib.txt | 2 + src/bridgetree.rs | 60 +++++++++----------- src/lib.rs | 104 ++++++++++++++++++++++------------- src/sample.rs | 33 +++-------- 4 files changed, 101 insertions(+), 98 deletions(-) diff --git a/proptest-regressions/lib.txt b/proptest-regressions/lib.txt index aeca088..07c7718 100644 --- a/proptest-regressions/lib.txt +++ b/proptest-regressions/lib.txt @@ -10,3 +10,5 @@ cc aa498d80d63c58720a913f2edcc6ba19da462f7a824f5be906e63c7ca1566076 # shrinks to cc 7cebce1664b804d55e259c466ebcc140c4a3708f4ec49ed865d6009736b0a568 # shrinks to ops = [Append("d"), Checkpoint, Witness, Unwitness("d"), Rewind, Unwitness("d")] cc 9078a200bceb14aa2f78743f545dab9719db29ddbbb5f1f67fae01805da90f4b # shrinks to ops = [Append("a"), Unwitness("a"), Checkpoint, Checkpoint, Rewind, Append("a"), Rewind, Append("a")] cc 20e43678bbf38f4c40fdd9e6f5bbc2b8eebcb1756b599f443f3268d6cf8e9d22 # shrinks to ops = [Append("o"), Checkpoint, Witness, Checkpoint, Unwitness("o"), Rewind, Rewind] +cc 2b82b514115d3ddb4990853b0178e997632226be6b8f7c9cf07e94a3e9f41690 # shrinks to ops = [Checkpoint, Witness, Rewind, Unwitness(Position(0), "x"), Checkpoint, Rewind, Checkpoint, Unwitness(Position(0), "x"), Unwitness(Position(0), "x"), Witness, Unwitness(Position(0), "x"), Append("x"), Append("b"), Authpath(Position(0), "x"), Authpath(Position(0), "x"), Rewind, Rewind, Authpath(Position(0), "x"), Witness, Rewind, Witness, Rewind, Append("x"), Rewind, Authpath(Position(0), "x"), Checkpoint, Authpath(Position(0), "x"), Witness, Checkpoint, Witness, Checkpoint, Rewind, Unwitness(Position(0), "x"), Unwitness(Position(0), "x"), Rewind, Witness, Rewind, Authpath(Position(0), "x"), Unwitness(Position(0), "x"), Authpath(Position(0), "x"), Witness, Witness, Checkpoint, Rewind, Checkpoint] +cc 248258b4e2fd558b56611179f6c8493326a8a35bd3fd3ffb55634c14f6fe417a # shrinks to ops = [Append(SipHashable(0)), Checkpoint, Witness, Rewind, Rewind, Authpath(Position(0), SipHashable(0)), Authpath(Position(0), SipHashable(0)), Append(SipHashable(0)), Witness, Authpath(Position(0), SipHashable(0)), Unwitness(Position(0), SipHashable(0)), Unwitness(Position(0), SipHashable(0)), Authpath(Position(0), SipHashable(0)), Authpath(Position(0), SipHashable(0)), Unwitness(Position(0), SipHashable(0)), Witness, Rewind, Checkpoint, Authpath(Position(0), SipHashable(0)), Witness, Rewind, Authpath(Position(0), SipHashable(0)), Authpath(Position(0), SipHashable(0)), Unwitness(Position(0), SipHashable(0)), Rewind, Append(SipHashable(0)), Witness, Authpath(Position(1), SipHashable(0)), Witness, Witness, Unwitness(Position(3), SipHashable(0)), Append(SipHashable(0)), Checkpoint, Append(SipHashable(0)), Witness, Witness, Checkpoint, Unwitness(Position(5), SipHashable(0)), Unwitness(Position(1), SipHashable(0)), Append(SipHashable(0)), Checkpoint, Authpath(Position(2), SipHashable(0)), Witness, Append(SipHashable(0)), Unwitness(Position(6), SipHashable(0)), Authpath(Position(2), SipHashable(0)), Witness, Unwitness(Position(5), SipHashable(0)), Checkpoint, Rewind, Unwitness(Position(1), SipHashable(0)), Checkpoint, Rewind, Witness, Checkpoint, Unwitness(Position(5), SipHashable(0)), Authpath(Position(2), SipHashable(0)), Append(SipHashable(0)), Checkpoint, Append(SipHashable(0)), Checkpoint, Append(SipHashable(0)), Unwitness(Position(4), SipHashable(0)), Rewind, Append(SipHashable(7)), Checkpoint, Append(SipHashable(9)), Authpath(Position(5), SipHashable(0)), Checkpoint, Authpath(Position(10), SipHashable(7)), Rewind, Checkpoint, Rewind] diff --git a/src/bridgetree.rs b/src/bridgetree.rs index 8492b0a..cd56712 100644 --- a/src/bridgetree.rs +++ b/src/bridgetree.rs @@ -1050,45 +1050,35 @@ impl Tree for BridgeTree bool { match self.checkpoints.pop() { Some(mut c) => { - if self.saved.values().any(|saved_idx| { - c.bridges_len == 0 - || (!c.is_witnessed && *saved_idx >= c.bridges_len - 1) - || (c.is_witnessed && *saved_idx >= c.bridges_len) - }) { - // there is a witnessed value at a later position, or the - // current position was witnessed since the checkpoint was - // created, so we restore the removed checkpoint and return - // failure - self.checkpoints.push(c); - false - } else { - self.bridges.truncate(c.bridges_len); - self.saved.append(&mut c.forgotten); + // drop witnessed values at and above the checkpoint height; + // we will re-witness if necessary. + self.saved + .retain(|_, saved_idx| *saved_idx + 1 < c.bridges_len); + self.bridges.truncate(c.bridges_len); + self.saved.append(&mut c.forgotten); - let was_duplicate_checkpoint = self - .checkpoints - .last() - .iter() - .any(|c0| c0.bridges_len == c.bridges_len); - let len = self.bridges.len(); - if c.is_witnessed { - // if the checkpointed state was witnessed, we need to - // restore the witness, as the successor bridge will have - // been removed by truncation. - self.witness(); - } else if len > 0 && was_duplicate_checkpoint { - // if the checkpoint was a duplicate, we need to create - // a successor so that future appends do not mutate the - // state at the tip. - self.bridges.push(self.bridges[len - 1].successor(false)); - } - true + let was_duplicate_checkpoint = self + .checkpoints + .last() + .iter() + .any(|c0| c0.bridges_len == c.bridges_len); + let len = self.bridges.len(); + if c.is_witnessed { + // if the checkpointed state was witnessed, we need to + // restore the witness, as the successor bridge will have + // been removed by truncation. + self.witness(); + } else if len > 0 && was_duplicate_checkpoint { + // if the checkpoint was a duplicate, we need to create + // a successor so that future appends do not mutate the + // state at the tip. + self.bridges.push(self.bridges[len - 1].successor(false)); } + true } None => false, } @@ -1198,11 +1188,11 @@ mod tests { t.witness(); t.append(&"b".to_string()); t.append(&"c".to_string()); - assert!(!t.rewind(), "Rewind is expected to fail."); assert!( t.drop_oldest_checkpoint(), "Checkpoint drop is expected to succeed" ); + assert!(!t.rewind(), "Rewind is expected to fail."); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 47f4bfb..59c2d77 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -270,12 +270,7 @@ pub trait Tree: Frontier { /// that checkpoint record. If there are multiple checkpoints at a given /// tree state, the tree state will not be altered until all checkpoints /// at that tree state have been removed using `rewind`. This function - /// will fail and return false if there is no previous checkpoint or in - /// the event witness data would be destroyed in the process. - /// - /// In the case that this method returns `false`, the user should have - /// explicitly called `remove_witness` for each witnessed leaf marked - /// since the last checkpoint. + /// return false and leave the tree unmodified if no checkpoints exist. fn rewind(&mut self) -> bool; /// Start a recording of append operations performed on a tree. @@ -371,7 +366,6 @@ pub(crate) mod tests { t.checkpoint(); t.witness(); t.append(&"a".to_string()); - assert!(!t.rewind()); t.append(&"a".to_string()); t.append(&"a".to_string()); assert_eq!(t.root(), "aaaa____________"); @@ -599,17 +593,19 @@ pub(crate) mod tests { pub(crate) fn check_checkpoint_rewind, F: Fn(usize) -> T>(new_tree: F) { let mut t = new_tree(100); - t.append(&"a".to_string()); - t.checkpoint(); - t.append(&"b".to_string()); - t.witness(); assert!(!t.rewind()); + let mut t = new_tree(100); + t.checkpoint(); + assert!(t.rewind()); + let mut t = new_tree(100); t.append(&"a".to_string()); t.checkpoint(); + t.append(&"b".to_string()); t.witness(); - assert!(!t.rewind()); + assert!(t.rewind()); + assert_eq!(Some((Position::from(0), "a".to_string())), t.current_leaf()); let mut t = new_tree(100); t.append(&"a".to_string()); @@ -622,7 +618,8 @@ pub(crate) mod tests { t.checkpoint(); t.witness(); t.append(&"a".to_string()); - assert!(!t.rewind()); + assert!(t.rewind()); + assert_eq!(Some((Position::from(0), "a".to_string())), t.current_leaf()); let mut t = new_tree(100); t.append(&"a".to_string()); @@ -636,6 +633,20 @@ pub(crate) mod tests { } pub(crate) fn check_rewind_remove_witness, F: Fn(usize) -> T>(new_tree: F) { + let mut tree = new_tree(100); + tree.append(&"e".to_string()); + tree.witness(); + tree.checkpoint(); + assert!(tree.rewind()); + assert!(tree.remove_witness(0usize.into(), &"e".to_string())); + + let mut tree = new_tree(100); + tree.append(&"e".to_string()); + tree.checkpoint(); + tree.witness(); + assert!(tree.rewind()); + assert!(!tree.remove_witness(0usize.into(), &"e".to_string())); + let mut tree = new_tree(100); tree.append(&"e".to_string()); tree.witness(); @@ -657,7 +668,7 @@ pub(crate) mod tests { assert!(!tree.remove_witness(0usize.into(), &"a".to_string())); tree.checkpoint(); assert!(tree.witness().is_some()); - assert!(!tree.rewind()); + assert!(tree.rewind()); let mut tree = new_tree(100); tree.append(&"a".to_string()); @@ -671,50 +682,69 @@ pub(crate) mod tests { // test framework itself previously did not correctly handle // chain state restoration. - let ops = vec![ - Append("a".to_string()), - Unwitness(0usize.into(), "a".to_string()), - Checkpoint, - Witness, - Rewind, - ]; + fn append(x: &str) -> Operation { + Append(x.to_string()) + } + + fn unwitness(pos: usize, x: &str) -> Operation { + Unwitness(Position::from(pos), x.to_string()) + } + + let ops = vec![append("x"), Checkpoint, Witness, Rewind, unwitness(0, "x")]; let result = check_operations(ops); - assert!(matches!(result, Ok(())), "Test failed: {:?}", result); + assert!( + matches!(result, Ok(())), + "Reference/Test mismatch: {:?}", + result + ); let ops = vec![ - Append("s".to_string()), - Witness, - Append("m".to_string()), + append("d"), Checkpoint, - Unwitness(0usize.into(), "s".to_string()), + Witness, + unwitness(0, "d"), Rewind, - Unwitness(0usize.into(), "s".to_string()), + unwitness(0, "d"), ]; let result = check_operations(ops); - assert!(matches!(result, Ok(())), "Test failed: {:?}", result); + assert!( + matches!(result, Ok(())), + "Reference/Test mismatch: {:?}", + result + ); let ops = vec![ - Append("d".to_string()), + append("o"), Checkpoint, Witness, - Unwitness(0usize.into(), "d".to_string()), + Checkpoint, + unwitness(0, "o"), + Rewind, Rewind, - Unwitness(0usize.into(), "d".to_string()), ]; let result = check_operations(ops); - assert!(matches!(result, Ok(())), "Test failed: {:?}", result); + assert!( + matches!(result, Ok(())), + "Reference/Test mismatch: {:?}", + result + ); let ops = vec![ - Append("o".to_string()), - Checkpoint, + append("s"), Witness, + append("m"), Checkpoint, - Unwitness(0usize.into(), "o".to_string()), - Rewind, + unwitness(0, "s"), Rewind, + unwitness(0, "s"), + unwitness(0, "m"), ]; let result = check_operations(ops); - assert!(matches!(result, Ok(())), "Test failed: {:?}", result); + assert!( + matches!(result, Ok(())), + "Reference/Test mismatch: {:?}", + result + ); } // diff --git a/src/sample.rs b/src/sample.rs index f7fe226..4a46cd4 100644 --- a/src/sample.rs +++ b/src/sample.rs @@ -1,6 +1,5 @@ +//! Sample implementation of the Tree interface. use super::{Altitude, Frontier, Hashable, Position, Recording, Tree}; -/// Sample implementation of the Tree interface. -use std::convert::TryInto; #[derive(Clone, Debug)] pub struct TreeState { @@ -144,7 +143,7 @@ impl TreeState { #[derive(Clone, Debug)] pub struct CompleteTree { tree_state: TreeState, - checkpoints: Vec<(TreeState, bool)>, + checkpoints: Vec>, max_checkpoints: usize, } @@ -228,36 +227,18 @@ impl Tree for CompleteTree { /// Marks the current tree state as a checkpoint if it is not already a /// checkpoint. fn checkpoint(&mut self) { - let is_witnessed = self - .tree_state - .current_leaf() - .into_iter() - .any(|(p, l)| self.tree_state.is_witnessed(p, &l)); - self.checkpoints - .push((self.tree_state.clone(), is_witnessed)); + self.checkpoints.push(self.tree_state.clone()); if self.checkpoints.len() > self.max_checkpoints { self.drop_oldest_checkpoint(); } } /// Rewinds the tree state to the previous checkpoint. This function will - /// fail and return false if there is no previous checkpoint or in the event - /// witness data would be destroyed in the process. + /// return false and leave the tree unmodified if no checkpoints exist. fn rewind(&mut self) -> bool { - if let Some((checkpointed_state, is_witnessed)) = self.checkpoints.pop() { - // if there are any witnessed leaves in the current tree state - // that would be removed, we don't rewind - if self.tree_state.witnesses.iter().any(|&(pos, _)| { - let offset: usize = (pos + 1).try_into().unwrap(); - offset > checkpointed_state.current_offset - || (offset == checkpointed_state.current_offset && !is_witnessed) - }) { - self.checkpoints.push((checkpointed_state, is_witnessed)); - false - } else { - self.tree_state = checkpointed_state; - true - } + if let Some(checkpointed_state) = self.checkpoints.pop() { + self.tree_state = checkpointed_state; + true } else { false }