From fa8b41150fcfe60701bfe4d4cc4c51e81f73831f Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 16 May 2026 19:58:33 +0700 Subject: [PATCH 1/3] feat: add Element::NotCountedOrSummed wrapper for combined sum+count opt-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a third element wrapper that suppresses BOTH count AND sum contributions when inserted into a count-and-sum-bearing parent. The existing wrappers each cover one axis: NonCounted suppresses count, NotSummed suppresses sum. ## Element variant ```rust Element::NotCountedOrSummed(Box) ``` - Inner element MUST be one of the four sum-bearing tree variants: `SumTree`, `BigSumTree`, `CountSumTree`, `ProvableCountSumTree`. (Same allow-list as `NotSummed`; `SumTree` / `BigSumTree` have an implicit count of 1 that this wrapper can suppress.) - Parent at insert time MUST be `CountSumTree` or `ProvableCountSumTree` — the only tree types where suppressing both axes is meaningful. Any other parent (including `NormalTree`, `SumTree`, `BigSumTree`, `CountTree`, `ProvableCountTree`) rejects the wrapper at insert with `InvalidInputError` (per-merk) or `InvalidBatchOperation` (batch). - The three wrappers are mutually exclusive — constructors and (de)serializers reject any nesting in any direction. Aggregation suppression flows through the standard helpers: - `count_value_or_default` returns 0 - `sum_value_or_default` / `big_sum_value_or_default` return 0 - `count_sum_value_or_default` returns (0, 0) ## Discriminant layout - Wrapper byte: 17 (after `NonCounted`=15 and `NotSummed`=16) - Twin prefix: `0xc0` (so synthetic twins are `NotCountedOrSummedSumTree`=196, `NotCountedOrSummedBigSumTree`=197, `NotCountedOrSummedCountSumTree`=199, `NotCountedOrSummedProvableCountSumTree`=202). - Upper-nibble compare distinguishes the three wrapper-twin ranges (`0x80` / `0xb0` / `0xc0`); the deserialize pre-check rejects every ordered pair of wrapper bytes to bound bincode recursion. ## Wiring - `grovedb-element`: constructor (`new_not_counted_or_summed`, `into_not_counted_or_summed`), helpers (`is_not_counted_or_summed`, `is_wrapped`), display, type-twin mapping, serde shadow, and serialize/deserialize guards. - `merk`: `TreeType::is_count_and_sum_bearing()` predicate; insert guards on `insert`/`insert_reference`/`insert_subtree`; `ElementReconstructExtensions`, `tree_type` extensions, cost paths, and v0/v1 `get` paths look through the new wrapper. - `grovedb`: `GroveOp::InsertTreeWithRootHash` gains a `not_counted_or_summed: bool` field carried through propagation and re-applied at execution via `into_not_counted_or_summed()`. Batch validation, query/proof/get match arms, debugger and estimated-cost paths all see the new wrapper. ## Tests - `grovedb-element`: discriminant pinning, helper predicates, serde round-trip and rejection cases. - `merk`: `TreeType::is_count_and_sum_bearing` matrix; per-merk insert acceptance in `CountSumTree`, rejection in `NormalTree`/`SumTree`/`CountTree`; aggregate-suppression assertion. - `grovedb`: end-to-end constructor invariants, batch insert rejection in non-count-sum parents, direct + batch insert in `CountSumTree` and `ProvableCountSumTree` with both axes suppressed, batch propagation preserves the wrapper on disk, and `check_subtree_exists` traverses through the wrapper. 421 merk lib tests pass, 83 grovedb-element tests pass, 1552 grovedb lib tests pass; clippy clean on the touched crates. Co-Authored-By: Claude Opus 4.7 (1M context) --- grovedb-element/src/element/constructor.rs | 78 ++- grovedb-element/src/element/helpers.rs | 72 ++- grovedb-element/src/element/mod.rs | 131 +++++- grovedb-element/src/element/serialize.rs | 91 +++- grovedb-element/src/element/visualize.rs | 5 + grovedb-element/src/element_type.rs | 228 ++++++++- grovedb/src/batch/mod.rs | 74 ++- grovedb/src/debugger.rs | 6 +- .../src/estimated_costs/average_case_costs.rs | 6 +- .../src/estimated_costs/worst_case_costs.rs | 12 +- grovedb/src/lib.rs | 2 +- grovedb/src/operations/get/query.rs | 14 +- grovedb/src/operations/insert/mod.rs | 2 +- grovedb/src/operations/proof/generate.rs | 8 +- grovedb/src/operations/proof/verify.rs | 8 +- grovedb/src/tests/batch_coverage_tests.rs | 1 + grovedb/src/tests/batch_rejection_tests.rs | 1 + grovedb/src/tests/batch_unit_tests.rs | 2 + grovedb/src/tests/mod.rs | 1 + .../src/tests/not_counted_or_summed_tests.rs | 444 ++++++++++++++++++ merk/src/element/costs.rs | 14 +- merk/src/element/get.rs | 20 +- merk/src/element/insert.rs | 96 ++++ merk/src/element/reconstruct.rs | 3 + merk/src/element/tree_type.rs | 36 +- merk/src/tree_type/mod.rs | 51 ++ 26 files changed, 1247 insertions(+), 159 deletions(-) create mode 100644 grovedb/src/tests/not_counted_or_summed_tests.rs diff --git a/grovedb-element/src/element/constructor.rs b/grovedb-element/src/element/constructor.rs index 04e58cd71..6b36ea236 100644 --- a/grovedb-element/src/element/constructor.rs +++ b/grovedb-element/src/element/constructor.rs @@ -388,13 +388,16 @@ impl Element { /// tree's aggregate count when inserted. Sums (if any) still propagate. /// /// Returns `InvalidInput` if `inner` is already wrapped in any wrapper - /// variant (`NonCounted` or `NotSummed`) — the wrappers are mutually - /// exclusive and may not nest in either direction. Use - /// `into_non_counted` to wrap idempotently when `inner` may already be - /// `NonCounted`; use that helper's `Result` return for the + /// variant (`NonCounted`, `NotSummed`, or `NotCountedOrSummed`) — the + /// wrappers are mutually exclusive and may not nest in either direction. + /// Use `into_non_counted` to wrap idempotently when `inner` may already + /// be `NonCounted`; use that helper's `Result` return for the /// cross-wrapper case. pub fn new_non_counted(inner: Element) -> Result { - if matches!(inner, Element::NonCounted(_) | Element::NotSummed(_)) { + if matches!( + inner, + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) + ) { return Err(ElementError::InvalidInput( "NonCounted cannot wrap another wrapper", )); @@ -405,15 +408,19 @@ impl Element { /// Wrap `self` in `NonCounted`. If `self` is already `NonCounted`, /// returns it unchanged (idempotent on `NonCounted`). /// - /// Returns `InvalidInput` if `self` is `NotSummed` — the two wrappers - /// are mutually exclusive. Callers that need the unconditional wrapping - /// path should ensure the input is a non-wrapper variant before calling. + /// Returns `InvalidInput` if `self` is any other wrapper variant — the + /// wrappers are mutually exclusive. Callers that need the unconditional + /// wrapping path should ensure the input is a non-wrapper variant + /// before calling. pub fn into_non_counted(self) -> Result { match self { Element::NonCounted(_) => Ok(self), Element::NotSummed(_) => Err(ElementError::InvalidInput( "cannot wrap NotSummed in NonCounted; wrappers are mutually exclusive", )), + Element::NotCountedOrSummed(_) => Err(ElementError::InvalidInput( + "cannot wrap NotCountedOrSummed in NonCounted; wrappers are mutually exclusive", + )), other => Ok(Element::NonCounted(Box::new(other))), } } @@ -425,7 +432,8 @@ impl Element { /// Only the four sum-tree variants are accepted: `SumTree`, `BigSumTree`, /// `CountSumTree`, `ProvableCountSumTree`. Any other element — including /// items, sum items, references, non-sum trees, and any wrapper - /// (`NonCounted`, `NotSummed`) — is rejected with `InvalidInput`. + /// (`NonCounted`, `NotSummed`, `NotCountedOrSummed`) — is rejected with + /// `InvalidInput`. pub fn new_not_summed(inner: Element) -> Result { match inner { Element::SumTree(..) @@ -442,16 +450,62 @@ impl Element { /// Wrap `self` in `NotSummed`. If `self` is already `NotSummed`, returns /// it unchanged (idempotent on `NotSummed`). /// - /// Returns `InvalidInput` if `self` is `NonCounted` (the two wrappers - /// are mutually exclusive) or any non-sum-tree variant. Mirrors - /// [`Element::into_non_counted`]. + /// Returns `InvalidInput` if `self` is any other wrapper (the three + /// wrappers are mutually exclusive) or any non-sum-tree variant. + /// Mirrors [`Element::into_non_counted`]. pub fn into_not_summed(self) -> Result { match self { Element::NotSummed(_) => Ok(self), Element::NonCounted(_) => Err(ElementError::InvalidInput( "cannot wrap NonCounted in NotSummed; wrappers are mutually exclusive", )), + Element::NotCountedOrSummed(_) => Err(ElementError::InvalidInput( + "cannot wrap NotCountedOrSummed in NotSummed; wrappers are mutually exclusive", + )), other => Self::new_not_summed(other), } } + + /// Wrap a sum-tree variant in `NotCountedOrSummed` so it contributes 0 + /// to BOTH its parent's running sum AND its parent's count when + /// inserted. + /// + /// Only the four sum-tree variants are accepted: `SumTree`, `BigSumTree`, + /// `CountSumTree`, `ProvableCountSumTree`. Any other element — including + /// items, sum items, references, non-sum trees, and any wrapper + /// (`NonCounted`, `NotSummed`, `NotCountedOrSummed`) — is rejected with + /// `InvalidInput`. + /// + /// Note: at insert time the parent must be `CountSumTree` or + /// `ProvableCountSumTree`. The merk-layer insert guard enforces that. + pub fn new_not_counted_or_summed(inner: Element) -> Result { + match inner { + Element::SumTree(..) + | Element::BigSumTree(..) + | Element::CountSumTree(..) + | Element::ProvableCountSumTree(..) => Ok(Element::NotCountedOrSummed(Box::new(inner))), + _ => Err(ElementError::InvalidInput( + "NotCountedOrSummed inner element must be a sum-tree variant (SumTree, \ + BigSumTree, CountSumTree, or ProvableCountSumTree)", + )), + } + } + + /// Wrap `self` in `NotCountedOrSummed`. If `self` is already + /// `NotCountedOrSummed`, returns it unchanged (idempotent). + /// + /// Returns `InvalidInput` if `self` is any other wrapper (the three + /// wrappers are mutually exclusive) or any non-sum-tree variant. + pub fn into_not_counted_or_summed(self) -> Result { + match self { + Element::NotCountedOrSummed(_) => Ok(self), + Element::NonCounted(_) => Err(ElementError::InvalidInput( + "cannot wrap NonCounted in NotCountedOrSummed; wrappers are mutually exclusive", + )), + Element::NotSummed(_) => Err(ElementError::InvalidInput( + "cannot wrap NotSummed in NotCountedOrSummed; wrappers are mutually exclusive", + )), + other => Self::new_not_counted_or_summed(other), + } + } } diff --git a/grovedb-element/src/element/helpers.rs b/grovedb-element/src/element/helpers.rs index 8b68e80fc..63435c6b5 100644 --- a/grovedb-element/src/element/helpers.rs +++ b/grovedb-element/src/element/helpers.rs @@ -27,15 +27,37 @@ impl Element { matches!(self, Element::NotSummed(_)) } - /// Returns the wrapped element if `self` is a wrapper (`NonCounted` or - /// `NotSummed`), else `self`. Use this when you need to inspect the - /// actual element type and don't care whether it is wrapped. + /// Returns `true` if this element is wrapped in + /// `Element::NotCountedOrSummed`. The wrapper suppresses BOTH count + /// and sum propagation to the parent tree but leaves all other + /// behavior (storage, hashing, internal aggregation) unchanged. + pub fn is_not_counted_or_summed(&self) -> bool { + matches!(self, Element::NotCountedOrSummed(_)) + } + + /// Returns `true` if this element is wrapped in any of the wrapper + /// variants (`NonCounted`, `NotSummed`, `NotCountedOrSummed`). Useful + /// for paths that need to add the +1 wrapper-byte cost overhead + /// regardless of which wrapper is in use. + pub fn is_wrapped(&self) -> bool { + matches!( + self, + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) + ) + } + + /// Returns the wrapped element if `self` is any wrapper (`NonCounted`, + /// `NotSummed`, or `NotCountedOrSummed`), else `self`. Use this when + /// you need to inspect the actual element type and don't care whether + /// it is wrapped. /// /// Only unwraps one level — the constructors and (de)serializers reject /// any wrapper nesting, so a single unwrap is always sufficient. pub fn underlying(&self) -> &Element { match self { - Element::NonCounted(inner) | Element::NotSummed(inner) => inner, + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner, other => other, } } @@ -43,7 +65,9 @@ impl Element { /// Mutable variant of [`underlying`]. pub fn underlying_mut(&mut self) -> &mut Element { match self { - Element::NonCounted(inner) | Element::NotSummed(inner) => inner, + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner, other => other, } } @@ -51,7 +75,9 @@ impl Element { /// Owned variant of [`underlying`]. pub fn into_underlying(self) -> Element { match self { - Element::NonCounted(inner) | Element::NotSummed(inner) => *inner, + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => *inner, other => other, } } @@ -61,12 +87,12 @@ impl Element { /// /// `NonCounted` delegates to its inner element — sums still propagate /// when the wrapper is inserted into a sum-bearing parent. - /// `NotSummed` returns 0 — the wrapper's whole purpose is to contribute - /// nothing to the parent sum tree. + /// `NotSummed` and `NotCountedOrSummed` return 0 — the wrappers' + /// purpose is to contribute nothing to the parent sum tree. pub fn sum_value_or_default(&self) -> i64 { match self { Element::NonCounted(inner) => inner.sum_value_or_default(), - Element::NotSummed(_) => 0, + Element::NotSummed(_) | Element::NotCountedOrSummed(_) => 0, Element::SumItem(sum_value, _) | Element::ItemWithSumItem(_, sum_value, _) | Element::SumTree(_, sum_value, _) @@ -79,12 +105,12 @@ impl Element { /// Decoded the integer value in the CountTree element type, returns 1 for /// everything else. /// - /// `NonCounted` returns 0 — the wrapper's whole purpose is to contribute - /// nothing to the parent count tree. + /// `NonCounted` and `NotCountedOrSummed` return 0 — both wrappers + /// suppress the parent count contribution. /// `NotSummed` delegates to its inner — counts still propagate. pub fn count_value_or_default(&self) -> u64 { match self { - Element::NonCounted(_) => 0, + Element::NonCounted(_) | Element::NotCountedOrSummed(_) => 0, Element::NotSummed(inner) => inner.count_value_or_default(), Element::CountTree(_, count_value, _) | Element::CountSumTree(_, count_value, ..) @@ -101,10 +127,12 @@ impl Element { /// propagates. /// `NotSummed` returns `(inner_count, 0)` — sum is suppressed, count /// still propagates. + /// `NotCountedOrSummed` returns `(0, 0)` — both are suppressed. pub fn count_sum_value_or_default(&self) -> (u64, i64) { match self { Element::NonCounted(inner) => (0, inner.sum_value_or_default()), Element::NotSummed(inner) => (inner.count_value_or_default(), 0), + Element::NotCountedOrSummed(_) => (0, 0), Element::SumItem(sum_value, _) | Element::ItemWithSumItem(_, sum_value, _) | Element::SumTree(_, sum_value, _) => (1, *sum_value), @@ -120,11 +148,11 @@ impl Element { /// Decoded the integer value in the SumItem element type, returns 0 for /// everything else. `NonCounted` delegates to its inner. `NotSummed` - /// returns 0. + /// and `NotCountedOrSummed` return 0. pub fn big_sum_value_or_default(&self) -> i128 { match self { Element::NonCounted(inner) => inner.big_sum_value_or_default(), - Element::NotSummed(_) => 0, + Element::NotSummed(_) | Element::NotCountedOrSummed(_) => 0, Element::SumItem(sum_value, _) | Element::ItemWithSumItem(_, sum_value, _) | Element::SumTree(_, sum_value, _) @@ -394,7 +422,9 @@ impl Element { | Element::MmrTree(.., flags) | Element::BulkAppendTree(.., flags) | Element::DenseAppendOnlyFixedSizeTree(.., flags) => flags, - Element::NonCounted(inner) | Element::NotSummed(inner) => inner.get_flags(), + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.get_flags(), } } @@ -417,7 +447,9 @@ impl Element { | Element::MmrTree(.., flags) | Element::BulkAppendTree(.., flags) | Element::DenseAppendOnlyFixedSizeTree(.., flags) => flags, - Element::NonCounted(inner) | Element::NotSummed(inner) => inner.get_flags_owned(), + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.get_flags_owned(), } } @@ -440,7 +472,9 @@ impl Element { | Element::MmrTree(.., flags) | Element::BulkAppendTree(.., flags) | Element::DenseAppendOnlyFixedSizeTree(.., flags) => flags, - Element::NonCounted(inner) | Element::NotSummed(inner) => inner.get_flags_mut(), + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.get_flags_mut(), } } @@ -463,7 +497,9 @@ impl Element { | Element::MmrTree(.., flags) | Element::BulkAppendTree(.., flags) | Element::DenseAppendOnlyFixedSizeTree(.., flags) => *flags = new_flags, - Element::NonCounted(inner) | Element::NotSummed(inner) => inner.set_flags(new_flags), + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.set_flags(new_flags), } } diff --git a/grovedb-element/src/element/mod.rs b/grovedb-element/src/element/mod.rs index 6f6452df4..28c38704b 100644 --- a/grovedb-element/src/element/mod.rs +++ b/grovedb-element/src/element/mod.rs @@ -45,9 +45,11 @@ pub trait ElementCostSizeExtension { /// /// `serde::Deserialize` is implemented manually (under the `serde` feature) /// so it can enforce the same wrapper invariants as `Element::deserialize`: -/// `NonCounted` and `NotSummed` may not nest in any combination, and -/// `NotSummed` may only wrap a sum-tree variant. `serde::Serialize` is -/// derived; serialization of valid `Element` values is always safe. +/// `NonCounted`, `NotSummed`, and `NotCountedOrSummed` may not nest in any +/// combination, and the sum-bearing wrappers (`NotSummed`, +/// `NotCountedOrSummed`) may only wrap a sum-tree variant. +/// `serde::Serialize` is derived; serialization of valid `Element` values is +/// always safe. #[derive(Clone, Encode, Decode, PartialEq, Eq, Hash)] #[cfg_attr(not(feature = "visualize"), derive(Debug))] #[cfg_attr(feature = "serde", derive(serde::Serialize))] @@ -151,9 +153,31 @@ pub enum Element { /// Invariants (enforced at construction, serialization, and /// deserialization): /// - The inner element MUST be one of the four sum-tree variants above. - /// - A `NotSummed` may not wrap another `NotSummed`, a `NonCounted`, or - /// any non-tree element. + /// - A `NotSummed` may not wrap another `NotSummed`, a `NonCounted`, + /// `NotCountedOrSummed`, or any non-tree element. NotSummed(Box), + /// Not-counted-or-summed wrapper: contains a sum-bearing tree variant + /// (`SumTree`, `BigSumTree`, `CountSumTree`, `ProvableCountSumTree`) and + /// behaves identically to it for storage, hashing, and its own internal + /// aggregates, but contributes 0 to BOTH its parent's running sum AND + /// the parent's count when inserted. + /// + /// May only be inserted into count-AND-sum-bearing trees (`CountSumTree`, + /// `ProvableCountSumTree`) — the only tree types where suppressing both + /// axes is meaningful. Any other parent rejects the wrapper at insert. + /// + /// For `SumTree` / `BigSumTree` inners, this wrapper suppresses the + /// implicit count-of-one a subtree would contribute to a parent count + /// tree (it stores a sum internally, but counts as a single element). + /// For `CountSumTree` / `ProvableCountSumTree` inners, it suppresses + /// the explicit count value as well as the sum. + /// + /// Invariants (enforced at construction, serialization, and + /// deserialization): + /// - The inner element MUST be one of the four sum-tree variants above. + /// - A `NotCountedOrSummed` may not wrap any other wrapper or any + /// non-tree element. + NotCountedOrSummed(Box), } pub fn hex_to_ascii(hex_value: &[u8]) -> String { @@ -345,6 +369,9 @@ impl fmt::Display for Element { Element::NotSummed(inner) => { write!(f, "NotSummed({})", inner) } + Element::NotCountedOrSummed(inner) => { + write!(f, "NotCountedOrSummed({})", inner) + } } } } @@ -406,6 +433,17 @@ impl Element { // unreachable case. other => other, }, + Element::NotCountedOrSummed(inner) => match inner.element_type() { + ElementType::SumTree => ElementType::NotCountedOrSummedSumTree, + ElementType::BigSumTree => ElementType::NotCountedOrSummedBigSumTree, + ElementType::CountSumTree => ElementType::NotCountedOrSummedCountSumTree, + ElementType::ProvableCountSumTree => { + ElementType::NotCountedOrSummedProvableCountSumTree + } + // Inner is always one of the 4 sum-tree variants above — + // see comment on the NotSummed arm above. + other => other, + }, } } @@ -414,9 +452,11 @@ impl Element { } /// Verify the wrapper invariants for `self`: - /// - `NonCounted` and `NotSummed` may not nest in any combination. - /// - `NotSummed` may only wrap one of the four sum-tree variants - /// (`SumTree`, `BigSumTree`, `CountSumTree`, `ProvableCountSumTree`). + /// - `NonCounted`, `NotSummed`, and `NotCountedOrSummed` may not nest + /// in any combination. + /// - `NotSummed` and `NotCountedOrSummed` may only wrap one of the four + /// sum-tree variants (`SumTree`, `BigSumTree`, `CountSumTree`, + /// `ProvableCountSumTree`). /// /// Constructors and the `serialize`/`deserialize` paths already enforce /// these rules; this helper exists so external callers (most importantly @@ -427,7 +467,10 @@ impl Element { pub fn validate_wrapper_invariants(&self) -> Result<(), crate::error::ElementError> { match self { Element::NonCounted(inner) => { - if matches!(**inner, Element::NonCounted(_) | Element::NotSummed(_)) { + if matches!( + **inner, + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) + ) { return Err(crate::error::ElementError::InvalidInput( "NonCounted cannot wrap another wrapper", )); @@ -445,6 +488,18 @@ impl Element { )); } }, + Element::NotCountedOrSummed(inner) => match **inner { + Element::SumTree(..) + | Element::BigSumTree(..) + | Element::CountSumTree(..) + | Element::ProvableCountSumTree(..) => {} + _ => { + return Err(crate::error::ElementError::InvalidInput( + "NotCountedOrSummed inner element must be a sum-tree variant \ + (SumTree, BigSumTree, CountSumTree, or ProvableCountSumTree)", + )); + } + }, _ => {} } Ok(()) @@ -514,6 +569,7 @@ mod serde_impl { DenseAppendOnlyFixedSizeTree(u16, u8, Option), NonCounted(Box), NotSummed(Box), + NotCountedOrSummed(Box), } impl From for Element { @@ -544,6 +600,9 @@ mod serde_impl { ElementShadow::NotSummed(inner) => { Element::NotSummed(Box::new(Element::from(*inner))) } + ElementShadow::NotCountedOrSummed(inner) => { + Element::NotCountedOrSummed(Box::new(Element::from(*inner))) + } } } } @@ -573,7 +632,10 @@ mod serde_impl { element: &Element, ) -> Result<(), crate::error::ElementError> { element.validate_wrapper_invariants()?; - if let Element::NonCounted(inner) | Element::NotSummed(inner) = element { + if let Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) = element + { Self::check_recursive_wrapper_invariants(inner)?; } Ok(()) @@ -592,6 +654,9 @@ mod serde_impl { Element::SumTree(Some(b"r".to_vec()), 42, None), Element::new_non_counted(Element::Item(b"x".to_vec(), None)).unwrap(), Element::new_not_summed(Element::SumTree(None, 100, None)).unwrap(), + Element::new_not_counted_or_summed(Element::CountSumTree(None, 3, 100, None)) + .unwrap(), + Element::new_not_counted_or_summed(Element::SumTree(None, 50, None)).unwrap(), ]; for original in cases { let json = serde_json::to_string(&original).expect("serialize"); @@ -600,6 +665,52 @@ mod serde_impl { } } + /// `NotCountedOrSummed(NotCountedOrSummed(_))` and cross-nestings + /// involving the new wrapper must be rejected. + #[test] + fn serde_rejects_not_counted_or_summed_nesting() { + // Self-nest. + let payload = + r#"{"NotCountedOrSummed":{"NotCountedOrSummed":{"SumTree":[null,0,null]}}}"#; + assert!(serde_json::from_str::(payload).is_err()); + + // Cross with NonCounted (both directions). + let payload = r#"{"NotCountedOrSummed":{"NonCounted":{"SumTree":[null,0,null]}}}"#; + assert!(serde_json::from_str::(payload).is_err()); + let payload = r#"{"NonCounted":{"NotCountedOrSummed":{"SumTree":[null,0,null]}}}"#; + assert!(serde_json::from_str::(payload).is_err()); + + // Cross with NotSummed (both directions). + let payload = r#"{"NotCountedOrSummed":{"NotSummed":{"SumTree":[null,0,null]}}}"#; + assert!(serde_json::from_str::(payload).is_err()); + let payload = r#"{"NotSummed":{"NotCountedOrSummed":{"SumTree":[null,0,null]}}}"#; + assert!(serde_json::from_str::(payload).is_err()); + } + + /// `NotCountedOrSummed(non_sum_tree)` must be rejected for every + /// illegal inner type. + #[test] + fn serde_rejects_not_counted_or_summed_with_non_sum_tree_inner() { + let illegal_inners = [ + r#"{"Item":[[120],null]}"#, + r#"{"SumItem":[7,null]}"#, + r#"{"Tree":[null,null]}"#, + r#"{"CountTree":[null,0,null]}"#, + r#"{"ProvableCountTree":[null,0,null]}"#, + r#"{"MmrTree":[0,null]}"#, + ]; + for inner in illegal_inners { + let payload = format!(r#"{{"NotCountedOrSummed":{}}}"#, inner); + let result: Result = serde_json::from_str(&payload); + assert!( + result.is_err(), + "NotCountedOrSummed({}) must be rejected; got {:?}", + inner, + result + ); + } + } + /// A serde payload that constructs a nested `NonCounted(NonCounted)` /// must be rejected. #[test] diff --git a/grovedb-element/src/element/serialize.rs b/grovedb-element/src/element/serialize.rs index ef9fe1163..6621378e9 100644 --- a/grovedb-element/src/element/serialize.rs +++ b/grovedb-element/src/element/serialize.rs @@ -6,7 +6,10 @@ use grovedb_version::{check_grovedb_v0, version::GroveVersion}; use crate::{ element::Element, - element_type::{NON_COUNTED_WRAPPER_DISCRIMINANT, NOT_SUMMED_WRAPPER_DISCRIMINANT}, + element_type::{ + NON_COUNTED_WRAPPER_DISCRIMINANT, NOT_COUNTED_OR_SUMMED_WRAPPER_DISCRIMINANT, + NOT_SUMMED_WRAPPER_DISCRIMINANT, + }, error::ElementError, }; @@ -14,21 +17,25 @@ impl Element { /// Serializes self. Returns vector of u8s. /// /// Rejects: - /// - `NonCounted(NonCounted(_))` — `NonCounted` cannot nest. - /// - `NotSummed(NotSummed(_))`, `NotSummed(NonCounted(_))`, - /// `NonCounted(NotSummed(_))` — wrappers cannot cross-nest. - /// - `NotSummed(x)` where `x` is not one of the four sum-tree variants - /// (`SumTree`, `BigSumTree`, `CountSumTree`, `ProvableCountSumTree`). + /// - Any wrapper nesting in any combination — `NonCounted`, `NotSummed`, + /// and `NotCountedOrSummed` are mutually exclusive. + /// - `NotSummed(x)` / `NotCountedOrSummed(x)` where `x` is not one of + /// the four sum-tree variants (`SumTree`, `BigSumTree`, `CountSumTree`, + /// `ProvableCountSumTree`). /// - /// Constructed via `Element::new_non_counted` / `Element::new_not_summed` - /// these are impossible, but a caller could build them directly. + /// Constructed via the `new_non_counted` / `new_not_summed` / + /// `new_not_counted_or_summed` constructors these are impossible, but a + /// caller could build them directly. pub fn serialize(&self, grove_version: &GroveVersion) -> Result, ElementError> { check_grovedb_v0!( "Element::serialize", grove_version.grovedb_versions.element.serialize ); if let Element::NonCounted(inner) = self - && matches!(**inner, Element::NonCounted(_) | Element::NotSummed(_)) + && matches!( + **inner, + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) + ) { return Err(ElementError::CorruptedData( "NonCounted cannot wrap another wrapper".to_string(), @@ -47,6 +54,19 @@ impl Element { } } } + if let Element::NotCountedOrSummed(inner) = self { + match **inner { + Element::SumTree(..) + | Element::BigSumTree(..) + | Element::CountSumTree(..) + | Element::ProvableCountSumTree(..) => {} + _ => { + return Err(ElementError::CorruptedData( + "NotCountedOrSummed inner must be a sum-tree variant".to_string(), + )); + } + } + } let config = config::standard().with_big_endian().with_no_limit(); bincode::encode_to_vec(self, config) .map_err(|e| ElementError::CorruptedData(format!("unable to serialize element {}", e))) @@ -71,11 +91,9 @@ impl Element { /// overflow the stack) before any post-decode check could fire. The /// pre-check is O(1) — only the first two bytes matter. /// - /// The four rejected leading byte pairs are: - /// - `[15, 15, ..]` — `NonCounted(NonCounted(..))` - /// - `[15, 16, ..]` — `NonCounted(NotSummed(..))` - /// - `[16, 15, ..]` — `NotSummed(NonCounted(..))` - /// - `[16, 16, ..]` — `NotSummed(NotSummed(..))` + /// The rejected leading byte pairs are every ordered combination of + /// the three wrapper discriminants (15 NonCounted, 16 NotSummed, + /// 17 NotCountedOrSummed) — nine in total. pub fn deserialize(bytes: &[u8], grove_version: &GroveVersion) -> Result { check_grovedb_v0!( "Element::deserialize", @@ -84,16 +102,23 @@ impl Element { // Pre-check: if the wire starts with a wrapper discriminant, the // very next byte must NOT be ANY wrapper discriminant. This bounds // the recursion bincode will attempt. - match bytes { - [NON_COUNTED_WRAPPER_DISCRIMINANT, NON_COUNTED_WRAPPER_DISCRIMINANT, ..] - | [NON_COUNTED_WRAPPER_DISCRIMINANT, NOT_SUMMED_WRAPPER_DISCRIMINANT, ..] - | [NOT_SUMMED_WRAPPER_DISCRIMINANT, NON_COUNTED_WRAPPER_DISCRIMINANT, ..] - | [NOT_SUMMED_WRAPPER_DISCRIMINANT, NOT_SUMMED_WRAPPER_DISCRIMINANT, ..] => { - return Err(ElementError::CorruptedData( - "deserialized wrapper wrapping another wrapper".to_string(), - )); - } - _ => {} + if let [outer, inner, ..] = bytes + && matches!( + *outer, + NON_COUNTED_WRAPPER_DISCRIMINANT + | NOT_SUMMED_WRAPPER_DISCRIMINANT + | NOT_COUNTED_OR_SUMMED_WRAPPER_DISCRIMINANT + ) + && matches!( + *inner, + NON_COUNTED_WRAPPER_DISCRIMINANT + | NOT_SUMMED_WRAPPER_DISCRIMINANT + | NOT_COUNTED_OR_SUMMED_WRAPPER_DISCRIMINANT + ) + { + return Err(ElementError::CorruptedData( + "deserialized wrapper wrapping another wrapper".to_string(), + )); } let config = config::standard().with_big_endian().with_no_limit(); let elem: Element = bincode::decode_from_slice(bytes, config) @@ -105,7 +130,10 @@ impl Element { // bincode/discriminant changes that could let a nested wrapper // sneak past the pre-check). if let Element::NonCounted(inner) = &elem - && matches!(**inner, Element::NonCounted(_) | Element::NotSummed(_)) + && matches!( + **inner, + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) + ) { return Err(ElementError::CorruptedData( "deserialized NonCounted wrapping another wrapper".to_string(), @@ -124,6 +152,19 @@ impl Element { } } } + if let Element::NotCountedOrSummed(inner) = &elem { + match **inner { + Element::SumTree(..) + | Element::BigSumTree(..) + | Element::CountSumTree(..) + | Element::ProvableCountSumTree(..) => {} + _ => { + return Err(ElementError::CorruptedData( + "deserialized NotCountedOrSummed with non-sum-tree inner".to_string(), + )); + } + } + } Ok(elem) } } diff --git a/grovedb-element/src/element/visualize.rs b/grovedb-element/src/element/visualize.rs index f82f98701..26886154c 100644 --- a/grovedb-element/src/element/visualize.rs +++ b/grovedb-element/src/element/visualize.rs @@ -186,6 +186,11 @@ impl Visualize for Element { drawer = inner.visualize(drawer)?; drawer.write(b")")?; } + Element::NotCountedOrSummed(inner) => { + drawer.write(b"not_counted_or_summed(")?; + drawer = inner.visualize(drawer)?; + drawer.write(b")")?; + } } Ok(drawer) } diff --git a/grovedb-element/src/element_type.rs b/grovedb-element/src/element_type.rs index 1019cf4ce..f1cbd9b9f 100644 --- a/grovedb-element/src/element_type.rs +++ b/grovedb-element/src/element_type.rs @@ -42,6 +42,28 @@ pub const NOT_SUMMED_TWIN_PREFIX: u8 = 0xb0; /// low nibble is sufficient. pub const NOT_SUMMED_BASE_MASK: u8 = 0x0F; +/// Bincode discriminant byte for `Element::NotCountedOrSummed`. Tied to the +/// declaration order of the `Element` enum (0-indexed, 18th variant). +/// +/// Like the other two wrapper discriminants, this byte has no direct +/// `ElementType` variant. `from_serialized_value` reads the next byte and +/// resolves to one of the four `NotCountedOrSummedXxx` synthetic twins. Only +/// the four sum-tree base discriminants are legal as the inner byte. +pub const NOT_COUNTED_OR_SUMMED_WRAPPER_DISCRIMINANT: u8 = 17; + +/// Twin-discriminant prefix for `NotCountedOrSummedXxx` types: every twin is +/// encoded as `NOT_COUNTED_OR_SUMMED_TWIN_PREFIX | base`. The prefix has the +/// high bit set (so all wrappers cluster in `0x80..` range) plus bit 6, which +/// distinguishes it from `NON_COUNTED_FLAG`'s `0x80` upper-nibble and +/// `NOT_SUMMED_TWIN_PREFIX`'s `0xb0` upper-nibble. Detection is therefore +/// an upper-nibble compare: `disc & 0xf0 == 0xc0`. +pub const NOT_COUNTED_OR_SUMMED_TWIN_PREFIX: u8 = 0xc0; + +/// Mask to recover the base type discriminant from a +/// `NotCountedOrSummedXxx` discriminant. Base discriminants are `0..=14` +/// (4 bits) so masking the low nibble is sufficient. +pub const NOT_COUNTED_OR_SUMMED_BASE_MASK: u8 = 0x0F; + /// Indicates which type of proof node should be used when generating proofs. /// /// This determines whether the verifier will recompute the value hash (secure) @@ -131,10 +153,14 @@ pub enum ProofNodeType { /// Not-summed twins follow the same scheme but use the prefix `0xb0` and only /// cover the four sum-tree base discriminants (4, 5, 7, 10), placing them at /// `180, 181, 183, 186`. The wrapper byte is `NOT_SUMMED_WRAPPER_DISCRIMINANT` -/// (16). Both wrapper twin ranges have bit 7 set, so all wrappers cluster in -/// `0x80..`, and the upper nibble distinguishes them: `0x80` for `NonCounted`, -/// `0xb0` for `NotSummed`. The two wrappers are mutually exclusive — the -/// constructors and (de)serializers reject any nesting in either direction. +/// (16). Not-counted-or-summed twins use the prefix `0xc0` over the same four +/// sum-tree base discriminants, placing them at `196, 197, 199, 202`. The +/// wrapper byte is `NOT_COUNTED_OR_SUMMED_WRAPPER_DISCRIMINANT` (17). All +/// three wrapper twin ranges have bit 7 set, so wrappers cluster in `0x80..`, +/// and the upper nibble distinguishes them: `0x80` for `NonCounted`, `0xb0` +/// for `NotSummed`, `0xc0` for `NotCountedOrSummed`. The three wrappers are +/// mutually exclusive — the constructors and (de)serializers reject any +/// nesting in either direction. /// /// IMPORTANT: Base values (0..=14) must match the order of variants in the /// `Element` enum. The `test_element_serialization_discriminants_match_element_type` @@ -212,6 +238,14 @@ pub enum ElementType { NotSummedCountSumTree = 183, /// Not-summed wrapper around `ProvableCountSumTree` - discriminant 186 (`0xb0 | 10`) NotSummedProvableCountSumTree = 186, + /// Not-counted-or-summed wrapper around `SumTree` - discriminant 196 (`0xc0 | 4`) + NotCountedOrSummedSumTree = 196, + /// Not-counted-or-summed wrapper around `BigSumTree` - discriminant 197 (`0xc0 | 5`) + NotCountedOrSummedBigSumTree = 197, + /// Not-counted-or-summed wrapper around `CountSumTree` - discriminant 199 (`0xc0 | 7`) + NotCountedOrSummedCountSumTree = 199, + /// Not-counted-or-summed wrapper around `ProvableCountSumTree` - discriminant 202 (`0xc0 | 10`) + NotCountedOrSummedProvableCountSumTree = 202, } impl ElementType { @@ -266,7 +300,7 @@ impl ElementType { ) })?; // Only the four sum-tree base discriminants are legal here. - // Anything else — including the wrapper bytes 15/16, the + // Anything else — including the wrapper bytes 15/16/17, the // synthetic twin ranges, and the unrelated base types — is // rejected so that round-tripping `from_serialized_value` always // yields a valid `NotSummedXxx` twin. @@ -278,6 +312,25 @@ impl ElementType { inner_byte ))), } + } else if first_byte == NOT_COUNTED_OR_SUMMED_WRAPPER_DISCRIMINANT { + let inner_byte = *serialized_value.get(1).ok_or_else(|| { + ElementError::CorruptedData( + "NotCountedOrSummed wrapper has no inner element discriminant byte".to_string(), + ) + })?; + // Same whitelist as NotSummed: only the four sum-bearing tree + // base discriminants are legal here. The new wrapper additionally + // suppresses the parent count contribution (the implicit +1 for + // SumTree/BigSumTree, or the explicit count for CountSumTree / + // ProvableCountSumTree). + match inner_byte { + 4 | 5 | 7 | 10 => Self::try_from(NOT_COUNTED_OR_SUMMED_TWIN_PREFIX | inner_byte), + _ => Err(ElementError::CorruptedData(format!( + "NotCountedOrSummed inner discriminant must be a sum-tree base type \ + (4=SumTree, 5=BigSumTree, 7=CountSumTree, 10=ProvableCountSumTree), got {}", + inner_byte + ))), + } } else { Self::try_from(first_byte) } @@ -296,13 +349,20 @@ impl ElementType { (self as u8) & 0xf0 == NOT_SUMMED_TWIN_PREFIX } + /// Returns true if this is a `NotCountedOrSummedXxx` discriminant. + #[inline] + pub const fn is_not_counted_or_summed(self) -> bool { + (self as u8) & 0xf0 == NOT_COUNTED_OR_SUMMED_TWIN_PREFIX + } + /// Returns the underlying base ElementType, stripping any wrapper flag /// bits. For base types, returns `self` unchanged. /// - /// The two wrapper twin ranges share bit 7 but are distinguished by the - /// upper nibble (`0x80` for `NonCounted`, `0xb0` for `NotSummed`). - /// Constructors and (de)serializers reject any wrapper nesting, so only - /// one wrapper status is ever set on any valid `ElementType` instance. + /// The three wrapper twin ranges share bit 7 but are distinguished by + /// the upper nibble (`0x80` for `NonCounted`, `0xb0` for `NotSummed`, + /// `0xc0` for `NotCountedOrSummed`). Constructors and (de)serializers + /// reject any wrapper nesting, so only one wrapper status is ever set + /// on any valid `ElementType` instance. #[inline] pub fn base(self) -> ElementType { let disc = self as u8; @@ -317,6 +377,11 @@ impl ElementType { // sum-tree base discriminants {4, 5, 7, 10}. ElementType::try_from(disc & NOT_SUMMED_BASE_MASK) .expect("NotSummed twin always has a valid base") + } else if self.is_not_counted_or_summed() { + // Safe: every NotCountedOrSummedXxx is constructed from one of + // the four sum-tree base discriminants {4, 5, 7, 10}. + ElementType::try_from(disc & NOT_COUNTED_OR_SUMMED_BASE_MASK) + .expect("NotCountedOrSummed twin always has a valid base") } else { self } @@ -502,6 +567,12 @@ impl ElementType { ElementType::NotSummedBigSumTree => "not_summed big sum tree", ElementType::NotSummedCountSumTree => "not_summed count sum tree", ElementType::NotSummedProvableCountSumTree => "not_summed provable count sum tree", + ElementType::NotCountedOrSummedSumTree => "not_counted_or_summed sum tree", + ElementType::NotCountedOrSummedBigSumTree => "not_counted_or_summed big sum tree", + ElementType::NotCountedOrSummedCountSumTree => "not_counted_or_summed count sum tree", + ElementType::NotCountedOrSummedProvableCountSumTree => { + "not_counted_or_summed provable count sum tree" + } } } } @@ -552,6 +623,10 @@ impl TryFrom for ElementType { 181 => Ok(ElementType::NotSummedBigSumTree), 183 => Ok(ElementType::NotSummedCountSumTree), 186 => Ok(ElementType::NotSummedProvableCountSumTree), + 196 => Ok(ElementType::NotCountedOrSummedSumTree), + 197 => Ok(ElementType::NotCountedOrSummedBigSumTree), + 199 => Ok(ElementType::NotCountedOrSummedCountSumTree), + 202 => Ok(ElementType::NotCountedOrSummedProvableCountSumTree), _ => Err(ElementError::CorruptedData(format!( "Unknown element type discriminant: {}", value @@ -667,8 +742,40 @@ mod tests { bad ); } - // Bytes past the highest NotSummed twin are invalid. + // Bytes between NotSummed and NotCountedOrSummed twin ranges are invalid. assert!(ElementType::try_from(187).is_err()); + assert!(ElementType::try_from(195).is_err()); + + // NotCountedOrSummed twins (0xc0 | base): only the four sum-tree + // bases {4, 5, 7, 10} are legal → discriminants {196, 197, 199, 202}. + assert_eq!( + ElementType::try_from(196).unwrap(), + ElementType::NotCountedOrSummedSumTree + ); + assert_eq!( + ElementType::try_from(197).unwrap(), + ElementType::NotCountedOrSummedBigSumTree + ); + assert_eq!( + ElementType::try_from(199).unwrap(), + ElementType::NotCountedOrSummedCountSumTree + ); + assert_eq!( + ElementType::try_from(202).unwrap(), + ElementType::NotCountedOrSummedProvableCountSumTree + ); + // Other bytes in 0xc0..=0xce (non-sum-tree bases) are invalid. + for bad in [ + 0xc0u8, 0xc1, 0xc2, 0xc3, 0xc6, 0xc8, 0xc9, 0xcb, 0xcc, 0xcd, 0xce, + ] { + assert!( + ElementType::try_from(bad).is_err(), + "{:#x} should be rejected", + bad + ); + } + // Bytes past the highest NotCountedOrSummed twin are invalid. + assert!(ElementType::try_from(203).is_err()); assert!(ElementType::try_from(255).is_err()); } @@ -681,11 +788,14 @@ mod tests { assert!(ElementType::NonCountedTree.is_non_counted()); assert!(ElementType::NonCountedDenseAppendOnlyFixedSizeTree.is_non_counted()); - // The two wrapper twin ranges share bit 7, but only NonCounted has - // upper-nibble 0x80. NotSummed (upper-nibble 0xb0) must NOT be - // counted as NonCounted. + // All three wrapper twin ranges share bit 7, but only NonCounted has + // upper-nibble 0x80. NotSummed (upper-nibble 0xb0) and + // NotCountedOrSummed (upper-nibble 0xc0) must NOT be counted as + // NonCounted. assert!(!ElementType::NotSummedSumTree.is_non_counted()); assert!(!ElementType::NotSummedProvableCountSumTree.is_non_counted()); + assert!(!ElementType::NotCountedOrSummedSumTree.is_non_counted()); + assert!(!ElementType::NotCountedOrSummedProvableCountSumTree.is_non_counted()); // base() strips the wrapper and returns the underlying type. assert_eq!(ElementType::Item.base(), ElementType::Item); @@ -713,6 +823,7 @@ mod tests { assert!(!ElementType::Item.is_not_summed()); assert!(!ElementType::SumTree.is_not_summed()); assert!(!ElementType::NonCountedSumTree.is_not_summed()); + assert!(!ElementType::NotCountedOrSummedSumTree.is_not_summed()); assert!(ElementType::NotSummedSumTree.is_not_summed()); assert!(ElementType::NotSummedBigSumTree.is_not_summed()); assert!(ElementType::NotSummedCountSumTree.is_not_summed()); @@ -744,6 +855,52 @@ mod tests { ); } + #[test] + fn test_not_counted_or_summed_helpers() { + // is_not_counted_or_summed: upper-nibble compare against 0xc0. + assert!(!ElementType::Item.is_not_counted_or_summed()); + assert!(!ElementType::SumTree.is_not_counted_or_summed()); + assert!(!ElementType::NonCountedSumTree.is_not_counted_or_summed()); + assert!(!ElementType::NotSummedSumTree.is_not_counted_or_summed()); + assert!(ElementType::NotCountedOrSummedSumTree.is_not_counted_or_summed()); + assert!(ElementType::NotCountedOrSummedBigSumTree.is_not_counted_or_summed()); + assert!(ElementType::NotCountedOrSummedCountSumTree.is_not_counted_or_summed()); + assert!(ElementType::NotCountedOrSummedProvableCountSumTree.is_not_counted_or_summed()); + + // The three wrapper twin ranges have distinct upper nibbles, so the + // three predicates are mutually exclusive on any single discriminant. + assert!(!ElementType::NotCountedOrSummedSumTree.is_non_counted()); + assert!(!ElementType::NotCountedOrSummedSumTree.is_not_summed()); + + // base() strips the wrapper and returns the underlying type. + assert_eq!( + ElementType::NotCountedOrSummedSumTree.base(), + ElementType::SumTree + ); + assert_eq!( + ElementType::NotCountedOrSummedBigSumTree.base(), + ElementType::BigSumTree + ); + assert_eq!( + ElementType::NotCountedOrSummedCountSumTree.base(), + ElementType::CountSumTree + ); + assert_eq!( + ElementType::NotCountedOrSummedProvableCountSumTree.base(), + ElementType::ProvableCountSumTree + ); + + // The discriminant relationship: twin = base | 0xc0. + assert_eq!( + ElementType::NotCountedOrSummedSumTree as u8, + ElementType::SumTree as u8 | NOT_COUNTED_OR_SUMMED_TWIN_PREFIX + ); + assert_eq!( + ElementType::NotCountedOrSummedProvableCountSumTree as u8, + ElementType::ProvableCountSumTree as u8 | NOT_COUNTED_OR_SUMMED_TWIN_PREFIX + ); + } + #[test] fn test_simple_vs_combined_hash() { // Items have simple hash @@ -1334,9 +1491,11 @@ mod tests { // All other inner bytes are rejected: non-sum-tree base types, // wrapper bytes, synthetic NonCounted twins (128..142), synthetic - // NotSummed twins (180..186), and unallocated ranges. + // NotSummed twins (180..186), NotCountedOrSummed twins (196..202), + // and unallocated ranges. for bad in [ - 0u8, 1, 2, 3, 6, 8, 9, 11, 12, 13, 14, 15, 16, 17, 100, 128, 142, 180, 186, 200, 255, + 0u8, 1, 2, 3, 6, 8, 9, 11, 12, 13, 14, 15, 16, 17, 100, 128, 142, 180, 186, 196, 202, + 255, ] { assert!( ElementType::from_serialized_value(&[16, bad]).is_err(), @@ -1345,4 +1504,43 @@ mod tests { ); } } + + /// Validate the resolver paths around byte 17 (NotCountedOrSummed + /// wrapper). Mirrors `test_from_serialized_value_not_summed_paths`. + #[test] + fn test_from_serialized_value_not_counted_or_summed_paths() { + // Truncated wrapper (no inner byte) is rejected. + assert!(ElementType::from_serialized_value(&[17]).is_err()); + + // Each of the four legal inner discriminants resolves to the right + // synthetic twin. + assert_eq!( + ElementType::from_serialized_value(&[17, 4]).unwrap(), + ElementType::NotCountedOrSummedSumTree + ); + assert_eq!( + ElementType::from_serialized_value(&[17, 5]).unwrap(), + ElementType::NotCountedOrSummedBigSumTree + ); + assert_eq!( + ElementType::from_serialized_value(&[17, 7]).unwrap(), + ElementType::NotCountedOrSummedCountSumTree + ); + assert_eq!( + ElementType::from_serialized_value(&[17, 10]).unwrap(), + ElementType::NotCountedOrSummedProvableCountSumTree + ); + + // All other inner bytes are rejected. + for bad in [ + 0u8, 1, 2, 3, 6, 8, 9, 11, 12, 13, 14, 15, 16, 17, 100, 128, 142, 180, 186, 196, 202, + 255, + ] { + assert!( + ElementType::from_serialized_value(&[17, bad]).is_err(), + "[17, {}] should be rejected", + bad + ); + } + } } diff --git a/grovedb/src/batch/mod.rs b/grovedb/src/batch/mod.rs index 3a2f8e7da..db7215239 100644 --- a/grovedb/src/batch/mod.rs +++ b/grovedb/src/batch/mod.rs @@ -289,6 +289,12 @@ pub enum GroveOp { /// the wrapper and the parent sum tree's running sum excludes /// the subtree. not_summed: bool, + /// True if the original element was wrapped in + /// `Element::NotCountedOrSummed`. Set during propagation; on + /// execution the reconstructed sum-bearing-tree element is + /// re-wrapped so the on-disk bytes preserve the wrapper, and the + /// parent's running count AND sum both exclude the subtree. + not_counted_or_summed: bool, }, /// **Internal only — do not construct directly.** /// Replace root hash for a non-Merk tree (CommitmentTree, MmrTree, @@ -1498,7 +1504,7 @@ where // underlying() unwraps a single level; the constructor and // (de)serializer reject nested wrappers, so these are // unreachable by construction. - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) => { unreachable!("wrappers may not nest") } } @@ -1652,7 +1658,9 @@ where .wrap_with_cost(cost) } // Wrappers are unwrapped via underlying() above. - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) + | Element::NotSummed(_) + | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } } @@ -1699,7 +1707,9 @@ where .wrap_with_cost(cost) } // Wrappers are unwrapped via underlying() above. - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) + | Element::NotSummed(_) + | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } }, @@ -1902,6 +1912,16 @@ where )) .wrap_with_cost(cost); } + if element.is_not_counted_or_summed() + && !in_tree_type.is_count_and_sum_bearing() + { + return Err(Error::InvalidBatchOperation( + "not-counted-or-summed elements may only be inserted into trees \ + that bear BOTH count and sum (CountSumTree or \ + ProvableCountSumTree)", + )) + .wrap_with_cost(cost); + } // Look through NonCounted; methods called on `element` // (serialize, get_feature_type, insert_*_into_batch_operations, // element_at_key_already_exists) are wrapper-aware via the @@ -2097,7 +2117,9 @@ where } } // Wrappers are unwrapped via underlying() above. - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) + | Element::NotSummed(_) + | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } } @@ -2271,6 +2293,7 @@ where aggregate_data, non_counted, not_summed, + not_counted_or_summed, } => { // Standard Merk trees — infer element from aggregate_data let element = match aggregate_data { @@ -2316,9 +2339,9 @@ where // Re-wrap if the original element was wrapped, so the // on-disk bytes preserve the wrapper and the parent's // aggregate excludes this subtree from the right - // dimension. The two flags are mutually exclusive — set - // only one during propagation. The `element` here is a - // freshly-constructed bare tree built from + // dimension. The three flags are mutually exclusive — + // set only one during propagation. The `element` here is + // a freshly-constructed bare tree built from // `aggregate_data` above, so the conditional wrappers // should never see a pre-existing wrapper input — but // surface a typed error rather than panic if the @@ -2337,6 +2360,13 @@ where during InsertTreeWithRootHash propagation", ) }) + } else if not_counted_or_summed { + element.into_not_counted_or_summed().map_err(|_| { + Error::CorruptedCodeExecution( + "into_not_counted_or_summed called on a non-sum-tree or \ + wrapped element during InsertTreeWithRootHash propagation", + ) + }) } else { Ok(element) }; @@ -2495,13 +2525,8 @@ where // path (the wrapper byte costs +1 over the // bare type, mirroring `wrapper_overhead` // in `merk/src/element/costs.rs`). - let wrapper_overhead = if new_element.is_non_counted() - || new_element.is_not_summed() - { - 1u32 - } else { - 0 - }; + let wrapper_overhead = + if new_element.is_wrapped() { 1u32 } else { 0 }; match new_element.underlying() { Element::Tree(..) | Element::SumTree(..) @@ -2734,11 +2759,14 @@ impl GroveDb { // wrapper byte would be silently dropped // from storage and the parent's aggregate // would include a value it should not. - // The two wrappers are mutually exclusive - // (constructors reject nesting), so at - // most one flag is true here. + // The three wrappers are mutually + // exclusive (constructors reject + // nesting), so at most one flag is true + // here. let non_counted = element.is_non_counted(); let not_summed = element.is_not_summed(); + let not_counted_or_summed = + element.is_not_counted_or_summed(); let element = element.underlying(); // Standard Merk trees if let Element::Tree(_, flags) = element { @@ -2751,6 +2779,7 @@ impl GroveDb { AggregateData::NoAggregateData, non_counted, not_summed, + not_counted_or_summed, } } else if let Element::SumTree(.., flags) = element @@ -2763,6 +2792,7 @@ impl GroveDb { aggregate_data, non_counted, not_summed, + not_counted_or_summed, } } else if let Element::BigSumTree(.., flags) = element @@ -2775,6 +2805,7 @@ impl GroveDb { aggregate_data, non_counted, not_summed, + not_counted_or_summed, } } else if let Element::CountTree(.., flags) = element @@ -2787,6 +2818,7 @@ impl GroveDb { aggregate_data, non_counted, not_summed, + not_counted_or_summed, } } else if let Element::CountSumTree(.., flags) = element @@ -2799,6 +2831,7 @@ impl GroveDb { aggregate_data, non_counted, not_summed, + not_counted_or_summed, } } else if let Element::ProvableCountTree( .., @@ -2813,6 +2846,7 @@ impl GroveDb { aggregate_data, non_counted, not_summed, + not_counted_or_summed, } } else if let Element::ProvableCountSumTree( .., @@ -2827,10 +2861,12 @@ impl GroveDb { aggregate_data, non_counted, not_summed, + not_counted_or_summed, } // Non-Merk trees → InsertNonMerkTree - // (none of these can be NotSummed — - // they aren't sum-tree variants.) + // (none of these can be NotSummed or + // NotCountedOrSummed — they aren't + // sum-tree variants.) } else if let Element::CommitmentTree( total_count, chunk_power, diff --git a/grovedb/src/debugger.rs b/grovedb/src/debugger.rs index 1c82b3c2e..d1204131a 100644 --- a/grovedb/src/debugger.rs +++ b/grovedb/src/debugger.rs @@ -813,9 +813,9 @@ fn element_to_grovedbg(element: crate::Element) -> grovedbg_types::Element { } // The visualizer wire format has no wrapper variants; render the // inner element. The wrapper is invisible at the debug-UI layer. - crate::Element::NonCounted(inner) | crate::Element::NotSummed(inner) => { - element_to_grovedbg(*inner) - } + crate::Element::NonCounted(inner) + | crate::Element::NotSummed(inner) + | crate::Element::NotCountedOrSummed(inner) => element_to_grovedbg(*inner), } } diff --git a/grovedb/src/estimated_costs/average_case_costs.rs b/grovedb/src/estimated_costs/average_case_costs.rs index 1230429f7..9982657b4 100644 --- a/grovedb/src/estimated_costs/average_case_costs.rs +++ b/grovedb/src/estimated_costs/average_case_costs.rs @@ -329,11 +329,7 @@ impl GroveDb { // Look through wrapper variants for cost dispatch (the wrapper byte // is accounted for via `wrapper_overhead` parallel to // `merk/src/element/costs.rs`). - let wrapper_overhead = if value.is_non_counted() || value.is_not_summed() { - 1u32 - } else { - 0 - }; + let wrapper_overhead = if value.is_wrapped() { 1u32 } else { 0 }; match value.underlying() { Element::Tree(_, flags) | Element::SumTree(_, _, flags) diff --git a/grovedb/src/estimated_costs/worst_case_costs.rs b/grovedb/src/estimated_costs/worst_case_costs.rs index ea72cae7b..5f189aa43 100644 --- a/grovedb/src/estimated_costs/worst_case_costs.rs +++ b/grovedb/src/estimated_costs/worst_case_costs.rs @@ -187,11 +187,7 @@ impl GroveDb { let mut cost = OperationCost::default(); let key_len = key.max_length() as u32; // Look through wrapper variants for cost dispatch. - let wrapper_overhead = if value.is_non_counted() || value.is_not_summed() { - 1u32 - } else { - 0 - }; + let wrapper_overhead = if value.is_wrapped() { 1u32 } else { 0 }; match value.underlying() { Element::Tree(_, flags) | Element::SumTree(_, _, flags) @@ -248,11 +244,7 @@ impl GroveDb { let mut cost = OperationCost::default(); let key_len = key.max_length() as u32; // Look through wrapper variants for cost dispatch. - let wrapper_overhead = if value.is_non_counted() || value.is_not_summed() { - 1u32 - } else { - 0 - }; + let wrapper_overhead = if value.is_wrapped() { 1u32 } else { 0 }; match value.underlying() { Element::Tree(_, flags) | Element::SumTree(_, _, flags) => { let flags_len = flags.as_ref().map_or(0, |flags| { diff --git a/grovedb/src/lib.rs b/grovedb/src/lib.rs index 127503a5e..911b07677 100644 --- a/grovedb/src/lib.rs +++ b/grovedb/src/lib.rs @@ -1110,7 +1110,7 @@ impl GroveDb { ); } } - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } } diff --git a/grovedb/src/operations/get/query.rs b/grovedb/src/operations/get/query.rs index 45a23a18e..363379375 100644 --- a/grovedb/src/operations/get/query.rs +++ b/grovedb/src/operations/get/query.rs @@ -274,7 +274,7 @@ where { | Element::DenseAppendOnlyFixedSizeTree(..) => { Err(Error::InvalidQuery("path_queries can not refer to trees")) } - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } } @@ -410,7 +410,9 @@ where { | Element::DenseAppendOnlyFixedSizeTree(..) => Err(Error::InvalidQuery( "path_queries can only refer to items and references", )), - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) + | Element::NotSummed(_) + | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } } @@ -565,7 +567,9 @@ where { "path_queries can only refer to items, sum items, references and sum \ trees", )), - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) + | Element::NotSummed(_) + | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } } @@ -1006,7 +1010,9 @@ where { "path_queries over sum items can only refer to sum items and \ references", )), - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) + | Element::NotSummed(_) + | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } } diff --git a/grovedb/src/operations/insert/mod.rs b/grovedb/src/operations/insert/mod.rs index dd53c6370..b63a0fe74 100644 --- a/grovedb/src/operations/insert/mod.rs +++ b/grovedb/src/operations/insert/mod.rs @@ -351,7 +351,7 @@ impl GroveDb { // forbidden by the constructor and (de)serializer, but the public // insert path can still receive a hand-built nested wrapper — // return a typed error rather than panic. - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) => { return Err(Error::InvalidInput( "nested element wrappers are not allowed", )) diff --git a/grovedb/src/operations/proof/generate.rs b/grovedb/src/operations/proof/generate.rs index 935af6772..386143093 100644 --- a/grovedb/src/operations/proof/generate.rs +++ b/grovedb/src/operations/proof/generate.rs @@ -591,7 +591,9 @@ impl GroveDb { | Ok(Element::BulkAppendTree(..)) | Ok(Element::DenseAppendOnlyFixedSizeTree(..)) => continue, // NonCounted is unwrapped above via into_underlying(). - Ok(Element::NonCounted(_)) | Ok(Element::NotSummed(_)) => { + Ok(Element::NonCounted(_)) + | Ok(Element::NotSummed(_)) + | Ok(Element::NotCountedOrSummed(_)) => { unreachable!("unwrapped above") } Err(e) => { @@ -1501,7 +1503,9 @@ impl GroveDb { | Ok(Element::BulkAppendTree(..)) | Ok(Element::DenseAppendOnlyFixedSizeTree(..)) => continue, // NonCounted is unwrapped above via into_underlying(). - Ok(Element::NonCounted(_)) | Ok(Element::NotSummed(_)) => { + Ok(Element::NonCounted(_)) + | Ok(Element::NotSummed(_)) + | Ok(Element::NotCountedOrSummed(_)) => { unreachable!("unwrapped above") } Err(e) => { diff --git a/grovedb/src/operations/proof/verify.rs b/grovedb/src/operations/proof/verify.rs index 60ab08d35..fb90757b1 100644 --- a/grovedb/src/operations/proof/verify.rs +++ b/grovedb/src/operations/proof/verify.rs @@ -643,7 +643,9 @@ impl GroveDb { "V1 proof has lower layer for a non-tree element.".to_string(), )); } - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) + | Element::NotSummed(_) + | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } } @@ -1620,7 +1622,9 @@ impl GroveDb { "Proof has lower layer for a non Tree.".to_string(), )); } - Element::NonCounted(_) | Element::NotSummed(_) => { + Element::NonCounted(_) + | Element::NotSummed(_) + | Element::NotCountedOrSummed(_) => { unreachable!("unwrapped above") } } diff --git a/grovedb/src/tests/batch_coverage_tests.rs b/grovedb/src/tests/batch_coverage_tests.rs index 2ecb90bc7..7da11cc2d 100644 --- a/grovedb/src/tests/batch_coverage_tests.rs +++ b/grovedb/src/tests/batch_coverage_tests.rs @@ -542,6 +542,7 @@ mod tests { non_counted: false, not_summed: false, + not_counted_or_summed: false, }, }; diff --git a/grovedb/src/tests/batch_rejection_tests.rs b/grovedb/src/tests/batch_rejection_tests.rs index b9cf042d8..9573af504 100644 --- a/grovedb/src/tests/batch_rejection_tests.rs +++ b/grovedb/src/tests/batch_rejection_tests.rs @@ -77,6 +77,7 @@ fn test_apply_batch_rejects_insert_tree_with_root_hash() { non_counted: false, not_summed: false, + not_counted_or_summed: false, }, }; diff --git a/grovedb/src/tests/batch_unit_tests.rs b/grovedb/src/tests/batch_unit_tests.rs index 7a61e47f3..8b2a94ce3 100644 --- a/grovedb/src/tests/batch_unit_tests.rs +++ b/grovedb/src/tests/batch_unit_tests.rs @@ -96,6 +96,7 @@ mod tests { non_counted: false, not_summed: false, + not_counted_or_summed: false, }, GroveOp::ReplaceTreeRootKey { // 4 @@ -475,6 +476,7 @@ mod tests { non_counted: false, not_summed: false, + not_counted_or_summed: false, }, }; let dbg = format!("{:?}", internal_op2); diff --git a/grovedb/src/tests/mod.rs b/grovedb/src/tests/mod.rs index e6cd67617..1e59c7e2e 100644 --- a/grovedb/src/tests/mod.rs +++ b/grovedb/src/tests/mod.rs @@ -29,6 +29,7 @@ mod is_empty_tree_tests; mod misc_coverage_tests; mod mmr_tree_tests; mod non_counted_tests; +mod not_counted_or_summed_tests; mod not_summed_tests; mod operations_coverage_tests; mod partial_batch_consistency_tests; diff --git a/grovedb/src/tests/not_counted_or_summed_tests.rs b/grovedb/src/tests/not_counted_or_summed_tests.rs new file mode 100644 index 000000000..e3af8fae1 --- /dev/null +++ b/grovedb/src/tests/not_counted_or_summed_tests.rs @@ -0,0 +1,444 @@ +//! Regression tests for `Element::NotCountedOrSummed` end-to-end behavior. +//! +//! Symmetric to `not_summed_tests.rs` / `non_counted_tests.rs`. The wrapper: +//! - May only be inserted into trees that bear BOTH a count and a sum +//! (`CountSumTree`, `ProvableCountSumTree`). +//! - Inner element must be one of the four sum-tree variants (`SumTree`, +//! `BigSumTree`, `CountSumTree`, `ProvableCountSumTree`). +//! - Contributes 0 to BOTH the parent's running sum AND its count. Internal +//! aggregates of the wrapped tree itself remain intact. + +#[cfg(test)] +mod tests { + use grovedb_storage::StorageBatch; + use grovedb_version::version::GroveVersion; + + use crate::{ + batch::QualifiedGroveDbOp, + tests::{make_test_grovedb, TEST_LEAF}, + Element, + }; + + /// Establish a count-sum tree under `TEST_LEAF/` for use as a host + /// parent in the tests below. + fn make_count_sum_tree_parent(db: &crate::GroveDb, key: &[u8], grove_version: &GroveVersion) { + db.insert( + [TEST_LEAF].as_ref(), + key, + Element::empty_count_sum_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert count sum tree"); + } + + /// Establish a provable count-sum tree under `TEST_LEAF/`. + fn make_provable_count_sum_tree_parent( + db: &crate::GroveDb, + key: &[u8], + grove_version: &GroveVersion, + ) { + db.insert( + [TEST_LEAF].as_ref(), + key, + Element::empty_provable_count_sum_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert provable count sum tree"); + } + + #[test] + fn batch_insert_rejects_not_counted_or_summed_into_normal_tree() { + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + // TEST_LEAF is a normal tree; the wrapper must be rejected there. + let w = Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let op = + QualifiedGroveDbOp::insert_or_replace_op(vec![TEST_LEAF.to_vec()], b"k".to_vec(), w); + + let err = db + .apply_batch(vec![op], None, None, grove_version) + .unwrap() + .expect_err("batch insert into NormalTree must fail"); + let msg = format!("{err:?}"); + assert!( + msg.contains("not-counted-or-summed") || msg.contains("not_counted_or_summed"), + "expected NotCountedOrSummed parent-type guard error, got: {msg}" + ); + } + + #[test] + fn batch_insert_rejects_not_counted_or_summed_into_count_tree() { + // CountTree bears a count but no sum, so the wrapper has nothing to + // suppress on the sum axis. The parent-type guard must reject it. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + db.insert( + [TEST_LEAF].as_ref(), + b"ct", + Element::empty_count_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert ct"); + + let w = Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let op = QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"ct".to_vec()], + b"k".to_vec(), + w, + ); + + let err = db + .apply_batch(vec![op], None, None, grove_version) + .unwrap() + .expect_err("batch insert into CountTree must fail"); + let msg = format!("{err:?}"); + assert!( + msg.contains("not-counted-or-summed") || msg.contains("not_counted_or_summed"), + "expected NotCountedOrSummed parent-type guard error, got: {msg}" + ); + } + + #[test] + fn batch_insert_rejects_not_counted_or_summed_into_sum_tree() { + // SumTree bears a sum but no count, so the wrapper has nothing to + // suppress on the count axis. The parent-type guard must reject it. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + db.insert( + [TEST_LEAF].as_ref(), + b"st", + Element::empty_sum_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert st"); + + let w = Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let op = QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"st".to_vec()], + b"k".to_vec(), + w, + ); + + let err = db + .apply_batch(vec![op], None, None, grove_version) + .unwrap() + .expect_err("batch insert into SumTree must fail"); + let msg = format!("{err:?}"); + assert!( + msg.contains("not-counted-or-summed") || msg.contains("not_counted_or_summed"), + "expected NotCountedOrSummed parent-type guard error, got: {msg}" + ); + } + + #[test] + fn direct_insert_not_counted_or_summed_in_count_sum_tree_excludes_both_axes() { + // A bare SumTree inside a CountSumTree contributes (1, internal_sum) + // to the parent's (count, sum). Wrapped in NotCountedOrSummed, it + // must contribute (0, 0). + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + make_count_sum_tree_parent(&db, b"outer", grove_version); + + // A plain sum item contributes (1, 5). + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"plain", + Element::new_sum_item(5), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert plain"); + + // A NotCountedOrSummed(SumTree(_, 100)) must contribute (0, 0). + let inner = Element::new_sum_tree_with_flags_and_sum_value(None, 0, None); + let wrapped = Element::new_not_counted_or_summed(inner).expect("wrap ok"); + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"w", + wrapped, + None, + None, + grove_version, + ) + .unwrap() + .expect("insert wrapped sum tree"); + + // Add a sum item under the wrapped subtree to give it a non-trivial + // internal sum that must NOT bubble up. + db.insert( + [TEST_LEAF, b"outer", b"w"].as_ref(), + b"inner_item", + Element::new_sum_item(99), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert inner sum item"); + + // The outer count-sum tree's aggregate must be (count=1, sum=5). + let batch = StorageBatch::new(); + let tx = db.start_transaction(); + let outer_merk = db + .open_transactional_merk_at_path( + [TEST_LEAF, b"outer"].as_ref().into(), + &tx, + Some(&batch), + grove_version, + ) + .unwrap() + .expect("open outer merk"); + let aggregate = outer_merk + .aggregate_data() + .expect("read outer aggregate data"); + assert_eq!( + aggregate.as_count_u64(), + 1, + "wrapped subtree must not contribute to outer count; got {:?}", + aggregate + ); + assert_eq!( + aggregate.as_sum_i64(), + 5, + "wrapped subtree must not contribute to outer sum; got {:?}", + aggregate + ); + } + + #[test] + fn direct_insert_not_counted_or_summed_in_provable_count_sum_tree_excludes_both_axes() { + // Same scenario but with a ProvableCountSumTree parent. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + make_provable_count_sum_tree_parent(&db, b"outer", grove_version); + + // A plain item contributes (1, 0). + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"plain", + Element::new_item(b"x".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert plain item"); + + // NotCountedOrSummed(ProvableCountSumTree(_, 3, 100)) contributes + // (0, 0) even though the inner aggregates are non-trivial. + let inner = Element::new_provable_count_sum_tree_with_flags_and_sum_and_count_value( + None, 3, 100, None, + ); + let wrapped = Element::new_not_counted_or_summed(inner).expect("wrap ok"); + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"w", + wrapped, + None, + None, + grove_version, + ) + .unwrap() + .expect("insert wrapped provable count sum tree"); + + // Aggregate must be (count=1, sum=0): only `plain` contributes the + // count; the wrapped tree's count=3 and sum=100 are both suppressed. + let batch = StorageBatch::new(); + let tx = db.start_transaction(); + let outer_merk = db + .open_transactional_merk_at_path( + [TEST_LEAF, b"outer"].as_ref().into(), + &tx, + Some(&batch), + grove_version, + ) + .unwrap() + .expect("open outer merk"); + let aggregate = outer_merk + .aggregate_data() + .expect("read outer aggregate data"); + assert_eq!(aggregate.as_count_u64(), 1); + assert_eq!(aggregate.as_sum_i64(), 0); + } + + #[test] + fn batch_propagation_preserves_not_counted_or_summed_wrapper_on_subtree() { + // A batch that inserts a NotCountedOrSummed(SumTree) AND writes a + // child under it forces propagation through InsertTreeWithRootHash. + // The on-disk parent element must come back wrapped, and the outer + // tree's count and sum must both exclude the subtree. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + make_count_sum_tree_parent(&db, b"outer", grove_version); + + // Plain sum item contributes (1, 5). + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"plain", + Element::new_sum_item(5), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert plain"); + + // Batch: insert NotCountedOrSummed(SumTree) under outer + a sum-item + // child under that wrapped tree, forcing the propagation path. + let wrapped = + Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let inner_child = Element::new_sum_item(123); + let ops = vec![ + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"outer".to_vec()], + b"w".to_vec(), + wrapped, + ), + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"outer".to_vec(), b"w".to_vec()], + b"child".to_vec(), + inner_child, + ), + ]; + db.apply_batch(ops, None, None, grove_version) + .unwrap() + .expect("batch should succeed"); + + // The element stored at outer/w must STILL be NotCountedOrSummed + // after propagation. If it were dropped, the parent's count would + // be 2 (instead of 1) and sum 128 (instead of 5). + let stored = db + .get_raw( + grovedb_path::SubtreePath::from(&[TEST_LEAF, b"outer"]), + b"w", + None, + grove_version, + ) + .unwrap() + .expect("get_raw w"); + assert!( + matches!(stored, Element::NotCountedOrSummed(_)), + "wrapper must survive batch propagation; got {:?}", + stored + ); + + // Outer aggregate must NOT include the wrapped subtree's count (1) + // or sum (123). Only `plain` contributes (1, 5). + let batch = StorageBatch::new(); + let tx = db.start_transaction(); + let outer_merk = db + .open_transactional_merk_at_path( + [TEST_LEAF, b"outer"].as_ref().into(), + &tx, + Some(&batch), + grove_version, + ) + .unwrap() + .expect("open outer merk"); + let aggregate = outer_merk + .aggregate_data() + .expect("read outer aggregate data"); + assert_eq!( + aggregate.as_count_u64(), + 1, + "wrapped subtree must not contribute to outer count; got {:?}", + aggregate + ); + assert_eq!( + aggregate.as_sum_i64(), + 5, + "wrapped subtree must not contribute to outer sum; got {:?}", + aggregate + ); + } + + #[test] + fn check_subtree_exists_through_not_counted_or_summed_wrapper() { + // A NotCountedOrSummed-wrapped tree at the parent path must satisfy + // check_subtree_exists, otherwise APIs that gate on it would reject + // paths through wrapped parents. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + make_count_sum_tree_parent(&db, b"outer", grove_version); + + // NotCountedOrSummed(SumTree) inside the outer count-sum tree. + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"w", + Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert wrapped sum tree"); + + // Inserting into the wrapped subtree exercises check_subtree_exists + // on a path whose parent is `NotCountedOrSummed(SumTree)` — should + // succeed. + db.insert( + [TEST_LEAF, b"outer", b"w"].as_ref(), + b"child", + Element::new_sum_item(42), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert into wrapped subtree must succeed"); + } + + #[test] + fn constructor_invariants() { + // Only the four sum-bearing tree variants are accepted as inner. + assert!(Element::new_not_counted_or_summed(Element::new_sum_tree(None)).is_ok()); + assert!(Element::new_not_counted_or_summed(Element::new_big_sum_tree(None)).is_ok()); + assert!(Element::new_not_counted_or_summed(Element::new_count_sum_tree(None)).is_ok()); + assert!( + Element::new_not_counted_or_summed(Element::new_provable_count_sum_tree(None)).is_ok() + ); + + // Non-sum-tree inners are rejected. + assert!(Element::new_not_counted_or_summed(Element::new_item(b"x".to_vec())).is_err()); + assert!(Element::new_not_counted_or_summed(Element::new_sum_item(7)).is_err()); + assert!(Element::new_not_counted_or_summed(Element::new_tree(None)).is_err()); + assert!(Element::new_not_counted_or_summed(Element::new_count_tree(None)).is_err()); + assert!( + Element::new_not_counted_or_summed(Element::new_provable_count_tree(None)).is_err() + ); + + // Wrappers cannot nest in any direction. + let nc = Element::new_non_counted(Element::new_sum_tree(None)).expect("nc ok"); + assert!(Element::new_not_counted_or_summed(nc).is_err()); + let ns = Element::new_not_summed(Element::new_sum_tree(None)).expect("ns ok"); + assert!(Element::new_not_counted_or_summed(ns).is_err()); + let ncos = + Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("ncos ok"); + assert!(Element::new_not_counted_or_summed(ncos.clone()).is_err()); + + // And the other wrappers reject NotCountedOrSummed-as-inner. + assert!(Element::new_non_counted(ncos.clone()).is_err()); + assert!(Element::new_not_summed(ncos).is_err()); + } +} diff --git a/merk/src/element/costs.rs b/merk/src/element/costs.rs index b30efa021..053547ebd 100644 --- a/merk/src/element/costs.rs +++ b/merk/src/element/costs.rs @@ -51,9 +51,9 @@ pub trait ElementCostPrivateExtensions { } impl ElementCostPrivateExtensions for Element { - /// Get tree cost for the element. For `NonCounted` and `NotSummed`, - /// delegates to the inner element and adds 1 byte for the wrapper - /// discriminant. + /// Get tree cost for the element. For `NonCounted`, `NotSummed`, and + /// `NotCountedOrSummed`, delegates to the inner element and adds 1 + /// byte for the wrapper discriminant. fn get_specialized_cost(&self, grove_version: &GroveVersion) -> Result { check_grovedb_v0!( "get_specialized_cost", @@ -72,7 +72,9 @@ impl ElementCostPrivateExtensions for Element { Element::CountSumTree(..) => Ok(COUNT_SUM_TREE_COST_SIZE), Element::ProvableCountTree(..) => Ok(COUNT_TREE_COST_SIZE), Element::ProvableCountSumTree(..) => Ok(COUNT_SUM_TREE_COST_SIZE), - Element::NonCounted(inner) | Element::NotSummed(inner) => { + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => { Ok(inner.get_specialized_cost(grove_version)? + 1) } _ => Err(Error::CorruptedCodeExecution( @@ -106,7 +108,9 @@ impl ElementCostExtensions for Element { // that use cost-size constants, we add 1 byte of `wrapper_overhead` // to keep the on-disk byte count exact. let (element, wrapper_overhead) = match element { - Element::NonCounted(inner) | Element::NotSummed(inner) => (*inner, 1u32), + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => (*inner, 1u32), other => (other, 0u32), }; let cost = match element { diff --git a/merk/src/element/get.rs b/merk/src/element/get.rs index 10e372539..7e480907f 100644 --- a/merk/src/element/get.rs +++ b/merk/src/element/get.rs @@ -416,13 +416,7 @@ impl ElementFetchFromStoragePrivateExtensions for Element { // accounting exact. let wrapper_overhead = element .as_ref() - .map(|e| { - if e.is_non_counted() || e.is_not_summed() { - 1u32 - } else { - 0 - } - }) + .map(|e| if e.is_wrapped() { 1u32 } else { 0 }) .unwrap_or(0); let element_for_cost = element.as_ref().map(|e| e.underlying()); match element_for_cost { @@ -486,7 +480,9 @@ impl ElementFetchFromStoragePrivateExtensions for Element { // Wrappers are unwrapped above; reaching these arms means // the inner type wasn't one of the explicit arms (impossible given // exhaustiveness above). - Some(Element::NonCounted(_)) | Some(Element::NotSummed(_)) => {} + Some(Element::NonCounted(_)) + | Some(Element::NotSummed(_)) + | Some(Element::NotCountedOrSummed(_)) => {} None => {} } Ok(element).wrap_with_cost(cost) @@ -532,11 +528,7 @@ impl ElementFetchFromStoragePrivateExtensions for Element { // Look through wrapper variants for cost computation; see V0 path // above for rationale. Capture the wrapper byte before unwrapping so // the tree- and sum-item arms can include it in value_len. - let wrapper_overhead = if element.is_non_counted() || element.is_not_summed() { - 1u32 - } else { - 0 - }; + let wrapper_overhead = if element.is_wrapped() { 1u32 } else { 0 }; let element_for_cost = element.underlying(); match element_for_cost { Element::Item(..) | Element::Reference(..) => { @@ -601,7 +593,7 @@ impl ElementFetchFromStoragePrivateExtensions for Element { ) as u64 } // Wrappers are unwrapped above. - Element::NonCounted(_) | Element::NotSummed(_) => {} + Element::NonCounted(_) | Element::NotSummed(_) | Element::NotCountedOrSummed(_) => {} } Ok(Some(element)).wrap_with_cost(cost) } diff --git a/merk/src/element/insert.rs b/merk/src/element/insert.rs index ab738b94a..1b6ecea10 100644 --- a/merk/src/element/insert.rs +++ b/merk/src/element/insert.rs @@ -177,6 +177,14 @@ impl ElementInsertToStorageExtensions for Element { .wrap_with_cost(Default::default()); } + if self.is_not_counted_or_summed() && !merk.tree_type.is_count_and_sum_bearing() { + return Err(Error::InvalidInputError( + "not-counted-or-summed elements may only be inserted into trees that bear \ + BOTH count and sum (CountSumTree or ProvableCountSumTree)", + )) + .wrap_with_cost(Default::default()); + } + if !merk.tree_type.allows_sum_item() && self.is_sum_item() { return Err(Error::InvalidInputError( "cannot add sum item to non sum tree", @@ -455,6 +463,14 @@ impl ElementInsertToStorageExtensions for Element { .wrap_with_cost(Default::default()); } + if self.is_not_counted_or_summed() && !merk.tree_type.is_count_and_sum_bearing() { + return Err(Error::InvalidInputError( + "not-counted-or-summed elements may only be inserted into trees that bear \ + BOTH count and sum (CountSumTree or ProvableCountSumTree)", + )) + .wrap_with_cost(Default::default()); + } + let serialized = match self.serialize(grove_version) { Ok(s) => s, Err(e) => return Err(e.into()).wrap_with_cost(Default::default()), @@ -554,6 +570,14 @@ impl ElementInsertToStorageExtensions for Element { .wrap_with_cost(Default::default()); } + if self.is_not_counted_or_summed() && !merk.tree_type.is_count_and_sum_bearing() { + return Err(Error::InvalidInputError( + "not-counted-or-summed elements may only be inserted into trees that bear \ + BOTH count and sum (CountSumTree or ProvableCountSumTree)", + )) + .wrap_with_cost(Default::default()); + } + let serialized = match self.serialize(grove_version) { Ok(s) => s, Err(e) => return Err(e.into()).wrap_with_cost(Default::default()), @@ -924,6 +948,78 @@ mod tests { ); } + #[test] + fn not_counted_or_summed_rejected_in_normal_tree() { + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::NormalTree); + let w = Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let result = w + .insert_subtree(&mut merk, b"k", [0u8; 32], None, grove_version) + .unwrap(); + assert!(matches!(result, Err(Error::InvalidInputError(_)))); + } + + #[test] + fn not_counted_or_summed_rejected_in_sum_tree() { + // SumTree bears only a sum, not a count, so suppressing both axes + // has no meaning. The guard must reject. + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::SumTree); + let w = Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let result = w + .insert_subtree(&mut merk, b"k", [0u8; 32], None, grove_version) + .unwrap(); + assert!(matches!(result, Err(Error::InvalidInputError(_)))); + } + + #[test] + fn not_counted_or_summed_rejected_in_count_tree() { + // CountTree bears only a count, not a sum, so suppressing both axes + // has no meaning. The guard must reject. + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::CountTree); + let w = Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let result = w + .insert_subtree(&mut merk, b"k", [0u8; 32], None, grove_version) + .unwrap(); + assert!(matches!(result, Err(Error::InvalidInputError(_)))); + } + + #[test] + fn not_counted_or_summed_in_count_sum_tree_excludes_both_axes() { + // A NotCountedOrSummed(SumTree(_, 100)) inside a CountSumTree must + // contribute (count=0, sum=0). One bare sum item contributes (1, 7). + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::CountSumTree); + + Element::new_sum_item(7) + .insert(&mut merk, b"k1", None, grove_version) + .unwrap() + .expect("insert sum item"); + + let w = Element::new_not_counted_or_summed(Element::new_sum_tree_with_flags_and_sum_value( + None, 100, None, + )) + .expect("wrap ok"); + w.insert_subtree(&mut merk, b"k2", [0u8; 32], None, grove_version) + .unwrap() + .expect("insert wrapped sum tree"); + + let agg = merk.aggregate_data().expect("aggregate ok"); + assert_eq!( + agg.as_count_u64(), + 1, + "wrapped sum tree must not be counted; got {:?}", + agg + ); + assert_eq!( + agg.as_sum_i64(), + 7, + "wrapped sum tree's 100 must not propagate; got {:?}", + agg + ); + } + #[test] fn not_summed_in_provable_count_sum_tree_keeps_count_drops_sum() { // A NotSummed(SumTree(_, 100, _)) inside a ProvableCountSumTree diff --git a/merk/src/element/reconstruct.rs b/merk/src/element/reconstruct.rs index 2c99ff986..1e0d2ca0e 100644 --- a/merk/src/element/reconstruct.rs +++ b/merk/src/element/reconstruct.rs @@ -78,6 +78,9 @@ impl ElementReconstructExtensions for Element { Element::NotSummed(inner) => inner .reconstruct_with_root_key(maybe_root_key, aggregate_data) .map(|reconstructed| Element::NotSummed(Box::new(reconstructed))), + Element::NotCountedOrSummed(inner) => inner + .reconstruct_with_root_key(maybe_root_key, aggregate_data) + .map(|reconstructed| Element::NotCountedOrSummed(Box::new(reconstructed))), _ => None, } } diff --git a/merk/src/element/tree_type.rs b/merk/src/element/tree_type.rs index 235e2d3ac..6212facf3 100644 --- a/merk/src/element/tree_type.rs +++ b/merk/src/element/tree_type.rs @@ -60,9 +60,9 @@ impl ElementTreeTypeExtensions for Element { Element::DenseAppendOnlyFixedSizeTree(_, height, _) => { Some((None, TreeType::DenseAppendOnlyFixedSizeTree(height))) } - Element::NonCounted(inner) | Element::NotSummed(inner) => { - inner.root_key_and_tree_type_owned() - } + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.root_key_and_tree_type_owned(), _ => None, } } @@ -95,9 +95,9 @@ impl ElementTreeTypeExtensions for Element { &NONE_ROOT_KEY, TreeType::DenseAppendOnlyFixedSizeTree(*height), )), - Element::NonCounted(inner) | Element::NotSummed(inner) => { - inner.root_key_and_tree_type() - } + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.root_key_and_tree_type(), _ => None, } } @@ -125,7 +125,9 @@ impl ElementTreeTypeExtensions for Element { Element::DenseAppendOnlyFixedSizeTree(_, height, flags) => { Some((flags, TreeType::DenseAppendOnlyFixedSizeTree(*height))) } - Element::NonCounted(inner) | Element::NotSummed(inner) => inner.tree_flags_and_type(), + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.tree_flags_and_type(), _ => None, } } @@ -151,7 +153,9 @@ impl ElementTreeTypeExtensions for Element { Element::DenseAppendOnlyFixedSizeTree(_, height, _) => { Some(TreeType::DenseAppendOnlyFixedSizeTree(*height)) } - Element::NonCounted(inner) | Element::NotSummed(inner) => inner.tree_type(), + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.tree_type(), _ => None, } } @@ -175,7 +179,9 @@ impl ElementTreeTypeExtensions for Element { Element::MmrTree(..) => Some(BasicMerkNode), Element::BulkAppendTree(..) => Some(BasicMerkNode), Element::DenseAppendOnlyFixedSizeTree(..) => Some(BasicMerkNode), - Element::NonCounted(inner) | Element::NotSummed(inner) => inner.tree_feature_type(), + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.tree_feature_type(), _ => None, } } @@ -201,7 +207,9 @@ impl ElementTreeTypeExtensions for Element { Element::DenseAppendOnlyFixedSizeTree(_, height, _) => { MaybeTree::Tree(TreeType::DenseAppendOnlyFixedSizeTree(*height)) } - Element::NonCounted(inner) | Element::NotSummed(inner) => inner.maybe_tree_type(), + Element::NonCounted(inner) + | Element::NotSummed(inner) + | Element::NotCountedOrSummed(inner) => inner.maybe_tree_type(), _ => MaybeTree::NotTree, } } @@ -209,11 +217,13 @@ impl ElementTreeTypeExtensions for Element { /// Get the tree feature type. /// /// `count_value_or_default` and `count_sum_value_or_default` already - /// return 0 (resp. (0, inner_sum)) for `Element::NonCounted`, and + /// return 0 (resp. (0, inner_sum)) for `Element::NonCounted`, /// `sum_value_or_default` / `big_sum_value_or_default` / /// `count_sum_value_or_default` already return 0 (resp. (inner_count, 0)) - /// for `Element::NotSummed`. So the existing dispatch produces the right - /// feature type for either wrapper without an explicit branch here. + /// for `Element::NotSummed`, and all four helpers return 0 (resp. + /// (0, 0)) for `Element::NotCountedOrSummed`. So the existing dispatch + /// produces the right feature type for every wrapper without an + /// explicit branch here. fn get_feature_type(&self, parent_tree_type: TreeType) -> Result { match parent_tree_type { TreeType::NormalTree => Ok(BasicMerkNode), diff --git a/merk/src/tree_type/mod.rs b/merk/src/tree_type/mod.rs index 9e1c80cc0..5154f393d 100644 --- a/merk/src/tree_type/mod.rs +++ b/merk/src/tree_type/mod.rs @@ -149,6 +149,19 @@ impl TreeType { ) } + /// Returns whether this tree type carries BOTH a count and a sum + /// aggregate. Only these tree types may host + /// `Element::NotCountedOrSummed` children — in any other parent the + /// wrapper would suppress an aggregate the parent doesn't track, so it + /// is rejected at insert time. Equivalent to `is_count_bearing() && + /// is_sum_bearing()`. + pub const fn is_count_and_sum_bearing(&self) -> bool { + matches!( + self, + TreeType::CountSumTree | TreeType::ProvableCountSumTree + ) + } + /// Returns whether this tree type allows sum items as children. pub fn allows_sum_item(&self) -> bool { match self { @@ -331,6 +344,44 @@ mod tests { assert!(!TreeType::DenseAppendOnlyFixedSizeTree(0).is_sum_bearing()); } + #[test] + fn is_count_and_sum_bearing() { + assert!(!TreeType::NormalTree.is_count_and_sum_bearing()); + assert!(!TreeType::SumTree.is_count_and_sum_bearing()); + assert!(!TreeType::BigSumTree.is_count_and_sum_bearing()); + assert!(!TreeType::CountTree.is_count_and_sum_bearing()); + assert!(TreeType::CountSumTree.is_count_and_sum_bearing()); + assert!(!TreeType::ProvableCountTree.is_count_and_sum_bearing()); + assert!(TreeType::ProvableCountSumTree.is_count_and_sum_bearing()); + assert!(!TreeType::CommitmentTree(0).is_count_and_sum_bearing()); + assert!(!TreeType::MmrTree.is_count_and_sum_bearing()); + assert!(!TreeType::BulkAppendTree(0).is_count_and_sum_bearing()); + assert!(!TreeType::DenseAppendOnlyFixedSizeTree(0).is_count_and_sum_bearing()); + + // Equivalence: is_count_and_sum_bearing iff both is_count_bearing + // and is_sum_bearing. + for tt in [ + TreeType::NormalTree, + TreeType::SumTree, + TreeType::BigSumTree, + TreeType::CountTree, + TreeType::CountSumTree, + TreeType::ProvableCountTree, + TreeType::ProvableCountSumTree, + TreeType::CommitmentTree(0), + TreeType::MmrTree, + TreeType::BulkAppendTree(0), + TreeType::DenseAppendOnlyFixedSizeTree(0), + ] { + assert_eq!( + tt.is_count_and_sum_bearing(), + tt.is_count_bearing() && tt.is_sum_bearing(), + "mismatch for {:?}", + tt + ); + } + } + #[test] fn allows_sum_item() { assert!(!TreeType::NormalTree.allows_sum_item()); From 290641acd84b3ca23101a4bb733b0dde61705fea Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 16 May 2026 20:11:26 +0700 Subject: [PATCH 2/3] test: raise NotCountedOrSummed patch coverage above 90% MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codecov/patch flagged 65 missing lines on #666 (83.29%). Adds focused tests for every new wrapper arm so each touchpoint is exercised: - grovedb-element/src/element/mod.rs: Display, element_type twin mapping for all four sum-tree inners, validate_wrapper_invariants on every arm (NonCounted / NotSummed / NotCountedOrSummed and their cross-nesting + non-sum-tree-inner failure modes). - grovedb-element/src/element/helpers.rs: full `not_counted_or_summed_tests` module — constructor accept/reject, idempotent `into_…`, cross-wrapper rejection in `into_non_counted` / `into_not_summed`, `is_*` and `is_wrapped` predicates, `underlying{,_mut,into_}`, flag accessors and `set_flags`, all four aggregation helpers zero out for the new wrapper, bincode round-trip, deserialize rejection of every cross-wrapper combo plus the long-chain pre-check, and serialize+deserialize rejection of non-sum-tree inners. - merk/src/element/tree_type.rs: ElementTreeTypeExtensions delegation through the new wrapper for all four sum-tree inners, plus `get_feature_type` zeroing both axes in CountSumTree and ProvableCountSumTree parents. - merk/src/element/reconstruct.rs: wrapper survives root-key propagation (matches the existing NonCounted / NotSummed tests). - merk/src/element/insert.rs: guards fire on the `insert` and `insert_reference` entry points (not just `insert_subtree`), ProvableCountSumTree parent acceptance with both axes suppressed, and a storage round-trip that exercises the v1 read path and the +1 wrapper-overhead accounting. - grovedb/src/tests/not_counted_or_summed_tests.rs: batch propagation through a ProvableCountSumTree parent — mirrors the existing CountSumTree batch test and exercises the second propagation arm. 431 merk lib tests pass, 100 grovedb-element tests pass, 1553 grovedb lib tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- grovedb-element/src/element/helpers.rs | 257 ++++++++++++++++++ grovedb-element/src/element/mod.rs | 69 +++++ .../src/tests/not_counted_or_summed_tests.rs | 76 ++++++ merk/src/element/insert.rs | 87 ++++++ merk/src/element/reconstruct.rs | 20 ++ merk/src/element/tree_type.rs | 75 +++++ 6 files changed, 584 insertions(+) diff --git a/grovedb-element/src/element/helpers.rs b/grovedb-element/src/element/helpers.rs index 63435c6b5..249efb01b 100644 --- a/grovedb-element/src/element/helpers.rs +++ b/grovedb-element/src/element/helpers.rs @@ -885,3 +885,260 @@ mod not_summed_tests { assert!(bad.serialize(grove_version).is_err()); } } + +#[cfg(test)] +mod not_counted_or_summed_tests { + use grovedb_version::version::GroveVersion; + + use crate::element::Element; + + #[test] + fn new_not_counted_or_summed_wraps_sum_tree_variants() { + // All four sum-tree variants are accepted. + for inner in [ + Element::new_sum_tree(None), + Element::new_big_sum_tree(None), + Element::new_count_sum_tree(None), + Element::new_provable_count_sum_tree(None), + ] { + let wrapped = Element::new_not_counted_or_summed(inner.clone()).expect("wrap ok"); + assert!(wrapped.is_not_counted_or_summed()); + assert!(wrapped.is_wrapped()); + assert_eq!(wrapped.underlying(), &inner); + } + } + + #[test] + fn new_not_counted_or_summed_rejects_non_sum_tree_inner() { + // Items, sum items, references, and non-sum trees are all rejected. + assert!(Element::new_not_counted_or_summed(Element::new_item(b"x".to_vec())).is_err()); + assert!(Element::new_not_counted_or_summed(Element::new_sum_item(7)).is_err()); + assert!(Element::new_not_counted_or_summed(Element::new_tree(None)).is_err()); + assert!(Element::new_not_counted_or_summed(Element::new_count_tree(None)).is_err()); + assert!( + Element::new_not_counted_or_summed(Element::new_provable_count_tree(None)).is_err() + ); + + // Wrappers cannot nest in either direction. + let nc = Element::new_non_counted(Element::new_sum_tree(None)).expect("nc ok"); + assert!(Element::new_not_counted_or_summed(nc).is_err()); + let ns = Element::new_not_summed(Element::new_sum_tree(None)).expect("ns ok"); + assert!(Element::new_not_counted_or_summed(ns).is_err()); + let ncos = + Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("ncos ok"); + assert!(Element::new_not_counted_or_summed(ncos).is_err()); + } + + #[test] + fn into_not_counted_or_summed_is_idempotent_and_rejects_other_wrappers() { + // Idempotent on NotCountedOrSummed. + let ncos = + Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let twice = ncos + .clone() + .into_not_counted_or_summed() + .expect("rewrap ok"); + assert_eq!(ncos, twice); + + // Rejects NonCounted/NotSummed. + let nc = Element::new_non_counted(Element::new_sum_tree(None)).expect("nc ok"); + assert!(nc.into_not_counted_or_summed().is_err()); + let ns = Element::new_not_summed(Element::new_sum_tree(None)).expect("ns ok"); + assert!(ns.into_not_counted_or_summed().is_err()); + + // Rejects non-sum-tree inners (delegates to new_not_counted_or_summed). + assert!(Element::new_item(b"x".to_vec()) + .into_not_counted_or_summed() + .is_err()); + + // Wraps bare sum tree. + let wrapped = Element::new_sum_tree(None) + .into_not_counted_or_summed() + .expect("wrap ok"); + assert!(wrapped.is_not_counted_or_summed()); + } + + #[test] + fn into_non_counted_and_into_not_summed_reject_not_counted_or_summed_inner() { + // The cross-wrapper rejection arms in into_non_counted / + // into_not_summed must fire for NotCountedOrSummed. + let ncos = + Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("ncos ok"); + assert!(ncos.clone().into_non_counted().is_err()); + assert!(ncos.into_not_summed().is_err()); + } + + #[test] + fn predicates_look_through_wrapper() { + let cst = Element::new_count_sum_tree_with_flags_and_sum_and_count_value( + Some(b"r".to_vec()), + 7, + 100, + None, + ); + let ncos = Element::new_not_counted_or_summed(cst).expect("wrap ok"); + assert!(ncos.is_any_tree()); + assert!(ncos.is_non_empty_tree()); + assert!(ncos.is_non_empty_merk_tree()); + assert!(!ncos.is_any_item()); + assert!(!ncos.is_basic_tree()); + assert!(!ncos.is_sum_tree()); // inner is CountSumTree, not SumTree + } + + #[test] + fn sum_and_count_helpers_zero_for_not_counted_or_summed() { + // Inner CountSumTree(_, 5, 100) — both axes suppressed when wrapped. + let cst = + Element::new_count_sum_tree_with_flags_and_sum_and_count_value(None, 5, 100, None); + assert_eq!(cst.count_value_or_default(), 5); + assert_eq!(cst.sum_value_or_default(), 100); + assert_eq!(cst.big_sum_value_or_default(), 100); + assert_eq!(cst.count_sum_value_or_default(), (5, 100)); + + let ncos = Element::new_not_counted_or_summed(cst).expect("wrap ok"); + assert_eq!(ncos.count_value_or_default(), 0); + assert_eq!(ncos.sum_value_or_default(), 0); + assert_eq!(ncos.big_sum_value_or_default(), 0); + assert_eq!(ncos.count_sum_value_or_default(), (0, 0)); + } + + #[test] + fn implicit_count_one_suppressed_for_sum_tree_inner() { + // A bare SumTree contributes count=1 to a count-bearing parent by + // default. NotCountedOrSummed must suppress that to 0. + let st = Element::new_sum_tree_with_flags_and_sum_value(None, 50, None); + assert_eq!(st.count_value_or_default(), 1); + assert_eq!(st.sum_value_or_default(), 50); + + let ncos = Element::new_not_counted_or_summed(st).expect("wrap ok"); + assert_eq!(ncos.count_value_or_default(), 0); + assert_eq!(ncos.sum_value_or_default(), 0); + assert_eq!(ncos.count_sum_value_or_default(), (0, 0)); + } + + #[test] + fn flags_delegate_through_wrapper() { + let flags = Some(vec![1, 2, 3]); + let st = Element::new_sum_tree_with_flags(None, flags.clone()); + let mut ncos = Element::new_not_counted_or_summed(st).expect("wrap ok"); + assert_eq!(ncos.get_flags(), &flags); + // get_flags_mut allows mutation through the wrapper. + *ncos.get_flags_mut() = Some(vec![9]); + assert_eq!(ncos.get_flags(), &Some(vec![9])); + // set_flags routes through the wrapper. + ncos.set_flags(Some(vec![1, 2])); + assert_eq!(ncos.get_flags(), &Some(vec![1, 2])); + // get_flags_owned consumes self. + assert_eq!(ncos.get_flags_owned(), Some(vec![1, 2])); + } + + #[test] + fn underlying_methods_unwrap_one_level() { + let inner = Element::SumTree(Some(b"r".to_vec()), 42, None); + let ncos = Element::new_not_counted_or_summed(inner.clone()).expect("wrap ok"); + + // Borrow. + assert_eq!(ncos.underlying(), &inner); + // Owned. + assert_eq!(ncos.clone().into_underlying(), inner); + // Mutable. + let mut ncos_mut = ncos; + let m = ncos_mut.underlying_mut(); + assert!(matches!(m, Element::SumTree(..))); + } + + #[test] + fn bincode_round_trip_through_wrapper() { + let grove_version = GroveVersion::latest(); + let inner = Element::new_provable_count_sum_tree_with_flags_and_sum_and_count_value( + Some(b"root".to_vec()), + 3, + 42, + Some(vec![9, 8]), + ); + let wrapped = Element::new_not_counted_or_summed(inner).expect("wrap ok"); + let bytes = wrapped.serialize(grove_version).expect("serialize ok"); + let back = Element::deserialize(&bytes, grove_version).expect("deserialize ok"); + assert_eq!(back, wrapped); + } + + #[test] + fn deserialize_rejects_nested_or_cross_wrappers() { + // serialize() also rejects, but bincode-encode directly to test the + // *deserialize* pre-check and post-check paths. + use bincode::config; + let cfg = config::standard().with_big_endian().with_no_limit(); + let grove_version = GroveVersion::latest(); + + // NotCountedOrSummed(NotCountedOrSummed(SumTree)) + let bad = Element::NotCountedOrSummed(Box::new(Element::NotCountedOrSummed(Box::new( + Element::SumTree(None, 0, None), + )))); + let bytes = bincode::encode_to_vec(&bad, cfg).expect("encode"); + assert!(Element::deserialize(&bytes, grove_version).is_err()); + + // NotCountedOrSummed(NonCounted(SumTree)) + let bad = Element::NotCountedOrSummed(Box::new(Element::NonCounted(Box::new( + Element::SumTree(None, 0, None), + )))); + let bytes = bincode::encode_to_vec(&bad, cfg).expect("encode"); + assert!(Element::deserialize(&bytes, grove_version).is_err()); + + // NotCountedOrSummed(NotSummed(SumTree)) + let bad = Element::NotCountedOrSummed(Box::new(Element::NotSummed(Box::new( + Element::SumTree(None, 0, None), + )))); + let bytes = bincode::encode_to_vec(&bad, cfg).expect("encode"); + assert!(Element::deserialize(&bytes, grove_version).is_err()); + + // NonCounted(NotCountedOrSummed(SumTree)) + let bad = Element::NonCounted(Box::new(Element::NotCountedOrSummed(Box::new( + Element::SumTree(None, 0, None), + )))); + let bytes = bincode::encode_to_vec(&bad, cfg).expect("encode"); + assert!(Element::deserialize(&bytes, grove_version).is_err()); + + // NotSummed(NotCountedOrSummed(SumTree)) + let bad = Element::NotSummed(Box::new(Element::NotCountedOrSummed(Box::new( + Element::SumTree(None, 0, None), + )))); + let bytes = bincode::encode_to_vec(&bad, cfg).expect("encode"); + assert!(Element::deserialize(&bytes, grove_version).is_err()); + } + + #[test] + fn deserialize_rejects_not_counted_or_summed_with_non_sum_tree_inner() { + // A NotCountedOrSummed wrapping a plain Item must be rejected at + // deserialize, exercising the post-check arm. + use bincode::config; + let cfg = config::standard().with_big_endian().with_no_limit(); + let bad = Element::NotCountedOrSummed(Box::new(Element::Item(b"x".to_vec(), None))); + let bytes = bincode::encode_to_vec(&bad, cfg).expect("encode"); + let grove_version = GroveVersion::latest(); + assert!(Element::deserialize(&bytes, grove_version).is_err()); + } + + #[test] + fn serialize_rejects_not_counted_or_summed_with_non_sum_tree_inner() { + let bad = Element::NotCountedOrSummed(Box::new(Element::Item(b"x".to_vec(), None))); + let grove_version = GroveVersion::latest(); + assert!(bad.serialize(grove_version).is_err()); + } + + #[test] + fn deserialize_rejects_long_nested_wrapper_chain_without_recursion() { + let grove_version = GroveVersion::latest(); + // 1024 NotCountedOrSummed wrapper bytes followed by a valid SumTree. + // Pre-check stops on byte 1. + let mut bytes = vec![17u8; 1024]; + bytes.extend_from_slice(&[4, 0, 0, 0]); + let err = Element::deserialize(&bytes, grove_version) + .expect_err("nested wrapper bytes must be rejected"); + let msg = format!("{:?}", err); + assert!( + msg.contains("wrapper") || msg.contains("Wrapper"), + "error should mention wrapper: {}", + msg + ); + } +} diff --git a/grovedb-element/src/element/mod.rs b/grovedb-element/src/element/mod.rs index 28c38704b..48f0b9739 100644 --- a/grovedb-element/src/element/mod.rs +++ b/grovedb-element/src/element/mod.rs @@ -863,4 +863,73 @@ mod tests { let s = format!("{}", wrapped); assert!(s.starts_with("NonCounted("), "got: {}", s); } + + #[test] + fn display_renders_not_counted_or_summed_wrapper() { + let inner = Element::CountSumTree(Some(b"r".to_vec()), 3, 100, None); + let wrapped = Element::new_not_counted_or_summed(inner).expect("wrap ok"); + let s = format!("{}", wrapped); + assert!(s.starts_with("NotCountedOrSummed("), "got: {}", s); + assert!(s.contains("CountSumTree"), "got: {}", s); + } + + #[test] + fn element_type_resolves_not_counted_or_summed_twins() { + // Each of the four sum-tree variants maps to its + // NotCountedOrSummed twin. + let cases: [(Element, ElementType); 4] = [ + ( + Element::NotCountedOrSummed(Box::new(Element::SumTree(None, 0, None))), + ElementType::NotCountedOrSummedSumTree, + ), + ( + Element::NotCountedOrSummed(Box::new(Element::BigSumTree(None, 0, None))), + ElementType::NotCountedOrSummedBigSumTree, + ), + ( + Element::NotCountedOrSummed(Box::new(Element::CountSumTree(None, 0, 0, None))), + ElementType::NotCountedOrSummedCountSumTree, + ), + ( + Element::NotCountedOrSummed(Box::new(Element::ProvableCountSumTree( + None, 0, 0, None, + ))), + ElementType::NotCountedOrSummedProvableCountSumTree, + ), + ]; + for (element, expected) in cases { + assert_eq!(element.element_type(), expected); + assert_eq!(element.type_str(), expected.as_str()); + } + } + + #[test] + fn validate_wrapper_invariants_covers_all_arms() { + // Valid: bare elements pass. + assert!(Element::Item(b"x".to_vec(), None) + .validate_wrapper_invariants() + .is_ok()); + + // Valid NonCounted/NotSummed/NotCountedOrSummed pass. + let nc = Element::NonCounted(Box::new(Element::Item(b"x".to_vec(), None))); + assert!(nc.validate_wrapper_invariants().is_ok()); + let ns = Element::NotSummed(Box::new(Element::SumTree(None, 0, None))); + assert!(ns.validate_wrapper_invariants().is_ok()); + let ncos = Element::NotCountedOrSummed(Box::new(Element::SumTree(None, 0, None))); + assert!(ncos.validate_wrapper_invariants().is_ok()); + + // Invalid: NonCounted wrapping another wrapper. + let bad = Element::NonCounted(Box::new(Element::NotCountedOrSummed(Box::new( + Element::SumTree(None, 0, None), + )))); + assert!(bad.validate_wrapper_invariants().is_err()); + + // Invalid: NotSummed with non-sum-tree inner. + let bad = Element::NotSummed(Box::new(Element::Item(b"x".to_vec(), None))); + assert!(bad.validate_wrapper_invariants().is_err()); + + // Invalid: NotCountedOrSummed with non-sum-tree inner. + let bad = Element::NotCountedOrSummed(Box::new(Element::Item(b"x".to_vec(), None))); + assert!(bad.validate_wrapper_invariants().is_err()); + } } diff --git a/grovedb/src/tests/not_counted_or_summed_tests.rs b/grovedb/src/tests/not_counted_or_summed_tests.rs index e3af8fae1..208dbb2b1 100644 --- a/grovedb/src/tests/not_counted_or_summed_tests.rs +++ b/grovedb/src/tests/not_counted_or_summed_tests.rs @@ -372,6 +372,82 @@ mod tests { ); } + #[test] + fn batch_propagation_preserves_wrapper_under_provable_count_sum_tree() { + // Same as the CountSumTree propagation test but with a + // ProvableCountSumTree parent — exercises the + // ProvableCountSumTree propagation arm in batch_structure. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + make_provable_count_sum_tree_parent(&db, b"outer", grove_version); + + // Plain item contributes (1, 0). + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"plain", + Element::new_item(b"x".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert plain"); + + let wrapped = + Element::new_not_counted_or_summed(Element::new_provable_count_sum_tree(None)) + .expect("wrap ok"); + let inner_child = Element::new_sum_item(99); + let ops = vec![ + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"outer".to_vec()], + b"w".to_vec(), + wrapped, + ), + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"outer".to_vec(), b"w".to_vec()], + b"child".to_vec(), + inner_child, + ), + ]; + db.apply_batch(ops, None, None, grove_version) + .unwrap() + .expect("batch should succeed"); + + // Wrapper must survive propagation. + let stored = db + .get_raw( + grovedb_path::SubtreePath::from(&[TEST_LEAF, b"outer"]), + b"w", + None, + grove_version, + ) + .unwrap() + .expect("get_raw w"); + assert!( + matches!(stored, Element::NotCountedOrSummed(_)), + "wrapper must survive batch propagation; got {:?}", + stored + ); + + // Outer aggregate: count=1 (plain only), sum=0. + use grovedb_storage::StorageBatch; + let batch = StorageBatch::new(); + let tx = db.start_transaction(); + let outer_merk = db + .open_transactional_merk_at_path( + [TEST_LEAF, b"outer"].as_ref().into(), + &tx, + Some(&batch), + grove_version, + ) + .unwrap() + .expect("open outer merk"); + let aggregate = outer_merk.aggregate_data().expect("read aggregate"); + assert_eq!(aggregate.as_count_u64(), 1); + assert_eq!(aggregate.as_sum_i64(), 0); + } + #[test] fn check_subtree_exists_through_not_counted_or_summed_wrapper() { // A NotCountedOrSummed-wrapped tree at the parent path must satisfy diff --git a/merk/src/element/insert.rs b/merk/src/element/insert.rs index 1b6ecea10..f723470fa 100644 --- a/merk/src/element/insert.rs +++ b/merk/src/element/insert.rs @@ -985,6 +985,93 @@ mod tests { assert!(matches!(result, Err(Error::InvalidInputError(_)))); } + #[test] + fn not_counted_or_summed_round_trips_through_storage() { + // Insert a wrapped tree, then read it back via `Element::get`. This + // exercises the v1 storage-read path (and the wrapper-overhead + // accounting in `merk/src/element/get.rs`). + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::CountSumTree); + + let inner = Element::new_count_sum_tree_with_flags_and_sum_and_count_value( + None, + 3, + 42, + Some(vec![9, 9, 9]), + ); + let wrapped = Element::new_not_counted_or_summed(inner.clone()).expect("wrap ok"); + + wrapped + .clone() + .insert_subtree(&mut merk, b"k", [0u8; 32], None, grove_version) + .unwrap() + .expect("insert"); + + let read_back = Element::get(&merk, b"k", true, grove_version) + .unwrap() + .expect("get"); + assert_eq!(read_back, wrapped); + // Inner structure preserved. + if let Element::NotCountedOrSummed(boxed) = read_back { + assert!( + matches!(*boxed, Element::CountSumTree(_, 3, 42, ref f) if f == &Some(vec![9, 9, 9])) + ); + } else { + panic!("expected NotCountedOrSummed"); + } + } + + #[test] + fn not_counted_or_summed_rejected_via_insert_entry_point() { + // The wrapper's guard fires in the `insert` entry point too, not + // just `insert_subtree`. Use the bare `insert` API on a wrong + // parent to exercise that guard. (`new_not_counted_or_summed` + // only allows tree inners, so this is an unusual API use — but + // the guard is symmetric across all three entry points.) + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::NormalTree); + let w = Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let result = w.insert(&mut merk, b"k", None, grove_version).unwrap(); + assert!(matches!(result, Err(Error::InvalidInputError(_)))); + } + + #[test] + fn not_counted_or_summed_rejected_via_insert_reference_entry_point() { + // Symmetric coverage for the `insert_reference` guard. + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::NormalTree); + let w = Element::new_not_counted_or_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let result = w + .insert_reference(&mut merk, b"k", [0u8; 32], None, grove_version) + .unwrap(); + assert!(matches!(result, Err(Error::InvalidInputError(_)))); + } + + #[test] + fn not_counted_or_summed_in_provable_count_sum_tree_excludes_both_axes() { + // Symmetric to the CountSumTree case below but with a + // ProvableCountSumTree parent — exercises the second + // is_count_and_sum_bearing variant. + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::ProvableCountSumTree); + + // Bare ProvableCountSumTree(_, 3, 100) contributes (3, 100). + // Wrapped, contributes (0, 0). + let w = Element::new_not_counted_or_summed( + Element::new_provable_count_sum_tree_with_flags_and_sum_and_count_value( + None, 3, 100, None, + ), + ) + .expect("wrap ok"); + w.insert_subtree(&mut merk, b"k", [0u8; 32], None, grove_version) + .unwrap() + .expect("insert wrapped"); + + let agg = merk.aggregate_data().expect("aggregate ok"); + assert_eq!(agg.as_count_u64(), 0); + assert_eq!(agg.as_sum_i64(), 0); + } + #[test] fn not_counted_or_summed_in_count_sum_tree_excludes_both_axes() { // A NotCountedOrSummed(SumTree(_, 100)) inside a CountSumTree must diff --git a/merk/src/element/reconstruct.rs b/merk/src/element/reconstruct.rs index 1e0d2ca0e..3fa4146b2 100644 --- a/merk/src/element/reconstruct.rs +++ b/merk/src/element/reconstruct.rs @@ -135,4 +135,24 @@ mod tests { .reconstruct_with_root_key(None, AggregateData::NoAggregateData) .is_none()); } + + #[test] + fn reconstruct_preserves_not_counted_or_summed_wrapper() { + // Symmetric to reconstruct_preserves_non_counted_wrapper / + // reconstruct_preserves_not_summed_wrapper. A wrapped tree must come + // back wrapped after root-key propagation; otherwise the on-disk + // parent element would lose its wrapper on subtree mutations and the + // parent's aggregate would erroneously include the subtree. + let inner = + Element::new_count_sum_tree_with_flags_and_sum_and_count_value(None, 3, 100, None); + let wrapped = Element::new_not_counted_or_summed(inner).expect("wrap ok"); + let new_root = Some(b"new_root".to_vec()); + let reconstructed = wrapped + .reconstruct_with_root_key(new_root.clone(), AggregateData::CountAndSum(3, 100)) + .expect("reconstruct ok"); + assert!(matches!(reconstructed, Element::NotCountedOrSummed(_))); + if let Element::NotCountedOrSummed(boxed) = reconstructed { + assert!(matches!(*boxed, Element::CountSumTree(ref k, 3, 100, _) if k == &new_root)); + } + } } diff --git a/merk/src/element/tree_type.rs b/merk/src/element/tree_type.rs index 6212facf3..98b5d52e5 100644 --- a/merk/src/element/tree_type.rs +++ b/merk/src/element/tree_type.rs @@ -355,6 +355,81 @@ mod tests { } } + #[test] + fn tree_type_extensions_look_through_not_counted_or_summed() { + // Every ElementTreeTypeExtensions method must delegate through + // NotCountedOrSummed to the inner sum-tree variant. + let inner_root = Some(b"r".to_vec()); + let cases: [(Element, TreeType); 4] = [ + ( + Element::SumTree(inner_root.clone(), 100, None), + TreeType::SumTree, + ), + ( + Element::BigSumTree(inner_root.clone(), 100, None), + TreeType::BigSumTree, + ), + ( + Element::CountSumTree(inner_root.clone(), 7, 100, None), + TreeType::CountSumTree, + ), + ( + Element::ProvableCountSumTree(inner_root.clone(), 7, 100, None), + TreeType::ProvableCountSumTree, + ), + ]; + + for (inner, expected_tree_type) in cases { + let wrapped = Element::new_not_counted_or_summed(inner.clone()).expect("wrap ok"); + + assert_eq!(wrapped.tree_type(), Some(expected_tree_type)); + assert_eq!( + wrapped.maybe_tree_type(), + MaybeTree::Tree(expected_tree_type) + ); + let (rk, tt) = wrapped.root_key_and_tree_type().expect("Some"); + assert_eq!(*rk, inner_root); + assert_eq!(tt, expected_tree_type); + let (rk, tt) = wrapped + .clone() + .root_key_and_tree_type_owned() + .expect("Some"); + assert_eq!(rk, inner_root); + assert_eq!(tt, expected_tree_type); + + let (flags, tt) = wrapped.tree_flags_and_type().expect("Some"); + assert!(flags.is_none()); + assert_eq!(tt, expected_tree_type); + + assert!(wrapped.tree_feature_type().is_some()); + } + } + + #[test] + fn get_feature_type_zeros_both_axes_for_not_counted_or_summed() { + // NotCountedOrSummed must zero out BOTH count and sum in + // count-and-sum-bearing parents. + let inner = Element::CountSumTree(None, 7, 100, None); + let ncos = Element::new_not_counted_or_summed(inner).expect("wrap ok"); + + // CountSumTree parent: count=0, sum=0. + assert_eq!( + ncos.get_feature_type(TreeType::CountSumTree).unwrap(), + CountedSummedMerkNode(0, 0) + ); + + // ProvableCountSumTree parent: same. + match ncos + .get_feature_type(TreeType::ProvableCountSumTree) + .unwrap() + { + TreeFeatureType::ProvableCountedSummedMerkNode(c, s) => { + assert_eq!((c, s), (0, 0)); + } + other => panic!("expected ProvableCountedSummedMerkNode, got {:?}", other), + } + } + #[test] fn get_feature_type_zeros_sum_for_not_summed_in_sum_parents() { // Every sum-bearing parent type must zero out the wrapped sum From 627efc2b6202ec8d90888a1cee8a096f6633ecd7 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sat, 16 May 2026 20:15:50 +0700 Subject: [PATCH 3/3] review: apply CodeRabbit fixes (constructor revalidation + typed error asserts) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two items from CodeRabbit's review on #666: 1. constructor.rs (actionable A1): `into_non_counted`, `into_not_summed`, and `into_not_counted_or_summed` all return `Ok(self)` on the idempotent already-wrapped match arm. Add a `validate_wrapper_invariants()` re-check on each so a hand-built nested-wrapper value can't slip through this path (constructors validate at creation, but the idempotent arm takes `self` as-is). 2. not_counted_or_summed_tests.rs (nitpick N4): replace `format!("{err:?}").contains("...")` substring matching with a typed `Error::InvalidBatchOperation` variant match via a new local `assert_is_not_counted_or_summed_guard_error` helper. Test no longer depends on debug-format wording — only the typed variant and a stable error string. CodeRabbit's remaining suggestions were considered and skipped: - N1 (reconstruct.rs preservation test): already added in the prior commit. - N2 (cost_return_on_error! in insert.rs guards): the existing NonCounted and NotSummed guards in the same file use the identical `return Err(...).wrap_with_cost(Default::default())` pattern; keeping the three guards consistent. - N3 (collapse three booleans into an enum on InsertTreeWithRootHash): larger refactor that would touch all three wrapper propagation paths; better as a separate cleanup PR. - A2 (validate inner-tree type in batch guard): the constructor and serialize/deserialize both enforce the four-sum-bearing-tree whitelist, and an inner-validation check would need to be applied uniformly to NotSummed as well; out of scope for this PR. 83 grovedb-element tests pass, 9 NotCountedOrSummed grovedb tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- grovedb-element/src/element/constructor.rs | 23 ++++++++++-- .../src/tests/not_counted_or_summed_tests.rs | 35 ++++++++++--------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/grovedb-element/src/element/constructor.rs b/grovedb-element/src/element/constructor.rs index 6b36ea236..fc97b5b55 100644 --- a/grovedb-element/src/element/constructor.rs +++ b/grovedb-element/src/element/constructor.rs @@ -414,7 +414,14 @@ impl Element { /// before calling. pub fn into_non_counted(self) -> Result { match self { - Element::NonCounted(_) => Ok(self), + Element::NonCounted(_) => { + // Re-validate the idempotent path: even though only valid + // NonCounted values should reach here via construction, a + // hand-built nested-wrapper value would slip through without + // this check. + self.validate_wrapper_invariants()?; + Ok(self) + } Element::NotSummed(_) => Err(ElementError::InvalidInput( "cannot wrap NotSummed in NonCounted; wrappers are mutually exclusive", )), @@ -455,7 +462,12 @@ impl Element { /// Mirrors [`Element::into_non_counted`]. pub fn into_not_summed(self) -> Result { match self { - Element::NotSummed(_) => Ok(self), + Element::NotSummed(_) => { + // Re-validate the idempotent path; see `into_non_counted` + // for rationale. + self.validate_wrapper_invariants()?; + Ok(self) + } Element::NonCounted(_) => Err(ElementError::InvalidInput( "cannot wrap NonCounted in NotSummed; wrappers are mutually exclusive", )), @@ -498,7 +510,12 @@ impl Element { /// wrappers are mutually exclusive) or any non-sum-tree variant. pub fn into_not_counted_or_summed(self) -> Result { match self { - Element::NotCountedOrSummed(_) => Ok(self), + Element::NotCountedOrSummed(_) => { + // Re-validate the idempotent path; see `into_non_counted` + // for rationale. + self.validate_wrapper_invariants()?; + Ok(self) + } Element::NonCounted(_) => Err(ElementError::InvalidInput( "cannot wrap NonCounted in NotCountedOrSummed; wrappers are mutually exclusive", )), diff --git a/grovedb/src/tests/not_counted_or_summed_tests.rs b/grovedb/src/tests/not_counted_or_summed_tests.rs index 208dbb2b1..ac71f8902 100644 --- a/grovedb/src/tests/not_counted_or_summed_tests.rs +++ b/grovedb/src/tests/not_counted_or_summed_tests.rs @@ -16,9 +16,24 @@ mod tests { use crate::{ batch::QualifiedGroveDbOp, tests::{make_test_grovedb, TEST_LEAF}, - Element, + Element, Error, }; + /// Helper: assert the error is the typed `InvalidBatchOperation` whose + /// message mentions the NotCountedOrSummed guard. Pattern-matches the + /// concrete variant rather than the whole debug-format string so that + /// future error-context wording can change without breaking the test. + fn assert_is_not_counted_or_summed_guard_error(err: &Error) { + match err { + Error::InvalidBatchOperation(msg) => assert!( + msg.contains("not-counted-or-summed"), + "expected NotCountedOrSummed parent-type guard message, \ + got: {msg}" + ), + other => panic!("expected Error::InvalidBatchOperation, got: {other:?}"), + } + } + /// Establish a count-sum tree under `TEST_LEAF/` for use as a host /// parent in the tests below. fn make_count_sum_tree_parent(db: &crate::GroveDb, key: &[u8], grove_version: &GroveVersion) { @@ -66,11 +81,7 @@ mod tests { .apply_batch(vec![op], None, None, grove_version) .unwrap() .expect_err("batch insert into NormalTree must fail"); - let msg = format!("{err:?}"); - assert!( - msg.contains("not-counted-or-summed") || msg.contains("not_counted_or_summed"), - "expected NotCountedOrSummed parent-type guard error, got: {msg}" - ); + assert_is_not_counted_or_summed_guard_error(&err); } #[test] @@ -102,11 +113,7 @@ mod tests { .apply_batch(vec![op], None, None, grove_version) .unwrap() .expect_err("batch insert into CountTree must fail"); - let msg = format!("{err:?}"); - assert!( - msg.contains("not-counted-or-summed") || msg.contains("not_counted_or_summed"), - "expected NotCountedOrSummed parent-type guard error, got: {msg}" - ); + assert_is_not_counted_or_summed_guard_error(&err); } #[test] @@ -138,11 +145,7 @@ mod tests { .apply_batch(vec![op], None, None, grove_version) .unwrap() .expect_err("batch insert into SumTree must fail"); - let msg = format!("{err:?}"); - assert!( - msg.contains("not-counted-or-summed") || msg.contains("not_counted_or_summed"), - "expected NotCountedOrSummed parent-type guard error, got: {msg}" - ); + assert_is_not_counted_or_summed_guard_error(&err); } #[test]