diff --git a/grovedb-element/Cargo.toml b/grovedb-element/Cargo.toml index abe40c365..769a08f4a 100644 --- a/grovedb-element/Cargo.toml +++ b/grovedb-element/Cargo.toml @@ -27,3 +27,6 @@ verify = [] constructor = [] serde = ["dep:serde"] visualize = ["dep:grovedb-visualize"] + +[dev-dependencies] +serde_json = "1" diff --git a/grovedb-element/src/element/constructor.rs b/grovedb-element/src/element/constructor.rs index da39913c5..04e58cd71 100644 --- a/grovedb-element/src/element/constructor.rs +++ b/grovedb-element/src/element/constructor.rs @@ -387,24 +387,71 @@ impl Element { /// Wrap an element in `NonCounted` so it contributes 0 to its parent count /// tree's aggregate count when inserted. Sums (if any) still propagate. /// - /// Returns `InvalidInput` if `inner` is itself a `NonCounted`. Wrapping - /// is not idempotent at the type level — use `into_non_counted` to wrap - /// without that risk. + /// 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 + /// cross-wrapper case. pub fn new_non_counted(inner: Element) -> Result { - if matches!(inner, Element::NonCounted(_)) { + if matches!(inner, Element::NonCounted(_) | Element::NotSummed(_)) { return Err(ElementError::InvalidInput( - "NonCounted cannot wrap another NonCounted", + "NonCounted cannot wrap another wrapper", )); } Ok(Element::NonCounted(Box::new(inner))) } - /// Wrap `self` in `NonCounted`. If `self` is already `NonCounted`, returns - /// `self` unchanged (idempotent). - pub fn into_non_counted(self) -> Self { + /// 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. + 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", + )), + other => Ok(Element::NonCounted(Box::new(other))), + } + } + + /// Wrap a sum-tree variant in `NotSummed` so it contributes 0 to its + /// parent sum tree's running sum when inserted. Counts (if any) still + /// propagate. + /// + /// 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`. + pub fn new_not_summed(inner: Element) -> Result { + match inner { + Element::SumTree(..) + | Element::BigSumTree(..) + | Element::CountSumTree(..) + | Element::ProvableCountSumTree(..) => Ok(Element::NotSummed(Box::new(inner))), + _ => Err(ElementError::InvalidInput( + "NotSummed inner element must be a sum-tree variant (SumTree, BigSumTree, \ + CountSumTree, or ProvableCountSumTree)", + )), + } + } + + /// 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`]. + pub fn into_not_summed(self) -> Result { match self { - Element::NonCounted(_) => self, - other => Element::NonCounted(Box::new(other)), + Element::NotSummed(_) => Ok(self), + Element::NonCounted(_) => Err(ElementError::InvalidInput( + "cannot wrap NonCounted in NotSummed; wrappers are mutually exclusive", + )), + other => Self::new_not_summed(other), } } } diff --git a/grovedb-element/src/element/helpers.rs b/grovedb-element/src/element/helpers.rs index fa6b65d5b..8b68e80fc 100644 --- a/grovedb-element/src/element/helpers.rs +++ b/grovedb-element/src/element/helpers.rs @@ -19,12 +19,23 @@ impl Element { matches!(self, Element::NonCounted(_)) } - /// Returns the wrapped element if `self` is `NonCounted`, 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::NotSummed`. + /// The wrapper suppresses sum propagation to the parent sum tree but + /// leaves all other behavior (storage, hashing, count propagation, + /// internal aggregation) unchanged. + pub fn is_not_summed(&self) -> bool { + 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. + /// + /// 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) => inner, + Element::NonCounted(inner) | Element::NotSummed(inner) => inner, other => other, } } @@ -32,7 +43,7 @@ impl Element { /// Mutable variant of [`underlying`]. pub fn underlying_mut(&mut self) -> &mut Element { match self { - Element::NonCounted(inner) => inner, + Element::NonCounted(inner) | Element::NotSummed(inner) => inner, other => other, } } @@ -40,7 +51,7 @@ impl Element { /// Owned variant of [`underlying`]. pub fn into_underlying(self) -> Element { match self { - Element::NonCounted(inner) => *inner, + Element::NonCounted(inner) | Element::NotSummed(inner) => *inner, other => other, } } @@ -50,9 +61,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. pub fn sum_value_or_default(&self) -> i64 { match self { Element::NonCounted(inner) => inner.sum_value_or_default(), + Element::NotSummed(_) => 0, Element::SumItem(sum_value, _) | Element::ItemWithSumItem(_, sum_value, _) | Element::SumTree(_, sum_value, _) @@ -67,9 +81,11 @@ impl Element { /// /// `NonCounted` returns 0 — the wrapper's whole purpose is to contribute /// nothing to the parent count tree. + /// `NotSummed` delegates to its inner — counts still propagate. pub fn count_value_or_default(&self) -> u64 { match self { Element::NonCounted(_) => 0, + Element::NotSummed(inner) => inner.count_value_or_default(), Element::CountTree(_, count_value, _) | Element::CountSumTree(_, count_value, ..) | Element::ProvableCountTree(_, count_value, _) @@ -83,9 +99,12 @@ impl Element { /// /// `NonCounted` returns `(0, inner_sum)` — count is suppressed, sum still /// propagates. + /// `NotSummed` returns `(inner_count, 0)` — sum is suppressed, count + /// still propagates. 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::SumItem(sum_value, _) | Element::ItemWithSumItem(_, sum_value, _) | Element::SumTree(_, sum_value, _) => (1, *sum_value), @@ -100,10 +119,12 @@ impl Element { } /// Decoded the integer value in the SumItem element type, returns 0 for - /// everything else. `NonCounted` delegates to its inner. + /// everything else. `NonCounted` delegates to its inner. `NotSummed` + /// returns 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::SumItem(sum_value, _) | Element::ItemWithSumItem(_, sum_value, _) | Element::SumTree(_, sum_value, _) @@ -373,7 +394,7 @@ impl Element { | Element::MmrTree(.., flags) | Element::BulkAppendTree(.., flags) | Element::DenseAppendOnlyFixedSizeTree(.., flags) => flags, - Element::NonCounted(inner) => inner.get_flags(), + Element::NonCounted(inner) | Element::NotSummed(inner) => inner.get_flags(), } } @@ -396,7 +417,7 @@ impl Element { | Element::MmrTree(.., flags) | Element::BulkAppendTree(.., flags) | Element::DenseAppendOnlyFixedSizeTree(.., flags) => flags, - Element::NonCounted(inner) => inner.get_flags_owned(), + Element::NonCounted(inner) | Element::NotSummed(inner) => inner.get_flags_owned(), } } @@ -419,7 +440,7 @@ impl Element { | Element::MmrTree(.., flags) | Element::BulkAppendTree(.., flags) | Element::DenseAppendOnlyFixedSizeTree(.., flags) => flags, - Element::NonCounted(inner) => inner.get_flags_mut(), + Element::NonCounted(inner) | Element::NotSummed(inner) => inner.get_flags_mut(), } } @@ -442,7 +463,7 @@ impl Element { | Element::MmrTree(.., flags) | Element::BulkAppendTree(.., flags) | Element::DenseAppendOnlyFixedSizeTree(.., flags) => *flags = new_flags, - Element::NonCounted(inner) => inner.set_flags(new_flags), + Element::NonCounted(inner) | Element::NotSummed(inner) => inner.set_flags(new_flags), } } @@ -520,12 +541,30 @@ mod non_counted_tests { #[test] fn into_non_counted_is_idempotent() { let inner = Element::Item(b"x".to_vec(), None); - let once = inner.clone().into_non_counted(); - let twice = once.clone().into_non_counted(); + let once = inner.clone().into_non_counted().expect("wrap ok"); + let twice = once.clone().into_non_counted().expect("rewrap ok"); assert_eq!(once, twice); assert!(twice.is_non_counted()); } + #[test] + fn into_non_counted_rejects_not_summed() { + // The two wrappers are mutually exclusive — wrapping NotSummed in + // NonCounted must fail rather than silently succeed (which would + // produce a doubly-wrapped element that the (de)serializer rejects + // anyway, but earlier in the pipeline). + let ns = Element::new_not_summed(Element::new_sum_tree(None)).expect("wrap ok"); + assert!(ns.into_non_counted().is_err()); + } + + #[test] + fn new_non_counted_rejects_not_summed() { + // Symmetric to `new_not_summed_rejects_non_counted` — `new_non_counted` + // must reject any pre-existing wrapper inner, including NotSummed. + let ns = Element::new_not_summed(Element::new_sum_tree(None)).expect("wrap ok"); + assert!(Element::new_non_counted(ns).is_err()); + } + #[test] fn predicates_look_through_wrapper() { let tree = Element::new_tree(None); @@ -617,8 +656,8 @@ mod non_counted_tests { .expect_err("nested wrapper bytes must be rejected"); let msg = format!("{:?}", err); assert!( - msg.contains("NonCounted") || msg.contains("non_counted"), - "error should mention nested NonCounted: {}", + msg.contains("NonCounted") || msg.contains("non_counted") || msg.contains("wrapper"), + "error should mention nested wrapper: {}", msg ); } @@ -631,3 +670,182 @@ mod non_counted_tests { assert!(bad.serialize(grove_version).is_err()); } } + +#[cfg(test)] +mod not_summed_tests { + use grovedb_version::version::GroveVersion; + + use crate::element::Element; + + #[test] + fn new_not_summed_wraps_sum_tree_variants() { + // The 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_summed(inner.clone()).expect("wrap ok"); + assert!(wrapped.is_not_summed()); + assert_eq!(wrapped.underlying(), &inner); + } + } + + #[test] + fn new_not_summed_rejects_non_sum_tree_inner() { + // Items, sum items, references, and non-sum trees are all rejected. + assert!(Element::new_not_summed(Element::new_item(b"x".to_vec())).is_err()); + assert!(Element::new_not_summed(Element::new_sum_item(7)).is_err()); + assert!(Element::new_not_summed(Element::new_tree(None)).is_err()); + assert!(Element::new_not_summed(Element::new_count_tree(None)).is_err()); + assert!(Element::new_not_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_summed(nc).is_err()); + let ns = Element::new_not_summed(Element::new_sum_tree(None)).expect("ns ok"); + assert!(Element::new_not_summed(ns).is_err()); + } + + #[test] + fn predicates_look_through_wrapper() { + let st = Element::new_sum_tree_with_flags_and_sum_value(Some(b"r".to_vec()), 100, None); + let ns = Element::new_not_summed(st).expect("wrap ok"); + // Tree predicates pass through. + assert!(ns.is_any_tree()); + assert!(ns.is_sum_tree()); + assert!(ns.is_non_empty_tree()); + assert!(!ns.is_any_item()); + assert!(!ns.is_basic_tree()); + } + + #[test] + fn sum_value_or_default_is_zero_for_not_summed() { + // Bare SumTree with internal sum 100 contributes 100. + let st = Element::new_sum_tree_with_flags_and_sum_value(None, 100, None); + assert_eq!(st.sum_value_or_default(), 100); + + // NotSummed wrapper suppresses that to 0. + let ns = Element::new_not_summed(st).expect("wrap ok"); + assert_eq!(ns.sum_value_or_default(), 0); + assert_eq!(ns.big_sum_value_or_default(), 0); + } + + #[test] + fn count_value_or_default_propagates_through_not_summed() { + // A NotSummed-wrapped CountSumTree(_, 5, 100, _) still contributes + // count = 5. Only the sum is suppressed. + 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); + + let ns = Element::new_not_summed(cst).expect("wrap ok"); + assert_eq!(ns.count_value_or_default(), 5); + assert_eq!(ns.sum_value_or_default(), 0); + } + + #[test] + fn count_sum_value_or_default_zeros_sum_keeps_count() { + let cst = Element::new_count_sum_tree_with_flags_and_sum_and_count_value(None, 3, 42, None); + assert_eq!(cst.count_sum_value_or_default(), (3, 42)); + + let ns = Element::new_not_summed(cst).expect("wrap ok"); + assert_eq!(ns.count_sum_value_or_default(), (3, 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 ns = Element::new_not_summed(st).expect("wrap ok"); + assert_eq!(ns.get_flags(), &flags); + } + + #[test] + fn bincode_round_trip_through_wrapper() { + let grove_version = GroveVersion::latest(); + let inner = Element::new_sum_tree_with_flags_and_sum_value( + Some(b"root".to_vec()), + 42, + Some(vec![9, 8]), + ); + let wrapped = Element::new_not_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_wrappers() { + // Construct nested wrappers manually, bypassing the constructor. + // serialize() rejects too, but use bincode directly to test the + // *deserialize* path. + use bincode::config; + let cfg = config::standard().with_big_endian().with_no_limit(); + let grove_version = GroveVersion::latest(); + + // NotSummed(NotSummed(SumTree)). + let nested_nn = Element::NotSummed(Box::new(Element::NotSummed(Box::new( + Element::SumTree(None, 0, None), + )))); + let bytes = bincode::encode_to_vec(&nested_nn, cfg).expect("encode"); + assert!(Element::deserialize(&bytes, grove_version).is_err()); + + // NotSummed(NonCounted(SumTree)). + let cross = Element::NotSummed(Box::new(Element::NonCounted(Box::new(Element::SumTree( + None, 0, None, + ))))); + let bytes = bincode::encode_to_vec(&cross, cfg).expect("encode"); + assert!(Element::deserialize(&bytes, grove_version).is_err()); + + // NonCounted(NotSummed(SumTree)). + let cross2 = Element::NonCounted(Box::new(Element::NotSummed(Box::new(Element::SumTree( + None, 0, None, + ))))); + let bytes = bincode::encode_to_vec(&cross2, cfg).expect("encode"); + assert!(Element::deserialize(&bytes, grove_version).is_err()); + } + + /// The pre-check before bincode decode is the actual stack-overflow + /// guard: a long chain of wrapper bytes is rejected without bincode + /// recursing through them. Mirrors the NonCounted long-chain test. + #[test] + fn deserialize_rejects_long_nested_wrapper_chain_without_recursion() { + let grove_version = GroveVersion::latest(); + // 1024 wrapper bytes (alternating NotSummed/NotSummed) followed by a + // base sum tree. With the post-check alone, bincode would recurse + // through all 1024 Box wrappers — pre-check stops it on + // byte 1. + let mut bytes = vec![16u8; 1024]; + // Append a minimal valid SumTree (disc 4, no root key, varint sum 0, + // no flags). + 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 nested wrapper: {}", + msg + ); + } + + #[test] + fn deserialize_rejects_not_summed_with_non_sum_tree_inner() { + // A NotSummed wrapping a plain Item must be rejected at deserialize. + use bincode::config; + let cfg = config::standard().with_big_endian().with_no_limit(); + let bad = Element::NotSummed(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_summed_with_non_sum_tree_inner() { + let bad = Element::NotSummed(Box::new(Element::Item(b"x".to_vec(), None))); + let grove_version = GroveVersion::latest(); + assert!(bad.serialize(grove_version).is_err()); + } +} diff --git a/grovedb-element/src/element/mod.rs b/grovedb-element/src/element/mod.rs index 0a83ab816..6f6452df4 100644 --- a/grovedb-element/src/element/mod.rs +++ b/grovedb-element/src/element/mod.rs @@ -42,9 +42,15 @@ pub trait ElementCostSizeExtension { /// /// ONLY APPEND TO THIS LIST!!! Because /// of how serialization works. +/// +/// `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. #[derive(Clone, Encode, Decode, PartialEq, Eq, Hash)] #[cfg_attr(not(feature = "visualize"), derive(Debug))] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] pub enum Element { /// An ordinary value Item(Vec, Option), @@ -133,6 +139,21 @@ pub enum Element { /// Invariant: a `NonCounted` may not wrap another `NonCounted`. Enforced /// at construction and at deserialization. NonCounted(Box), + /// Not-summed wrapper: contains a sum-bearing tree variant (`SumTree`, + /// `BigSumTree`, `CountSumTree`, `ProvableCountSumTree`) and behaves + /// identically to it for storage, hashing, and its own internal sum + /// aggregate, but contributes 0 to its parent sum tree's running sum + /// when inserted. Counts still propagate. + /// + /// May only be inserted into sum-bearing trees (`SumTree`, `BigSumTree`, + /// `CountSumTree`, `ProvableCountSumTree`). + /// + /// 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. + NotSummed(Box), } pub fn hex_to_ascii(hex_value: &[u8]) -> String { @@ -321,6 +342,9 @@ impl fmt::Display for Element { Element::NonCounted(inner) => { write!(f, "NonCounted({})", inner) } + Element::NotSummed(inner) => { + write!(f, "NotSummed({})", inner) + } } } } @@ -371,10 +395,361 @@ impl Element { // forbidden at construction and (de)serialization. already_non_counted => already_non_counted, }, + Element::NotSummed(inner) => match inner.element_type() { + ElementType::SumTree => ElementType::NotSummedSumTree, + ElementType::BigSumTree => ElementType::NotSummedBigSumTree, + ElementType::CountSumTree => ElementType::NotSummedCountSumTree, + ElementType::ProvableCountSumTree => ElementType::NotSummedProvableCountSumTree, + // Inner is always one of the 4 sum-tree variants above — + // construction and (de)serialization forbid anything else. + // Returning the inner type is the safest fallback for the + // unreachable case. + other => other, + }, } } pub fn type_str(&self) -> &str { self.element_type().as_str() } + + /// 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`). + /// + /// Constructors and the `serialize`/`deserialize` paths already enforce + /// these rules; this helper exists so external callers (most importantly + /// the manual `serde::Deserialize` impl) can apply the same checks. + /// + /// Only the immediate wrapper layer is checked — wrapper nesting is + /// forbidden by these very rules, so deeper recursion is not needed. + pub fn validate_wrapper_invariants(&self) -> Result<(), crate::error::ElementError> { + match self { + Element::NonCounted(inner) => { + if matches!(**inner, Element::NonCounted(_) | Element::NotSummed(_)) { + return Err(crate::error::ElementError::InvalidInput( + "NonCounted cannot wrap another wrapper", + )); + } + } + Element::NotSummed(inner) => match **inner { + Element::SumTree(..) + | Element::BigSumTree(..) + | Element::CountSumTree(..) + | Element::ProvableCountSumTree(..) => {} + _ => { + return Err(crate::error::ElementError::InvalidInput( + "NotSummed inner element must be a sum-tree variant (SumTree, \ + BigSumTree, CountSumTree, or ProvableCountSumTree)", + )); + } + }, + _ => {} + } + Ok(()) + } +} + +#[cfg(feature = "serde")] +mod serde_impl { + //! Manual `serde::Deserialize` for [`Element`] that enforces the same + //! wrapper invariants as [`Element::deserialize`]. Without this, a serde + //! payload could construct invalid `NotSummed(Item)` or cross-wrapper + //! values that the rest of the system assumes do not exist. + //! + //! ### Why a shadow enum? + //! + //! Serde does not provide a derive-and-validate passthrough for whole + //! types. The relevant attributes don't fit: + //! - `#[serde(try_from = "...")]` needs a *separate* source type — you + //! can't point it back at `Self` (infinite recursion at the type + //! level). + //! - `#[serde(remote = "...")]` is only for foreign types you don't own. + //! - `#[serde(deserialize_with = "...")]` is field-level, not type-level. + //! - `#[serde(transparent)]` / `#[serde(flatten)]` are for single-field + //! structs and field embedding respectively. + //! + //! That leaves three real options: (1) duplicate the variants in a + //! shadow type and convert via `From`, (2) write a manual `Visitor` + //! that walks all 17 variants by hand, or (3) drop the `Deserialize` + //! derive entirely. The shadow is the shortest of (1) and (2), and + //! we keep `Deserialize` because external tooling consumers may rely + //! on it. + //! + //! ### Approach + //! + //! A private shadow enum (`ElementShadow`) mirrors `Element` verbatim + //! with `#[serde(rename = "Element")]` so the wire format is identical. + //! The derived `Deserialize` on the shadow handles the recursive + //! descent; we then convert to `Element` and call + //! [`Element::check_recursive_wrapper_invariants`]. + //! + //! Each `Box` field deserializes through the shadow's + //! own `Deserialize` impl, so validation fires at every level of the + //! tree as the conversion unwinds. + + use serde::de::Error as _; + + use super::{BigSumValue, CountValue, Element, ElementFlags, MaxReferenceHop, SumValue}; + use crate::reference_path::ReferencePathType; + + #[derive(serde::Deserialize)] + #[serde(rename = "Element")] + enum ElementShadow { + Item(Vec, Option), + Reference(ReferencePathType, MaxReferenceHop, Option), + Tree(Option>, Option), + SumItem(SumValue, Option), + SumTree(Option>, SumValue, Option), + BigSumTree(Option>, BigSumValue, Option), + CountTree(Option>, CountValue, Option), + CountSumTree(Option>, CountValue, SumValue, Option), + ProvableCountTree(Option>, CountValue, Option), + ItemWithSumItem(Vec, SumValue, Option), + ProvableCountSumTree(Option>, CountValue, SumValue, Option), + CommitmentTree(u64, u8, Option), + MmrTree(u64, Option), + BulkAppendTree(u64, u8, Option), + DenseAppendOnlyFixedSizeTree(u16, u8, Option), + NonCounted(Box), + NotSummed(Box), + } + + impl From for Element { + fn from(s: ElementShadow) -> Self { + match s { + ElementShadow::Item(v, f) => Element::Item(v, f), + ElementShadow::Reference(p, h, f) => Element::Reference(p, h, f), + ElementShadow::Tree(k, f) => Element::Tree(k, f), + ElementShadow::SumItem(v, f) => Element::SumItem(v, f), + ElementShadow::SumTree(k, s, f) => Element::SumTree(k, s, f), + ElementShadow::BigSumTree(k, s, f) => Element::BigSumTree(k, s, f), + ElementShadow::CountTree(k, c, f) => Element::CountTree(k, c, f), + ElementShadow::CountSumTree(k, c, s, f) => Element::CountSumTree(k, c, s, f), + ElementShadow::ProvableCountTree(k, c, f) => Element::ProvableCountTree(k, c, f), + ElementShadow::ItemWithSumItem(v, s, f) => Element::ItemWithSumItem(v, s, f), + ElementShadow::ProvableCountSumTree(k, c, s, f) => { + Element::ProvableCountSumTree(k, c, s, f) + } + ElementShadow::CommitmentTree(c, p, f) => Element::CommitmentTree(c, p, f), + ElementShadow::MmrTree(s, f) => Element::MmrTree(s, f), + ElementShadow::BulkAppendTree(c, p, f) => Element::BulkAppendTree(c, p, f), + ElementShadow::DenseAppendOnlyFixedSizeTree(c, h, f) => { + Element::DenseAppendOnlyFixedSizeTree(c, h, f) + } + ElementShadow::NonCounted(inner) => { + Element::NonCounted(Box::new(Element::from(*inner))) + } + ElementShadow::NotSummed(inner) => { + Element::NotSummed(Box::new(Element::from(*inner))) + } + } + } + } + + impl<'de> serde::Deserialize<'de> for Element { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let shadow = ElementShadow::deserialize(deserializer)?; + let element = Element::from(shadow); + // Validate immediate wrapper invariants. Inner elements were + // built by recursive `From` calls, so the check + // at each level catches a violation at any depth. + Self::check_recursive_wrapper_invariants(&element).map_err(D::Error::custom)?; + Ok(element) + } + } + + impl Element { + /// Walk the element tree and run `validate_wrapper_invariants` at + /// every level. Used by the manual `serde::Deserialize`; bincode + /// goes through `Element::deserialize` which already validates the + /// outer wrapper plus a leading-byte pre-check that rejects + /// nesting before recursion. + pub(super) fn check_recursive_wrapper_invariants( + element: &Element, + ) -> Result<(), crate::error::ElementError> { + element.validate_wrapper_invariants()?; + if let Element::NonCounted(inner) | Element::NotSummed(inner) = element { + Self::check_recursive_wrapper_invariants(inner)?; + } + Ok(()) + } + } + + #[cfg(test)] + mod tests { + use super::*; + + /// Round-trip a valid `Element` through serde JSON and back. + #[test] + fn serde_round_trip_valid_elements() { + let cases = vec![ + Element::Item(b"abc".to_vec(), None), + 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(), + ]; + for original in cases { + let json = serde_json::to_string(&original).expect("serialize"); + let back: Element = serde_json::from_str(&json).expect("deserialize"); + assert_eq!(back, original, "round trip mismatch for {:?}", original); + } + } + + /// A serde payload that constructs a nested `NonCounted(NonCounted)` + /// must be rejected. + #[test] + fn serde_rejects_nested_non_counted() { + // Wire payload: NonCounted(NonCounted(Item)) + let json = r#"{"NonCounted":{"NonCounted":{"Item":[[120],null]}}}"#; + let result: Result = serde_json::from_str(json); + assert!( + result.is_err(), + "nested NonCounted must be rejected, got {:?}", + result + ); + } + + /// `NotSummed(NotSummed(_))` must be rejected. + #[test] + fn serde_rejects_nested_not_summed() { + let json = r#"{"NotSummed":{"NotSummed":{"SumTree":[null,0,null]}}}"#; + let result: Result = serde_json::from_str(json); + assert!(result.is_err(), "got {:?}", result); + } + + /// `NonCounted(NotSummed(_))` and `NotSummed(NonCounted(_))` must + /// both be rejected — wrappers are mutually exclusive. + #[test] + fn serde_rejects_cross_wrapper_nesting() { + let cross_a = r#"{"NonCounted":{"NotSummed":{"SumTree":[null,0,null]}}}"#; + let result: Result = serde_json::from_str(cross_a); + assert!( + result.is_err(), + "NonCounted(NotSummed) must be rejected; got {:?}", + result + ); + + let cross_b = r#"{"NotSummed":{"NonCounted":{"SumTree":[null,0,null]}}}"#; + let result: Result = serde_json::from_str(cross_b); + assert!( + result.is_err(), + "NotSummed(NonCounted) must be rejected; got {:?}", + result + ); + } + + /// `NotSummed(non_sum_tree)` must be rejected for every illegal + /// inner type. + #[test] + fn serde_rejects_not_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#"{{"NotSummed":{}}}"#, inner); + let result: Result = serde_json::from_str(&payload); + assert!( + result.is_err(), + "NotSummed({}) must be rejected; got {:?}", + inner, + result + ); + } + } + + /// Deeply-nested wrapper payloads must be rejected (recursion bound). + /// This pairs with the bincode pre-check; for serde we rely on the + /// recursive `From` calls hitting validation at each + /// level. With the immediate-level check at every step, the top-level + /// `NonCounted(NonCounted(...))` rejects without recursing through + /// the rest. + #[test] + fn serde_rejects_deeply_nested_wrapper_chain() { + // Build NonCounted(NonCounted(NonCounted(...(Item)))). + let depth = 64; + let mut payload = r#"{"Item":[[120],null]}"#.to_string(); + for _ in 0..depth { + payload = format!(r#"{{"NonCounted":{}}}"#, payload); + } + let result: Result = serde_json::from_str(&payload); + assert!(result.is_err(), "depth-{} chain must be rejected", depth); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn display_renders_not_summed_wrapper() { + let inner = Element::SumTree(Some(b"r".to_vec()), 100, None); + let wrapped = Element::new_not_summed(inner).expect("wrap ok"); + let s = format!("{}", wrapped); + assert!(s.starts_with("NotSummed("), "got: {}", s); + assert!(s.contains("SumTree"), "got: {}", s); + } + + #[test] + fn element_type_resolves_not_summed_twins() { + // The four sum-tree variants each map to their NotSummed twin. + let cases: [(Element, ElementType); 4] = [ + ( + Element::NotSummed(Box::new(Element::SumTree(None, 0, None))), + ElementType::NotSummedSumTree, + ), + ( + Element::NotSummed(Box::new(Element::BigSumTree(None, 0, None))), + ElementType::NotSummedBigSumTree, + ), + ( + Element::NotSummed(Box::new(Element::CountSumTree(None, 0, 0, None))), + ElementType::NotSummedCountSumTree, + ), + ( + Element::NotSummed(Box::new(Element::ProvableCountSumTree(None, 0, 0, None))), + ElementType::NotSummedProvableCountSumTree, + ), + ]; + for (element, expected) in cases { + assert_eq!(element.element_type(), expected); + assert_eq!(element.type_str(), expected.as_str()); + } + } + + #[test] + fn element_type_resolves_non_counted_twins() { + // Spot-check a few NonCounted twins to lock in the dispatch. + assert_eq!( + Element::NonCounted(Box::new(Element::Item(b"x".to_vec(), None))).element_type(), + ElementType::NonCountedItem + ); + assert_eq!( + Element::NonCounted(Box::new(Element::SumTree(None, 0, None))).element_type(), + ElementType::NonCountedSumTree + ); + assert_eq!( + Element::NonCounted(Box::new(Element::ProvableCountSumTree(None, 0, 0, None))) + .element_type(), + ElementType::NonCountedProvableCountSumTree + ); + } + + #[test] + fn display_renders_non_counted_wrapper() { + let inner = Element::Item(b"abc".to_vec(), None); + let wrapped = Element::new_non_counted(inner).expect("wrap ok"); + let s = format!("{}", wrapped); + assert!(s.starts_with("NonCounted("), "got: {}", s); + } } diff --git a/grovedb-element/src/element/serialize.rs b/grovedb-element/src/element/serialize.rs index 27f35c9b8..ef9fe1163 100644 --- a/grovedb-element/src/element/serialize.rs +++ b/grovedb-element/src/element/serialize.rs @@ -5,27 +5,48 @@ use bincode::config; use grovedb_version::{check_grovedb_v0, version::GroveVersion}; use crate::{ - element::Element, element_type::NON_COUNTED_WRAPPER_DISCRIMINANT, error::ElementError, + element::Element, + element_type::{NON_COUNTED_WRAPPER_DISCRIMINANT, NOT_SUMMED_WRAPPER_DISCRIMINANT}, + error::ElementError, }; impl Element { /// Serializes self. Returns vector of u8s. /// - /// Rejects `NonCounted(NonCounted(_))` — the wrapper is not allowed to - /// nest. Constructed via `Element::new_non_counted` this is impossible, - /// but a caller could build it directly. + /// 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`). + /// + /// Constructed via `Element::new_non_counted` / `Element::new_not_summed` + /// 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(_)) + && matches!(**inner, Element::NonCounted(_) | Element::NotSummed(_)) { return Err(ElementError::CorruptedData( - "NonCounted cannot wrap another NonCounted".to_string(), + "NonCounted cannot wrap another wrapper".to_string(), )); } + if let Element::NotSummed(inner) = self { + match **inner { + Element::SumTree(..) + | Element::BigSumTree(..) + | Element::CountSumTree(..) + | Element::ProvableCountSumTree(..) => {} + _ => { + return Err(ElementError::CorruptedData( + "NotSummed 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))) @@ -43,32 +64,36 @@ impl Element { /// Deserializes given bytes and sets as self. /// - /// Pre-checks the leading bytes for a nested `NonCounted` wrapper and + /// Pre-checks the leading bytes for any wrapper-byte combination and /// rejects before invoking bincode. This closes a stack-exhaustion - /// vector: a hostile payload of repeated `NON_COUNTED_WRAPPER_DISCRIMINANT` - /// bytes would otherwise cause bincode to recursively decode the - /// `Box` chain (and overflow the stack) before any post-decode - /// check could fire. The pre-check is O(1) — only the first two bytes - /// matter. + /// vector: a hostile payload of repeated wrapper bytes would otherwise + /// cause bincode to recursively decode the `Box` chain (and + /// 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(..))` pub fn deserialize(bytes: &[u8], grove_version: &GroveVersion) -> Result { check_grovedb_v0!( "Element::deserialize", grove_version.grovedb_versions.element.deserialize ); - // Pre-check: if the wire starts with the wrapper discriminant, the - // very next byte must NOT be the wrapper discriminant again. This - // bounds the recursion bincode will attempt. - if matches!( - bytes, - [ - NON_COUNTED_WRAPPER_DISCRIMINANT, - NON_COUNTED_WRAPPER_DISCRIMINANT, - .. - ] - ) { - return Err(ElementError::CorruptedData( - "deserialized NonCounted wrapping another NonCounted".to_string(), - )); + // 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(), + )); + } + _ => {} } let config = config::standard().with_big_endian().with_no_limit(); let elem: Element = bincode::decode_from_slice(bytes, config) @@ -80,12 +105,25 @@ 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(_)) + && matches!(**inner, Element::NonCounted(_) | Element::NotSummed(_)) { return Err(ElementError::CorruptedData( - "deserialized NonCounted wrapping another NonCounted".to_string(), + "deserialized NonCounted wrapping another wrapper".to_string(), )); } + if let Element::NotSummed(inner) = &elem { + match **inner { + Element::SumTree(..) + | Element::BigSumTree(..) + | Element::CountSumTree(..) + | Element::ProvableCountSumTree(..) => {} + _ => { + return Err(ElementError::CorruptedData( + "deserialized NotSummed 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 6ff668b1d..f82f98701 100644 --- a/grovedb-element/src/element/visualize.rs +++ b/grovedb-element/src/element/visualize.rs @@ -181,6 +181,11 @@ impl Visualize for Element { drawer = inner.visualize(drawer)?; drawer.write(b")")?; } + Element::NotSummed(inner) => { + drawer.write(b"not_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 2e5d7d915..1019cf4ce 100644 --- a/grovedb-element/src/element_type.rs +++ b/grovedb-element/src/element_type.rs @@ -21,6 +21,27 @@ pub const NON_COUNTED_FLAG: u8 = 0x80; /// discriminant. pub const NON_COUNTED_BASE_MASK: u8 = 0x7F; +/// Bincode discriminant byte for `Element::NotSummed`. Tied to the +/// declaration order of the `Element` enum (0-indexed, 17th variant). +/// +/// Like `NON_COUNTED_WRAPPER_DISCRIMINANT`, this byte has no direct +/// `ElementType` variant. `from_serialized_value` reads the next byte and +/// resolves to one of the four `NotSummedXxx` synthetic twins. Only the four +/// sum-tree base discriminants are legal as the inner byte. +pub const NOT_SUMMED_WRAPPER_DISCRIMINANT: u8 = 16; + +/// Twin-discriminant prefix for `NotSummedXxx` types: every twin is encoded +/// as `NOT_SUMMED_TWIN_PREFIX | base`. The prefix has the high bit set +/// (so all wrappers cluster in `0x80..` range) plus bits 4 and 5, which +/// distinguishes it from `NON_COUNTED_FLAG`'s `0x80` upper-nibble. Detection +/// is therefore an upper-nibble compare: `disc & 0xf0 == 0xb0`. +pub const NOT_SUMMED_TWIN_PREFIX: u8 = 0xb0; + +/// Mask to recover the base type discriminant from a `NotSummedXxx` +/// discriminant. Base discriminants are `0..=14` (4 bits) so masking the +/// low nibble is sufficient. +pub const NOT_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) @@ -107,6 +128,14 @@ pub enum ProofNodeType { /// the inner element's bytes; `from_serialized_value` synthesizes the /// `NonCountedXxx` variant by peeking at the second byte. /// +/// 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. +/// /// IMPORTANT: Base values (0..=14) must match the order of variants in the /// `Element` enum. The `test_element_serialization_discriminants_match_element_type` /// test catches drift. @@ -175,6 +204,14 @@ pub enum ElementType { NonCountedBulkAppendTree = 141, /// Non-counted wrapper around `DenseAppendOnlyFixedSizeTree` - discriminant 142 NonCountedDenseAppendOnlyFixedSizeTree = 142, + /// Not-summed wrapper around `SumTree` - discriminant 180 (`0xb0 | 4`) + NotSummedSumTree = 180, + /// Not-summed wrapper around `BigSumTree` - discriminant 181 (`0xb0 | 5`) + NotSummedBigSumTree = 181, + /// Not-summed wrapper around `CountSumTree` - discriminant 183 (`0xb0 | 7`) + NotSummedCountSumTree = 183, + /// Not-summed wrapper around `ProvableCountSumTree` - discriminant 186 (`0xb0 | 10`) + NotSummedProvableCountSumTree = 186, } impl ElementType { @@ -207,7 +244,8 @@ impl ElementType { // than NON_COUNTED_WRAPPER_DISCRIMINANT (15). Bytes 15+ are not // valid on-disk inner discriminants: // - 15 itself is the wrapper byte (nested wrappers forbidden), - // - 16..=127 are unallocated, + // - 16 is the NotSummed wrapper byte (cross-nesting forbidden), + // - 17..=127 are unallocated, // - 128..=142 are the synthetic NonCountedXxx twins which // never appear on disk; without this check, the bitwise OR // below would collapse `0x80 | inner_byte` into `inner_byte` @@ -221,27 +259,64 @@ impl ElementType { ))); } Self::try_from(NON_COUNTED_FLAG | inner_byte) + } else if first_byte == NOT_SUMMED_WRAPPER_DISCRIMINANT { + let inner_byte = *serialized_value.get(1).ok_or_else(|| { + ElementError::CorruptedData( + "NotSummed wrapper has no inner element discriminant byte".to_string(), + ) + })?; + // Only the four sum-tree base discriminants are legal here. + // Anything else — including the wrapper bytes 15/16, the + // synthetic twin ranges, and the unrelated base types — is + // rejected so that round-tripping `from_serialized_value` always + // yields a valid `NotSummedXxx` twin. + match inner_byte { + 4 | 5 | 7 | 10 => Self::try_from(NOT_SUMMED_TWIN_PREFIX | inner_byte), + _ => Err(ElementError::CorruptedData(format!( + "NotSummed 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) } } - /// Returns true if this is a `NonCountedXxx` discriminant (bit 7 set). + /// Returns true if this is a `NonCountedXxx` discriminant. Tested by + /// upper-nibble compare since `NotSummedXxx` also has bit 7 set. #[inline] pub const fn is_non_counted(self) -> bool { - (self as u8) & NON_COUNTED_FLAG != 0 + (self as u8) & 0xf0 == NON_COUNTED_FLAG } - /// Returns the underlying base ElementType, stripping the NonCounted bit. - /// For base types, returns `self` unchanged. + /// Returns true if this is a `NotSummedXxx` discriminant. + #[inline] + pub const fn is_not_summed(self) -> bool { + (self as u8) & 0xf0 == NOT_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. #[inline] pub fn base(self) -> ElementType { + let disc = self as u8; if self.is_non_counted() { // Safe: every NonCountedXxx is constructed from a valid base // discriminant 0..=14, so masking the high bit yields a valid // base discriminant. - ElementType::try_from((self as u8) & NON_COUNTED_BASE_MASK) + ElementType::try_from(disc & NON_COUNTED_BASE_MASK) .expect("NonCounted twin always has a valid base") + } else if self.is_not_summed() { + // Safe: every NotSummedXxx is constructed from one of the four + // 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 { self } @@ -423,6 +498,10 @@ impl ElementType { ElementType::NonCountedMmrTree => "non_counted mmr tree", ElementType::NonCountedBulkAppendTree => "non_counted bulk_append_tree", ElementType::NonCountedDenseAppendOnlyFixedSizeTree => "non_counted dense_tree", + ElementType::NotSummedSumTree => "not_summed sum tree", + ElementType::NotSummedBigSumTree => "not_summed big sum tree", + ElementType::NotSummedCountSumTree => "not_summed count sum tree", + ElementType::NotSummedProvableCountSumTree => "not_summed provable count sum tree", } } } @@ -469,6 +548,10 @@ impl TryFrom for ElementType { 140 => Ok(ElementType::NonCountedMmrTree), 141 => Ok(ElementType::NonCountedBulkAppendTree), 142 => Ok(ElementType::NonCountedDenseAppendOnlyFixedSizeTree), + 180 => Ok(ElementType::NotSummedSumTree), + 181 => Ok(ElementType::NotSummedBigSumTree), + 183 => Ok(ElementType::NotSummedCountSumTree), + 186 => Ok(ElementType::NotSummedProvableCountSumTree), _ => Err(ElementError::CorruptedData(format!( "Unknown element type discriminant: {}", value @@ -527,7 +610,7 @@ mod tests { assert!(ElementType::try_from(15).is_err()); assert!(ElementType::try_from(16).is_err()); - // High-bit twins + // NonCounted twins (0x80 | base): 128..142 assert_eq!( ElementType::try_from(128).unwrap(), ElementType::NonCountedItem @@ -540,21 +623,70 @@ mod tests { ElementType::try_from(142).unwrap(), ElementType::NonCountedDenseAppendOnlyFixedSizeTree ); - // Bytes between the base and twin ranges are invalid. + // Bytes between the base and NonCounted-twin ranges are invalid. assert!(ElementType::try_from(127).is_err()); - // Bytes past the highest twin are invalid. + // Bytes between NonCounted-twin and NotSummed-twin ranges are invalid. assert!(ElementType::try_from(143).is_err()); + assert!(ElementType::try_from(179).is_err()); + + // NotSummed twins (0xb0 | base): only the four sum-tree bases + // {4, 5, 7, 10} are legal → discriminants {180, 181, 183, 186}. + assert_eq!( + ElementType::try_from(180).unwrap(), + ElementType::NotSummedSumTree + ); + assert_eq!( + ElementType::try_from(181).unwrap(), + ElementType::NotSummedBigSumTree + ); + assert_eq!( + ElementType::try_from(183).unwrap(), + ElementType::NotSummedCountSumTree + ); + assert_eq!( + ElementType::try_from(186).unwrap(), + ElementType::NotSummedProvableCountSumTree + ); + // Other bytes in 0xb0..=0xbe (non-sum-tree bases) are invalid. + for bad in [ + 0xb0u8, // base 0 (Item) — not a sum-tree variant + 0xb1, // base 1 (Reference) + 0xb2, // base 2 (Tree) + 0xb3, // base 3 (SumItem) — leaf, not a tree + 0xb6, // base 6 (CountTree) + 0xb8, // base 8 (ProvableCountTree) + 0xb9, // base 9 (ItemWithSumItem) + 0xbb, // base 11 (CommitmentTree) + 0xbc, // base 12 (MmrTree) + 0xbd, // base 13 (BulkAppendTree) + 0xbe, // base 14 (DenseAppendOnlyFixedSizeTree) + ] { + assert!( + ElementType::try_from(bad).is_err(), + "{:#x} should be rejected", + bad + ); + } + // Bytes past the highest NotSummed twin are invalid. + assert!(ElementType::try_from(187).is_err()); + assert!(ElementType::try_from(255).is_err()); } #[test] fn test_non_counted_helpers() { - // is_non_counted: high bit means non-counted + // is_non_counted: upper-nibble compare against 0x80. assert!(!ElementType::Item.is_non_counted()); assert!(!ElementType::Tree.is_non_counted()); assert!(ElementType::NonCountedItem.is_non_counted()); 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. + assert!(!ElementType::NotSummedSumTree.is_non_counted()); + assert!(!ElementType::NotSummedProvableCountSumTree.is_non_counted()); + // base() strips the wrapper and returns the underlying type. assert_eq!(ElementType::Item.base(), ElementType::Item); assert_eq!(ElementType::NonCountedItem.base(), ElementType::Item); @@ -575,6 +707,43 @@ mod tests { ); } + #[test] + fn test_not_summed_helpers() { + // is_not_summed: upper-nibble compare against 0xb0. + assert!(!ElementType::Item.is_not_summed()); + assert!(!ElementType::SumTree.is_not_summed()); + assert!(!ElementType::NonCountedSumTree.is_not_summed()); + assert!(ElementType::NotSummedSumTree.is_not_summed()); + assert!(ElementType::NotSummedBigSumTree.is_not_summed()); + assert!(ElementType::NotSummedCountSumTree.is_not_summed()); + assert!(ElementType::NotSummedProvableCountSumTree.is_not_summed()); + + // base() strips the wrapper and returns the underlying type. + assert_eq!(ElementType::NotSummedSumTree.base(), ElementType::SumTree); + assert_eq!( + ElementType::NotSummedBigSumTree.base(), + ElementType::BigSumTree + ); + assert_eq!( + ElementType::NotSummedCountSumTree.base(), + ElementType::CountSumTree + ); + assert_eq!( + ElementType::NotSummedProvableCountSumTree.base(), + ElementType::ProvableCountSumTree + ); + + // The discriminant relationship: twin = base | 0xb0. + assert_eq!( + ElementType::NotSummedSumTree as u8, + ElementType::SumTree as u8 | NOT_SUMMED_TWIN_PREFIX + ); + assert_eq!( + ElementType::NotSummedProvableCountSumTree as u8, + ElementType::ProvableCountSumTree as u8 | NOT_SUMMED_TWIN_PREFIX + ); + } + #[test] fn test_simple_vs_combined_hash() { // Items have simple hash @@ -1061,4 +1230,119 @@ mod tests { ); } } + + /// Pins the bincode discriminant for `Element::NotSummed` to + /// `NOT_SUMMED_WRAPPER_DISCRIMINANT` and the four allowed inner + /// discriminants. Mirrors `test_non_counted_wrapper_discriminant_pinned`. + #[test] + fn test_not_summed_wrapper_discriminant_pinned() { + use grovedb_version::version::GroveVersion; + + use crate::element::Element; + + let grove_version = GroveVersion::latest(); + + let cases: Vec<(Element, ElementType, u8, &str)> = vec![ + ( + Element::NotSummed(Box::new(Element::SumTree(None, 0, None))), + ElementType::NotSummedSumTree, + 4, + "NotSummed(SumTree)", + ), + ( + Element::NotSummed(Box::new(Element::BigSumTree(None, 0, None))), + ElementType::NotSummedBigSumTree, + 5, + "NotSummed(BigSumTree)", + ), + ( + Element::NotSummed(Box::new(Element::CountSumTree(None, 0, 0, None))), + ElementType::NotSummedCountSumTree, + 7, + "NotSummed(CountSumTree)", + ), + ( + Element::NotSummed(Box::new(Element::ProvableCountSumTree(None, 0, 0, None))), + ElementType::NotSummedProvableCountSumTree, + 10, + "NotSummed(ProvableCountSumTree)", + ), + ]; + + for (element, expected_type, expected_inner_disc, name) in cases { + let serialized = element + .serialize(grove_version) + .unwrap_or_else(|e| panic!("Failed to serialize {}: {:?}", name, e)); + + assert!( + serialized.len() >= 2, + "Serialized {} should have at least 2 bytes", + name + ); + assert_eq!( + serialized[0], NOT_SUMMED_WRAPPER_DISCRIMINANT, + "{}: first byte should be the wrapper discriminant (16)", + name + ); + assert_eq!( + serialized[1], expected_inner_disc, + "{}: second byte should match the inner element's discriminant", + name + ); + + let parsed = ElementType::from_serialized_value(&serialized) + .unwrap_or_else(|e| panic!("Failed to parse {}: {:?}", name, e)); + assert_eq!( + parsed, expected_type, + "{}: from_serialized_value returned {:?}, expected {:?}", + name, parsed, expected_type + ); + // The synthetic discriminant follows the 0xb0|base rule. + assert_eq!( + parsed as u8, + expected_inner_disc | NOT_SUMMED_TWIN_PREFIX, + "{}: NotSummedXxx = inner_disc | 0xb0", + name + ); + } + } + + /// Validate the new resolver paths around byte 16 (NotSummed wrapper). + #[test] + fn test_from_serialized_value_not_summed_paths() { + // Truncated wrapper (no inner byte) is rejected. + assert!(ElementType::from_serialized_value(&[16]).is_err()); + + // Each of the four legal inner discriminants resolves to the right + // synthetic twin. + assert_eq!( + ElementType::from_serialized_value(&[16, 4]).unwrap(), + ElementType::NotSummedSumTree + ); + assert_eq!( + ElementType::from_serialized_value(&[16, 5]).unwrap(), + ElementType::NotSummedBigSumTree + ); + assert_eq!( + ElementType::from_serialized_value(&[16, 7]).unwrap(), + ElementType::NotSummedCountSumTree + ); + assert_eq!( + ElementType::from_serialized_value(&[16, 10]).unwrap(), + ElementType::NotSummedProvableCountSumTree + ); + + // 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. + for bad in [ + 0u8, 1, 2, 3, 6, 8, 9, 11, 12, 13, 14, 15, 16, 17, 100, 128, 142, 180, 186, 200, 255, + ] { + assert!( + ElementType::from_serialized_value(&[16, bad]).is_err(), + "[16, {}] should be rejected", + bad + ); + } + } } diff --git a/grovedb-query/src/proofs/tree_feature_type.rs b/grovedb-query/src/proofs/tree_feature_type.rs index fc5d604aa..b5fa81cb0 100644 --- a/grovedb-query/src/proofs/tree_feature_type.rs +++ b/grovedb-query/src/proofs/tree_feature_type.rs @@ -283,3 +283,50 @@ impl Decode for TreeFeatureType { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn zero_count_only_zeros_count() { + let mut basic = BasicMerkNode; + basic.zero_count(); + assert_eq!(basic, BasicMerkNode); + + let mut summed = SummedMerkNode(42); + summed.zero_count(); + assert_eq!(summed, SummedMerkNode(42)); + + let mut big_summed = BigSummedMerkNode(42); + big_summed.zero_count(); + assert_eq!(big_summed, BigSummedMerkNode(42)); + + let mut counted = CountedMerkNode(7); + counted.zero_count(); + assert_eq!(counted, CountedMerkNode(0)); + + let mut count_sum = CountedSummedMerkNode(7, 42); + count_sum.zero_count(); + assert_eq!(count_sum, CountedSummedMerkNode(0, 42)); + + let mut prov_counted = ProvableCountedMerkNode(7); + prov_counted.zero_count(); + assert_eq!(prov_counted, ProvableCountedMerkNode(0)); + + let mut prov_count_sum = ProvableCountedSummedMerkNode(7, 42); + prov_count_sum.zero_count(); + assert_eq!(prov_count_sum, ProvableCountedSummedMerkNode(0, 42)); + } + + #[test] + fn count_helper_returns_some_only_for_count_bearing() { + assert_eq!(BasicMerkNode.count(), None); + assert_eq!(SummedMerkNode(42).count(), None); + assert_eq!(BigSummedMerkNode(42).count(), None); + assert_eq!(CountedMerkNode(7).count(), Some(7)); + assert_eq!(CountedSummedMerkNode(7, 42).count(), Some(7)); + assert_eq!(ProvableCountedMerkNode(7).count(), Some(7)); + assert_eq!(ProvableCountedSummedMerkNode(7, 42).count(), Some(7)); + } +} diff --git a/grovedb/src/batch/mod.rs b/grovedb/src/batch/mod.rs index aeb117956..3a2f8e7da 100644 --- a/grovedb/src/batch/mod.rs +++ b/grovedb/src/batch/mod.rs @@ -283,6 +283,12 @@ pub enum GroveOp { /// element is re-wrapped so the on-disk bytes preserve the wrapper /// and the parent count tree's aggregate excludes the subtree. non_counted: bool, + /// True if the original element was wrapped in `Element::NotSummed`. + /// Set during propagation; on execution the reconstructed + /// sum-tree element is re-wrapped so the on-disk bytes preserve + /// the wrapper and the parent sum tree's running sum excludes + /// the subtree. + not_summed: bool, }, /// **Internal only — do not construct directly.** /// Replace root hash for a non-Merk tree (CommitmentTree, MmrTree, @@ -1490,9 +1496,11 @@ where )) .wrap_with_cost(cost), // underlying() unwraps a single level; the constructor and - // (de)serializer reject nested NonCounted, so this is unreachable - // by construction. - Element::NonCounted(_) => unreachable!("NonCounted may not nest"), + // (de)serializer reject nested wrappers, so these are + // unreachable by construction. + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("wrappers may not nest") + } } } @@ -1643,8 +1651,10 @@ where )) .wrap_with_cost(cost) } - // NonCounted is unwrapped via underlying() above. - Element::NonCounted(_) => unreachable!("unwrapped above"), + // Wrappers are unwrapped via underlying() above. + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } } } GroveOp::InsertWithKnownToNotAlreadyExist { element } @@ -1688,8 +1698,10 @@ where )) .wrap_with_cost(cost) } - // NonCounted is unwrapped via underlying() above. - Element::NonCounted(_) => unreachable!("unwrapped above"), + // Wrappers are unwrapped via underlying() above. + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } }, GroveOp::RefreshReference { reference_path_type, @@ -1873,10 +1885,10 @@ where } } - // Mirror the per-merk insert guard: NonCounted children - // are only valid inside count-bearing parents. Without - // this check, batch users could persist NonCounted - // elements into Normal/Sum/Big-Sum trees and silently + // Mirror the per-merk insert guard: wrapper children are + // only valid inside the matching aggregate-bearing parents. + // Without these checks, batch users could persist + // wrapped elements into the wrong tree types and silently // violate the wrapper invariant. if element.is_non_counted() && !in_tree_type.is_count_bearing() { return Err(Error::InvalidBatchOperation( @@ -1884,6 +1896,12 @@ where )) .wrap_with_cost(cost); } + if element.is_not_summed() && !in_tree_type.is_sum_bearing() { + return Err(Error::InvalidBatchOperation( + "not-summed elements may only be inserted into sum-bearing trees", + )) + .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 @@ -2078,8 +2096,10 @@ where ); } } - // NonCounted is unwrapped via underlying() above. - Element::NonCounted(_) => unreachable!("unwrapped above"), + // Wrappers are unwrapped via underlying() above. + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } } } GroveOp::RefreshReference { @@ -2250,6 +2270,7 @@ where flags, aggregate_data, non_counted, + not_summed, } => { // Standard Merk trees — infer element from aggregate_data let element = match aggregate_data { @@ -2292,14 +2313,34 @@ where Element::ProvableCountSumTree(root_key, count_value, sum_value, flags) } }; - // Re-wrap if the original element was NonCounted, so the - // on-disk bytes preserve the wrapper and the parent - // count tree's aggregate excludes this subtree. + // 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 + // `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 + // invariant is ever violated by a future change. let element = if non_counted { - element.into_non_counted() + element.into_non_counted().map_err(|_| { + Error::CorruptedCodeExecution( + "into_non_counted called on a wrapped element during \ + InsertTreeWithRootHash propagation", + ) + }) + } else if not_summed { + element.into_not_summed().map_err(|_| { + Error::CorruptedCodeExecution( + "into_not_summed called on a non-sum-tree or wrapped element \ + during InsertTreeWithRootHash propagation", + ) + }) } else { - element + Ok(element) }; + let element = cost_return_on_error_no_add!(cost, element); let merk_feature_type = cost_return_on_error_into_no_add!( cost, element.get_feature_type(in_tree_type) @@ -2325,9 +2366,18 @@ where .. } => { let element = meta.to_element(flags); - // Re-wrap as above for the non-Merk tree path. + // Re-wrap as above for the non-Merk tree path. `element` + // is freshly built from `meta.to_element(...)` so it is + // never a pre-existing wrapper — surface a typed error + // if a future change ever violates that. let element = if non_counted { - element.into_non_counted() + let wrapped = element.into_non_counted().map_err(|_| { + Error::CorruptedCodeExecution( + "into_non_counted called on a wrapped element during \ + InsertNonMerkTree propagation", + ) + }); + cost_return_on_error_no_add!(cost, wrapped) } else { element }; @@ -2441,11 +2491,13 @@ where // we need to give back the value defined cost in the case that the // new element is a tree. // - // Look through `NonCounted` for the cost 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() { + // Look through wrapper variants for the cost + // 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 @@ -2672,17 +2724,21 @@ impl GroveDb { | GroveOp::InsertIfNotExists { element, .. } | GroveOp::Replace { element } | GroveOp::Patch { element, .. } => { - // Look through NonCounted: a wrapped tree + // Look through wrappers: a wrapped tree // still needs to be converted into the // appropriate InsertTreeWithRootHash / // InsertNonMerkTree variant during - // upward propagation. Capture the wrapper + // upward propagation. Capture wrapper // status so execution can re-wrap the // reconstructed element — otherwise the // wrapper byte would be silently dropped - // from storage and the parent count tree - // would aggregate a value it should not. + // 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. let non_counted = element.is_non_counted(); + let not_summed = element.is_not_summed(); let element = element.underlying(); // Standard Merk trees if let Element::Tree(_, flags) = element { @@ -2694,6 +2750,7 @@ impl GroveDb { aggregate_data: AggregateData::NoAggregateData, non_counted, + not_summed, } } else if let Element::SumTree(.., flags) = element @@ -2705,6 +2762,7 @@ impl GroveDb { flags: flags.clone(), aggregate_data, non_counted, + not_summed, } } else if let Element::BigSumTree(.., flags) = element @@ -2716,6 +2774,7 @@ impl GroveDb { flags: flags.clone(), aggregate_data, non_counted, + not_summed, } } else if let Element::CountTree(.., flags) = element @@ -2727,6 +2786,7 @@ impl GroveDb { flags: flags.clone(), aggregate_data, non_counted, + not_summed, } } else if let Element::CountSumTree(.., flags) = element @@ -2738,6 +2798,7 @@ impl GroveDb { flags: flags.clone(), aggregate_data, non_counted, + not_summed, } } else if let Element::ProvableCountTree( .., @@ -2751,6 +2812,7 @@ impl GroveDb { flags: flags.clone(), aggregate_data, non_counted, + not_summed, } } else if let Element::ProvableCountSumTree( .., @@ -2764,8 +2826,11 @@ impl GroveDb { flags: flags.clone(), aggregate_data, non_counted, + not_summed, } // Non-Merk trees → InsertNonMerkTree + // (none of these can be NotSummed — + // 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 c6e7cad6c..1c82b3c2e 100644 --- a/grovedb/src/debugger.rs +++ b/grovedb/src/debugger.rs @@ -811,9 +811,11 @@ fn element_to_grovedbg(element: crate::Element) -> grovedbg_types::Element { element_flags, } } - // The visualizer wire format has no NonCounted variant; render the + // 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) => element_to_grovedbg(*inner), + crate::Element::NonCounted(inner) | crate::Element::NotSummed(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 49c9661ed..1230429f7 100644 --- a/grovedb/src/estimated_costs/average_case_costs.rs +++ b/grovedb/src/estimated_costs/average_case_costs.rs @@ -326,10 +326,14 @@ impl GroveDb { let mut cost = OperationCost::default(); let key_len = key.max_length() as u32; - // Look through `NonCounted` for cost dispatch (the wrapper byte is - // accounted for via `wrapper_overhead` parallel to + // 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() { 1u32 } else { 0 }; + let wrapper_overhead = if value.is_non_counted() || value.is_not_summed() { + 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 2c5d07fa7..ea72cae7b 100644 --- a/grovedb/src/estimated_costs/worst_case_costs.rs +++ b/grovedb/src/estimated_costs/worst_case_costs.rs @@ -186,8 +186,12 @@ impl GroveDb { let mut cost = OperationCost::default(); let key_len = key.max_length() as u32; - // Look through `NonCounted` for cost dispatch. - let wrapper_overhead = if value.is_non_counted() { 1u32 } else { 0 }; + // Look through wrapper variants for cost dispatch. + let wrapper_overhead = if value.is_non_counted() || value.is_not_summed() { + 1u32 + } else { + 0 + }; match value.underlying() { Element::Tree(_, flags) | Element::SumTree(_, _, flags) @@ -243,8 +247,12 @@ impl GroveDb { let mut cost = OperationCost::default(); let key_len = key.max_length() as u32; - // Look through `NonCounted` for cost dispatch. - let wrapper_overhead = if value.is_non_counted() { 1u32 } else { 0 }; + // Look through wrapper variants for cost dispatch. + let wrapper_overhead = if value.is_non_counted() || value.is_not_summed() { + 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 a11436537..127503a5e 100644 --- a/grovedb/src/lib.rs +++ b/grovedb/src/lib.rs @@ -1110,7 +1110,9 @@ impl GroveDb { ); } } - Element::NonCounted(_) => unreachable!("unwrapped above"), + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } } } Ok(issues) diff --git a/grovedb/src/operations/get/query.rs b/grovedb/src/operations/get/query.rs index 046f580e6..d6b5d1e03 100644 --- a/grovedb/src/operations/get/query.rs +++ b/grovedb/src/operations/get/query.rs @@ -271,7 +271,9 @@ where { | Element::DenseAppendOnlyFixedSizeTree(..) => { Err(Error::InvalidQuery("path_queries can not refer to trees")) } - Element::NonCounted(_) => unreachable!("unwrapped above"), + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } } } @@ -405,7 +407,9 @@ where { | Element::DenseAppendOnlyFixedSizeTree(..) => Err(Error::InvalidQuery( "path_queries can only refer to items and references", )), - Element::NonCounted(_) => unreachable!("unwrapped above"), + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } } } _ => Err(Error::CorruptedCodeExecution( @@ -558,7 +562,9 @@ where { "path_queries can only refer to items, sum items, references and sum \ trees", )), - Element::NonCounted(_) => unreachable!("unwrapped above"), + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } } } _ => Err(Error::CorruptedCodeExecution( @@ -741,7 +747,9 @@ where { "path_queries over sum items can only refer to sum items and \ references", )), - Element::NonCounted(_) => unreachable!("unwrapped above"), + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } } } _ => Err(Error::CorruptedCodeExecution( diff --git a/grovedb/src/operations/insert/mod.rs b/grovedb/src/operations/insert/mod.rs index a888a089a..dd53c6370 100644 --- a/grovedb/src/operations/insert/mod.rs +++ b/grovedb/src/operations/insert/mod.rs @@ -347,13 +347,13 @@ impl GroveDb { ) ); } - // `underlying()` only unwraps one level; nested NonCounted is + // `underlying()` only unwraps one level; nested wrappers are // 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::NonCounted(_) | Element::NotSummed(_) => { return Err(Error::InvalidInput( - "nested NonCounted wrappers are not allowed", + "nested element wrappers are not allowed", )) .wrap_with_cost(cost); } diff --git a/grovedb/src/operations/proof/generate.rs b/grovedb/src/operations/proof/generate.rs index 34e0593ce..6f136e466 100644 --- a/grovedb/src/operations/proof/generate.rs +++ b/grovedb/src/operations/proof/generate.rs @@ -576,7 +576,9 @@ impl GroveDb { | Ok(Element::BulkAppendTree(..)) | Ok(Element::DenseAppendOnlyFixedSizeTree(..)) => continue, // NonCounted is unwrapped above via into_underlying(). - Ok(Element::NonCounted(_)) => unreachable!("unwrapped above"), + Ok(Element::NonCounted(_)) | Ok(Element::NotSummed(_)) => { + unreachable!("unwrapped above") + } Err(e) => { return Err(Error::CorruptedData(format!( "failed to deserialize element during proof generation: {e}" @@ -1440,7 +1442,9 @@ impl GroveDb { | Ok(Element::BulkAppendTree(..)) | Ok(Element::DenseAppendOnlyFixedSizeTree(..)) => continue, // NonCounted is unwrapped above via into_underlying(). - Ok(Element::NonCounted(_)) => unreachable!("unwrapped above"), + Ok(Element::NonCounted(_)) | Ok(Element::NotSummed(_)) => { + unreachable!("unwrapped above") + } Err(e) => { return Err(Error::CorruptedData(format!( "failed to deserialize element during proof generation: {e}" diff --git a/grovedb/src/operations/proof/verify.rs b/grovedb/src/operations/proof/verify.rs index 1f8120893..60ab08d35 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(_) => unreachable!("unwrapped above"), + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } } } else if element.is_any_item() || !internal_query.has_subquery_or_matching_in_path_on_key(key) @@ -1618,7 +1620,9 @@ impl GroveDb { "Proof has lower layer for a non Tree.".to_string(), )); } - Element::NonCounted(_) => unreachable!("unwrapped above"), + Element::NonCounted(_) | Element::NotSummed(_) => { + unreachable!("unwrapped above") + } } } else if element.is_any_item() || !internal_query.has_subquery_or_matching_in_path_on_key(key) diff --git a/grovedb/src/reference_path.rs b/grovedb/src/reference_path.rs index 144f4f8d0..1239306f2 100644 --- a/grovedb/src/reference_path.rs +++ b/grovedb/src/reference_path.rs @@ -84,7 +84,15 @@ pub(crate) fn follow_reference<'db, 'b, 'c, B: AsRef<[u8]>>( }) ); - match element { + // Look through wrapper variants so a wrapper-wrapped reference is + // followed instead of being returned as a value, mirroring the + // unwrapping in `GroveDb::follow_reference`. The wrapper byte is + // already part of `value_hash` (computed above from the on-disk + // serialized value), so dropping it from `target_element` does not + // affect downstream cryptographic verification. `NotSummed` cannot + // wrap a reference by construction (whitelist), but the unwrap is + // forward-safe and symmetric to `NonCounted`. + match element.into_underlying() { Element::Reference(ref_path, ..) => { current_path = referred_path; current_key = referred_key; diff --git a/grovedb/src/tests/batch_coverage_tests.rs b/grovedb/src/tests/batch_coverage_tests.rs index 4f6969f10..2ecb90bc7 100644 --- a/grovedb/src/tests/batch_coverage_tests.rs +++ b/grovedb/src/tests/batch_coverage_tests.rs @@ -541,6 +541,7 @@ mod tests { aggregate_data: AggregateData::NoAggregateData, non_counted: false, + not_summed: false, }, }; diff --git a/grovedb/src/tests/batch_rejection_tests.rs b/grovedb/src/tests/batch_rejection_tests.rs index f1d63eb76..b9cf042d8 100644 --- a/grovedb/src/tests/batch_rejection_tests.rs +++ b/grovedb/src/tests/batch_rejection_tests.rs @@ -76,6 +76,7 @@ fn test_apply_batch_rejects_insert_tree_with_root_hash() { aggregate_data: AggregateData::NoAggregateData, non_counted: false, + not_summed: false, }, }; diff --git a/grovedb/src/tests/batch_unit_tests.rs b/grovedb/src/tests/batch_unit_tests.rs index 18423b26d..7a61e47f3 100644 --- a/grovedb/src/tests/batch_unit_tests.rs +++ b/grovedb/src/tests/batch_unit_tests.rs @@ -95,6 +95,7 @@ mod tests { aggregate_data: AggregateData::NoAggregateData, non_counted: false, + not_summed: false, }, GroveOp::ReplaceTreeRootKey { // 4 @@ -473,6 +474,7 @@ mod tests { aggregate_data: AggregateData::NoAggregateData, non_counted: false, + not_summed: false, }, }; let dbg = format!("{:?}", internal_op2); diff --git a/grovedb/src/tests/mod.rs b/grovedb/src/tests/mod.rs index 1aded513f..e6cd67617 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_summed_tests; mod operations_coverage_tests; mod partial_batch_consistency_tests; mod proof_advanced_tests; diff --git a/grovedb/src/tests/not_summed_tests.rs b/grovedb/src/tests/not_summed_tests.rs new file mode 100644 index 000000000..9f7991cc0 --- /dev/null +++ b/grovedb/src/tests/not_summed_tests.rs @@ -0,0 +1,330 @@ +//! Regression tests for `Element::NotSummed` end-to-end behavior. +//! +//! Symmetric to `non_counted_tests.rs`. The wrapper: +//! - May only be inserted into sum-bearing parents (`SumTree`, `BigSumTree`, +//! `CountSumTree`, `ProvableCountSumTree`). +//! - Inner element must be one of those four sum-tree variants. +//! - Contributes 0 to the parent's running sum; counts still propagate. + +#[cfg(test)] +mod tests { + use grovedb_version::version::GroveVersion; + + use crate::{ + batch::QualifiedGroveDbOp, + tests::{make_test_grovedb, TEST_LEAF}, + Element, + }; + + /// Establish a sum-tree under `TEST_LEAF/` rooted at the given key + /// for use as a host parent in the tests below. + fn make_sum_tree_parent(db: &crate::GroveDb, key: &[u8], grove_version: &GroveVersion) { + db.insert( + [TEST_LEAF].as_ref(), + key, + Element::empty_sum_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert sum tree"); + } + + #[test] + fn batch_insert_rejects_not_summed_into_normal_tree() { + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + // TEST_LEAF is a normal tree; inserting a NotSummed-wrapped sum tree + // into it via batch must be rejected, mirroring the per-merk + // insert guard. + let ns = Element::new_not_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(), ns); + + let err = db + .apply_batch(vec![op], None, None, grove_version) + .unwrap() + .expect_err("batch insert of NotSummed into NormalTree must fail"); + let msg = format!("{err:?}"); + assert!( + msg.contains("not-summed") || msg.contains("not_summed"), + "expected NotSummed parent-type guard error, got: {msg}" + ); + } + + #[test] + fn batch_insert_rejects_not_summed_into_count_tree() { + // CountTree is not sum-bearing, so NotSummed must be rejected. + 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 ns = Element::new_not_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(), + ns, + ); + + let err = db + .apply_batch(vec![op], None, None, grove_version) + .unwrap() + .expect_err("batch insert of NotSummed into CountTree must fail"); + let msg = format!("{err:?}"); + assert!( + msg.contains("not-summed") || msg.contains("not_summed"), + "expected NotSummed parent-type guard error, got: {msg}" + ); + } + + #[test] + fn direct_insert_not_summed_in_sum_tree_excludes_subtree_sum() { + // A bare SumTree(_, 100) inside a SumTree contributes 100 to the + // parent's running sum. Wrapped in NotSummed, it contributes 0. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + make_sum_tree_parent(&db, b"outer", grove_version); + + // Bare sum item contributes 7 to the parent's sum. + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"plain", + Element::new_sum_item(7), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert plain sum item"); + + // A wrapped SumTree subtree must contribute 0 to the parent's + // running sum even though it has its own internal sum aggregate. + let inner_st = Element::new_sum_tree_with_flags_and_sum_value(None, 0, None); + let wrapped = Element::new_not_summed(inner_st).expect("wrap ok"); + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"ns", + 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"ns"].as_ref(), + b"inner_item", + Element::new_sum_item(99), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert inner sum item"); + + // The outer sum tree's running aggregate must be 7 (only `plain` + // contributes; the wrapped subtree's internal 99 is suppressed). + 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 outer aggregate data"); + assert_eq!( + aggregate.as_sum_i64(), + 7, + "wrapped sum tree subtree must not contribute to outer sum tree's aggregate; got {:?}", + aggregate + ); + } + + #[test] + fn batch_propagation_preserves_not_summed_wrapper_on_subtree() { + // A batch that inserts a NotSummed(SumTree) AND writes a child under + // it forces the propagation path through InsertTreeWithRootHash. + // The on-disk parent element must come back wrapped, and the outer + // sum tree's aggregate must exclude the subtree's sum. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + // Outer sum tree. + make_sum_tree_parent(&db, b"outer", grove_version); + + // Plain sum item contributes 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 NotSummed(SumTree) under outer + a sum-item child + // under that wrapped tree, forcing the propagation path. + let ns_inner = Element::new_not_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"ns".to_vec(), + ns_inner, + ), + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"outer".to_vec(), b"ns".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/ns must STILL be NotSummed after + // propagation. + let stored = db + .get_raw( + grovedb_path::SubtreePath::from(&[TEST_LEAF, b"outer"]), + b"ns", + None, + grove_version, + ) + .unwrap() + .expect("get_raw ns"); + assert!( + matches!(stored, Element::NotSummed(_)), + "wrapper must survive batch propagation; got {:?}", + stored + ); + + // The outer sum tree's aggregate must NOT include the wrapped + // subtree's internal sum (123). Only `plain` (5) should + // contribute. If propagation dropped the wrapper, the subtree's + // 123 would bubble up to 128. + 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 outer aggregate data"); + assert_eq!( + aggregate.as_sum_i64(), + 5, + "not-summed subtree must not contribute to outer sum tree's running sum; got {:?}", + aggregate + ); + } + + #[test] + fn batch_insert_rejects_cross_wrapper_non_counted_around_not_summed() { + // A hand-built `NonCounted(NotSummed(SumTree))` would bypass the + // constructor (which now rejects cross-wrapper nesting) and could + // reach batch execution with `is_non_counted() == true` and + // `underlying() == NotSummed`. The batch path must reject it + // explicitly rather than hitting the supposedly-unreachable + // wrapper arm. + // + // We assemble the cross-wrapper element directly (bypassing + // `new_non_counted`) to exercise the validation path. With the + // P1 fix, `apply_batch` rejects this with an `InvalidInput` / + // `InvalidBatchOperation` rather than panicking. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + // Need a sum-bearing parent so the standard NotSummed parent-type + // guard does not fire first; we want to exercise the cross-wrapper + // detection. + make_sum_tree_parent(&db, b"outer", grove_version); + + let inner = Element::SumTree(None, 0, None); + let bad = Element::NonCounted(Box::new(Element::NotSummed(Box::new(inner)))); + let op = QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"outer".to_vec()], + b"k".to_vec(), + bad, + ); + + // Either the bincode serialize on its way to storage or the batch + // parent-type guard rejects it — both are acceptable typed + // failures. The important thing is that we do not panic. + let result = db.apply_batch(vec![op], None, None, grove_version).unwrap(); + assert!( + result.is_err(), + "cross-wrapper NonCounted(NotSummed(_)) must be rejected, got {:?}", + result + ); + } + + #[test] + fn check_subtree_exists_through_not_summed_wrapper() { + // A NotSummed-wrapped tree at the parent path must satisfy + // check_subtree_exists, otherwise APIs that gate on it (e.g. + // inserts into the wrapped tree) would reject paths through + // wrapped parents. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + make_sum_tree_parent(&db, b"outer", grove_version); + + // A NotSummed(SumTree) inside the outer sum tree. + db.insert( + [TEST_LEAF, b"outer"].as_ref(), + b"ns", + Element::new_not_summed(Element::new_sum_tree(None)).expect("wrap ok"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert ns inner"); + + // Inserting into the wrapped subtree exercises check_subtree_exists + // on a path whose parent is `NotSummed(SumTree)` — should succeed. + db.insert( + [TEST_LEAF, b"outer", b"ns"].as_ref(), + b"child", + Element::new_sum_item(42), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert into wrapped subtree must succeed"); + } +} diff --git a/merk/src/element/costs.rs b/merk/src/element/costs.rs index 45d35cde3..b30efa021 100644 --- a/merk/src/element/costs.rs +++ b/merk/src/element/costs.rs @@ -51,8 +51,9 @@ pub trait ElementCostPrivateExtensions { } impl ElementCostPrivateExtensions for Element { - /// Get tree cost for the element. For `NonCounted`, delegates to the - /// inner element and adds 1 byte for the wrapper discriminant. + /// Get tree cost for the element. For `NonCounted` and `NotSummed`, + /// 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", @@ -71,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) => Ok(inner.get_specialized_cost(grove_version)? + 1), + Element::NonCounted(inner) | Element::NotSummed(inner) => { + Ok(inner.get_specialized_cost(grove_version)? + 1) + } _ => Err(Error::CorruptedCodeExecution( "trying to get tree cost from non tree element", )), @@ -96,14 +99,14 @@ impl ElementCostExtensions for Element { ); // todo: we actually don't need to deserialize the whole element let element = Element::deserialize(value, grove_version)?; - // Look through NonCounted: the wrapper has no per-variant cost - // semantics — the cost is determined by the inner element's type. - // For the catch-all (Item / Reference) path, value.len() already - // includes the wrapper byte. For tree- and sum-item paths that use - // cost-size constants, we add 1 byte of `wrapper_overhead` to keep - // the on-disk byte count exact. + // Look through wrapper variants: the wrappers have no per-variant + // cost semantics — the cost is determined by the inner element's + // type. For the catch-all (Item / Reference) path, value.len() + // already includes the wrapper byte. For tree- and sum-item paths + // 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) => (*inner, 1u32), + Element::NonCounted(inner) | Element::NotSummed(inner) => (*inner, 1u32), other => (other, 0u32), }; let cost = match element { @@ -251,9 +254,9 @@ impl ElementCostExtensions for Element { let key_len = key.len() as u32; KV::node_value_byte_cost_size(key_len, value_len, node_type) } - // Item / Reference / NonCounted-of-NonCounted (impossible by - // construction): catch-all uses raw value.len() which already - // includes any wrapper byte present. + // Item / Reference / nested wrappers (impossible by construction): + // catch-all uses raw value.len() which already includes any + // wrapper byte present. _ => KV::node_value_byte_cost_size(key.len() as u32, value.len() as u32, node_type), }; Ok(cost) diff --git a/merk/src/element/get.rs b/merk/src/element/get.rs index c7968a8de..10e372539 100644 --- a/merk/src/element/get.rs +++ b/merk/src/element/get.rs @@ -408,15 +408,21 @@ impl ElementFetchFromStoragePrivateExtensions for Element { }) .transpose() ); - // Look through `NonCounted` for cost computation: the wrapper does - // not change which cost path applies — its inner element type does. - // For the catch-all (Item / Reference) path, value.len() already - // includes the wrapper byte. For tree- and sum-item paths that use - // cost-size constants, add `wrapper_overhead` to keep accounting - // exact. + // Look through wrapper variants for cost computation: the wrappers + // do not change which cost path applies — the inner element type + // does. For the catch-all (Item / Reference) path, value.len() + // already includes the wrapper byte. For tree- and sum-item paths + // that use cost-size constants, add `wrapper_overhead` to keep + // accounting exact. let wrapper_overhead = element .as_ref() - .map(|e| if e.is_non_counted() { 1u32 } else { 0 }) + .map(|e| { + if e.is_non_counted() || e.is_not_summed() { + 1u32 + } else { + 0 + } + }) .unwrap_or(0); let element_for_cost = element.as_ref().map(|e| e.underlying()); match element_for_cost { @@ -477,10 +483,10 @@ impl ElementFetchFromStoragePrivateExtensions for Element { NodeType::NormalNode, ) as u64 } - // NonCounted wrappers are unwrapped above; reaching this arm means + // 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::NonCounted(_)) | Some(Element::NotSummed(_)) => {} None => {} } Ok(element).wrap_with_cost(cost) @@ -523,10 +529,14 @@ impl ElementFetchFromStoragePrivateExtensions for Element { Error::CorruptedData(format!("unable to deserialize element: {e}")) }) ); - // Look through `NonCounted` 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() { 1u32 } else { 0 }; + // 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 element_for_cost = element.underlying(); match element_for_cost { Element::Item(..) | Element::Reference(..) => { @@ -590,8 +600,8 @@ impl ElementFetchFromStoragePrivateExtensions for Element { node_type, ) as u64 } - // NonCounted wrappers are unwrapped above. - Element::NonCounted(_) => {} + // Wrappers are unwrapped above. + Element::NonCounted(_) | Element::NotSummed(_) => {} } Ok(Some(element)).wrap_with_cost(cost) } diff --git a/merk/src/element/insert.rs b/merk/src/element/insert.rs index 90e5e22c9..ab738b94a 100644 --- a/merk/src/element/insert.rs +++ b/merk/src/element/insert.rs @@ -170,6 +170,13 @@ impl ElementInsertToStorageExtensions for Element { .wrap_with_cost(Default::default()); } + if self.is_not_summed() && !merk.tree_type.is_sum_bearing() { + return Err(Error::InvalidInputError( + "not-summed elements may only be inserted into sum-bearing trees", + )) + .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", @@ -441,6 +448,13 @@ impl ElementInsertToStorageExtensions for Element { .wrap_with_cost(Default::default()); } + if self.is_not_summed() && !merk.tree_type.is_sum_bearing() { + return Err(Error::InvalidInputError( + "not-summed elements may only be inserted into sum-bearing trees", + )) + .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()), @@ -533,6 +547,13 @@ impl ElementInsertToStorageExtensions for Element { .wrap_with_cost(Default::default()); } + if self.is_not_summed() && !merk.tree_type.is_sum_bearing() { + return Err(Error::InvalidInputError( + "not-summed elements may only be inserted into sum-bearing trees", + )) + .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()), @@ -833,6 +854,96 @@ mod tests { assert!(matches!(result, Err(Error::InvalidInputError(_)))); } + #[test] + fn not_summed_rejected_in_normal_tree() { + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::NormalTree); + let ns = Element::new_not_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let result = ns.insert(&mut merk, b"k", None, grove_version).unwrap(); + assert!(matches!(result, Err(Error::InvalidInputError(_)))); + } + + #[test] + fn not_summed_rejected_in_count_tree() { + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::CountTree); + let ns = Element::new_not_summed(Element::new_sum_tree(None)).expect("wrap ok"); + let result = ns + .insert_subtree(&mut merk, b"k", [0u8; 32], None, grove_version) + .unwrap(); + assert!(matches!(result, Err(Error::InvalidInputError(_)))); + } + + #[test] + fn not_summed_constructor_rejects_non_sum_tree_inner() { + // Items, references, plain trees, and non-sum-tree variants must all + // be rejected at construction time. + assert!(Element::new_not_summed(Element::new_item(b"x".to_vec())).is_err()); + assert!(Element::new_not_summed(Element::new_sum_item(7)).is_err()); + assert!(Element::new_not_summed(Element::new_tree(None)).is_err()); + assert!(Element::new_not_summed(Element::new_count_tree(None)).is_err()); + assert!(Element::new_not_summed(Element::new_provable_count_tree(None)).is_err()); + // Wrappers cannot nest. + let nc = Element::new_non_counted(Element::new_sum_tree(None)).expect("wrap ok"); + assert!(Element::new_not_summed(nc).is_err()); + let ns = Element::new_not_summed(Element::new_sum_tree(None)).expect("wrap ok"); + assert!(Element::new_not_summed(ns).is_err()); + // The four sum-tree variants are accepted. + assert!(Element::new_not_summed(Element::new_sum_tree(None)).is_ok()); + assert!(Element::new_not_summed(Element::new_big_sum_tree(None)).is_ok()); + assert!(Element::new_not_summed(Element::new_count_sum_tree(None)).is_ok()); + assert!(Element::new_not_summed(Element::new_provable_count_sum_tree(None)).is_ok()); + } + + #[test] + fn not_summed_accepted_in_sum_tree_contributes_zero_sum() { + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::SumTree); + + // One bare sum item contributes 7. + Element::new_sum_item(7) + .insert(&mut merk, b"k1", None, grove_version) + .unwrap() + .expect("insert k1"); + // A bare SumTree(_, 100, _) child via insert_subtree would + // contribute 100. The wrapped version must contribute 0. + let ns_subtree = Element::new_not_summed(Element::new_sum_tree_with_flags_and_sum_value( + None, 100, None, + )) + .expect("wrap ok"); + ns_subtree + .insert_subtree(&mut merk, b"k2", [0u8; 32], None, grove_version) + .unwrap() + .expect("insert wrapped sum tree subtree"); + + let agg = merk.aggregate_data().expect("aggregate ok"); + assert_eq!( + agg.as_sum_i64(), + 7, + "wrapped sum tree's 100 should be suppressed; only the bare sum item contributes" + ); + } + + #[test] + fn not_summed_in_provable_count_sum_tree_keeps_count_drops_sum() { + // A NotSummed(SumTree(_, 100, _)) inside a ProvableCountSumTree + // contributes count = 1, sum = 0. + let grove_version = GroveVersion::latest(); + let mut merk = TempMerk::new_with_tree_type(grove_version, TreeType::ProvableCountSumTree); + + let ns = Element::new_not_summed(Element::new_sum_tree_with_flags_and_sum_value( + None, 100, None, + )) + .expect("wrap ok"); + ns.insert_subtree(&mut merk, b"k", [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); + assert_eq!(agg.as_sum_i64(), 0); + } + #[test] fn non_counted_count_tree_inside_count_tree_suppresses_subtree_count() { // Bare ProvableCountTree(_, 5, _) inside CountTree contributes 5. diff --git a/merk/src/element/reconstruct.rs b/merk/src/element/reconstruct.rs index 79c9f9082..2c99ff986 100644 --- a/merk/src/element/reconstruct.rs +++ b/merk/src/element/reconstruct.rs @@ -70,11 +70,14 @@ impl ElementReconstructExtensions for Element { } // Recurse on the inner element and re-wrap. Without this, a // batch that mutates a subtree under a wrapped tree would lose - // the NonCounted wrapper on the parent's stored element when - // its root key gets propagated upward. + // the wrapper on the parent's stored element when its root key + // gets propagated upward. Element::NonCounted(inner) => inner .reconstruct_with_root_key(maybe_root_key, aggregate_data) .map(|reconstructed| Element::NonCounted(Box::new(reconstructed))), + Element::NotSummed(inner) => inner + .reconstruct_with_root_key(maybe_root_key, aggregate_data) + .map(|reconstructed| Element::NotSummed(Box::new(reconstructed))), _ => None, } } @@ -107,6 +110,21 @@ mod tests { } } + #[test] + fn reconstruct_preserves_not_summed_wrapper() { + // Symmetric to reconstruct_preserves_non_counted_wrapper. + let inner = Element::new_sum_tree_with_flags_and_sum_value(None, 100, None); + let wrapped = Element::new_not_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::Sum(100)) + .expect("reconstruct ok"); + assert!(matches!(reconstructed, Element::NotSummed(_))); + if let Element::NotSummed(boxed) = reconstructed { + assert!(matches!(*boxed, Element::SumTree(ref k, 100, _) if k == &new_root)); + } + } + #[test] fn reconstruct_returns_none_for_non_tree() { let item = Element::new_item(b"x".to_vec()); diff --git a/merk/src/element/tree_type.rs b/merk/src/element/tree_type.rs index 576419359..235e2d3ac 100644 --- a/merk/src/element/tree_type.rs +++ b/merk/src/element/tree_type.rs @@ -60,7 +60,9 @@ impl ElementTreeTypeExtensions for Element { Element::DenseAppendOnlyFixedSizeTree(_, height, _) => { Some((None, TreeType::DenseAppendOnlyFixedSizeTree(height))) } - Element::NonCounted(inner) => inner.root_key_and_tree_type_owned(), + Element::NonCounted(inner) | Element::NotSummed(inner) => { + inner.root_key_and_tree_type_owned() + } _ => None, } } @@ -93,7 +95,9 @@ impl ElementTreeTypeExtensions for Element { &NONE_ROOT_KEY, TreeType::DenseAppendOnlyFixedSizeTree(*height), )), - Element::NonCounted(inner) => inner.root_key_and_tree_type(), + Element::NonCounted(inner) | Element::NotSummed(inner) => { + inner.root_key_and_tree_type() + } _ => None, } } @@ -121,7 +125,7 @@ impl ElementTreeTypeExtensions for Element { Element::DenseAppendOnlyFixedSizeTree(_, height, flags) => { Some((flags, TreeType::DenseAppendOnlyFixedSizeTree(*height))) } - Element::NonCounted(inner) => inner.tree_flags_and_type(), + Element::NonCounted(inner) | Element::NotSummed(inner) => inner.tree_flags_and_type(), _ => None, } } @@ -147,7 +151,7 @@ impl ElementTreeTypeExtensions for Element { Element::DenseAppendOnlyFixedSizeTree(_, height, _) => { Some(TreeType::DenseAppendOnlyFixedSizeTree(*height)) } - Element::NonCounted(inner) => inner.tree_type(), + Element::NonCounted(inner) | Element::NotSummed(inner) => inner.tree_type(), _ => None, } } @@ -171,7 +175,7 @@ impl ElementTreeTypeExtensions for Element { Element::MmrTree(..) => Some(BasicMerkNode), Element::BulkAppendTree(..) => Some(BasicMerkNode), Element::DenseAppendOnlyFixedSizeTree(..) => Some(BasicMerkNode), - Element::NonCounted(inner) => inner.tree_feature_type(), + Element::NonCounted(inner) | Element::NotSummed(inner) => inner.tree_feature_type(), _ => None, } } @@ -197,7 +201,7 @@ impl ElementTreeTypeExtensions for Element { Element::DenseAppendOnlyFixedSizeTree(_, height, _) => { MaybeTree::Tree(TreeType::DenseAppendOnlyFixedSizeTree(*height)) } - Element::NonCounted(inner) => inner.maybe_tree_type(), + Element::NonCounted(inner) | Element::NotSummed(inner) => inner.maybe_tree_type(), _ => MaybeTree::NotTree, } } @@ -205,11 +209,11 @@ 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`, so the - /// existing dispatch produces the right feature type for the wrapper - /// without an explicit branch here. Sum-bearing parents still see the - /// inner element's sum because `sum_value_or_default` and - /// `big_sum_value_or_default` delegate through the wrapper. + /// return 0 (resp. (0, inner_sum)) for `Element::NonCounted`, and + /// `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. fn get_feature_type(&self, parent_tree_type: TreeType) -> Result { match parent_tree_type { TreeType::NormalTree => Ok(BasicMerkNode), @@ -284,4 +288,93 @@ mod tests { _ => panic!("unexpected cost type"), } } + + #[test] + fn tree_type_extensions_look_through_not_summed() { + // All ElementTreeTypeExtensions methods must delegate through + // NotSummed to the inner sum-tree variant, mirroring NonCounted. + 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_summed(inner.clone()).expect("wrap ok"); + + // tree_type() / maybe_tree_type() / root_key_and_tree_type{,_owned} + // all return the inner's tree type. + 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); + + // tree_flags_and_type returns the inner's flags (None) and type. + let (flags, tt) = wrapped.tree_flags_and_type().expect("Some"); + assert!(flags.is_none()); + assert_eq!(tt, expected_tree_type); + + // tree_feature_type returns the inner's feature type unchanged + // (it is the per-element-type discriminant, not the parent + // aggregation — that's `get_feature_type` below). + assert!(wrapped.tree_feature_type().is_some()); + } + } + + #[test] + fn get_feature_type_zeros_sum_for_not_summed_in_sum_parents() { + // Every sum-bearing parent type must zero out the wrapped sum + // through `get_feature_type`. Counts (in CountSumTree / + // ProvableCountSumTree) still propagate. + let inner = Element::SumTree(None, 100, None); + let ns = Element::new_not_summed(inner).expect("wrap ok"); + + assert_eq!( + ns.get_feature_type(TreeType::SumTree).unwrap(), + SummedMerkNode(0) + ); + assert_eq!( + ns.get_feature_type(TreeType::BigSumTree).unwrap(), + BigSummedMerkNode(0) + ); + + // CountSumTree parent: sum=0, count=1 (the wrapped tree counts as + // one element). + assert_eq!( + ns.get_feature_type(TreeType::CountSumTree).unwrap(), + CountedSummedMerkNode(1, 0) + ); + + // ProvableCountSumTree: same as above, just provable variant. + match ns.get_feature_type(TreeType::ProvableCountSumTree).unwrap() { + TreeFeatureType::ProvableCountedSummedMerkNode(c, s) => { + assert_eq!((c, s), (1, 0)); + } + other => panic!("expected ProvableCountedSummedMerkNode, got {:?}", other), + } + } } diff --git a/merk/src/tree_type/mod.rs b/merk/src/tree_type/mod.rs index d75250ada..9e1c80cc0 100644 --- a/merk/src/tree_type/mod.rs +++ b/merk/src/tree_type/mod.rs @@ -135,6 +135,20 @@ impl TreeType { ) } + /// Returns whether this tree type carries a sum aggregate that children + /// can contribute to. Only sum-bearing trees may host + /// `Element::NotSummed` children — in any other parent the wrapper would + /// have no semantic effect, so it is rejected at insert time. + pub const fn is_sum_bearing(&self) -> bool { + matches!( + self, + TreeType::SumTree + | TreeType::BigSumTree + | TreeType::CountSumTree + | TreeType::ProvableCountSumTree + ) + } + /// Returns whether this tree type allows sum items as children. pub fn allows_sum_item(&self) -> bool { match self { @@ -302,6 +316,21 @@ mod tests { assert!(!TreeType::DenseAppendOnlyFixedSizeTree(0).is_count_bearing()); } + #[test] + fn is_sum_bearing() { + assert!(!TreeType::NormalTree.is_sum_bearing()); + assert!(TreeType::SumTree.is_sum_bearing()); + assert!(TreeType::BigSumTree.is_sum_bearing()); + assert!(!TreeType::CountTree.is_sum_bearing()); + assert!(TreeType::CountSumTree.is_sum_bearing()); + assert!(!TreeType::ProvableCountTree.is_sum_bearing()); + assert!(TreeType::ProvableCountSumTree.is_sum_bearing()); + assert!(!TreeType::CommitmentTree(0).is_sum_bearing()); + assert!(!TreeType::MmrTree.is_sum_bearing()); + assert!(!TreeType::BulkAppendTree(0).is_sum_bearing()); + assert!(!TreeType::DenseAppendOnlyFixedSizeTree(0).is_sum_bearing()); + } + #[test] fn allows_sum_item() { assert!(!TreeType::NormalTree.allows_sum_item());