diff --git a/shardtree/CHANGELOG.md b/shardtree/CHANGELOG.md index 2f7fd13..a14a872 100644 --- a/shardtree/CHANGELOG.md +++ b/shardtree/CHANGELOG.md @@ -5,6 +5,29 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- Checkpoint truncation now discards cached cap roots that commit to positions + beyond the checkpoint, so replacement commitments produce the correct root. +- `shardtree::LocatedPrunableTree::truncate_to_position` no longer retains the + cached root annotations of the parent nodes it reconstructs along the + truncation path. Such an annotation is a hash over the parent's complete + subtree — including the data being truncated away — and a stale annotation + caused spurious `Insert(Conflict(..))` errors (or silently incorrect roots) + when the truncated region was later refilled with different data, e.g. when + re-scanning a divergent chain after a reorg. +- `ShardTree::truncate_to_checkpoint` and + `ShardTree::truncate_to_checkpoint_depth` now fail with + `QueryError::CheckpointPruned`, leaving the store unmodified, when the leaf + at the checkpoint's position is not individually represented in the tree + (because it has been pruned into a merged hash, or removed entirely): + truncation at such a position would have to discard retained data along with + the requested suffix. Previously the shard and checkpoint truncation was + silently skipped — after the cap had already been modified — while still + reporting success, leaving the store internally inconsistent with all + post-checkpoint state in place. + ## [0.7.0] - 2026-07-09 ### Added @@ -43,7 +66,6 @@ and this project adheres to Rust's notion of checkpoints sharing a tree position, as a stalled chain produces, triggered it within a single pruning window. - ## [0.6.2] - 2026-02-20 ### Added diff --git a/shardtree/src/lib.rs b/shardtree/src/lib.rs index 2457ced..e1ab619 100644 --- a/shardtree/src/lib.rs +++ b/shardtree/src/lib.rs @@ -806,6 +806,11 @@ impl< /// Returns `true` if the truncation succeeds or has no effect, or `false` if no checkpoint /// exists at the specified depth. Depth 0 refers to the most recent checkpoint in the tree; /// + /// Returns [`QueryError::CheckpointPruned`] if the leaf at the checkpoint's position is not + /// individually represented in the tree (it has been pruned into a merged hash, or removed + /// entirely), since truncation at such a position would have to discard retained data along + /// with the requested suffix; the store is left unmodified in this case. + /// /// ## Parameters /// - `checkpoint_depth`: A zero-based index over the checkpoints that have been added to the /// tree, in reverse checkpoint identifier order. @@ -829,6 +834,11 @@ impl< /// /// Returns `true` if the truncation succeeds or has no effect, or `false` if no checkpoint /// exists for the specified checkpoint identifier. + /// + /// Returns [`QueryError::CheckpointPruned`] if the leaf at the checkpoint's position is not + /// individually represented in the tree (it has been pruned into a merged hash, or removed + /// entirely), since truncation at such a position would have to discard retained data along + /// with the requested suffix; the store is left unmodified in this case. pub fn truncate_to_checkpoint( &mut self, checkpoint_id: &C, @@ -863,35 +873,39 @@ impl< .map_err(ShardTreeError::Storage)?; } TreeState::AtPosition(position) => { + // Truncation is only possible if the boundary shard individually represents + // the checkpoint's position; if the leaf at that position has been pruned + // (merged into a larger hash, or removed entirely), fail — before any part + // of the store has been modified — rather than either discarding retained + // data the caller asked to keep or leaving the discarded suffix in place. let subtree_addr = Self::subtree_addr(position); - let replacement = self + let truncated_shard = self .store .get_shard(subtree_addr) .map_err(ShardTreeError::Storage)? - .and_then(|s| s.truncate_to_position(position)); + .and_then(|s| s.truncate_to_position(position)) + .ok_or(ShardTreeError::Query(QueryError::CheckpointPruned))?; + // The cap's nodes are caches of hashes recomputable from the shards, so a + // cached entry spanning the truncation boundary is discarded (lossy + // truncation) rather than making the position unusable. let cap_tree = LocatedTree { root_addr: Self::root_addr(), root: self.store.get_cap().map_err(ShardTreeError::Storage)?, }; + self.store + .put_cap(cap_tree.truncate_cache_to_position(position).root) + .map_err(ShardTreeError::Storage)?; - if let Some(truncated_cap) = cap_tree.truncate_to_position(position) { - self.store - .put_cap(truncated_cap.root) - .map_err(ShardTreeError::Storage)?; - }; - - if let Some(truncated) = replacement { - self.store - .truncate_shards(subtree_addr.index()) - .map_err(ShardTreeError::Storage)?; - self.store - .put_shard(truncated) - .map_err(ShardTreeError::Storage)?; - self.store - .truncate_checkpoints_retaining(checkpoint_id) - .map_err(ShardTreeError::Storage)?; - } + self.store + .truncate_shards(subtree_addr.index()) + .map_err(ShardTreeError::Storage)?; + self.store + .put_shard(truncated_shard) + .map_err(ShardTreeError::Storage)?; + self.store + .truncate_checkpoints_retaining(checkpoint_id) + .map_err(ShardTreeError::Storage)?; } } @@ -1657,12 +1671,12 @@ mod tests { use crate::{ error::{QueryError, ShardTreeError}, - store::{memory::MemoryShardStore, ShardStore}, + store::{memory::MemoryShardStore, Checkpoint, ShardStore}, testing::{ arb_char_str, arb_shard_layout, arb_shardtree_sized, check_shard_sizes, check_shardtree_insertion, check_witness_with_pruned_subtrees, }, - InsertionError, LocatedPrunableTree, LocatedTree, ShardTree, + InsertionError, LocatedPrunableTree, LocatedTree, RetentionFlags, ShardTree, Tree, }; #[test] @@ -2487,6 +2501,139 @@ mod tests { ); } + #[test] + fn truncate_to_checkpoint_discards_stale_annotations() { + // Regression test: `LocatedPrunableTree::truncate_to_position` reconstructed + // the parent nodes along the truncation path with their cached root annotations + // intact. Such an annotation is a hash over the parent's complete subtree — + // including the data being truncated away — so a retained annotation caused + // spurious `Conflict` errors (or silently incorrect roots) once the truncated + // region was refilled with different data, as happens when a wallet re-scans a + // divergent chain after a reorg. + let mut tree = empty_tree::(); + + // The original chain: leaves at positions 0..=5, checkpointed at positions 4 + // and 5. + for (leaf, retention) in [ + ("a", Retention::Ephemeral), + ("b", Retention::Ephemeral), + ("c", Retention::Ephemeral), + ("d", Retention::Ephemeral), + ( + "e", + Retention::Checkpoint { + id: 1, + marking: Marking::None, + }, + ), + ( + "f", + Retention::Checkpoint { + id: 2, + marking: Marking::None, + }, + ), + ] { + tree.append(leaf.to_string(), retention).unwrap(); + } + + // Inserting a frontier at position 6 (as a wallet does with the chain state at + // the start of each scanned batch) writes the frontier's ommers into the tree; + // the ommer covering positions 4..=5 is recorded as a cached annotation on the + // existing parent node for those positions. + tree.insert_frontier( + Frontier::from_parts( + Position::from(6), + "g".to_string(), + vec!["ef".to_string(), "abcd".to_string()], + ) + .unwrap(), + Retention::Checkpoint { + id: 3, + marking: Marking::None, + }, + ) + .unwrap(); + + // Truncate back to the checkpoint at position 4, discarding the data at + // positions 5 and 6. + assert!(tree.truncate_to_checkpoint(&1).unwrap()); + + // On the divergent chain, positions 5 and 6 hold different leaves. Inserting + // its frontier must succeed: nothing of the discarded chain may remain in the + // tree to conflict with the replacement data. + tree.insert_frontier( + Frontier::from_parts( + Position::from(6), + "y".to_string(), + vec!["ex".to_string(), "abcd".to_string()], + ) + .unwrap(), + Retention::Checkpoint { + id: 4, + marking: Marking::None, + }, + ) + .unwrap(); + + // The tree's state at the new checkpoint reflects the divergent chain. + assert_eq!( + tree.root_at_checkpoint_id(&4).unwrap(), + Some("abcdexy_________".to_string()), + ); + } + + #[test] + fn truncate_to_checkpoint_fails_within_pruned_subtree() { + // Regression test: when the checkpoint's position fell in the interior of a + // pruned subtree (a merged hash node), `truncate_to_position` returned `None`, + // and `truncate_to_checkpoint` reacted by silently skipping the truncation + // while still reporting success, leaving all post-checkpoint state in place + // (and, before the preceding cap fix, having already truncated the cap, + // leaving the store internally inconsistent). + // + // Truncation at such a position must instead fail entirely, without modifying + // the store: succeeding would require discarding retained data along with the + // requested suffix, and it is the caller's responsibility to select a position + // at which truncation is possible. + let shard = LocatedTree { + root_addr: Address::from_parts(Level::from(2), 0), + root: Tree::parent( + None, + Tree::leaf(("ab".to_string(), RetentionFlags::EPHEMERAL)), + Tree::leaf(("cd".to_string(), RetentionFlags::EPHEMERAL)), + ), + }; + let mut tree = empty_tree::(); + tree.store.put_shard(shard.clone()).unwrap(); + tree.store + .add_checkpoint(1, Checkpoint::at_position(Position::from(0))) + .unwrap(); + tree.store + .add_checkpoint(2, Checkpoint::at_position(Position::from(3))) + .unwrap(); + + assert_matches!( + tree.truncate_to_checkpoint(&1), + Err(ShardTreeError::Query(QueryError::CheckpointPruned)) + ); + + // The store is unmodified: both checkpoints and the full shard contents remain. + assert_eq!(tree.store.checkpoint_count().unwrap(), 2); + assert_eq!( + tree.store + .get_shard(Address::from_parts(Level::from(2), 0)) + .unwrap() + .map(|s| s.root), + Some(shard.root), + ); + + // Truncating to the checkpoint at position 3 — the merged subtree's maximum + // position — is possible, and removes nothing. + assert!(tree.truncate_to_checkpoint(&2).unwrap()); + assert_eq!(tree.store.checkpoint_count().unwrap(), 2); + } + #[test] fn cached_parent_annotation_does_not_short_circuit_truncation() { // Regression test: the Parent handler's annotation fast-path in root_internal @@ -2625,6 +2772,84 @@ mod tests { ); } + #[test] + fn truncate_clears_cap_hashes_beyond_checkpoint() { + fn insert_first_shard(tree: &mut ShardTree, 6, 3>) { + tree.batch_insert( + Position::from(0), + ('a'..='h').map(|c| { + ( + c.to_string(), + if c == 'h' { + Retention::Checkpoint { + id: 1, + marking: Marking::None, + } + } else { + Retention::Ephemeral + }, + ) + }), + ) + .unwrap(); + } + + let mut tree: ShardTree, 6, 3> = + ShardTree::new(MemoryShardStore::empty(), 100); + + // Simulate imported subtree roots and cache their combined hash. + tree.insert( + Address::from_parts(Level::from(3), 0), + "abcdefgh".to_string(), + ) + .unwrap(); + tree.insert( + Address::from_parts(Level::from(3), 1), + "ijklmnop".to_string(), + ) + .unwrap(); + assert_eq!( + tree.root_caching( + ShardTree::, 6, 3>::root_addr(), + Position::from(16), + ), + Ok(format!("abcdefghijklmnop{}", "_".repeat(48))) + ); + + // Scanning replaces the first imported shard root with its leaves and records a + // checkpoint at the end of that shard. + insert_first_shard(&mut tree); + + assert_eq!(tree.truncate_to_checkpoint(&1), Ok(true)); + + // The imported root committed to both shards. After truncation, inserting a + // replacement second shard must not reuse that stale cached root. + tree.insert( + Address::from_parts(Level::from(3), 1), + "IJKLMNOP".to_string(), + ) + .unwrap(); + let expected_root = format!("abcdefghIJKLMNOP{}", "_".repeat(48)); + assert_eq!( + tree.root_at_checkpoint_depth(None), + Ok(Some(expected_root.clone())) + ); + + let mut reference: ShardTree, 6, 3> = + ShardTree::new(MemoryShardStore::empty(), 100); + insert_first_shard(&mut reference); + reference + .insert( + Address::from_parts(Level::from(3), 1), + "IJKLMNOP".to_string(), + ) + .unwrap(); + assert_eq!( + reference.root_at_checkpoint_depth(None), + Ok(Some(expected_root)) + ); + } + #[test] fn insert_frontier_nodes_sub_shard_height() { let mut frontier = NonEmptyFrontier::new("a".to_string()); diff --git a/shardtree/src/prunable.rs b/shardtree/src/prunable.rs index b099d62..55cff7d 100644 --- a/shardtree/src/prunable.rs +++ b/shardtree/src/prunable.rs @@ -731,12 +731,20 @@ where } } - /// Prunes this tree by replacing all nodes that are right-hand children along the path - /// to the specified position with [`Node::Nil`]. + /// Truncates this tree by discarding all data at positions greater than `position`. /// - /// The leaf at the specified position is retained. Returns the truncated tree if a leaf or - /// subtree root with the specified position as its maximum position exists, or `None` - /// otherwise. + /// The leaf at `position`, and all data at lesser positions, is retained. Cached root + /// annotations of parent nodes whose subtrees lose data in the truncation are discarded, + /// since such an annotation is a hash over the parent's complete subtree, including the + /// discarded data; subtrees lying entirely at or below `position` are retained unmodified, + /// annotations included. + /// + /// Truncation is only possible at a position that the tree individually represents: + /// returns `None` if `position` is outside the position range of this tree, if it falls in + /// the interior of a pruned subtree whose leaves have been merged into a single hash (the + /// hash incorporates data beyond `position`, and its constituent parts cannot be + /// recovered), or if the tree contains no data at `position`. It is the caller's + /// responsibility to select a position at which truncation is possible. pub fn truncate_to_position(&self, position: Position) -> Option { /// Pre-condition: `root_addr` must be the address of `root`. fn go( @@ -747,8 +755,19 @@ where where H: Hashable + Clone + PartialEq, { + if root_addr.max_position() <= position { + // Nothing above `position` exists within this subtree, so it is retained + // unmodified, along with any cached root annotation. + return Some(root.clone()); + } match &root.0 { - Node::Parent { ann, left, right } => { + Node::Parent { + ann: _, + left, + right, + } => { + // This node's subtree loses data in the truncation, so any cached root + // annotation for it (a hash over its complete subtree) is discarded. let (l_child, r_child) = root_addr .children() .expect("has children because we checked `root` is a parent"); @@ -756,25 +775,25 @@ where // we are truncating within the range of the left node, so recurse // to the left to truncate the left child and then reconstruct the // node with `Nil` as the right sibling - go(position, l_child, left.as_ref()).map(|left| { - Tree::unite(l_child.level(), ann.clone(), left, Tree::empty()) - }) + go(position, l_child, left.as_ref()) + .map(|left| Tree::unite(l_child.level(), None, left, Tree::empty())) } else { // we are truncating within the range of the right node, so recurse // to the right to truncate the right child and then reconstruct the // node with the left sibling unchanged go(position, r_child, right.as_ref()).map(|right| { - Tree::unite(r_child.level(), ann.clone(), left.as_ref().clone(), right) + Tree::unite(r_child.level(), None, left.as_ref().clone(), right) }) } } - Node::Leaf { .. } => { - if root_addr.max_position() <= position { - Some(root.clone()) - } else { - None - } - } + // The truncation position falls in the interior of a pruned subtree whose + // leaves have been merged into a single hash (the guard above establishes + // that this subtree extends beyond `position`). The merged hash incorporates + // data beyond the truncation position, and the constituent parts from which + // a replacement hash could be computed are unrecoverable, so truncation at + // this position is not possible. + Node::Leaf { .. } => None, + // The tree contains no data at `position`. Node::Nil => None, } } @@ -789,6 +808,53 @@ where } } + /// Removes cached nodes and annotations that commit to positions beyond `position`. + /// + /// Unlike [`Self::truncate_to_position`], this may discard a cached leaf that spans the + /// boundary because the underlying shard data remains authoritative. + pub(crate) fn truncate_cache_to_position(&self, position: Position) -> Self { + /// Pre-condition: `root_addr` must be the address of `root`. + fn go(position: Position, root_addr: Address, root: &PrunableTree) -> PrunableTree + where + H: Hashable + Clone + PartialEq, + { + if position < root_addr.position_range_start() { + Tree::empty() + } else if root_addr.max_position() <= position { + root.clone() + } else { + match &root.0 { + Node::Parent { left, right, .. } => { + let (l_child, r_child) = root_addr + .children() + .expect("has children because we checked `root` is a parent"); + if position < r_child.position_range_start() { + Tree::unite( + l_child.level(), + None, + go(position, l_child, left.as_ref()), + Tree::empty(), + ) + } else { + Tree::unite( + l_child.level(), + None, + left.as_ref().clone(), + go(position, r_child, right.as_ref()), + ) + } + } + Node::Leaf { .. } | Node::Nil => Tree::empty(), + } + } + } + + LocatedTree { + root_addr: self.root_addr, + root: go(position, self.root_addr, &self.root), + } + } + /// Inserts a descendant subtree into this subtree, creating empty sibling nodes as necessary /// to fill out the tree. /// @@ -1304,6 +1370,7 @@ fn accumulate_result_with( mod tests { use assert_matches::assert_matches; use std::collections::{BTreeMap, BTreeSet}; + use std::sync::Arc; use incrementalmerkletree::{frontier::NonEmptyFrontier, Address, Level, Position}; use proptest::proptest; @@ -1315,7 +1382,7 @@ mod tests { testing::{arb_char_str, arb_prunable_tree}, tree::{ tests::{leaf, nil, parent}, - LocatedTree, + LocatedTree, Tree, }, }; @@ -1389,6 +1456,157 @@ mod tests { ); } + #[test] + fn truncate_cache_to_position_discards_nodes_that_span_cut() { + let root_addr = Address::from_parts(2.into(), 0); + let tree: LocatedPrunableTree = LocatedTree { + root_addr, + root: parent( + parent( + leaf(("a".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("b".to_string(), RetentionFlags::EPHEMERAL)), + ), + leaf(("cd".to_string(), RetentionFlags::EPHEMERAL)), + ), + }; + assert_eq!( + tree.truncate_cache_to_position(Position::from(2)), + LocatedTree { + root_addr, + root: parent( + parent( + leaf(("a".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("b".to_string(), RetentionFlags::EPHEMERAL)), + ), + nil(), + ), + } + ); + + let annotated: LocatedPrunableTree = LocatedTree { + root_addr, + root: Tree::parent( + Some(Arc::new("abcd".to_string())), + Tree::parent( + Some(Arc::new("ab".to_string())), + leaf(("a".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("b".to_string(), RetentionFlags::EPHEMERAL)), + ), + parent( + leaf(("c".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("d".to_string(), RetentionFlags::EPHEMERAL)), + ), + ), + }; + assert_eq!( + annotated.truncate_cache_to_position(Position::from(2)), + LocatedTree { + root_addr, + root: Tree::parent( + None, + Tree::parent( + Some(Arc::new("ab".to_string())), + leaf(("a".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("b".to_string(), RetentionFlags::EPHEMERAL)), + ), + parent(leaf(("c".to_string(), RetentionFlags::EPHEMERAL)), nil()), + ), + } + ); + } + + #[test] + fn truncate_to_position_discards_annotations() { + // A cached annotation on a parent node is a hash over the parent's complete + // subtree — including any data above the truncation position — so truncation + // along a path through an annotated parent must discard the annotation. + let t: LocatedPrunableTree = LocatedTree { + root_addr: Address::from_parts(Level::from(2), 0), + root: Tree::parent( + Some(Arc::new("abcd".to_string())), + parent( + leaf(("a".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("b".to_string(), RetentionFlags::EPHEMERAL)), + ), + parent( + leaf(("c".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("d".to_string(), RetentionFlags::EPHEMERAL)), + ), + ), + }; + + let truncated = t + .truncate_to_position(Position::from(1)) + .expect("position is in range"); + assert_eq!( + truncated.root, + Tree::parent( + None, + parent( + leaf(("a".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("b".to_string(), RetentionFlags::EPHEMERAL)), + ), + nil() + ) + ); + + // Truncating at the tree's max position removes nothing: the tree is retained + // unchanged, including its cached annotations. + let untouched = t + .truncate_to_position(Position::from(3)) + .expect("position is in range"); + assert_eq!(untouched.root, t.root); + } + + #[test] + fn truncate_to_position_fails_inside_pruned_subtree() { + // When the truncation position falls in the interior of a pruned subtree whose + // leaves have been merged into a single hash, the merged hash incorporates data + // above the truncation position and its constituent parts cannot be recovered: + // truncation at that position must be refused. It is the caller's responsibility + // to select a position at which truncation is possible. + let t: LocatedPrunableTree = LocatedTree { + root_addr: Address::from_parts(Level::from(2), 0), + root: parent( + leaf(("ab".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("cd".to_string(), RetentionFlags::EPHEMERAL)), + ), + }; + + assert_eq!(t.truncate_to_position(Position::from(0)), None); + + // Truncating at the merged node's max position retains it whole. + let truncated = t + .truncate_to_position(Position::from(1)) + .expect("position is in range"); + assert_eq!( + truncated.root, + parent(leaf(("ab".to_string(), RetentionFlags::EPHEMERAL)), nil()) + ); + } + + #[test] + fn truncate_cache_to_position_discards_boundary_spanning_nodes() { + // The cache variant used for the cap discards a node whose merged hash spans + // the truncation boundary, along with everything above it, instead of refusing + // as `truncate_to_position` does on the same tree: every node in such a + // structure is a cache of data recoverable from elsewhere. + let t: LocatedPrunableTree = LocatedTree { + root_addr: Address::from_parts(Level::from(2), 0), + root: parent( + leaf(("ab".to_string(), RetentionFlags::EPHEMERAL)), + leaf(("cd".to_string(), RetentionFlags::EPHEMERAL)), + ), + }; + + assert_eq!(t.truncate_cache_to_position(Position::from(0)).root, nil()); + assert_eq!( + t.truncate_cache_to_position(Position::from(1)).root, + parent(leaf(("ab".to_string(), RetentionFlags::EPHEMERAL)), nil()) + ); + assert_eq!(t.truncate_cache_to_position(Position::from(3)).root, t.root); + } + #[test] fn merge_checked() { let t0: PrunableTree =