Skip to content

Commit c975533

Browse files
address review
1 parent 86398bb commit c975533

13 files changed

Lines changed: 114 additions & 93 deletions

File tree

storage/src/bitmap/authenticated.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -459,10 +459,10 @@ impl<E: Context, D: Digest, const N: usize, S: Strategy> MerkleizedBitMap<E, D,
459459
/// Return an inclusion proof for the specified bit, along with the chunk of the bitmap
460460
/// containing that bit. The proof can be used to prove any bit in the chunk.
461461
///
462-
/// The bitmap proof stores the number of bits in the bitmap within the `size` field of the
463-
/// proof instead of MMR size since the underlying MMR's size does not reflect the number of
464-
/// bits in any partial chunk. The underlying MMR size can be derived from the number of
465-
/// bits as `leaf_num_to_pos(proof.size / BitMap<_, N>::CHUNK_SIZE_BITS)`.
462+
/// The bitmap proof stores the number of bits in the bitmap within the proof's `leaves` field
463+
/// instead of the underlying MMR leaf count, since the MMR does not reflect the number of bits
464+
/// in any partial chunk. The underlying MMR size can be derived from the number of bits as
465+
/// `leaf_num_to_pos(proof.leaves / BitMap<_, N>::CHUNK_SIZE_BITS)`.
466466
///
467467
/// # Errors
468468
///

storage/src/journal/authenticated.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,21 @@ impl<F: Family, H: Hasher, Item: Encode + Send + Sync, S: Strategy>
9595

9696
/// Merkleize the batch.
9797
/// `base` provides committed node data as fallback during hash computation.
98-
pub fn merkleize(mut self, base: &Mem<F, H::Digest>) -> MerkleizedBatchArc<F, H, Item, S> {
99-
let items = Arc::new(core::mem::take(&mut self.items));
100-
let merkle = self.inner.merkleize(base, &self.hasher);
101-
let ancestor_items = Self::collect_ancestor_items(&self.parent);
98+
pub fn merkleize(self, base: &Mem<F, H::Digest>) -> MerkleizedBatchArc<F, H, Item, S> {
99+
let Self {
100+
inner,
101+
hasher,
102+
items,
103+
parent,
104+
} = self;
105+
106+
let items = Arc::new(items);
107+
let merkle = inner.merkleize(base, &hasher);
108+
let ancestor_items = Self::collect_ancestor_items(&parent);
102109
Arc::new(MerkleizedBatch {
103110
inner: merkle,
104111
items,
105-
parent: self.parent.as_ref().map(Arc::downgrade),
112+
parent: parent.as_ref().map(Arc::downgrade),
106113
ancestor_items,
107114
})
108115
}
@@ -593,7 +600,7 @@ where
593600
}
594601

595602
/// Generate a historical proof with respect to the state of the Merkle structure when it had
596-
/// `historical_leaves` leaves, using `spec` to determine peak bagging.
603+
/// `historical_leaves` leaves.
597604
///
598605
/// Returns a proof and the items corresponding to the leaves in the range `start_loc..end_loc`,
599606
/// where `end_loc` is the minimum of `historical_leaves` and `start_loc + max_ops`.

storage/src/merkle/mem.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,9 @@ impl<F: Family, D: Digest> Mem<F, D> {
264264
self.pruning_boundary = pos;
265265
}
266266

267-
/// Return an inclusion proof for the element at location `loc` using an explicit root spec.
267+
/// Return an inclusion proof for the element at location `loc`.
268+
///
269+
/// The proof commits to `inactive_peaks`; peak bagging is selected by `hasher`.
268270
///
269271
/// # Errors
270272
///
@@ -287,8 +289,9 @@ impl<F: Family, D: Digest> Mem<F, D> {
287289
})
288290
}
289291

290-
/// Return an inclusion proof for all elements within the provided `range` of locations
291-
/// using an explicit root spec.
292+
/// Return an inclusion proof for all elements within the provided `range` of locations.
293+
///
294+
/// The proof commits to `inactive_peaks`; peak bagging is selected by `hasher`.
292295
///
293296
/// # Errors
294297
///
@@ -1149,7 +1152,7 @@ mod tests {
11491152
);
11501153
}
11511154

