diff --git a/docs/crates/costs.md b/docs/crates/costs.md index d2d4c7dcc..40c5dbedb 100644 --- a/docs/crates/costs.md +++ b/docs/crates/costs.md @@ -144,13 +144,16 @@ over an epoch matches the database growth. `removed_bytes` stays `NoStorageRemoval` in both — a rolling buffer refunds nobody. Stored bytes, roots and proofs are identical under both versions. -Mechanically: the dense tree's `SlotWriteAccounting::AgainstCommitted` -reads the slot's committed value before the write (the read is billed) and -attaches `KeyValueStorageCost::for_in_place_value_rewrite`; the MMR +Mechanically: `BulkAppendTree` reads the committed value of a slot that +already holds one (judged against the count at open; never this session's +cache) before rewriting it — one billed seek plus the value's bytes — and +asks the dense tree for `SlotWriteAccounting::Overwrite { previous_value_len }`, +which attaches `KeyValueStorageCost::for_in_place_value_rewrite`; the MMR `MmrStore` takes a `LeafValueStorageCost::PartlyPrepaid` policy that the -bulk tree feeds `chunk_blob_entry_bytes`; `BulkAppendTree` appends report -`prepaid_chunk_bytes` for the caller to bill (already included in the cost -of the `CostResult`-returning `append_deferred_roots`). +bulk tree feeds `chunk_blob_entry_bytes`; the `Result`-returning appends +report a `storage_accounting_cost` (the prepaid share plus the slot read) +for the caller to bill, while the `CostResult`-returning +`append_deferred_roots` already includes it in its cost. ## Cost Context diff --git a/grovedb-bulk-append-tree/src/cost/mod.rs b/grovedb-bulk-append-tree/src/cost/mod.rs index 180e2e536..298c1c7b9 100644 --- a/grovedb-bulk-append-tree/src/cost/mod.rs +++ b/grovedb-bulk-append-tree/src/cost/mod.rs @@ -22,21 +22,31 @@ mod v0; mod v1; -#[cfg(feature = "storage")] -use grovedb_dense_fixed_sized_merkle_tree::SlotWriteAccounting; #[cfg(feature = "storage")] use grovedb_merkle_mountain_range::LeafValueStorageCost; use grovedb_version::{error::GroveVersionError, version::GroveVersion}; use crate::BulkAppendError; +/// How the dense-buffer slot write is sized for the storage cost layer. +#[cfg(feature = "storage")] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum SlotRewriteAccounting { + /// Every slot write is new storage (no cost information on the put). + AsNew, + /// A slot that holds a committed value is read first (the read is + /// billed) and the write is reported as its replacement; a slot written + /// for the first time stays new storage. + AgainstCommitted, +} + /// How an append's data-storage writes are reported to the storage cost /// layer. Selected per grove version by [`append_storage_accounting`]. #[cfg(feature = "storage")] #[derive(Clone, Copy)] pub(crate) struct AppendStorageAccounting { - /// How the dense-buffer slot write is reported. - pub slot_write: SlotWriteAccounting, + /// How the dense-buffer slot write is sized. + pub slot_rewrite: SlotRewriteAccounting, /// How the chunk-blob leaf is reported when the MMR overlay is flushed. pub chunk_leaf: LeafValueStorageCost, /// Whether the entry's chunk-blob share is charged as added storage at diff --git a/grovedb-bulk-append-tree/src/cost/v0.rs b/grovedb-bulk-append-tree/src/cost/v0.rs index 841617dfd..2d9ad59ad 100644 --- a/grovedb-bulk-append-tree/src/cost/v0.rs +++ b/grovedb-bulk-append-tree/src/cost/v0.rs @@ -7,14 +7,12 @@ //! Locked: GROVE_V1..V3 are released and CommitmentTree has been billing this //! figure on mainnet. -#[cfg(feature = "storage")] -use grovedb_dense_fixed_sized_merkle_tree::SlotWriteAccounting; use grovedb_merkle_mountain_range::hash_count_for_push; #[cfg(feature = "storage")] use grovedb_merkle_mountain_range::LeafValueStorageCost; #[cfg(feature = "storage")] -use super::AppendStorageAccounting; +use super::{AppendStorageAccounting, SlotRewriteAccounting}; pub(super) fn compaction_hash_count(leaf_count: u64) -> u32 { hash_count_for_push(leaf_count) @@ -30,7 +28,7 @@ pub(super) fn compaction_hash_count(leaf_count: u64) -> u32 { #[cfg(feature = "storage")] pub(super) fn append_storage_accounting() -> AppendStorageAccounting { AppendStorageAccounting { - slot_write: SlotWriteAccounting::AsNew, + slot_rewrite: SlotRewriteAccounting::AsNew, chunk_leaf: LeafValueStorageCost::New, prepay_chunk_share: false, } diff --git a/grovedb-bulk-append-tree/src/cost/v1.rs b/grovedb-bulk-append-tree/src/cost/v1.rs index 6a51fe3c2..17433561f 100644 --- a/grovedb-bulk-append-tree/src/cost/v1.rs +++ b/grovedb-bulk-append-tree/src/cost/v1.rs @@ -2,14 +2,12 @@ //! //! Adds the peak-bagging merges [`v0`](super::v0) omitted. Used from GROVE_V4. -#[cfg(feature = "storage")] -use grovedb_dense_fixed_sized_merkle_tree::SlotWriteAccounting; #[cfg(feature = "storage")] use grovedb_merkle_mountain_range::LeafValueStorageCost; use grovedb_merkle_mountain_range::{hash_count_for_push, hash_count_for_root_bagging}; #[cfg(feature = "storage")] -use super::AppendStorageAccounting; +use super::{AppendStorageAccounting, SlotRewriteAccounting}; #[cfg(feature = "storage")] use crate::chunk::chunk_blob_entry_bytes; @@ -26,16 +24,17 @@ pub(super) fn compaction_hash_count(leaf_count: u64, mmr_size_after_push: u64) - /// - The entry's chunk-blob share (its own bytes) is charged as added /// storage at its append — these are the bytes that persist. /// - The buffer slot write is sized against the value the slot already holds -/// in committed storage: a rewrite (epoch 2 onward) is replaced, growth is -/// added, shrink is not credited; a slot written for the first time stays -/// fully added. +/// in committed storage, which is read first (and the read billed): a +/// rewrite (epoch 2 onward) is replaced, growth is added, shrink is not +/// credited; a slot written for the first time stays fully added and is +/// not read. /// - The compaction blob is reported as a replacement of the entry bytes it /// supersedes (all prepaid), leaving only its framing — and the MMR /// internal nodes — as added storage. #[cfg(feature = "storage")] pub(super) fn append_storage_accounting() -> AppendStorageAccounting { AppendStorageAccounting { - slot_write: SlotWriteAccounting::AgainstCommitted, + slot_rewrite: SlotRewriteAccounting::AgainstCommitted, chunk_leaf: LeafValueStorageCost::PartlyPrepaid(chunk_blob_entry_bytes), prepay_chunk_share: true, } diff --git a/grovedb-bulk-append-tree/src/tree/append.rs b/grovedb-bulk-append-tree/src/tree/append.rs index db2049c6d..f2ccbe6e4 100644 --- a/grovedb-bulk-append-tree/src/tree/append.rs +++ b/grovedb-bulk-append-tree/src/tree/append.rs @@ -1,6 +1,7 @@ //! Append and compaction logic for BulkAppendTree. use grovedb_costs::{CostResult, CostsExt, OperationCost}; +use grovedb_dense_fixed_sized_merkle_tree::{position_key, SlotWriteAccounting}; use grovedb_merkle_mountain_range::{mmr_size_to_leaf_count, MmrKeySize, MmrNode, MmrStore, MMR}; use grovedb_storage::StorageContext; use grovedb_version::version::GroveVersion; @@ -11,7 +12,10 @@ use super::{ }; use crate::{ chunk::serialize_chunk_blob, - cost::{append_storage_accounting, compaction_hash_count}, + cost::{ + append_storage_accounting, compaction_hash_count, AppendStorageAccounting, + SlotRewriteAccounting, + }, BulkAppendError, }; @@ -29,6 +33,7 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { mmr_overlay: Vec::new(), // Empty tree → empty MMR → zero root. last_mmr_root: Some([0u8; 32]), + committed_total_count: 0, }) } @@ -56,9 +61,61 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { // Lazy: the restored MMR may not be readable until an append occurs, // so don't compute the root here. The first append fills the cache. last_mmr_root: None, + committed_total_count: total_count, }) } + /// Whether buffer slot `position` holds a value in committed storage: + /// every slot once a chunk has ever been completed (the buffer was full + /// when it compacted), otherwise the slots below the committed buffer + /// count. Judged against the state at open — see `committed_total_count`. + pub(crate) fn slot_is_committed(&self, position: u16) -> bool { + let epoch_size = self.epoch_size(); + self.committed_total_count / epoch_size > 0 + || (position as u64) < self.committed_total_count % epoch_size + } + + /// Decide how the next buffer-slot write is reported, reading the slot's + /// committed value when the accounting sizes rewrites against it. + /// + /// The read — one seek, the committed value's bytes — is billed into + /// `cost`. It goes to the underlying storage (committed state plus the + /// surrounding transaction), never to the session's write-through cache: + /// a `StorageBatch` keeps one put per key, so the put that is eventually + /// charged must describe the transition from the committed value. A + /// slot written for the first time, and a full buffer (the append + /// compacts and writes no slot), are not read. A committed slot that + /// storage does not hold — corruption, not a state this code produces — + /// is charged as new, the safe direction. + fn slot_write_accounting( + &self, + accounting: &AppendStorageAccounting, + cost: &mut OperationCost, + ) -> Result { + if accounting.slot_rewrite == SlotRewriteAccounting::AsNew { + return Ok(SlotWriteAccounting::AsNew); + } + let position = self.dense_tree.count(); + if position >= self.dense_tree.capacity() || !self.slot_is_committed(position) { + return Ok(SlotWriteAccounting::AsNew); + } + match self + .dense_tree + .storage + .get(position_key(position)) + .unwrap_add_cost(cost) + { + Ok(Some(previous)) => Ok(SlotWriteAccounting::Overwrite { + previous_value_len: previous.len() as u32, + }), + Ok(None) => Ok(SlotWriteAccounting::AsNew), + Err(e) => Err(BulkAppendError::StorageError(format!( + "committed slot {} read before rewrite failed: {}", + position, e + ))), + } + } + /// Append a value to the tree. /// /// Handles dense tree insert, auto-compaction when the buffer fills, and @@ -78,7 +135,7 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { // +1 for the blake3 state-root computation we just did. hash_count: r.hash_count.saturating_add(1), compacted: r.compacted, - prepaid_chunk_bytes: r.prepaid_chunk_bytes, + storage_accounting_cost: r.storage_accounting_cost, }) } @@ -97,7 +154,7 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { /// version; what the version selects is the reported `hash_count` (for an /// append that compacts) and the storage accounting of the data writes — /// the cost information attached to the slot put, and the - /// `prepaid_chunk_bytes` the caller bills as added storage. + /// `storage_accounting_cost` the caller bills. pub fn append_no_state_root( &mut self, value: &[u8], @@ -106,11 +163,13 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { let mut hash_count: u32 = 0; let global_position = self.total_count; let accounting = append_storage_accounting(grove_version)?; + let mut storage_accounting_cost = OperationCost::default(); + let slot_write = self.slot_write_accounting(&accounting, &mut storage_accounting_cost)?; // 1. Try to insert into the dense tree buffer. let try_result = self .dense_tree - .try_insert_with_accounting(value, accounting.slot_write) + .try_insert_with_accounting(value, slot_write) .unwrap() .map_err(|e| { BulkAppendError::StorageError(format!("dense tree insert failed: {}", e)) @@ -142,11 +201,16 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { self.total_count += 1; + storage_accounting_cost.storage_cost.added_bytes = storage_accounting_cost + .storage_cost + .added_bytes + .saturating_add(accounting.prepaid_chunk_bytes(value.len())); + Ok(AppendNoStateRootResult { global_position, hash_count, compacted, - prepaid_chunk_bytes: accounting.prepaid_chunk_bytes(value.len()), + storage_accounting_cost, }) } @@ -178,10 +242,19 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { Ok(a) => a, Err(e) => return Err(e).wrap_with_cost(cost), }; + // The committed-slot read is billed here, in the returned cost, and + // mirrored (with the prepaid share) in the result for information. + let mut storage_accounting_cost = OperationCost::default(); + let slot_write = self.slot_write_accounting(&accounting, &mut storage_accounting_cost); + cost += storage_accounting_cost.clone(); + let slot_write = match slot_write { + Ok(s) => s, + Err(e) => return Err(e).wrap_with_cost(cost), + }; let try_result = match self .dense_tree - .try_insert_no_root_with_accounting(value, accounting.slot_write) + .try_insert_no_root_with_accounting(value, slot_write) .unwrap_add_cost(&mut cost) { Ok(r) => r, @@ -238,12 +311,16 @@ impl<'db, S: StorageContext<'db>> BulkAppendTree { .storage_cost .added_bytes .saturating_add(prepaid_chunk_bytes); + storage_accounting_cost.storage_cost.added_bytes = storage_accounting_cost + .storage_cost + .added_bytes + .saturating_add(prepaid_chunk_bytes); Ok(AppendNoStateRootResult { global_position, hash_count, compacted, - prepaid_chunk_bytes, + storage_accounting_cost, }) .wrap_with_cost(cost) } diff --git a/grovedb-bulk-append-tree/src/tree/mod.rs b/grovedb-bulk-append-tree/src/tree/mod.rs index f8dfe3e47..01130febe 100644 --- a/grovedb-bulk-append-tree/src/tree/mod.rs +++ b/grovedb-bulk-append-tree/src/tree/mod.rs @@ -22,6 +22,8 @@ mod storage_accounting_tests; #[cfg(all(test, feature = "storage"))] mod tests; +#[cfg(feature = "storage")] +use grovedb_costs::OperationCost; use grovedb_dense_fixed_sized_merkle_tree::DenseFixedSizedMerkleTree; use grovedb_merkle_mountain_range::MmrNode; @@ -40,10 +42,9 @@ pub struct AppendResult { pub hash_count: u32, /// Whether compaction (epoch flush) occurred. pub compacted: bool, - /// The entry's chunk-blob share, which the caller bills as - /// `storage_cost.added_bytes`. See - /// [`AppendNoStateRootResult::prepaid_chunk_bytes`]. - pub prepaid_chunk_bytes: u32, + /// The storage-accounting cost the caller bills. See + /// [`AppendNoStateRootResult::storage_accounting_cost`]. + pub storage_accounting_cost: OperationCost, } /// Result returned by [`BulkAppendTree::append_no_state_root`]. @@ -52,7 +53,7 @@ pub struct AppendResult { /// once at the end of a batch via /// [`BulkAppendTree::compute_current_state_root`]. #[cfg(feature = "storage")] -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub struct AppendNoStateRootResult { /// The 0-based global position of the appended value. pub global_position: u64, @@ -61,22 +62,28 @@ pub struct AppendNoStateRootResult { pub hash_count: u32, /// Whether compaction (epoch flush) occurred. pub compacted: bool, - /// The entry's share of the chunk blob it will eventually be compacted - /// into — its own bytes — to be charged as `storage_cost.added_bytes` at - /// this append, so that the blob written at compaction can be reported as - /// a replacement of bytes already paid for (issue #822). `0` under the - /// shipped accounting (GROVE_V1..V3), where the blob is charged in full - /// at compaction instead. + /// The cost of this append's storage accounting, which the + /// `Result`-returning appends cannot otherwise surface: + /// + /// - `storage_cost.added_bytes`: the entry's share of the chunk blob it + /// will eventually be compacted into — its own bytes — charged at this + /// append so that the blob written at compaction can be reported as a + /// replacement of bytes already paid for (issue #822); + /// - `seek_count` / `storage_loaded_bytes`: the read of the committed + /// value a buffer slot already holds (epoch 2 onward), performed to + /// size the rewrite. + /// + /// Zero under the shipped accounting (GROVE_V1..V3), where the blob is + /// charged in full at compaction and no slot is read. /// /// Like `hash_count`, this follows the "caller bills" convention of the /// `Result`-returning appends ([`append`](BulkAppendTree::append), - /// [`append_no_state_root`](BulkAppendTree::append_no_state_root)): - /// add it to the operation's `storage_cost.added_bytes`. The - /// `CostResult`-returning + /// [`append_no_state_root`](BulkAppendTree::append_no_state_root)): add it + /// to the operation's `OperationCost`. The `CostResult`-returning /// [`append_deferred_roots`](BulkAppendTree::append_deferred_roots) - /// already includes it in the returned cost; there the field is a - /// mirror for information only — do not bill it twice. - pub prepaid_chunk_bytes: u32, + /// already includes it in the returned cost; there the field is a mirror + /// for information only — do not bill it twice. + pub storage_accounting_cost: OperationCost, } /// Compute MMR size from leaf count: `2 * n - popcount(n)`. @@ -129,6 +136,15 @@ pub struct BulkAppendTree { /// /// [`from_state`]: BulkAppendTree::from_state pub(crate) last_mmr_root: Option<[u8; 32]>, + /// `total_count` as of the open ([`new`] → 0, [`from_state`] → the + /// persisted count): what committed storage holds, which this session's + /// appends have not changed. Used to tell a buffer slot that holds a + /// committed value — whose rewrite is read and reported as a replacement + /// — from one written for the first time. + /// + /// [`new`]: BulkAppendTree::new + /// [`from_state`]: BulkAppendTree::from_state + pub(crate) committed_total_count: u64, } impl BulkAppendTree { diff --git a/grovedb-bulk-append-tree/src/tree/storage_accounting_tests.rs b/grovedb-bulk-append-tree/src/tree/storage_accounting_tests.rs index f01e9d65e..6c73fef10 100644 --- a/grovedb-bulk-append-tree/src/tree/storage_accounting_tests.rs +++ b/grovedb-bulk-append-tree/src/tree/storage_accounting_tests.rs @@ -2,17 +2,22 @@ //! the level of the cost information each put carries. //! //! The in-memory context records every `put` with its `cost_info`, which is -//! exactly what a real storage context hands the commit path to bill. -//! Note that the in-memory context is immediate — reads see earlier writes -//! of the same session — so the "slot holds a committed value" cases here -//! stand in for a slot written in an earlier, committed session; the -//! batch-dedup behaviour of a real transactional context is pinned by the -//! GroveDB-level tests. - -use grovedb_costs::storage_cost::{ - key_value_cost::KeyValueStorageCost, removal::StorageRemovedBytes::NoStorageRemoval, +//! exactly what a real storage context hands the commit path to bill. A +//! buffer slot counts as holding a committed value by the `total_count` the +//! tree was opened with, so each epoch here is run on a tree re-opened with +//! `from_state` over the previous epoch's storage — the way every GroveDB +//! operation opens it. + +use grovedb_costs::{ + storage_cost::{ + key_value_cost::KeyValueStorageCost, removal::StorageRemovedBytes::NoStorageRemoval, + StorageCost, + }, + OperationCost, +}; +use grovedb_version::version::{ + v1::GROVE_V1, v2::GROVE_V2, v3::GROVE_V3, v4::GROVE_V4, GroveVersion, }; -use grovedb_version::version::{v1::GROVE_V1, v3::GROVE_V3, v4::GROVE_V4, GroveVersion}; use crate::{test_utils::MemStorageContext, BulkAppendError, BulkAppendTree}; @@ -31,10 +36,8 @@ fn paid(len: u32) -> u32 { } /// Dense-buffer slot puts (2-byte position keys), in order. -fn slot_puts(tree: &BulkAppendTree) -> Vec> { - tree.dense_tree - .storage - .puts +fn slot_puts(ctx: &MemStorageContext) -> Vec> { + ctx.puts .borrow() .iter() .filter(|(k, _)| k.len() == 2) @@ -43,10 +46,8 @@ fn slot_puts(tree: &BulkAppendTree) -> Vec) -> Vec> { - tree.dense_tree - .storage - .puts +fn mmr_puts(ctx: &MemStorageContext) -> Vec> { + ctx.puts .borrow() .iter() .filter(|(k, _)| k.len() == 4) @@ -68,30 +69,53 @@ const VALUES: [&[u8]; 12] = [ &[12; 8], // compaction 3 ]; -fn run(version: &GroveVersion) -> (BulkAppendTree, Vec, [u8; 32]) { - let mut tree = BulkAppendTree::new(2, MemStorageContext::new()).expect("new"); - let mut prepaid = Vec::new(); - for v in VALUES { - let r = tree.append_no_state_root(v, version).expect("append"); - prepaid.push(r.prepaid_chunk_bytes); +struct Run { + ctx: MemStorageContext, + accounting: Vec, + root: [u8; 32], +} + +/// Run `VALUES` one epoch per session: each epoch re-opens the tree with +/// `from_state` over the storage the previous one left behind. +fn run(version: &GroveVersion) -> Run { + let mut ctx = MemStorageContext::new(); + let mut accounting = Vec::new(); + let mut root = [0u8; 32]; + for (epoch, values) in VALUES.chunks(4).enumerate() { + let mut tree = BulkAppendTree::from_state((epoch * 4) as u64, 2, ctx).expect("open"); + for v in values { + let r = tree.append_no_state_root(v, version).expect("append"); + accounting.push(r.storage_accounting_cost); + } + root = tree.compute_current_state_root().expect("root"); + tree.commit_mmr(version).expect("commit"); + ctx = tree.dense_tree.storage; + } + Run { + ctx, + accounting, + root, } - let root = tree.compute_current_state_root().expect("root"); - tree.commit_mmr(version).expect("commit"); - (tree, prepaid, root) } /// v0 (GROVE_V1..V3): every data put — slot, blob, MMR node — is issued /// with no cost information, so the commit path bills each as new storage, -/// and nothing is prepaid at append time. +/// nothing is prepaid at append time and no slot is read. #[test] -fn v0_issues_every_put_without_cost_info_and_prepays_nothing() { - for version in [&GROVE_V1, &GROVE_V3] { - let (tree, prepaid, _) = run(version); - assert!(prepaid.iter().all(|&p| p == 0), "{prepaid:?}"); - let slots = slot_puts(&tree); +fn v0_issues_every_put_without_cost_info_and_bills_no_accounting() { + for version in [&GROVE_V1, &GROVE_V2, &GROVE_V3] { + let run = run(version); + assert!( + run.accounting + .iter() + .all(|c| *c == OperationCost::default()), + "{:?}", + run.accounting + ); + let slots = slot_puts(&run.ctx); assert_eq!(slots.len(), 9, "three slots per epoch, three epochs"); assert!(slots.iter().all(Option::is_none), "{slots:?}"); - let nodes = mmr_puts(&tree); + let nodes = mmr_puts(&run.ctx); // 3 leaves + 1 merge (leaf count 1 -> 2) = 4 nodes; the third leaf // (count 2 -> 3) collapses nothing. assert_eq!(nodes.len(), 4); @@ -100,22 +124,34 @@ fn v0_issues_every_put_without_cost_info_and_prepays_nothing() { } /// v1 (GROVE_V4): a slot written for the first time is new storage; a slot -/// that already holds a value is a replacement — growth added, shrink not -/// credited, key not charged; every append prepays its own bytes. +/// that already holds a committed value is a replacement — growth added, +/// shrink not credited, key not charged; every append prepays its own bytes. #[test] fn v1_slot_rewrites_are_replacements_and_every_append_prepays_its_bytes() { - let (tree, prepaid, _) = run(&GROVE_V4); + let run = run(&GROVE_V4); + // The in-memory context reports no cost for its reads, so the + // accounting cost carries the prepaid share only; the read's seek and + // bytes are pinned against RocksDB by the GroveDB-level tests. + let prepaid: Vec = run + .accounting + .iter() + .map(|c| c.storage_cost.added_bytes) + .collect(); let expected_prepaid: Vec = VALUES.iter().map(|v| v.len() as u32).collect(); assert_eq!(prepaid, expected_prepaid); + assert!(run + .accounting + .iter() + .all(|c| c.storage_cost.replaced_bytes == 0 && c.hash_node_calls == 0)); - let slots = slot_puts(&tree); + let slots = slot_puts(&run.ctx); assert_eq!(slots.len(), 9); // Epoch 1: fresh slots. assert!(slots[..3].iter().all(Option::is_none), "{slots:?}"); let rewrite = |previous: u32, new: u32| KeyValueStorageCost { key_storage_cost: Default::default(), - value_storage_cost: grovedb_costs::storage_cost::StorageCost { + value_storage_cost: StorageCost { added_bytes: paid(new).saturating_sub(paid(previous)), replaced_bytes: paid(new).min(paid(previous)), removed_bytes: NoStorageRemoval, @@ -143,13 +179,55 @@ fn v1_slot_rewrites_are_replacements_and_every_append_prepays_its_bytes() { assert_eq!(slots[8], Some(rewrite(4, 8))); } +/// Whether a slot is rewritten is judged against the state at open: inside +/// one session a slot written for the first time and rewritten after an +/// in-session compaction is still new storage (nothing is committed), so +/// it is never read and never reported as a replacement — a `StorageBatch` +/// will charge its last put, which must describe the transition from +/// committed storage. A slot the open count says is committed but storage +/// does not hold is charged as new, the safe direction. +#[test] +fn v1_judges_committed_slots_by_the_count_at_open() { + // Two epochs on a tree created in this session: no slot is committed. + let mut tree = BulkAppendTree::new(2, MemStorageContext::new()).expect("new"); + for v in &VALUES[..8] { + tree.append_no_state_root(v, &GROVE_V4).expect("append"); + } + let slots = slot_puts(&tree.dense_tree.storage); + assert_eq!(slots.len(), 6); + assert!(slots.iter().all(Option::is_none), "{slots:?}"); + + // Opened with two committed buffer entries and no chunk: slots 0 and 1 + // are committed, slot 2 is not and is written without a read. + let mut seeded = BulkAppendTree::new(2, MemStorageContext::new()).expect("new"); + for v in &VALUES[..2] { + seeded.append_no_state_root(v, &GROVE_V4).expect("seed"); + } + let mut tree = BulkAppendTree::from_state(2, 2, seeded.dense_tree.storage).expect("open"); + assert!(tree.slot_is_committed(0) && tree.slot_is_committed(1)); + assert!(!tree.slot_is_committed(2)); + tree.append_no_state_root(&[1; 8], &GROVE_V4) + .expect("slot 2"); + let slots = slot_puts(&tree.dense_tree.storage); + assert_eq!(slots, vec![None, None, None]); + + // Opened after a completed chunk: every slot is committed — and one + // that storage does not hold (corruption, not a state this code + // produces) is charged as new rather than as a rewrite of nothing. + let mut tree = BulkAppendTree::from_state(4, 2, MemStorageContext::new()).expect("open"); + assert!((0..3).all(|p| tree.slot_is_committed(p))); + tree.append_no_state_root(&[1; 8], &GROVE_V4) + .expect("slot 0"); + assert_eq!(slot_puts(&tree.dense_tree.storage), vec![None]); +} + /// v1: the compaction blob is reported as a replacement of the entry bytes /// it supersedes — all prepaid — with only its framing added; internal MMR /// nodes are new storage. #[test] fn v1_commit_mmr_reports_blob_as_replacement_of_prepaid_entry_bytes() { - let (tree, _, _) = run(&GROVE_V4); - let nodes = mmr_puts(&tree); + let run = run(&GROVE_V4); + let nodes = mmr_puts(&run.ctx); assert_eq!(nodes.len(), 4, "{nodes:?}"); let leaf = |entry_bytes: u32, blob_len: u32| { @@ -157,7 +235,7 @@ fn v1_commit_mmr_reports_blob_as_replacement_of_prepaid_entry_bytes() { let node_len = 37 + blob_len; KeyValueStorageCost { key_storage_cost: Default::default(), - value_storage_cost: grovedb_costs::storage_cost::StorageCost { + value_storage_cost: StorageCost { added_bytes: paid(node_len) - entry_bytes, replaced_bytes: entry_bytes, removed_bytes: NoStorageRemoval, @@ -179,34 +257,63 @@ fn v1_commit_mmr_reports_blob_as_replacement_of_prepaid_entry_bytes() { /// The tree itself must not depend on the accounting version. #[test] fn stored_state_and_roots_are_identical_across_accounting_versions() { - let (t3, _, root3) = run(&GROVE_V3); - let (t4, _, root4) = run(&GROVE_V4); - assert_eq!(root3, root4); + let r3 = run(&GROVE_V3); + let r4 = run(&GROVE_V4); + assert_eq!(r3.root, r4.root); assert_eq!( - *t3.dense_tree.storage.data.borrow(), - *t4.dense_tree.storage.data.borrow(), + *r3.ctx.data.borrow(), + *r4.ctx.data.borrow(), "byte-identical storage" ); } -/// The cost-propagating append bills the prepaid share in its returned cost -/// and mirrors it in the result; the plain appends leave billing to the -/// caller. +/// The cost-propagating append bills the accounting cost in its returned +/// cost and mirrors it in the result; the plain appends leave billing to +/// the caller. #[test] -fn append_deferred_roots_bills_prepaid_share_in_cost() { +fn append_deferred_roots_bills_accounting_cost() { let mut tree = BulkAppendTree::new(2, MemStorageContext::new()).expect("new"); let ctx = tree.append_deferred_roots(&[7u8; 20], &GROVE_V4); let r = ctx.value.expect("append"); - assert_eq!(r.prepaid_chunk_bytes, 20); + assert_eq!(r.storage_accounting_cost.storage_cost.added_bytes, 20); assert_eq!(ctx.cost.storage_cost.added_bytes, 20); assert_eq!(ctx.cost.storage_cost.replaced_bytes, 0); let mut legacy = BulkAppendTree::new(2, MemStorageContext::new()).expect("new"); let ctx = legacy.append_deferred_roots(&[7u8; 20], &GROVE_V3); - assert_eq!(ctx.value.expect("append").prepaid_chunk_bytes, 0); + assert_eq!( + ctx.value.expect("append").storage_accounting_cost, + OperationCost::default() + ); assert_eq!(ctx.cost.storage_cost.added_bytes, 0); } +/// A storage fault on the committed-slot read surfaces as an error and +/// nothing is written; the shipped accounting never performs that read. +#[test] +fn committed_slot_read_failure_surfaces() { + let ctx = MemStorageContext::new(); + ctx.fail_reads(); + let mut tree = BulkAppendTree::from_state(4, 2, ctx).expect("open"); + let err = tree + .append_no_state_root(&[1; 8], &GROVE_V4) + .expect_err("read failed"); + assert!( + matches!(&err, BulkAppendError::StorageError(m) if m.contains("committed slot")), + "{err:?}" + ); + let err = tree + .append_deferred_roots(&[1; 8], &GROVE_V4) + .value + .expect_err("read failed"); + assert!(matches!(err, BulkAppendError::StorageError(_))); + assert!(tree.dense_tree.storage.puts.borrow().is_empty()); + + // v0 reads nothing, so it writes straight through the broken reader. + tree.append_no_state_root(&[1; 8], &GROVE_V3) + .expect("no read under the shipped accounting"); +} + /// An unknown accounting version is rejected at every entry that consults /// it, never silently treated as one of the implemented ones. #[test] @@ -234,5 +341,5 @@ fn unknown_storage_accounting_version_is_rejected() { // The overlay survives the rejected flush. tree.commit_mmr(&GROVE_V4) .expect("flush with a known version"); - assert_eq!(mmr_puts(&tree).len(), 1); + assert_eq!(mmr_puts(&tree.dense_tree.storage).len(), 1); } diff --git a/grovedb-commitment-tree/src/commitment_tree/cost/mod.rs b/grovedb-commitment-tree/src/commitment_tree/cost/mod.rs index cfb4f410f..de191e075 100644 --- a/grovedb-commitment-tree/src/commitment_tree/cost/mod.rs +++ b/grovedb-commitment-tree/src/commitment_tree/cost/mod.rs @@ -50,13 +50,13 @@ pub(crate) fn frontier_save_cost_info( #[cfg(test)] mod tests { - use grovedb_version::version::{v1::GROVE_V1, v3::GROVE_V3, v4::GROVE_V4}; + use grovedb_version::version::{v1::GROVE_V1, v2::GROVE_V2, v3::GROVE_V3, v4::GROVE_V4}; use super::*; #[test] fn v0_attaches_no_cost_info() { - for version in [&GROVE_V1, &GROVE_V3] { + for version in [&GROVE_V1, &GROVE_V2, &GROVE_V3] { assert!(frontier_save_cost_info(None, 74, version) .unwrap() .is_none()); diff --git a/grovedb-commitment-tree/src/commitment_tree/mod.rs b/grovedb-commitment-tree/src/commitment_tree/mod.rs index 9802c3011..1079a3e2e 100644 --- a/grovedb-commitment-tree/src/commitment_tree/mod.rs +++ b/grovedb-commitment-tree/src/commitment_tree/mod.rs @@ -348,12 +348,10 @@ impl<'db, S: StorageContext<'db>, M: MemoSize> CommitmentTree { cost.hash_node_calls += bulk_result.hash_count; // The note's chunk-blob share — its permanent bytes — charged as // added storage now, so the compaction blob is later reported as a - // replacement of bytes already paid for. Zero under the shipped + // replacement of bytes already paid for, plus the read of the + // committed slot that sizes a rewrite. Zero under the shipped // accounting (GROVE_V1..V3). See `BulkAppendTree` issue #822. - cost.storage_cost.added_bytes = cost - .storage_cost - .added_bytes - .saturating_add(bulk_result.prepaid_chunk_bytes); + cost += bulk_result.storage_accounting_cost; // 2. Append cmx to Sinsemilla frontier (tracks sinsemilla_hash_calls) let sinsemilla_root = match self.frontier.append(cmx) { @@ -506,11 +504,9 @@ impl<'db, S: StorageContext<'db>, M: MemoSize> CommitmentTree { } }; hash_count = hash_count.saturating_add(r.hash_count); - // The note's chunk-blob share, billed per entry as in `append_raw`. - cost.storage_cost.added_bytes = cost - .storage_cost - .added_bytes - .saturating_add(r.prepaid_chunk_bytes); + // The note's chunk-blob share and the committed-slot read, billed + // per entry as in `append_raw`. + cost += r.storage_accounting_cost; if r.compacted { any_compacted = true; } diff --git a/grovedb-dense-fixed-sized-merkle-tree/src/test_utils.rs b/grovedb-dense-fixed-sized-merkle-tree/src/test_utils.rs index 591982db7..8912910b7 100644 --- a/grovedb-dense-fixed-sized-merkle-tree/src/test_utils.rs +++ b/grovedb-dense-fixed-sized-merkle-tree/src/test_utils.rs @@ -1,9 +1,6 @@ //! Test utilities: in-memory StorageContext implementations. -use std::{ - cell::{Cell, RefCell}, - collections::HashMap, -}; +use std::{cell::RefCell, collections::HashMap}; use grovedb_costs::{ storage_cost::key_value_cost::KeyValueStorageCost, ChildrenSizesWithIsSumTree, CostContext, @@ -21,8 +18,6 @@ pub(crate) struct MemStorageContext { pub data: RefCell, Vec>>, /// Every data `put` in order, with the cost information it carried. pub puts: RefCell, Option)>>, - /// When set, every `get` returns a storage error (fault injection). - pub fail_get: Cell, } impl MemStorageContext { @@ -36,12 +31,6 @@ impl<'db> StorageContext<'db> for MemStorageContext { type RawIterator = MemRawIterator; fn get>(&self, key: K) -> CostResult>, grovedb_storage::Error> { - if self.fail_get.get() { - return Err(grovedb_storage::Error::StorageError( - "injected read failure".to_string(), - )) - .wrap_with_cost(OperationCost::default()); - } Ok(self.data.borrow().get(key.as_ref()).cloned()).wrap_with_cost(OperationCost::default()) } diff --git a/grovedb-dense-fixed-sized-merkle-tree/src/tests.rs b/grovedb-dense-fixed-sized-merkle-tree/src/tests.rs index c7d7215a3..6e225af94 100644 --- a/grovedb-dense-fixed-sized-merkle-tree/src/tests.rs +++ b/grovedb-dense-fixed-sized-merkle-tree/src/tests.rs @@ -1123,62 +1123,43 @@ mod slot_write_accounting { assert!(puts.iter().all(|(_, c)| c.is_none())); } - /// `AgainstCommitted` reads the slot first: an empty slot is new - /// storage; a held value makes the write a replacement (growth added, - /// shrink not credited, key not charged), and the read is billed. + /// `Overwrite` reports the write as a replacement of the committed value + /// the owner measured: growth added, shrink not credited, key not + /// charged — and nothing is read by the tree itself. #[test] - fn against_committed_sizes_the_rewrite_from_the_stored_value() { + fn overwrite_sizes_the_rewrite_from_the_supplied_previous_length() { let mut tree = DenseFixedSizedMerkleTree::new(2, MemStorageContext::new()).unwrap(); - // Nothing stored yet: new storage, as `AsNew`. - tree.try_insert_no_root_with_accounting(&[1u8; 8], SlotWriteAccounting::AgainstCommitted) - .unwrap() - .unwrap(); - tree.try_insert_with_accounting(&[2u8; 8], SlotWriteAccounting::AgainstCommitted) - .unwrap() - .unwrap(); - assert!(tree.storage.puts.borrow().iter().all(|(_, c)| c.is_none())); - - // A new cycle over the same keys. - tree.reset(); let grow = tree - .try_insert_no_root_with_accounting(&[9u8; 16], SlotWriteAccounting::AgainstCommitted) + .try_insert_no_root_with_accounting( + &[9u8; 16], + SlotWriteAccounting::Overwrite { + previous_value_len: 8, + }, + ) .unwrap() .unwrap(); assert_eq!(grow, Some(0)); let shrink = tree - .try_insert_with_accounting(&[9u8; 4], SlotWriteAccounting::AgainstCommitted) + .try_insert_with_accounting( + &[9u8; 4], + SlotWriteAccounting::Overwrite { + previous_value_len: 8, + }, + ) .unwrap() .unwrap(); assert_eq!(shrink.map(|(_, p)| p), Some(1)); let puts = tree.storage.puts.borrow(); - let c = puts[2].1.as_ref().expect("rewrite carries cost info"); + let c = puts[0].1.as_ref().expect("rewrite carries cost info"); assert!(!c.new_node); assert!(c.needs_value_verification); assert_eq!(c.key_storage_cost, Default::default()); assert_eq!(c.value_storage_cost.replaced_bytes, 9, "paid(8)"); assert_eq!(c.value_storage_cost.added_bytes, 8, "paid(16) - paid(8)"); - let c = puts[3].1.as_ref().expect("rewrite carries cost info"); + let c = puts[1].1.as_ref().expect("rewrite carries cost info"); assert_eq!(c.value_storage_cost.replaced_bytes, 5, "paid(4)"); assert_eq!(c.value_storage_cost.added_bytes, 0); assert_eq!(c.value_storage_cost.removed_bytes, NoStorageRemoval); } - - /// A storage fault on the read that sizes the rewrite surfaces as an - /// error, and nothing is written. - #[test] - fn against_committed_surfaces_a_failing_read() { - let mut tree = DenseFixedSizedMerkleTree::new(2, MemStorageContext::new()).unwrap(); - tree.storage.fail_get.set(true); - let err = tree - .try_insert_no_root_with_accounting(&[1u8; 8], SlotWriteAccounting::AgainstCommitted) - .unwrap() - .expect_err("the read before the overwrite failed"); - assert!( - err.to_string().contains("before overwrite"), - "error should name the read: {err}" - ); - assert!(tree.storage.puts.borrow().is_empty(), "nothing written"); - assert_eq!(tree.count(), 0); - } } diff --git a/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs b/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs index 74e986adf..c76b6f224 100644 --- a/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs +++ b/grovedb-dense-fixed-sized-merkle-tree/src/tree.rs @@ -34,7 +34,8 @@ pub fn position_key(pos: u16) -> [u8; 2] { /// only [`reset`](DenseFixedSizedMerkleTree::reset) — used by the bulk-append /// tree to start a new epoch over the same position keys — makes a later /// insert land on a key that already holds a committed value. The owner, -/// which knows whether that can be the case, chooses the mode. +/// which knows whether that is the case and what the committed value is, +/// chooses the mode; the tree attaches the matching cost information. #[cfg(feature = "storage")] #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum SlotWriteAccounting { @@ -42,23 +43,24 @@ pub enum SlotWriteAccounting { /// key and the value as new storage. Right for a slot that has never /// been written (and what every shipped version reports for all slots). AsNew, - /// The slot may already hold a committed value. Read it first (one - /// billed storage read) and, if present, report the write as replacing - /// it: `replaced_bytes` is the smaller of the previous and the new paid - /// size, `added_bytes` is growth only, and shrink is not credited (no - /// refund semantics for a rolling buffer). The key, which already exists, - /// is not charged. A slot found empty is reported as [`AsNew`]. + /// The slot holds a committed value of `previous_value_len` bytes: report + /// the write as replacing it — `replaced_bytes` is the smaller of the + /// previous and the new paid size, `added_bytes` is growth only, and + /// shrink is not credited (no refund semantics for a rolling buffer). + /// The key, which already exists, is not charged. /// - /// The read goes to the underlying storage context — committed state - /// plus the surrounding transaction — never to this session's - /// write-through cache. That is deliberate: a `StorageBatch` keeps one - /// put per key, so when a session writes the same slot twice (an epoch - /// boundary inside one batch) only the last put is charged, and it must - /// describe the transition from the committed value, not from the - /// intermediate one. - /// - /// [`AsNew`]: SlotWriteAccounting::AsNew - AgainstCommitted, + /// The owner supplies the committed size (the bulk-append tree reads + /// the slot from storage — committed state plus the surrounding + /// transaction, never this session's write-through cache — and bills + /// that read). That is deliberate: a `StorageBatch` keeps one put per + /// key, so when a session writes the same slot twice (an epoch boundary + /// inside one batch) only the last put is charged, and it must describe + /// the transition from the committed value, not from the intermediate + /// one. + Overwrite { + /// Length of the value the slot holds in committed storage. + previous_value_len: u32, + }, } /// A dense fixed-sized Merkle tree with embedded storage. @@ -200,7 +202,7 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { /// Returns `None` if the tree is full, otherwise returns /// `Some((root_hash, position))`. The write is reported as new storage; /// see [`try_insert_with_accounting`](Self::try_insert_with_accounting) - /// for a slot that may already hold a committed value. + /// for a slot that already holds a committed value. pub fn try_insert( &mut self, value: &[u8], @@ -380,25 +382,11 @@ impl<'db, S: StorageContext<'db>> DenseFixedSizedMerkleTree { let cost_info = match accounting { SlotWriteAccounting::AsNew => None, - SlotWriteAccounting::AgainstCommitted => { - // Committed/transaction state only — NOT the session cache; - // see `SlotWriteAccounting::AgainstCommitted`. - let previous = match self.storage.get(key).unwrap_add_cost(&mut cost) { - Ok(v) => v, - Err(e) => { - return Err(DenseMerkleError::StoreError(format!( - "get at pos {} before overwrite: {}", - position, e - ))) - .wrap_with_cost(cost); - } - }; - previous.map(|old| { - KeyValueStorageCost::for_in_place_value_rewrite( - old.len() as u32, - value.len() as u32, - ) - }) + SlotWriteAccounting::Overwrite { previous_value_len } => { + Some(KeyValueStorageCost::for_in_place_value_rewrite( + previous_value_len, + value.len() as u32, + )) } }; diff --git a/grovedb/src/batch/estimated_costs/average_case_costs.rs b/grovedb/src/batch/estimated_costs/average_case_costs.rs index 4e8b43ee3..c325db7ac 100644 --- a/grovedb/src/batch/estimated_costs/average_case_costs.rs +++ b/grovedb/src/batch/estimated_costs/average_case_costs.rs @@ -271,28 +271,64 @@ impl GroveOp { propagate, grove_version, ); - // Additional cost: buffer write + running hash. - // Most appends only write to the buffer (O(1)). Compaction - // happens once per epoch_size appends and is amortized. use grovedb_costs::storage_cost::{removal::StorageRemovedBytes, StorageCost}; let entry_size = value.len() as u32; - // 1 blake3 hash for running buffer hash chain + // Every append writes its buffer slot and, under the GROVE_V4 + // accounting (issue #822), charges its chunk-blob share as + // added storage, reads the slot's committed value to size a + // rewrite (epoch 2 on) and reports that rewrite as replaced. + // The compaction — the blob replacing the epoch's entry + // bytes, plus its framing — is charged at the tree's epoch + // scale when its own layer is declared with + // `TreeType::BulkAppendTree(chunk_power)`, and amortized to + // one entry otherwise. + // + // A BulkAppendTree accepts variable-size values, so the epoch + // is modelled as values the size of this one: exact for + // same-size values, an average otherwise — the bound-seeking + // worst-case arm saturates that dimension instead. + let epoch_entries: u32 = append_tree_chunk_power + .map(|chunk_power| 1u32 << chunk_power.min(16) as u32) + .unwrap_or(1); + let paid_entry = entry_size.saturating_add(entry_size.required_space() as u32); + // MMR leaf key + envelope, variable-format header and + // per-entry length prefixes, and one internal node. + let blob_framing = 37u32 + .saturating_add(37) + .saturating_add(1) + .saturating_add(epoch_entries.saturating_mul(4)) + .saturating_add(71); + // Hashes. Undeclared: the historical amortized figure (one + // running-hash call), an average with no epoch to scale by. + // Declared: an upper bound for the epoch — the dense-root + // walk over a full buffer (two hashes per filled position), + // the state root, and a compaction's chunk-leaf hash plus the + // MMR push merges and root bagging (each bounded by the + // 64-bit position space). const AVG_HASH_CALLS: u32 = 1; + const MAX_MMR_MERGES: u32 = 65; + let hash_calls = if append_tree_chunk_power.is_some() { + 2u32.saturating_mul(epoch_entries.saturating_sub(1)) + .saturating_add(2) + .saturating_add(2 * MAX_MMR_MERGES) + } else { + AVG_HASH_CALLS + }; item_cost.add_cost(OperationCost { - seek_count: 1, // 1 buffer entry write + // 1 buffer entry write + 1 committed-slot read. + seek_count: 2, storage_cost: StorageCost { - // The value's chunk-blob share, charged at every - // append (issue #822), plus its buffer slot when - // written for the first time. - added_bytes: entry_size.saturating_mul(2), - // The slot rewrite from epoch 2 on, plus the - // compaction blob — a replacement of the epoch's - // prepaid entry bytes — amortized per append. - replaced_bytes: entry_size.saturating_mul(2), + // Slot (new in epoch 1) + chunk-blob share + framing. + added_bytes: entry_size.saturating_mul(2).saturating_add(blob_framing), + // Slot rewrite + the blob replacing the epoch's + // entry bytes. + replaced_bytes: paid_entry + .saturating_add(epoch_entries.saturating_mul(entry_size)), removed_bytes: StorageRemovedBytes::NoStorageRemoval, }, - storage_loaded_bytes: 0, - hash_node_calls: AVG_HASH_CALLS, + // The committed slot value read to size the rewrite. + storage_loaded_bytes: entry_size as u64, + hash_node_calls: hash_calls, sinsemilla_hash_calls: 0, }) } @@ -815,7 +851,9 @@ impl TreeCache for AverageCaseTreeCacheKnownPaths { // instead of the constructor-enforced cap. let append_tree_chunk_power = if matches!( op, - GroveOp::CommitmentTreeInsert { .. } | GroveOp::PrivateDocumentStoreInsert { .. } + GroveOp::CommitmentTreeInsert { .. } + | GroveOp::PrivateDocumentStoreInsert { .. } + | GroveOp::BulkAppend { .. } ) { crate::batch::batch_structure::keyless_op_tree_key(&key).and_then(|tree_key| { // Match the declared layer by KEY BYTES, not by @@ -863,7 +901,8 @@ impl TreeCache for AverageCaseTreeCacheKnownPaths { | ( GroveOp::PrivateDocumentStoreInsert { .. }, TreeType::PrivateDocumentStore(cp), - ) => cp, + ) + | (GroveOp::BulkAppend { .. }, TreeType::BulkAppendTree(cp)) => cp, _ => return None, }; // A declared chunk power outside the range the diff --git a/grovedb/src/batch/estimated_costs/mod.rs b/grovedb/src/batch/estimated_costs/mod.rs index 327c9d70d..d82f80522 100644 --- a/grovedb/src/batch/estimated_costs/mod.rs +++ b/grovedb/src/batch/estimated_costs/mod.rs @@ -184,10 +184,10 @@ pub(in crate::batch) fn commitment_tree_insert_op_cost( + epoch_size as u64 * entry_size; OperationCost { - // 2 reads (CommitmentTree element + frontier) and up to - // 3 + FRONTIER_DEPTH writes (note entry, frontier, chunk blob, - // MMR internal nodes). - seek_count: 5 + FRONTIER_DEPTH, + // 3 reads (CommitmentTree element, frontier, and the committed + // note slot a rewrite is sized against) and up to 3 + FRONTIER_DEPTH + // writes (note entry, frontier, chunk blob, MMR internal nodes). + seek_count: 6 + FRONTIER_DEPTH, storage_cost: StorageCost { added_bytes: u32::try_from(added_bytes_u64).unwrap_or(u32::MAX), // The parent-Merk node replacement is charged by the @@ -197,11 +197,13 @@ pub(in crate::batch) fn commitment_tree_insert_op_cost( }, // Reads: the stored CommitmentTree element (fixed serialized // fields + Merk node framing, plus the caller-supplied flags - // bound) + the serialized frontier. + // bound) + the serialized frontier + the committed note the + // rewritten buffer slot holds (epoch 2 on). storage_loaded_bytes: (CT_ELEMENT_LOAD_BASE + element_flags_load_bound + MAX_FRONTIER_SIZE - + PER_PUT_OVERHEAD) as u64, + + PER_PUT_OVERHEAD) as u64 + + entry_size, // Blake3: the dense buffer's root recompute visits every filled // slot (2 hashes each, up to a full buffer), plus on compaction // the chunk-leaf hash and MMR merge cascade, plus the bulk diff --git a/grovedb/src/batch/estimated_costs/worst_case_costs.rs b/grovedb/src/batch/estimated_costs/worst_case_costs.rs index 59d6cafc7..1531d9a79 100644 --- a/grovedb/src/batch/estimated_costs/worst_case_costs.rs +++ b/grovedb/src/batch/estimated_costs/worst_case_costs.rs @@ -251,34 +251,52 @@ impl GroveOp { // Worst case: compaction trigger. Buffer fills → serialize // chunk blob → compute dense Merkle root → push to MMR. use grovedb_costs::storage_cost::{removal::StorageRemovedBytes, StorageCost}; - // Chunk blob worst case depends on epoch_size. For a single - // append the value itself is always written. If compaction - // triggers, the chunk blob is epoch_size * avg_value_size. - // We use value.len() for the per-append write and a capped - // compaction overhead. let value_size = value.len() as u32; - // Max compaction overhead: 64KB safe bound for chunk blob - const MAX_COMPACTION_BLOB: u32 = 65536; + /// Largest epoch the type permits: `2^16` entries. + const MAX_EPOCH_ENTRIES: u32 = 1 << 16; // Dense Merkle root: epoch_size hashes. Buffer hash: 1. // MMR push: up to 64 merges. // epoch hashes + buffer + MMR const MAX_HASH_CALLS: u32 = 1024 + 1 + 65; + const MAX_MMR_MERGES: u32 = 65; // Writes: buffer entry + chunk blob + MMR nodes - const MAX_WRITES: u32 = 1 + 1 + 65; + const MAX_WRITES: u32 = 1 + 1 + MAX_MMR_MERGES; const MAX_READS: u32 = 64; // MMR sibling reads + // Added storage under the GROVE_V4 accounting (issue #822): + // the value's buffer slot (new, or grown on a rewrite) and + // its chunk-blob share, plus on compaction the blob's framing + // — MMR leaf key and envelope, variable-format header and one + // 4-byte length prefix per entry of the largest epoch, the + // value-length varint — and every MMR internal node the push + // creates (key + 33-byte node + length). + const PER_PUT_KEY_AND_LENGTHS: u32 = 50; + const MAX_BLOB_FRAMING: u32 = 37 + 37 + 1 + 4 * MAX_EPOCH_ENTRIES + 5; + const MMR_INTERNAL_NODE_PUT: u32 = 37 + 33 + 1; + let max_added = value_size + .saturating_mul(2) + .saturating_add(PER_PUT_KEY_AND_LENGTHS) + .saturating_add(MAX_BLOB_FRAMING) + .saturating_add(MMR_INTERNAL_NODE_PUT * MAX_MMR_MERGES); item_cost.add_cost(OperationCost { - seek_count: MAX_WRITES + MAX_READS, + // +1: the read of the committed slot value that sizes a + // rewrite. + seek_count: MAX_WRITES + MAX_READS + 1, storage_cost: StorageCost { - added_bytes: value_size + MAX_COMPACTION_BLOB, - // GROVE_V4 accounting (issue #822): a rewritten - // buffer slot and the compaction blob (a replacement - // of the epoch's prepaid entry bytes) are reported - // as replaced, so the bound carries the same volume - // on that side too. - replaced_bytes: value_size + MAX_COMPACTION_BLOB, + added_bytes: max_added, + // The compaction blob is reported as a replacement + // of the epoch's entry bytes — the sum of whatever + // values an earlier state buffered, which neither + // the op nor the worst-case layer information can + // bound: the type permits values up to u32::MAX + // bytes (chunk entry lengths are u32). A smaller + // figure would not be an upper bound, so this + // dimension saturates. + replaced_bytes: u32::MAX, removed_bytes: StorageRemovedBytes::NoStorageRemoval, }, - storage_loaded_bytes: (33 * MAX_READS) as u64, + // MMR sibling reads + the committed slot value, which is + // bounded only by the u32 entry length. + storage_loaded_bytes: (33 * MAX_READS) as u64 + u32::MAX as u64, hash_node_calls: MAX_HASH_CALLS, sinsemilla_hash_calls: 0, }) diff --git a/grovedb/src/operations/bulk_append_tree.rs b/grovedb/src/operations/bulk_append_tree.rs index b7303e3a7..847c17cb2 100644 --- a/grovedb/src/operations/bulk_append_tree.rs +++ b/grovedb/src/operations/bulk_append_tree.rs @@ -91,12 +91,10 @@ impl GroveDb { cost.hash_node_calls += result.hash_count; // The value's chunk-blob share — its permanent bytes — is billed at - // its own append (zero under the shipped accounting); the compaction + // its own append, with the read of the committed slot that sizes a + // rewrite (both zero under the shipped accounting); the compaction // blob is then a replacement of bytes already paid for (issue #822). - cost.storage_cost.added_bytes = cost - .storage_cost - .added_bytes - .saturating_add(result.prepaid_chunk_bytes); + cost += result.storage_accounting_cost; let new_state_root = result.state_root; let new_total_count = tree.total_count; @@ -550,12 +548,9 @@ impl GroveDb { tree.append(value, grove_version).map_err(map_bulk_err) ); cost.hash_node_calls += result.hash_count; - // The value's chunk-blob share, billed per append as in the - // direct path (issue #822). - cost.storage_cost.added_bytes = cost - .storage_cost - .added_bytes - .saturating_add(result.prepaid_chunk_bytes); + // The value's chunk-blob share and the committed-slot read, + // billed per append as in the direct path (issue #822). + cost += result.storage_accounting_cost; } // Compute final state root diff --git a/grovedb/src/tests/append_storage_accounting_tests.rs b/grovedb/src/tests/append_storage_accounting_tests.rs index 967fc8a36..fe462fdd2 100644 --- a/grovedb/src/tests/append_storage_accounting_tests.rs +++ b/grovedb/src/tests/append_storage_accounting_tests.rs @@ -6,7 +6,8 @@ //! - every append charges the entry's chunk-blob share (its own bytes) as //! `added_bytes`; //! - a dense-buffer slot that already holds a committed value (epoch 2 on) -//! is `replaced_bytes`, growth added, shrink not credited; +//! is read (one billed seek plus the committed bytes) and its rewrite is +//! `replaced_bytes`, growth added, shrink not credited; //! - the compaction blob replaces the epoch's prepaid entry bytes, so only //! its framing and the MMR internal nodes are added; //! - the commitment tree's frontier rewrite replaces the frontier loaded at @@ -19,21 +20,39 @@ //! GROVE_V4 and once under GROVE_V4 with the two accounting gates switched //! off — so that the difference between the two costs is exactly the //! accounting change and nothing else (the parent-Merk update, the hash -//! counts and the Sinsemilla work are identical on both sides). The legacy -//! figures themselves are then pinned by the model below. +//! counts and the Sinsemilla work are identical on both sides; the only +//! I/O difference is the committed-slot read). The legacy figures +//! themselves are then pinned by the model below. + +use std::collections::HashMap; use grovedb_commitment_tree::{ CommitmentFrontier, DashMemo, NoteBytesData, TransmittedNoteCiphertext, }; -use grovedb_costs::OperationCost; +use grovedb_costs::{storage_cost::removal::StorageRemovedBytes::NoStorageRemoval, OperationCost}; +use grovedb_merk::{ + estimated_costs::{ + average_case_costs::{ + EstimatedLayerCount::EstimatedLevel, + EstimatedLayerInformation, + EstimatedLayerSizes::{AllItems, AllSubtrees}, + EstimatedSumTrees::NoSumTrees, + }, + worst_case_costs::WorstCaseLayerInformation::MaxElementsNumber, + }, + tree_type::TreeType, +}; use grovedb_version::version::{ v1::GROVE_V1, v2::GROVE_V2, v3::GROVE_V3, v4::GROVE_V4, GroveVersion, }; use crate::{ - batch::QualifiedGroveDbOp, + batch::{ + estimated_costs::EstimatedCostsType::{AverageCaseCostsType, WorstCaseCostsType}, + KeyInfoPath, QualifiedGroveDbOp, + }, tests::{common::EMPTY_PATH, make_empty_grovedb, TempGroveDb}, - Element, + Element, GroveDb, }; /// A stored note entry: cmx (32) || rho (32) || cv_net (32) || DashMemo @@ -128,6 +147,19 @@ fn expected_delta( (added, replaced) } +/// Expected `(seek_count, storage_loaded_bytes)` difference, V4 minus +/// legacy: the read of the committed value a buffer slot holds, which sizes +/// its rewrite — one seek and `committed_len` bytes, only for a buffered +/// (non-compacting) append onto a slot committed in an earlier epoch. +fn expected_read_delta(position: u64, chunk_power: u8, committed_len: u32) -> (u32, u64) { + let epoch = 1u64 << chunk_power; + if position % epoch == epoch - 1 || position < epoch { + (0, 0) + } else { + (1, committed_len as u64) + } +} + fn delta(v4: &OperationCost, legacy: &OperationCost) -> (i64, i64) { ( v4.storage_cost.added_bytes as i64 - legacy.storage_cost.added_bytes as i64, @@ -135,12 +167,23 @@ fn delta(v4: &OperationCost, legacy: &OperationCost) -> (i64, i64) { ) } -/// Everything except the storage figures must agree: the accounting gates -/// move bytes between `added` and `replaced`, nothing else. -fn assert_only_storage_differs(v4: &OperationCost, legacy: &OperationCost, what: &str) { - assert_eq!(v4.seek_count, legacy.seek_count, "{what}: seek_count"); +/// Everything except the storage figures and the committed-slot read must +/// agree: the accounting gates move bytes between `added` and `replaced` +/// and add that one read, nothing else. +fn assert_only_accounting_differs( + v4: &OperationCost, + legacy: &OperationCost, + what: &str, + (extra_seeks, extra_loaded): (u32, u64), +) { + assert_eq!( + v4.seek_count, + legacy.seek_count + extra_seeks, + "{what}: seek_count" + ); assert_eq!( - v4.storage_loaded_bytes, legacy.storage_loaded_bytes, + v4.storage_loaded_bytes, + legacy.storage_loaded_bytes + extra_loaded, "{what}: storage_loaded_bytes" ); assert_eq!( @@ -318,7 +361,12 @@ fn commitment_tree_append_storage_accounting_matches_model_across_epochs() { expected_delta(position, CHUNK_POWER, NOTE_ENTRY, true), "position {position}: v4 {v4_cost:?}\nlegacy {legacy_cost:?}" ); - assert_only_storage_differs(&v4_cost, &legacy_cost, &format!("position {position}")); + assert_only_accounting_differs( + &v4_cost, + &legacy_cost, + &format!("position {position}"), + expected_read_delta(position, CHUNK_POWER, NOTE_ENTRY), + ); assert_eq!( root_hash(&v4_db, &GROVE_V4), root_hash(&legacy_db, &legacy), @@ -344,6 +392,13 @@ fn frontier_rewrite_replaces_previous_size_and_adds_only_growth() { let legacy_cost = ct_insert(&legacy_db, position as u32, &legacy); let v4_cost = ct_insert(&v4_db, position as u32, &GROVE_V4); let (d_added, d_replaced) = delta(&v4_cost, &legacy_cost); + // Epoch 1: fresh slots, nothing read. + assert_only_accounting_differs( + &v4_cost, + &legacy_cost, + &format!("position {position}"), + (0, 0), + ); // Strip the (epoch-1, fresh-slot) share so only the frontier is left. let frontier_added = d_added - NOTE_ENTRY as i64; @@ -430,7 +485,8 @@ fn epoch_boundary_at_chunk_power_11_is_not_an_added_bytes_spike() { delta(&v4_cost, &legacy_cost), expected_delta((EPOCH - 1) as u64, CHUNK_POWER, NOTE_ENTRY, true) ); - assert_only_storage_differs(&v4_cost, &legacy_cost, "boundary"); + // A compacting append writes no slot and reads none. + assert_only_accounting_differs(&v4_cost, &legacy_cost, "boundary", (0, 0)); assert_eq!(root_hash(&v4_db, &GROVE_V4), root_hash(&legacy_db, &legacy)); } @@ -509,7 +565,8 @@ fn epoch_boundary_inside_one_batch_charges_slots_once_as_new() { v4_ctx.cost, legacy_ctx.cost ); - assert_only_storage_differs(&v4_ctx.cost, &legacy_ctx.cost, "batch"); + // Nothing was committed, so no slot is read. + assert_only_accounting_differs(&v4_ctx.cost, &legacy_ctx.cost, "batch", (0, 0)); assert_eq!(root_hash(&v4_db, &GROVE_V4), root_hash(&legacy_db, &legacy)); assert_verifies(&v4_db, &GROVE_V4); } @@ -554,6 +611,7 @@ fn bulk_append_tree_accounting_with_variable_size_values() { let mut added: i64 = len as i64; // the share let mut replaced: i64 = 0; + let mut read: (u32, u64) = (0, 0); epoch_bytes += len; if position % 4 == 3 { // Compaction: the blob replaces the epoch's entry bytes. @@ -566,6 +624,8 @@ fn bulk_append_tree_accounting_with_variable_size_values() { added += paid(len).saturating_sub(paid(previous)) as i64 - (SLOT_KEY_PAID + paid(len)) as i64; replaced += paid(len).min(paid(previous)) as i64; + // The committed value is read to size the rewrite. + read = (1, previous as u64); } slots[slot] = Some(len); } @@ -576,20 +636,19 @@ fn bulk_append_tree_accounting_with_variable_size_values() { v4_ctx.cost, legacy_ctx.cost ); - assert_only_storage_differs( + assert_only_accounting_differs( &v4_ctx.cost, &legacy_ctx.cost, &format!("position {position}"), + read, ); } assert_eq!(root_hash(&v4_db, &GROVE_V4), root_hash(&legacy_db, &legacy)); assert_verifies(&v4_db, &GROVE_V4); } -/// `PrivateDocumentStore` (fixed entry size) follows the same model. Its -/// append propagates the dense tree's costs, so V4 also bills the read that -/// sizes the slot rewrite: one seek per buffered append, loading the -/// committed entry from epoch 2 on. +/// `PrivateDocumentStore` (fixed entry size) follows the same model, +/// including the billed read of the committed entry a rewritten slot holds. #[test] fn private_document_store_accounting_matches_model() { const CHUNK_POWER: u8 = 2; // epoch 4 @@ -633,25 +692,12 @@ fn private_document_store_accounting_matches_model() { v4_ctx.cost, legacy_ctx.cost ); - let compacting = position % 4 == 3; - let (extra_seeks, extra_loaded) = if compacting { - (0, 0) - } else if position >= 4 { - (1, ENTRY as u64) - } else { - (1, 0) - }; - assert_eq!( - v4_ctx.cost.seek_count, - legacy_ctx.cost.seek_count + extra_seeks, - "position {position}: the slot read before a buffered write" - ); - assert_eq!( - v4_ctx.cost.storage_loaded_bytes, - legacy_ctx.cost.storage_loaded_bytes + extra_loaded, - "position {position}: the committed entry loaded to size the rewrite" + assert_only_accounting_differs( + &v4_ctx.cost, + &legacy_ctx.cost, + &format!("position {position}"), + expected_read_delta(position, CHUNK_POWER, ENTRY), ); - assert_eq!(v4_ctx.cost.hash_node_calls, legacy_ctx.cost.hash_node_calls); } assert_eq!(root_hash(&v4_db, &GROVE_V4), root_hash(&legacy_db, &legacy)); assert_verifies(&v4_db, &GROVE_V4); @@ -707,3 +753,152 @@ fn standalone_mmr_and_dense_trees_are_unaffected() { assert_eq!(a, b, "dense insert {i}"); } } + +// ── BulkAppend estimates vs actual at compaction ───────────────────── + +fn bulk_op(value: Vec) -> QualifiedGroveDbOp { + QualifiedGroveDbOp::bulk_append_op(vec![b"bulk".to_vec()], value) +} + +/// Average-case estimate with the bulk tree's own layer declared as +/// `TreeType::BulkAppendTree(chunk_power)` (or not, when `None`). +fn bulk_average_case_estimate( + ops: Vec, + declared_chunk_power: Option, + value_size: u32, + grove_version: &GroveVersion, +) -> OperationCost { + let mut paths = HashMap::new(); + paths.insert( + KeyInfoPath(vec![]), + EstimatedLayerInformation { + tree_type: TreeType::NormalTree, + estimated_layer_count: EstimatedLevel(1, false), + estimated_layer_sizes: AllSubtrees(4, NoSumTrees, None), + }, + ); + if let Some(chunk_power) = declared_chunk_power { + paths.insert( + KeyInfoPath::from_known_owned_path(vec![b"bulk".to_vec()]), + EstimatedLayerInformation { + tree_type: TreeType::BulkAppendTree(chunk_power), + estimated_layer_count: EstimatedLevel(16, false), + estimated_layer_sizes: AllItems(8, value_size, None), + }, + ); + } + GroveDb::estimated_case_operations_for_batch( + AverageCaseCostsType(paths), + ops, + None, + |_cost, _old_flags, _new_flags| Ok(false), + |_flags, _removed_key_bytes, _removed_value_bytes| Ok((NoStorageRemoval, NoStorageRemoval)), + grove_version, + ) + .cost_as_result() + .expect("average case estimate for BulkAppend") +} + +fn bulk_worst_case_estimate( + ops: Vec, + grove_version: &GroveVersion, +) -> OperationCost { + let mut paths = HashMap::new(); + paths.insert(KeyInfoPath(vec![]), MaxElementsNumber(2)); + GroveDb::estimated_case_operations_for_batch( + WorstCaseCostsType(paths), + ops, + None, + |_cost, _old_flags, _new_flags| Ok(false), + |_flags, _removed_key_bytes, _removed_value_bytes| Ok((NoStorageRemoval, NoStorageRemoval)), + grove_version, + ) + .cost_as_result() + .expect("worst case estimate for BulkAppend") +} + +/// Seed `seed` values into a fresh chunk_power-4 bulk tree, then apply +/// `last` as its own batch and return that batch's actual cost. +fn bulk_compaction_actual(seed: Vec>, last: Vec) -> OperationCost { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + db.insert( + EMPTY_PATH, + b"bulk", + Element::empty_bulk_append_tree(4).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert bulk append tree"); + db.apply_batch( + seed.into_iter().map(bulk_op).collect(), + None, + None, + grove_version, + ) + .unwrap() + .expect("seed"); + let ctx = db.apply_batch(vec![bulk_op(last)], None, None, grove_version); + ctx.value.expect("compaction append"); + ctx.cost +} + +/// The compaction blob replaces the epoch's entry bytes — whatever sizes +/// an earlier state buffered — so the BulkAppend estimates must dominate a +/// compaction whose actual `replaced_bytes` comes from large buffered +/// values even when the compacting value itself is tiny: the worst-case +/// arm (no declaration channel) saturates that dimension; the declared +/// average-case arm models an epoch of same-size values and is an upper +/// bound exactly there. +#[test] +fn bulk_append_estimates_dominate_actual_compaction_with_variable_sizes() { + let grove_version = GroveVersion::latest(); + const BIG: usize = 10 * 1024; + + // Fifteen 10 KiB values, then a 16-byte overflow value that compacts + // them: 15 * 10 KiB of replaced bytes for a 16-byte op. + let small = vec![9u8; 16]; + let actual_mixed = + bulk_compaction_actual((0..15u8).map(|i| vec![i; BIG]).collect(), small.clone()); + assert!( + actual_mixed.storage_cost.replaced_bytes as usize >= 15 * BIG, + "the blob replaces the buffered entry bytes: {actual_mixed:?}" + ); + let worst = bulk_worst_case_estimate(vec![bulk_op(small.clone())], grove_version); + assert!( + worst.worse_or_eq_than(&actual_mixed), + "worst case must dominate a compaction over larger buffered values;\nestimated \ + {worst:?}\nactual {actual_mixed:?}" + ); + // The undeclared average is an amortized one-entry figure; the declared + // one models same-size values — neither claims to bound this shape. + let average_undeclared = + bulk_average_case_estimate(vec![bulk_op(small.clone())], None, 16, grove_version); + assert!( + average_undeclared.storage_cost.replaced_bytes < actual_mixed.storage_cost.replaced_bytes + ); + + // Sixteen same-size values: the declared average-case estimate is an + // upper bound of the compaction, and the worst case still dominates. + let big = vec![15u8; BIG]; + let actual_same = + bulk_compaction_actual((0..15u8).map(|i| vec![i; BIG]).collect(), big.clone()); + let average_declared = bulk_average_case_estimate( + vec![bulk_op(big.clone())], + Some(4), + BIG as u32, + grove_version, + ); + assert!( + average_declared.worse_or_eq_than(&actual_same), + "declared average case must dominate a same-size compaction;\nestimated \ + {average_declared:?}\nactual {actual_same:?}" + ); + let worst = bulk_worst_case_estimate(vec![bulk_op(big)], grove_version); + assert!( + worst.worse_or_eq_than(&actual_same), + "worst case must dominate;\nestimated {worst:?}\nactual {actual_same:?}" + ); +}