Skip to content

Commit

Permalink
shardtree: Rework rewind & checkpoint depth handling.
Browse files Browse the repository at this point in the history
  • Loading branch information
nuttycom committed Sep 27, 2024
1 parent 05f23d9 commit 9a77e51
Show file tree
Hide file tree
Showing 9 changed files with 477 additions and 228 deletions.
83 changes: 81 additions & 2 deletions shardtree/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,90 @@ and this project adheres to Rust's notion of

## Unreleased

### Changed
- MSRV is now 1.64
This release includes a significant refactoring and rework of several methods
of the `shardtree::ShardTree` type and the `shardtree::store::ShardStore`
trait. Please read the notes for this release carefully as the semantics of
important methods have changed. These changes may require changes to clients of
this crate; in particular, the existence of a checkpoint is required for all
witnessing and rewinding operations.

### Added
- `shardtree::store::ShardStore::for_each_checkpoint`
- `shardtree::ShardTree::truncate_to_checkpoint_depth`. This replaces
the `ShardTree::truncate_to_depth` method, with modified semantics such that
the provided `checkpoint_depth` argument is now treated strictly as a
zero-based index into the checkpoints applied to the tree, in reverse order
of checkpoint identifier. It is no longer possible to truncate the tree if no
checkpoints have been created.
- `shardtree::ShardTree::truncate_to_checkpoint` replaces
`ShardTree::truncate_removing_checkpoint`. Instead of removing
the checkpoint, this replacement method removes all state from the tree
related to leaves having positions greater than the checkpointed position,
but unlike `truncate_removing_checkpoint` it leaves the checkpoint itself
in place.
- `shardtree::store::ShardStore::truncate_shards` replaces
`ShardStore::truncate`. Instead of taking an `Address` and requiring that
implementations impose additional runtime restriction on the level of that
address, the replacement method directly takes a shard index.
- `ShardStore::truncate_truncate_checkpoints_retaining` replaces
`ShardStore::truncate_checkpoints`. Instead of removing the checkpoint
having the specified identifier, the associated checkpoint should be retained
but any additional metadata stored with the checkpoint, such as information
about marks removed after the creation of the checkpoint, should be deleted.

### Changed
- MSRV is now 1.64
- `shardtree::ShardTree`:
- `ShardTree::max_leaf_position` now takes its `checkpoint_depth` argument
as `Option<usize>` instead of `usize`. The semantics of this method are now
changed such that if a checkpoint depth is provided, it is now treated
strictly as a zero-based index into the checkpoints of the tree in reverse
order of checkpoint identifier, and an error is returned if no checkpoint
exists at the given index. A `None` value passed for this argument causes
it to return the maximum position among all leaves added to the tree.
- `ShardTree::root_at_checkpoint_id` and `root_at_checkpoint_id_caching` now
each return the computed root as an optional value, returning `Ok(None)` if
the checkpoint corresponding to the requested identifier does not exist.
- `ShardTree::root_at_checkpoint_depth` and `root_at_checkpoint_depth_caching`
now takes the `checkpoint_depth` argument as `Option<usize>` instead of
`usize`. The semantics of this method are now changed such that if a
checkpoint depth is provided, it is now treated strictly as a zero-based
index into the checkpoints of the tree in reverse order of checkpoint
identifier, and an error is returned if no checkpoint exists at the given
index. A `None` value passed for this argument causes it to return the root
computed over all of the leaves in the tree. These methods now each return
the computed root as an optional value, returning `Ok(None)` if no checkpoint
exist at the requested checkpoint depth.
- `ShardTree::witness_at_checkpoint_id` and `witness_at_checkpoint_id_caching`
now each return the computed witness as an optional value, returning
`Ok(None)` if the checkpoint corresponding to the requested identifier does
not exist.
- `ShardTree::witness_at_checkpoint_depth` and `witness_at_checkpoint_depth_caching`
now each return the computed witness as an optional value, returning
`Ok(None)` if no checkpoint was available at the given checkpoint depth.The
semantics of this method are now changed such that if a checkpoint depth is
provided, it is now treated strictly as a zero-based index into the
checkpoints of the tree in reverse order of checkpoint identifier, and an
error is returned if no checkpoint exists at the given index. IT IS NO LONGER
POSSIBLE TO COMPUTE A WITNESS WITHOUT A CHECKPOINT BEING PRESENT IN THE TREE.
- `shardtree::store::ShardStore`:
- The semantics of `ShardStore::get_checkpoint_at_depth` HAVE CHANGED WITHOUT
CHANGES TO THE METHOD SIGNATURE. The `checkpoint_depth` argument to this
method is now treated strictly as a zero-based index into the checkpoints
of the tree in reverse order of checkpoint identifier. Previously, this
method always returned `None` for `checkpoint_depth == 0`, and
`checkpoint_depth` was otherwise treated as a 1-based index instead of a
0-based index.

