From 4b203aa79bfc8b82eeae24a015241f2c90eeb2e4 Mon Sep 17 00:00:00 2001 From: Michael Korovkin Date: Thu, 8 Jan 2026 15:30:23 -0800 Subject: [PATCH 1/4] Addressing todo item: optimize PMMR segments to only include hashes at pruning boundaries; tests updated as segment size dropped from 521 to 281 bytes (7 hashes -> 1 hash) --- chain/src/txhashset/segmenter.rs | 10 ++++--- chain/tests/bitmap_segment.rs | 2 +- core/src/core/pmmr/segment.rs | 51 +++++++++++++++++++++++++++----- core/tests/segment.rs | 2 +- store/tests/segment.rs | 35 +++++++++++----------- 5 files changed, 70 insertions(+), 30 deletions(-) diff --git a/chain/src/txhashset/segmenter.rs b/chain/src/txhashset/segmenter.rs index ed6a2aee22..345a25829b 100644 --- a/chain/src/txhashset/segmenter.rs +++ b/chain/src/txhashset/segmenter.rs @@ -57,7 +57,7 @@ impl Segmenter { let now = Instant::now(); let txhashset = self.txhashset.read(); let kernel_pmmr = txhashset.kernel_pmmr_at(&self.header); - let segment = Segment::from_pmmr(id, &kernel_pmmr, false)?; + let segment = Segment::from_pmmr(id, &kernel_pmmr, None)?; debug!( "kernel_segment: id: ({}, {}), leaves: {}, hashes: {}, proof hashes: {}, took {}ms", segment.id().height, @@ -93,7 +93,7 @@ impl Segmenter { ) -> Result<(Segment, Hash), Error> { let now = Instant::now(); let bitmap_pmmr = self.bitmap_snapshot.readonly_pmmr(); - let segment = Segment::from_pmmr(id, &bitmap_pmmr, false)?; + let segment = Segment::from_pmmr(id, &bitmap_pmmr, None)?; let output_root = self.output_root()?; debug!( "bitmap_segment: id: ({}, {}), leaves: {}, hashes: {}, proof hashes: {}, took {}ms", @@ -113,9 +113,10 @@ impl Segmenter { id: SegmentIdentifier, ) -> Result<(Segment, Hash), Error> { let now = Instant::now(); + let bitmap = self.bitmap_snapshot.as_bitmap().ok(); let txhashset = self.txhashset.read(); let output_pmmr = txhashset.output_pmmr_at(&self.header); - let segment = Segment::from_pmmr(id, &output_pmmr, true)?; + let segment = Segment::from_pmmr(id, &output_pmmr, bitmap.as_ref())?; let bitmap_root = self.bitmap_root()?; debug!( "output_segment: id: ({}, {}), leaves: {}, hashes: {}, proof hashes: {}, took {}ms", @@ -132,9 +133,10 @@ impl Segmenter { /// Create a rangeproof segment. pub fn rangeproof_segment(&self, id: SegmentIdentifier) -> Result, Error> { let now = Instant::now(); + let bitmap = self.bitmap_snapshot.as_bitmap().ok(); let txhashset = self.txhashset.read(); let pmmr = txhashset.rangeproof_pmmr_at(&self.header); - let segment = Segment::from_pmmr(id, &pmmr, true)?; + let segment = Segment::from_pmmr(id, &pmmr, bitmap.as_ref())?; debug!( "rangeproof_segment: id: ({}, {}), leaves: {}, hashes: {}, proof hashes: {}, took {}ms", segment.id().height, diff --git a/chain/tests/bitmap_segment.rs b/chain/tests/bitmap_segment.rs index 39092b8e9a..ed5f3340ee 100644 --- a/chain/tests/bitmap_segment.rs +++ b/chain/tests/bitmap_segment.rs @@ -65,7 +65,7 @@ fn test_roundtrip(entries: usize) { .unwrap(); let mmr = accumulator.readonly_pmmr(); - let segment = Segment::from_pmmr(identifier, &mmr, false).unwrap(); + let segment = Segment::from_pmmr(identifier, &mmr, None).unwrap(); // Convert to `BitmapSegment` let bms = BitmapSegment::from(segment.clone()); diff --git a/core/src/core/pmmr/segment.rs b/core/src/core/pmmr/segment.rs index f9f555bd40..cde8e16f7e 100644 --- a/core/src/core/pmmr/segment.rs +++ b/core/src/core/pmmr/segment.rs @@ -340,10 +340,11 @@ where T: Readable + Writeable + Debug, { /// Generate a segment from a PMMR + /// If bitmap is provided, only hashes at pruning boundaries are included. pub fn from_pmmr( segment_id: SegmentIdentifier, pmmr: &ReadonlyPMMR<'_, U, B>, - prunable: bool, + bitmap: Option<&Bitmap>, ) -> Result where U: PMMRable, @@ -364,15 +365,40 @@ where segment.leaf_data.push(data); segment.leaf_pos.push(pos0); continue; - } else if !prunable { + } else if bitmap.is_none() { return Err(SegmentError::MissingLeaf(pos0)); } } - // TODO: optimize, no need to send every intermediary hash - if prunable { - if let Some(hash) = pmmr.get_from_file(pos0) { - segment.hashes.push(hash); - segment.hash_pos.push(pos0); + if let Some(bm) = bitmap { + // Only include hash if this subtree is fully pruned + // AND the sibling subtree is NOT fully pruned (pruning boundary) + if subtree_fully_pruned(pos0, bm, mmr_size) { + // Find sibling position + let height = pmmr::bintree_postorder_height(pos0); + let subtree_size = (1u64 << (height + 1)) - 1; + let sibling_pos0 = if pmmr::is_left_sibling(pos0) { + // Right sibling is at pos0 + size of this subtree + Some(pos0 + subtree_size) + } else { + // Left sibling: go back by sibling's subtree size + pos0.checked_sub(subtree_size) + }; + // Need hash if sibling exists in segment and is not fully pruned + let need_hash = match sibling_pos0 { + Some(sib) if sib >= segment_first_pos && sib <= segment_last_pos => { + !subtree_fully_pruned(sib, bm, mmr_size) + } + _ => { + // Sibling outside segment or underflow - may need hash for proof + true + } + }; + if need_hash { + if let Some(hash) = pmmr.get_from_file(pos0) { + segment.hashes.push(hash); + segment.hash_pos.push(pos0); + } + } } } } @@ -845,3 +871,14 @@ impl Writeable for SegmentProof { Ok(()) } } + +/// Check if a subtree rooted at pos0 is fully pruned (no unspent leaves in bitmap) +fn subtree_fully_pruned(pos0: u64, bitmap: &Bitmap, mmr_size: u64) -> bool { + let leftmost = pmmr::bintree_leftmost(pos0); + let rightmost = pmmr::bintree_rightmost(pos0); + let n_leaves = pmmr::n_leaves(mmr_size); + let start_leaf = pmmr::n_leaves(leftmost + 1).saturating_sub(1); + let end_leaf = min(pmmr::n_leaves(rightmost + 1), n_leaves); + // If any leaf in range is in bitmap (unspent), subtree is not fully pruned + bitmap.range_cardinality(start_leaf as u32..end_leaf as u32) == 0 +} diff --git a/core/tests/segment.rs b/core/tests/segment.rs index 3ab04f7a4b..c59af3b1df 100644 --- a/core/tests/segment.rs +++ b/core/tests/segment.rs @@ -41,7 +41,7 @@ fn test_unprunable_size(height: u8, n_leaves: u32) { for idx in 0..n_segments { let id = SegmentIdentifier { height, idx }; - let segment = Segment::from_pmmr(id, &mmr, false).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, None).unwrap(); println!( "\n\n>>>>>>> N_LEAVES = {}, LAST_POS = {}, SEGMENT = {}:\n{:#?}", n_leaves, last_pos, idx, segment diff --git a/store/tests/segment.rs b/store/tests/segment.rs index a24c1b8c5c..49ec24f3d6 100644 --- a/store/tests/segment.rs +++ b/store/tests/segment.rs @@ -54,7 +54,7 @@ fn prunable_mmr() { // Validate a segment before any pruning let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!( segment.root(last_pos, Some(&bitmap)).unwrap().unwrap(), mmr.get_hash(29).unwrap() @@ -70,7 +70,7 @@ fn prunable_mmr() { // Validate let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!( segment.root(last_pos, Some(&bitmap)).unwrap().unwrap(), mmr.get_hash(29).unwrap() @@ -86,7 +86,7 @@ fn prunable_mmr() { // Validate let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!( segment.root(last_pos, Some(&bitmap)).unwrap().unwrap(), mmr.get_hash(29).unwrap() @@ -102,7 +102,7 @@ fn prunable_mmr() { // Validate let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!( segment.root(last_pos, Some(&bitmap)).unwrap().unwrap(), mmr.get_hash(29).unwrap() @@ -117,13 +117,13 @@ fn prunable_mmr() { ba.sync().unwrap(); let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - assert!(Segment::from_pmmr(id, &mmr, true).is_ok()); + assert!(Segment::from_pmmr(id, &mmr, Some(&bitmap)).is_ok()); // Final segment is not full, test it before pruning let id = SegmentIdentifier { height: 3, idx: 9 }; let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); segment.validate(last_pos, Some(&bitmap), root).unwrap(); // Prune second and third to last leaves (a full peak in the MMR) @@ -135,7 +135,7 @@ fn prunable_mmr() { // Validate let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); segment.validate(last_pos, Some(&bitmap), root).unwrap(); // Prune final element @@ -147,7 +147,7 @@ fn prunable_mmr() { // Validate let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); segment.validate(last_pos, Some(&bitmap), root).unwrap(); std::mem::drop(ba); @@ -185,7 +185,7 @@ fn pruned_segment() { // Validate the empty segment 1 let id = SegmentIdentifier { height: 2, idx: 1 }; let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!(segment.leaf_iter().count(), 0); assert_eq!(segment.hash_iter().count(), 1); assert_eq!( @@ -206,7 +206,7 @@ fn pruned_segment() { // Validate the empty segment 1 again let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!(segment.leaf_iter().count(), 0); assert_eq!(segment.hash_iter().count(), 1); // Since both 7 and 14 are now pruned, the first unpruned hash will be at 15 @@ -228,7 +228,7 @@ fn pruned_segment() { // Validate the empty segment 1 again let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!(segment.leaf_iter().count(), 0); assert_eq!(segment.hash_iter().count(), 1); // Since both 15 and 30 are now pruned, the first unpruned hash will be at 31: the mmr root @@ -260,7 +260,7 @@ fn pruned_segment() { // Validate segment 4 let id = SegmentIdentifier { height: 2, idx: 4 }; let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!(segment.leaf_iter().count(), 0); assert_eq!(segment.hash_iter().count(), 1); assert_eq!( @@ -275,7 +275,7 @@ fn pruned_segment() { // Segment 5 has 2 peaks let id = SegmentIdentifier { height: 2, idx: 5 }; let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!(segment.leaf_iter().count(), 3); assert_eq!( segment @@ -297,7 +297,7 @@ fn pruned_segment() { // Segment 5 should be unchanged let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!(segment, prev_segment); segment.validate(last_pos, Some(&bitmap), root).unwrap(); @@ -310,7 +310,7 @@ fn pruned_segment() { // Validate segment 5 again let mmr = ReadonlyPMMR::at(&mut ba, last_pos); - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); assert_eq!(segment.leaf_iter().count(), 1); assert_eq!(segment.hash_iter().count(), 1); assert_eq!( @@ -354,14 +354,15 @@ fn ser_round_trip() { let mmr = ReadonlyPMMR::at(&ba, last_pos); let id = SegmentIdentifier { height: 3, idx: 0 }; - let segment = Segment::from_pmmr(id, &mmr, true).unwrap(); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); let mut cursor = Cursor::new(Vec::::new()); let mut writer = BinWriter::new(&mut cursor, ProtocolVersion(1)); Writeable::write(&segment, &mut writer).unwrap(); + // With optimization, only 1 hash is needed (at pruning boundary) instead of 7 assert_eq!( cursor.position(), - (9) + (8 + 7 * (8 + 32)) + (8 + 6 * (8 + 16)) + (8 + 2 * 32) + (9) + (8 + 1 * (8 + 32)) + (8 + 6 * (8 + 16)) + (8 + 2 * 32) ); cursor.set_position(0); From e7b149914cfe0f8e0ab5132a058f9ba03c507f0c Mon Sep 17 00:00:00 2001 From: iho Date: Fri, 17 Jul 2026 08:01:53 +0300 Subject: [PATCH 2/4] Reject segment hashes that would overwrite already-applied PMMR positions apply_output_segment / apply_rangeproof_segment: a pruned-subtree hash must cover a region that is entirely absent locally. If any position under it was already applied (leftmost < current size), collapsing via push_pruned_subtree silently discards already-applied leaves/hashes and desyncs the prune list from the physical hash file. Detect this and return Error::InvalidSegment instead of corrupting local state. --- chain/src/txhashset/txhashset.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/chain/src/txhashset/txhashset.rs b/chain/src/txhashset/txhashset.rs index 5d88cd290d..e351143ce0 100644 --- a/chain/src/txhashset/txhashset.rs +++ b/chain/src/txhashset/txhashset.rs @@ -1426,6 +1426,22 @@ impl<'a> Extension<'a> { .rewind(0, &Bitmap::new()) .map_err(&Error::TxHashSetErr)?; } + // A pruned-subtree hash must cover a region that is + // entirely absent locally. If any position under + // pos0 was already applied (leftmost < current + // size), collapsing here would silently discard + // already-applied leaves/hashes and desynchronize + // the prune list from the physical hash file - + // reject instead of corrupting local state. + let leftmost = pmmr::bintree_leftmost(pos0); + if leftmost < self.output_pmmr.size { + return Err(Error::InvalidSegment(format!( + "output segment hash at pos {} would overwrite {} already-applied position(s) starting at {} (sparse/dense segment mismatch)", + pos0, + self.output_pmmr.size - leftmost, + leftmost + ))); + } self.output_pmmr .push_pruned_subtree(hashes[idx], pos0) .map_err(&Error::TxHashSetErr)?; @@ -1470,6 +1486,18 @@ impl<'a> Extension<'a> { .rewind(0, &Bitmap::new()) .map_err(&Error::TxHashSetErr)?; } + // See apply_output_segment: reject a collapse that + // would overwrite already-applied positions instead + // of silently corrupting the prune list. + let leftmost = pmmr::bintree_leftmost(pos0); + if leftmost < self.rproof_pmmr.size { + return Err(Error::InvalidSegment(format!( + "rangeproof segment hash at pos {} would overwrite {} already-applied position(s) starting at {} (sparse/dense segment mismatch)", + pos0, + self.rproof_pmmr.size - leftmost, + leftmost + ))); + } self.rproof_pmmr .push_pruned_subtree(hashes[idx], pos0) .map_err(&Error::TxHashSetErr)?; From 9f4f9a295a78a6e74ba289418fabf3e9d61f5c30 Mon Sep 17 00:00:00 2001 From: iho Date: Sat, 18 Jul 2026 08:24:26 +0300 Subject: [PATCH 3/4] Derive sparse segment hashes from receiver reconstruction, not the bitmap The bitmap-boundary heuristic decided hash inclusion from spent status alone while leaf inclusion was decided from on-disk presence. For a subtree fully spent per the bitmap but only partially compacted on disk (the normal state of a node between compaction passes) those two signals disagree: spent-but-present leaves were shipped as data AND collapsed into a higher boundary hash. Such a segment passes validation (the extra leaves are ignored by root reconstruction and the boundary hash is genuinely bound to the root), but apply pushes the leaves at the frontier and then registers the boundary subtree as pruned over them, desyncing the prune list from the hash file. The corruption only surfaced later as an unattributable Invalid Root during full state validation, causing the reported PIBD restart loop. Select leaves and hashes with the same bottom-up walk the receiver's root reconstruction performs: ship exactly the leaves root() consumes, then walk the segment in postorder tracking which positions are derivable from them, emitting a hash wherever reconstruction will look one up - the missing sibling at each partially-known parent, plus any subtree root left unknown at the top (fully pruned peaks of the final segment, or the root of a fully pruned segment). Producer and receiver can no longer disagree about which positions are covered by which hash, and redundant spent leaves fold into the boundary hash, shrinking the segment further. Adds store tests covering the mixed-compaction round trip through the desegmenter apply logic, plus a regression test documenting that the redundant leaf-plus-boundary-hash shape corrupts apply. --- core/src/core/pmmr/segment.rs | 138 ++++++++++++++++++-------- store/tests/segment.rs | 181 +++++++++++++++++++++++++++++++++- 2 files changed, 276 insertions(+), 43 deletions(-) diff --git a/core/src/core/pmmr/segment.rs b/core/src/core/pmmr/segment.rs index cde8e16f7e..f099ac09b7 100644 --- a/core/src/core/pmmr/segment.rs +++ b/core/src/core/pmmr/segment.rs @@ -341,6 +341,18 @@ where { /// Generate a segment from a PMMR /// If bitmap is provided, only hashes at pruning boundaries are included. + /// + /// Hash boundaries are computed via the same bottom-up postorder walk the + /// receiver-side reconstruction (`root`) performs, tracking which + /// positions are already derivable ("known") from the selected leaves, + /// instead of an independent bitmap-only heuristic. A position becomes + /// known if both children are known; if exactly one child is known, the + /// *other* child's hash is required and emitted (mirroring the + /// `(None, Some)` / `(Some, None)` lookups in `root`), after which the + /// parent is known too. This guarantees the produced segment is exactly + /// what `root` and `TxHashSet::apply_*_segment` expect: no position is + /// ever described twice (as both leaf data and part of a collapsed + /// ancestor hash), and no hash needed for reconstruction is missing. pub fn from_pmmr( segment_id: SegmentIdentifier, pmmr: &ReadonlyPMMR<'_, U, B>, @@ -357,50 +369,103 @@ where return Err(SegmentError::NonExistent); } - // Fill leaf data and hashes let (segment_first_pos, segment_last_pos) = segment.segment_pos_range(mmr_size); + + // Pass 1: select leaf data - present on disk, and (for prunable MMRs) + // required by the bitmap or its sibling, or the final uneven leaf: + // the exact set the receiver-side `root` consumes. for pos0 in segment_first_pos..=segment_last_pos { - if pmmr::is_leaf(pos0) { + if !pmmr::is_leaf(pos0) { + continue; + } + let include = match bitmap { + None => true, + Some(bm) => { + let idx_1 = pmmr::n_leaves(pos0 + 1) - 1; + let idx_2 = if pmmr::is_left_sibling(pos0) { + idx_1 + 1 + } else { + idx_1 - 1 + }; + bm.contains(idx_1 as u32) || bm.contains(idx_2 as u32) || pos0 == mmr_size - 1 + } + }; + if include { if let Some(data) = pmmr.get_data_from_file(pos0) { segment.leaf_data.push(data); segment.leaf_pos.push(pos0); - continue; } else if bitmap.is_none() { return Err(SegmentError::MissingLeaf(pos0)); } } - if let Some(bm) = bitmap { - // Only include hash if this subtree is fully pruned - // AND the sibling subtree is NOT fully pruned (pruning boundary) - if subtree_fully_pruned(pos0, bm, mmr_size) { - // Find sibling position - let height = pmmr::bintree_postorder_height(pos0); - let subtree_size = (1u64 << (height + 1)) - 1; - let sibling_pos0 = if pmmr::is_left_sibling(pos0) { - // Right sibling is at pos0 + size of this subtree - Some(pos0 + subtree_size) - } else { - // Left sibling: go back by sibling's subtree size - pos0.checked_sub(subtree_size) - }; - // Need hash if sibling exists in segment and is not fully pruned - let need_hash = match sibling_pos0 { - Some(sib) if sib >= segment_first_pos && sib <= segment_last_pos => { - !subtree_fully_pruned(sib, bm, mmr_size) - } - _ => { - // Sibling outside segment or underflow - may need hash for proof - true - } - }; - if need_hash { - if let Some(hash) = pmmr.get_from_file(pos0) { - segment.hashes.push(hash); - segment.hash_pos.push(pos0); + } + + // Pass 2: bottom-up walk mirroring `root`, deciding exactly which + // interior hashes are needed to reconstruct the root from the leaf + // set chosen above. + if bitmap.is_some() { + let leaf_set: std::collections::HashSet = + segment.leaf_pos.iter().copied().collect(); + let mut needed = Vec::<(u64, Hash)>::new(); + // (pos0, known) - `known` means the receiver can derive this + // position's hash from what the segment ships below it. + let mut stack = + Vec::<(u64, bool)>::with_capacity(2 * (segment.identifier.height as usize) + 1); + for pos0 in segment_first_pos..=segment_last_pos { + let height = pmmr::bintree_postorder_height(pos0); + let is_known = if height == 0 { + leaf_set.contains(&pos0) + } else { + let (_, right_known) = stack.pop().unwrap(); + let (_, left_known) = stack.pop().unwrap(); + match (left_known, right_known) { + (true, true) => true, + (false, false) => false, + (known_l, _known_r) => { + let left_child_pos = 1 + pos0 - (1 << height); + let right_child_pos = pos0; + let need_pos = if known_l { + right_child_pos - 1 + } else { + left_child_pos - 1 + }; + if let Some(hash) = pmmr.get_from_file(need_pos) { + needed.push((need_pos, hash)); + true + } else { + // The needed child hash is unavailable (its + // subtree is compacted beyond this level): + // leave the parent unknown so the requirement + // propagates upward, or ultimately resolves + // via the whole-segment fallback below. + false + } } } + }; + stack.push((pos0, is_known)); + } + // Any subtree root still unknown has no parent inside the + // segment to demand it: the segment root of a fully pruned full + // segment, or a fully pruned peak in the final (partial) + // segment. The receiver looks these up directly (`root` bags + // pruned in-segment peaks; `first_unpruned_parent` probes the + // segment root before climbing), so ship them if we have them. + for (pos0, known) in stack { + if !known { + if let Some(hash) = pmmr.get_from_file(pos0) { + needed.push((pos0, hash)); + } } } + // Serialization requires strictly increasing hash positions and + // the walk can demand a left child after a deeper right-subtree + // sibling, so sort before finalizing. + needed.sort_unstable_by_key(|&(pos0, _)| pos0); + for (pos0, hash) in needed { + segment.hash_pos.push(pos0); + segment.hashes.push(hash); + } } let mut start_pos = None; @@ -871,14 +936,3 @@ impl Writeable for SegmentProof { Ok(()) } } - -/// Check if a subtree rooted at pos0 is fully pruned (no unspent leaves in bitmap) -fn subtree_fully_pruned(pos0: u64, bitmap: &Bitmap, mmr_size: u64) -> bool { - let leftmost = pmmr::bintree_leftmost(pos0); - let rightmost = pmmr::bintree_rightmost(pos0); - let n_leaves = pmmr::n_leaves(mmr_size); - let start_leaf = pmmr::n_leaves(leftmost + 1).saturating_sub(1); - let end_leaf = min(pmmr::n_leaves(rightmost + 1), n_leaves); - // If any leaf in range is in bitmap (unspent), subtree is not fully pruned - bitmap.range_cardinality(start_leaf as u32..end_leaf as u32) == 0 -} diff --git a/store/tests/segment.rs b/store/tests/segment.rs index 49ec24f3d6..d5959883ab 100644 --- a/store/tests/segment.rs +++ b/store/tests/segment.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::core::core::hash::DefaultHashable; +use crate::core::core::hash::{DefaultHashable, Hash}; use crate::core::core::pmmr; use crate::core::core::pmmr::segment::{Segment, SegmentIdentifier}; use crate::core::core::pmmr::{Backend, ReadablePMMR, ReadonlyPMMR, PMMR}; @@ -389,6 +389,185 @@ where } } +// Replicates chain::txhashset::TxHashSet::apply_output_segment / +// sort_pmmr_hashes_and_leaves: insert hashes and leaves in position order, +// pushing hashes as pruned subtrees when pos0 >= size and leaves when +// pos0 == size. +fn apply_segment_like_desegmenter( + segment: Segment, + ba: &mut PMMRBackend, + unspent_bitmap: &Bitmap, +) -> Result { + let (_id, hash_pos, hashes, leaf_pos, leaf_data, _proof) = segment.parts(); + // (pos0, is_hash, data_index) + let mut inserts: Vec<(u64, bool, usize)> = Vec::new(); + for (idx, &pos0) in leaf_pos.iter().enumerate() { + inserts.push((pos0, false, idx)); + } + for (idx, &pos0) in hash_pos.iter().enumerate() { + inserts.push((pos0, true, idx)); + } + inserts.sort(); + + let mut mmr = PMMR::new(ba); + for (pos0, is_hash, idx) in inserts { + if is_hash { + if pos0 >= mmr.size { + mmr.push_pruned_subtree(hashes[idx], pos0)?; + } + } else { + if pos0 == mmr.size { + mmr.push(&leaf_data[idx])?; + } + if let Some(i) = pmmr::pmmr_leaf_to_insertion_index(pos0) { + if !unspent_bitmap.contains(i as u32) { + mmr.remove_from_leaf_set(pos0); + } + } + } + } + mmr.root() +} + +// Build a source PMMR in the mixed state every synced node is in between +// compaction passes: leaves 2,3 spent AND compacted away (prune list root at +// pos 5), leaves 0,1 spent but NOT yet compacted (still physically present in +// the files). +fn build_mixed_compaction_source(ba: &mut PMMRBackend) -> (u64, Hash, Bitmap) { + let n_leaves: u64 = 16; + let mut mmr = PMMR::new(ba); + for i in 0..n_leaves as u32 { + mmr.push(&TestElem([i / 7, i / 5, i / 3, i])).unwrap(); + } + let last_pos = mmr.unpruned_size(); + let root = mmr.root().unwrap(); + + let mut bitmap = Bitmap::new(); + bitmap.add_range(0..n_leaves as u32); + + let mut mmr = PMMR::at(ba, last_pos); + prune(&mut mmr, &mut bitmap, &[2, 3]); + ba.sync().unwrap(); + ba.check_compact(last_pos, &Bitmap::new()).unwrap(); + ba.sync().unwrap(); + + let mut mmr = PMMR::at(ba, last_pos); + prune(&mut mmr, &mut bitmap, &[0, 1]); + ba.sync().unwrap(); + + (last_pos, root, bitmap) +} + +// A segment produced from a partially-compacted region must apply cleanly on +// a fresh node and reproduce the source root. With the previous bitmap-only +// boundary heuristic this exact fixture produced a segment that passed +// validation but silently corrupted the receiving PMMR (the "Invalid Root" +// PIBD restart loop reported against the sparse segment optimization). +#[test] +fn sparse_segment_mixed_compaction_round_trip() { + let t = Utc::now(); + let data_dir = format!( + "./target/tmp/{}.{}-sparse_round_trip", + t.timestamp(), + t.timestamp_subsec_nanos() + ); + let src_dir = format!("{}/src", data_dir); + let dst_dir = format!("{}/dst", data_dir); + for d in [&src_dir, &dst_dir] { + fs::create_dir_all(d).unwrap(); + } + + let mut ba = PMMRBackend::new(&src_dir, true, ProtocolVersion(1), None).unwrap(); + let (last_pos, root, bitmap) = build_mixed_compaction_source(&mut ba); + + // Whole MMR in one segment + let id = SegmentIdentifier { height: 4, idx: 0 }; + let mmr = ReadonlyPMMR::at(&mut ba, last_pos); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); + + // The fully-spent-but-partially-compacted subtree over leaves 0..4 must + // be represented by its single boundary hash at pos 6, with no leaf data + // beneath it: describing the same positions twice (spent leaves as data + // AND collapsed into an ancestor hash) is what broke apply. + let leaf_positions: Vec = segment.leaf_iter().map(|(p, _)| p).collect(); + let hash_positions: Vec = segment.hash_iter().map(|(p, _)| p).collect(); + assert_eq!(hash_positions, vec![6]); + assert!(leaf_positions.iter().all(|&p| p > 6)); + + segment.validate(last_pos, Some(&bitmap), root).unwrap(); + + let mut ba_dst = PMMRBackend::new(&dst_dir, true, ProtocolVersion(1), None).unwrap(); + let applied_root = apply_segment_like_desegmenter(segment, &mut ba_dst, &bitmap).unwrap(); + assert_eq!(applied_root, root, "sparse segment must round-trip"); + + std::mem::drop(ba); + std::mem::drop(ba_dst); + fs::remove_dir_all(&data_dir).unwrap(); +} + +// Documents why boundary hashes cannot be chosen from the bitmap alone: the +// old heuristic shipped the spent-but-uncompacted leaves 0,1 as data AND +// collapsed leaves 0..4 into the pos 6 boundary hash. Such a segment passes +// validation (root reconstruction ignores the redundant leaves, the hash is +// genuinely bound to the root), but the desegmenter apply logic pushes +// leaves 0,1 at the frontier and then registers the pos 6 subtree as pruned +// over them, desyncing the prune list from the hash file. The resulting +// corruption only surfaced later as an unattributable "Invalid Root" during +// full state validation. +#[test] +fn redundant_leaf_and_boundary_hash_segment_corrupts_apply() { + let t = Utc::now(); + let data_dir = format!( + "./target/tmp/{}.{}-redundant_corrupts", + t.timestamp(), + t.timestamp_subsec_nanos() + ); + let src_dir = format!("{}/src", data_dir); + let dst_dir = format!("{}/dst", data_dir); + for d in [&src_dir, &dst_dir] { + fs::create_dir_all(d).unwrap(); + } + + let mut ba = PMMRBackend::new(&src_dir, true, ProtocolVersion(1), None).unwrap(); + let (last_pos, root, bitmap) = build_mixed_compaction_source(&mut ba); + + let id = SegmentIdentifier { height: 4, idx: 0 }; + let mmr = ReadonlyPMMR::at(&mut ba, last_pos); + let segment = Segment::from_pmmr(id, &mmr, Some(&bitmap)).unwrap(); + + // Reconstruct what the bitmap-only heuristic used to produce: the same + // segment plus data for the spent-but-still-present leaves 0,1 (pos 0,1) + // under the pos 6 boundary hash. + let (sid, hash_pos, hashes, mut leaf_pos, mut leaf_data, proof) = segment.parts(); + leaf_pos.splice(0..0, [0, 1]); + leaf_data.splice( + 0..0, + [ + mmr.get_data_from_file(0).unwrap(), + mmr.get_data_from_file(1).unwrap(), + ], + ); + let redundant = Segment::from_parts(sid, hash_pos, hashes, leaf_pos, leaf_data, proof); + + // Validation accepts it: the extra leaves are ignored by root + // reconstruction and the boundary hash is bound to the root. + redundant.validate(last_pos, Some(&bitmap), root).unwrap(); + + // ... but applying it does not reproduce the root the segment was just + // validated against. + let mut ba_dst = PMMRBackend::new(&dst_dir, true, ProtocolVersion(1), None).unwrap(); + let applied_root = apply_segment_like_desegmenter(redundant, &mut ba_dst, &bitmap); + assert_ne!( + applied_root.as_ref().ok(), + Some(&root), + "redundant segment must not silently reproduce the source root" + ); + + std::mem::drop(ba); + std::mem::drop(ba_dst); + fs::remove_dir_all(&data_dir).unwrap(); +} + #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct TestElem(pub [u32; 4]); From 7114b146187860d8bb09ecead55dacd1a46d2550 Mon Sep 17 00:00:00 2001 From: iho Date: Sat, 18 Jul 2026 08:24:44 +0300 Subject: [PATCH 4/4] Fix desegmenter edge cases stalling PIBD on small chains calc_bitmap_mmr_sizes: unwrap_or evaluates its fallback eagerly, so any output MMR with <= 1024 leaves (a single bitmap chunk) computed peaks(insertion_to_pmmr_index(0)) and panicked unwrapping the empty peak set. Evaluate lazily and degrade to 0 when even the reduced set is empty. next_desired_segments: the bitmap branch compared a segment's last position against the local PMMR size with a strict greater-than, so the first segment of a single-leaf bitmap MMR (last position 0, local size 0) was never requested and sync stalled silently. Positions 0..size-1 are present locally, so a segment whose last position equals the size still contains new data. Unreachable on mainnet (far more than 1024 outputs) but panics or stalls any small test chain, including the canned chains used by test_pibd_copy. --- chain/src/txhashset/desegmenter.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/chain/src/txhashset/desegmenter.rs b/chain/src/txhashset/desegmenter.rs index aaae3a3543..422c3e6997 100644 --- a/chain/src/txhashset/desegmenter.rs +++ b/chain/src/txhashset/desegmenter.rs @@ -486,9 +486,13 @@ impl Desegmenter { self.bitmap_mmr_size, self.default_bitmap_segment_height, ); - // Advance iterator to next expected segment + // Advance iterator to next expected segment. `local_pmmr_size` is + // a count (positions 0..size-1 are present locally), so a segment + // whose last position equals it still contains new data and must + // be requested: with a strict `>` a single-leaf bitmap MMR (last + // position 0, local size 0) is never requested and sync stalls. while let Some(id) = identifier_iter.next() { - if id.segment_pos_range(self.bitmap_mmr_size).1 > local_pmmr_size { + if id.segment_pos_range(self.bitmap_mmr_size).1 >= local_pmmr_size { if !self.has_bitmap_segment_with_id(id) { return_vec.push(SegmentTypeIdentifier::new(SegmentType::Bitmap, id)); if return_vec.len() >= max_elements { @@ -672,18 +676,23 @@ impl Desegmenter { "pibd_desegmenter - expected number of leaves in bitmap MMR: {}", self.bitmap_mmr_leaf_count ); - // Total size of Bitmap PMMR + // Total size of Bitmap PMMR. `unwrap_or` evaluates its argument + // eagerly, so the fallback used to run - and panic on an empty peak + // set - whenever the bitmap MMR had a single leaf (output MMR with + // <= 1024 leaves). Evaluate it lazily and degrade to 0 instead of + // panicking when even the reduced peak set is empty. self.bitmap_mmr_size = 1 + pmmr::peaks(pmmr::insertion_to_pmmr_index(self.bitmap_mmr_leaf_count)) .last() - .unwrap_or( - &(pmmr::peaks(pmmr::insertion_to_pmmr_index( - self.bitmap_mmr_leaf_count - 1, + .copied() + .unwrap_or_else(|| { + pmmr::peaks(pmmr::insertion_to_pmmr_index( + self.bitmap_mmr_leaf_count.saturating_sub(1), )) .last() - .unwrap()), - ) - .clone(); + .copied() + .unwrap_or(0) + }); trace!( "pibd_desegmenter - expected size of bitmap MMR: {}",