From 58c521240e6628d4f9231b1874c2f26f6ba90475 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 10 Mar 2026 11:32:48 +0700 Subject: [PATCH 1/9] feat: use transactional storage context for all specialized trees Specialized trees (DenseFixedSizedMerkleTree, MMR, BulkAppendTree, CommitmentTree) previously used immediate storage context which bypassed the StorageBatch pipeline. This meant commit_batch cost tracking had no effect on these code paths. This change adds in-memory caches that provide read-after-write visibility, enabling transactional storage context where writes are deferred to a StorageBatch: - DenseFixedSizedMerkleTree: write-through cache serves reads from memory, falling back to storage for pre-session values - MMRBatch: overlay persistence across compaction cycles so cross-epoch MMR reads find nodes pushed in previous compactions without storage - BulkAppendTree/CommitmentTree: MMR overlay held in-memory during session, flushed via commit_mmr() at session end All GroveDB operations (dense_tree, mmr_tree, bulk_append_tree, commitment_tree) now use get_transactional_storage_context + commit_multi_context_batch, matching the pattern used by Merk trees. Co-Authored-By: Claude Opus 4.6 --- grovedb-bulk-append-tree/src/proof/mod.rs | 5 +- grovedb-bulk-append-tree/src/tree/append.rs | 39 +++++++++++--- grovedb-bulk-append-tree/src/tree/fetch.rs | 11 ++-- grovedb-bulk-append-tree/src/tree/mod.rs | 6 +++ .../src/commitment_tree/mod.rs | 10 ++++ .../src/tests.rs | 10 +++- .../src/tree.rs | 53 ++++++++++++++++--- grovedb-merkle-mountain-range/src/mmr.rs | 12 +++++ .../src/mmr_store.rs | 20 +++++++ grovedb/src/operations/bulk_append_tree.rs | 34 ++++++++++-- grovedb/src/operations/commitment_tree.rs | 34 ++++++++++-- grovedb/src/operations/dense_tree.rs | 36 ++++++++++--- grovedb/src/operations/mmr_tree.rs | 30 +++++++++-- 13 files changed, 262 insertions(+), 38 deletions(-) diff --git a/grovedb-bulk-append-tree/src/proof/mod.rs b/grovedb-bulk-append-tree/src/proof/mod.rs index 1e7339d8a..7ee0a0d92 100644 --- a/grovedb-bulk-append-tree/src/proof/mod.rs +++ b/grovedb-bulk-append-tree/src/proof/mod.rs @@ -14,7 +14,7 @@ use bincode::{Decode, Encode}; use grovedb_dense_fixed_sized_merkle_tree::DenseTreeProof; use grovedb_merkle_mountain_range::MmrTreeProof; #[cfg(feature = "storage")] -use grovedb_merkle_mountain_range::{MMRStoreReadOps, MmrKeySize, MmrNode, MmrStore}; +use grovedb_merkle_mountain_range::{MmrKeySize, MmrNode, MmrStore, MMR}; use grovedb_query::{Query, QueryItem}; #[cfg(feature = "storage")] use grovedb_storage::StorageContext; @@ -239,8 +239,9 @@ impl BulkAppendTreeProof { let chunk_indices: Vec = chunk_indices_set.into_iter().collect(); let mmr_store = MmrStore::with_key_size(&tree.dense_tree.storage, MmrKeySize::U32); + let mmr = MMR::new_with_overlay(mmr_size, &mmr_store, tree.mmr_overlay.clone()); let get_node = |pos: u64| -> grovedb_merkle_mountain_range::Result> { - (&mmr_store).element_at_position(pos).unwrap() // unwrap CostResult + mmr.batch.element_at_position(pos).unwrap() // unwrap CostResult }; MmrTreeProof::generate(mmr_size, &chunk_indices, get_node).map_err(|e| { diff --git a/grovedb-bulk-append-tree/src/tree/append.rs b/grovedb-bulk-append-tree/src/tree/append.rs index efaa8a456..2c0cde406 100644 --- a/grovedb-bulk-append-tree/src/tree/append.rs +++ b/grovedb-bulk-append-tree/src/tree/append.rs @@ -19,6 +19,7 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { Ok(Self { total_count: 0, dense_tree, + mmr_overlay: Vec::new(), }) } @@ -42,6 +43,7 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { Ok(Self { total_count, dense_tree, + mmr_overlay: Vec::new(), }) } @@ -136,24 +138,26 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { let leaf_count = mmr_size_to_leaf_count(mmr_size); hash_count += hash_count_for_push(leaf_count); - // Create MmrStore on the fly from the dense tree's storage + // Create MmrStore on the fly from the dense tree's storage. + // Use the overlay from previous compactions so cross-compaction + // reads work without a storage round-trip. After push+get_root, + // take the overlay back (don't commit — that happens at session end). let mmr_root = { let mmr_store = MmrStore::with_key_size(&self.dense_tree.storage, MmrKeySize::U32); - let mut mmr = MMR::new(mmr_size, &mmr_store); + let mut mmr = + MMR::new_with_overlay(mmr_size, &mmr_store, std::mem::take(&mut self.mmr_overlay)); mmr.push(leaf) .unwrap() .map_err(|e| BulkAppendError::MmrError(format!("MMR push failed: {}", e)))?; - // Get root BEFORE commit — data is still in the MMRBatch overlay let root_node = mmr .get_root() .unwrap() .map_err(|e| BulkAppendError::MmrError(format!("MMR get_root failed: {}", e)))?; let root = root_node.hash(); - mmr.commit() - .unwrap() - .map_err(|e| BulkAppendError::MmrError(format!("MMR commit failed: {}", e)))?; + // Take overlay back instead of committing + self.mmr_overlay = mmr.batch.take_overlay(); root }; @@ -171,11 +175,32 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { return Ok([0u8; 32]); } let mmr_store = MmrStore::with_key_size(&self.dense_tree.storage, MmrKeySize::U32); - let mmr = MMR::new(mmr_size, &mmr_store); + let mmr = MMR::new_with_overlay(mmr_size, &mmr_store, self.mmr_overlay.clone()); let root_node = mmr .get_root() .unwrap() .map_err(|e| BulkAppendError::MmrError(format!("MMR get_root failed: {}", e)))?; Ok(root_node.hash()) } + + /// Flush the MMR overlay to storage. + /// + /// Call this at the end of a session to persist all MMR nodes that were + /// buffered during compaction cycles. This is a no-op if no compactions + /// occurred. + pub fn commit_mmr(&mut self) -> Result<(), BulkAppendError> { + if self.mmr_overlay.is_empty() { + return Ok(()); + } + let mmr_store = MmrStore::with_key_size(&self.dense_tree.storage, MmrKeySize::U32); + let mut mmr = MMR::new_with_overlay( + self.mmr_size(), + &mmr_store, + std::mem::take(&mut self.mmr_overlay), + ); + mmr.commit() + .unwrap() + .map_err(|e| BulkAppendError::MmrError(format!("MMR commit failed: {}", e)))?; + Ok(()) + } } diff --git a/grovedb-bulk-append-tree/src/tree/fetch.rs b/grovedb-bulk-append-tree/src/tree/fetch.rs index bb01e6118..3ad8bed63 100644 --- a/grovedb-bulk-append-tree/src/tree/fetch.rs +++ b/grovedb-bulk-append-tree/src/tree/fetch.rs @@ -1,7 +1,7 @@ //! Read operations for BulkAppendTree. use grovedb_dense_fixed_sized_merkle_tree::DenseTreeProof; -use grovedb_merkle_mountain_range::{leaf_to_pos, MMRStoreReadOps, MmrKeySize, MmrStore, MMR}; +use grovedb_merkle_mountain_range::{leaf_to_pos, MmrKeySize, MmrStore, MMR}; use grovedb_query::Query; use grovedb_storage::StorageContext; @@ -69,13 +69,18 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { /// /// This reads from the **chunk MMR**, which stores immutable epoch blobs. /// Returns `None` if the chunk hasn't been completed yet. + /// + /// Uses the MMR overlay to find nodes that were pushed during this session + /// but not yet committed to storage. pub fn get_chunk_value(&self, chunk_index: u64) -> Result>, BulkAppendError> { if chunk_index >= self.chunk_count() { return Ok(None); } let mmr_pos = leaf_to_pos(chunk_index); let mmr_store = MmrStore::with_key_size(&self.dense_tree.storage, MmrKeySize::U32); - let node = (&mmr_store) + let mmr = MMR::new_with_overlay(self.mmr_size(), &mmr_store, self.mmr_overlay.clone()); + let node = mmr + .batch .element_at_position(mmr_pos) .unwrap() .map_err(|e| { @@ -130,7 +135,7 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { (Vec::new(), [0u8; 32]) } else { let mmr_store = MmrStore::with_key_size(&self.dense_tree.storage, MmrKeySize::U32); - let mmr = MMR::new(mmr_size, &mmr_store); + let mmr = MMR::new_with_overlay(mmr_size, &mmr_store, self.mmr_overlay.clone()); let positions: Vec = chunk_indices.iter().map(|&idx| leaf_to_pos(idx)).collect(); let proof = mmr.gen_proof(positions).unwrap().map_err(|e| { diff --git a/grovedb-bulk-append-tree/src/tree/mod.rs b/grovedb-bulk-append-tree/src/tree/mod.rs index d15b1e5d2..2fe02f8a9 100644 --- a/grovedb-bulk-append-tree/src/tree/mod.rs +++ b/grovedb-bulk-append-tree/src/tree/mod.rs @@ -21,6 +21,7 @@ pub use fetch::{BufferQueryResult, ChunkQueryResult}; mod tests; use grovedb_dense_fixed_sized_merkle_tree::DenseFixedSizedMerkleTree; +use grovedb_merkle_mountain_range::MmrNode; #[cfg(feature = "storage")] use crate::BulkAppendError; @@ -69,6 +70,11 @@ pub struct BulkAppendTree { /// the MMR size and dense tree state. pub total_count: u64, pub dense_tree: DenseFixedSizedMerkleTree, + /// MMR node overlay: holds nodes pushed during this session that have + /// not yet been committed to storage. Persists across MMR instance + /// lifetimes (compaction cycles) so that reads can find recently-pushed + /// nodes without a storage round-trip. + pub(crate) mmr_overlay: Vec<(u64, Vec)>, } impl BulkAppendTree { diff --git a/grovedb-commitment-tree/src/commitment_tree/mod.rs b/grovedb-commitment-tree/src/commitment_tree/mod.rs index 214d30b51..28b461b66 100644 --- a/grovedb-commitment-tree/src/commitment_tree/mod.rs +++ b/grovedb-commitment-tree/src/commitment_tree/mod.rs @@ -366,6 +366,16 @@ impl<'db, S: StorageContext<'db>, M: MemoSize> CommitmentTree { // ── BulkAppendTree delegates ────────────────────────────────────── + /// Flush the MMR overlay to storage. + /// + /// Delegates to [`BulkAppendTree::commit_mmr`]. Call this at the end of a + /// session to persist MMR nodes buffered during compaction cycles. + pub fn commit_mmr(&mut self) -> Result<(), CommitmentTreeError> { + self.bulk_tree + .commit_mmr() + .map_err(|e| CommitmentTreeError::InvalidData(format!("MMR commit: {}", e))) + } + /// Get the total count of items appended (from the BulkAppendTree). pub fn total_count(&self) -> u64 { self.bulk_tree.total_count diff --git a/grovedb-dense-fixed-sized-merkle-tree/src/tests.rs b/grovedb-dense-fixed-sized-merkle-tree/src/tests.rs index 47b53958b..0802e62f1 100644 --- a/grovedb-dense-fixed-sized-merkle-tree/src/tests.rs +++ b/grovedb-dense-fixed-sized-merkle-tree/src/tests.rs @@ -681,8 +681,14 @@ fn test_get_store_inconsistency_errors() { #[test] fn test_proof_generate_store_failure() { - let mut tree = DenseFixedSizedMerkleTree::new(3, MemStorageContext::new()).expect("height 3"); - tree.insert(b"val").unwrap().expect("insert should succeed"); + // Use from_state (not insert) so the write-through cache is empty. + // Then corrupt the store to simulate missing data. + let store = MemStorageContext::new(); + store + .data + .borrow_mut() + .insert(position_key(0).to_vec(), b"val".to_vec()); + let tree = DenseFixedSizedMerkleTree::from_state(3, 1, store).expect("from_state"); // Corrupt the store by removing the value tree.storage diff --git a/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs b/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs index 6295d8774..2b511cd46 100644 --- a/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs +++ b/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs @@ -33,6 +33,11 @@ pub fn position_key(pos: u16) -> [u8; 2] { /// /// Storage is embedded directly on the struct (like Merk). /// +/// A write-through cache (`cache`) holds values written during this session. +/// Reads check the cache first, falling back to storage. This enables use +/// with transactional storage contexts where writes are deferred to a batch +/// and not yet visible through reads. +/// /// Note: root hash computation is O(n) per insert where n = count, since no /// intermediate hashes are cached. Suitable for small trees (epoch sizes /// typically 16-256). @@ -41,6 +46,10 @@ pub struct DenseFixedSizedMerkleTree { count: u16, /// The underlying storage context. pub storage: S, + /// Write-through cache: holds values written in this session. + /// Indexed by position. `None` means the value has not been written + /// in this session (fall back to storage). + cache: Vec>>, } // ── Pure accessors (no storage bounds needed) ───────────────────────── @@ -77,14 +86,20 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { /// Height must be between 1 and 16 inclusive. pub fn new(height: u8, storage: S) -> Result { validate_height(height)?; + let capacity = Self::capacity_for_height(height); Ok(Self { height, count: 0, storage, + cache: vec![None; capacity as usize], }) } /// Reconstitute a tree from stored state. + /// + /// The cache starts empty — pre-existing values are loaded from storage + /// on demand. Only values written via [`insert`] or [`try_insert`] in + /// this session are cached. pub fn from_state(height: u8, count: u16, storage: S) -> Result { validate_height(height)?; let capacity = Self::capacity_for_height(height); @@ -98,6 +113,7 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { height, count, storage, + cache: vec![None; capacity as usize], }) } @@ -204,16 +220,31 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { /// Reset the tree to empty state. /// - /// Sets count to 0. Old values remain in the underlying storage (they - /// will be overwritten on the next cycle). + /// Sets count to 0 and clears the write-through cache. Old values + /// remain in the underlying storage (they will be overwritten on the + /// next cycle). pub fn reset(&mut self) { self.count = 0; + self.cache.fill(None); } // ── Internal storage helpers ────────────────────────────────────── - /// Read a value by position from storage. + /// Read a value by position, checking the write-through cache first. + /// + /// Cache hits return deterministic costs (seek_count=1, + /// storage_loaded_bytes=len) matching the MMRBatch pattern, so fee + /// estimates are consistent regardless of cache state. pub(crate) fn get_value(&self, position: u16) -> CostResult>, DenseMerkleError> { + // Check write-through cache first + if let Some(Some(cached)) = self.cache.get(position as usize) { + return Ok(Some(cached.clone())).wrap_with_cost(OperationCost { + seek_count: 1, + storage_loaded_bytes: cached.len() as u64, + ..Default::default() + }); + } + // Fall back to storage let mut cost = OperationCost::default(); let key = position_key(position); let result = self.storage.get(key).unwrap_add_cost(&mut cost); @@ -227,8 +258,12 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { } } - /// Write a value by position to storage. - fn put_value(&self, position: u16, value: &[u8]) -> CostResult<(), DenseMerkleError> { + /// Write a value by position to storage and cache. + /// + /// On success, the value is stored in the write-through cache so that + /// subsequent reads (e.g., during root hash computation) can be served + /// from memory even when the storage context defers writes. + fn put_value(&mut self, position: u16, value: &[u8]) -> CostResult<(), DenseMerkleError> { let mut cost = OperationCost::default(); let key = position_key(position); let result = self @@ -236,7 +271,13 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { .put(key, value, None, None) .unwrap_add_cost(&mut cost); match result { - Ok(()) => Ok(()).wrap_with_cost(cost), + Ok(()) => { + // Cache on successful write + if let Some(slot) = self.cache.get_mut(position as usize) { + *slot = Some(value.to_vec()); + } + Ok(()).wrap_with_cost(cost) + } Err(e) => Err(DenseMerkleError::StoreError(format!( "put at pos {}: {}", position, e diff --git a/grovedb-merkle-mountain-range/src/mmr.rs b/grovedb-merkle-mountain-range/src/mmr.rs index 908a04243..880c8f9aa 100644 --- a/grovedb-merkle-mountain-range/src/mmr.rs +++ b/grovedb-merkle-mountain-range/src/mmr.rs @@ -42,6 +42,18 @@ impl MMR { } } + /// Create an MMR with a pre-populated overlay. + /// + /// The overlay contains nodes from previous push operations that have + /// not yet been committed to storage. This allows an MMR instance to + /// read nodes pushed by a previous instance without a storage round-trip. + pub fn new_with_overlay(mmr_size: u64, store: S, overlay: Vec<(u64, Vec)>) -> Self { + MMR { + mmr_size, + batch: MMRBatch::with_overlay(store, overlay), + } + } + /// Returns `true` if the MMR contains no elements. pub fn is_empty(&self) -> bool { self.mmr_size == 0 diff --git a/grovedb-merkle-mountain-range/src/mmr_store.rs b/grovedb-merkle-mountain-range/src/mmr_store.rs index f462e185e..1aae775bc 100644 --- a/grovedb-merkle-mountain-range/src/mmr_store.rs +++ b/grovedb-merkle-mountain-range/src/mmr_store.rs @@ -22,6 +22,26 @@ impl MMRBatch { } } + /// Create a batch with pre-populated overlay data. + /// + /// Used to restore an overlay from a previous session (e.g., across + /// multiple compaction cycles in BulkAppendTree) so that reads can + /// find nodes that were pushed but not yet committed to storage. + pub(crate) fn with_overlay(store: Store, overlay: Vec<(u64, Vec)>) -> Self { + MMRBatch { + memory_batch: overlay, + store, + } + } + + /// Extract the in-memory overlay data, leaving the batch empty. + /// + /// Used to persist the overlay across MMR instance lifetimes without + /// committing to storage. + pub fn take_overlay(&mut self) -> Vec<(u64, Vec)> { + std::mem::take(&mut self.memory_batch) + } + /// Buffer a contiguous run of elements starting at `pos`. /// /// Entries must be appended with monotonically increasing positions diff --git a/grovedb/src/operations/bulk_append_tree.rs b/grovedb/src/operations/bulk_append_tree.rs index 6764c223b..bcd1b5f64 100644 --- a/grovedb/src/operations/bulk_append_tree.rs +++ b/grovedb/src/operations/bulk_append_tree.rs @@ -65,14 +65,16 @@ impl GroveDb { } }; - // 2. Open storage + // 2. Open transactional storage (write-through cache + MMR overlay + // provide read-after-write visibility) let subtree_path_vec = self.build_subtree_path_for_bulk(&path, key); let subtree_path_refs: Vec<&[u8]> = subtree_path_vec.iter().map(|v| v.as_slice()).collect(); let subtree_path = SubtreePath::from(subtree_path_refs.as_slice()); + let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_immediate_storage_context(subtree_path, tx.as_ref()) + .get_transactional_storage_context(subtree_path, Some(&data_batch), tx.as_ref()) .unwrap_add_cost(&mut cost); // 3. Load tree, append @@ -88,9 +90,20 @@ impl GroveDb { let new_state_root = result.state_root; let new_total_count = tree.total_count; + // Flush MMR overlay to storage (through the batch) + cost_return_on_error_no_add!(cost, tree.commit_mmr().map_err(map_bulk_err)); + // Drop tree (and its embedded storage context) before opening merk drop(tree); + // Commit data batch to make writes visible in the transaction + cost_return_on_error!( + &mut cost, + self.db + .commit_multi_context_batch(data_batch, Some(tx.as_ref())) + .map_err(Into::into) + ); + // 4. Update element in parent Merk let batch = StorageBatch::new(); let mut parent_merk = cost_return_on_error!( @@ -487,15 +500,17 @@ impl GroveDb { } }; - // Open immediate storage (for read-after-write visibility) + // Open transactional storage (write-through cache + MMR overlay + // provide read-after-write visibility) let mut st_path_vec = path_vec.clone(); st_path_vec.push(key_bytes.clone()); let st_path_refs: Vec<&[u8]> = st_path_vec.iter().map(|v| v.as_slice()).collect(); let st_path = SubtreePath::from(st_path_refs.as_slice()); + let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_immediate_storage_context(st_path, transaction) + .get_transactional_storage_context(st_path, Some(&data_batch), transaction) .unwrap_add_cost(&mut cost); // Load tree with embedded storage @@ -521,9 +536,20 @@ impl GroveDb { let current_total_count = tree.total_count; + // Flush MMR overlay to storage (through the batch) + cost_return_on_error_no_add!(cost, tree.commit_mmr().map_err(map_bulk_err)); + // Drop tree (and its embedded storage context) drop(tree); + // Commit data batch to make writes visible in the transaction + cost_return_on_error!( + &mut cost, + self.db + .commit_multi_context_batch(data_batch, Some(transaction)) + .map_err(Into::into) + ); + // Create replacement op // Key is restored for downstream (from_ops, execute_ops_on_path) let replacement = QualifiedGroveDbOp { diff --git a/grovedb/src/operations/commitment_tree.rs b/grovedb/src/operations/commitment_tree.rs index 9abc82931..3a53c607f 100644 --- a/grovedb/src/operations/commitment_tree.rs +++ b/grovedb/src/operations/commitment_tree.rs @@ -116,14 +116,16 @@ impl GroveDb { } }; - // 2. Build subtree path and open storage context + // 2. Build subtree path and open transactional storage (write-through + // cache + MMR overlay provide read-after-write visibility) let ct_path_vec = self.build_ct_path(&path, key); let ct_path_refs: Vec<&[u8]> = ct_path_vec.iter().map(|v| v.as_slice()).collect(); let ct_path = SubtreePath::from(ct_path_refs.as_slice()); + let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_immediate_storage_context(ct_path, tx.as_ref()) + .get_transactional_storage_context(ct_path, Some(&data_batch), tx.as_ref()) .unwrap_add_cost(&mut cost); // 3. Open composite CommitmentTree and append (uses default DashMemo for @@ -156,9 +158,20 @@ impl GroveDb { &bulk_state_root, ); + // Flush MMR overlay to storage (through the batch) + cost_return_on_error_no_add!(cost, ct.commit_mmr().map_err(map_ct_err)); + // Drop ct (and its storage context) before opening merk drop(ct); + // Commit data batch to make writes visible in the transaction + cost_return_on_error!( + &mut cost, + self.db + .commit_multi_context_batch(data_batch, Some(tx.as_ref())) + .map_err(Into::into) + ); + // 5. Update element in parent Merk let batch = StorageBatch::new(); let mut parent_merk = cost_return_on_error!( @@ -467,15 +480,17 @@ impl GroveDb { } }; - // Build subtree path and open single storage context + // Build subtree path and open transactional storage (write-through + // cache + MMR overlay provide read-after-write visibility) let mut ct_path_vec = path_vec.clone(); ct_path_vec.push(key_bytes.clone()); let ct_path_refs: Vec<&[u8]> = ct_path_vec.iter().map(|v| v.as_slice()).collect(); let ct_path = SubtreePath::from(ct_path_refs.as_slice()); + let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_immediate_storage_context(ct_path, transaction) + .get_transactional_storage_context(ct_path, Some(&data_batch), transaction) .unwrap_add_cost(&mut cost); // Open composite CommitmentTree @@ -504,9 +519,20 @@ impl GroveDb { ); let current_total_count = ct.total_count(); + // Flush MMR overlay to storage (through the batch) + cost_return_on_error_no_add!(cost, ct.commit_mmr().map_err(map_ct_err)); + // Drop ct (and its storage context) drop(ct); + // Commit data batch to make writes visible in the transaction + cost_return_on_error!( + &mut cost, + self.db + .commit_multi_context_batch(data_batch, Some(transaction)) + .map_err(Into::into) + ); + // Create a ReplaceNonMerkTreeRoot // Key is restored for downstream (from_ops, execute_ops_on_path) let replacement = QualifiedGroveDbOp { diff --git a/grovedb/src/operations/dense_tree.rs b/grovedb/src/operations/dense_tree.rs index 452b279c1..7aa1dacc7 100644 --- a/grovedb/src/operations/dense_tree.rs +++ b/grovedb/src/operations/dense_tree.rs @@ -67,10 +67,13 @@ impl GroveDb { let subtree_path_refs: Vec<&[u8]> = subtree_path_vec.iter().map(|v| v.as_slice()).collect(); let subtree_path = SubtreePath::from(subtree_path_refs.as_slice()); - // 3. Open storage, create tree with embedded storage, insert + // 3. Open storage, create tree with embedded storage, insert. + // Uses transactional context — the dense tree's write-through cache + // provides read-after-write visibility for root hash computation. + let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_immediate_storage_context(subtree_path.clone(), tx.as_ref()) + .get_transactional_storage_context(subtree_path.clone(), Some(&data_batch), tx.as_ref()) .unwrap_add_cost(&mut cost); let mut tree = cost_return_on_error_no_add!( @@ -87,9 +90,17 @@ impl GroveDb { let new_count = tree.count(); - // Drop tree (and its embedded storage context) before opening merk + // Drop tree (and its embedded storage context) before committing drop(tree); + // Commit data writes to the transaction + cost_return_on_error!( + &mut cost, + self.db + .commit_multi_context_batch(data_batch, Some(tx.as_ref())) + .map_err(Into::into) + ); + // 4. Update element and propagate let batch = StorageBatch::new(); let mut parent_merk = cost_return_on_error!( @@ -391,10 +402,14 @@ impl GroveDb { let st_path_refs: Vec<&[u8]> = st_path_vec.iter().map(|v| v.as_slice()).collect(); let st_path = SubtreePath::from(st_path_refs.as_slice()); - // Use immediate storage for read-after-write visibility + // Use transactional storage context with a batch. The dense + // tree's write-through cache provides read-after-write + // visibility for values written during this session, so we + // don't need immediate context. + let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_immediate_storage_context(st_path.clone(), transaction) + .get_transactional_storage_context(st_path.clone(), Some(&data_batch), transaction) .unwrap_add_cost(&mut cost); let mut tree = cost_return_on_error_no_add!( @@ -416,9 +431,18 @@ impl GroveDb { let new_count = tree.count(); - // Drop tree (and its embedded storage context) + // Drop tree (and its embedded storage context) before committing drop(tree); + // Commit the data batch to the transaction so writes are + // visible to subsequent operations in the pipeline + cost_return_on_error!( + &mut cost, + self.db + .commit_multi_context_batch(data_batch, Some(transaction)) + .map_err(Into::into) + ); + // Key is restored for downstream (from_ops, execute_ops_on_path) let replacement = QualifiedGroveDbOp { path: crate::batch::KeyInfoPath::from_known_owned_path(path_vec), diff --git a/grovedb/src/operations/mmr_tree.rs b/grovedb/src/operations/mmr_tree.rs index 6dc507559..a2bb47314 100644 --- a/grovedb/src/operations/mmr_tree.rs +++ b/grovedb/src/operations/mmr_tree.rs @@ -69,10 +69,13 @@ impl GroveDb { let subtree_path_refs: Vec<&[u8]> = subtree_path_vec.iter().map(|v| v.as_slice()).collect(); let subtree_path = SubtreePath::from(subtree_path_refs.as_slice()); - // 3. Open storage, create store adapter, push value + // 3. Open storage, create store adapter, push value. + // Uses transactional context — the MMRBatch overlay provides + // read-after-write, and root is computed before commit. + let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_immediate_storage_context(subtree_path.clone(), tx.as_ref()) + .get_transactional_storage_context(subtree_path.clone(), Some(&data_batch), tx.as_ref()) .unwrap_add_cost(&mut cost); let store = MmrStore::new(&storage_ctx); @@ -107,6 +110,14 @@ impl GroveDb { #[allow(clippy::drop_non_drop)] drop(storage_ctx); + // Commit data writes to the transaction + cost_return_on_error!( + &mut cost, + self.db + .commit_multi_context_batch(data_batch, Some(tx.as_ref())) + .map_err(Into::into) + ); + // 4. Open parent Merk and update the MmrTree element let batch = StorageBatch::new(); let mut parent_merk = cost_return_on_error!( @@ -408,10 +419,13 @@ impl GroveDb { let st_path_refs: Vec<&[u8]> = st_path_vec.iter().map(|v| v.as_slice()).collect(); let st_path = SubtreePath::from(st_path_refs.as_slice()); - // Open immediate storage context (consistent with other preprocessors) + // Use transactional storage context. The MMRBatch overlay + // provides read-after-write for newly pushed values, and root + // is computed before commit, so immediate context is not needed. + let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_immediate_storage_context(st_path, transaction) + .get_transactional_storage_context(st_path, Some(&data_batch), transaction) .unwrap_add_cost(&mut cost); let store = MmrStore::new(&storage_ctx); @@ -448,6 +462,14 @@ impl GroveDb { #[allow(clippy::drop_non_drop)] drop(storage_ctx); + // Commit data writes to the transaction + cost_return_on_error!( + &mut cost, + self.db + .commit_multi_context_batch(data_batch, Some(transaction)) + .map_err(Into::into) + ); + // Create a ReplaceNonMerkTreeRoot — MMR root flows as child hash // Key is restored for downstream (from_ops, execute_ops_on_path) let replacement = QualifiedGroveDbOp { From e90e6fe6c27374887f0c1433fdf19e24cb0832b0 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 10 Mar 2026 11:37:27 +0700 Subject: [PATCH 2/9] refactor: switch read-only specialized tree ops to transactional context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aligns with Merk which always uses transactional storage context for both reads and writes. The remaining get_immediate_storage_context calls in grovedb/src/operations/ are now zero — all specialized tree operations (dense tree, MMR, bulk append, commitment tree, and proof generation) consistently use get_transactional_storage_context. Read-only operations pass None as the batch parameter since they don't write. Co-Authored-By: Claude Opus 4.6 --- grovedb/src/operations/bulk_append_tree.rs | 6 +++--- grovedb/src/operations/commitment_tree.rs | 4 ++-- grovedb/src/operations/dense_tree.rs | 4 ++-- grovedb/src/operations/mmr_tree.rs | 4 ++-- grovedb/src/operations/proof/generate.rs | 8 ++++---- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/grovedb/src/operations/bulk_append_tree.rs b/grovedb/src/operations/bulk_append_tree.rs index bcd1b5f64..c7d7c786e 100644 --- a/grovedb/src/operations/bulk_append_tree.rs +++ b/grovedb/src/operations/bulk_append_tree.rs @@ -201,7 +201,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(subtree_path, tx.as_ref()) + .get_transactional_storage_context(subtree_path, None, tx.as_ref()) .unwrap_add_cost(&mut cost); let tree = cost_return_on_error_no_add!( @@ -281,7 +281,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(subtree_path, tx.as_ref()) + .get_transactional_storage_context(subtree_path, None, tx.as_ref()) .unwrap_add_cost(&mut cost); let tree = cost_return_on_error_no_add!( @@ -334,7 +334,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(subtree_path, tx.as_ref()) + .get_transactional_storage_context(subtree_path, None, tx.as_ref()) .unwrap_add_cost(&mut cost); let tree = cost_return_on_error_no_add!( diff --git a/grovedb/src/operations/commitment_tree.rs b/grovedb/src/operations/commitment_tree.rs index 3a53c607f..47af5b042 100644 --- a/grovedb/src/operations/commitment_tree.rs +++ b/grovedb/src/operations/commitment_tree.rs @@ -265,7 +265,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(ct_path, tx.as_ref()) + .get_transactional_storage_context(ct_path, None, tx.as_ref()) .unwrap_add_cost(&mut cost); let ct = cost_return_on_error!( @@ -320,7 +320,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(ct_path, tx.as_ref()) + .get_transactional_storage_context(ct_path, None, tx.as_ref()) .unwrap_add_cost(&mut cost); let ct = cost_return_on_error!( diff --git a/grovedb/src/operations/dense_tree.rs b/grovedb/src/operations/dense_tree.rs index 7aa1dacc7..f0d41ac41 100644 --- a/grovedb/src/operations/dense_tree.rs +++ b/grovedb/src/operations/dense_tree.rs @@ -195,7 +195,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(subtree_path, tx.as_ref()) + .get_transactional_storage_context(subtree_path, None, tx.as_ref()) .unwrap_add_cost(&mut cost); // Read directly from storage using position_key (no need to construct tree) @@ -250,7 +250,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(subtree_path, tx.as_ref()) + .get_transactional_storage_context(subtree_path, None, tx.as_ref()) .unwrap_add_cost(&mut cost); let tree = cost_return_on_error_no_add!( diff --git a/grovedb/src/operations/mmr_tree.rs b/grovedb/src/operations/mmr_tree.rs index a2bb47314..d31d05d40 100644 --- a/grovedb/src/operations/mmr_tree.rs +++ b/grovedb/src/operations/mmr_tree.rs @@ -211,7 +211,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(subtree_path, tx.as_ref()) + .get_transactional_storage_context(subtree_path, None, tx.as_ref()) .unwrap_add_cost(&mut cost); let store = MmrStore::new(&storage_ctx); @@ -270,7 +270,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(subtree_path, tx.as_ref()) + .get_transactional_storage_context(subtree_path, None, tx.as_ref()) .unwrap_add_cost(&mut cost); let store = MmrStore::new(&storage_ctx); diff --git a/grovedb/src/operations/proof/generate.rs b/grovedb/src/operations/proof/generate.rs index d96451cb0..9cc3e4ec0 100644 --- a/grovedb/src/operations/proof/generate.rs +++ b/grovedb/src/operations/proof/generate.rs @@ -1346,7 +1346,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(storage_path, tx) + .get_transactional_storage_context(storage_path, None, tx) .unwrap_add_cost(&mut cost); // Generate the MMR proof using MmrStore for correct key format @@ -1426,7 +1426,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(storage_path, tx) + .get_transactional_storage_context(storage_path, None, tx) .unwrap_add_cost(&mut cost); // Create BulkAppendTree from state with embedded storage @@ -1498,7 +1498,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(storage_path, tx) + .get_transactional_storage_context(storage_path, None, tx) .unwrap_add_cost(&mut cost); let sinsemilla_root = match storage_ctx.get(COMMITMENT_TREE_DATA_KEY).value { @@ -1590,7 +1590,7 @@ impl GroveDb { let storage_ctx = self .db - .get_immediate_storage_context(storage_path, tx) + .get_transactional_storage_context(storage_path, None, tx) .unwrap_add_cost(&mut cost); // Create dense tree with embedded storage From 3a3605aa0350e8913f260ece5b2bcef0357497a7 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 10 Mar 2026 11:49:28 +0700 Subject: [PATCH 3/9] fix: restore MMR overlay on error, clear cache on rollback (audit fixes) - compact_with_value: if mmr.push() or mmr.get_root() fails after std::mem::take drains the overlay, restore it before returning the error. Previously the overlay data from all prior compaction cycles would be permanently lost. - DenseFixedSizedMerkleTree insert/try_insert: clear the cache slot when rolling back count after a compute_root_hash failure. Prevents a stale cache entry from diverging from the logical tree state. - put_value: add debug_assert! that position < cache capacity. - commit_mmr: add doc comment explaining why cost tracking is intentionally omitted at this boundary. Co-Authored-By: Claude Opus 4.6 --- grovedb-bulk-append-tree/src/tree/append.rs | 30 ++++++++++++++----- .../src/tree.rs | 18 +++++++++-- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/grovedb-bulk-append-tree/src/tree/append.rs b/grovedb-bulk-append-tree/src/tree/append.rs index 2c0cde406..9096b5f26 100644 --- a/grovedb-bulk-append-tree/src/tree/append.rs +++ b/grovedb-bulk-append-tree/src/tree/append.rs @@ -146,15 +146,25 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { let mmr_store = MmrStore::with_key_size(&self.dense_tree.storage, MmrKeySize::U32); let mut mmr = MMR::new_with_overlay(mmr_size, &mmr_store, std::mem::take(&mut self.mmr_overlay)); - mmr.push(leaf) - .unwrap() - .map_err(|e| BulkAppendError::MmrError(format!("MMR push failed: {}", e)))?; - let root_node = mmr - .get_root() - .unwrap() - .map_err(|e| BulkAppendError::MmrError(format!("MMR get_root failed: {}", e)))?; - let root = root_node.hash(); + let push_result = mmr.push(leaf).unwrap(); + if let Err(e) = push_result { + // Restore overlay before returning error + self.mmr_overlay = mmr.batch.take_overlay(); + return Err(BulkAppendError::MmrError(format!("MMR push failed: {}", e))); + } + + let root_result = mmr.get_root().unwrap(); + let root = match root_result { + Ok(node) => node.hash(), + Err(e) => { + self.mmr_overlay = mmr.batch.take_overlay(); + return Err(BulkAppendError::MmrError(format!( + "MMR get_root failed: {}", + e + ))); + } + }; // Take overlay back instead of committing self.mmr_overlay = mmr.batch.take_overlay(); @@ -188,6 +198,10 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { /// Call this at the end of a session to persist all MMR nodes that were /// buffered during compaction cycles. This is a no-op if no compactions /// occurred. + /// + /// Cost tracking is intentionally omitted at this boundary: + /// BulkAppendTree returns plain `Result`, not `CostResult`. Storage + /// I/O costs are captured by the caller's `commit_multi_context_batch`. pub fn commit_mmr(&mut self) -> Result<(), BulkAppendError> { if self.mmr_overlay.is_empty() { return Ok(()); diff --git a/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs b/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs index 2b511cd46..5484c0412 100644 --- a/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs +++ b/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs @@ -143,10 +143,13 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { // succeeds, which needs a storage fault (get fails on a key that was // just written). Not reachable with any StorageContext implementation. Err(e) => { - // Roll back count so the tree state remains consistent. - // Note: the value remains in the store; the caller is - // responsible for store-level cleanup if needed. + // Roll back count and cache so the tree state remains + // consistent. The value remains in the store; the caller + // is responsible for store-level cleanup if needed. self.count -= 1; + if let Some(slot) = self.cache.get_mut(position as usize) { + *slot = None; + } Err(e).wrap_with_cost(cost) } } @@ -174,6 +177,9 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { Ok(root_hash) => Ok(Some((root_hash, position))).wrap_with_cost(cost), Err(e) => { self.count -= 1; + if let Some(slot) = self.cache.get_mut(position as usize) { + *slot = None; + } Err(e).wrap_with_cost(cost) } } @@ -264,6 +270,12 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { /// subsequent reads (e.g., during root hash computation) can be served /// from memory even when the storage context defers writes. fn put_value(&mut self, position: u16, value: &[u8]) -> CostResult<(), DenseMerkleError> { + debug_assert!( + (position as usize) < self.cache.len(), + "put_value called with position {} >= cache capacity {}", + position, + self.cache.len() + ); let mut cost = OperationCost::default(); let key = position_key(position); let result = self From 4baefde47e8a1c68c10a8bfa7f6c97bb43d42b45 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 10 Mar 2026 13:10:28 +0700 Subject: [PATCH 4/9] fix: preprocessing shares StorageBatch for atomic batch discard Preprocessing functions (dense, MMR, bulk append, commitment) previously created their own StorageBatch and committed it directly to the transaction. This meant failed batches with borrowed (external) transactions would leak orphaned preprocessing data into the transaction. Now all preprocessing functions receive the shared StorageBatch from apply_batch, so writes are only committed atomically at the end. Batch discard tests switched from internal (owned) transactions to external (borrowed) transactions to verify no preprocessing data leaks into the transaction on batch failure. Co-Authored-By: Claude Opus 4.6 --- grovedb/src/batch/mod.rs | 16 +- grovedb/src/operations/bulk_append_tree.rs | 12 +- grovedb/src/operations/commitment_tree.rs | 12 +- grovedb/src/operations/dense_tree.rs | 19 +- grovedb/src/operations/mmr_tree.rs | 12 +- grovedb/src/tests/bulk_append_tree_tests.rs | 364 +++++++++++++++++ grovedb/src/tests/commitment_tree_tests.rs | 426 ++++++++++++++++++++ grovedb/src/tests/dense_tree_tests.rs | 300 ++++++++++++++ grovedb/src/tests/mmr_tree_tests.rs | 294 ++++++++++++++ 9 files changed, 1405 insertions(+), 50 deletions(-) diff --git a/grovedb/src/batch/mod.rs b/grovedb/src/batch/mod.rs index 65d58fdf5..3652ad66a 100644 --- a/grovedb/src/batch/mod.rs +++ b/grovedb/src/batch/mod.rs @@ -3437,28 +3437,28 @@ impl GroveDb { // then convert to ReplaceTreeRootKey ops let ops = cost_return_on_error!( &mut cost, - self.preprocess_commitment_tree_ops(ops, tx.as_ref(), grove_version) + self.preprocess_commitment_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) ); // Preprocess MmrTreeAppend ops: execute MMR operations // then convert to ReplaceTreeRootKey ops let ops = cost_return_on_error!( &mut cost, - self.preprocess_mmr_tree_ops(ops, tx.as_ref(), grove_version) + self.preprocess_mmr_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) ); // Preprocess BulkAppend ops: execute bulk append operations // then convert to ReplaceTreeRootKey ops let ops = cost_return_on_error!( &mut cost, - self.preprocess_bulk_append_ops(ops, tx.as_ref(), grove_version) + self.preprocess_bulk_append_ops(ops, tx.as_ref(), &storage_batch, grove_version) ); // Preprocess DenseTreeInsert ops: execute dense tree operations // then convert to ReplaceTreeRootKey ops let ops = cost_return_on_error!( &mut cost, - self.preprocess_dense_tree_ops(ops, tx.as_ref(), grove_version) + self.preprocess_dense_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) ); // Collect paths of subtrees being deleted, separated by type. @@ -3806,25 +3806,25 @@ impl GroveDb { // Preprocess CommitmentTreeInsert ops let ops = cost_return_on_error!( &mut cost, - self.preprocess_commitment_tree_ops(ops, tx.as_ref(), grove_version) + self.preprocess_commitment_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) ); // Preprocess MmrTreeAppend ops let ops = cost_return_on_error!( &mut cost, - self.preprocess_mmr_tree_ops(ops, tx.as_ref(), grove_version) + self.preprocess_mmr_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) ); // Preprocess BulkAppend ops let ops = cost_return_on_error!( &mut cost, - self.preprocess_bulk_append_ops(ops, tx.as_ref(), grove_version) + self.preprocess_bulk_append_ops(ops, tx.as_ref(), &storage_batch, grove_version) ); // Preprocess DenseTreeInsert ops let ops = cost_return_on_error!( &mut cost, - self.preprocess_dense_tree_ops(ops, tx.as_ref(), grove_version) + self.preprocess_dense_tree_ops(ops, tx.as_ref(), &storage_batch, grove_version) ); // See comment in apply_batch_with_element_flags_update for why diff --git a/grovedb/src/operations/bulk_append_tree.rs b/grovedb/src/operations/bulk_append_tree.rs index c7d7c786e..534963ef6 100644 --- a/grovedb/src/operations/bulk_append_tree.rs +++ b/grovedb/src/operations/bulk_append_tree.rs @@ -434,6 +434,7 @@ impl GroveDb { &self, ops: Vec, transaction: &Transaction, + storage_batch: &StorageBatch, grove_version: &GroveVersion, ) -> CostResult, Error> { let mut cost = OperationCost::default(); @@ -507,10 +508,9 @@ impl GroveDb { let st_path_refs: Vec<&[u8]> = st_path_vec.iter().map(|v| v.as_slice()).collect(); let st_path = SubtreePath::from(st_path_refs.as_slice()); - let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_transactional_storage_context(st_path, Some(&data_batch), transaction) + .get_transactional_storage_context(st_path, Some(storage_batch), transaction) .unwrap_add_cost(&mut cost); // Load tree with embedded storage @@ -542,14 +542,6 @@ impl GroveDb { // Drop tree (and its embedded storage context) drop(tree); - // Commit data batch to make writes visible in the transaction - cost_return_on_error!( - &mut cost, - self.db - .commit_multi_context_batch(data_batch, Some(transaction)) - .map_err(Into::into) - ); - // Create replacement op // Key is restored for downstream (from_ops, execute_ops_on_path) let replacement = QualifiedGroveDbOp { diff --git a/grovedb/src/operations/commitment_tree.rs b/grovedb/src/operations/commitment_tree.rs index 47af5b042..03528325c 100644 --- a/grovedb/src/operations/commitment_tree.rs +++ b/grovedb/src/operations/commitment_tree.rs @@ -409,6 +409,7 @@ impl GroveDb { &self, ops: Vec, transaction: &Transaction, + storage_batch: &StorageBatch, grove_version: &GroveVersion, ) -> CostResult, Error> { let mut cost = OperationCost::default(); @@ -487,10 +488,9 @@ impl GroveDb { let ct_path_refs: Vec<&[u8]> = ct_path_vec.iter().map(|v| v.as_slice()).collect(); let ct_path = SubtreePath::from(ct_path_refs.as_slice()); - let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_transactional_storage_context(ct_path, Some(&data_batch), transaction) + .get_transactional_storage_context(ct_path, Some(storage_batch), transaction) .unwrap_add_cost(&mut cost); // Open composite CommitmentTree @@ -525,14 +525,6 @@ impl GroveDb { // Drop ct (and its storage context) drop(ct); - // Commit data batch to make writes visible in the transaction - cost_return_on_error!( - &mut cost, - self.db - .commit_multi_context_batch(data_batch, Some(transaction)) - .map_err(Into::into) - ); - // Create a ReplaceNonMerkTreeRoot // Key is restored for downstream (from_ops, execute_ops_on_path) let replacement = QualifiedGroveDbOp { diff --git a/grovedb/src/operations/dense_tree.rs b/grovedb/src/operations/dense_tree.rs index f0d41ac41..8fe28c65e 100644 --- a/grovedb/src/operations/dense_tree.rs +++ b/grovedb/src/operations/dense_tree.rs @@ -316,6 +316,7 @@ impl GroveDb { &self, ops: Vec, transaction: &Transaction, + storage_batch: &StorageBatch, grove_version: &GroveVersion, ) -> CostResult, Error> { let mut cost = OperationCost::default(); @@ -406,10 +407,13 @@ impl GroveDb { // tree's write-through cache provides read-after-write // visibility for values written during this session, so we // don't need immediate context. - let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_transactional_storage_context(st_path.clone(), Some(&data_batch), transaction) + .get_transactional_storage_context( + st_path.clone(), + Some(storage_batch), + transaction, + ) .unwrap_add_cost(&mut cost); let mut tree = cost_return_on_error_no_add!( @@ -431,18 +435,9 @@ impl GroveDb { let new_count = tree.count(); - // Drop tree (and its embedded storage context) before committing + // Drop tree (and its embedded storage context) drop(tree); - // Commit the data batch to the transaction so writes are - // visible to subsequent operations in the pipeline - cost_return_on_error!( - &mut cost, - self.db - .commit_multi_context_batch(data_batch, Some(transaction)) - .map_err(Into::into) - ); - // Key is restored for downstream (from_ops, execute_ops_on_path) let replacement = QualifiedGroveDbOp { path: crate::batch::KeyInfoPath::from_known_owned_path(path_vec), diff --git a/grovedb/src/operations/mmr_tree.rs b/grovedb/src/operations/mmr_tree.rs index d31d05d40..96cb9cfed 100644 --- a/grovedb/src/operations/mmr_tree.rs +++ b/grovedb/src/operations/mmr_tree.rs @@ -347,6 +347,7 @@ impl GroveDb { &self, ops: Vec, transaction: &Transaction, + storage_batch: &StorageBatch, grove_version: &GroveVersion, ) -> CostResult, Error> { let mut cost = OperationCost::default(); @@ -422,10 +423,9 @@ impl GroveDb { // Use transactional storage context. The MMRBatch overlay // provides read-after-write for newly pushed values, and root // is computed before commit, so immediate context is not needed. - let data_batch = StorageBatch::new(); let storage_ctx = self .db - .get_transactional_storage_context(st_path, Some(&data_batch), transaction) + .get_transactional_storage_context(st_path, Some(storage_batch), transaction) .unwrap_add_cost(&mut cost); let store = MmrStore::new(&storage_ctx); @@ -462,14 +462,6 @@ impl GroveDb { #[allow(clippy::drop_non_drop)] drop(storage_ctx); - // Commit data writes to the transaction - cost_return_on_error!( - &mut cost, - self.db - .commit_multi_context_batch(data_batch, Some(transaction)) - .map_err(Into::into) - ); - // Create a ReplaceNonMerkTreeRoot — MMR root flows as child hash // Key is restored for downstream (from_ops, execute_ops_on_path) let replacement = QualifiedGroveDbOp { diff --git a/grovedb/src/tests/bulk_append_tree_tests.rs b/grovedb/src/tests/bulk_append_tree_tests.rs index 1aa51df2e..1e78d62e9 100644 --- a/grovedb/src/tests/bulk_append_tree_tests.rs +++ b/grovedb/src/tests/bulk_append_tree_tests.rs @@ -1713,3 +1713,367 @@ fn test_bulk_batch_with_non_bulk_element() { result ); } + +// =========================================================================== +// Batch discard / transaction rollback tests +// =========================================================================== + +/// A batch with bulk appends succeeds, then a later op in the batch fails. +/// The entire batch must be discarded — the bulk count should remain at its +/// pre-batch value. +#[test] +fn test_bulk_batch_discarded_on_later_op_failure() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + + db.insert( + [b"parent"].as_ref(), + b"bulk", + Element::empty_bulk_append_tree(TEST_CHUNK_POWER).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert bulk tree"); + + // Pre-insert an item so insert_if_not_exists will fail + db.insert( + [b"parent"].as_ref(), + b"existing", + Element::new_item(b"old".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert existing item"); + + // Batch: bulk append (should succeed internally) + conflicting insert + let ops = vec![ + QualifiedGroveDbOp::bulk_append_op( + vec![b"parent".to_vec(), b"bulk".to_vec()], + b"note_data".to_vec(), + ), + QualifiedGroveDbOp::insert_if_not_exists_op( + vec![b"parent".to_vec()], + b"existing".to_vec(), + Element::new_item(b"conflict".to_vec()), + ), + ]; + + // Use an external (borrowed) transaction to verify no preprocessing data + // leaks into it when the batch fails. + let tx = db.start_transaction(); + let result = db.apply_batch(ops, None, Some(&tx), grove_version).unwrap(); + assert!(result.is_err(), "batch should fail due to duplicate key"); + + // Bulk count must remain 0 inside the transaction + let count_in_tx = db + .bulk_count([b"parent"].as_ref(), b"bulk", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!( + count_in_tx, 0, + "bulk count should be 0 inside tx after batch discard" + ); + + // Also verify outside the transaction + let count = db + .bulk_count([b"parent"].as_ref(), b"bulk", None, grove_version) + .unwrap() + .expect("count"); + assert_eq!(count, 0, "bulk count should be 0 after batch discard"); + + db.rollback_transaction(&tx).expect("rollback"); +} + +/// Bulk appends that trigger compaction, then a later op fails. Verifies +/// both the dense tree buffer and the MMR state are discarded. +#[test] +fn test_bulk_batch_discarded_after_compaction() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + + // chunk_power=2 → capacity=3, epoch_size=4 → compaction on 4th append + db.insert( + [b"parent"].as_ref(), + b"bulk", + Element::empty_bulk_append_tree(TEST_CHUNK_POWER).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert bulk tree"); + + db.insert( + [b"parent"].as_ref(), + b"existing", + Element::new_item(b"old".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert existing item"); + + // Build a batch with enough appends to trigger compaction (4 appends for + // chunk_power=2), followed by a conflicting insert to fail the batch + let mut ops: Vec = (0..TEST_CHUNK_SIZE as u8) + .map(|i| { + QualifiedGroveDbOp::bulk_append_op(vec![b"parent".to_vec(), b"bulk".to_vec()], vec![i]) + }) + .collect(); + ops.push(QualifiedGroveDbOp::insert_if_not_exists_op( + vec![b"parent".to_vec()], + b"existing".to_vec(), + Element::new_item(b"conflict".to_vec()), + )); + + // Use an external (borrowed) transaction to verify no preprocessing data + // leaks into it when the batch fails — even after compaction. + let tx = db.start_transaction(); + let result = db.apply_batch(ops, None, Some(&tx), grove_version).unwrap(); + assert!( + result.is_err(), + "batch should fail after compaction + conflict" + ); + + // Bulk count must be 0 inside the transaction + let count_in_tx = db + .bulk_count([b"parent"].as_ref(), b"bulk", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!( + count_in_tx, 0, + "bulk count should be 0 inside tx after batch discard (even after compaction)" + ); + + // Also verify outside the transaction + let count = db + .bulk_count([b"parent"].as_ref(), b"bulk", None, grove_version) + .unwrap() + .expect("count"); + assert_eq!( + count, 0, + "bulk count should be 0 after batch discard (even after compaction)" + ); + + db.rollback_transaction(&tx).expect("rollback"); +} + +/// Bulk appends inside a transaction, then the transaction is rolled back. +/// Verifies the count reverts to pre-transaction value. +#[test] +fn test_bulk_transaction_rollback_reverts_appends() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"bulk", + Element::empty_bulk_append_tree(TEST_CHUNK_POWER).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert bulk tree"); + + // Insert one value outside the transaction + db.bulk_append(EMPTY_PATH, b"bulk", b"v0".to_vec(), None, grove_version) + .unwrap() + .expect("append v0"); + + let count_before = db + .bulk_count(EMPTY_PATH, b"bulk", None, grove_version) + .unwrap() + .expect("count before"); + assert_eq!(count_before, 1); + + // Start transaction and append more (enough to trigger compaction) + let tx = db.start_transaction(); + + for i in 1..(TEST_CHUNK_SIZE as u8 + 2) { + db.bulk_append(EMPTY_PATH, b"bulk", vec![i], Some(&tx), grove_version) + .unwrap() + .expect("append in tx"); + } + + // Rollback + db.rollback_transaction(&tx).expect("rollback"); + + // Count should revert to 1 + let count_after = db + .bulk_count(EMPTY_PATH, b"bulk", None, grove_version) + .unwrap() + .expect("count after rollback"); + assert_eq!(count_after, 1, "bulk count should revert after rollback"); +} + +/// After a failed batch, a subsequent successful batch on the same bulk +/// append tree should work correctly — no stale cache/overlay state leaks. +#[test] +fn test_bulk_successful_batch_after_failed_batch() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + + db.insert( + [b"parent"].as_ref(), + b"bulk", + Element::empty_bulk_append_tree(TEST_CHUNK_POWER).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert bulk tree"); + + db.insert( + [b"parent"].as_ref(), + b"existing", + Element::new_item(b"old".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert existing item"); + + // Use an external transaction — the failing batch must not leak + // preprocessing data into the tx. + let tx = db.start_transaction(); + + // First batch: bulk append + conflicting insert → fails + let ops_fail = vec![ + QualifiedGroveDbOp::bulk_append_op( + vec![b"parent".to_vec(), b"bulk".to_vec()], + b"ghost".to_vec(), + ), + QualifiedGroveDbOp::insert_if_not_exists_op( + vec![b"parent".to_vec()], + b"existing".to_vec(), + Element::new_item(b"conflict".to_vec()), + ), + ]; + + let result = db + .apply_batch(ops_fail, None, Some(&tx), grove_version) + .unwrap(); + assert!(result.is_err(), "first batch should fail"); + + // Tx should be clean — no preprocessing data leaked + let count_after_fail = db + .bulk_count([b"parent"].as_ref(), b"bulk", Some(&tx), grove_version) + .unwrap() + .expect("count after fail"); + assert_eq!( + count_after_fail, 0, + "bulk count in tx should be 0 after failed batch" + ); + + // Second batch: bulk append only → should succeed in the same tx + let ops_ok = vec![QualifiedGroveDbOp::bulk_append_op( + vec![b"parent".to_vec(), b"bulk".to_vec()], + b"real".to_vec(), + )]; + + db.apply_batch(ops_ok, None, Some(&tx), grove_version) + .unwrap() + .expect("second batch should succeed"); + + // Commit the transaction + db.commit_transaction(tx) + .unwrap() + .expect("commit transaction"); + + let count = db + .bulk_count([b"parent"].as_ref(), b"bulk", None, grove_version) + .unwrap() + .expect("count"); + assert_eq!(count, 1, "only the second batch's value should be present"); +} + +/// Multiple compaction cycles via batch + transaction, then rollback. +/// Verifies the tree fully reverts even when multiple chunks were created. +#[test] +fn test_bulk_batch_multi_compaction_transaction_rollback() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"bulk", + Element::empty_bulk_append_tree(TEST_CHUNK_POWER).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert bulk tree"); + + let tx = db.start_transaction(); + + // Append enough to trigger 2 full compaction cycles + partial buffer + // chunk_power=2: epoch_size=4, so 9 appends = 2 compactions + 1 buffered + let total_appends = TEST_CHUNK_SIZE * 2 + 1; + let ops: Vec = (0..total_appends as u8) + .map(|i| QualifiedGroveDbOp::bulk_append_op(vec![b"bulk".to_vec()], vec![i])) + .collect(); + + db.apply_batch(ops, None, Some(&tx), grove_version) + .unwrap() + .expect("batch in tx"); + + let count_in_tx = db + .bulk_count(EMPTY_PATH, b"bulk", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!(count_in_tx, total_appends as u64); + + // Drop transaction + drop(tx); + + let count_after = db + .bulk_count(EMPTY_PATH, b"bulk", None, grove_version) + .unwrap() + .expect("count after drop"); + assert_eq!( + count_after, 0, + "bulk tree should be empty after multi-compaction tx rollback" + ); +} diff --git a/grovedb/src/tests/commitment_tree_tests.rs b/grovedb/src/tests/commitment_tree_tests.rs index b476a8377..06b86bf5c 100644 --- a/grovedb/src/tests/commitment_tree_tests.rs +++ b/grovedb/src/tests/commitment_tree_tests.rs @@ -2070,3 +2070,429 @@ fn test_verify_grovedb_commitment_tree_empty() { issues ); } + +// =========================================================================== +// Batch discard / transaction rollback tests +// =========================================================================== + +/// A batch with commitment tree inserts succeeds, then a later op in the +/// batch fails. The entire batch must be discarded — count and anchor +/// should remain at pre-batch values. +#[test] +fn test_commitment_batch_discarded_on_later_op_failure() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + + db.insert( + [b"parent"].as_ref(), + b"pool", + Element::empty_commitment_tree(TEST_CHUNK_POWER).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert commitment tree"); + + // Pre-insert an item so insert_if_not_exists will fail + db.insert( + [b"parent"].as_ref(), + b"existing", + Element::new_item(b"old".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert existing item"); + + // Capture anchor before batch + let anchor_before = db + .commitment_tree_anchor([b"parent"].as_ref(), b"pool", None, grove_version) + .unwrap() + .expect("anchor before"); + + // Batch: commitment insert + conflicting insert + let ops = vec![ + QualifiedGroveDbOp::commitment_tree_insert_op_typed( + vec![b"parent".to_vec(), b"pool".to_vec()], + test_cmx(1), + test_rho(1), + &test_ciphertext(1), + ), + QualifiedGroveDbOp::insert_if_not_exists_op( + vec![b"parent".to_vec()], + b"existing".to_vec(), + Element::new_item(b"conflict".to_vec()), + ), + ]; + + // Use an external (borrowed) transaction to verify no preprocessing data + // leaks into it when the batch fails. + let tx = db.start_transaction(); + let result = db.apply_batch(ops, None, Some(&tx), grove_version).unwrap(); + assert!(result.is_err(), "batch should fail due to duplicate key"); + + // Count must remain 0 inside the transaction + let count_in_tx = db + .commitment_tree_count([b"parent"].as_ref(), b"pool", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!( + count_in_tx, 0, + "count should be 0 inside tx after batch discard" + ); + + // Anchor must not change inside the transaction + let anchor_in_tx = db + .commitment_tree_anchor([b"parent"].as_ref(), b"pool", Some(&tx), grove_version) + .unwrap() + .expect("anchor in tx"); + assert_eq!( + anchor_before, anchor_in_tx, + "anchor should not change inside tx after batch discard" + ); + + // Also verify outside the transaction + let count = db + .commitment_tree_count([b"parent"].as_ref(), b"pool", None, grove_version) + .unwrap() + .expect("count"); + assert_eq!(count, 0, "count should be 0 after batch discard"); + + let anchor_after = db + .commitment_tree_anchor([b"parent"].as_ref(), b"pool", None, grove_version) + .unwrap() + .expect("anchor after"); + assert_eq!( + anchor_before, anchor_after, + "anchor should not change after batch discard" + ); + + db.rollback_transaction(&tx).expect("rollback"); +} + +/// Commitment tree inserts inside a transaction, then the transaction is +/// rolled back. Verifies count and anchor revert. +#[test] +fn test_commitment_transaction_rollback_reverts_inserts() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"pool", + Element::empty_commitment_tree(TEST_CHUNK_POWER).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert commitment tree"); + + // Insert one value outside the transaction + db.commitment_tree_insert( + EMPTY_PATH, + b"pool", + test_cmx(1), + test_rho(1), + test_ciphertext(1), + None, + grove_version, + ) + .unwrap() + .expect("insert 1"); + + let count_before = db + .commitment_tree_count(EMPTY_PATH, b"pool", None, grove_version) + .unwrap() + .expect("count before"); + assert_eq!(count_before, 1); + + let anchor_before = db + .commitment_tree_anchor(EMPTY_PATH, b"pool", None, grove_version) + .unwrap() + .expect("anchor before"); + + // Start transaction and insert more + let tx = db.start_transaction(); + + for i in 2u8..5 { + db.commitment_tree_insert( + EMPTY_PATH, + b"pool", + test_cmx(i), + test_rho(i), + test_ciphertext(i), + Some(&tx), + grove_version, + ) + .unwrap() + .expect("insert in tx"); + } + + // Rollback + db.rollback_transaction(&tx).expect("rollback"); + + let count_after = db + .commitment_tree_count(EMPTY_PATH, b"pool", None, grove_version) + .unwrap() + .expect("count after rollback"); + assert_eq!(count_after, 1, "count should revert after rollback"); + + let anchor_after = db + .commitment_tree_anchor(EMPTY_PATH, b"pool", None, grove_version) + .unwrap() + .expect("anchor after rollback"); + assert_eq!( + anchor_before, anchor_after, + "anchor should revert after rollback" + ); +} + +/// Batch with multiple commitment tree inserts in a transaction, then +/// the transaction is dropped. Verifies the state fully reverts. +#[test] +fn test_commitment_batch_in_transaction_rollback() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"pool", + Element::empty_commitment_tree(TEST_CHUNK_POWER).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert commitment tree"); + + let tx = db.start_transaction(); + + let ops: Vec = (1u8..4) + .map(|i| { + QualifiedGroveDbOp::commitment_tree_insert_op_typed( + vec![b"pool".to_vec()], + test_cmx(i), + test_rho(i), + &test_ciphertext(i), + ) + }) + .collect(); + + db.apply_batch(ops, None, Some(&tx), grove_version) + .unwrap() + .expect("batch in tx"); + + // Visible inside tx + let count_in_tx = db + .commitment_tree_count(EMPTY_PATH, b"pool", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!(count_in_tx, 3); + + // Drop transaction (implicit rollback) + drop(tx); + + let count_after = db + .commitment_tree_count(EMPTY_PATH, b"pool", None, grove_version) + .unwrap() + .expect("count after drop"); + assert_eq!( + count_after, 0, + "commitment tree should be empty after tx drop" + ); +} + +/// After a failed batch, a subsequent successful batch on the same +/// commitment tree should work correctly — no stale frontier/cache/overlay. +#[test] +fn test_commitment_successful_batch_after_failed_batch() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + + db.insert( + [b"parent"].as_ref(), + b"pool", + Element::empty_commitment_tree(TEST_CHUNK_POWER).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert commitment tree"); + + db.insert( + [b"parent"].as_ref(), + b"existing", + Element::new_item(b"old".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert existing item"); + + // Use an external transaction — the failing batch must not leak + // preprocessing data into the tx. + let tx = db.start_transaction(); + + // First batch: commitment insert + conflicting insert → fails + let ops_fail = vec![ + QualifiedGroveDbOp::commitment_tree_insert_op_typed( + vec![b"parent".to_vec(), b"pool".to_vec()], + test_cmx(1), + test_rho(1), + &test_ciphertext(1), + ), + QualifiedGroveDbOp::insert_if_not_exists_op( + vec![b"parent".to_vec()], + b"existing".to_vec(), + Element::new_item(b"conflict".to_vec()), + ), + ]; + + let result = db + .apply_batch(ops_fail, None, Some(&tx), grove_version) + .unwrap(); + assert!(result.is_err(), "first batch should fail"); + + // Tx should be clean — no preprocessing data leaked + let count_after_fail = db + .commitment_tree_count([b"parent"].as_ref(), b"pool", Some(&tx), grove_version) + .unwrap() + .expect("count after fail"); + assert_eq!( + count_after_fail, 0, + "count in tx should be 0 after failed batch" + ); + + // Second batch: commitment insert only → should succeed in the same tx + let ops_ok = vec![QualifiedGroveDbOp::commitment_tree_insert_op_typed( + vec![b"parent".to_vec(), b"pool".to_vec()], + test_cmx(2), + test_rho(2), + &test_ciphertext(2), + )]; + + db.apply_batch(ops_ok, None, Some(&tx), grove_version) + .unwrap() + .expect("second batch should succeed"); + + // Commit the transaction + db.commit_transaction(tx) + .unwrap() + .expect("commit transaction"); + + let count = db + .commitment_tree_count([b"parent"].as_ref(), b"pool", None, grove_version) + .unwrap() + .expect("count"); + assert_eq!(count, 1, "only the second batch's insert should be present"); + + // Anchor should reflect only cmx(2), not cmx(1) + let anchor = db + .commitment_tree_anchor([b"parent"].as_ref(), b"pool", None, grove_version) + .unwrap() + .expect("anchor"); + let expected = expected_root(&[test_cmx(2)]); + assert_eq!( + anchor.to_bytes(), + expected, + "anchor should reflect only the successful batch" + ); +} + +/// Commitment tree inserts that trigger BulkAppendTree compaction within +/// a transaction, then the transaction is rolled back. Verifies that both +/// the dense buffer, MMR chunks, and Sinsemilla frontier fully revert. +#[test] +fn test_commitment_compaction_transaction_rollback() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + // Use small chunk_power=2 so compaction triggers after 4 appends + let small_chunk_power: u8 = 2; + let epoch_size = 1u64 << (small_chunk_power as u64); // 4 + + db.insert( + EMPTY_PATH, + b"pool", + Element::empty_commitment_tree(small_chunk_power).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert commitment tree"); + + let tx = db.start_transaction(); + + // Insert enough to trigger compaction: epoch_size = 4 + for i in 1u8..=(epoch_size as u8 + 1) { + db.commitment_tree_insert( + EMPTY_PATH, + b"pool", + test_cmx(i), + test_rho(i), + test_ciphertext(i), + Some(&tx), + grove_version, + ) + .unwrap() + .expect("insert in tx"); + } + + let count_in_tx = db + .commitment_tree_count(EMPTY_PATH, b"pool", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!(count_in_tx, epoch_size + 1); + + // Rollback + db.rollback_transaction(&tx).expect("rollback"); + + let count_after = db + .commitment_tree_count(EMPTY_PATH, b"pool", None, grove_version) + .unwrap() + .expect("count after rollback"); + assert_eq!( + count_after, 0, + "count should be 0 after compaction + rollback" + ); + + let anchor_after = db + .commitment_tree_anchor(EMPTY_PATH, b"pool", None, grove_version) + .unwrap() + .expect("anchor after rollback"); + // Empty tree anchor + let empty_anchor = expected_root(&[]); + assert_eq!( + anchor_after.to_bytes(), + empty_anchor, + "anchor should revert to empty tree after compaction + rollback" + ); +} diff --git a/grovedb/src/tests/dense_tree_tests.rs b/grovedb/src/tests/dense_tree_tests.rs index 2aeb905a8..14ef75de7 100644 --- a/grovedb/src/tests/dense_tree_tests.rs +++ b/grovedb/src/tests/dense_tree_tests.rs @@ -2185,3 +2185,303 @@ fn test_is_empty_tree_dense_tree_subtree() { .expect("is_empty_tree should succeed for non-empty dense tree"); assert!(!is_empty, "non-empty dense tree should report as not empty"); } + +// =========================================================================== +// Batch discard / transaction rollback tests +// =========================================================================== + +/// A batch with dense tree inserts succeeds, then a later op in the batch +/// fails. The entire batch must be discarded — the dense tree count should +/// remain at its pre-batch value. +#[test] +fn test_dense_batch_discarded_on_later_op_failure() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + // Create a parent tree with a dense tree and a regular item inside it + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + + db.insert( + [b"parent"].as_ref(), + b"dense", + Element::empty_dense_tree(4), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert dense tree"); + + // Pre-insert an item so insert_if_not_exists will fail + db.insert( + [b"parent"].as_ref(), + b"existing", + Element::new_item(b"old".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert existing item"); + + // Batch: dense insert (should succeed internally) + insert_if_not_exists + // on an existing key (will fail, discarding the entire batch) + let ops = vec![ + QualifiedGroveDbOp::dense_tree_insert_op( + vec![b"parent".to_vec(), b"dense".to_vec()], + b"value_a".to_vec(), + ), + QualifiedGroveDbOp::insert_if_not_exists_op( + vec![b"parent".to_vec()], + b"existing".to_vec(), + Element::new_item(b"conflict".to_vec()), + ), + ]; + + // Use an external (borrowed) transaction to verify no preprocessing data + // leaks into it when the batch fails. + let tx = db.start_transaction(); + let result = db.apply_batch(ops, None, Some(&tx), grove_version).unwrap(); + assert!(result.is_err(), "batch should fail due to duplicate key"); + + // Dense tree count must remain 0 inside the transaction — no preprocessing + // data should have leaked into it. + let count_in_tx = db + .dense_tree_count([b"parent"].as_ref(), b"dense", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!( + count_in_tx, 0, + "dense tree count should be 0 inside tx after batch discard" + ); + + // Also verify outside the transaction + let count = db + .dense_tree_count([b"parent"].as_ref(), b"dense", None, grove_version) + .unwrap() + .expect("count"); + assert_eq!(count, 0, "dense tree count should be 0 after batch discard"); + + db.rollback_transaction(&tx).expect("rollback"); +} + +/// Dense tree inserts inside a transaction, then the transaction is rolled +/// back. Verifies the count reverts to pre-transaction value. +#[test] +fn test_dense_transaction_rollback_reverts_inserts() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"dense", + Element::empty_dense_tree(4), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert dense tree"); + + // Insert one value outside the transaction + db.dense_tree_insert(EMPTY_PATH, b"dense", b"v0".to_vec(), None, grove_version) + .unwrap() + .expect("insert v0"); + + let count_before = db + .dense_tree_count(EMPTY_PATH, b"dense", None, grove_version) + .unwrap() + .expect("count before"); + assert_eq!(count_before, 1); + + // Start transaction and insert more values + let tx = db.start_transaction(); + + for i in 1u8..4 { + db.dense_tree_insert( + EMPTY_PATH, + b"dense", + format!("v{}", i).into_bytes(), + Some(&tx), + grove_version, + ) + .unwrap() + .expect("insert in tx"); + } + + // Verify count is updated inside tx + let count_in_tx = db + .dense_tree_count(EMPTY_PATH, b"dense", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!(count_in_tx, 4, "should see 4 values inside transaction"); + + // Rollback + db.rollback_transaction(&tx).expect("rollback"); + + // Count should revert to 1 + let count_after = db + .dense_tree_count(EMPTY_PATH, b"dense", None, grove_version) + .unwrap() + .expect("count after rollback"); + assert_eq!( + count_after, 1, + "dense tree count should revert after rollback" + ); +} + +/// Batch with multiple dense tree inserts is applied in a transaction, then +/// the transaction is rolled back. Verifies both the element metadata and +/// the stored values revert. +#[test] +fn test_dense_batch_in_transaction_rollback() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"dense", + Element::empty_dense_tree(4), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert dense tree"); + + let tx = db.start_transaction(); + + let ops: Vec = (0u8..3) + .map(|i| { + QualifiedGroveDbOp::dense_tree_insert_op( + vec![b"dense".to_vec()], + format!("val_{}", i).into_bytes(), + ) + }) + .collect(); + + db.apply_batch(ops, None, Some(&tx), grove_version) + .unwrap() + .expect("batch in tx"); + + // Visible inside tx + let count_in_tx = db + .dense_tree_count(EMPTY_PATH, b"dense", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!(count_in_tx, 3); + + // Drop transaction (implicit rollback) + drop(tx); + + // Count should be 0 outside tx + let count_after = db + .dense_tree_count(EMPTY_PATH, b"dense", None, grove_version) + .unwrap() + .expect("count after drop"); + assert_eq!(count_after, 0, "dense tree should be empty after tx drop"); +} + +/// After a failed batch, a subsequent successful batch on the same dense +/// tree should work correctly — verifying no stale cache state leaks. +#[test] +fn test_dense_successful_batch_after_failed_batch() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + + db.insert( + [b"parent"].as_ref(), + b"dense", + Element::empty_dense_tree(4), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert dense tree"); + + db.insert( + [b"parent"].as_ref(), + b"existing", + Element::new_item(b"old".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert existing item"); + + // Use an external transaction — the failing batch must not leak + // preprocessing data into the tx, and a subsequent success batch in the + // same tx must work cleanly. + let tx = db.start_transaction(); + + // First batch: dense insert + conflicting insert → fails + let ops_fail = vec![ + QualifiedGroveDbOp::dense_tree_insert_op( + vec![b"parent".to_vec(), b"dense".to_vec()], + b"ghost_value".to_vec(), + ), + QualifiedGroveDbOp::insert_if_not_exists_op( + vec![b"parent".to_vec()], + b"existing".to_vec(), + Element::new_item(b"conflict".to_vec()), + ), + ]; + + let result = db + .apply_batch(ops_fail, None, Some(&tx), grove_version) + .unwrap(); + assert!(result.is_err(), "first batch should fail"); + + // Tx should be clean — no preprocessing data leaked + let count_after_fail = db + .dense_tree_count([b"parent"].as_ref(), b"dense", Some(&tx), grove_version) + .unwrap() + .expect("count after fail"); + assert_eq!( + count_after_fail, 0, + "dense tree count in tx should be 0 after failed batch" + ); + + // Second batch: dense insert only → should succeed in the same tx + let ops_ok = vec![QualifiedGroveDbOp::dense_tree_insert_op( + vec![b"parent".to_vec(), b"dense".to_vec()], + b"real_value".to_vec(), + )]; + + db.apply_batch(ops_ok, None, Some(&tx), grove_version) + .unwrap() + .expect("second batch should succeed"); + + // Commit the transaction + db.commit_transaction(tx) + .unwrap() + .expect("commit transaction"); + + let count = db + .dense_tree_count([b"parent"].as_ref(), b"dense", None, grove_version) + .unwrap() + .expect("count"); + assert_eq!(count, 1, "only the second batch's value should be present"); +} diff --git a/grovedb/src/tests/mmr_tree_tests.rs b/grovedb/src/tests/mmr_tree_tests.rs index 57369530f..12dc83e0d 100644 --- a/grovedb/src/tests/mmr_tree_tests.rs +++ b/grovedb/src/tests/mmr_tree_tests.rs @@ -1672,3 +1672,297 @@ fn test_verify_grovedb_mmr_tree_empty() { issues ); } + +// =========================================================================== +// Batch discard / transaction rollback tests +// =========================================================================== + +/// A batch with MMR tree appends succeeds, then a later op in the batch +/// fails. The entire batch must be discarded — the MMR leaf count should +/// remain at its pre-batch value. +#[test] +fn test_mmr_batch_discarded_on_later_op_failure() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + + db.insert( + [b"parent"].as_ref(), + b"mmr", + Element::empty_mmr_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert mmr tree"); + + // Pre-insert an item so insert_if_not_exists will fail + db.insert( + [b"parent"].as_ref(), + b"existing", + Element::new_item(b"old".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert existing item"); + + // Batch: mmr append (should succeed internally) + conflicting insert + let ops = vec![ + QualifiedGroveDbOp::mmr_tree_append_op( + vec![b"parent".to_vec(), b"mmr".to_vec()], + b"leaf_data".to_vec(), + ), + QualifiedGroveDbOp::insert_if_not_exists_op( + vec![b"parent".to_vec()], + b"existing".to_vec(), + Element::new_item(b"conflict".to_vec()), + ), + ]; + + // Use an external (borrowed) transaction to verify no preprocessing data + // leaks into it when the batch fails. + let tx = db.start_transaction(); + let result = db.apply_batch(ops, None, Some(&tx), grove_version).unwrap(); + assert!(result.is_err(), "batch should fail due to duplicate key"); + + // MMR leaf count must remain 0 inside the transaction + let leaf_count_in_tx = db + .mmr_tree_leaf_count([b"parent"].as_ref(), b"mmr", Some(&tx), grove_version) + .unwrap() + .expect("leaf count in tx"); + assert_eq!( + leaf_count_in_tx, 0, + "mmr leaf count should be 0 inside tx after batch discard" + ); + + // Also verify outside the transaction + let leaf_count = db + .mmr_tree_leaf_count([b"parent"].as_ref(), b"mmr", None, grove_version) + .unwrap() + .expect("leaf count"); + assert_eq!( + leaf_count, 0, + "mmr leaf count should be 0 after batch discard" + ); + + db.rollback_transaction(&tx).expect("rollback"); +} + +/// MMR tree appends inside a transaction, then the transaction is rolled +/// back. Verifies the leaf count reverts. +#[test] +fn test_mmr_transaction_rollback_reverts_appends() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"mmr", + Element::empty_mmr_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert mmr tree"); + + // Append one value outside the transaction + db.mmr_tree_append(EMPTY_PATH, b"mmr", b"leaf_0".to_vec(), None, grove_version) + .unwrap() + .expect("append leaf_0"); + + let count_before = db + .mmr_tree_leaf_count(EMPTY_PATH, b"mmr", None, grove_version) + .unwrap() + .expect("count before"); + assert_eq!(count_before, 1); + + // Start transaction and append more + let tx = db.start_transaction(); + + for i in 1u8..4 { + db.mmr_tree_append( + EMPTY_PATH, + b"mmr", + format!("leaf_{}", i).into_bytes(), + Some(&tx), + grove_version, + ) + .unwrap() + .expect("append in tx"); + } + + let count_in_tx = db + .mmr_tree_leaf_count(EMPTY_PATH, b"mmr", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!(count_in_tx, 4, "should see 4 leaves inside transaction"); + + // Rollback + db.rollback_transaction(&tx).expect("rollback"); + + let count_after = db + .mmr_tree_leaf_count(EMPTY_PATH, b"mmr", None, grove_version) + .unwrap() + .expect("count after rollback"); + assert_eq!( + count_after, 1, + "mmr leaf count should revert after rollback" + ); +} + +/// Batch with multiple MMR appends in a transaction, then transaction is +/// dropped. Verifies the leaf count reverts. +#[test] +fn test_mmr_batch_in_transaction_rollback() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"mmr", + Element::empty_mmr_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert mmr tree"); + + let tx = db.start_transaction(); + + let ops: Vec = (0u8..5) + .map(|i| { + QualifiedGroveDbOp::mmr_tree_append_op( + vec![b"mmr".to_vec()], + format!("leaf_{}", i).into_bytes(), + ) + }) + .collect(); + + db.apply_batch(ops, None, Some(&tx), grove_version) + .unwrap() + .expect("batch in tx"); + + let count_in_tx = db + .mmr_tree_leaf_count(EMPTY_PATH, b"mmr", Some(&tx), grove_version) + .unwrap() + .expect("count in tx"); + assert_eq!(count_in_tx, 5); + + // Drop transaction (implicit rollback) + drop(tx); + + let count_after = db + .mmr_tree_leaf_count(EMPTY_PATH, b"mmr", None, grove_version) + .unwrap() + .expect("count after drop"); + assert_eq!(count_after, 0, "mmr should be empty after tx drop"); +} + +/// After a failed batch, a subsequent successful batch on the same MMR +/// tree should work correctly — no stale overlay state leaks. +#[test] +fn test_mmr_successful_batch_after_failed_batch() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + + db.insert( + EMPTY_PATH, + b"parent", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert parent"); + + db.insert( + [b"parent"].as_ref(), + b"mmr", + Element::empty_mmr_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert mmr tree"); + + db.insert( + [b"parent"].as_ref(), + b"existing", + Element::new_item(b"old".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert existing item"); + + // Use an external transaction — the failing batch must not leak + // preprocessing data into the tx. + let tx = db.start_transaction(); + + // First batch: mmr append + conflicting insert → fails + let ops_fail = vec![ + QualifiedGroveDbOp::mmr_tree_append_op( + vec![b"parent".to_vec(), b"mmr".to_vec()], + b"ghost_leaf".to_vec(), + ), + QualifiedGroveDbOp::insert_if_not_exists_op( + vec![b"parent".to_vec()], + b"existing".to_vec(), + Element::new_item(b"conflict".to_vec()), + ), + ]; + + let result = db + .apply_batch(ops_fail, None, Some(&tx), grove_version) + .unwrap(); + assert!(result.is_err(), "first batch should fail"); + + // Tx should be clean — no preprocessing data leaked + let count_after_fail = db + .mmr_tree_leaf_count([b"parent"].as_ref(), b"mmr", Some(&tx), grove_version) + .unwrap() + .expect("count after fail"); + assert_eq!( + count_after_fail, 0, + "mmr leaf count in tx should be 0 after failed batch" + ); + + // Second batch: mmr append only → should succeed in the same tx + let ops_ok = vec![QualifiedGroveDbOp::mmr_tree_append_op( + vec![b"parent".to_vec(), b"mmr".to_vec()], + b"real_leaf".to_vec(), + )]; + + db.apply_batch(ops_ok, None, Some(&tx), grove_version) + .unwrap() + .expect("second batch should succeed"); + + // Commit the transaction + db.commit_transaction(tx) + .unwrap() + .expect("commit transaction"); + + let count = db + .mmr_tree_leaf_count([b"parent"].as_ref(), b"mmr", None, grove_version) + .unwrap() + .expect("leaf count"); + assert_eq!(count, 1, "only the second batch's leaf should be present"); +} From 58a36f054fb98fb665d3afbd03c179ce522e3cb9 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 10 Mar 2026 13:46:22 +0700 Subject: [PATCH 5/9] test: add raw subtree storage assertions to batch discard tests The count-based checks (dense_tree_count, mmr_tree_leaf_count, bulk_count) only read the parent element which is properly discarded by the batch. They don't detect leaked preprocessing data in the actual subtree storage. Add raw_subtree_get() test helper that reads directly from subtree storage, bypassing the parent element. Use it in dense, MMR, and bulk append batch discard tests to verify no data leaks into the transaction at position 0. Verified: all 7 dense/MMR/bulk tests now fail without the preprocessing atomicity fix (previously they passed despite data leakage). Combined with the 2 commitment tree tests that already caught the bug via anchor checks, all 9 tests properly detect the atomicity issue. Co-Authored-By: Claude Opus 4.6 --- grovedb/src/lib.rs | 27 ++++++++++++ grovedb/src/tests/bulk_append_tree_tests.rs | 46 +++++++++++++++++++++ grovedb/src/tests/dense_tree_tests.rs | 25 +++++++++++ grovedb/src/tests/mmr_tree_tests.rs | 25 +++++++++++ 4 files changed, 123 insertions(+) diff --git a/grovedb/src/lib.rs b/grovedb/src/lib.rs index 972ebc1e6..7c2148e15 100644 --- a/grovedb/src/lib.rs +++ b/grovedb/src/lib.rs @@ -1194,3 +1194,30 @@ impl GroveDb { } } } + +/// Test-only helpers for verifying internal storage state. +#[cfg(all(test, feature = "minimal"))] +impl GroveDb { + /// Read a raw key from a subtree's transactional storage context. + /// + /// This bypasses all element-level checks (count, type) and reads directly + /// from the subtree's storage. Used in tests to verify that no data leaked + /// into the transaction after a failed batch. + pub(crate) fn raw_subtree_get<'b, B: AsRef<[u8]> + 'b>( + &self, + path: SubtreePath<'b, B>, + key: &[u8], + transaction: &Transaction, + ) -> Result>, Error> { + let storage_ctx = self + .db + .get_transactional_storage_context(path, None, transaction) + .value; + + let result = storage_ctx.get(key).value; + match result { + Ok(opt) => Ok(opt.map(|v| v.to_vec())), + Err(e) => Err(e.into()), + } + } +} diff --git a/grovedb/src/tests/bulk_append_tree_tests.rs b/grovedb/src/tests/bulk_append_tree_tests.rs index 1e78d62e9..c17739017 100644 --- a/grovedb/src/tests/bulk_append_tree_tests.rs +++ b/grovedb/src/tests/bulk_append_tree_tests.rs @@ -1789,6 +1789,19 @@ fn test_bulk_batch_discarded_on_later_op_failure() { "bulk count should be 0 inside tx after batch discard" ); + // Verify no raw subtree data leaked into the transaction. The bulk append + // tree's dense buffer uses 2-byte position keys; position 0 → [0, 0]. + use grovedb_dense_fixed_sized_merkle_tree::position_key; + let subtree_path: Vec> = vec![b"parent".to_vec(), b"bulk".to_vec()]; + let subtree_refs: Vec<&[u8]> = subtree_path.iter().map(|v| v.as_slice()).collect(); + let raw_pos0 = db + .raw_subtree_get(subtree_refs.as_slice().into(), &position_key(0), &tx) + .expect("raw get should not error"); + assert!( + raw_pos0.is_none(), + "raw bulk subtree storage at position 0 should be empty inside tx after batch discard" + ); + // Also verify outside the transaction let count = db .bulk_count([b"parent"].as_ref(), b"bulk", None, grove_version) @@ -1872,6 +1885,27 @@ fn test_bulk_batch_discarded_after_compaction() { "bulk count should be 0 inside tx after batch discard (even after compaction)" ); + // Verify no raw subtree data leaked — check both dense buffer (position 0) + // and MMR data (MSB-tagged position 0) since compaction was triggered. + use grovedb_dense_fixed_sized_merkle_tree::position_key; + let subtree_path: Vec> = vec![b"parent".to_vec(), b"bulk".to_vec()]; + let subtree_refs: Vec<&[u8]> = subtree_path.iter().map(|v| v.as_slice()).collect(); + let raw_dense_pos0 = db + .raw_subtree_get(subtree_refs.as_slice().into(), &position_key(0), &tx) + .expect("raw dense get should not error"); + assert!( + raw_dense_pos0.is_none(), + "raw dense buffer at position 0 should be empty inside tx after compaction discard" + ); + let mmr_key_pos0 = 0x8000_0000_0000_0000u64.to_be_bytes(); + let raw_mmr_pos0 = db + .raw_subtree_get(subtree_refs.as_slice().into(), &mmr_key_pos0, &tx) + .expect("raw mmr get should not error"); + assert!( + raw_mmr_pos0.is_none(), + "raw MMR data at position 0 should be empty inside tx after compaction discard" + ); + // Also verify outside the transaction let count = db .bulk_count([b"parent"].as_ref(), b"bulk", None, grove_version) @@ -2006,6 +2040,18 @@ fn test_bulk_successful_batch_after_failed_batch() { "bulk count in tx should be 0 after failed batch" ); + // Verify no raw subtree data leaked into the transaction + use grovedb_dense_fixed_sized_merkle_tree::position_key; + let subtree_path: Vec> = vec![b"parent".to_vec(), b"bulk".to_vec()]; + let subtree_refs: Vec<&[u8]> = subtree_path.iter().map(|v| v.as_slice()).collect(); + let raw_pos0 = db + .raw_subtree_get(subtree_refs.as_slice().into(), &position_key(0), &tx) + .expect("raw get should not error"); + assert!( + raw_pos0.is_none(), + "raw bulk subtree storage at position 0 should be empty inside tx after failed batch" + ); + // Second batch: bulk append only → should succeed in the same tx let ops_ok = vec![QualifiedGroveDbOp::bulk_append_op( vec![b"parent".to_vec(), b"bulk".to_vec()], diff --git a/grovedb/src/tests/dense_tree_tests.rs b/grovedb/src/tests/dense_tree_tests.rs index 14ef75de7..86967d23f 100644 --- a/grovedb/src/tests/dense_tree_tests.rs +++ b/grovedb/src/tests/dense_tree_tests.rs @@ -2264,6 +2264,19 @@ fn test_dense_batch_discarded_on_later_op_failure() { "dense tree count should be 0 inside tx after batch discard" ); + // Verify no raw subtree data leaked into the transaction. The count check + // above only reads the parent element; this reads the actual subtree storage. + use grovedb_dense_fixed_sized_merkle_tree::position_key; + let subtree_path: Vec> = vec![b"parent".to_vec(), b"dense".to_vec()]; + let subtree_refs: Vec<&[u8]> = subtree_path.iter().map(|v| v.as_slice()).collect(); + let raw_pos0 = db + .raw_subtree_get(subtree_refs.as_slice().into(), &position_key(0), &tx) + .expect("raw get should not error"); + assert!( + raw_pos0.is_none(), + "raw subtree storage at position 0 should be empty inside tx after batch discard" + ); + // Also verify outside the transaction let count = db .dense_tree_count([b"parent"].as_ref(), b"dense", None, grove_version) @@ -2464,6 +2477,18 @@ fn test_dense_successful_batch_after_failed_batch() { "dense tree count in tx should be 0 after failed batch" ); + // Verify no raw subtree data leaked into the transaction + use grovedb_dense_fixed_sized_merkle_tree::position_key; + let subtree_path: Vec> = vec![b"parent".to_vec(), b"dense".to_vec()]; + let subtree_refs: Vec<&[u8]> = subtree_path.iter().map(|v| v.as_slice()).collect(); + let raw_pos0 = db + .raw_subtree_get(subtree_refs.as_slice().into(), &position_key(0), &tx) + .expect("raw get should not error"); + assert!( + raw_pos0.is_none(), + "raw subtree storage at position 0 should be empty inside tx after failed batch" + ); + // Second batch: dense insert only → should succeed in the same tx let ops_ok = vec![QualifiedGroveDbOp::dense_tree_insert_op( vec![b"parent".to_vec(), b"dense".to_vec()], diff --git a/grovedb/src/tests/mmr_tree_tests.rs b/grovedb/src/tests/mmr_tree_tests.rs index 12dc83e0d..9a1fe32c6 100644 --- a/grovedb/src/tests/mmr_tree_tests.rs +++ b/grovedb/src/tests/mmr_tree_tests.rs @@ -1748,6 +1748,19 @@ fn test_mmr_batch_discarded_on_later_op_failure() { "mmr leaf count should be 0 inside tx after batch discard" ); + // Verify no raw subtree data leaked into the transaction. MMR stores nodes + // at 8-byte MSB-tagged position keys; position 0 → 0x80_00_00_00_00_00_00_00. + let mmr_key_pos0 = 0x8000_0000_0000_0000u64.to_be_bytes(); + let subtree_path: Vec> = vec![b"parent".to_vec(), b"mmr".to_vec()]; + let subtree_refs: Vec<&[u8]> = subtree_path.iter().map(|v| v.as_slice()).collect(); + let raw_pos0 = db + .raw_subtree_get(subtree_refs.as_slice().into(), &mmr_key_pos0, &tx) + .expect("raw get should not error"); + assert!( + raw_pos0.is_none(), + "raw MMR subtree storage at position 0 should be empty inside tx after batch discard" + ); + // Also verify outside the transaction let leaf_count = db .mmr_tree_leaf_count([b"parent"].as_ref(), b"mmr", None, grove_version) @@ -1945,6 +1958,18 @@ fn test_mmr_successful_batch_after_failed_batch() { "mmr leaf count in tx should be 0 after failed batch" ); + // Verify no raw subtree data leaked into the transaction + let mmr_key_pos0 = 0x8000_0000_0000_0000u64.to_be_bytes(); + let subtree_path: Vec> = vec![b"parent".to_vec(), b"mmr".to_vec()]; + let subtree_refs: Vec<&[u8]> = subtree_path.iter().map(|v| v.as_slice()).collect(); + let raw_pos0 = db + .raw_subtree_get(subtree_refs.as_slice().into(), &mmr_key_pos0, &tx) + .expect("raw get should not error"); + assert!( + raw_pos0.is_none(), + "raw MMR subtree storage at position 0 should be empty inside tx after failed batch" + ); + // Second batch: mmr append only → should succeed in the same tx let ops_ok = vec![QualifiedGroveDbOp::mmr_tree_append_op( vec![b"parent".to_vec(), b"mmr".to_vec()], From 8fd82761ce05ce93161251f0952f50ed0ff10a19 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 10 Mar 2026 13:53:17 +0700 Subject: [PATCH 6/9] fix: restore MMR overlay on commit_mmr failure, gate cache field - commit_mmr(): restore mmr_overlay via take_overlay() if mmr.commit() fails, matching the pattern already used in compact_with_value(). Without this, a failed commit permanently loses staged MMR nodes. - DenseFixedSizedMerkleTree: gate cache field with #[cfg(feature = "storage")] to suppress "field is never read" warning when compiled without storage feature. All cache usage is already inside the storage-gated impl block. Co-Authored-By: Claude Opus 4.6 --- grovedb-bulk-append-tree/src/tree/append.rs | 12 +++++++++--- grovedb-dense-fixed-sized-merkle-tree/src/tree.rs | 2 ++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/grovedb-bulk-append-tree/src/tree/append.rs b/grovedb-bulk-append-tree/src/tree/append.rs index 9096b5f26..5b7e56626 100644 --- a/grovedb-bulk-append-tree/src/tree/append.rs +++ b/grovedb-bulk-append-tree/src/tree/append.rs @@ -212,9 +212,15 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { &mmr_store, std::mem::take(&mut self.mmr_overlay), ); - mmr.commit() - .unwrap() - .map_err(|e| BulkAppendError::MmrError(format!("MMR commit failed: {}", e)))?; + if let Err(e) = mmr.commit().unwrap() { + // Restore overlay before returning error so retries/get_mmr_root + // still see the staged nodes. + self.mmr_overlay = mmr.batch.take_overlay(); + return Err(BulkAppendError::MmrError(format!( + "MMR commit failed: {}", + e + ))); + } Ok(()) } } diff --git a/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs b/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs index 5484c0412..905e74088 100644 --- a/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs +++ b/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs @@ -49,6 +49,8 @@ pub struct DenseFixedSizedMerkleTree { /// Write-through cache: holds values written in this session. /// Indexed by position. `None` means the value has not been written /// in this session (fall back to storage). + /// Only compiled when storage-dependent operations are available. + #[cfg(feature = "storage")] cache: Vec>>, } From 1dc465bb218fc4a302fdcb0c7d2447c8632c29b6 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 10 Mar 2026 13:55:46 +0700 Subject: [PATCH 7/9] docs: document direct-path atomicity limitation in specialized tree ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each direct API function (dense_tree_insert, mmr_tree_append, bulk_append, commitment_tree_insert) commits subtree data to the transaction before the parent Merk element update. If the Merk update fails, subtree data is orphaned in the transaction. This is the same pattern as other direct GroveDB operations — callers are expected to rollback the transaction on error. The batch path (preprocess_*_ops) avoids this via a shared StorageBatch. Co-Authored-By: Claude Opus 4.6 --- grovedb/src/operations/bulk_append_tree.rs | 8 +++++++- grovedb/src/operations/commitment_tree.rs | 8 +++++++- grovedb/src/operations/dense_tree.rs | 8 +++++++- grovedb/src/operations/mmr_tree.rs | 8 +++++++- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/grovedb/src/operations/bulk_append_tree.rs b/grovedb/src/operations/bulk_append_tree.rs index 534963ef6..88c967985 100644 --- a/grovedb/src/operations/bulk_append_tree.rs +++ b/grovedb/src/operations/bulk_append_tree.rs @@ -96,7 +96,13 @@ impl GroveDb { // Drop tree (and its embedded storage context) before opening merk drop(tree); - // Commit data batch to make writes visible in the transaction + // Commit data batch to make writes visible in the transaction. + // Note: this commits subtree data before the parent element update + // below. If the parent Merk update fails, the subtree data is orphaned + // in the transaction. This is the same pattern as other direct GroveDB + // operations — the caller is expected to rollback the tx on error. + // The batch path (preprocess_bulk_append_ops) avoids this by using a + // shared StorageBatch that commits atomically with all other ops. cost_return_on_error!( &mut cost, self.db diff --git a/grovedb/src/operations/commitment_tree.rs b/grovedb/src/operations/commitment_tree.rs index 03528325c..92ecf446d 100644 --- a/grovedb/src/operations/commitment_tree.rs +++ b/grovedb/src/operations/commitment_tree.rs @@ -164,7 +164,13 @@ impl GroveDb { // Drop ct (and its storage context) before opening merk drop(ct); - // Commit data batch to make writes visible in the transaction + // Commit data batch to make writes visible in the transaction. + // Note: this commits subtree data before the parent element update + // below. If the parent Merk update fails, the subtree data is orphaned + // in the transaction. This is the same pattern as other direct GroveDB + // operations — the caller is expected to rollback the tx on error. + // The batch path (preprocess_commitment_tree_ops) avoids this by using + // a shared StorageBatch that commits atomically with all other ops. cost_return_on_error!( &mut cost, self.db diff --git a/grovedb/src/operations/dense_tree.rs b/grovedb/src/operations/dense_tree.rs index 8fe28c65e..f0e6f3135 100644 --- a/grovedb/src/operations/dense_tree.rs +++ b/grovedb/src/operations/dense_tree.rs @@ -93,7 +93,13 @@ impl GroveDb { // Drop tree (and its embedded storage context) before committing drop(tree); - // Commit data writes to the transaction + // Commit data writes to the transaction. + // Note: this commits subtree data before the parent element update + // below. If the parent Merk update fails, the subtree data is orphaned + // in the transaction. This is the same pattern as other direct GroveDB + // operations — the caller is expected to rollback the tx on error. + // The batch path (preprocess_dense_tree_ops) avoids this by using a + // shared StorageBatch that commits atomically with all other ops. cost_return_on_error!( &mut cost, self.db diff --git a/grovedb/src/operations/mmr_tree.rs b/grovedb/src/operations/mmr_tree.rs index 96cb9cfed..243bbb10d 100644 --- a/grovedb/src/operations/mmr_tree.rs +++ b/grovedb/src/operations/mmr_tree.rs @@ -110,7 +110,13 @@ impl GroveDb { #[allow(clippy::drop_non_drop)] drop(storage_ctx); - // Commit data writes to the transaction + // Commit data writes to the transaction. + // Note: this commits subtree data before the parent element update + // below. If the parent Merk update fails, the subtree data is orphaned + // in the transaction. This is the same pattern as other direct GroveDB + // operations — the caller is expected to rollback the tx on error. + // The batch path (preprocess_mmr_tree_ops) avoids this by using a + // shared StorageBatch that commits atomically with all other ops. cost_return_on_error!( &mut cost, self.db From 77ea7956bf077e8356aeab5207c5be07cc77b018 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 10 Mar 2026 14:02:18 +0700 Subject: [PATCH 8/9] docs: fix misleading doc comment on commitment_tree_insert_raw The doc comment incorrectly stated this function was "used by batch preprocessing". In reality, batch preprocessing (preprocess_commitment_tree_ops) performs its own inline processing with a shared StorageBatch and never calls this function. This function is only used by the direct API and apply_operations_without_batching. Co-Authored-By: Claude Opus 4.6 --- docs/book/src/commitment-tree.md | 6 ++++-- grovedb/src/operations/commitment_tree.rs | 9 +++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/book/src/commitment-tree.md b/docs/book/src/commitment-tree.md index fe46b49b5..6756218e2 100644 --- a/docs/book/src/commitment-tree.md +++ b/docs/book/src/commitment-tree.md @@ -330,8 +330,10 @@ db.commitment_tree_count(path, key, tx, version) The typed `commitment_tree_insert` accepts a `TransmittedNoteCiphertext` and serializes it internally. The raw `commitment_tree_insert_raw` (pub(crate)) -accepts `Vec` and is used by batch preprocessing where payloads are already -serialized. +accepts `Vec` and is used by the direct (non-batch) API and +`apply_operations_without_batching`. Batch preprocessing performs its own +inline processing using a shared `StorageBatch` and does not call +`commitment_tree_insert_raw`. ### commitment_tree_insert diff --git a/grovedb/src/operations/commitment_tree.rs b/grovedb/src/operations/commitment_tree.rs index 92ecf446d..4f98e32cd 100644 --- a/grovedb/src/operations/commitment_tree.rs +++ b/grovedb/src/operations/commitment_tree.rs @@ -80,8 +80,13 @@ impl GroveDb { /// Insert a note commitment into a CommitmentTree subtree using raw payload /// bytes. /// - /// This is the raw write operation used by batch preprocessing. The payload - /// is validated against `DashMemo`'s expected size by `append_raw`. + /// This is the raw write operation used by the direct (non-batch) API and + /// by `apply_operations_without_batching`. Batch preprocessing does NOT + /// call this function — it performs its own inline processing using a + /// shared `StorageBatch` (see `preprocess_commitment_tree_ops`). + /// + /// The payload is validated against `DashMemo`'s expected size by + /// `append_raw`. pub(crate) fn commitment_tree_insert_raw<'b, B, P>( &self, path: P, From b6ce60b1595f0136e72560570c5b657b194d644d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Tue, 10 Mar 2026 14:12:42 +0700 Subject: [PATCH 9/9] fix: use U32 MMR key in bulk_append compaction discard test BulkAppendTree compaction writes MMR nodes via MmrKeySize::U32 (4-byte tagged keys), not U64 (8-byte). The test was probing with an 8-byte key that would never match, causing a false pass. Use the correct 4-byte key format: 0x8000_0000u32.to_be_bytes(). Co-Authored-By: Claude Opus 4.6 --- grovedb/src/tests/bulk_append_tree_tests.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/grovedb/src/tests/bulk_append_tree_tests.rs b/grovedb/src/tests/bulk_append_tree_tests.rs index c17739017..7028e8e0d 100644 --- a/grovedb/src/tests/bulk_append_tree_tests.rs +++ b/grovedb/src/tests/bulk_append_tree_tests.rs @@ -1897,7 +1897,9 @@ fn test_bulk_batch_discarded_after_compaction() { raw_dense_pos0.is_none(), "raw dense buffer at position 0 should be empty inside tx after compaction discard" ); - let mmr_key_pos0 = 0x8000_0000_0000_0000u64.to_be_bytes(); + // BulkAppendTree compaction uses MmrKeySize::U32 (4-byte tagged keys), + // not U64 (8-byte). Position 0 with U32 tag: 0x8000_0000. + let mmr_key_pos0 = 0x8000_0000u32.to_be_bytes(); let raw_mmr_pos0 = db .raw_subtree_get(subtree_refs.as_slice().into(), &mmr_key_pos0, &tx) .expect("raw mmr get should not error");