### Removed
- `shardtree::ShardTree::truncate_to_depth` has been replaced by
`ShardTree::truncate_to_checkpoint_depth`
- `shardtree::ShardTree::truncate_removing_checkpoint` has been replaced by
`ShardTree::truncate_to_checkpoint`
- `shardtree::store::ShardStore::truncate` has been replaced by
`ShardStore::truncate_shards`
- `shardtree::store::ShardStore::truncate_checkpoints` has been replaced by
`ShardStore::truncate_checkpoints_retaining`

## [0.4.0] - 2024-08-12

Expand Down
7 changes: 6 additions & 1 deletion shardtree/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ proptest.workspace = true
legacy-api = ["incrementalmerkletree/legacy-api"]
# The test-depenencies feature can be enabled to expose types and functions
# that are useful for testing `shardtree` functionality.
test-dependencies = ["proptest", "assert_matches", "incrementalmerkletree/test-dependencies"]
test-dependencies = [
"dep:proptest",
"dep:assert_matches",
"dep:incrementalmerkletree-testing",
"incrementalmerkletree/test-dependencies"
]

[target.'cfg(unix)'.dev-dependencies]
tempfile = ">=3, <3.7.0" # 3.7 has MSRV 1.63
3 changes: 3 additions & 0 deletions shardtree/proptest-regressions/lib.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@ cc d53a73021238de143764ee1d48b944abb93bd4bc54f35d16e514261220d3eb78 # shrinks to
cc d9460b8acbc5b4d112cae5d9e2296fcd793999b2b2e1d5405722f2bd8d176c31 # shrinks to ops = [Append("a", Checkpoint { id: (), is_marked: true }), Rewind, Append("a", Ephemeral), Rewind, Unmark(Position(0))]
cc 644c7763bc7bdc65bd9e6eb156b3b1a9b0632571a571c462bd44f3e04a389ca0 # shrinks to ops = [Append("a", Ephemeral), Append("a", Checkpoint { id: (), is_marked: true }), Append("a", Ephemeral), Append("a", Ephemeral), Unmark(Position(1)), Witness(Position(1), 0)]
cc 12790169d3df4280dd155d9cdfa76719318b8ec97a80bd562b7cb182d4f9bc79 # shrinks to ops = [CurrentPosition, CurrentPosition, Append(SipHashable(0), Ephemeral), Append(SipHashable(0), Marked), Append(SipHashable(0), Ephemeral), Checkpoint(()), Checkpoint(()), Checkpoint(()), Unmark(Position(1)), Checkpoint(()), Witness(Position(1), 0)]
cc 7cc1bd3b98a0ddbc7409077c010220ed8611936693eb955748df9d86167b1488 # shrinks to ops = [Append(SipHashable(0), Ephemeral), Append(SipHashable(0), Marked), Checkpoint(()), Witness(Position(1), 1)]
cc 7e6c5a3b74e14acaf101f9d411e3d7af878c335abacc6cdbbe92615426a6c277 # shrinks to ops = [Append("a", Ephemeral), Append("a", Ephemeral), Append("a", Ephemeral), CurrentPosition, CurrentPosition, Append("a", Ephemeral), Append("a", Ephemeral), Append("a", Ephemeral), Append("a", Marked), Checkpoint(()), Checkpoint(()), Checkpoint(()), Checkpoint(()), Checkpoint(()), Witness(Position(6), 5)]
cc 04866a1bb2691ed38b853576489ad858a2d0e09e1579e120fd825e9f809d544b # shrinks to ops = [Append("a", Checkpoint { id: (), marking: Marked }), Append("a", Checkpoint { id: (), marking: Marked }), Checkpoint(()), Checkpoint(()), Append("a", Checkpoint { id: (), marking: Marked }), Append("a", Ephemeral), Append("a", Ephemeral), Append("a", Ephemeral), Append("a", Ephemeral), Append("a", Checkpoint { id: (), marking: Marked }), Witness(Position(0), 1)]
32 changes: 24 additions & 8 deletions shardtree/src/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ impl<
///
/// This method operates on a single thread. If you have parallelism available, consider using
/// [`LocatedPrunableTree::from_iter`] and [`Self::insert_tree`] instead.
///
/// [`Node::Nil`]: crate::tree::Node::Nil
#[allow(clippy::type_complexity)]
pub fn batch_insert<I: Iterator<Item = (H, Retention<C>)>>(
&mut self,
Expand Down Expand Up @@ -96,6 +98,8 @@ pub struct BatchInsertionResult<H, C: Ord, I: Iterator<Item = (H, Retention<C>)>
/// The vector of addresses of [`Node::Nil`] nodes that were inserted into the tree as part of
/// the insertion operation, for nodes that are required in order to construct a witness for
/// each inserted leaf with [`Retention::Marked`] retention.
///
/// [`Node::Nil`]: crate::tree::Node::Nil
pub incomplete: Vec<IncompleteAt>,
/// The maximum position at which a leaf was inserted.
pub max_insert_position: Option<Position>,
Expand Down Expand Up @@ -507,8 +511,8 @@ mod tests {
fn batch_insert_matches_insert_tree() {
{
let (lhs, rhs) = build_insert_tree_and_batch_insert(vec![]);
assert_eq!(lhs.max_leaf_position(0), Ok(None));
assert_eq!(rhs.max_leaf_position(0), Ok(None));
assert_eq!(lhs.max_leaf_position(None), Ok(None));
assert_eq!(rhs.max_leaf_position(None), Ok(None));
}

for i in 0..64 {
Expand All @@ -524,11 +528,23 @@ mod tests {
});

let (lhs, rhs) = build_insert_tree_and_batch_insert(leaves);
assert_eq!(lhs.max_leaf_position(0), Ok(Some(Position::from(i as u64))));
assert_eq!(rhs.max_leaf_position(0), Ok(Some(Position::from(i as u64))));

assert_eq!(lhs.root_at_checkpoint_depth(0).unwrap(), expected_root);
assert_eq!(rhs.root_at_checkpoint_depth(0).unwrap(), expected_root);
assert_eq!(
lhs.max_leaf_position(None),
Ok(Some(Position::from(i as u64)))
);
assert_eq!(
rhs.max_leaf_position(None),
Ok(Some(Position::from(i as u64)))
);

assert_eq!(
lhs.root_at_checkpoint_depth(None).unwrap().as_ref(),
Some(&expected_root)
);
assert_eq!(
rhs.root_at_checkpoint_depth(None).unwrap().as_ref(),
Some(&expected_root)
);
}
}
}
Expand All @@ -548,7 +564,7 @@ mod proptests {
let (left, right) = build_insert_tree_and_batch_insert(leaves);

// Check that the resulting trees are equal.
assert_eq!(left.root_at_checkpoint_depth(0), right.root_at_checkpoint_depth(0));
assert_eq!(left.root_at_checkpoint_depth(None), right.root_at_checkpoint_depth(None));
}
}
}
Loading

0 comments on commit 9a77e51

Please sign in to comment.