1152-
fn split_root_spec_matches_recompute<F: Family>() {
1155+
fn split_root_matches_recompute<F: Family>() {
11531156
let hasher: H = Standard::new();
11541157
let plain = build::<F>(&hasher, 49);
11551158
let inactive_peaks = 1;
@@ -1263,8 +1266,8 @@ mod tests {
12631266
apply_batch_overwrite_only_ancestor::<crate::mmr::Family>();
12641267
}
12651268
#[test]
1266-
fn mmr_split_root_spec_matches_recompute() {
1267-
split_root_spec_matches_recompute::<crate::mmr::Family>();
1269+
fn mmr_split_root_matches_recompute() {
1270+
split_root_matches_recompute::<crate::mmr::Family>();
12681271
}
12691272

12701273
// --- MMB tests ---
@@ -1362,7 +1365,7 @@ mod tests {
13621365
apply_batch_overwrite_only_ancestor::<crate::mmb::Family>();
13631366
}
13641367
#[test]
1365-
fn mmb_split_root_spec_matches_recompute() {
1366-
split_root_spec_matches_recompute::<crate::mmb::Family>();
1368+
fn mmb_split_root_matches_recompute() {
1369+
split_root_matches_recompute::<crate::mmb::Family>();
13671370
}
13681371
}

storage/src/merkle/persisted/full.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,8 @@ impl<F: Family, E: RStorage + Clock + Metrics, D: Digest, S: Strategy> Merkle<F,
243243

244244
/// Read-only peek at the persisted structure's root and boundaries.
245245
///
246-
/// The root spec must match the spec used by the caller for the persisted structure.
246+
/// `inactive_peaks` and `hasher.root_bagging()` must match the root shape expected by the
247+
/// caller for the persisted structure.
247248
///
248249
/// Returns `Ok(None)` when:
249250
/// - Journal size is structurally invalid and would require a rewind (i.e.
@@ -994,7 +995,9 @@ impl<F: Family, E: RStorage + Clock + Metrics + Sync, D: Digest, S: Strategy>
994995

995996
impl<F: Family, E: RStorage + Clock + Metrics, D: Digest, S: Strategy> Merkle<F, E, D, S> {
996997
/// Return an inclusion proof for the element at the location `loc` against a historical
997-
/// state with `leaves` leaves, using `spec` to determine peak bagging.
998+
/// state with `leaves` leaves.
999+
///
1000+
/// The proof commits to `inactive_peaks`; peak bagging is selected by `hasher`.
9981001
///
9991002
/// # Errors
10001003
///
@@ -1019,7 +1022,9 @@ impl<F: Family, E: RStorage + Clock + Metrics, D: Digest, S: Strategy> Merkle<F,
10191022
}
10201023

10211024
/// Return an inclusion proof for the elements in `range` against a historical state with
1022-
/// `leaves` leaves, using `spec` to determine peak bagging.
1025+
/// `leaves` leaves.
1026+
///
1027+
/// The proof commits to `inactive_peaks`; peak bagging is selected by `hasher`.
10231028
///
10241029
/// # Errors
10251030
///
@@ -1050,7 +1055,9 @@ impl<F: Family, E: RStorage + Clock + Metrics, D: Digest, S: Strategy> Merkle<F,
10501055
}
10511056

10521057
/// Return an inclusion proof for the element at the location `loc` that can be verified against
1053-
/// the current root, using `spec` to determine peak bagging.
1058+
/// the current root.
1059+
///
1060+
/// The proof commits to `inactive_peaks`; peak bagging is selected by `hasher`.
10541061
///
10551062
/// Unlike the in-memory `Mem::proof`, this async method can read from the backing journal for
10561063
/// nodes that have been synced out of memory.
@@ -1074,8 +1081,9 @@ impl<F: Family, E: RStorage + Clock + Metrics, D: Digest, S: Strategy> Merkle<F,
10741081
self.range_proof(hasher, loc..loc + 1, inactive_peaks).await
10751082
}
10761083

1077-
/// Return an inclusion proof for the elements within the specified location range, using
1078-
/// `spec` to determine peak bagging.
1084+
/// Return an inclusion proof for the elements within the specified location range.
1085+
///
1086+
/// The proof commits to `inactive_peaks`; peak bagging is selected by `hasher`.
10791087
///
10801088
/// Unlike the in-memory `Mem::range_proof`, this async method can read from the backing
10811089
/// journal for nodes that have been synced out of memory.

storage/src/merkle/proof.rs

Lines changed: 42 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,12 @@ pub enum ReconstructionError {
5454
/// in depth-first (forward consumption) order for each range peak.
5555
///
5656
/// Multi-proofs use a different layout: `digests` contains the sorted set of node digests required
57-
/// by the selected root spec, including individual prefix peak digests rather than a folded prefix
58-
/// accumulator. This layout is intentionally position-keyed: every multi-proof digest corresponds
59-
/// to a concrete Merkle node position. For `BackwardFold`, this may include active suffix peaks that
60-
/// a single range proof could collapse into a smaller synthetic suffix accumulator. We keep the
61-
/// extra active peaks so multi-proofs and proof stores can stay simple and derive witnesses from
62-
/// positions alone.
57+
/// by the requested `inactive_peaks` and bagging policy, including individual prefix peak digests
58+
/// rather than a folded prefix accumulator. This layout is intentionally position-keyed: every
59+
/// multi-proof digest corresponds to a concrete Merkle node position. For `BackwardFold`, this may
60+
/// include active suffix peaks that a single range proof could collapse into a smaller synthetic
61+
/// suffix accumulator. We keep the extra active peaks so multi-proofs and proof stores can stay
62+
/// simple and derive witnesses from positions alone.
6363
#[derive(Clone, Debug, Eq)]
6464
pub struct Proof<F: Family, D: Digest> {
6565
/// The total number of leaves in the data structure. For MMR proofs, this is the number of
@@ -175,7 +175,11 @@ impl<F: Family, D: Digest> Proof<F, D> {
175175

176176
/// Returns true if this proof's `inactive_peaks` field matches the canonical value derived
177177
/// from `size` and `inactivity_floor`.
178-
pub fn matches_canonical_spec(&self, size: Position<F>, inactivity_floor: Location<F>) -> bool {
178+
pub fn matches_canonical_inactive_peaks(
179+
&self,
180+
size: Position<F>,
181+
inactivity_floor: Location<F>,
182+
) -> bool {
179183
self.inactive_peaks == F::inactive_peaks(size, inactivity_floor)
180184
}
181185

@@ -343,7 +347,10 @@ impl<F: Family, D: Digest> Proof<F, D> {
343347
Ok(collected_digests)
344348
}
345349

346-
/// Verify this proof and the pinned nodes against `root` using `spec`.
350+
/// Verify this proof and the pinned nodes against `root`.
351+
///
352+
/// The proof's `inactive_peaks` field commits to the split boundary; peak bagging is selected
353+
/// by `hasher`.
347354
///
348355
/// The `pinned_nodes` are the peak digests of the sub-structure at `start_loc`, in the order
349356
/// returned by `Family::nodes_to_pin`. The proof authenticates the prefix `[0, start_loc)` via:
@@ -1171,24 +1178,24 @@ mod tests {
11711178
Standard::with_bagging(bagging)
11721179
}
11731180

1174-
fn push_unique_spec(specs: &mut Vec<(Bagging, usize)>, spec: (Bagging, usize)) {
1175-
if !specs.contains(&spec) {
1176-
specs.push(spec);
1181+
fn push_unique_shape(shapes: &mut Vec<(Bagging, usize)>, shape: (Bagging, usize)) {
1182+
if !shapes.contains(&shape) {
1183+
shapes.push(shape);
11771184
}
11781185
}
11791186

1180-
fn supported_root_specs<F: Family>(leaves: Location<F>) -> Vec<(Bagging, usize)> {
1187+
fn supported_root_shapes<F: Family>(leaves: Location<F>) -> Vec<(Bagging, usize)> {
11811188
let peak_count = F::peaks(F::location_to_position(leaves)).count();
1182-
let mut specs = Vec::new();
1189+
let mut shapes = Vec::new();
11831190

1184-
push_unique_spec(&mut specs, (Bagging::ForwardFold, 0));
1185-
push_unique_spec(&mut specs, (Bagging::BackwardFold, 0));
1191+
push_unique_shape(&mut shapes, (Bagging::ForwardFold, 0));
1192+
push_unique_shape(&mut shapes, (Bagging::BackwardFold, 0));
11861193
for inactive_peaks in 0..=peak_count {
1187-
push_unique_spec(&mut specs, (Bagging::ForwardFold, inactive_peaks));
1188-
push_unique_spec(&mut specs, (Bagging::BackwardFold, inactive_peaks));
1194+
push_unique_shape(&mut shapes, (Bagging::ForwardFold, inactive_peaks));
1195+
push_unique_shape(&mut shapes, (Bagging::BackwardFold, inactive_peaks));
11891196
}
11901197

1191-
specs
1198+
shapes
11921199
}
11931200

11941201
fn inactive_leaf_floor<F: Family>(leaves: Location<F>, inactive_peaks: usize) -> u64 {
@@ -1198,7 +1205,7 @@ mod tests {
11981205
.sum()
11991206
}
12001207

1201-
fn active_start_for_spec<F: Family>(
1208+
fn active_start_for_shape<F: Family>(
12021209
leaves: Location<F>,
12031210
inactive_peaks: usize,
12041211
width: u64,
@@ -1210,13 +1217,13 @@ mod tests {
12101217
Location::new(*leaves - width)
12111218
}
12121219

1213-
fn range_proofs_verify_for_supported_root_specs<F: Family>() {
1220+
fn range_proofs_verify_for_supported_root_shapes<F: Family>() {
12141221
let mem = build_raw::<F>(&H::new(), 123);
12151222
let leaves = mem.leaves();
12161223

1217-
for (bagging, inactive_peaks) in supported_root_specs::<F>(leaves) {
1224+
for (bagging, inactive_peaks) in supported_root_shapes::<F>(leaves) {
12181225
let hasher = hasher_for_bagging(bagging);
1219-
let range_start = active_start_for_spec::<F>(leaves, inactive_peaks, 3);
1226+
let range_start = active_start_for_shape::<F>(leaves, inactive_peaks, 3);
12201227
let range = range_start..range_start + 3;
12211228
let root = mem.root(&hasher, inactive_peaks).unwrap();
12221229
let elements: Vec<_> = (*range.start..*range.end)
@@ -1257,13 +1264,13 @@ mod tests {
12571264
}
12581265
}
12591266

1260-
fn multi_proofs_verify_for_supported_root_specs<F: Family>() {
1267+
fn multi_proofs_verify_for_supported_root_shapes<F: Family>() {
12611268
let mem = build_raw::<F>(&H::new(), 123);
12621269
let leaves = mem.leaves();
12631270

1264-
for (bagging, inactive_peaks) in supported_root_specs::<F>(leaves) {
1271+
for (bagging, inactive_peaks) in supported_root_shapes::<F>(leaves) {
12651272
let hasher = hasher_for_bagging(bagging);
1266-
let first = active_start_for_spec::<F>(leaves, inactive_peaks, 12);
1273+
let first = active_start_for_shape::<F>(leaves, inactive_peaks, 12);
12671274
let locations = [first, first + 5, first + 11];
12681275
let nodes = nodes_required_for_multi_proof(leaves, inactive_peaks, bagging, &locations)
12691276
.expect("test locations valid");
@@ -1358,7 +1365,7 @@ mod tests {
13581365
}
13591366

13601367
#[test]
1361-
fn full_backward_root_spec_proves_like_split_zero() {
1368+
fn full_backward_root_proves_like_split_zero() {
13621369
let hasher: H = Standard::backward();
13631370
let mem = build_raw::<mmb::Family>(&hasher, 123);
13641371
let range = Location::new(2)..Location::new(3);
@@ -1403,7 +1410,7 @@ mod tests {
14031410
&full_backward_root,
14041411
));
14051412

1406-
// Split { inactive_peaks: 0 } is byte-identical to Full when inactive_peaks == 0.
1413+
// A zero inactive boundary is byte-identical to the corresponding full root.
14071414
let split_root_value = mem.root(&hasher, 0).unwrap();
14081415
assert_eq!(full_backward_root, split_root_value);
14091416
let split_proof: Result<Proof<mmb::Family, D>, Error<mmb::Family>> = build_range_proof(
@@ -2549,12 +2556,12 @@ mod tests {
25492556
range_proof_reconstruction::<mmr::Family>();
25502557
}
25512558
#[test]
2552-
fn mmr_range_proofs_verify_for_supported_root_specs() {
2553-
range_proofs_verify_for_supported_root_specs::<mmr::Family>();
2559+
fn mmr_range_proofs_verify_for_supported_root_shapes() {
2560+
range_proofs_verify_for_supported_root_shapes::<mmr::Family>();
25542561
}
25552562
#[test]
2556-
fn mmr_multi_proofs_verify_for_supported_root_specs() {
2557-
multi_proofs_verify_for_supported_root_specs::<mmr::Family>();
2563+
fn mmr_multi_proofs_verify_for_supported_root_shapes() {
2564+
multi_proofs_verify_for_supported_root_shapes::<mmr::Family>();
25582565
}
25592566
#[test]
25602567
fn mmr_verify_element_inclusion() {
@@ -2642,12 +2649,12 @@ mod tests {
26422649
range_proof_reconstruction::<mmb::Family>();
26432650
}
26442651
#[test]
2645-
fn mmb_range_proofs_verify_for_supported_root_specs() {
2646-
range_proofs_verify_for_supported_root_specs::<mmb::Family>();
2652+
fn mmb_range_proofs_verify_for_supported_root_shapes() {
2653+
range_proofs_verify_for_supported_root_shapes::<mmb::Family>();
26472654
}
26482655
#[test]
2649-
fn mmb_multi_proofs_verify_for_supported_root_specs() {
2650-
multi_proofs_verify_for_supported_root_specs::<mmb::Family>();
2656+
fn mmb_multi_proofs_verify_for_supported_root_shapes() {
2657+
multi_proofs_verify_for_supported_root_shapes::<mmb::Family>();
26512658
}
26522659
#[test]
26532660
fn mmb_verify_element_inclusion() {

storage/src/merkle/read.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use core::ops::Range;
1111
/// construction is intentionally *not* part of the trait: every concrete implementation exposes
1212
/// inherent `proof` / `range_proof` methods that take an explicit `inactive_peaks` count and read
1313
/// the bagging policy from the supplied [`crate::merkle::hasher::Hasher`], so callers cannot
14-
/// accidentally pair a split-spec root with a forward-fold proof from the same state.
14+
/// accidentally pair a split root with a forward-fold proof from the same state.
1515
pub trait Readable: Send + Sync {
1616
/// The Merkle family implemented by this structure.
1717
type Family: Family;

storage/src/merkle/verification.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,9 @@ impl<F: Family, D: Digest> ProofStore<F, D> {
273273
}
274274
}
275275

276-
/// Return a range proof for the nodes corresponding to the given location range, using `spec`
277-
/// to determine peak bagging.
276+
/// Return a range proof for the nodes corresponding to the given location range.
277+
///
278+
/// The proof commits to `inactive_peaks`; peak bagging is selected by `hasher`.
278279
///
279280
/// # Errors
280281
///
@@ -345,8 +346,10 @@ pub async fn historical_range_proof<
345346
)
346347
}
347348

348-
/// Return an inclusion proof for the elements at the specified locations using `spec`. This is
349-
/// analogous to range_proof but supports non-contiguous locations.
349+
/// Return an inclusion proof for the elements at the specified locations. This is analogous to
350+
/// range_proof but supports non-contiguous locations.
351+
///
352+
/// The proof commits to `inactive_peaks`; peak bagging is supplied by `bagging`.
350353
///
351354
/// The order of positions does not affect the output (sorted internally).
352355
///

storage/src/qmdb/any/db.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,8 @@ where
340340
///
341341
/// When this database is configured with `split_root: true`, `historical_size` must be a
342342
/// commit-boundary size: the operation at `historical_size - 1` must itself be a commit
343-
/// op declaring the governing inactivity floor. With `split_root: false` the spec does
344-
/// not depend on the floor and any retained `historical_size` works.
343+
/// op declaring the governing inactivity floor. With `split_root: false`, historical proof
344+
/// roots do not depend on the floor and any retained `historical_size` works.
345345
///
346346
/// # Errors
347347
///

storage/src/qmdb/any/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -847,7 +847,7 @@ pub(crate) mod test {
847847

848848
// Apply two single-write batches and capture the commit-boundary size after the
849849
// first batch. `historical_proof` requires the historical size to land on a commit
850-
// boundary when the db uses a split-root spec.
850+
// boundary when the db commits to an inactive peak boundary.
851851
let mut historical_op_count = Location::new(0);
852852
for i in 0u64..2 {
853853
let k = Sha256::hash(&i.to_be_bytes());
@@ -987,7 +987,7 @@ pub(crate) mod test {
987987

988988
// Apply a sequence of single-write batches and record the commit-boundary size
989989
// reached after each. `historical_proof` requires the historical size to be a
990-
// commit boundary when the db uses a split-root spec, so we anchor each test on
990+
// commit boundary when the db commits to an inactive peak boundary, so we anchor each test on
991991
// one of the boundaries we recorded here rather than hardcoding sizes that depend
992992
// on internal floor-raising behavior.
993993
let initial_size = db.bounds().await.end;

0 commit comments

Comments
 (0)