From 82726a010db99b562f47fe2a1656077d43aab943 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sun, 2 Aug 2026 17:28:51 +0700 Subject: [PATCH 1/6] feat(grovedb): expose indexed-axis proof verification to verify-only builds The verify-side entry points of the indexed-axis proof envelope (verify_indexed_axis_top_k and friends) were minimal-gated as a module, so a consumer compiling with --no-default-features --features verify could not reach them. Gate the prove-side items individually instead and open the module to both feature sets, mirroring how the rest of the proof code splits prover from verifier. Co-Authored-By: Claude Fable 5 --- .../operations/proof/indexed_axis/axis_api.rs | 18 +++++++++++++++++- .../src/operations/proof/indexed_axis/mod.rs | 1 + grovedb/src/operations/proof/mod.rs | 6 +++++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/grovedb/src/operations/proof/indexed_axis/axis_api.rs b/grovedb/src/operations/proof/indexed_axis/axis_api.rs index 1ac184bf7..cc22bba15 100644 --- a/grovedb/src/operations/proof/indexed_axis/axis_api.rs +++ b/grovedb/src/operations/proof/indexed_axis/axis_api.rs @@ -3,13 +3,18 @@ //! //! Each wrapper pins the [`IndexAxis`] and forwards; no logic lives here. +#[cfg(feature = "minimal")] use grovedb_costs::CostResult; use grovedb_element::indexed::IndexAxis; use grovedb_merk::proofs::Query as MerkQuery; +#[cfg(feature = "minimal")] use grovedb_path::SubtreePath; +#[cfg(feature = "minimal")] use grovedb_version::version::GroveVersion; -use crate::{Error, GroveDb, TransactionArg}; +#[cfg(feature = "minimal")] +use crate::TransactionArg; +use crate::{Error, GroveDb}; use super::{IndexedAxisAggregateResult, IndexedAxisPaginatedResult, IndexedAxisQueryResult}; @@ -18,6 +23,7 @@ impl GroveDb { /// Prove the top-`k` entries of the count axis. Thin wrapper over /// [`Self::prove_indexed_axis_top_k`] with `axis = Count`. + #[cfg(feature = "minimal")] pub fn prove_indexed_count_top_k<'b, B, P>( &self, path: P, @@ -41,6 +47,7 @@ impl GroveDb { } /// Prove an offset-paginated top-`k` window on the count axis. + #[cfg(feature = "minimal")] pub fn prove_indexed_count_top_k_paginated<'b, B, P>( &self, path: P, @@ -66,6 +73,7 @@ impl GroveDb { } /// Prove an arbitrary query against the count-axis secondary. + #[cfg(feature = "minimal")] pub fn prove_indexed_count_query<'b, B, P>( &self, path: P, @@ -90,6 +98,7 @@ impl GroveDb { /// Prove the aggregate count of entries whose `count_value` is in /// `[lo_count, hi_count]`. + #[cfg(feature = "minimal")] pub fn prove_indexed_count_range_aggregate<'b, B, P>( &self, path: P, @@ -181,6 +190,7 @@ impl GroveDb { // ---------- sum axis ---------- /// Prove the top-`k` entries of the sum axis. + #[cfg(feature = "minimal")] pub fn prove_indexed_sum_top_k<'b, B, P>( &self, path: P, @@ -207,6 +217,7 @@ impl GroveDb { /// Note: the secondary is a `ProvableSumTree`, which has no /// count-bound offset primitive, so the proof size is /// O(offset + k). Use sparingly with large offsets. + #[cfg(feature = "minimal")] pub fn prove_indexed_sum_top_k_paginated<'b, B, P>( &self, path: P, @@ -232,6 +243,7 @@ impl GroveDb { } /// Prove an arbitrary query against the sum-axis secondary. + #[cfg(feature = "minimal")] pub fn prove_indexed_sum_query<'b, B, P>( &self, path: P, @@ -256,6 +268,7 @@ impl GroveDb { /// Prove the aggregate sum of entries whose `sum_value` is in /// `[lo_sum, hi_sum]`. + #[cfg(feature = "minimal")] pub fn prove_indexed_sum_range_aggregate<'b, B, P>( &self, path: P, @@ -349,6 +362,7 @@ impl GroveDb { /// Prove the top-`k` entries of the avg axis. PCPSIT-only. No /// aggregate variant exists — averaging an average over a range is /// not closed-form. + #[cfg(feature = "minimal")] pub fn prove_indexed_avg_top_k<'b, B, P>( &self, path: P, @@ -372,6 +386,7 @@ impl GroveDb { } /// Prove an offset-paginated top-`k` window on the avg axis. + #[cfg(feature = "minimal")] pub fn prove_indexed_avg_top_k_paginated<'b, B, P>( &self, path: P, @@ -397,6 +412,7 @@ impl GroveDb { } /// Prove an arbitrary query against the avg-axis secondary. + #[cfg(feature = "minimal")] pub fn prove_indexed_avg_query<'b, B, P>( &self, path: P, diff --git a/grovedb/src/operations/proof/indexed_axis/mod.rs b/grovedb/src/operations/proof/indexed_axis/mod.rs index 2eaa252d8..42177527b 100644 --- a/grovedb/src/operations/proof/indexed_axis/mod.rs +++ b/grovedb/src/operations/proof/indexed_axis/mod.rs @@ -63,6 +63,7 @@ mod axis_api; mod envelope; +#[cfg(feature = "minimal")] mod generate; mod verify; diff --git a/grovedb/src/operations/proof/mod.rs b/grovedb/src/operations/proof/mod.rs index 6f6a794d2..ea5381d5e 100644 --- a/grovedb/src/operations/proof/mod.rs +++ b/grovedb/src/operations/proof/mod.rs @@ -10,7 +10,11 @@ mod aggregate_count_and_sum; mod aggregate_sum; #[cfg(feature = "minimal")] mod generate; -#[cfg(feature = "minimal")] +// The prover lives in `indexed_axis::generate` and is `minimal`-gated there; +// the envelope types and verification entry points must be reachable from a +// verifier-only build (Dash Platform's `drive` crate compiles its +// proof-verification layer with `--no-default-features --features verify`). +#[cfg(any(feature = "minimal", feature = "verify"))] pub mod indexed_axis; /// Utility functions for proof display and conversion. pub mod util; From e2168c144b004a19c4d73ea9d91411ee9d0c5b1d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sun, 2 Aug 2026 19:04:00 +0700 Subject: [PATCH 2/6] fix(grovedb): bind, don't reject, a lower layer with no query below it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #657 added a hard rejection in `verify_layer_proof_v1`: if a V1 proof carries a lower layer for a tree whose key the query stops at, verification fails with "the element bytes would be unbound". The concern is real — a `KVValueHash`-family node hashes only `(key, value_hash)`, so reporting the element without a child-hash check would let a prover attach a dummy lower layer and swap in forged element bytes under a genuine root hash. At v5.0.1 that path pushed the element with no binding at all. But "the proof descends below the query" is the ordinary shape of a SUBSET verification, not an attack: `verify_subset_query` exists to run a narrower query against a proof generated for a wider one. Rejecting it broke every caller that reads a tree element out of a proof that descended into it. Dash Platform hit this on three shielded-notes tests, which pull a `CommitmentTree`'s total note count from the note-fetch proof they already hold via a single-key, no-subquery, limit-1 subset query. Bind the element instead of refusing it. The lower layer is now consumed for its root hash in both cases, and the existing `combine_hash(H(value), child_root)` chain check does the binding; only the reporting differs. With no query below, none of the lower layer's rows belong in the result set, so the tree element itself is reported — under its PARENT path, matching query_raw and the terminal arm. Every lower-layer flavour already derives its root independently of the query (the query only selects rows), so the existing MMR / BulkAppend / CommitmentTree / DenseTree verifiers take a `report_contents` flag and return the root early; Merk layers get their root from an empty query, which matches nothing and consumes no limit. Succinct mode (`verify_query`) still rejects outright: for that query the layer is data the caller never asked for. The result is strictly stronger than v5.0.1, which bound nothing here. Tests: subset verification of a tree element against a descending proof, for both a plain Tree and the exact Platform CommitmentTree note-count shape; two tamper tests proving the binding is load-bearing in subset mode — a dummy lower layer and a sibling subtree's real-but-wrong layer are both rejected. All three fail before this change with the reported error. --- grovedb/src/operations/proof/verify.rs | 201 +++++++++++++++++---- grovedb/src/tests/commitment_tree_tests.rs | 119 ++++++++++++ grovedb/src/tests/succinctness_gap_test.rs | 167 ++++++++++++++++- 3 files changed, 450 insertions(+), 37 deletions(-) diff --git a/grovedb/src/operations/proof/verify.rs b/grovedb/src/operations/proof/verify.rs index f133e5014..8b4826934 100644 --- a/grovedb/src/operations/proof/verify.rs +++ b/grovedb/src/operations/proof/verify.rs @@ -675,6 +675,35 @@ impl GroveDb { } } + /// Derive a Merk layer's root hash without reporting any of its rows. + /// + /// Used when a subset verification stops at a tree ELEMENT that the proof + /// descended into: the parent still has to bind the element bytes with + /// `combine_hash(H(value), child_root)`, but none of the child layer's rows + /// belong in this query's result set. An empty query walks the proof for + /// its root and matches nothing, so no row is produced and no limit is + /// consumed. + /// + /// The layer's own `lower_layers` are deliberately not descended into. The + /// root returned here is computed from this layer's nodes alone, whose + /// value hashes already commit to everything beneath them; the deeper + /// layers exist only to authenticate rows that are not being reported. + fn merk_layer_root_hash( + merk_proof_bytes: &[u8], + query: &PathQuery, + ) -> Result { + let (root_hash, _) = Query::new() + .execute_proof(merk_proof_bytes, None, true, PROOF_VERSION_LATEST) + .unwrap() + .map_err(|e| { + Error::InvalidProof( + query.clone(), + format!("Invalid V1 lower layer proof (root derivation): {}", e), + ) + })?; + Ok(root_hash) + } + /// Takes the layer's Merk proof BYTES and its lower layers separately /// rather than a `&LayerProof`. /// @@ -1014,23 +1043,44 @@ impl GroveDb { | Element::DenseAppendOnlyFixedSizeTree(..) => { path.push(key); *last_parent_tree_type = element.tree_feature_type(); - if query.query_items_at_path(&path, grove_version)?.is_none() { - // Query targets the tree itself, not its - // contents — but a lower layer was still - // supplied, which an honest prover never - // does (every `lower_layers.insert` site is - // gated on there being a subquery for this - // key). Pushing the value here would skip - // both binding mechanisms: the - // `combine_hash` chain check below (which - // needs a query at the lower path) and the - // `child_hash_verified` requirement applied - // on the no-lower-layer path. Since a - // `KVValueHash` node commits only to - // (key, value_hash), accepting it would let - // a prover attach a dummy lower layer and - // substitute forged element bytes under a - // genuine root hash. Fail closed. + // Does the current query ask for anything BELOW + // this tree, or only for the tree element + // itself? + // + // "Only the element itself" while the proof + // still carries a lower layer is the ordinary + // shape of a SUBSET verification: the proof was + // generated for a wider query that descended + // here, and is now being re-verified against a + // narrower one that stops at the tree. Dash + // Platform does exactly this to read a + // CommitmentTree's total note count out of the + // note-fetch proof it already has. + // + // Either way the element bytes must stay bound + // to the parent-committed `value_hash`: a + // `KVValueHash`-family node hashes only + // (key, value_hash), so reporting `value` + // unchecked would let a prover attach a dummy + // lower layer and substitute forged element + // bytes under a genuine root hash. The + // `combine_hash` chain check below is what binds + // it, and it needs the lower layer's root — so + // the lower layer is consumed for its root hash + // in BOTH cases. Only the reporting differs: + // with no query below, none of the lower layer's + // rows belong in this query's result set, so the + // tree element itself is reported instead. + let has_query_below = + query.query_items_at_path(&path, grove_version)?.is_some(); + + if !has_query_below && options.verify_proof_succinctness { + // Succinct mode demands the proof carry + // nothing beyond what the query needs, and + // an honest prover never emits this layer + // for this query (every `lower_layers.insert` + // site is gated on there being a subquery + // for the key). Reject as extra data. return Err(Error::InvalidProof( query.clone(), format!( @@ -1040,15 +1090,19 @@ impl GroveDb { hex::encode(key), ), )); - } else { + } + + { // Known limitation: this parent tree result // is pushed without decrementing limit_left. // Will be addressed by per-level limits // redesign. - if query.should_add_parent_tree_at_path( - current_path, - grove_version, - )? { + if has_query_below + && query.should_add_parent_tree_at_path( + current_path, + grove_version, + )? + { let path_key_optional_value = ProvedPathKeyOptionalValue::from_proved_key_value( path.iter().map(|p| p.to_vec()).collect(), @@ -1060,23 +1114,34 @@ impl GroveDb { ); } - // Dispatch based on lower layer proof type + // Dispatch based on lower layer proof type. + // `has_query_below == false` derives the + // layer's root WITHOUT reporting any of its + // contents — every lower-layer flavour + // computes its root independently of the + // query, which only ever selects rows. let lower_hash = match &lower_layer.merk_proof { ProofBytes::Merk(_) => { // Standard Merk subtree - recurse - Self::verify_layer_proof_v1( - Self::merk_bytes_of_layer(lower_layer, query)?, - &lower_layer.lower_layers, - prove_options, - query, - limit_left, - &path, - result, - last_parent_tree_type, - options, - current_depth + 1, - grove_version, - )? + let merk_bytes = + Self::merk_bytes_of_layer(lower_layer, query)?; + if has_query_below { + Self::verify_layer_proof_v1( + merk_bytes, + &lower_layer.lower_layers, + prove_options, + query, + limit_left, + &path, + result, + last_parent_tree_type, + options, + current_depth + 1, + grove_version, + )? + } else { + Self::merk_layer_root_hash(merk_bytes, query)? + } } ProofBytes::MMR(mmr_bytes) => Self::verify_mmr_lower_layer( mmr_bytes, @@ -1085,6 +1150,7 @@ impl GroveDb { limit_left, result, query, + has_query_below, grove_version, )?, ProofBytes::BulkAppendTree(bulk_bytes) => { @@ -1095,6 +1161,7 @@ impl GroveDb { limit_left, result, query, + has_query_below, grove_version, )? } @@ -1106,6 +1173,7 @@ impl GroveDb { limit_left, result, query, + has_query_below, grove_version, )? } @@ -1117,6 +1185,7 @@ impl GroveDb { limit_left, result, query, + has_query_below, grove_version, )? } @@ -1154,6 +1223,33 @@ impl GroveDb { ), )); } + + if !has_query_below { + // The tree element itself is the + // result, now bound by the + // `combine_hash` check above. Report it + // under the PARENT path (not the path + // we pushed the key onto for the root + // derivation), matching query_raw and + // the no-lower-layer terminal arm — + // including the key would make + // `(path, key)` lookups miss. + let parent_path: Vec> = + current_path.iter().map(|p| p.to_vec()).collect(); + let path_key_optional_value = + ProvedPathKeyOptionalValue::from_proved_key_value( + parent_path, + proved_key_value, + ); + result.push( + path_key_optional_value + .try_into_versioned(grove_version)?, + ); + limit_left + .iter_mut() + .for_each(|limit| *limit = limit.saturating_sub(1)); + } + if limit_left == &Some(0) { break; } @@ -1346,6 +1442,7 @@ impl GroveDb { /// Returns the computed MMR root hash, which the caller uses as the /// child hash for Merk authentication (`combine_hash(value_hash || /// mmr_root)`). + #[allow(clippy::too_many_arguments)] fn verify_mmr_lower_layer( mmr_bytes: &[u8], element: &Element, @@ -1353,6 +1450,7 @@ impl GroveDb { limit_left: &mut Option, result: &mut Vec, query: &PathQuery, + report_contents: bool, grove_version: &GroveVersion, ) -> Result where @@ -1406,6 +1504,13 @@ impl GroveDb { .verify_and_get_root() .map_err(|e| Error::InvalidProof(query.clone(), format!("{}", e)))?; + // Root only: the caller is binding the parent element and does not + // report this layer's leaves, so there is no query at this path to + // check completeness/succinctness against. + if !report_contents { + return Ok(mmr_root); + } + // Get the sub-query items for this path to enforce succinctness. let sub_query = query @@ -1477,6 +1582,7 @@ impl GroveDb { /// For both `BulkAppendTree` and `CommitmentTree` elements: verifies /// internal consistency and returns the computed state_root as the lower /// hash (authenticated via child Merk hash). + #[allow(clippy::too_many_arguments)] fn verify_bulk_append_lower_layer( bulk_bytes: &[u8], element: &Element, @@ -1484,6 +1590,7 @@ impl GroveDb { limit_left: &mut Option, result: &mut Vec, query: &PathQuery, + report_contents: bool, grove_version: &GroveVersion, ) -> Result where @@ -1509,6 +1616,13 @@ impl GroveDb { .verify_and_compute_root(element_height, element_total_count) .map_err(|e| Error::InvalidProof(query.clone(), format!("{}", e)))?; + // Root only: the caller is binding the parent element and does not + // report this layer's entries, so there is no query at this path to + // extract a position range from. + if !report_contents { + return Ok(bulk_state_root); + } + // Get the query range from the path query to extract matching values let sub_query = query @@ -1589,6 +1703,7 @@ impl GroveDb { /// Verifies the BulkAppendTree proof to get `bulk_state_root`, then returns /// `blake3("ct_state" || sinsemilla_root || bulk_state_root)` as the /// authenticated child hash. + #[allow(clippy::too_many_arguments)] fn verify_commitment_tree_lower_layer( ct_bytes: &[u8], element: &Element, @@ -1596,6 +1711,7 @@ impl GroveDb { limit_left: &mut Option, result: &mut Vec, query: &PathQuery, + report_contents: bool, grove_version: &GroveVersion, ) -> Result where @@ -1622,6 +1738,7 @@ impl GroveDb { limit_left, result, query, + report_contents, grove_version, )?; @@ -1638,6 +1755,7 @@ impl GroveDb { /// Verify a DenseAppendOnlyFixedSizeTree lower layer proof and add results. /// Returns NULL_HASH since DenseTree has no child Merk. + #[allow(clippy::too_many_arguments)] fn verify_dense_tree_lower_layer( dense_bytes: &[u8], element: &Element, @@ -1645,6 +1763,7 @@ impl GroveDb { limit_left: &mut Option, result: &mut Vec, query: &PathQuery, + report_contents: bool, grove_version: &GroveVersion, ) -> Result where @@ -1666,6 +1785,16 @@ impl GroveDb { grovedb_dense_fixed_sized_merkle_tree::DenseTreeProof::decode_from_slice(dense_bytes) .map_err(|e| Error::CorruptedData(format!("{}", e)))?; + // Root only: the caller is binding the parent element and does not + // report this layer's entries, so there is no query at this path to + // check completeness/soundness against. + if !report_contents { + let (computed_root, _entries): ([u8; 32], Vec<(u16, Vec)>) = dense_proof + .verify_and_get_root(element_height, element_count) + .map_err(|e| Error::InvalidProof(query.clone(), format!("{}", e)))?; + return Ok(computed_root); + } + // Get the sub-query items for this path to build a query for // verify_for_query, which enforces both completeness and soundness. let sub_query = diff --git a/grovedb/src/tests/commitment_tree_tests.rs b/grovedb/src/tests/commitment_tree_tests.rs index a62d63e1a..2d76f08b4 100644 --- a/grovedb/src/tests/commitment_tree_tests.rs +++ b/grovedb/src/tests/commitment_tree_tests.rs @@ -2739,3 +2739,122 @@ fn replace_subtree_root_rejects_non_tree_element() { "expected InvalidInput for non-tree element, got {result:?}" ); } + +/// Regression (Dash Platform shielded-notes shape): the note-count query. +/// +/// Platform fetches shielded notes with one proof that subqueries INTO the +/// `CommitmentTree`, then extracts the on-chain total note count from those +/// SAME proof bytes by subset-verifying a single-key `PathQuery` that targets +/// the `CommitmentTree` element itself — no subquery, limit 1 — and reading +/// `Element::CommitmentTree(total_count, ..)`. +/// +/// The proof therefore carries a lower layer at the tree's key while the +/// count query has nothing below it. That must verify, and the element must +/// come back bound to the parent-committed value hash via the lower layer's +/// derived state root. +#[test] +fn test_commitment_tree_element_count_subset_query_against_note_fetch_proof() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + let chunk_power: u8 = 2; + + db.insert( + EMPTY_PATH, + b"root", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert root tree"); + + db.insert( + &[b"root"], + b"pool", + Element::empty_commitment_tree(chunk_power).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert commitment tree"); + + // 6 notes: one full chunk (4) plus 2 buffered. + const NOTE_COUNT: u64 = 6; + for i in 0..NOTE_COUNT as u8 { + db.commitment_tree_insert( + &[b"root"], + b"pool", + test_cmx(i), + test_rho(i), + test_cv_net(i), + test_ciphertext(i), + None, + grove_version, + ) + .unwrap() + .expect("commitment tree insert"); + } + + // The note-fetch proof: descends into the CommitmentTree. + let mut inner_query = Query::new(); + inner_query.insert_range_inclusive(0u64.to_be_bytes().to_vec()..=5u64.to_be_bytes().to_vec()); + let notes_query = PathQuery { + path: vec![b"root".to_vec()], + query: SizedQuery { + query: Query { + items: vec![QueryItem::Key(b"pool".to_vec())], + default_subquery_branch: SubqueryBranch { + subquery_path: None, + subquery: Some(inner_query.into()), + }, + left_to_right: true, + conditional_subquery_branches: None, + add_parent_tree_on_subquery: false, + }, + limit: None, + offset: None, + }, + }; + + let proof_bytes = db + .prove_query(¬es_query, None, grove_version) + .unwrap() + .expect("generate note-fetch proof"); + + // The count query: the CommitmentTree element itself, no subquery. + let count_query = PathQuery { + path: vec![b"root".to_vec()], + query: SizedQuery { + query: Query::new_single_key(b"pool".to_vec()), + limit: Some(1), + offset: None, + }, + }; + + let (count_root_hash, count_results) = + GroveDb::verify_subset_query(&proof_bytes, &count_query, grove_version) + .expect("count query must subset-verify against the note-fetch proof"); + + let expected_root = db.grove_db.root_hash(None, grove_version).unwrap().unwrap(); + assert_eq!( + count_root_hash, expected_root, + "the count sub-proof must derive the same root as the note-fetch proof" + ); + assert_eq!(count_results.len(), 1, "exactly the CommitmentTree element"); + + let (path, key, element) = &count_results[0]; + assert_eq!(path, &vec![b"root".to_vec()]); + assert_eq!(key, b"pool"); + match element.as_ref().expect("element present") { + Element::CommitmentTree(total_count, height, _) => { + assert_eq!( + *total_count, NOTE_COUNT, + "total_count must be the on-chain note count" + ); + assert_eq!(*height, chunk_power); + } + other => panic!("expected CommitmentTree element, got {:?}", other), + } +} diff --git a/grovedb/src/tests/succinctness_gap_test.rs b/grovedb/src/tests/succinctness_gap_test.rs index ff48598d4..3db5e03d1 100644 --- a/grovedb/src/tests/succinctness_gap_test.rs +++ b/grovedb/src/tests/succinctness_gap_test.rs @@ -10,7 +10,7 @@ use grovedb_version::version::{v1::GROVE_V1, GroveVersion}; use crate::{ - operations::proof::GroveDBProof, + operations::proof::{GroveDBProof, LayerProof, ProofBytes}, tests::{make_deep_tree, TEST_LEAF}, GroveDb, PathQuery, Query, }; @@ -262,3 +262,168 @@ fn test_missing_lower_layer_for_non_empty_tree_is_rejected_v0() { "V0: verify_subset_query must reject proof missing a non-empty subtree's lower layer" ); } + +/// Regression: a subset verification whose query stops at a tree ELEMENT must +/// still verify against a proof that descended INTO that tree. +/// +/// This is the ordinary shape of `verify_subset_query`: a wide proof is +/// generated once, then re-verified against a narrower query. Dash Platform +/// reads a shielded `CommitmentTree`'s total note count exactly this way — +/// single-key query, no subquery, against the note-fetch proof it already +/// holds. +/// +/// The tree element is the only result, and it must come back bound: the +/// verifier derives the lower layer's root and checks +/// `combine_hash(H(value), child_root)` against the parent-committed value +/// hash, without reporting any of the lower layer's rows. +#[test] +fn test_subset_query_for_tree_element_itself_against_descending_proof() { + let grove_version = GroveVersion::latest(); + let db = make_deep_tree(grove_version); + let expected_root = db.root_hash(None, grove_version).unwrap().unwrap(); + + // Wide proof: descends into innertree, so the proof carries a lower layer + // at that key. + let mut inner = Query::new(); + inner.insert_all(); + let mut outer = Query::new(); + outer.insert_key(b"innertree".to_vec()); + outer.set_subquery(inner); + let broad_query = PathQuery::new_unsized(vec![TEST_LEAF.to_vec()], outer); + + let proof_bytes = db + .prove_query(&broad_query, None, grove_version) + .unwrap() + .expect("should generate descending proof"); + + // Narrow query: the innertree element itself, no subquery. + let narrow_query = PathQuery::new_single_key(vec![TEST_LEAF.to_vec()], b"innertree".to_vec()); + + let (subset_root, subset_results) = + GroveDb::verify_subset_query(&proof_bytes, &narrow_query, grove_version) + .expect("subset verification must accept a query that stops at the tree element"); + + assert_eq!(subset_root, expected_root); + assert_eq!( + subset_results.len(), + 1, + "only the tree element itself is a result; the lower layer's rows are not reported" + ); + let (path, key, element) = &subset_results[0]; + assert_eq!( + path, + &vec![TEST_LEAF.to_vec()], + "the tree must be reported under its PARENT path, so (path, key) lookups hit" + ); + assert_eq!(key, b"innertree"); + assert!( + element + .as_ref() + .expect("element present") + .is_non_empty_merk_tree(), + "the reported element must be the innertree subtree itself" + ); + + // Succinct mode still refuses: for this narrow query the lower layer is + // data the query never asked for. + assert!( + GroveDb::verify_query(&proof_bytes, &narrow_query, grove_version).is_err(), + "verify_query must still reject a proof carrying an unrequested lower layer" + ); +} + +/// The element bytes reported by the subset path above stay BOUND. +/// +/// A `KVValueHash`-family node hashes only `(key, value_hash)`, so a prover +/// that could get a tree element reported without any child-hash check could +/// swap in forged element bytes under a genuine root hash. Two tampers prove +/// the binding is load-bearing rather than incidental: +/// +/// 1. a dummy lower layer attached where none belongs, and +/// 2. a real-but-wrong lower layer (a sibling subtree's proof). +/// +/// Both must be rejected even though succinctness checking is OFF. +#[test] +fn test_subset_mode_still_binds_element_bytes_to_lower_layer() { + let grove_version = GroveVersion::latest(); + let db = make_deep_tree(grove_version); + let config = bincode::config::standard() + .with_big_endian() + .with_no_limit(); + + // Wide proof descending into BOTH innertree and innertree4, so the two + // sibling lower layers are available to swap. + let mut inner = Query::new(); + inner.insert_all(); + let mut outer = Query::new(); + outer.insert_all(); + outer.set_subquery(inner); + let broad_query = PathQuery::new_unsized(vec![TEST_LEAF.to_vec()], outer); + let proof_bytes = db + .prove_query(&broad_query, None, grove_version) + .unwrap() + .expect("should generate descending proof"); + + let narrow_query = PathQuery::new_single_key(vec![TEST_LEAF.to_vec()], b"innertree".to_vec()); + + // Sanity: untampered, the narrow subset query verifies. + GroveDb::verify_subset_query(&proof_bytes, &narrow_query, grove_version) + .expect("honest proof must verify with the narrow query"); + + let decode = |bytes: &[u8]| -> GroveDBProof { + bincode::decode_from_slice(bytes, config) + .expect("should decode proof") + .0 + }; + let test_leaf_key = TEST_LEAF.to_vec(); + let innertree_key = b"innertree".to_vec(); + let innertree4_key = b"innertree4".to_vec(); + + // These proofs are V1 envelopes; only V1 carries the typed lower-layer + // proof bytes this test tampers with. + fn root_layer_mut(proof: &mut GroveDBProof) -> &mut LayerProof { + match proof { + GroveDBProof::V1(v1) => &mut v1.root_layer, + GroveDBProof::V0(_) => panic!("expected a V1 proof envelope"), + } + } + + // Tamper 1: replace innertree's lower layer with an empty dummy. + let mut dummy = decode(&proof_bytes); + root_layer_mut(&mut dummy) + .lower_layers + .get_mut(&test_leaf_key) + .expect("TEST_LEAF layer") + .lower_layers + .insert( + innertree_key.clone(), + LayerProof { + merk_proof: ProofBytes::Merk(Vec::new()), + lower_layers: Default::default(), + }, + ); + let dummy_bytes = bincode::encode_to_vec(&dummy, config).expect("re-encode"); + assert!( + GroveDb::verify_subset_query(&dummy_bytes, &narrow_query, grove_version).is_err(), + "a dummy lower layer must not let unbound element bytes through in subset mode" + ); + + // Tamper 2: give innertree its SIBLING's lower layer — a structurally + // valid Merk proof that commits to a different root. + let mut swapped = decode(&proof_bytes); + let leaf_layers = &mut root_layer_mut(&mut swapped) + .lower_layers + .get_mut(&test_leaf_key) + .expect("TEST_LEAF layer") + .lower_layers; + let sibling = leaf_layers + .get(&innertree4_key) + .expect("innertree4 lower layer") + .clone(); + leaf_layers.insert(innertree_key, sibling); + let swapped_bytes = bincode::encode_to_vec(&swapped, config).expect("re-encode"); + assert!( + GroveDb::verify_subset_query(&swapped_bytes, &narrow_query, grove_version).is_err(), + "the reported element must be bound to ITS OWN child root, not any valid subtree proof" + ); +} From ff77fc006f3abb5bc0d52560d48e273ac0800fd1 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Sun, 2 Aug 2026 23:43:23 +0700 Subject: [PATCH 3/6] fix(grovedb): bind terminal non-Merk tree element bytes to the parent value_hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a V1 proof reported an Element::CommitmentTree / MmrTree / BulkAppendTree / DenseAppendOnlyFixedSizeTree as a TERMINAL result — the query targets the tree element itself and the prover emits no lower layer — nothing tied the serialized element bytes to the value_hash its parent Merk commits to. Both existing binding mechanisms skipped these four types. The empty-tree combine_hash(H(value), NULL_HASH) check is gated on `!is_non_empty_tree()`, but `is_non_empty_tree()` returns true unconditionally for them. The `child_hash_verified` requirement was gated on `is_non_empty_merk_tree()`, which by construction excludes them. The prover, meanwhile, only rewrote regular non-empty Merk trees to KVValueHashFeatureTypeWithChildHash and left these emitting a bare KVValueHash, which hashes only (key, value_hash) — the value bytes never enter the node hash. A prover could therefore serve forged element bytes (an inflated or deflated CommitmentTree total_count, a different MMR size) alongside the genuine value_hash and still reconstruct the correct root hash. Dash Platform's GetShieldedNotesCount verifier reads total_count from exactly such a terminal element, so a malicious node could misreport a wallet's shielded sync denominator. Note this could NOT be fixed verifier-side by asserting hash == value_hash(value_bytes): these types are written through insert_subtree, so the parent commits combine_hash(H(value), state_root), not plain H(value), and the state root is not derivable from the element bytes. It has to travel in the proof. No new proof format is needed. KVValueHashFeatureTypeWithChildHash already verifies combine_hash(H(value), child_hash) == value_hash, which is exactly the composition these types commit — the prover simply was not using it here. Prover: the terminal arm now covers all four types (CommitmentTree moved out of the empty-trees arm, since it is bound whether or not it holds notes) and rewrites the node to carry the tree's state root, computed by the new `non_merk_tree_child_hash`. Each arm mirrors its write path: MmrTree, BulkAppendTree and DenseAppendOnlyFixedSizeTree are inserted with NULL_HASH while empty — note an empty BulkAppendTree's compute_current_state_root() is NOT NULL_HASH, so that case short-circuits — while CommitmentTree needs no special case because the sinsemilla/bulk composition already yields EMPTY_COMMITMENT_TREE_STATE_ROOT at count 0. A self-check fails loudly if a recomputed root does not reproduce the committed value hash, so any future convention drift surfaces as a prover error instead of an unverifiable proof. Verifier: the child-hash requirement widens from is_non_empty_merk_tree() to is_non_empty_tree(), which adds exactly these four types. This tightens verification: proofs from an un-upgraded prover are now rejected, so provers and verifiers must upgrade together. New proofs still verify under old verifiers, which simply do not enforce the check. Left ungated, matching the existing non-empty-Merk-tree requirement and e2168c1. V0 envelopes are deliberately untouched. The gap is broader there (a regular non-empty CountTree's count is forgeable the same way) and V0 documents the child-hash check as V1-only; it is a frozen wire format and Platform no longer accepts V0 proofs. Tests cover all four types plus empty instances, each forgery tried in two shapes: tampering the value bytes inside the honest child-hash node, and downgrading the node back to bare KVValueHash — the latter is what exercises the verifier's new requirement. Co-Authored-By: Claude Opus 5 --- grovedb/src/operations/proof/generate.rs | 235 ++++++++++- grovedb/src/operations/proof/verify.rs | 23 +- grovedb/src/tests/proof_coverage_tests.rs | 460 ++++++++++++++++++++++ 3 files changed, 706 insertions(+), 12 deletions(-) diff --git a/grovedb/src/operations/proof/generate.rs b/grovedb/src/operations/proof/generate.rs index 64540df78..67e4df338 100644 --- a/grovedb/src/operations/proof/generate.rs +++ b/grovedb/src/operations/proof/generate.rs @@ -2339,13 +2339,78 @@ impl GroveDb { lower_layers.insert(key.clone(), layer_proof); } - // MmrTree/BulkAppendTree without subquery (query targets the tree - // itself) - Ok(Element::MmrTree(..)) - | Ok(Element::BulkAppendTree(..)) - | Ok(Element::DenseAppendOnlyFixedSizeTree(..)) + // Non-Merk tree that is itself the result, with + // nothing queried below it. These types have no + // child Merk, so there is no lower layer to bind + // them — a bare `KVValueHash` node hashes only + // (key, value_hash) and would leave the element + // bytes (and with them the entry count a caller + // reads) free for a prover to forge under a + // genuine root hash. Their parent commits + // `combine_hash(H(value), state_root)`, exactly + // the two-input form + // `KVValueHashFeatureTypeWithChildHash` is + // verified with, so carry the state root in the + // node and let the merk verifier close the loop. + Ok(ref non_merk_elem @ Element::MmrTree(..)) + | Ok(ref non_merk_elem @ Element::BulkAppendTree(..)) + | Ok( + ref non_merk_elem @ Element::DenseAppendOnlyFixedSizeTree(..), + ) + | Ok(ref non_merk_elem @ Element::CommitmentTree(..)) if !done_with_results => { + let mut child_path = path.clone(); + child_path.push(key.as_slice()); + + let child_hash = cost_return_on_error!( + &mut cost, + self.non_merk_tree_child_hash( + non_merk_elem, + &child_path, + &tx, + ) + ); + + let key_owned = key.to_owned(); + let value_owned = value.to_owned(); + let element_vh = + value_hash(&value_owned).unwrap_add_cost(&mut cost); + let recomputed = combine_hash(&element_vh, &child_hash) + .unwrap_add_cost(&mut cost); + let (vh, ft) = match node { + Node::KVValueHashFeatureType(_, _, vh, ft) => (*vh, *ft), + Node::KVValueHash(_, _, vh) => { + (*vh, TreeFeatureType::BasicMerkNode) + } + _ => (recomputed, TreeFeatureType::BasicMerkNode), + }; + + // Self-check: if the recomputed state root does + // not reproduce the committed value_hash, the + // node we are about to emit would be rejected + // by the verifier. Fail here, where the cause + // is visible, rather than shipping a proof that + // cannot verify. + if recomputed != vh { + return Err(Error::CorruptedData(format!( + "non-Merk tree at key {} has state root {} which does \ + not reproduce the committed value hash {}", + hex::encode(&key_owned), + hex::encode(child_hash), + hex::encode(vh), + ))) + .wrap_with_cost(cost); + } + + *node = Node::KVValueHashFeatureTypeWithChildHash( + key_owned, + value_owned, + vh, + ft, + child_hash, + ); + if let Some(limit) = overall_limit.as_mut() { *limit -= 1; } @@ -2579,7 +2644,10 @@ impl GroveDb { } lower_layers.insert(key.clone(), layer_proof); } - // Empty trees and CommitmentTree without subquery + // Empty trees without subquery. CommitmentTree is + // NOT here — like the other non-Merk trees it is + // bound by the child-hash arm above, which applies + // whether or not it holds any notes. Ok(Element::Tree(None, _)) | Ok(Element::SumTree(None, ..)) | Ok(Element::BigSumTree(None, ..)) @@ -2665,6 +2733,161 @@ impl GroveDb { .wrap_with_cost(cost) } + /// Compute the child hash that a non-Merk tree element's parent Merk + /// commits to, i.e. the `child_hash` satisfying + /// `combine_hash(H(value), child_hash) == value_hash`. + /// + /// `CommitmentTree`, `MmrTree`, `BulkAppendTree` and + /// `DenseAppendOnlyFixedSizeTree` have no child Merk; their parent entry is + /// written by `insert_subtree` with the tree's own state root as the + /// supplied hash. This reproduces that hash so a terminal proof of the tree + /// element itself can carry it and stay bound to the element bytes. + /// + /// Each arm must mirror the corresponding write path exactly: + /// - `MmrTree` / `DenseAppendOnlyFixedSizeTree` / `BulkAppendTree` are + /// inserted with `NULL_HASH` while still empty, and only start committing + /// a computed root once the first append lands. Note that an empty + /// `BulkAppendTree`'s `compute_current_state_root()` is *not* `NULL_HASH`, + /// so the zero-count case has to short-circuit. + /// - `CommitmentTree` is inserted with `EMPTY_COMMITMENT_TREE_STATE_ROOT`, + /// which is exactly what the sinsemilla/bulk composition below yields at + /// count 0 — no special case needed. + fn non_merk_tree_child_hash( + &self, + element: &Element, + subtree_path: &[&[u8]], + tx: &Transaction, + ) -> CostResult { + use grovedb_merk::tree::NULL_HASH; + + let mut cost = OperationCost::default(); + + let path_vec: Vec> = subtree_path.iter().map(|s| s.to_vec()).collect(); + let path_refs: Vec<&[u8]> = path_vec.iter().map(|v| v.as_slice()).collect(); + let storage_path = grovedb_path::SubtreePath::from(path_refs.as_slice()); + + match element { + Element::MmrTree(mmr_size, _) => { + if *mmr_size == 0 { + return Ok(NULL_HASH).wrap_with_cost(cost); + } + let storage_ctx = self + .db + .get_transactional_storage_context(storage_path, None, tx) + .unwrap_add_cost(&mut cost); + let store = grovedb_merkle_mountain_range::MmrStore::new(&storage_ctx); + let mmr = grovedb_merkle_mountain_range::MMR::new(*mmr_size, &store); + let root = cost_return_on_error!( + &mut cost, + mmr.get_root() + .map_err(|e| Error::CorruptedData(format!("MMR get_root failed: {}", e))) + ); + Ok(root.hash()).wrap_with_cost(cost) + } + Element::DenseAppendOnlyFixedSizeTree(count, height, _) => { + if *count == 0 { + return Ok(NULL_HASH).wrap_with_cost(cost); + } + let storage_ctx = self + .db + .get_transactional_storage_context(storage_path, None, tx) + .unwrap_add_cost(&mut cost); + let tree = cost_return_on_error_no_add!( + cost, + grovedb_dense_fixed_sized_merkle_tree::DenseFixedSizedMerkleTree::from_state( + *height, + *count, + storage_ctx, + ) + .map_err(|e| Error::CorruptedData(format!("dense tree state error: {}", e))) + ); + let root_hash = cost_return_on_error!( + &mut cost, + tree.root_hash().map_err(|e| Error::CorruptedData(format!( + "dense tree root hash error: {}", + e + ))) + ); + Ok(root_hash).wrap_with_cost(cost) + } + Element::BulkAppendTree(total_count, chunk_power, _) => { + if *total_count == 0 { + return Ok(NULL_HASH).wrap_with_cost(cost); + } + let storage_ctx = self + .db + .get_transactional_storage_context(storage_path, None, tx) + .unwrap_add_cost(&mut cost); + let tree = cost_return_on_error_no_add!( + cost, + grovedb_bulk_append_tree::BulkAppendTree::from_state( + *total_count, + *chunk_power, + storage_ctx, + ) + .map_err(|e| Error::CorruptedData(format!( + "failed to create BulkAppendTree: {}", + e + ))) + ); + let state_root = cost_return_on_error_no_add!( + cost, + tree.compute_current_state_root().map_err(|e| { + Error::CorruptedData(format!("bulk append state root failed: {}", e)) + }) + ); + Ok(state_root).wrap_with_cost(cost) + } + Element::CommitmentTree(total_count, chunk_power, _) => { + let storage_ctx = self + .db + .get_transactional_storage_context(storage_path, None, tx) + .unwrap_add_cost(&mut cost); + + let sinsemilla_root = match storage_ctx.get(COMMITMENT_TREE_DATA_KEY).value { + Ok(Some(frontier_bytes)) => { + match grovedb_commitment_tree::CommitmentFrontier::deserialize( + frontier_bytes.as_ref(), + ) { + Ok(frontier) => frontier.root_hash(), + Err(_) => grovedb_commitment_tree::EMPTY_SINSEMILLA_ROOT, + } + } + _ => grovedb_commitment_tree::EMPTY_SINSEMILLA_ROOT, + }; + + let tree = cost_return_on_error_no_add!( + cost, + grovedb_bulk_append_tree::BulkAppendTree::from_state( + *total_count, + *chunk_power, + storage_ctx, + ) + .map_err(|e| Error::CorruptedData(format!( + "failed to create BulkAppendTree: {}", + e + ))) + ); + let bulk_state_root = cost_return_on_error_no_add!( + cost, + tree.compute_current_state_root().map_err(|e| { + Error::CorruptedData(format!("bulk append state root failed: {}", e)) + }) + ); + + Ok(grovedb_commitment_tree::compute_commitment_tree_state_root( + &sinsemilla_root, + &bulk_state_root, + )) + .wrap_with_cost(cost) + } + _ => Err(Error::CorruptedCodeExecution( + "non_merk_tree_child_hash called on an element that is not a non-Merk tree", + )) + .wrap_with_cost(cost), + } + } + /// Generate an MMR tree layer proof for a subquery. fn generate_mmr_layer_proof( &self, diff --git a/grovedb/src/operations/proof/verify.rs b/grovedb/src/operations/proof/verify.rs index 8b4826934..395f1d1df 100644 --- a/grovedb/src/operations/proof/verify.rs +++ b/grovedb/src/operations/proof/verify.rs @@ -1371,17 +1371,28 @@ impl GroveDb { } } - // For non-empty Merk trees without a subquery (no - // lower layer proof), the prover must use + // For trees reported without a subquery (no lower layer + // proof), the prover must use // KVValueHashFeatureTypeWithChildHash so the merk // verifier can confirm combine_hash(H(value), // child_hash) == value_hash. If child_hash_verified is // false, an attacker may have downgraded the node type // to hide child hash verification. - // Non-Merk trees (MmrTree, BulkAppendTree, etc.) are - // excluded — they use different proof structures. - if element.is_non_empty_merk_tree() && !proved_key_value.child_hash_verified - { + // + // This covers non-empty Merk trees (child_hash = child + // Merk root) and all four non-Merk trees — + // CommitmentTree, MmrTree, BulkAppendTree, + // DenseAppendOnlyFixedSizeTree (child_hash = the tree's + // own state root, which their parent commits through + // the same two-input combine_hash). `is_non_empty_tree` + // is true unconditionally for those four, so an empty + // one is bound too — its committed child hash is + // NULL_HASH, or EMPTY_COMMITMENT_TREE_STATE_ROOT for a + // CommitmentTree. Without this, the element bytes of a + // terminally-reported non-Merk tree were unbound and a + // prover could forge the entry count callers read from + // them. + if element.is_non_empty_tree() && !proved_key_value.child_hash_verified { return Err(Error::InvalidProof( query.clone(), format!( diff --git a/grovedb/src/tests/proof_coverage_tests.rs b/grovedb/src/tests/proof_coverage_tests.rs index caa4aaebc..70ea0d5dd 100644 --- a/grovedb/src/tests/proof_coverage_tests.rs +++ b/grovedb/src/tests/proof_coverage_tests.rs @@ -8127,4 +8127,464 @@ mod tests { } false } + + // ========================================================================= + // Terminal non-Merk tree elements must stay bound to the parent value_hash + // + // A CommitmentTree / MmrTree / BulkAppendTree / + // DenseAppendOnlyFixedSizeTree reported as a terminal result (the query + // targets the tree element itself and the prover emits no lower layer) + // used to be proved with a bare `KVValueHash` node. That node hashes only + // (key, value_hash), so the serialized element bytes — which carry the + // entry count a caller reads — were never bound to the value_hash the + // parent Merk commits to. The prover now emits + // `KVValueHashFeatureTypeWithChildHash` carrying the subtree's state root, + // and the verifier requires it. + // ========================================================================= + + /// How a forged terminal non-Merk tree node is dressed up in the proof. + #[derive(Clone, Copy)] + enum TerminalForgery { + /// Keep the `KVValueHashFeatureTypeWithChildHash` node the honest + /// prover emits (value_hash and child_hash untouched) and only swap the + /// element bytes. The merk verifier's + /// `combine_hash(H(value), child_hash) == value_hash` check must catch + /// this. + KeepChildHash, + /// Downgrade to the bare `KVValueHash` node the prover used to emit, + /// dropping the child hash entirely. This is the shape the soundness + /// gap allowed: nothing in the node ties the element bytes to the + /// value_hash, so only the verifier's `child_hash_verified` requirement + /// can catch it. + DowngradeToKvValueHash, + } + + /// Rewrite the terminal proof node for `target_key` in the TEST_LEAF layer, + /// substituting `fake_element_bytes` for the element bytes while keeping + /// the genuine value_hash. Returns the re-encoded proof. + fn forge_terminal_tree_element( + proof_bytes: &[u8], + target_key: &[u8], + fake_element_bytes: &[u8], + forgery: TerminalForgery, + ) -> Vec { + use grovedb_merk::proofs::{encode_into, Decoder, Node, Op}; + + let config = bincode::config::standard() + .with_big_endian() + .with_limit::<{ 256 * 1024 * 1024 }>(); + let (mut grovedb_proof, _): (GroveDBProof, _) = + bincode::decode_from_slice(proof_bytes, config).expect("decode"); + + let GroveDBProof::V1(ref mut v1) = grovedb_proof else { + panic!("expected a V1 envelope"); + }; + let leaf_layer = v1 + .root_layer + .lower_layers + .get_mut(TEST_LEAF) + .expect("TEST_LEAF lower layer"); + let bytes = match leaf_layer.merk_proof { + crate::operations::proof::ProofBytes::Merk(ref mut bytes) => bytes, + _ => panic!("expected Merk proof bytes at the TEST_LEAF layer"), + }; + + let mut ops: Vec = Decoder::new(bytes).map(|r| r.expect("decode op")).collect(); + + let mut forged = false; + for op in ops.iter_mut() { + let Op::Push(Node::KVValueHashFeatureTypeWithChildHash( + key, + _value, + value_hash, + feature_type, + child_hash, + )) = op + else { + continue; + }; + if key.as_slice() != target_key { + continue; + } + *op = match forgery { + TerminalForgery::KeepChildHash => { + Op::Push(Node::KVValueHashFeatureTypeWithChildHash( + key.clone(), + fake_element_bytes.to_vec(), + *value_hash, + *feature_type, + *child_hash, + )) + } + TerminalForgery::DowngradeToKvValueHash => Op::Push(Node::KVValueHash( + key.clone(), + fake_element_bytes.to_vec(), + *value_hash, + )), + }; + forged = true; + break; + } + assert!( + forged, + "honest proof should carry a KVValueHashFeatureTypeWithChildHash node for the \ + terminal tree — the prover must bind its element bytes" + ); + + let mut new_bytes = Vec::new(); + encode_into(ops.iter(), &mut new_bytes); + *bytes = new_bytes; + + bincode::encode_to_vec(&grovedb_proof, config).expect("re-encode") + } + + #[test] + fn terminal_commitment_tree_count_forgery_is_detected() { + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + // A populated CommitmentTree under TEST_LEAF. + db.insert( + [TEST_LEAF].as_ref(), + b"pool", + Element::empty_commitment_tree(10).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert commitment tree"); + + for i in 1..=3u8 { + let mut cmx = [0u8; 32]; + cmx[0] = i; + cmx[31] &= 0x7f; + let mut rho = [0u8; 32]; + rho[0] = i; + rho[1] = 0xAA; + let mut cv_net = [0u8; 32]; + cv_net[0] = i; + cv_net[1] = 0xCC; + + let mut epk_bytes = [0u8; 32]; + epk_bytes[0] = i; + let mut enc_data = [0u8; 104]; + enc_data[0] = i; + let mut out_ciphertext = [0u8; 80]; + out_ciphertext[0] = i; + let ciphertext = grovedb_commitment_tree::TransmittedNoteCiphertext::< + grovedb_commitment_tree::DashMemo, + >::from_parts( + epk_bytes, + grovedb_commitment_tree::NoteBytesData(enc_data), + out_ciphertext, + ); + + db.commitment_tree_insert( + [TEST_LEAF].as_ref(), + b"pool", + cmx, + rho, + cv_net, + ciphertext, + None, + grove_version, + ) + .unwrap() + .expect("append note"); + } + + // Query the tree element itself — no subquery, so the prover reports + // it terminally with no lower layer. + let mut query = Query::new(); + query.insert_key(b"pool".to_vec()); + let path_query = PathQuery::new_unsized(vec![TEST_LEAF.to_vec()], query); + + let proof_bytes = db + .prove_query(&path_query, None, grove_version) + .unwrap() + .expect("prove"); + + let (_, results) = + GroveDb::verify_query_raw(&proof_bytes, &path_query, grove_version).expect("verify"); + let real_element_bytes = results[0].value.clone(); + let element = Element::deserialize(&real_element_bytes, grove_version).expect("deser"); + let (chunk_power, flags) = match &element { + Element::CommitmentTree(total_count, chunk_power, flags) => { + assert_eq!(*total_count, 3, "honest proof should report 3 notes"); + (*chunk_power, flags.clone()) + } + other => panic!("expected CommitmentTree, got {:?}", other), + }; + + // Forge the note count while keeping the genuine value_hash. This is + // the denominator a shielded-balance client reads, so an inflated or + // deflated count under a real root hash is directly exploitable. + let fake_element_bytes = Element::CommitmentTree(999, chunk_power, flags) + .serialize(grove_version) + .expect("serialize"); + + for forgery in [ + TerminalForgery::KeepChildHash, + TerminalForgery::DowngradeToKvValueHash, + ] { + let tampered_proof_bytes = + forge_terminal_tree_element(&proof_bytes, b"pool", &fake_element_bytes, forgery); + + let tampered_result = + GroveDb::verify_query_raw(&tampered_proof_bytes, &path_query, grove_version); + assert!( + tampered_result.is_err(), + "forged CommitmentTree total_count must be rejected, but verification accepted \ + it: {:?}", + tampered_result.map(|(_, r)| r + .iter() + .map(|p| Element::deserialize(&p.value, grove_version)) + .collect::>()) + ); + } + + // The honest proof still verifies and still reports the real count. + let (_, results) = + GroveDb::verify_query_raw(&proof_bytes, &path_query, grove_version).expect("verify"); + assert_eq!(results[0].value, real_element_bytes); + } + + #[test] + fn terminal_mmr_tree_size_forgery_is_detected() { + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + db.insert( + [TEST_LEAF].as_ref(), + b"mmr", + Element::empty_mmr_tree(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert mmr tree"); + + for i in 0..3u8 { + db.mmr_tree_append( + [TEST_LEAF].as_ref(), + b"mmr", + vec![i; 8], + None, + grove_version, + ) + .unwrap() + .expect("append leaf"); + } + + let mut query = Query::new(); + query.insert_key(b"mmr".to_vec()); + let path_query = PathQuery::new_unsized(vec![TEST_LEAF.to_vec()], query); + + let proof_bytes = db + .prove_query(&path_query, None, grove_version) + .unwrap() + .expect("prove"); + + let (_, results) = + GroveDb::verify_query_raw(&proof_bytes, &path_query, grove_version).expect("verify"); + let real_element_bytes = results[0].value.clone(); + let element = Element::deserialize(&real_element_bytes, grove_version).expect("deser"); + let flags = match &element { + Element::MmrTree(mmr_size, flags) => { + assert!(*mmr_size > 0, "honest proof should report a populated MMR"); + flags.clone() + } + other => panic!("expected MmrTree, got {:?}", other), + }; + + let fake_element_bytes = Element::MmrTree(999, flags) + .serialize(grove_version) + .expect("serialize"); + + for forgery in [ + TerminalForgery::KeepChildHash, + TerminalForgery::DowngradeToKvValueHash, + ] { + let tampered_proof_bytes = + forge_terminal_tree_element(&proof_bytes, b"mmr", &fake_element_bytes, forgery); + assert!( + GroveDb::verify_query_raw(&tampered_proof_bytes, &path_query, grove_version) + .is_err(), + "forged MmrTree size must be rejected" + ); + } + } + + #[test] + fn terminal_bulk_append_and_dense_tree_forgeries_are_detected() { + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + db.insert( + [TEST_LEAF].as_ref(), + b"bulk", + Element::empty_bulk_append_tree(4).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert bulk append tree"); + db.insert( + [TEST_LEAF].as_ref(), + b"dense", + Element::empty_dense_tree(4), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert dense tree"); + + for i in 0..3u8 { + db.bulk_append( + [TEST_LEAF].as_ref(), + b"bulk", + vec![i; 8], + None, + grove_version, + ) + .unwrap() + .expect("bulk append"); + db.dense_tree_insert( + [TEST_LEAF].as_ref(), + b"dense", + vec![i; 8], + None, + grove_version, + ) + .unwrap() + .expect("dense insert"); + } + + for (key, forge) in [ + ( + b"bulk".as_slice(), + &(|e: &Element| match e { + Element::BulkAppendTree(_, chunk_power, flags) => { + Element::BulkAppendTree(999, *chunk_power, flags.clone()) + } + other => panic!("expected BulkAppendTree, got {:?}", other), + }) as &dyn Fn(&Element) -> Element, + ), + ( + b"dense".as_slice(), + &(|e: &Element| match e { + Element::DenseAppendOnlyFixedSizeTree(_, height, flags) => { + Element::DenseAppendOnlyFixedSizeTree(9, *height, flags.clone()) + } + other => panic!("expected DenseAppendOnlyFixedSizeTree, got {:?}", other), + }) as &dyn Fn(&Element) -> Element, + ), + ] { + let mut query = Query::new(); + query.insert_key(key.to_vec()); + let path_query = PathQuery::new_unsized(vec![TEST_LEAF.to_vec()], query); + + let proof_bytes = db + .prove_query(&path_query, None, grove_version) + .unwrap() + .expect("prove"); + let (_, results) = GroveDb::verify_query_raw(&proof_bytes, &path_query, grove_version) + .expect("honest proof should verify"); + let element = Element::deserialize(&results[0].value, grove_version).expect("deser"); + let fake_element_bytes = forge(&element).serialize(grove_version).expect("serialize"); + + for forgery in [ + TerminalForgery::KeepChildHash, + TerminalForgery::DowngradeToKvValueHash, + ] { + let tampered_proof_bytes = + forge_terminal_tree_element(&proof_bytes, key, &fake_element_bytes, forgery); + assert!( + GroveDb::verify_query_raw(&tampered_proof_bytes, &path_query, grove_version) + .is_err(), + "forged count for {} must be rejected", + String::from_utf8_lossy(key) + ); + } + } + } + + #[test] + fn empty_non_merk_trees_still_prove_and_verify() { + // The child-hash binding applies to empty non-Merk trees too, whose + // committed child hash is NULL_HASH (or + // EMPTY_COMMITMENT_TREE_STATE_ROOT for a CommitmentTree). These honest + // proofs must keep verifying. + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + + for (key, element) in [ + ( + b"ct".as_slice(), + Element::empty_commitment_tree(10).expect("valid chunk_power"), + ), + (b"mmr".as_slice(), Element::empty_mmr_tree()), + ( + b"bulk".as_slice(), + Element::empty_bulk_append_tree(4).expect("valid chunk_power"), + ), + (b"dense".as_slice(), Element::empty_dense_tree(4)), + ] { + db.insert( + [TEST_LEAF].as_ref(), + key, + element.clone(), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert empty non-merk tree"); + + let mut query = Query::new(); + query.insert_key(key.to_vec()); + let path_query = PathQuery::new_unsized(vec![TEST_LEAF.to_vec()], query); + + let proof_bytes = db + .prove_query(&path_query, None, grove_version) + .unwrap() + .expect("prove"); + let (_, results) = GroveDb::verify_query_raw(&proof_bytes, &path_query, grove_version) + .unwrap_or_else(|e| { + panic!( + "empty {} proof should verify, got {:?}", + String::from_utf8_lossy(key), + e + ) + }); + assert_eq!( + results[0].value, + element.serialize(grove_version).expect("serialize"), + "empty {} should round-trip", + String::from_utf8_lossy(key) + ); + + // And a forgery on the empty tree is still caught. + let fake_element_bytes = Element::empty_mmr_tree() + .serialize(grove_version) + .expect("serialize"); + if fake_element_bytes != results[0].value { + let tampered = forge_terminal_tree_element( + &proof_bytes, + key, + &fake_element_bytes, + TerminalForgery::KeepChildHash, + ); + assert!( + GroveDb::verify_query_raw(&tampered, &path_query, grove_version).is_err(), + "type swap on empty {} must be rejected", + String::from_utf8_lossy(key) + ); + } + } + } } From 906c2fe4d407e2301b42913533f87b258b238e39 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 3 Aug 2026 01:15:43 +0700 Subject: [PATCH 4/6] fix(version): gate the terminal non-Merk tree child hash behind GROVE_V4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The binding fix in the previous commit was applied unconditionally, which is wrong: GROVE_V3 is live, and per v4.rs a fix that changes an accepted/rejected outcome or a tracked cost cannot land on a released version without nodes carrying it diverging from nodes that do not. This change does both. An upgraded verifier rejects proofs a released one accepts, and deriving the tree's state root costs the prover storage reads and hash calls that V1..V3 never paid — and cost feeds fees. Adds `proof.terminal_non_merk_tree_child_hash`, 0 in V1..V3 and 1 in V4, and branches both sides on it: - Prover: under v0 the node is left exactly as it has always been emitted (bare KVValueHash) and only the limit moves, so the released byte and cost shape is untouched. Under v1 it computes the state root and rewrites the node. - Verifier: non-empty *Merk* trees keep requiring the child hash at every version — that has been released behaviour since V3 and is unchanged. Only the four non-Merk types are added, and only from V4, so a V4 verifier never rejects an honest V3 proof and a V3 verifier never demands a node a V3 prover does not emit. The consequence worth being explicit about: the forgery stays exploitable on V1..V3 and closes when protocol v4 activates. That is inherent to gating — fixing it in place is the divergence v4.rs exists to prevent — and it matches how the other fixes parked on V4 are being handled. `terminal_non_merk_tree_child_hash_version_gate` pins both sides: it asserts GROVE_V3 still emits a bare KVValueHash and still accepts the forged total_count, and that GROVE_V4 emits the child-hash node and rejects it. The V3 assertion is deliberately an assertion about a hole — if it starts failing, the fix has leaked into a released version. Also documents the gate in v4.rs's header alongside the two existing ones, as that file asks. Co-Authored-By: Claude Opus 5 --- .../src/version/grovedb_versions.rs | 27 +++ grovedb-version/src/version/v1.rs | 1 + grovedb-version/src/version/v2.rs | 1 + grovedb-version/src/version/v3.rs | 1 + grovedb-version/src/version/v4.rs | 12 ++ grovedb/src/operations/proof/generate.rs | 107 ++++++---- grovedb/src/operations/proof/verify.rs | 46 ++-- grovedb/src/tests/proof_coverage_tests.rs | 196 ++++++++++++++++++ 8 files changed, 332 insertions(+), 59 deletions(-) diff --git a/grovedb-version/src/version/grovedb_versions.rs b/grovedb-version/src/version/grovedb_versions.rs index 70b884f93..b63af3694 100644 --- a/grovedb-version/src/version/grovedb_versions.rs +++ b/grovedb-version/src/version/grovedb_versions.rs @@ -139,6 +139,33 @@ pub struct GroveDBOperationsProofVersions { pub verify_subset_query_with_absence_proof: FeatureVersion, pub verify_query_with_chained_path_queries: FeatureVersion, pub verify_query_get_parent_tree_info_with_options: FeatureVersion, + /// Whether a V1 proof binds the element bytes of a **terminally-reported + /// non-Merk tree** — `CommitmentTree`, `MmrTree`, `BulkAppendTree`, + /// `DenseAppendOnlyFixedSizeTree` — to the `value_hash` its parent Merk + /// commits to. "Terminal" means the query targets the tree element itself + /// and the prover emits no lower layer. + /// + /// - `0` (V1..V3): the prover emits a bare `KVValueHash` node and the + /// verifier does not require a child hash. That node hashes only + /// `(key, value_hash)`, so the serialized element bytes are unbound: a + /// prover can serve a forged entry count (an inflated or deflated + /// `CommitmentTree` `total_count`, a different MMR size) alongside the + /// genuine `value_hash` and still reconstruct the correct root hash. + /// - `1` (V4+): the prover emits + /// `KVValueHashFeatureTypeWithChildHash` carrying the tree's own state + /// root, and the verifier requires it, so the merk-level + /// `combine_hash(H(value), child_hash) == value_hash` check closes the + /// loop. This is exactly the composition the parent commits, since these + /// types are written through `insert_subtree`. + /// + /// Gated rather than applied unconditionally on two counts. It flips an + /// accepted/rejected outcome — an upgraded verifier rejects proofs a + /// released one accepts — and computing the state root costs the prover + /// extra storage reads and hash calls on a released path. The + /// non-Merk tree types this covers are the only elements affected; + /// non-empty **Merk** trees have required the child hash since V3 and + /// stay bound at every version. + pub terminal_non_merk_tree_child_hash: FeatureVersion, } #[derive(Clone, Debug, Default)] diff --git a/grovedb-version/src/version/v1.rs b/grovedb-version/src/version/v1.rs index 8d44fd30a..be2b1d472 100644 --- a/grovedb-version/src/version/v1.rs +++ b/grovedb-version/src/version/v1.rs @@ -166,6 +166,7 @@ pub const GROVE_V1: GroveVersion = GroveVersion { verify_subset_query_with_absence_proof: 0, verify_query_with_chained_path_queries: 0, verify_query_get_parent_tree_info_with_options: 0, + terminal_non_merk_tree_child_hash: 0, }, average_case: GroveDBOperationsAverageCaseVersions { add_average_case_get_merk_at_path: 0, diff --git a/grovedb-version/src/version/v2.rs b/grovedb-version/src/version/v2.rs index d7f15c9b3..14f8d0936 100644 --- a/grovedb-version/src/version/v2.rs +++ b/grovedb-version/src/version/v2.rs @@ -166,6 +166,7 @@ pub const GROVE_V2: GroveVersion = GroveVersion { verify_subset_query_with_absence_proof: 0, verify_query_with_chained_path_queries: 0, verify_query_get_parent_tree_info_with_options: 0, + terminal_non_merk_tree_child_hash: 0, }, average_case: GroveDBOperationsAverageCaseVersions { add_average_case_get_merk_at_path: 0, diff --git a/grovedb-version/src/version/v3.rs b/grovedb-version/src/version/v3.rs index 7f42fb351..c65b8518f 100644 --- a/grovedb-version/src/version/v3.rs +++ b/grovedb-version/src/version/v3.rs @@ -170,6 +170,7 @@ pub const GROVE_V3: GroveVersion = GroveVersion { verify_subset_query_with_absence_proof: 0, verify_query_with_chained_path_queries: 0, verify_query_get_parent_tree_info_with_options: 0, + terminal_non_merk_tree_child_hash: 0, }, average_case: GroveDBOperationsAverageCaseVersions { add_average_case_get_merk_at_path: 0, diff --git a/grovedb-version/src/version/v4.rs b/grovedb-version/src/version/v4.rs index d701a17c2..7b53a2033 100644 --- a/grovedb-version/src/version/v4.rs +++ b/grovedb-version/src/version/v4.rs @@ -17,6 +17,17 @@ //! case. Same shape as the gate above: one extra stored-element read per //! overwrite-capable op, so V1..V3 keep their released cost shape. //! +//! - `proof.terminal_non_merk_tree_child_hash: 1` — a V1 proof that reports a +//! `CommitmentTree` / `MmrTree` / `BulkAppendTree` / +//! `DenseAppendOnlyFixedSizeTree` as a terminal result (query targets the +//! tree element itself, no lower layer) carries the tree's state root in a +//! `KVValueHashFeatureTypeWithChildHash` node, and the verifier requires it. +//! V1..V3 emit a bare `KVValueHash`, which hashes only `(key, value_hash)` +//! and so leaves the element bytes — including the entry count callers read +//! — free for a prover to forge under a genuine root hash. Gated because it +//! flips a rejected/accepted outcome and because deriving the state root +//! costs the prover extra storage reads and hash calls. +//! //! Note that `GroveVersion::latest()` resolves to this version, so anything //! defaulting to "latest" — tests, benchmarks, tools — exercises every gate //! listed above rather than V3 behaviour. @@ -205,6 +216,7 @@ pub const GROVE_V4: GroveVersion = GroveVersion { verify_subset_query_with_absence_proof: 0, verify_query_with_chained_path_queries: 0, verify_query_get_parent_tree_info_with_options: 0, + terminal_non_merk_tree_child_hash: 1, // bind terminal non-Merk tree element bytes to the parent value_hash }, average_case: GroveDBOperationsAverageCaseVersions { add_average_case_get_merk_at_path: 0, diff --git a/grovedb/src/operations/proof/generate.rs b/grovedb/src/operations/proof/generate.rs index 67e4df338..5fa92a40a 100644 --- a/grovedb/src/operations/proof/generate.rs +++ b/grovedb/src/operations/proof/generate.rs @@ -2352,6 +2352,15 @@ impl GroveDb { // `KVValueHashFeatureTypeWithChildHash` is // verified with, so carry the state root in the // node and let the merk verifier close the loop. + // + // Version-gated on + // `proof.terminal_non_merk_tree_child_hash`: + // deriving the state root costs storage reads and + // hash calls that V1..V3 did not pay, and cost + // feeds fees. Under those versions the node is + // left as the prover has always emitted it and + // only the limit moves — the released shape, which + // the matching verifier gate still accepts. Ok(ref non_merk_elem @ Element::MmrTree(..)) | Ok(ref non_merk_elem @ Element::BulkAppendTree(..)) | Ok( @@ -2360,57 +2369,65 @@ impl GroveDb { | Ok(ref non_merk_elem @ Element::CommitmentTree(..)) if !done_with_results => { - let mut child_path = path.clone(); - child_path.push(key.as_slice()); + if grove_version + .grovedb_versions + .operations + .proof + .terminal_non_merk_tree_child_hash + >= 1 + { + let mut child_path = path.clone(); + child_path.push(key.as_slice()); - let child_hash = cost_return_on_error!( - &mut cost, - self.non_merk_tree_child_hash( - non_merk_elem, - &child_path, - &tx, - ) - ); + let child_hash = cost_return_on_error!( + &mut cost, + self.non_merk_tree_child_hash( + non_merk_elem, + &child_path, + &tx, + ) + ); - let key_owned = key.to_owned(); - let value_owned = value.to_owned(); - let element_vh = - value_hash(&value_owned).unwrap_add_cost(&mut cost); - let recomputed = combine_hash(&element_vh, &child_hash) - .unwrap_add_cost(&mut cost); - let (vh, ft) = match node { - Node::KVValueHashFeatureType(_, _, vh, ft) => (*vh, *ft), - Node::KVValueHash(_, _, vh) => { - (*vh, TreeFeatureType::BasicMerkNode) + let key_owned = key.to_owned(); + let value_owned = value.to_owned(); + let element_vh = + value_hash(&value_owned).unwrap_add_cost(&mut cost); + let recomputed = combine_hash(&element_vh, &child_hash) + .unwrap_add_cost(&mut cost); + let (vh, ft) = match node { + Node::KVValueHashFeatureType(_, _, vh, ft) => (*vh, *ft), + Node::KVValueHash(_, _, vh) => { + (*vh, TreeFeatureType::BasicMerkNode) + } + _ => (recomputed, TreeFeatureType::BasicMerkNode), + }; + + // Self-check: if the recomputed state root + // does not reproduce the committed + // value_hash, the node we are about to emit + // would be rejected by the verifier. Fail + // here, where the cause is visible, rather + // than shipping a proof that cannot verify. + if recomputed != vh { + return Err(Error::CorruptedData(format!( + "non-Merk tree at key {} has state root {} which \ + does not reproduce the committed value hash {}", + hex::encode(&key_owned), + hex::encode(child_hash), + hex::encode(vh), + ))) + .wrap_with_cost(cost); } - _ => (recomputed, TreeFeatureType::BasicMerkNode), - }; - // Self-check: if the recomputed state root does - // not reproduce the committed value_hash, the - // node we are about to emit would be rejected - // by the verifier. Fail here, where the cause - // is visible, rather than shipping a proof that - // cannot verify. - if recomputed != vh { - return Err(Error::CorruptedData(format!( - "non-Merk tree at key {} has state root {} which does \ - not reproduce the committed value hash {}", - hex::encode(&key_owned), - hex::encode(child_hash), - hex::encode(vh), - ))) - .wrap_with_cost(cost); + *node = Node::KVValueHashFeatureTypeWithChildHash( + key_owned, + value_owned, + vh, + ft, + child_hash, + ); } - *node = Node::KVValueHashFeatureTypeWithChildHash( - key_owned, - value_owned, - vh, - ft, - child_hash, - ); - if let Some(limit) = overall_limit.as_mut() { *limit -= 1; } diff --git a/grovedb/src/operations/proof/verify.rs b/grovedb/src/operations/proof/verify.rs index 395f1d1df..ab3db34cd 100644 --- a/grovedb/src/operations/proof/verify.rs +++ b/grovedb/src/operations/proof/verify.rs @@ -1379,20 +1379,38 @@ impl GroveDb { // false, an attacker may have downgraded the node type // to hide child hash verification. // - // This covers non-empty Merk trees (child_hash = child - // Merk root) and all four non-Merk trees — - // CommitmentTree, MmrTree, BulkAppendTree, - // DenseAppendOnlyFixedSizeTree (child_hash = the tree's - // own state root, which their parent commits through - // the same two-input combine_hash). `is_non_empty_tree` - // is true unconditionally for those four, so an empty - // one is bound too — its committed child hash is - // NULL_HASH, or EMPTY_COMMITMENT_TREE_STATE_ROOT for a - // CommitmentTree. Without this, the element bytes of a - // terminally-reported non-Merk tree were unbound and a - // prover could forge the entry count callers read from - // them. - if element.is_non_empty_tree() && !proved_key_value.child_hash_verified { + // Non-empty Merk trees (child_hash = child Merk root) + // have required this since V3 and are checked at every + // version. + // + // The four non-Merk trees — CommitmentTree, MmrTree, + // BulkAppendTree, DenseAppendOnlyFixedSizeTree + // (child_hash = the tree's own state root, which their + // parent commits through the same two-input + // combine_hash) — are checked only from V4, under + // `proof.terminal_non_merk_tree_child_hash`. V1..V3 + // provers emit a bare KVValueHash here, so demanding + // the child hash from them would reject honest + // released proofs; the gate moves prover and verifier + // together at the protocol boundary. Until it + // activates, the element bytes of a terminally-reported + // non-Merk tree stay unbound and a prover can forge the + // entry count callers read from them. + // + // `is_non_empty_tree` is true unconditionally for those + // four, so an empty one is bound too — its committed + // child hash is NULL_HASH, or + // EMPTY_COMMITMENT_TREE_STATE_ROOT for a + // CommitmentTree. + let requires_child_hash = element.is_non_empty_merk_tree() + || (grove_version + .grovedb_versions + .operations + .proof + .terminal_non_merk_tree_child_hash + >= 1 + && element.is_non_empty_tree()); + if requires_child_hash && !proved_key_value.child_hash_verified { return Err(Error::InvalidProof( query.clone(), format!( diff --git a/grovedb/src/tests/proof_coverage_tests.rs b/grovedb/src/tests/proof_coverage_tests.rs index 70ea0d5dd..386258658 100644 --- a/grovedb/src/tests/proof_coverage_tests.rs +++ b/grovedb/src/tests/proof_coverage_tests.rs @@ -8238,6 +8238,202 @@ mod tests { bincode::encode_to_vec(&grovedb_proof, config).expect("re-encode") } + /// Version-agnostic sibling of [`forge_terminal_tree_element`]: swaps the + /// element bytes of whatever terminal node the prover emitted for + /// `target_key`, keeping its value_hash, and reports whether that node was + /// the child-hash-bearing kind. Used to pin behaviour on both sides of the + /// `terminal_non_merk_tree_child_hash` gate, where the node shape differs. + fn forge_terminal_node_any_shape( + proof_bytes: &[u8], + target_key: &[u8], + fake_element_bytes: &[u8], + ) -> (bool, Vec) { + use grovedb_merk::proofs::{encode_into, Decoder, Node, Op}; + + let config = bincode::config::standard() + .with_big_endian() + .with_limit::<{ 256 * 1024 * 1024 }>(); + let (mut grovedb_proof, _): (GroveDBProof, _) = + bincode::decode_from_slice(proof_bytes, config).expect("decode"); + + let GroveDBProof::V1(ref mut v1) = grovedb_proof else { + panic!("expected a V1 envelope"); + }; + let leaf_layer = v1 + .root_layer + .lower_layers + .get_mut(TEST_LEAF) + .expect("TEST_LEAF lower layer"); + let bytes = match leaf_layer.merk_proof { + crate::operations::proof::ProofBytes::Merk(ref mut bytes) => bytes, + _ => panic!("expected Merk proof bytes at the TEST_LEAF layer"), + }; + + let mut ops: Vec = Decoder::new(bytes).map(|r| r.expect("decode op")).collect(); + + let mut had_child_hash = None; + for op in ops.iter_mut() { + match op { + Op::Push(Node::KVValueHashFeatureTypeWithChildHash( + key, + _v, + value_hash, + feature_type, + child_hash, + )) if key.as_slice() == target_key => { + had_child_hash = Some(true); + *op = Op::Push(Node::KVValueHashFeatureTypeWithChildHash( + key.clone(), + fake_element_bytes.to_vec(), + *value_hash, + *feature_type, + *child_hash, + )); + break; + } + Op::Push(Node::KVValueHash(key, _v, value_hash)) + if key.as_slice() == target_key => + { + had_child_hash = Some(false); + *op = Op::Push(Node::KVValueHash( + key.clone(), + fake_element_bytes.to_vec(), + *value_hash, + )); + break; + } + _ => continue, + } + } + let had_child_hash = + had_child_hash.expect("proof should carry a value-bearing node for the target key"); + + let mut new_bytes = Vec::new(); + encode_into(ops.iter(), &mut new_bytes); + *bytes = new_bytes; + + ( + had_child_hash, + bincode::encode_to_vec(&grovedb_proof, config).expect("re-encode"), + ) + } + + /// Consensus version gate for `proof.terminal_non_merk_tree_child_hash`. + /// + /// A terminal non-Merk tree (here a populated `CommitmentTree`) is proved + /// with a bare `KVValueHash` under **v0** (`GROVE_V1`..`GROVE_V3` — the + /// released shape), which hashes only `(key, value_hash)` and so leaves the + /// element bytes unbound: a forged `total_count` verifies against the real + /// root hash. Under **v1** (`GROVE_V4`+) the prover emits + /// `KVValueHashFeatureTypeWithChildHash` carrying the tree's state root and + /// the verifier requires it, so the same forgery is rejected. + /// + /// This pins both sides so the gate cannot silently collapse to one + /// behaviour — which would either reject honest released proofs (if V3 + /// started demanding the child hash) or silently reopen the forgery (if V4 + /// stopped). + #[test] + fn terminal_non_merk_tree_child_hash_version_gate() { + use grovedb_version::version::v3::GROVE_V3; + + let build_and_forge = |grove_version: &GroveVersion| { + let db = make_test_grovedb(grove_version); + db.insert( + [TEST_LEAF].as_ref(), + b"pool", + Element::empty_commitment_tree(10).expect("valid chunk_power"), + None, + None, + grove_version, + ) + .unwrap() + .expect("insert commitment tree"); + + let mut cmx = [0u8; 32]; + cmx[0] = 1; + cmx[31] &= 0x7f; + let mut rho = [0u8; 32]; + rho[0] = 1; + rho[1] = 0xAA; + let mut cv_net = [0u8; 32]; + cv_net[0] = 1; + cv_net[1] = 0xCC; + let ciphertext = grovedb_commitment_tree::TransmittedNoteCiphertext::< + grovedb_commitment_tree::DashMemo, + >::from_parts( + [7u8; 32], + grovedb_commitment_tree::NoteBytesData([3u8; 104]), + [5u8; 80], + ); + db.commitment_tree_insert( + [TEST_LEAF].as_ref(), + b"pool", + cmx, + rho, + cv_net, + ciphertext, + None, + grove_version, + ) + .unwrap() + .expect("append note"); + + let mut query = Query::new(); + query.insert_key(b"pool".to_vec()); + let path_query = PathQuery::new_unsized(vec![TEST_LEAF.to_vec()], query); + + let proof_bytes = db + .prove_query(&path_query, None, grove_version) + .unwrap() + .expect("prove"); + + // The honest proof must verify at every version. + let (_, results) = GroveDb::verify_query_raw(&proof_bytes, &path_query, grove_version) + .expect("honest proof should verify"); + let element = Element::deserialize(&results[0].value, grove_version).expect("deser"); + let (chunk_power, flags) = match &element { + Element::CommitmentTree(total_count, chunk_power, flags) => { + assert_eq!(*total_count, 1, "honest proof should report 1 note"); + (*chunk_power, flags.clone()) + } + other => panic!("expected CommitmentTree, got {:?}", other), + }; + + let fake_element_bytes = Element::CommitmentTree(999, chunk_power, flags) + .serialize(grove_version) + .expect("serialize"); + let (had_child_hash, tampered) = + forge_terminal_node_any_shape(&proof_bytes, b"pool", &fake_element_bytes); + let accepted = GroveDb::verify_query_raw(&tampered, &path_query, grove_version).is_ok(); + (had_child_hash, accepted) + }; + + // v0 — GROVE_V3, the released shape. Bare KVValueHash, forgery + // accepted. This is the hole; it cannot be closed in place because + // V3 is live. + let (v3_child_hash, v3_accepted) = build_and_forge(&GROVE_V3); + assert!( + !v3_child_hash, + "GROVE_V3 must keep emitting a bare KVValueHash for a terminal non-Merk tree" + ); + assert!( + v3_accepted, + "GROVE_V3 is expected to still accept the forgery — if this now fails, the fix \ + leaked into a released version and changes consensus behaviour" + ); + + // v1 — GROVE_V4, latest. Child-hash node, forgery rejected. + let (v4_child_hash, v4_accepted) = build_and_forge(GroveVersion::latest()); + assert!( + v4_child_hash, + "GROVE_V4 must emit KVValueHashFeatureTypeWithChildHash for a terminal non-Merk tree" + ); + assert!( + !v4_accepted, + "GROVE_V4 must reject a forged CommitmentTree total_count" + ); + } + #[test] fn terminal_commitment_tree_count_forgery_is_detected() { let grove_version = GroveVersion::latest(); From 0e8a674f41b936df39a5ec81078db66116c4263a Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 3 Aug 2026 01:31:55 +0700 Subject: [PATCH 5/6] refactor(grovedb): make the terminal non-Merk tree gate a file-versioned function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate added in the previous commit was an inline `if ...terminal_non_merk_tree_child_hash >= 1` branch inside `prove_subqueries_v1`. That does not follow the versioning system's code structure: every other gated behaviour in the repo is a module directory with one file per version and a dispatching `mod.rs`, so each version's behaviour reads on its own instead of being reconstructed from a conditional. Restructures it into `operations/proof/bind_terminal_non_merk_tree/{mod,v0,v1}.rs`, following `operations/insert/add_element_on_transaction/`: - `mod.rs` matches the slot and dispatches, with an `UnknownVersionMismatch` arm, and carries the docs on what differs between versions. - `v0.rs` is the released no-op. It takes the same arguments and ignores them, and documents that the unbound element bytes are a known gap preserved because GROVE_V3 is live — not an oversight. - `v1.rs` holds the state-root derivation, the self-check and the node rewrite. `non_merk_tree_child_hash` moves here from `generate.rs`, since only v1 needs it. The gated unit is the binding step rather than the whole enclosing function, which keeps the per-version files small — duplicating the ~1000-line `prove_subqueries_v1` would not have been reasonable. `generate.rs` now just calls the dispatcher. Passing `&mut Node` and deriving key/value inside also resolves the borrow that forced the old code to clone them up front. Behaviour is unchanged: `terminal_non_merk_tree_child_hash_version_gate` still pins GROVE_V3 to the bare KVValueHash (forgery accepted) and GROVE_V4 to the child-hash node (forgery rejected). The module is `minimal`-gated, so verify-only builds are unaffected. Co-Authored-By: Claude Opus 5 --- .../proof/bind_terminal_non_merk_tree/mod.rs | 84 ++++++ .../proof/bind_terminal_non_merk_tree/v0.rs | 37 +++ .../proof/bind_terminal_non_merk_tree/v1.rs | 248 ++++++++++++++++++ grovedb/src/operations/proof/generate.rs | 237 ++--------------- grovedb/src/operations/proof/mod.rs | 5 + 5 files changed, 391 insertions(+), 220 deletions(-) create mode 100644 grovedb/src/operations/proof/bind_terminal_non_merk_tree/mod.rs create mode 100644 grovedb/src/operations/proof/bind_terminal_non_merk_tree/v0.rs create mode 100644 grovedb/src/operations/proof/bind_terminal_non_merk_tree/v1.rs diff --git a/grovedb/src/operations/proof/bind_terminal_non_merk_tree/mod.rs b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/mod.rs new file mode 100644 index 000000000..47bf72ed8 --- /dev/null +++ b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/mod.rs @@ -0,0 +1,84 @@ +//! `bind_terminal_non_merk_tree` — versioned dispatch. +//! +//! Binds the serialized element bytes of a **terminally-reported non-Merk +//! tree** — `CommitmentTree`, `MmrTree`, `BulkAppendTree`, +//! `DenseAppendOnlyFixedSizeTree` — to the `value_hash` its parent Merk +//! commits to. "Terminal" means the query targets the tree element itself and +//! the prover emits no lower layer, so there is no child layer to chain +//! through. +//! +//! These four types have no child Merk. Their parent entry is written by +//! `insert_subtree`, which commits `combine_hash(H(value), state_root)` — the +//! same two-input form that `Node::KVValueHashFeatureTypeWithChildHash` is +//! verified with. Carrying the state root in the node is therefore enough for +//! the merk verifier to close the loop; no new proof node type is needed. +//! +//! Whether it is carried is **consensus-critical** and version-gated on +//! `proof.terminal_non_merk_tree_child_hash`: +//! +//! * **[v0]** — released behaviour, `GROVE_V1`..`GROVE_V3`. The node is left +//! exactly as the prover emitted it (a bare `Node::KVValueHash`), which +//! hashes only `(key, value_hash)`. The element bytes are unbound: a prover +//! can serve a forged entry count — an inflated or deflated `CommitmentTree` +//! `total_count`, a different MMR size — alongside the genuine `value_hash` +//! and still reconstruct the correct root hash. +//! * **[v1]** — `GROVE_V4`+. The tree's state root is derived from storage and +//! the node is rewritten to `KVValueHashFeatureTypeWithChildHash`, so the +//! merk verifier's `combine_hash(H(value), child_hash) == value_hash` check +//! catches forged bytes. The matching verifier gate in +//! [`verify`](super::verify) requires the node from the same version. +//! +//! The split cannot be applied unconditionally on two counts: it flips an +//! accepted/rejected outcome, and deriving the state root costs the prover +//! storage reads and hash calls that the released versions never paid — cost +//! feeds fees. See `grovedb-version`'s `v4.rs` for the landing-zone rationale. +//! +//! [v0]: self::v0 +//! [v1]: self::v1 + +mod v0; +mod v1; + +use grovedb_costs::{CostResult, CostsExt, OperationCost}; +use grovedb_merk::proofs::Node; +use grovedb_version::version::GroveVersion; + +use crate::{Element, Error, GroveDb, Transaction}; + +impl GroveDb { + /// Bind a terminally-reported non-Merk tree's element bytes to the + /// parent-committed `value_hash`, if the grove version calls for it. + /// + /// `node` is the proof node standing for the tree element; `element` is + /// that node's already-deserialized (and `NonCounted`-unwrapped) value, and + /// must be one of the four non-Merk tree types. `parent_path` is the path + /// of the Merk holding the element — the tree's own data lives one level + /// below, under the node's key, which the versioned implementations append + /// themselves. + pub(crate) fn bind_terminal_non_merk_tree( + &self, + node: &mut Node, + element: &Element, + parent_path: &[&[u8]], + tx: &Transaction, + grove_version: &GroveVersion, + ) -> CostResult<(), Error> { + match grove_version + .grovedb_versions + .operations + .proof + .terminal_non_merk_tree_child_hash + { + 0 => self.bind_terminal_non_merk_tree_v0(node, element, parent_path, tx, grove_version), + 1 => self.bind_terminal_non_merk_tree_v1(node, element, parent_path, tx, grove_version), + version => Err(Error::VersionError( + grovedb_version::error::GroveVersionError::UnknownVersionMismatch { + method: "bind_terminal_non_merk_tree".to_string(), + known_versions: vec![0, 1], + received: version, + }, + )) + .wrap_with_cost(OperationCost::default()), + } + } +} diff --git a/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v0.rs b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v0.rs new file mode 100644 index 000000000..19d4fcff4 --- /dev/null +++ b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v0.rs @@ -0,0 +1,37 @@ +//! `bind_terminal_non_merk_tree` — **v0** (released behaviour, +//! `GROVE_V1`..`GROVE_V3`). +//! +//! Does nothing. The proof node for a terminally-reported non-Merk tree is left +//! exactly as the prover emitted it — a bare `Node::KVValueHash`, which hashes +//! only `(key, value_hash)` and leaves the serialized element bytes unbound. +//! +//! This is a **known soundness gap**, not an oversight to fix in place: a +//! prover can serve a forged entry count alongside the genuine `value_hash` and +//! still reconstruct the correct root hash. It is preserved here because +//! `GROVE_V3` is live — closing it changes both an accepted/rejected outcome +//! and the prover's tracked cost, so nodes carrying the fix would diverge from +//! nodes that do not. [`super::v1`] closes it from `GROVE_V4` onward; the hole +//! shuts when that protocol version activates. +//! +//! Deliberately takes the same arguments as [`super::v1`] and ignores them, so +//! the dispatch in [`super`][`mod@super`] stays a plain version match. + +use grovedb_costs::{CostResult, CostsExt, OperationCost}; +use grovedb_merk::proofs::Node; +use grovedb_version::version::GroveVersion; + +use crate::{Element, Error, GroveDb, Transaction}; + +impl GroveDb { + /// `bind_terminal_non_merk_tree` v0 — see the module documentation. + pub(crate) fn bind_terminal_non_merk_tree_v0( + &self, + _node: &mut Node, + _element: &Element, + _parent_path: &[&[u8]], + _tx: &Transaction, + _grove_version: &GroveVersion, + ) -> CostResult<(), Error> { + Ok(()).wrap_with_cost(OperationCost::default()) + } +} diff --git a/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v1.rs b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v1.rs new file mode 100644 index 000000000..f92384e65 --- /dev/null +++ b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v1.rs @@ -0,0 +1,248 @@ +//! `bind_terminal_non_merk_tree` — **v1** (`GROVE_V4`+). +//! +//! Derives the tree's own state root from storage and rewrites the proof node +//! to `Node::KVValueHashFeatureTypeWithChildHash` carrying it. The merk +//! verifier then checks `combine_hash(H(value), child_hash) == value_hash`, +//! which is exactly the composition `insert_subtree` commits for these types — +//! so forged element bytes no longer verify against a genuine root hash. +//! +//! This differs from [`super::v0`] (which leaves the node untouched) in that it +//! both reads storage and mutates the node. The extra reads and hash calls are +//! why it cannot apply to the released versions; see the module docs in +//! [`super`][`mod@super`]. + +use grovedb_costs::{ + cost_return_on_error, cost_return_on_error_no_add, CostResult, CostsExt, OperationCost, +}; +use grovedb_merk::{ + proofs::Node, + tree::{combine_hash, value_hash, NULL_HASH}, + CryptoHash, TreeFeatureType, +}; +use grovedb_storage::{Storage, StorageContext}; +use grovedb_version::version::GroveVersion; + +use crate::{Element, Error, GroveDb, Transaction}; + +impl GroveDb { + /// `bind_terminal_non_merk_tree` v1 — see the module documentation. + pub(crate) fn bind_terminal_non_merk_tree_v1( + &self, + node: &mut Node, + element: &Element, + parent_path: &[&[u8]], + tx: &Transaction, + _grove_version: &GroveVersion, + ) -> CostResult<(), Error> { + let mut cost = OperationCost::default(); + + // Read what we need out of the node before mutating it. The key also + // names the child subtree holding this tree's data. + let (key, value) = match &*node { + Node::KV(key, value) + | Node::KVValueHash(key, value, ..) + | Node::KVValueHashFeatureType(key, value, ..) + | Node::KVValueHashFeatureTypeWithChildHash(key, value, ..) => { + (key.clone(), value.clone()) + } + other => { + return Err(Error::CorruptedData(format!( + "bind_terminal_non_merk_tree called on a non-value-bearing proof node: {}", + other + ))) + .wrap_with_cost(cost); + } + }; + + let mut child_path: Vec<&[u8]> = parent_path.to_vec(); + child_path.push(key.as_slice()); + + let child_hash = cost_return_on_error!( + &mut cost, + self.non_merk_tree_child_hash(element, &child_path, tx) + ); + + let element_vh = value_hash(&value).unwrap_add_cost(&mut cost); + let recomputed = combine_hash(&element_vh, &child_hash).unwrap_add_cost(&mut cost); + + let (vh, ft) = match &*node { + Node::KVValueHashFeatureType(_, _, vh, ft) => (*vh, *ft), + Node::KVValueHash(_, _, vh) => (*vh, TreeFeatureType::BasicMerkNode), + _ => (recomputed, TreeFeatureType::BasicMerkNode), + }; + + // Self-check: if the recomputed state root does not reproduce the + // committed value_hash, the node we are about to emit would be rejected + // by the verifier. Fail here, where the cause is visible, rather than + // shipping a proof that cannot verify. + if recomputed != vh { + return Err(Error::CorruptedData(format!( + "non-Merk tree at key {} has state root {} which does not reproduce the \ + committed value hash {}", + hex::encode(&key), + hex::encode(child_hash), + hex::encode(vh), + ))) + .wrap_with_cost(cost); + } + + *node = Node::KVValueHashFeatureTypeWithChildHash(key, value, vh, ft, child_hash); + + Ok(()).wrap_with_cost(cost) + } + + /// Compute the child hash that a non-Merk tree element's parent Merk + /// commits to, i.e. the `child_hash` satisfying + /// `combine_hash(H(value), child_hash) == value_hash`. + /// + /// `CommitmentTree`, `MmrTree`, `BulkAppendTree` and + /// `DenseAppendOnlyFixedSizeTree` have no child Merk; their parent entry is + /// written by `insert_subtree` with the tree's own state root as the + /// supplied hash. This reproduces that hash. + /// + /// Each arm must mirror the corresponding write path exactly: + /// - `MmrTree` / `DenseAppendOnlyFixedSizeTree` / `BulkAppendTree` are + /// inserted with `NULL_HASH` while still empty, and only start committing + /// a computed root once the first append lands. Note that an empty + /// `BulkAppendTree`'s `compute_current_state_root()` is *not* `NULL_HASH`, + /// so the zero-count case has to short-circuit. + /// - `CommitmentTree` is inserted with `EMPTY_COMMITMENT_TREE_STATE_ROOT`, + /// which is exactly what the sinsemilla/bulk composition below yields at + /// count 0 — no special case needed. + fn non_merk_tree_child_hash( + &self, + element: &Element, + subtree_path: &[&[u8]], + tx: &Transaction, + ) -> CostResult { + let mut cost = OperationCost::default(); + + let path_vec: Vec> = subtree_path.iter().map(|s| s.to_vec()).collect(); + let path_refs: Vec<&[u8]> = path_vec.iter().map(|v| v.as_slice()).collect(); + let storage_path = grovedb_path::SubtreePath::from(path_refs.as_slice()); + + match element { + Element::MmrTree(mmr_size, _) => { + if *mmr_size == 0 { + return Ok(NULL_HASH).wrap_with_cost(cost); + } + let storage_ctx = self + .db + .get_transactional_storage_context(storage_path, None, tx) + .unwrap_add_cost(&mut cost); + let store = grovedb_merkle_mountain_range::MmrStore::new(&storage_ctx); + let mmr = grovedb_merkle_mountain_range::MMR::new(*mmr_size, &store); + let root = cost_return_on_error!( + &mut cost, + mmr.get_root() + .map_err(|e| Error::CorruptedData(format!("MMR get_root failed: {}", e))) + ); + Ok(root.hash()).wrap_with_cost(cost) + } + Element::DenseAppendOnlyFixedSizeTree(count, height, _) => { + if *count == 0 { + return Ok(NULL_HASH).wrap_with_cost(cost); + } + let storage_ctx = self + .db + .get_transactional_storage_context(storage_path, None, tx) + .unwrap_add_cost(&mut cost); + let tree = cost_return_on_error_no_add!( + cost, + grovedb_dense_fixed_sized_merkle_tree::DenseFixedSizedMerkleTree::from_state( + *height, + *count, + storage_ctx, + ) + .map_err(|e| Error::CorruptedData(format!("dense tree state error: {}", e))) + ); + let root_hash = cost_return_on_error!( + &mut cost, + tree.root_hash().map_err(|e| Error::CorruptedData(format!( + "dense tree root hash error: {}", + e + ))) + ); + Ok(root_hash).wrap_with_cost(cost) + } + Element::BulkAppendTree(total_count, chunk_power, _) => { + if *total_count == 0 { + return Ok(NULL_HASH).wrap_with_cost(cost); + } + let storage_ctx = self + .db + .get_transactional_storage_context(storage_path, None, tx) + .unwrap_add_cost(&mut cost); + let tree = cost_return_on_error_no_add!( + cost, + grovedb_bulk_append_tree::BulkAppendTree::from_state( + *total_count, + *chunk_power, + storage_ctx, + ) + .map_err(|e| Error::CorruptedData(format!( + "failed to create BulkAppendTree: {}", + e + ))) + ); + let state_root = cost_return_on_error_no_add!( + cost, + tree.compute_current_state_root().map_err(|e| { + Error::CorruptedData(format!("bulk append state root failed: {}", e)) + }) + ); + Ok(state_root).wrap_with_cost(cost) + } + Element::CommitmentTree(total_count, chunk_power, _) => { + let storage_ctx = self + .db + .get_transactional_storage_context(storage_path, None, tx) + .unwrap_add_cost(&mut cost); + + let sinsemilla_root = match storage_ctx + .get(grovedb_commitment_tree::COMMITMENT_TREE_DATA_KEY) + .value + { + Ok(Some(frontier_bytes)) => { + match grovedb_commitment_tree::CommitmentFrontier::deserialize( + frontier_bytes.as_ref(), + ) { + Ok(frontier) => frontier.root_hash(), + Err(_) => grovedb_commitment_tree::EMPTY_SINSEMILLA_ROOT, + } + } + _ => grovedb_commitment_tree::EMPTY_SINSEMILLA_ROOT, + }; + + let tree = cost_return_on_error_no_add!( + cost, + grovedb_bulk_append_tree::BulkAppendTree::from_state( + *total_count, + *chunk_power, + storage_ctx, + ) + .map_err(|e| Error::CorruptedData(format!( + "failed to create BulkAppendTree: {}", + e + ))) + ); + let bulk_state_root = cost_return_on_error_no_add!( + cost, + tree.compute_current_state_root().map_err(|e| { + Error::CorruptedData(format!("bulk append state root failed: {}", e)) + }) + ); + + Ok(grovedb_commitment_tree::compute_commitment_tree_state_root( + &sinsemilla_root, + &bulk_state_root, + )) + .wrap_with_cost(cost) + } + _ => Err(Error::CorruptedCodeExecution( + "non_merk_tree_child_hash called on an element that is not a non-Merk tree", + )) + .wrap_with_cost(cost), + } + } +} diff --git a/grovedb/src/operations/proof/generate.rs b/grovedb/src/operations/proof/generate.rs index 5fa92a40a..b095ee0f5 100644 --- a/grovedb/src/operations/proof/generate.rs +++ b/grovedb/src/operations/proof/generate.rs @@ -2354,13 +2354,13 @@ impl GroveDb { // node and let the merk verifier close the loop. // // Version-gated on - // `proof.terminal_non_merk_tree_child_hash`: - // deriving the state root costs storage reads and - // hash calls that V1..V3 did not pay, and cost - // feeds fees. Under those versions the node is - // left as the prover has always emitted it and - // only the limit moves — the released shape, which - // the matching verifier gate still accepts. + // `proof.terminal_non_merk_tree_child_hash`, so the + // binding itself lives in + // `bind_terminal_non_merk_tree`: deriving the state + // root costs storage reads and hash calls that + // V1..V3 did not pay, and cost feeds fees. Under + // those versions the node is left as the prover has + // always emitted it and only the limit moves. Ok(ref non_merk_elem @ Element::MmrTree(..)) | Ok(ref non_merk_elem @ Element::BulkAppendTree(..)) | Ok( @@ -2369,64 +2369,16 @@ impl GroveDb { | Ok(ref non_merk_elem @ Element::CommitmentTree(..)) if !done_with_results => { - if grove_version - .grovedb_versions - .operations - .proof - .terminal_non_merk_tree_child_hash - >= 1 - { - let mut child_path = path.clone(); - child_path.push(key.as_slice()); - - let child_hash = cost_return_on_error!( - &mut cost, - self.non_merk_tree_child_hash( - non_merk_elem, - &child_path, - &tx, - ) - ); - - let key_owned = key.to_owned(); - let value_owned = value.to_owned(); - let element_vh = - value_hash(&value_owned).unwrap_add_cost(&mut cost); - let recomputed = combine_hash(&element_vh, &child_hash) - .unwrap_add_cost(&mut cost); - let (vh, ft) = match node { - Node::KVValueHashFeatureType(_, _, vh, ft) => (*vh, *ft), - Node::KVValueHash(_, _, vh) => { - (*vh, TreeFeatureType::BasicMerkNode) - } - _ => (recomputed, TreeFeatureType::BasicMerkNode), - }; - - // Self-check: if the recomputed state root - // does not reproduce the committed - // value_hash, the node we are about to emit - // would be rejected by the verifier. Fail - // here, where the cause is visible, rather - // than shipping a proof that cannot verify. - if recomputed != vh { - return Err(Error::CorruptedData(format!( - "non-Merk tree at key {} has state root {} which \ - does not reproduce the committed value hash {}", - hex::encode(&key_owned), - hex::encode(child_hash), - hex::encode(vh), - ))) - .wrap_with_cost(cost); - } - - *node = Node::KVValueHashFeatureTypeWithChildHash( - key_owned, - value_owned, - vh, - ft, - child_hash, - ); - } + cost_return_on_error!( + &mut cost, + self.bind_terminal_non_merk_tree( + node, + non_merk_elem, + &path, + &tx, + grove_version, + ) + ); if let Some(limit) = overall_limit.as_mut() { *limit -= 1; @@ -2750,161 +2702,6 @@ impl GroveDb { .wrap_with_cost(cost) } - /// Compute the child hash that a non-Merk tree element's parent Merk - /// commits to, i.e. the `child_hash` satisfying - /// `combine_hash(H(value), child_hash) == value_hash`. - /// - /// `CommitmentTree`, `MmrTree`, `BulkAppendTree` and - /// `DenseAppendOnlyFixedSizeTree` have no child Merk; their parent entry is - /// written by `insert_subtree` with the tree's own state root as the - /// supplied hash. This reproduces that hash so a terminal proof of the tree - /// element itself can carry it and stay bound to the element bytes. - /// - /// Each arm must mirror the corresponding write path exactly: - /// - `MmrTree` / `DenseAppendOnlyFixedSizeTree` / `BulkAppendTree` are - /// inserted with `NULL_HASH` while still empty, and only start committing - /// a computed root once the first append lands. Note that an empty - /// `BulkAppendTree`'s `compute_current_state_root()` is *not* `NULL_HASH`, - /// so the zero-count case has to short-circuit. - /// - `CommitmentTree` is inserted with `EMPTY_COMMITMENT_TREE_STATE_ROOT`, - /// which is exactly what the sinsemilla/bulk composition below yields at - /// count 0 — no special case needed. - fn non_merk_tree_child_hash( - &self, - element: &Element, - subtree_path: &[&[u8]], - tx: &Transaction, - ) -> CostResult { - use grovedb_merk::tree::NULL_HASH; - - let mut cost = OperationCost::default(); - - let path_vec: Vec> = subtree_path.iter().map(|s| s.to_vec()).collect(); - let path_refs: Vec<&[u8]> = path_vec.iter().map(|v| v.as_slice()).collect(); - let storage_path = grovedb_path::SubtreePath::from(path_refs.as_slice()); - - match element { - Element::MmrTree(mmr_size, _) => { - if *mmr_size == 0 { - return Ok(NULL_HASH).wrap_with_cost(cost); - } - let storage_ctx = self - .db - .get_transactional_storage_context(storage_path, None, tx) - .unwrap_add_cost(&mut cost); - let store = grovedb_merkle_mountain_range::MmrStore::new(&storage_ctx); - let mmr = grovedb_merkle_mountain_range::MMR::new(*mmr_size, &store); - let root = cost_return_on_error!( - &mut cost, - mmr.get_root() - .map_err(|e| Error::CorruptedData(format!("MMR get_root failed: {}", e))) - ); - Ok(root.hash()).wrap_with_cost(cost) - } - Element::DenseAppendOnlyFixedSizeTree(count, height, _) => { - if *count == 0 { - return Ok(NULL_HASH).wrap_with_cost(cost); - } - let storage_ctx = self - .db - .get_transactional_storage_context(storage_path, None, tx) - .unwrap_add_cost(&mut cost); - let tree = cost_return_on_error_no_add!( - cost, - grovedb_dense_fixed_sized_merkle_tree::DenseFixedSizedMerkleTree::from_state( - *height, - *count, - storage_ctx, - ) - .map_err(|e| Error::CorruptedData(format!("dense tree state error: {}", e))) - ); - let root_hash = cost_return_on_error!( - &mut cost, - tree.root_hash().map_err(|e| Error::CorruptedData(format!( - "dense tree root hash error: {}", - e - ))) - ); - Ok(root_hash).wrap_with_cost(cost) - } - Element::BulkAppendTree(total_count, chunk_power, _) => { - if *total_count == 0 { - return Ok(NULL_HASH).wrap_with_cost(cost); - } - let storage_ctx = self - .db - .get_transactional_storage_context(storage_path, None, tx) - .unwrap_add_cost(&mut cost); - let tree = cost_return_on_error_no_add!( - cost, - grovedb_bulk_append_tree::BulkAppendTree::from_state( - *total_count, - *chunk_power, - storage_ctx, - ) - .map_err(|e| Error::CorruptedData(format!( - "failed to create BulkAppendTree: {}", - e - ))) - ); - let state_root = cost_return_on_error_no_add!( - cost, - tree.compute_current_state_root().map_err(|e| { - Error::CorruptedData(format!("bulk append state root failed: {}", e)) - }) - ); - Ok(state_root).wrap_with_cost(cost) - } - Element::CommitmentTree(total_count, chunk_power, _) => { - let storage_ctx = self - .db - .get_transactional_storage_context(storage_path, None, tx) - .unwrap_add_cost(&mut cost); - - let sinsemilla_root = match storage_ctx.get(COMMITMENT_TREE_DATA_KEY).value { - Ok(Some(frontier_bytes)) => { - match grovedb_commitment_tree::CommitmentFrontier::deserialize( - frontier_bytes.as_ref(), - ) { - Ok(frontier) => frontier.root_hash(), - Err(_) => grovedb_commitment_tree::EMPTY_SINSEMILLA_ROOT, - } - } - _ => grovedb_commitment_tree::EMPTY_SINSEMILLA_ROOT, - }; - - let tree = cost_return_on_error_no_add!( - cost, - grovedb_bulk_append_tree::BulkAppendTree::from_state( - *total_count, - *chunk_power, - storage_ctx, - ) - .map_err(|e| Error::CorruptedData(format!( - "failed to create BulkAppendTree: {}", - e - ))) - ); - let bulk_state_root = cost_return_on_error_no_add!( - cost, - tree.compute_current_state_root().map_err(|e| { - Error::CorruptedData(format!("bulk append state root failed: {}", e)) - }) - ); - - Ok(grovedb_commitment_tree::compute_commitment_tree_state_root( - &sinsemilla_root, - &bulk_state_root, - )) - .wrap_with_cost(cost) - } - _ => Err(Error::CorruptedCodeExecution( - "non_merk_tree_child_hash called on an element that is not a non-Merk tree", - )) - .wrap_with_cost(cost), - } - } - /// Generate an MMR tree layer proof for a subquery. fn generate_mmr_layer_proof( &self, diff --git a/grovedb/src/operations/proof/mod.rs b/grovedb/src/operations/proof/mod.rs index ea5381d5e..65e708338 100644 --- a/grovedb/src/operations/proof/mod.rs +++ b/grovedb/src/operations/proof/mod.rs @@ -8,6 +8,11 @@ mod aggregate_count; mod aggregate_count_and_sum; #[cfg(any(feature = "minimal", feature = "verify"))] mod aggregate_sum; +/// Versioned dispatch for `bind_terminal_non_merk_tree`, which binds a +/// terminally-reported non-Merk tree's element bytes to the parent-committed +/// `value_hash`. Consensus-critical — see the module docs. +#[cfg(feature = "minimal")] +mod bind_terminal_non_merk_tree; #[cfg(feature = "minimal")] mod generate; // The prover lives in `indexed_axis::generate` and is `minimal`-gated there; From a9cd466286965156e14a4c00b0872119d551342c Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 3 Aug 2026 01:45:11 +0700 Subject: [PATCH 6/6] perf(grovedb): drop the unused version arg and stop hashing on the bind hot path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cleanups to `bind_terminal_non_merk_tree`, both on a path that runs per terminal non-Merk tree while serving a latency-sensitive proof. `grove_version` was threaded into v0 and v1 and used by neither — the version is consumed by the dispatch in `mod.rs`, which is the point of the split. Dropped from both implementations and from the call sites. The self-check then went from an unconditional runtime error to a `debug_assert`. It was costing two blake3 hashes on every terminal non-Merk tree: `H(value)` plus a `combine_hash`, purely to re-derive a value_hash the node already carries. That is small next to the storage reads `non_merk_tree_child_hash` does in the same function, but it bought nothing in production — it can fire only on a prover bug or corrupted storage, never on attacker input, and every arm of the derivation is pinned by tests across all four types, empty and populated. If it ever did fire in release the verifier would reject the proof anyway; the check only made the diagnosis nicer. The common path now does no hashing at all: the `value_hash` the node carries is the one the parent committed, so it is reused as-is. Deriving it is confined to node shapes that carry none (`KV` / `KVCount` / `KVSum` / `KVCountSum`), which trees are not proved with in practice. The debug block is deliberately uncosted (`.unwrap()`, not `unwrap_add_cost`) so `OperationCost` stays identical between debug and release builds — a cost that varied by build profile would be far worse than the two hashes. Net effect on tracked cost: two fewer `hash_node_calls` per terminal non-Merk tree than the previous commit charged. That only moves V4 numbers, which are unreleased, so nothing shifts for V1..V3. Co-Authored-By: Claude Opus 5 --- .../proof/bind_terminal_non_merk_tree/mod.rs | 4 +- .../proof/bind_terminal_non_merk_tree/v0.rs | 2 - .../proof/bind_terminal_non_merk_tree/v1.rs | 54 +++++++++++++------ 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/grovedb/src/operations/proof/bind_terminal_non_merk_tree/mod.rs b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/mod.rs index 47bf72ed8..33e584c02 100644 --- a/grovedb/src/operations/proof/bind_terminal_non_merk_tree/mod.rs +++ b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/mod.rs @@ -69,8 +69,8 @@ impl GroveDb { .proof .terminal_non_merk_tree_child_hash { - 0 => self.bind_terminal_non_merk_tree_v0(node, element, parent_path, tx, grove_version), - 1 => self.bind_terminal_non_merk_tree_v1(node, element, parent_path, tx, grove_version), + 0 => self.bind_terminal_non_merk_tree_v0(node, element, parent_path, tx), + 1 => self.bind_terminal_non_merk_tree_v1(node, element, parent_path, tx), version => Err(Error::VersionError( grovedb_version::error::GroveVersionError::UnknownVersionMismatch { method: "bind_terminal_non_merk_tree".to_string(), diff --git a/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v0.rs b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v0.rs index 19d4fcff4..7463f7b26 100644 --- a/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v0.rs +++ b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v0.rs @@ -18,7 +18,6 @@ use grovedb_costs::{CostResult, CostsExt, OperationCost}; use grovedb_merk::proofs::Node; -use grovedb_version::version::GroveVersion; use crate::{Element, Error, GroveDb, Transaction}; @@ -30,7 +29,6 @@ impl GroveDb { _element: &Element, _parent_path: &[&[u8]], _tx: &Transaction, - _grove_version: &GroveVersion, ) -> CostResult<(), Error> { Ok(()).wrap_with_cost(OperationCost::default()) } diff --git a/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v1.rs b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v1.rs index f92384e65..664ccddc8 100644 --- a/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v1.rs +++ b/grovedb/src/operations/proof/bind_terminal_non_merk_tree/v1.rs @@ -7,9 +7,16 @@ //! so forged element bytes no longer verify against a genuine root hash. //! //! This differs from [`super::v0`] (which leaves the node untouched) in that it -//! both reads storage and mutates the node. The extra reads and hash calls are -//! why it cannot apply to the released versions; see the module docs in +//! both reads storage and mutates the node. Those extra reads are why it cannot +//! apply to the released versions; see the module docs in //! [`super`][`mod@super`]. +//! +//! Proof serving is latency-sensitive, so the common path does no hashing at +//! all: the `value_hash` the node already carries is the one the parent +//! committed, and is reused as-is. Only a node shape that carries no +//! `value_hash` has to derive one. The correctness of the derived state root is +//! checked by a `debug_assert` rather than at runtime — it can fire only on a +//! prover bug or corrupted storage, and the derivation is pinned by tests. use grovedb_costs::{ cost_return_on_error, cost_return_on_error_no_add, CostResult, CostsExt, OperationCost, @@ -20,7 +27,6 @@ use grovedb_merk::{ CryptoHash, TreeFeatureType, }; use grovedb_storage::{Storage, StorageContext}; -use grovedb_version::version::GroveVersion; use crate::{Element, Error, GroveDb, Transaction}; @@ -32,7 +38,6 @@ impl GroveDb { element: &Element, parent_path: &[&[u8]], tx: &Transaction, - _grove_version: &GroveVersion, ) -> CostResult<(), Error> { let mut cost = OperationCost::default(); @@ -62,28 +67,45 @@ impl GroveDb { self.non_merk_tree_child_hash(element, &child_path, tx) ); - let element_vh = value_hash(&value).unwrap_add_cost(&mut cost); - let recomputed = combine_hash(&element_vh, &child_hash).unwrap_add_cost(&mut cost); - + // Reuse the value_hash the node already carries — it is the one the + // parent committed, so there is nothing to recompute. Proof serving is + // latency-sensitive and this runs per terminal non-Merk tree, so the + // common path must not hash. let (vh, ft) = match &*node { Node::KVValueHashFeatureType(_, _, vh, ft) => (*vh, *ft), Node::KVValueHash(_, _, vh) => (*vh, TreeFeatureType::BasicMerkNode), - _ => (recomputed, TreeFeatureType::BasicMerkNode), + // A node shape that carries no value_hash to reuse (`KV`, + // `KVCount`, `KVSum`, `KVCountSum`). Only here do we have to + // derive it. Trees are proved with a value_hash-bearing node in + // practice, so this is the cold path. + _ => { + let element_vh = value_hash(&value).unwrap_add_cost(&mut cost); + let derived = combine_hash(&element_vh, &child_hash).unwrap_add_cost(&mut cost); + (derived, TreeFeatureType::BasicMerkNode) + } }; - // Self-check: if the recomputed state root does not reproduce the - // committed value_hash, the node we are about to emit would be rejected - // by the verifier. Fail here, where the cause is visible, rather than - // shipping a proof that cannot verify. - if recomputed != vh { - return Err(Error::CorruptedData(format!( + // Drift check: the derived state root must reproduce the committed + // value_hash, or the node we are about to emit would be rejected by the + // verifier. Debug-only and deliberately uncosted — it can fire only on + // a prover bug or corrupted storage, never on attacker input, and the + // arms of `non_merk_tree_child_hash` are pinned by tests across all + // four types, empty and populated. Keeping it out of release spares the + // hot path two hashes; keeping it uncosted keeps `OperationCost` + // identical between debug and release builds. + #[cfg(debug_assertions)] + { + let element_vh = value_hash(&value).unwrap(); + let recomputed = combine_hash(&element_vh, &child_hash).unwrap(); + debug_assert_eq!( + recomputed, + vh, "non-Merk tree at key {} has state root {} which does not reproduce the \ committed value hash {}", hex::encode(&key), hex::encode(child_hash), hex::encode(vh), - ))) - .wrap_with_cost(cost); + ); } *node = Node::KVValueHashFeatureTypeWithChildHash(key, value, vh, ft, child_hash);