From 94bf370d8aaffcc6f6889aa85094412eee98368e Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 4 Jun 2026 09:24:11 +0200 Subject: [PATCH 01/11] fix(drive): authenticate boundary in compacted absence proofs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compacted nullifier-changes and address-balance-changes proof verifiers derived the forward-scan lower bound (`start_key`) from raw, un-authenticated proof bytes. Because compacted keys are `(start_block_be, end_block_be)`, a range like `(100, 200)` that contains a requested height `150` sorts lexicographically before `(150, 150)`. A malicious node could therefore prove `range_from((150, 150)..)` directly — a valid GroveDB proof in which the containing range appears only as a hash-only boundary node — and the verifier would accept the incomplete proof and silently return zero results, concealing a spent nullifier (double-spend) or a balance change from trust-minimized light clients. Fix: never trust the proof bytes to self-report the boundary. Use GroveDB's `verify_query_with_chained_path_queries`: 1. A boundary query (descending `range_to_inclusive(..=(start, MAX))`, limit 1) authenticates the single greatest compacted key `<= (start, MAX)` against the real root hash. A malicious prover cannot substitute or omit it without breaking the root. 2. A generator derives the forward `start_key` from that authenticated boundary (the containing range iff its `end_block >= start`). 3. The chained forward query is verified against the same root. The provers now emit a single merged proof (`prove_query_many`) covering both the boundary key and the forward range. Both parallel instances are fixed (shielded nullifiers and address-fund balances). Adds PoC tests proving the malicious skip-descending proof is now rejected and that honest containing-range/roundtrip proofs still verify. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../v0/mod.rs | 129 ++++-- .../v0/mod.rs | 140 ++++-- .../v0/mod.rs | 409 ++++++++++-------- .../v0/mod.rs | 390 +++++++++++------ 4 files changed, 645 insertions(+), 423 deletions(-) diff --git a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs index df29ec6f9f2..c2ab4e24e0d 100644 --- a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs +++ b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs @@ -5,6 +5,7 @@ use dpp::balances::credits::BlockAwareCreditOperation; use dpp::ProtocolError; use grovedb::query_result_type::QueryResultType; use grovedb::{Element, PathQuery, Query, SizedQuery, TransactionArg}; +use grovedb_costs::CostContext; use platform_version::version::PlatformVersion; use std::collections::BTreeMap; @@ -189,11 +190,22 @@ impl Drive { /// Version 0 implementation for proving compacted address balance changes. /// - /// Uses a two-step approach: - /// 1. First query (non-proving): descending to find any range containing start_block_height - /// 2. Second query (proving): ascending from the found start_block or start_block_height + /// The proof must let the verifier authenticate **two** things against the + /// same root hash (see `verify_compacted_address_balance_changes_v0`): /// - /// This ensures the proof covers all relevant ranges efficiently. + /// 1. A **boundary query** — the single greatest compacted key + /// `<= (start_block_height, u64::MAX)`. This is what protects against the + /// compacted-range absence-proof attack: a range like `(100, 200)` that + /// contains the requested height sorts *before* `(150, 150)`, so the + /// verifier cannot trust a forward-only proof to surface it. + /// 2. A **forward query** — `range_from(start_key..)` where `start_key` is + /// the containing range (if any) or `(start_block_height, + /// start_block_height)`. + /// + /// We discover `start_key` with a non-proving descending query, then emit a + /// single merged proof (`prove_query_many`) that covers BOTH the boundary + /// key and the forward range so the verifier's chained queries are both + /// satisfiable. pub(super) fn prove_compacted_address_balance_changes_v0( &self, start_block_height: u64, @@ -203,10 +215,9 @@ impl Drive { ) -> Result, Error> { let path = Self::saved_compacted_block_transactions_address_balances_path_vec(); - // Step 1: Non-proving descending query to find any range containing start_block_height - let mut desc_end_key = Vec::with_capacity(16); - desc_end_key.extend_from_slice(&start_block_height.to_be_bytes()); - desc_end_key.extend_from_slice(&u64::MAX.to_be_bytes()); + // Step 1: Non-proving descending query to find the greatest compacted + // key <= (start_block_height, u64::MAX). + let desc_end_key = compacted_key(start_block_height, u64::MAX); let mut desc_query = Query::new_with_direction(false); // descending desc_query.insert_range_to_inclusive(..=desc_end_key); @@ -222,51 +233,79 @@ impl Drive { &platform_version.drive, )?; - // Determine the actual start key for the proved query - // If we found a containing range, use its exact key - // Otherwise use (start_block_height, start_block_height) since end_block >= start_block always - let start_key = if let Some((key, _)) = desc_results.to_key_elements().into_iter().next() { - if key.len() == 16 { + // `boundary_key` is the authenticated lower-bound anchor the verifier + // will re-derive from its boundary query. `forward_start` is the lower + // bound of the ascending result scan. + let (boundary_key, forward_start) = match desc_results.to_key_elements().into_iter().next() + { + Some((key, _)) => { + if key.len() != 16 { + return Err(Error::Protocol(Box::new( + ProtocolError::CorruptedSerialization( + "invalid compacted block key length, expected 16 bytes".to_string(), + ), + ))); + } let end_block = u64::from_be_bytes(key[8..16].try_into().unwrap()); - // If this range contains start_block_height, use its exact key - if end_block >= start_block_height { - key + let forward_start = if end_block >= start_block_height { + key.clone() } else { - // No containing range, use (start_block_height, start_block_height) - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key - } - } else { - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key + compacted_key(start_block_height, start_block_height) + }; + (Some(key), forward_start) } - } else { - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key + None => (None, compacted_key(start_block_height, start_block_height)), }; - // Step 2: Proved ascending query from start_key - - let mut query = Query::new(); - query.insert_range_from(start_key..); - - let path_query = PathQuery::new(path, SizedQuery::new(query, limit, None)); - - self.grove_get_proved_path_query( - &path_query, - transaction, - &mut vec![], - &platform_version.drive, - ) + // Step 2: build the single merged proof. See the nullifier prover and + // the verifier docs for the soundness rationale. `PathQuery::merge` + // rejects per-query limits, so the merged queries carry no limits; the + // forward query's caller limit is applied to the verifier's forward + // subset query instead (subset verification accepts a superset proof). + match boundary_key { + Some(boundary_key) => { + let mut boundary_point = Query::new(); + boundary_point.insert_key(boundary_key); + let boundary_point_query = + PathQuery::new(path.clone(), SizedQuery::new(boundary_point, None, None)); + + let mut forward_query = Query::new(); + forward_query.insert_range_from(forward_start..); + let forward_path_query = + PathQuery::new(path.clone(), SizedQuery::new(forward_query, None, None)); + + let CostContext { value, .. } = self.grove.prove_query_many( + vec![&boundary_point_query, &forward_path_query], + None, + &platform_version.drive.grove_version, + ); + value.map_err(Error::from) + } + None => { + let mut forward_query = Query::new(); + forward_query.insert_range_from(forward_start..); + let forward_path_query = + PathQuery::new(path.clone(), SizedQuery::new(forward_query, limit, None)); + + self.grove_get_proved_path_query( + &forward_path_query, + transaction, + &mut vec![], + &platform_version.drive, + ) + } + } } } +/// Builds the 16-byte big-endian compacted key `(start_block, end_block)`. +fn compacted_key(start_block: u64, end_block: u64) -> Vec { + let mut key = Vec::with_capacity(16); + key.extend_from_slice(&start_block.to_be_bytes()); + key.extend_from_slice(&end_block.to_be_bytes()); + key +} + #[cfg(test)] mod tests { use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; diff --git a/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs index 5b3fd7de66f..c2faf512cfb 100644 --- a/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs @@ -4,16 +4,36 @@ use crate::error::Error; use dpp::ProtocolError; use grovedb::query_result_type::QueryResultType; use grovedb::{PathQuery, Query, SizedQuery, TransactionArg}; +use grovedb_costs::CostContext; use platform_version::version::PlatformVersion; +/// Builds the 16-byte big-endian compacted key `(start_block, end_block)`. +fn compacted_key(start_block: u64, end_block: u64) -> Vec { + let mut key = Vec::with_capacity(16); + key.extend_from_slice(&start_block.to_be_bytes()); + key.extend_from_slice(&end_block.to_be_bytes()); + key +} + impl Drive { /// Version 0 implementation for proving compacted nullifier changes. /// - /// Uses a two-step approach: - /// 1. First query (non-proving): descending to find any range containing start_block_height - /// 2. Second query (proving): ascending from the found start_block or start_block_height + /// The proof must let the verifier authenticate **two** things against the + /// same root hash (see `verify_compacted_nullifier_changes_v0`): + /// + /// 1. A **boundary query** — the single greatest compacted key + /// `<= (start_block_height, u64::MAX)`. This is what protects against the + /// compacted-range absence-proof attack: a range like `(100, 200)` that + /// contains the requested height sorts *before* `(150, 150)`, so the + /// verifier cannot trust a forward-only proof to surface it. + /// 2. A **forward query** — `range_from(start_key..)` where `start_key` is + /// the containing range (if any) or `(start_block_height, + /// start_block_height)`. /// - /// This ensures the proof covers all relevant ranges efficiently. + /// We discover `start_key` with a non-proving descending query, then emit a + /// single merged proof (`prove_query_many`) that covers BOTH the boundary + /// key and the forward range so the verifier's chained queries are both + /// satisfiable. pub(super) fn prove_compacted_nullifier_changes_v0( &self, start_block_height: u64, @@ -23,10 +43,9 @@ impl Drive { ) -> Result, Error> { let path = shielded_compacted_nullifiers_path_vec(); - // Step 1: Non-proving descending query to find any range containing start_block_height - let mut desc_end_key = Vec::with_capacity(16); - desc_end_key.extend_from_slice(&start_block_height.to_be_bytes()); - desc_end_key.extend_from_slice(&u64::MAX.to_be_bytes()); + // Step 1: Non-proving descending query to find the greatest compacted + // key <= (start_block_height, u64::MAX). + let desc_end_key = compacted_key(start_block_height, u64::MAX); let mut desc_query = Query::new_with_direction(false); // descending desc_query.insert_range_to_inclusive(..=desc_end_key); @@ -42,52 +61,87 @@ impl Drive { &platform_version.drive, )?; - // Determine the actual start key for the proved query - // If we found a containing range, use its exact key - // Otherwise use (start_block_height, start_block_height) since end_block >= start_block always - let start_key = if let Some((key, _)) = desc_results.to_key_elements().into_iter().next() { - if key.len() == 16 { + // `boundary_key` is the authenticated lower-bound anchor the verifier + // will re-derive from its boundary query. `forward_start` is the lower + // bound of the ascending result scan. + let (boundary_key, forward_start) = match desc_results.to_key_elements().into_iter().next() + { + Some((key, _)) => { + if key.len() != 16 { + return Err(Error::Protocol(Box::new( + ProtocolError::CorruptedSerialization( + "invalid compacted block key length, expected 16 bytes".to_string(), + ), + ))); + } let end_block = u64::from_be_bytes(key[8..16].try_into().map_err(|_| { Error::Protocol(Box::new(ProtocolError::CorruptedSerialization( "invalid compacted key slice".to_string(), ))) })?); - // If this range contains start_block_height, use its exact key - if end_block >= start_block_height { - key + // If this range contains start_block_height, the forward scan + // must begin exactly at it; otherwise it begins at + // (start_block_height, start_block_height). + let forward_start = if end_block >= start_block_height { + key.clone() } else { - // No containing range, use (start_block_height, start_block_height) - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key - } - } else { - return Err(Error::Protocol(Box::new( - ProtocolError::CorruptedSerialization( - "invalid compacted block key length, expected 16 bytes".to_string(), - ), - ))); + compacted_key(start_block_height, start_block_height) + }; + (Some(key), forward_start) } - } else { - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key + None => (None, compacted_key(start_block_height, start_block_height)), }; - // Step 2: Proved ascending query from start_key + // Step 2: build the single merged proof. + // + // The verifier's chained boundary query is a descending + // `range_to_inclusive(..=(start, MAX))` limit-1 subset query. For it to + // authenticate `boundary_key` as the true greatest key, the proof must + // include `boundary_key` as a full node together with the contiguity + // information around it. We therefore include the boundary key as an + // explicit point query merged with the forward range. + // + // `PathQuery::merge` rejects per-query limits, so the merged queries + // carry no limits; the forward query's caller limit is applied to the + // verifier's forward subset query instead (the proof is a superset, + // which subset verification accepts). + match boundary_key { + Some(boundary_key) => { + let mut boundary_point = Query::new(); + boundary_point.insert_key(boundary_key); + let boundary_point_query = + PathQuery::new(path.clone(), SizedQuery::new(boundary_point, None, None)); - let mut query = Query::new(); - query.insert_range_from(start_key..); + let mut forward_query = Query::new(); + forward_query.insert_range_from(forward_start..); + let forward_path_query = + PathQuery::new(path.clone(), SizedQuery::new(forward_query, None, None)); - let path_query = PathQuery::new(path, SizedQuery::new(query, limit, None)); + let CostContext { value, .. } = self.grove.prove_query_many( + vec![&boundary_point_query, &forward_path_query], + None, + &platform_version.drive.grove_version, + ); + value.map_err(Error::from) + } + None => { + // No compacted key <= (start, MAX) exists at all (empty / + // absence). The forward query alone is a sound absence proof: + // the verifier's boundary query will authenticate that nothing + // exists <= (start, MAX), and the forward query authenticates + // the (possibly empty) ascending results. + let mut forward_query = Query::new(); + forward_query.insert_range_from(forward_start..); + let forward_path_query = + PathQuery::new(path.clone(), SizedQuery::new(forward_query, limit, None)); - self.grove_get_proved_path_query( - &path_query, - transaction, - &mut vec![], - &platform_version.drive, - ) + self.grove_get_proved_path_query( + &forward_path_query, + transaction, + &mut vec![], + &platform_version.drive, + ) + } + } } } diff --git a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs index bd34a9d70e5..eb05d812324 100644 --- a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs @@ -8,51 +8,59 @@ use dpp::address_funds::PlatformAddress; /// The subtree key for compacted address balances storage as u8 const COMPACTED_ADDRESS_BALANCES_KEY_U8: u8 = b'c'; use dpp::balances::credits::BlockAwareCreditOperation; -use grovedb::operations::proof::{GroveDBProof, ProofBytes}; -use grovedb::{ - GroveDb, MerkProofDecoder, MerkProofNode, MerkProofOp, PathQuery, Query, SizedQuery, -}; +use grovedb::query_result_type::PathKeyOptionalElementTrio; +use grovedb::{GroveDb, PathQuery, Query, SizedQuery}; use platform_version::version::PlatformVersion; use std::collections::BTreeMap; use super::VerifiedCompactedAddressBalanceChanges; -/// Extract KV entries from merk proof bytes using the proper decoder. -#[allow(clippy::type_complexity)] -fn extract_kv_entries_from_merk_proof(merk_proof: &[u8]) -> Result, Vec)>, Error> { - let mut entries = Vec::new(); - - let decoder = MerkProofDecoder::new(merk_proof); - - for op in decoder { - match op { - Ok(MerkProofOp::Push(MerkProofNode::KV(key, value))) - | Ok(MerkProofOp::PushInverted(MerkProofNode::KV(key, value))) => { - entries.push((key, value)); - } - Err(e) => { - tracing::error!(%e, "merk proof decode error"); - return Err(Error::Proof(ProofError::CorruptedProof(format!( - "failed to decode merk proof op: {}", - e - )))); - } - _ => {} - } - } +/// Builds the 16-byte big-endian compacted key `(start_block, end_block)`. +fn compacted_key(start_block: u64, end_block: u64) -> Vec { + let mut key = Vec::with_capacity(16); + key.extend_from_slice(&start_block.to_be_bytes()); + key.extend_from_slice(&end_block.to_be_bytes()); + key +} - Ok(entries) +/// Path to the compacted address balances subtree: +/// `[SavedBlockTransactions, COMPACTED_ADDRESS_BALANCES_KEY]`. +fn compacted_address_balances_path() -> Vec> { + vec![ + vec![RootTree::SavedBlockTransactions as u8], + vec![COMPACTED_ADDRESS_BALANCES_KEY_U8], + ] } impl Drive { /// Verifies compacted address balance changes proof. /// - /// This verification works by: - /// 1. Decoding the GroveDBProof structure - /// 2. Navigating to the compacted address balances layer ('c') - /// 3. Extracting KV entries from the merk proof - /// 4. Filtering entries where the key range contains start_block_height - /// 5. Verifying the root hash using a subset query + /// # Soundness + /// + /// Compacted keys are 16 bytes `(start_block_be, end_block_be)`. A range + /// such as `(100, 200)` that *contains* a requested height `150` sorts + /// lexicographically **before** `(150, 150)`. This means the lower bound of + /// the forward range scan (`start_key`) cannot be trusted to come from the + /// caller-requested height alone — a malicious prover could prove + /// `range_from((150, 150)..)` directly and the containing range `(100, 200)` + /// would appear only as a hash-only boundary node, silently hiding a change. + /// + /// To close this hole we never derive `start_key` from un-authenticated + /// proof bytes. Instead we use GroveDB's chained path query verification: + /// + /// 1. A **boundary query** (descending `range_to_inclusive(..=(start, MAX))`, + /// limit 1) authenticates, against the real root hash, the single + /// greatest compacted key `<= (start_block_height, u64::MAX)`. A malicious + /// prover cannot substitute or omit this key without breaking the root + /// hash. + /// 2. A **generator** inspects that authenticated boundary key. If its + /// `end_block >= start_block_height` the range contains (or starts at) the + /// requested height, so we use that exact key as the forward `start_key`. + /// Otherwise no containing range exists and we fall back to + /// `(start_block_height, start_block_height)`. + /// 3. The chained **forward query** (`range_from(start_key..)`, caller limit) + /// is verified against the same root hash, and its authenticated results + /// are decoded into the returned changes. pub(super) fn verify_compacted_address_balance_changes_v0( proof: &[u8], start_block_height: u64, @@ -63,103 +71,66 @@ impl Drive { .with_big_endian() .with_no_limit(); - // Decode the GroveDBProof to navigate its structure - let grovedb_proof: GroveDBProof = bincode::decode_from_slice(proof, bincode_config) - .map(|(p, _)| p) - .map_err(|e| { - Error::Proof(ProofError::CorruptedProof(format!( - "cannot decode GroveDBProof: {}", - e - ))) - })?; - - // Navigate to the compacted address balances layer - // Path: SavedBlockTransactions ('$' = 0x24) -> CompactedAddressBalances ('c' = 0x63) - let saved_block_key = vec![RootTree::SavedBlockTransactions as u8]; - let compacted_key = vec![COMPACTED_ADDRESS_BALANCES_KEY_U8]; - - // Extract KV entries from the compacted layer's merk proof to find - // if there's a containing range for start_block_height. - // V0 and V1 proofs have different layer types (MerkOnlyLayerProof vs LayerProof), - // so we handle them separately. - let kv_entries = match &grovedb_proof { - GroveDBProof::V0(v0) => { - let compacted_layer = v0 - .root_layer - .lower_layers - .get(&saved_block_key) - .and_then(|layer| layer.lower_layers.get(&compacted_key)); - compacted_layer - .map(|layer| extract_kv_entries_from_merk_proof(&layer.merk_proof)) - .transpose()? - .unwrap_or_default() - } - GroveDBProof::V1(v1) => { - let compacted_layer = v1 - .root_layer - .lower_layers - .get(&saved_block_key) - .and_then(|layer| layer.lower_layers.get(&compacted_key)); - compacted_layer - .map(|layer| match &layer.merk_proof { - ProofBytes::Merk(bytes) => extract_kv_entries_from_merk_proof(bytes), - other => Err(Error::Proof(ProofError::CorruptedProof(format!( - "unsupported V1 proof bytes variant for compacted address balances: {:?}", - std::mem::discriminant(other) - )))), + let path = compacted_address_balances_path(); + + // Step 1: boundary query — authenticate the single greatest compacted + // key <= (start_block_height, u64::MAX). Descending, limit 1. + let boundary_end_key = compacted_key(start_block_height, u64::MAX); + let mut boundary_inner = Query::new_with_direction(false); // descending + boundary_inner.insert_range_to_inclusive(..=boundary_end_key); + let boundary_query = + PathQuery::new(path.clone(), SizedQuery::new(boundary_inner, Some(1), None)); + + // Step 2: generator — derive the forward query's lower bound from the + // AUTHENTICATED boundary result (not from raw proof bytes). + let forward_path = path.clone(); + let generator = + move |boundary_results: Vec| -> Option { + let start_key = boundary_results + .iter() + .find_map(|(_path, key, _element)| { + if key.len() != 16 { + return None; + } + let end_block = u64::from_be_bytes( + key[8..16].try_into().expect("len checked to be 16"), + ); + if end_block >= start_block_height { + Some(key.clone()) + } else { + None + } }) - .transpose()? - .unwrap_or_default() - } - }; - - // Look for a KV entry where the range contains start_block_height - // Keys are 16 bytes: (start_block, end_block), both big-endian - let containing_key = kv_entries.iter().find_map(|(key, _)| { - if key.len() != 16 { - return None; - } - let range_start = u64::from_be_bytes(key[0..8].try_into().unwrap()); - let range_end = u64::from_be_bytes(key[8..16].try_into().unwrap()); - - // Check if this range contains start_block_height - if range_start <= start_block_height && start_block_height <= range_end { - Some(key.clone()) - } else { - None - } - }); - - // Determine the start_key for the query - // Use the containing range's key if found, otherwise (start_block_height, start_block_height) - let start_key = containing_key.unwrap_or_else(|| { - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key - }); - - // Verify the proof and get results using subset query - let path = vec![ - vec![RootTree::SavedBlockTransactions as u8], - vec![COMPACTED_ADDRESS_BALANCES_KEY_U8], - ]; - - let mut query = Query::new(); - query.insert_range_from(start_key..); - - let path_query = PathQuery::new(path, SizedQuery::new(query, limit, None)); + .unwrap_or_else(|| compacted_key(start_block_height, start_block_height)); + + let mut forward_inner = Query::new(); + forward_inner.insert_range_from(start_key..); + Some(PathQuery::new( + forward_path.clone(), + SizedQuery::new(forward_inner, limit, None), + )) + }; - let (root_hash, proved_key_values) = GroveDb::verify_subset_query( + // Step 3: verify the chained queries. GroveDB enforces that every + // sub-query binds to the SAME root hash. + let (root_hash, mut results) = GroveDb::verify_query_with_chained_path_queries( proof, - &path_query, + &boundary_query, + vec![generator], &platform_version.drive.grove_version, )?; - // Process the verified results + // results[0] is the boundary query, results[1] is the forward query. + let forward_results = results.pop().ok_or_else(|| { + Error::Proof(ProofError::CorruptedProof( + "chained verification returned no forward results".to_string(), + )) + })?; + + // Process the verified forward results. let mut compacted_changes = Vec::new(); - for (_path, key, maybe_element) in proved_key_values { + for (_path, key, maybe_element) in forward_results { let Some(element) = maybe_element else { continue; }; @@ -203,7 +174,7 @@ mod tests { use super::*; use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use dpp::address_funds::PlatformAddress; - use dpp::balances::credits::CreditOperation; + use dpp::balances::credits::{BlockAwareCreditOperation, CreditOperation}; use platform_version::version::PlatformVersion; use std::collections::BTreeMap; @@ -298,86 +269,140 @@ mod tests { ); } + /// Stores a single compacted address-balance entry directly under the + /// compacted address balances path with the given `(start_block, + /// end_block)` key. Bypasses normal compaction so tests can build exact + /// tree shapes (e.g. a containing range). + fn store_compacted_entry( + drive: &Drive, + start_block: u64, + end_block: u64, + changes: BTreeMap, + platform_version: &PlatformVersion, + ) { + use grovedb::Element; + use grovedb_costs::CostContext; + use grovedb_path::SubtreePath; + + let config = bincode::config::standard() + .with_big_endian() + .with_no_limit(); + + let key = compacted_key(start_block, end_block); + let value = bincode::encode_to_vec(&changes, config).expect("encode changes"); + + let path = Drive::saved_compacted_block_transactions_address_balances_path(); + + let CostContext { value: result, .. } = drive.grove.insert( + SubtreePath::from(path.as_ref()), + key.as_slice(), + Element::new_item(value), + None, + None, + &platform_version.drive.grove_version, + ); + result.expect("insert compacted entry"); + } + + /// Honest path returns the containing range `(100, 200)` for a start height + /// inside it (`150`). #[test] - fn test_verify_compacted_address_balance_changes_proof() { - // This proof was generated with start_block_height = 329 - // Path: [[36], [99]] = [['$'], ['c']] = SavedBlockTransactions -> CompactedAddressBalances - // Query: RangeTo(..[0, 0, 0, 0, 0, 0, 1, 73, 0, 0, 0, 0, 0, 0, 1, 73]) = RangeTo(..(329, 329)) - let proof: Vec = vec![ - 0, 251, 1, 24, 1, 24, 215, 122, 183, 233, 12, 7, 14, 37, 119, 175, 74, 229, 6, 76, 88, - 209, 79, 175, 135, 230, 45, 89, 116, 17, 76, 49, 87, 242, 4, 40, 35, 2, 33, 229, 84, - 218, 198, 132, 136, 197, 212, 253, 180, 247, 125, 228, 176, 179, 248, 100, 19, 202, - 143, 20, 196, 132, 112, 114, 44, 43, 11, 174, 49, 96, 16, 4, 1, 36, 0, 5, 2, 1, 1, 101, - 0, 126, 152, 206, 30, 157, 147, 101, 232, 212, 68, 50, 242, 52, 170, 136, 72, 39, 136, - 235, 41, 105, 28, 199, 93, 222, 168, 227, 241, 80, 234, 2, 185, 2, 120, 2, 233, 247, - 175, 89, 203, 114, 37, 58, 187, 95, 169, 151, 247, 245, 82, 228, 73, 77, 131, 5, 100, - 241, 7, 152, 139, 156, 94, 138, 33, 173, 16, 2, 124, 156, 122, 25, 79, 47, 39, 233, 96, - 90, 9, 165, 232, 130, 103, 245, 113, 145, 31, 183, 102, 94, 40, 86, 140, 146, 128, 172, - 85, 184, 13, 220, 16, 1, 48, 30, 57, 216, 246, 121, 172, 45, 163, 129, 189, 9, 238, - 108, 64, 21, 231, 140, 164, 160, 37, 184, 182, 34, 151, 148, 194, 74, 83, 124, 199, 87, - 17, 17, 2, 199, 32, 12, 6, 52, 58, 219, 92, 42, 60, 69, 132, 209, 186, 193, 248, 18, - 33, 26, 78, 216, 110, 114, 103, 202, 56, 45, 175, 163, 68, 139, 6, 16, 1, 62, 159, 56, - 33, 169, 193, 143, 7, 131, 179, 169, 31, 163, 163, 50, 117, 235, 211, 94, 247, 244, 60, - 246, 149, 186, 142, 105, 187, 230, 247, 212, 141, 17, 1, 1, 36, 125, 4, 1, 99, 0, 20, - 2, 1, 16, 0, 0, 0, 0, 0, 0, 1, 4, 0, 0, 0, 0, 0, 0, 1, 8, 0, 97, 206, 71, 247, 121, 93, - 103, 7, 209, 119, 82, 59, 209, 145, 171, 254, 112, 14, 204, 81, 30, 98, 213, 203, 146, - 141, 32, 167, 232, 34, 0, 37, 2, 170, 200, 228, 36, 229, 197, 116, 242, 100, 137, 25, - 37, 45, 57, 56, 2, 38, 8, 75, 144, 250, 71, 108, 90, 106, 133, 2, 231, 236, 42, 149, - 206, 16, 1, 77, 52, 100, 69, 203, 109, 100, 190, 84, 2, 6, 238, 168, 74, 208, 99, 16, - 56, 200, 98, 181, 205, 24, 79, 120, 235, 223, 144, 61, 197, 8, 215, 17, 1, 1, 99, 220, - 1, 28, 113, 173, 87, 207, 150, 171, 166, 221, 201, 207, 122, 14, 62, 119, 8, 5, 100, - 182, 50, 112, 191, 244, 5, 125, 9, 161, 17, 66, 201, 126, 148, 2, 170, 62, 172, 42, - 117, 12, 62, 165, 78, 141, 84, 194, 75, 135, 140, 198, 85, 219, 214, 218, 149, 56, 32, - 251, 16, 134, 21, 44, 31, 22, 80, 69, 16, 1, 191, 139, 156, 120, 188, 0, 187, 111, 169, - 41, 135, 146, 156, 103, 102, 125, 235, 0, 70, 180, 94, 103, 134, 250, 135, 56, 55, 144, - 156, 185, 212, 67, 2, 223, 93, 226, 244, 94, 203, 160, 13, 145, 161, 22, 104, 133, 135, - 132, 239, 27, 61, 12, 167, 134, 237, 120, 58, 229, 208, 50, 55, 70, 139, 211, 234, 16, - 2, 58, 253, 249, 36, 12, 24, 122, 198, 7, 161, 13, 237, 229, 3, 224, 98, 176, 51, 237, - 101, 105, 33, 10, 45, 111, 96, 89, 201, 212, 82, 1, 141, 5, 16, 0, 0, 0, 0, 0, 0, 1, - 32, 0, 0, 0, 0, 0, 0, 1, 36, 77, 228, 149, 252, 55, 96, 22, 145, 235, 199, 188, 83, 13, - 88, 13, 241, 48, 191, 78, 152, 20, 50, 252, 186, 199, 105, 134, 73, 62, 136, 228, 196, - 17, 17, 17, 0, 1, - ]; - - let start_block_height = 329u64; + fn should_return_containing_range_for_start_inside_it() { + let drive = setup_drive_with_initial_state_structure(None); let platform_version = PlatformVersion::latest(); - let result = Drive::verify_compacted_address_balance_changes( - &proof, - start_block_height, + let address = PlatformAddress::P2pkh([7; 20]); + let mut changes = BTreeMap::new(); + changes.insert( + address, + BlockAwareCreditOperation::from_operation(150, &CreditOperation::AddToCredits(12_345)), + ); + store_compacted_entry(&drive, 100, 200, changes.clone(), platform_version); + + let proof = drive + .prove_compacted_address_balance_changes(150, None, None, platform_version) + .expect("should prove compacted address balance changes"); + + let (_root_hash, compacted_changes) = Drive::verify_compacted_address_balance_changes( + proof.as_slice(), + 150, None, platform_version, - ); + ) + .expect("should verify proof"); - assert!( - result.is_ok(), - "proof verification failed: {:?}", - result.err() + assert_eq!( + compacted_changes.len(), + 1, + "the containing range (100, 200) must be surfaced for start=150" ); + let (start, end, returned_changes) = &compacted_changes[0]; + assert_eq!(*start, 100); + assert_eq!(*end, 200); + assert_eq!(returned_changes, &changes); + } - let (root_hash, compacted_changes) = result.unwrap(); + /// PoC: a malicious prover that skips descending discovery and proves + /// `range_from((150, 150)..)` directly MUST NOT make the verifier silently + /// return zero changes while a containing range `(100, 200)` holds a change. + #[test] + fn malicious_skip_descending_proof_is_rejected() { + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); - // Verify we got a valid root hash - assert!(!root_hash.is_empty(), "root hash should not be empty"); + let address = PlatformAddress::P2pkh([9; 20]); + let mut changes = BTreeMap::new(); + changes.insert( + address, + BlockAwareCreditOperation::from_operation(150, &CreditOperation::AddToCredits(99_999)), + ); + store_compacted_entry(&drive, 100, 200, changes, platform_version); + + // Craft the MALICIOUS proof the OLD (vulnerable) way: prove + // range_from((150, 150)..) directly. (100, 200) sorts before (150, 150) + // and so appears only as a hash-only boundary node. + let path = compacted_address_balances_path(); + let malicious_start_key = compacted_key(150, 150); + let mut malicious_inner = Query::new(); + malicious_inner.insert_range_from(malicious_start_key..); + let malicious_path_query = + PathQuery::new(path, SizedQuery::new(malicious_inner, None, None)); + + let grovedb_costs::CostContext { + value: malicious_proof_result, + .. + } = drive.grove.get_proved_path_query( + &malicious_path_query, + None, + None, + &platform_version.drive.grove_version, + ); + let malicious_proof = malicious_proof_result + .expect("should produce a (malicious) proof for the direct forward query"); - // The proof shows entry (288, 292) is the rightmost in the tree. - // Since 292 < 329 (our start_block_height), there are no results. - // The KVDigest at the boundary proves nothing exists >= (329, 329). - assert!( - compacted_changes.is_empty(), - "expected empty results since start_block_height 329 > last entry end_block 292" + let result = Drive::verify_compacted_address_balance_changes( + malicious_proof.as_slice(), + 150, + None, + platform_version, ); - // Log what we found for debugging - eprintln!("Root hash: {:?}", root_hash); - eprintln!("Number of compacted entries: {}", compacted_changes.len()); - for (start, end, changes) in &compacted_changes { - eprintln!( - " Blocks {}-{}: {} address changes", - start, - end, - changes.len() - ); + match result { + Err(_) => { + // Expected: the boundary query authenticates (100, 200) as the + // greatest key <= (150, MAX); the malicious proof cannot satisfy + // it, so verification fails. + } + Ok((_root_hash, returned_changes)) => { + assert!( + returned_changes + .iter() + .any(|(start, end, _)| *start == 100 && *end == 200), + "malicious proof must not silently hide the containing range \ + (100, 200); got {} changes", + returned_changes.len() + ); + } } } } diff --git a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs index 1bc888258ce..ba4904c8fce 100644 --- a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs @@ -1,173 +1,122 @@ -use crate::drive::shielded::nullifiers::queries::{ - shielded_compacted_nullifiers_path_vec, SHIELDED_COMPACTED_NULLIFIERS_KEY_U8, -}; +use crate::drive::shielded::nullifiers::queries::shielded_compacted_nullifiers_path_vec; use crate::drive::shielded::nullifiers::types::{CompactedNullifierChange, CompactedNullifiers}; -use crate::drive::shielded::paths::MAIN_SHIELDED_CREDIT_POOL_KEY_U8; use crate::drive::Drive; -use crate::drive::RootTree; use crate::error::proof::ProofError; use crate::error::Error; use crate::verify::RootHash; -use grovedb::operations::proof::{GroveDBProof, ProofBytes}; -use grovedb::{ - GroveDb, MerkProofDecoder, MerkProofNode, MerkProofOp, PathQuery, Query, SizedQuery, -}; +use grovedb::query_result_type::PathKeyOptionalElementTrio; +use grovedb::{GroveDb, PathQuery, Query, SizedQuery}; use platform_version::version::PlatformVersion; -/// Extract KV entries from merk proof bytes using the proper decoder. -#[allow(clippy::type_complexity)] -fn extract_kv_entries_from_merk_proof(merk_proof: &[u8]) -> Result, Vec)>, Error> { - let mut entries = Vec::new(); - - let decoder = MerkProofDecoder::new(merk_proof); - - for op in decoder { - match op { - Ok(MerkProofOp::Push(MerkProofNode::KV(key, value))) - | Ok(MerkProofOp::PushInverted(MerkProofNode::KV(key, value))) => { - entries.push((key, value)); - } - Err(e) => { - tracing::error!(%e, "merk proof decode error"); - return Err(Error::Proof(ProofError::CorruptedProof(format!( - "failed to decode merk proof op: {}", - e - )))); - } - _ => {} - } - } - - Ok(entries) +/// Builds the 16-byte big-endian compacted key `(start_block, end_block)`. +fn compacted_key(start_block: u64, end_block: u64) -> Vec { + let mut key = Vec::with_capacity(16); + key.extend_from_slice(&start_block.to_be_bytes()); + key.extend_from_slice(&end_block.to_be_bytes()); + key } impl Drive { /// Verifies compacted nullifier changes proof. /// - /// This verification works by: - /// 1. Decoding the GroveDBProof structure - /// 2. Navigating to the compacted nullifiers layer ('o') - /// 3. Extracting KV entries from the merk proof - /// 4. Filtering entries where the key range contains start_block_height - /// 5. Verifying the root hash using a subset query + /// # Soundness + /// + /// Compacted keys are 16 bytes `(start_block_be, end_block_be)`. A range + /// such as `(100, 200)` that *contains* a requested height `150` sorts + /// lexicographically **before** `(150, 150)`. This means the lower bound of + /// the forward range scan (`start_key`) cannot be trusted to come from the + /// caller-requested height alone — a malicious prover could prove + /// `range_from((150, 150)..)` directly and the containing range `(100, 200)` + /// would appear only as a hash-only boundary node, silently hiding a spend. + /// + /// To close this hole we never derive `start_key` from un-authenticated + /// proof bytes. Instead we use GroveDB's chained path query verification: + /// + /// 1. A **boundary query** (descending `range_to_inclusive(..=(start, MAX))`, + /// limit 1) authenticates, against the real root hash, the single + /// greatest compacted key `<= (start_block_height, u64::MAX)`. A malicious + /// prover cannot substitute or omit this key without breaking the root + /// hash. + /// 2. A **generator** inspects that authenticated boundary key. If its + /// `end_block >= start_block_height` the range contains (or starts at) the + /// requested height, so we use that exact key as the forward `start_key`. + /// Otherwise no containing range exists and we fall back to + /// `(start_block_height, start_block_height)`. + /// 3. The chained **forward query** (`range_from(start_key..)`, caller limit) + /// is verified against the same root hash, and its authenticated results + /// are decoded into the returned changes. pub(super) fn verify_compacted_nullifier_changes_v0( proof: &[u8], start_block_height: u64, limit: Option, platform_version: &PlatformVersion, ) -> Result<(RootHash, Vec), Error> { - // Decode the GroveDBProof to navigate its structure - let grovedb_proof: GroveDBProof = { - let config = bincode::config::standard() - .with_big_endian() - .with_limit::<{ 4 * 1024 * 1024 }>(); - let (proof_data, bytes_read): (GroveDBProof, usize) = - bincode::decode_from_slice(proof, config).map_err(|e| { - Error::Proof(ProofError::CorruptedProof(format!( - "cannot decode GroveDBProof: {}", - e - ))) - })?; - if bytes_read != proof.len() { - return Err(Error::Proof(ProofError::CorruptedProof(format!( - "trailing bytes after GroveDBProof decode: read {} of {}", - bytes_read, - proof.len() - )))); - } - proof_data - }; - - // Navigate to the compacted nullifiers layer - // Path: ShieldedBalances -> MAIN_SHIELDED_CREDIT_POOL_KEY -> CompactedNullifiers ('o') - let shielded_balances_key = vec![RootTree::ShieldedBalances as u8]; - let pool_key = vec![MAIN_SHIELDED_CREDIT_POOL_KEY_U8]; - let compacted_key = vec![SHIELDED_COMPACTED_NULLIFIERS_KEY_U8]; - - // Extract KV entries from the compacted layer's merk proof to find - // if there's a containing range for start_block_height. - // V0 and V1 proofs have different layer types (MerkOnlyLayerProof vs LayerProof), - // so we handle them separately. - let kv_entries = match &grovedb_proof { - GroveDBProof::V0(v0) => { - let compacted_layer = v0 - .root_layer - .lower_layers - .get(&shielded_balances_key) - .and_then(|layer| layer.lower_layers.get(&pool_key)) - .and_then(|layer| layer.lower_layers.get(&compacted_key)); - compacted_layer - .map(|layer| extract_kv_entries_from_merk_proof(&layer.merk_proof)) - .transpose()? - .unwrap_or_default() - } - GroveDBProof::V1(v1) => { - let compacted_layer = v1 - .root_layer - .lower_layers - .get(&shielded_balances_key) - .and_then(|layer| layer.lower_layers.get(&pool_key)) - .and_then(|layer| layer.lower_layers.get(&compacted_key)); - compacted_layer - .map(|layer| match &layer.merk_proof { - ProofBytes::Merk(bytes) => extract_kv_entries_from_merk_proof(bytes), - other => Err(Error::Proof(ProofError::CorruptedProof(format!( - "unsupported V1 proof bytes variant for compacted nullifiers: {:?}", - std::mem::discriminant(other) - )))), - }) - .transpose()? - .unwrap_or_default() - } - }; - - // Look for a KV entry where the range contains start_block_height - // Keys are 16 bytes: (start_block, end_block), both big-endian - let containing_key = kv_entries.iter().find_map(|(key, _)| { - if key.len() != 16 { - return None; - } - // Safety: length verified to be 16 above - let range_start = - u64::from_be_bytes(key[0..8].try_into().expect("len checked to be 16")); - let range_end = - u64::from_be_bytes(key[8..16].try_into().expect("len checked to be 16")); - - // Check if this range contains start_block_height - if range_start <= start_block_height && start_block_height <= range_end { - Some(key.clone()) - } else { - None - } - }); - - // Determine the start_key for the query - // Use the containing range's key if found, otherwise (start_block_height, start_block_height) - let start_key = containing_key.unwrap_or_else(|| { - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key.extend_from_slice(&start_block_height.to_be_bytes()); - key - }); - - // Verify the proof and get results using subset query let path = shielded_compacted_nullifiers_path_vec(); - let mut query = Query::new(); - query.insert_range_from(start_key..); - - let path_query = PathQuery::new(path, SizedQuery::new(query, limit, None)); + // Step 1: boundary query — authenticate the single greatest compacted + // key <= (start_block_height, u64::MAX). Descending, limit 1. + let boundary_end_key = compacted_key(start_block_height, u64::MAX); + let mut boundary_inner = Query::new_with_direction(false); // descending + boundary_inner.insert_range_to_inclusive(..=boundary_end_key); + let boundary_query = + PathQuery::new(path.clone(), SizedQuery::new(boundary_inner, Some(1), None)); + + // Step 2: generator — derive the forward query's lower bound from the + // AUTHENTICATED boundary result (not from raw proof bytes). + let forward_path = path.clone(); + let generator = + move |boundary_results: Vec| -> Option { + // The boundary key contains start_block_height when its end_block is + // at or beyond it. Use the authenticated key directly in that case; + // otherwise there is no containing range and we start at + // (start_block_height, start_block_height). + let start_key = boundary_results + .iter() + .find_map(|(_path, key, _element)| { + if key.len() != 16 { + return None; + } + let end_block = u64::from_be_bytes( + key[8..16].try_into().expect("len checked to be 16"), + ); + if end_block >= start_block_height { + Some(key.clone()) + } else { + None + } + }) + .unwrap_or_else(|| compacted_key(start_block_height, start_block_height)); + + let mut forward_inner = Query::new(); + forward_inner.insert_range_from(start_key..); + Some(PathQuery::new( + forward_path.clone(), + SizedQuery::new(forward_inner, limit, None), + )) + }; - let (root_hash, proved_key_values) = GroveDb::verify_subset_query( + // Step 3: verify the chained queries. GroveDB enforces that every + // sub-query binds to the SAME root hash, so the boundary key authenticated + // in step 1 and the forward results in step 3 are consistent with one + // another and with the real state. + let (root_hash, mut results) = GroveDb::verify_query_with_chained_path_queries( proof, - &path_query, + &boundary_query, + vec![generator], &platform_version.drive.grove_version, )?; - // Process the verified results + // results[0] is the boundary query, results[1] is the forward query. + let forward_results = results.pop().ok_or_else(|| { + Error::Proof(ProofError::CorruptedProof( + "chained verification returned no forward results".to_string(), + )) + })?; + + // Process the verified forward results. let mut compacted_changes = Vec::new(); - for (_path, key, maybe_element) in proved_key_values { + for (_path, key, maybe_element) in forward_results { let Some(element) = maybe_element else { continue; }; @@ -197,7 +146,9 @@ impl Drive { #[cfg(test)] mod tests { use super::*; + use crate::drive::shielded::nullifiers::queries::shielded_compacted_nullifiers_path_vec; use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; + use grovedb::{PathQuery, Query, SizedQuery}; use platform_version::version::PlatformVersion; #[test] @@ -280,4 +231,157 @@ mod tests { "should have no compacted entries when no data stored" ); } + + /// Stores a single compacted nullifier entry directly under the compacted + /// nullifiers path with the given `(start_block, end_block)` key. + /// + /// This bypasses the normal recent->compacted promotion so that tests can + /// construct exact tree shapes (e.g. a containing range) deterministically. + fn store_compacted_entry( + drive: &Drive, + start_block: u64, + end_block: u64, + nullifiers: Vec<[u8; 32]>, + platform_version: &PlatformVersion, + ) { + use crate::drive::shielded::nullifiers::queries::shielded_compacted_nullifiers_path; + use grovedb::Element; + use grovedb_costs::CostContext; + use grovedb_path::SubtreePath; + + let key = super::compacted_key(start_block, end_block); + let value = CompactedNullifiers::new(nullifiers) + .encode() + .expect("encode nullifiers"); + + let path = shielded_compacted_nullifiers_path(); + + let CostContext { value: result, .. } = drive.grove.insert( + SubtreePath::from(path.as_ref()), + key.as_slice(), + Element::new_item(value), + None, + None, + &platform_version.drive.grove_version, + ); + result.expect("insert compacted entry"); + } + + /// Verifies that the honest path returns the containing range `(100, 200)` + /// when querying from a height inside it (`150`). + #[test] + fn should_return_containing_range_for_start_inside_it() { + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); + + // A spend recorded inside a compacted range that straddles block 150. + let double_spend_nullifier = [0xAB; 32]; + store_compacted_entry( + &drive, + 100, + 200, + vec![double_spend_nullifier], + platform_version, + ); + + // Honest proof from block 150 (inside the (100, 200) range). + let proof = drive + .prove_compacted_nullifier_changes(150, None, None, platform_version) + .expect("should prove compacted nullifier changes"); + + let (_root_hash, compacted_changes) = Drive::verify_compacted_nullifier_changes( + proof.as_slice(), + 150, + None, + platform_version, + ) + .expect("should verify proof"); + + assert_eq!( + compacted_changes.len(), + 1, + "the containing range (100, 200) must be surfaced for start=150" + ); + let change = &compacted_changes[0]; + assert_eq!(change.start_block, 100); + assert_eq!(change.end_block, 200); + assert_eq!(change.nullifiers.as_slice(), &[double_spend_nullifier]); + } + + /// PoC: a malicious prover that skips the descending discovery step and + /// proves `range_from((150, 150)..)` directly MUST NOT be able to make the + /// verifier silently return zero nullifiers while a containing range + /// `(100, 200)` actually holds a spend. + /// + /// With the chained-boundary verifier, the boundary query authenticates + /// `(100, 200)` as the true greatest key `<= (150, MAX)`. The generator then + /// requires the forward query to start at `(100, 200)`. The malicious proof, + /// which only includes `(100, 200)` as a hash-only boundary node and lacks + /// its full value, cannot satisfy that forward query, so verification fails + /// (it can never silently return zero). + #[test] + fn malicious_skip_descending_proof_is_rejected() { + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); + + // Containing range (100, 200) holds a double-spend nullifier. + let double_spend_nullifier = [0xCD; 32]; + store_compacted_entry( + &drive, + 100, + 200, + vec![double_spend_nullifier], + platform_version, + ); + + // Craft the MALICIOUS proof the OLD (vulnerable) way: prove + // range_from((150, 150)..) directly, skipping descending discovery. + // (100, 200) sorts before (150, 150) lexicographically, so it appears + // only as a hash-only boundary node in this proof. + let path = shielded_compacted_nullifiers_path_vec(); + let malicious_start_key = super::compacted_key(150, 150); + let mut malicious_inner = Query::new(); + malicious_inner.insert_range_from(malicious_start_key..); + let malicious_path_query = + PathQuery::new(path, SizedQuery::new(malicious_inner, None, None)); + + let grovedb_costs::CostContext { + value: malicious_proof_result, + .. + } = drive.grove.get_proved_path_query( + &malicious_path_query, + None, + None, + &platform_version.drive.grove_version, + ); + let malicious_proof = malicious_proof_result + .expect("should produce a (malicious) proof for the direct forward query"); + + let result = Drive::verify_compacted_nullifier_changes( + malicious_proof.as_slice(), + 150, + None, + platform_version, + ); + + // The fix must NOT silently return zero nullifiers. Either verification + // fails outright, or it surfaces the (100, 200) double-spend. + match result { + Err(_) => { + // Expected: the boundary query authenticates (100, 200) as the + // greatest key <= (150, MAX); the malicious proof cannot satisfy + // the resulting forward query, so verification fails. + } + Ok((_root_hash, changes)) => { + assert!( + changes + .iter() + .any(|c| c.start_block == 100 && c.end_block == 200), + "malicious proof must not silently hide the containing range \ + (100, 200); got {} changes", + changes.len() + ); + } + } + } } From 16d1e43995854e955b1afbe613f2b08f4b43d838 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 12:20:53 +0200 Subject: [PATCH 02/11] docs(drive): document known liveness bug in compacted absence-proof fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review found that the chained boundary(descending) + forward(ascending) verification cannot be satisfied by a single one-directional GroveDB proof. When >=2 compacted ranges sort at/below the query height (the normal paginated sync case — the query handler hardcodes limit=25 and the SDK loops), the honest prove->verify roundtrip fails with "Cannot verify upper bound of queried range". The original tests only covered single-range and empty cases, so they missed it. Adds two #[ignore]d regression tests reproducing the failure and a "# KNOWN LIVENESS BUG" note on the verifier. The planned fix (deferred) is to re-key compacted entries by (end_block, start_block) so retrieval becomes a single ascending range_from((H,0)..), which also closes the original absence-proof soundness hole structurally. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../v0/mod.rs | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs index ba4904c8fce..022d70c9a59 100644 --- a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs @@ -45,6 +45,20 @@ impl Drive { /// 3. The chained **forward query** (`range_from(start_key..)`, caller limit) /// is verified against the same root hash, and its authenticated results /// are decoded into the returned changes. + /// + /// # KNOWN LIVENESS BUG (tracked in PR #3792 — fix deferred) + /// + /// The boundary query is **descending** (greatest key `<= bound`) while the + /// forward query is **ascending** (paginated `range_from`). A single GroveDB + /// proof is one-directional, so when **two or more** compacted ranges sort + /// at/below `start_block_height` the honest proof fails verification with + /// "Cannot verify upper bound of queried range" (see the two `#[ignore]`d + /// `*_below_query_height*` regression tests). The single-range and empty + /// cases work, which is why this slipped the original tests. The planned fix + /// is to re-key compacted entries by `(end_block, start_block)` so retrieval + /// becomes a single ascending `range_from((H, 0)..)` — which also closes the + /// original absence-proof soundness hole structurally (the containing range + /// becomes an ordinary in-range result that cannot be omitted). pub(super) fn verify_compacted_nullifier_changes_v0( proof: &[u8], start_block_height: u64, @@ -308,6 +322,101 @@ mod tests { assert_eq!(change.nullifiers.as_slice(), &[double_spend_nullifier]); } + /// ADVERSARIAL: multiple compacted ranges all sort at/below the query + /// height — the realistic light-client sync case. A reviewer flagged a + /// possible direction-mismatch liveness break (ascending merged proof vs + /// descending limit-1 boundary verify query) that the single-range tests + /// would not catch. This proves the honest prove->verify roundtrip succeeds + /// and returns the containing range when >=2 ranges are <= (start, MAX). + /// + /// KNOWN-FAILING (see PR #3792 description): the chained + /// boundary(descending) + forward(ascending) scheme cannot be satisfied by a + /// single one-directional GroveDB proof. With >=2 compacted ranges at/below + /// the query height the honest proof fails with "Cannot verify upper bound of + /// queried range". Planned fix: re-key compacted entries by + /// (end_block, start_block) so the query is a single ascending + /// `range_from((H,0)..)`. Un-ignore once fixed. + #[ignore = "known liveness bug: chained descending-boundary + ascending-forward \ + cannot share one GroveDB proof; fix = re-key by end_block (see PR #3792)"] + #[test] + fn multiple_ranges_below_query_height_verify() { + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); + + // Three contiguous compacted ranges, ALL with start_block <= 150. + store_compacted_entry(&drive, 1, 64, vec![[0x01; 32]], platform_version); + store_compacted_entry(&drive, 65, 128, vec![[0x02; 32]], platform_version); + store_compacted_entry(&drive, 129, 192, vec![[0x03; 32]], platform_version); + + // Query from 150: bound = (150, MAX). Keys <= bound: (1,64), (65,128), + // (129,192) — THREE keys. (129,192) contains 150 (129 <= 150 <= 192). + let proof = drive + .prove_compacted_nullifier_changes(150, None, None, platform_version) + .expect("should prove with multiple ranges below the query height"); + + let (_root, changes) = Drive::verify_compacted_nullifier_changes( + proof.as_slice(), + 150, + None, + platform_version, + ) + .expect("VERIFY MUST NOT FAIL with multiple ranges <= bound"); + + assert!( + changes + .iter() + .any(|c| c.start_block == 129 && c.end_block == 192), + "containing range (129,192) must be surfaced; got {:?}", + changes + .iter() + .map(|c| (c.start_block, c.end_block)) + .collect::>() + ); + } + + /// ADVERSARIAL variant: a non-zero gap below the containing range, querying + /// from a height strictly inside the third range with two full ranges below. + /// + /// KNOWN-FAILING (see PR #3792 description): same direction-mismatch bug as + /// `multiple_ranges_below_query_height_verify`. Un-ignore once the compacted + /// tree is re-keyed by (end_block, start_block). + #[ignore = "known liveness bug: chained descending-boundary + ascending-forward \ + cannot share one GroveDB proof; fix = re-key by end_block (see PR #3792)"] + #[test] + fn containing_range_with_two_lower_ranges_verifies() { + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); + + store_compacted_entry(&drive, 100, 200, vec![[0xA1; 32]], platform_version); + store_compacted_entry(&drive, 300, 400, vec![[0xA2; 32]], platform_version); + store_compacted_entry(&drive, 500, 600, vec![[0xA3; 32]], platform_version); + + // Query from 550: (500,600) contains it; (100,200) and (300,400) are + // both <= (550, MAX) and sort below the containing range. + let proof = drive + .prove_compacted_nullifier_changes(550, None, None, platform_version) + .expect("should prove"); + + let (_root, changes) = Drive::verify_compacted_nullifier_changes( + proof.as_slice(), + 550, + None, + platform_version, + ) + .expect("VERIFY MUST NOT FAIL"); + + assert!( + changes + .iter() + .any(|c| c.start_block == 500 && c.end_block == 600), + "containing range (500,600) must be surfaced; got {:?}", + changes + .iter() + .map(|c| (c.start_block, c.end_block)) + .collect::>() + ); + } + /// PoC: a malicious prover that skips the descending discovery step and /// proves `range_from((150, 150)..)` directly MUST NOT be able to make the /// verifier silently return zero nullifiers while a containing range From 219168026213b7c1f2666db6e1c87f705a4596b5 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 12:58:49 +0200 Subject: [PATCH 03/11] fix(drive): cap compacted-proof size + honor transaction (review feedback) Addresses automated review findings on the compacted absence-proof PR (the liveness bug itself remains deferred to the planned re-key): - BLOCKING (DoS): the Some(boundary_key) prover branch built the forward query with no limit and proved it via prove_query_many, dropping the caller's `limit`. The public handlers pass limit=25 "to stay within proof size limits"; bypassing it let an unauthenticated `prove=true` request from an early height force a proof over the entire compacted history (remote CPU/memory/bandwidth amplification). Both provers now prove a single query (boundary key + forward range) capped at `limit + 1` (the +1 covers the authenticated boundary point). The verifier already re-applies `limit` to its forward subset query, so the cap is soundness-neutral. - The same branch is now routed through the transaction-aware `grove_get_proved_path_query` instead of `prove_query_many` (which ignores the TransactionArg), removing a latent root-hash inconsistency under an active transaction. - Address-balance prover: use `try_into().map_err(..)?` instead of `.unwrap()` to match the nullifier prover. - Address-balance verifier: add the matching `KNOWN LIVENESS BUG` docstring and an `#[ignore]`d multi-range regression test (the bug was only documented on the nullifier path). - Add `query_past_last_range_returns_empty` tests (store (100,200), query at 300 -> Ok with zero results) to both verifiers, covering the no-containing-range fallback that was previously untested. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../v0/mod.rs | 49 +++++----- .../v0/mod.rs | 49 +++++----- .../v0/mod.rs | 95 +++++++++++++++++++ .../v0/mod.rs | 34 +++++++ 4 files changed, 181 insertions(+), 46 deletions(-) diff --git a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs index c2ab4e24e0d..585cc963e8a 100644 --- a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs +++ b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs @@ -5,7 +5,6 @@ use dpp::balances::credits::BlockAwareCreditOperation; use dpp::ProtocolError; use grovedb::query_result_type::QueryResultType; use grovedb::{Element, PathQuery, Query, SizedQuery, TransactionArg}; -use grovedb_costs::CostContext; use platform_version::version::PlatformVersion; use std::collections::BTreeMap; @@ -246,7 +245,11 @@ impl Drive { ), ))); } - let end_block = u64::from_be_bytes(key[8..16].try_into().unwrap()); + let end_block = u64::from_be_bytes(key[8..16].try_into().map_err(|_| { + Error::Protocol(Box::new(ProtocolError::CorruptedSerialization( + "invalid compacted key slice".to_string(), + ))) + })?); let forward_start = if end_block >= start_block_height { key.clone() } else { @@ -257,29 +260,31 @@ impl Drive { None => (None, compacted_key(start_block_height, start_block_height)), }; - // Step 2: build the single merged proof. See the nullifier prover and - // the verifier docs for the soundness rationale. `PathQuery::merge` - // rejects per-query limits, so the merged queries carry no limits; the - // forward query's caller limit is applied to the verifier's forward - // subset query instead (subset verification accepts a superset proof). + // Step 2: build the proof. See the nullifier prover and the verifier docs + // for the soundness rationale. match boundary_key { Some(boundary_key) => { - let mut boundary_point = Query::new(); - boundary_point.insert_key(boundary_key); - let boundary_point_query = - PathQuery::new(path.clone(), SizedQuery::new(boundary_point, None, None)); + // Cap the proof at the caller's `limit` (+1 for the authenticated + // boundary point) so a client requesting `prove=true` from an early + // height cannot force a proof spanning the entire compacted history + // (the public handler passes `limit = Some(25)` "to stay within proof + // size limits"). The verifier re-applies `limit` to its forward subset + // query, so this cap is soundness-neutral. Routed through the + // transaction-aware proof path (not `prove_query_many`, which ignores + // the transaction) for snapshot consistency. + let capped_limit = limit.map(|l| l.saturating_add(1)); + let mut query = Query::new(); + query.insert_key(boundary_key); + query.insert_range_from(forward_start..); + let path_query = + PathQuery::new(path.clone(), SizedQuery::new(query, capped_limit, None)); - let mut forward_query = Query::new(); - forward_query.insert_range_from(forward_start..); - let forward_path_query = - PathQuery::new(path.clone(), SizedQuery::new(forward_query, None, None)); - - let CostContext { value, .. } = self.grove.prove_query_many( - vec![&boundary_point_query, &forward_path_query], - None, - &platform_version.drive.grove_version, - ); - value.map_err(Error::from) + self.grove_get_proved_path_query( + &path_query, + transaction, + &mut vec![], + &platform_version.drive, + ) } None => { let mut forward_query = Query::new(); diff --git a/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs index c2faf512cfb..6af46e435c0 100644 --- a/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs @@ -4,7 +4,6 @@ use crate::error::Error; use dpp::ProtocolError; use grovedb::query_result_type::QueryResultType; use grovedb::{PathQuery, Query, SizedQuery, TransactionArg}; -use grovedb_costs::CostContext; use platform_version::version::PlatformVersion; /// Builds the 16-byte big-endian compacted key `(start_block, end_block)`. @@ -92,37 +91,39 @@ impl Drive { None => (None, compacted_key(start_block_height, start_block_height)), }; - // Step 2: build the single merged proof. + // Step 2: build the proof. // // The verifier's chained boundary query is a descending // `range_to_inclusive(..=(start, MAX))` limit-1 subset query. For it to // authenticate `boundary_key` as the true greatest key, the proof must // include `boundary_key` as a full node together with the contiguity - // information around it. We therefore include the boundary key as an - // explicit point query merged with the forward range. - // - // `PathQuery::merge` rejects per-query limits, so the merged queries - // carry no limits; the forward query's caller limit is applied to the - // verifier's forward subset query instead (the proof is a superset, - // which subset verification accepts). + // information around it. We therefore prove a single query that unions the + // boundary key (as an explicit point) with the forward range. match boundary_key { Some(boundary_key) => { - let mut boundary_point = Query::new(); - boundary_point.insert_key(boundary_key); - let boundary_point_query = - PathQuery::new(path.clone(), SizedQuery::new(boundary_point, None, None)); - - let mut forward_query = Query::new(); - forward_query.insert_range_from(forward_start..); - let forward_path_query = - PathQuery::new(path.clone(), SizedQuery::new(forward_query, None, None)); + // Cap the proof at the caller's `limit` (+1 for the authenticated + // boundary point) so a client requesting `prove=true` from an early + // height cannot force a proof spanning the entire compacted history + // — the public handler passes `limit = Some(25)` "to stay within + // proof size limits", and dropping it here was a remote + // CPU/memory/bandwidth amplification vector. The verifier re-applies + // `limit` to its forward subset query, so this cap is + // soundness-neutral. Routed through the transaction-aware proof path + // (not `prove_query_many`, which ignores the transaction) so an + // active transaction's snapshot is used consistently. + let capped_limit = limit.map(|l| l.saturating_add(1)); + let mut query = Query::new(); + query.insert_key(boundary_key); + query.insert_range_from(forward_start..); + let path_query = + PathQuery::new(path.clone(), SizedQuery::new(query, capped_limit, None)); - let CostContext { value, .. } = self.grove.prove_query_many( - vec![&boundary_point_query, &forward_path_query], - None, - &platform_version.drive.grove_version, - ); - value.map_err(Error::from) + self.grove_get_proved_path_query( + &path_query, + transaction, + &mut vec![], + &platform_version.drive, + ) } None => { // No compacted key <= (start, MAX) exists at all (empty / diff --git a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs index eb05d812324..7f6913d0567 100644 --- a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs @@ -61,6 +61,19 @@ impl Drive { /// 3. The chained **forward query** (`range_from(start_key..)`, caller limit) /// is verified against the same root hash, and its authenticated results /// are decoded into the returned changes. + /// + /// # KNOWN LIVENESS BUG (tracked in PR #3792 — fix deferred) + /// + /// Identical to the shielded-nullifier verifier: the boundary query is + /// **descending** while the forward query is **ascending**, and a single + /// GroveDB proof is one-directional, so when **two or more** compacted + /// address-balance ranges sort at/below `start_block_height` the honest proof + /// fails verification with "Cannot verify upper bound of queried range" (see + /// the `#[ignore]`d `multiple_ranges_below_query_height_verify` regression + /// test). The single-range and empty cases work. The planned fix is to re-key + /// compacted entries by `(end_block, start_block)` so retrieval becomes a + /// single ascending `range_from((H, 0)..)` — which also closes the original + /// absence-proof soundness hole structurally. pub(super) fn verify_compacted_address_balance_changes_v0( proof: &[u8], start_block_height: u64, @@ -405,4 +418,86 @@ mod tests { } } } + + /// Querying past the last compacted range: the boundary key `(100, 200)` has + /// `end_block < start_block_height`, so there is no containing range and the + /// forward scan finds nothing. This exercises the `find_map` fallback to + /// `(start, start)` on both prover and verifier — a single key `<= bound`, so + /// it is unaffected by the known multi-range liveness bug. + #[test] + fn query_past_last_range_returns_empty() { + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); + + let address = PlatformAddress::P2pkh([7; 20]); + let mut changes = BTreeMap::new(); + changes.insert( + address, + BlockAwareCreditOperation::from_operation(150, &CreditOperation::AddToCredits(12_345)), + ); + store_compacted_entry(&drive, 100, 200, changes, platform_version); + + // Query at 300, strictly past the only range (100, 200). + let proof = drive + .prove_compacted_address_balance_changes(300, None, None, platform_version) + .expect("should prove"); + let (_root, compacted_changes) = Drive::verify_compacted_address_balance_changes( + proof.as_slice(), + 300, + None, + platform_version, + ) + .expect("should verify"); + + assert!( + compacted_changes.is_empty(), + "querying past the last range must return zero changes, got {:?}", + compacted_changes + .iter() + .map(|(s, e, _)| (*s, *e)) + .collect::>() + ); + } + + /// ADVERSARIAL (mirror of the nullifier verifier): >=2 compacted ranges + /// at/below the query height. KNOWN-FAILING per PR #3792 — the chained + /// descending-boundary + ascending-forward scheme cannot be satisfied by one + /// one-directional GroveDB proof; fails with "Cannot verify upper bound of + /// queried range". Un-ignore once the compacted tree is re-keyed by + /// `(end_block, start_block)`. + #[ignore = "known liveness bug: chained descending-boundary + ascending-forward \ + cannot share one GroveDB proof; fix = re-key by end_block (see PR #3792)"] + #[test] + fn multiple_ranges_below_query_height_verify() { + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); + + let address = PlatformAddress::P2pkh([7; 20]); + for (s, e) in [(1u64, 64u64), (65, 128), (129, 192)] { + let mut changes = BTreeMap::new(); + changes.insert( + address, + BlockAwareCreditOperation::from_operation(e, &CreditOperation::AddToCredits(e)), + ); + store_compacted_entry(&drive, s, e, changes, platform_version); + } + + // Query from 150: (1,64),(65,128),(129,192) all sort <= (150, MAX). + let proof = drive + .prove_compacted_address_balance_changes(150, None, None, platform_version) + .expect("should prove with multiple ranges below the query height"); + let (_root, changes) = Drive::verify_compacted_address_balance_changes( + proof.as_slice(), + 150, + None, + platform_version, + ) + .expect("VERIFY MUST NOT FAIL with multiple ranges <= bound"); + + assert!( + changes.iter().any(|(s, e, _)| *s == 129 && *e == 192), + "containing range (129,192) must be surfaced; got {:?}", + changes.iter().map(|(s, e, _)| (*s, *e)).collect::>() + ); + } } diff --git a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs index 022d70c9a59..948695103bc 100644 --- a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs @@ -493,4 +493,38 @@ mod tests { } } } + + /// Querying past the last compacted range: the boundary key `(100, 200)` has + /// `end_block < start_block_height`, so there is no containing range and the + /// forward scan finds nothing. Exercises the `find_map` fallback to + /// `(start, start)` on both prover and verifier (a single key `<= bound`, so + /// unaffected by the known multi-range liveness bug). + #[test] + fn query_past_last_range_returns_empty() { + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); + + store_compacted_entry(&drive, 100, 200, vec![[0xAB; 32]], platform_version); + + // Query at 300, strictly past the only range (100, 200). + let proof = drive + .prove_compacted_nullifier_changes(300, None, None, platform_version) + .expect("should prove"); + let (_root, changes) = Drive::verify_compacted_nullifier_changes( + proof.as_slice(), + 300, + None, + platform_version, + ) + .expect("should verify"); + + assert!( + changes.is_empty(), + "querying past the last range must return zero changes, got {:?}", + changes + .iter() + .map(|c| (c.start_block, c.end_block)) + .collect::>() + ); + } } From 0cdf371ac962118800bff733a848d7e496e98860 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 13:09:26 +0200 Subject: [PATCH 04/11] docs(drive): update compacted-prover docstrings to current proof approach Both `prove_compacted_*_changes_v0` docstrings still described the old `prove_query_many` merged-proof approach, but commit 2191680 switched the Some(boundary_key) branch to a single combined `PathQuery` (boundary key via `insert_key` + forward `range_from`, capped at the caller's limit) proved through `grove_get_proved_path_query`. Updated both docstrings to match. Doc-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../fetch_compacted_address_balances/v0/mod.rs | 9 +++++---- .../prove/prove_compacted_nullifier_changes/v0/mod.rs | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs index 585cc963e8a..095d5365621 100644 --- a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs +++ b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs @@ -201,10 +201,11 @@ impl Drive { /// the containing range (if any) or `(start_block_height, /// start_block_height)`. /// - /// We discover `start_key` with a non-proving descending query, then emit a - /// single merged proof (`prove_query_many`) that covers BOTH the boundary - /// key and the forward range so the verifier's chained queries are both - /// satisfiable. + /// We discover `start_key` with a non-proving descending query, then prove a + /// single combined `PathQuery` (boundary key via `insert_key` + the forward + /// `range_from`, capped at the caller's limit) through + /// `grove_get_proved_path_query`, so the verifier's chained boundary and + /// forward queries are both satisfiable. pub(super) fn prove_compacted_address_balance_changes_v0( &self, start_block_height: u64, diff --git a/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs index 6af46e435c0..e4aeb51ae8b 100644 --- a/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs @@ -29,10 +29,11 @@ impl Drive { /// the containing range (if any) or `(start_block_height, /// start_block_height)`. /// - /// We discover `start_key` with a non-proving descending query, then emit a - /// single merged proof (`prove_query_many`) that covers BOTH the boundary - /// key and the forward range so the verifier's chained queries are both - /// satisfiable. + /// We discover `start_key` with a non-proving descending query, then prove a + /// single combined `PathQuery` (boundary key via `insert_key` + the forward + /// `range_from`, capped at the caller's limit) through + /// `grove_get_proved_path_query`, so the verifier's chained boundary and + /// forward queries are both satisfiable. pub(super) fn prove_compacted_nullifier_changes_v0( &self, start_block_height: u64, From a7c31c59c9b80fcd9bd39d23f8e1b044696c35f7 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 13:27:44 +0200 Subject: [PATCH 05/11] refactor(drive): classify chained-verify cardinality break as internal, not CorruptedProof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compacted verifiers used `results.pop().ok_or_else(|| CorruptedProof)` to extract the forward result set. Because the single chained generator always returns `Some`, `verify_query_with_chained_path_queries` always yields exactly two result sets, so that branch is unreachable from caller input — it can only fire on a GroveDB-internal invariant break. Mapping it to `ProofError::CorruptedProof` (a user-facing "your proof is corrupt" error) therefore misclassified an internal-API violation as caller proof corruption. Both verifiers now destructure `let [_boundary, forward_results] = results.try_into().map_err(..)?`, which makes the [boundary, forward] cardinality explicit at the type level and reports any deviation as `DriveError::CorruptedCodeExecution` (an internal-invariant error). No behavior change on any reachable path; all verify_compacted tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../v0/mod.rs | 20 ++++++++++++------- .../v0/mod.rs | 20 ++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs index 7f6913d0567..c74f5370733 100644 --- a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs @@ -1,5 +1,6 @@ use crate::drive::Drive; use crate::drive::RootTree; +use crate::error::drive::DriveError; use crate::error::proof::ProofError; use crate::error::Error; use crate::verify::RootHash; @@ -126,19 +127,24 @@ impl Drive { // Step 3: verify the chained queries. GroveDB enforces that every // sub-query binds to the SAME root hash. - let (root_hash, mut results) = GroveDb::verify_query_with_chained_path_queries( + let (root_hash, results) = GroveDb::verify_query_with_chained_path_queries( proof, &boundary_query, vec![generator], &platform_version.drive.grove_version, )?; - // results[0] is the boundary query, results[1] is the forward query. - let forward_results = results.pop().ok_or_else(|| { - Error::Proof(ProofError::CorruptedProof( - "chained verification returned no forward results".to_string(), - )) - })?; + // We pass exactly one chained (forward) generator that always returns + // `Some`, so GroveDB returns exactly two result sets: [boundary, forward]. + // A different cardinality can only be a GroveDB-internal invariant break + // (never caller-supplied proof corruption), so classify it as an internal + // code-execution error rather than `CorruptedProof`. + let [_boundary_results, forward_results]: [Vec; 2] = + results.try_into().map_err(|_| { + Error::Drive(DriveError::CorruptedCodeExecution( + "chained verification invariant: expected [boundary, forward] result sets", + )) + })?; // Process the verified forward results. let mut compacted_changes = Vec::new(); diff --git a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs index 948695103bc..f09cbd02754 100644 --- a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs @@ -1,6 +1,7 @@ use crate::drive::shielded::nullifiers::queries::shielded_compacted_nullifiers_path_vec; use crate::drive::shielded::nullifiers::types::{CompactedNullifierChange, CompactedNullifiers}; use crate::drive::Drive; +use crate::error::drive::DriveError; use crate::error::proof::ProofError; use crate::error::Error; use crate::verify::RootHash; @@ -113,19 +114,24 @@ impl Drive { // sub-query binds to the SAME root hash, so the boundary key authenticated // in step 1 and the forward results in step 3 are consistent with one // another and with the real state. - let (root_hash, mut results) = GroveDb::verify_query_with_chained_path_queries( + let (root_hash, results) = GroveDb::verify_query_with_chained_path_queries( proof, &boundary_query, vec![generator], &platform_version.drive.grove_version, )?; - // results[0] is the boundary query, results[1] is the forward query. - let forward_results = results.pop().ok_or_else(|| { - Error::Proof(ProofError::CorruptedProof( - "chained verification returned no forward results".to_string(), - )) - })?; + // We pass exactly one chained (forward) generator that always returns + // `Some`, so GroveDB returns exactly two result sets: [boundary, forward]. + // A different cardinality can only be a GroveDB-internal invariant break + // (never caller-supplied proof corruption), so classify it as an internal + // code-execution error rather than `CorruptedProof`. + let [_boundary_results, forward_results]: [Vec; 2] = + results.try_into().map_err(|_| { + Error::Drive(DriveError::CorruptedCodeExecution( + "chained verification invariant: expected [boundary, forward] result sets", + )) + })?; // Process the verified forward results. let mut compacted_changes = Vec::new(); From 98a62db678bc3b64ddeb7f47281c90b387349825 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 13:54:52 +0200 Subject: [PATCH 06/11] refactor(drive): centralize compacted_key into one shared helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 16-byte big-endian `(start_block, end_block)` compacted-key encoding was duplicated verbatim as a private `compacted_key` fn in all four touched modules (both provers and both verifiers). Since that encoding is part of the chained-proof contract, drift between any prover and its matching verifier would silently break chained verification — a correctness concern, not style. Moved it to a single `crate::util::common::compacted_key` (pub(crate)) and pointed all four modules at it via `use`. This also shrinks the planned `(end_block, start_block)` re-key to a one-place change instead of a four-place rewrite. No behavior change; all verify_compacted tests pass (13 passed, 3 ignored). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../fetch_compacted_address_balances/v0/mod.rs | 9 +-------- .../prove_compacted_nullifier_changes/v0/mod.rs | 9 +-------- packages/rs-drive/src/util/common/mod.rs | 15 +++++++++++++++ .../v0/mod.rs | 9 +-------- .../verify_compacted_nullifier_changes/v0/mod.rs | 9 +-------- 5 files changed, 19 insertions(+), 32 deletions(-) diff --git a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs index 095d5365621..fa1c86f624f 100644 --- a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs +++ b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs @@ -1,5 +1,6 @@ use crate::drive::Drive; use crate::error::Error; +use crate::util::common::compacted_key; use dpp::address_funds::PlatformAddress; use dpp::balances::credits::BlockAwareCreditOperation; use dpp::ProtocolError; @@ -304,14 +305,6 @@ impl Drive { } } -/// Builds the 16-byte big-endian compacted key `(start_block, end_block)`. -fn compacted_key(start_block: u64, end_block: u64) -> Vec { - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block.to_be_bytes()); - key.extend_from_slice(&end_block.to_be_bytes()); - key -} - #[cfg(test)] mod tests { use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; diff --git a/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs index e4aeb51ae8b..8aca4a64705 100644 --- a/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/drive/shielded/prove/prove_compacted_nullifier_changes/v0/mod.rs @@ -1,19 +1,12 @@ use crate::drive::shielded::nullifiers::queries::shielded_compacted_nullifiers_path_vec; use crate::drive::Drive; use crate::error::Error; +use crate::util::common::compacted_key; use dpp::ProtocolError; use grovedb::query_result_type::QueryResultType; use grovedb::{PathQuery, Query, SizedQuery, TransactionArg}; use platform_version::version::PlatformVersion; -/// Builds the 16-byte big-endian compacted key `(start_block, end_block)`. -fn compacted_key(start_block: u64, end_block: u64) -> Vec { - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block.to_be_bytes()); - key.extend_from_slice(&end_block.to_be_bytes()); - key -} - impl Drive { /// Version 0 implementation for proving compacted nullifier changes. /// diff --git a/packages/rs-drive/src/util/common/mod.rs b/packages/rs-drive/src/util/common/mod.rs index 1c80d24e06b..c353174fa0b 100644 --- a/packages/rs-drive/src/util/common/mod.rs +++ b/packages/rs-drive/src/util/common/mod.rs @@ -1,2 +1,17 @@ pub mod decode; pub mod encode; + +/// Builds the 16-byte big-endian compacted-block key `(start_block, end_block)` +/// shared by the shielded-nullifier and address-balance compacted trees. +/// +/// This 16-byte boundary-key encoding is part of the chained-proof contract: +/// every prover and its matching verifier MUST construct keys identically, or +/// chained verification silently breaks. It therefore lives in exactly one +/// place (the four compacted prove/verify modules import it) so the encoding +/// cannot drift between them. +pub(crate) fn compacted_key(start_block: u64, end_block: u64) -> Vec { + let mut key = Vec::with_capacity(16); + key.extend_from_slice(&start_block.to_be_bytes()); + key.extend_from_slice(&end_block.to_be_bytes()); + key +} diff --git a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs index c74f5370733..cf060cc1abc 100644 --- a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs @@ -3,6 +3,7 @@ use crate::drive::RootTree; use crate::error::drive::DriveError; use crate::error::proof::ProofError; use crate::error::Error; +use crate::util::common::compacted_key; use crate::verify::RootHash; use dpp::address_funds::PlatformAddress; @@ -16,14 +17,6 @@ use std::collections::BTreeMap; use super::VerifiedCompactedAddressBalanceChanges; -/// Builds the 16-byte big-endian compacted key `(start_block, end_block)`. -fn compacted_key(start_block: u64, end_block: u64) -> Vec { - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block.to_be_bytes()); - key.extend_from_slice(&end_block.to_be_bytes()); - key -} - /// Path to the compacted address balances subtree: /// `[SavedBlockTransactions, COMPACTED_ADDRESS_BALANCES_KEY]`. fn compacted_address_balances_path() -> Vec> { diff --git a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs index f09cbd02754..f7ba5e05f8e 100644 --- a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs @@ -4,19 +4,12 @@ use crate::drive::Drive; use crate::error::drive::DriveError; use crate::error::proof::ProofError; use crate::error::Error; +use crate::util::common::compacted_key; use crate::verify::RootHash; use grovedb::query_result_type::PathKeyOptionalElementTrio; use grovedb::{GroveDb, PathQuery, Query, SizedQuery}; use platform_version::version::PlatformVersion; -/// Builds the 16-byte big-endian compacted key `(start_block, end_block)`. -fn compacted_key(start_block: u64, end_block: u64) -> Vec { - let mut key = Vec::with_capacity(16); - key.extend_from_slice(&start_block.to_be_bytes()); - key.extend_from_slice(&end_block.to_be_bytes()); - key -} - impl Drive { /// Verifies compacted nullifier changes proof. /// From 404f1beb1ad67c2d8fa72bf0416ed6af2b98ee67 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 14:41:41 +0200 Subject: [PATCH 07/11] fix(drive): compacted-proof review hardening (limit=0, malformed boundary, shared path/key) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the automated-review quick-wins on the compacted-proof code (the liveness bug itself remains deferred to the planned re-key): - fetch_compacted_address_balances: return an empty vec immediately when `limit == Some(0)` — previously a zero-limit caller still got one entry whenever start_block_height fell inside a compacted range (the limit was only enforced after the boundary probe). - Both verifiers: reject a malformed (non-16-byte) AUTHENTICATED boundary key by returning `None` from the chained-query generator (which fails verification) instead of silently degrading to the (start, start) forward-only lower bound and accepting an incomplete result set. - verify_compacted_address_balance_changes: drop the duplicated local `COMPACTED_ADDRESS_BALANCES_KEY_U8` const + `compacted_address_balances_path` fn (and the now-unused RootTree import); use the canonical `Drive::saved_compacted_block_transactions_address_balances_path_vec()` so proof gen, verification, and test inserts share one path contract. - fetch_compacted_address_balances: build the desc/asc query bounds with the shared `compacted_key` helper instead of manual byte builders. No behavior change on existing paths; 30 compacted tests pass (3 ignored liveness repros). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../v0/mod.rs | 14 +++++----- .../v0/mod.rs | 27 +++++++------------ .../v0/mod.rs | 12 ++++++--- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs index fa1c86f624f..2a658265428 100644 --- a/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs +++ b/packages/rs-drive/src/drive/saved_block_transactions/fetch_compacted_address_balances/v0/mod.rs @@ -52,11 +52,15 @@ impl Drive { let mut compacted_changes = Vec::new(); let limit_usize = limit.map(|l| l as usize); + // A zero limit can never include any entry — return empty before the + // boundary probe, which would otherwise still surface a containing range. + if limit_usize == Some(0) { + return Ok(compacted_changes); + } + // Query 1: Find if there's a range containing start_block_height // Query descending from (start_block_height, u64::MAX) with limit 1 - let mut desc_end_key = Vec::with_capacity(16); - desc_end_key.extend_from_slice(&start_block_height.to_be_bytes()); - desc_end_key.extend_from_slice(&u64::MAX.to_be_bytes()); + let desc_end_key = compacted_key(start_block_height, u64::MAX); let mut desc_query = Query::new_with_direction(false); // descending desc_query.insert_range_to_inclusive(..=desc_end_key); @@ -120,9 +124,7 @@ impl Drive { // Always use (start_block_height, 0) for consistent proof verification // The result may overlap with descending query if descending found a range // starting exactly at start_block_height - we dedupe below - let mut asc_start_key = Vec::with_capacity(16); - asc_start_key.extend_from_slice(&start_block_height.to_be_bytes()); - asc_start_key.extend_from_slice(&0u64.to_be_bytes()); + let asc_start_key = compacted_key(start_block_height, 0); let mut asc_query = Query::new(); asc_query.insert_range_from(asc_start_key..); diff --git a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs index cf060cc1abc..d9113901470 100644 --- a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs @@ -1,5 +1,4 @@ use crate::drive::Drive; -use crate::drive::RootTree; use crate::error::drive::DriveError; use crate::error::proof::ProofError; use crate::error::Error; @@ -7,8 +6,6 @@ use crate::util::common::compacted_key; use crate::verify::RootHash; use dpp::address_funds::PlatformAddress; -/// The subtree key for compacted address balances storage as u8 -const COMPACTED_ADDRESS_BALANCES_KEY_U8: u8 = b'c'; use dpp::balances::credits::BlockAwareCreditOperation; use grovedb::query_result_type::PathKeyOptionalElementTrio; use grovedb::{GroveDb, PathQuery, Query, SizedQuery}; @@ -17,15 +14,6 @@ use std::collections::BTreeMap; use super::VerifiedCompactedAddressBalanceChanges; -/// Path to the compacted address balances subtree: -/// `[SavedBlockTransactions, COMPACTED_ADDRESS_BALANCES_KEY]`. -fn compacted_address_balances_path() -> Vec> { - vec![ - vec![RootTree::SavedBlockTransactions as u8], - vec![COMPACTED_ADDRESS_BALANCES_KEY_U8], - ] -} - impl Drive { /// Verifies compacted address balance changes proof. /// @@ -78,7 +66,7 @@ impl Drive { .with_big_endian() .with_no_limit(); - let path = compacted_address_balances_path(); + let path = Drive::saved_compacted_block_transactions_address_balances_path_vec(); // Step 1: boundary query — authenticate the single greatest compacted // key <= (start_block_height, u64::MAX). Descending, limit 1. @@ -93,12 +81,17 @@ impl Drive { let forward_path = path.clone(); let generator = move |boundary_results: Vec| -> Option { + // The boundary query is limit-1, so there is at most one + // authenticated key. A non-16-byte key is tree/proof corruption — + // reject (return None, which fails the chained verification) rather + // than silently degrading to the (start, start) forward-only lower + // bound and accepting an incomplete result set. + if boundary_results.iter().any(|(_p, key, _e)| key.len() != 16) { + return None; + } let start_key = boundary_results .iter() .find_map(|(_path, key, _element)| { - if key.len() != 16 { - return None; - } let end_block = u64::from_be_bytes( key[8..16].try_into().expect("len checked to be 16"), ); @@ -373,7 +366,7 @@ mod tests { // Craft the MALICIOUS proof the OLD (vulnerable) way: prove // range_from((150, 150)..) directly. (100, 200) sorts before (150, 150) // and so appears only as a hash-only boundary node. - let path = compacted_address_balances_path(); + let path = Drive::saved_compacted_block_transactions_address_balances_path_vec(); let malicious_start_key = compacted_key(150, 150); let mut malicious_inner = Query::new(); malicious_inner.insert_range_from(malicious_start_key..); diff --git a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs index f7ba5e05f8e..875431d070a 100644 --- a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs @@ -78,12 +78,18 @@ impl Drive { // at or beyond it. Use the authenticated key directly in that case; // otherwise there is no containing range and we start at // (start_block_height, start_block_height). + // + // The boundary query is limit-1, so there is at most one + // authenticated key. A non-16-byte key is tree/proof corruption — + // reject (return None, which fails the chained verification) rather + // than silently degrading to the (start, start) forward-only lower + // bound and accepting an incomplete result set. + if boundary_results.iter().any(|(_p, key, _e)| key.len() != 16) { + return None; + } let start_key = boundary_results .iter() .find_map(|(_path, key, _element)| { - if key.len() != 16 { - return None; - } let end_block = u64::from_be_bytes( key[8..16].try_into().expect("len checked to be 16"), ); From 04097f11cf8754f1c863b43c0c1164d4d946efd6 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 14:58:50 +0200 Subject: [PATCH 08/11] fix(drive): make compacted address-balance path verify-available (fix WASM build) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression from 404f1beb: the compacted address-balance dedup pointed the verifier at `Drive::saved_compacted_block_transactions_address_balances_path_vec()`, which lives in a `server`-gated impl. The verifier is compiled under the `verify` feature only (that's how wasm-drive-verify builds rs-drive), so `cargo check -p drive --no-default-features --features verify` failed with E0599 — breaking the WASM/JS build. Move the canonical path into the `verify`-available `crate::util::common` (alongside `compacted_key`): the verifier uses it directly, and the server-side `saved_compacted_block_transactions_address_balances_path_vec` now delegates to it. So storage and verification still share one path definition (the reviewer's dedup intent) AND the verify-only build compiles. Verified: `cargo check -p drive --no-default-features --features verify` succeeds; `cargo test -p drive --features fixtures-and-mocks compacted` = 30 passed, 3 ignored, no warnings. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/drive/saved_block_transactions/queries.rs | 10 ++++++---- packages/rs-drive/src/util/common/mod.rs | 15 +++++++++++++++ .../v0/mod.rs | 6 +++--- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/rs-drive/src/drive/saved_block_transactions/queries.rs b/packages/rs-drive/src/drive/saved_block_transactions/queries.rs index 2b8556805d8..4c0684ae7e5 100644 --- a/packages/rs-drive/src/drive/saved_block_transactions/queries.rs +++ b/packages/rs-drive/src/drive/saved_block_transactions/queries.rs @@ -42,11 +42,13 @@ impl Drive { } /// Path to compacted address balances under saved block transactions. + /// + /// Delegates to the `verify`-available canonical + /// [`crate::util::common::compacted_address_balances_path`] so the + /// storage/fetch (server) path and the proof verifier (verify) path share a + /// single definition and cannot drift. pub fn saved_compacted_block_transactions_address_balances_path_vec() -> Vec> { - vec![ - vec![RootTree::SavedBlockTransactions as u8], - vec![COMPACTED_ADDRESS_BALANCES_KEY_U8], - ] + crate::util::common::compacted_address_balances_path() } /// Path to compacted address balances under saved block transactions. diff --git a/packages/rs-drive/src/util/common/mod.rs b/packages/rs-drive/src/util/common/mod.rs index c353174fa0b..bed1d84c68b 100644 --- a/packages/rs-drive/src/util/common/mod.rs +++ b/packages/rs-drive/src/util/common/mod.rs @@ -15,3 +15,18 @@ pub(crate) fn compacted_key(start_block: u64, end_block: u64) -> Vec { key.extend_from_slice(&end_block.to_be_bytes()); key } + +/// Path to the compacted address-balance subtree under `SavedBlockTransactions`. +/// +/// Lives here (a `verify`-available module) so the **server-side** storage/fetch +/// path and the **verify-side** proof verifier share one definition — the +/// subtree location is part of the proof contract and must not drift between +/// them. The byte must stay identical to +/// `saved_block_transactions::queries::COMPACTED_ADDRESS_BALANCES_KEY_U8` +/// (which is `server`-gated and so not referenceable from the verifier). +pub(crate) fn compacted_address_balances_path() -> Vec> { + vec![ + vec![crate::drive::RootTree::SavedBlockTransactions as u8], + vec![b'c'], // COMPACTED_ADDRESS_BALANCES_KEY_U8 + ] +} diff --git a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs index d9113901470..05c1b4bca84 100644 --- a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs @@ -2,7 +2,7 @@ use crate::drive::Drive; use crate::error::drive::DriveError; use crate::error::proof::ProofError; use crate::error::Error; -use crate::util::common::compacted_key; +use crate::util::common::{compacted_address_balances_path, compacted_key}; use crate::verify::RootHash; use dpp::address_funds::PlatformAddress; @@ -66,7 +66,7 @@ impl Drive { .with_big_endian() .with_no_limit(); - let path = Drive::saved_compacted_block_transactions_address_balances_path_vec(); + let path = compacted_address_balances_path(); // Step 1: boundary query — authenticate the single greatest compacted // key <= (start_block_height, u64::MAX). Descending, limit 1. @@ -366,7 +366,7 @@ mod tests { // Craft the MALICIOUS proof the OLD (vulnerable) way: prove // range_from((150, 150)..) directly. (100, 200) sorts before (150, 150) // and so appears only as a hash-only boundary node. - let path = Drive::saved_compacted_block_transactions_address_balances_path_vec(); + let path = compacted_address_balances_path(); let malicious_start_key = compacted_key(150, 150); let mut malicious_inner = Query::new(); malicious_inner.insert_range_from(malicious_start_key..); From 89e1d360fca6374104ffd531746adbe5bddf2713 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 15:06:13 +0200 Subject: [PATCH 09/11] fix(drive): classify malformed authenticated boundary key as CorruptedProof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on the malformed-boundary-key rejection from 404f1beb. The generator returned `None` on a non-16-byte authenticated key, which made `verify_query_with_chained_path_queries` fail with a generic GroveDB `InvalidInput` — a proof-derived condition surfaced as a non-proof error — and left the adjacent "generator always returns Some" destructure comment factually stale. Both verifiers now: the generator skips a malformed key in its `find_map` (so it still always returns `Some`, keeping the [boundary, forward] cardinality and its CorruptedCodeExecution invariant accurate), and the authenticated boundary key length is validated AFTER the chained verify on the returned `boundary_results`, returning `ProofError::CorruptedProof`. This gives the right user-facing error class for a malformed-key proof while preserving the "don't silently degrade to (start, start)" behavior. Comments updated to match. No behavior change on honest proofs; verify-only build compiles and verify_compacted tests pass (13 passed, 3 ignored). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../v0/mod.rs | 37 +++++++++++------- .../v0/mod.rs | 39 +++++++++++-------- 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs index 05c1b4bca84..166cb9c9d89 100644 --- a/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/address_funds/verify_compacted_address_balance_changes/v0/mod.rs @@ -81,17 +81,16 @@ impl Drive { let forward_path = path.clone(); let generator = move |boundary_results: Vec| -> Option { - // The boundary query is limit-1, so there is at most one - // authenticated key. A non-16-byte key is tree/proof corruption — - // reject (return None, which fails the chained verification) rather - // than silently degrading to the (start, start) forward-only lower - // bound and accepting an incomplete result set. - if boundary_results.iter().any(|(_p, key, _e)| key.len() != 16) { - return None; - } + // A non-16-byte authenticated key can't be parsed, so it is skipped + // here; the post-verification check below then rejects it as proof + // corruption rather than letting it silently degrade the forward + // lower bound to (start, start). let start_key = boundary_results .iter() .find_map(|(_path, key, _element)| { + if key.len() != 16 { + return None; + } let end_block = u64::from_be_bytes( key[8..16].try_into().expect("len checked to be 16"), ); @@ -120,18 +119,28 @@ impl Drive { &platform_version.drive.grove_version, )?; - // We pass exactly one chained (forward) generator that always returns - // `Some`, so GroveDB returns exactly two result sets: [boundary, forward]. - // A different cardinality can only be a GroveDB-internal invariant break - // (never caller-supplied proof corruption), so classify it as an internal - // code-execution error rather than `CorruptedProof`. - let [_boundary_results, forward_results]: [Vec; 2] = + // The generator always returns `Some`, so on success GroveDB returns + // exactly two result sets: [boundary, forward]. A different cardinality + // here can only be a GroveDB-internal invariant break (the generator + // never declines), so classify it as an internal code-execution error + // rather than `CorruptedProof`. + let [boundary_results, forward_results]: [Vec; 2] = results.try_into().map_err(|_| { Error::Drive(DriveError::CorruptedCodeExecution( "chained verification invariant: expected [boundary, forward] result sets", )) })?; + // The authenticated boundary key must be a 16-byte `(start, end)` compacted + // key. A non-16-byte key never occurs for honest state, so a proof + // presenting one is corrupt — reject it (the generator above skipped it and + // would otherwise have silently degraded the forward lower bound). + if boundary_results.iter().any(|(_p, key, _e)| key.len() != 16) { + return Err(Error::Proof(ProofError::CorruptedProof( + "authenticated compacted boundary key is not 16 bytes".to_string(), + ))); + } + // Process the verified forward results. let mut compacted_changes = Vec::new(); diff --git a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs index 875431d070a..77acd8d6aa6 100644 --- a/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs +++ b/packages/rs-drive/src/verify/shielded/verify_compacted_nullifier_changes/v0/mod.rs @@ -77,19 +77,16 @@ impl Drive { // The boundary key contains start_block_height when its end_block is // at or beyond it. Use the authenticated key directly in that case; // otherwise there is no containing range and we start at - // (start_block_height, start_block_height). - // - // The boundary query is limit-1, so there is at most one - // authenticated key. A non-16-byte key is tree/proof corruption — - // reject (return None, which fails the chained verification) rather - // than silently degrading to the (start, start) forward-only lower - // bound and accepting an incomplete result set. - if boundary_results.iter().any(|(_p, key, _e)| key.len() != 16) { - return None; - } + // (start_block_height, start_block_height). A non-16-byte key can't + // be parsed, so it is skipped here; the post-verification check + // below then rejects it as proof corruption rather than letting it + // silently degrade the forward lower bound. let start_key = boundary_results .iter() .find_map(|(_path, key, _element)| { + if key.len() != 16 { + return None; + } let end_block = u64::from_be_bytes( key[8..16].try_into().expect("len checked to be 16"), ); @@ -120,18 +117,28 @@ impl Drive { &platform_version.drive.grove_version, )?; - // We pass exactly one chained (forward) generator that always returns - // `Some`, so GroveDB returns exactly two result sets: [boundary, forward]. - // A different cardinality can only be a GroveDB-internal invariant break - // (never caller-supplied proof corruption), so classify it as an internal - // code-execution error rather than `CorruptedProof`. - let [_boundary_results, forward_results]: [Vec; 2] = + // The generator always returns `Some`, so on success GroveDB returns + // exactly two result sets: [boundary, forward]. A different cardinality + // here can only be a GroveDB-internal invariant break (the generator + // never declines), so classify it as an internal code-execution error + // rather than `CorruptedProof`. + let [boundary_results, forward_results]: [Vec; 2] = results.try_into().map_err(|_| { Error::Drive(DriveError::CorruptedCodeExecution( "chained verification invariant: expected [boundary, forward] result sets", )) })?; + // The authenticated boundary key must be a 16-byte `(start, end)` compacted + // key. A non-16-byte key never occurs for honest state, so a proof + // presenting one is corrupt — reject it (the generator above skipped it and + // would otherwise have silently degraded the forward lower bound). + if boundary_results.iter().any(|(_p, key, _e)| key.len() != 16) { + return Err(Error::Proof(ProofError::CorruptedProof( + "authenticated compacted boundary key is not 16 bytes".to_string(), + ))); + } + // Process the verified forward results. let mut compacted_changes = Vec::new(); From 3e20289cf283010f14da060e1c1cb16b59eafa6d Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 15:27:12 +0200 Subject: [PATCH 10/11] refactor(drive): single-source the compacted-address-balance subtree byte Review nit follow-up: `util::common::compacted_address_balances_path` (the verify-available canonical helper) hardcoded the subtree byte as the literal `b'c'` while the server-gated `saved_block_transactions::queries` defined its own `COMPACTED_ADDRESS_BALANCES_KEY_U8 = b'c'`. Two literals for one proof-contract byte: a rename/repurpose on the server side would update with `cargo check` while the verify-side helper silently kept `b'c'`, and chained verification would then break against the new server path. Move the canonical `COMPACTED_ADDRESS_BALANCES_KEY_U8` constant into `util::common` (compiled in both the `verify` and `server` feature sets) and have `saved_block_transactions::queries` re-export it with `pub use`. The byte is now written once; the verify-side path helper references the constant directly, and every existing import path is unchanged because the re-export flows through the pre-existing `pub use queries::*` (server callers, drive initialization, and rs-drive-abci all keep `saved_block_transactions::COMPACTED_ADDRESS_BALANCES_KEY_U8`). No behavior change. Verify-only and server builds both compile; verify_compacted tests pass (13 passed, 3 ignored). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../drive/saved_block_transactions/queries.rs | 10 +++++++-- packages/rs-drive/src/util/common/mod.rs | 22 +++++++++++++------ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/rs-drive/src/drive/saved_block_transactions/queries.rs b/packages/rs-drive/src/drive/saved_block_transactions/queries.rs index 4c0684ae7e5..6722e213235 100644 --- a/packages/rs-drive/src/drive/saved_block_transactions/queries.rs +++ b/packages/rs-drive/src/drive/saved_block_transactions/queries.rs @@ -10,8 +10,14 @@ pub const ADDRESS_BALANCES_KEY_U8: u8 = b'm'; /// The subtree key for address balances storage pub const COMPACTED_ADDRESS_BALANCES_KEY: &[u8; 1] = b"c"; -/// The subtree key for compacted address balances storage as u8 -pub const COMPACTED_ADDRESS_BALANCES_KEY_U8: u8 = b'c'; +/// The subtree key for compacted address balances storage as u8. +/// +/// Re-exported from the `verify`-available [`crate::util::common`] so the proof +/// verifier and this server-side storage path share a single definition of the +/// byte (it is part of the proof contract and must not drift). The downstream +/// import path (`saved_block_transactions::COMPACTED_ADDRESS_BALANCES_KEY_U8`, +/// via `pub use queries::*`) is unchanged. +pub use crate::util::common::COMPACTED_ADDRESS_BALANCES_KEY_U8; /// The subtree key for compacted addresses expiration time storage pub const COMPACTED_ADDRESSES_EXPIRATION_TIME_KEY: &[u8; 1] = b"e"; diff --git a/packages/rs-drive/src/util/common/mod.rs b/packages/rs-drive/src/util/common/mod.rs index bed1d84c68b..beaf31b76ec 100644 --- a/packages/rs-drive/src/util/common/mod.rs +++ b/packages/rs-drive/src/util/common/mod.rs @@ -16,17 +16,25 @@ pub(crate) fn compacted_key(start_block: u64, end_block: u64) -> Vec { key } +/// Canonical subtree key for the compacted address-balance tree under +/// `SavedBlockTransactions`. +/// +/// Defined here (a `verify`-available module) rather than in the `server`-gated +/// `saved_block_transactions::queries` so the **verify-side** proof verifier and +/// the **server-side** storage/fetch path reference one definition — the subtree +/// location is part of the proof contract and must not drift between them. +/// `saved_block_transactions::queries` re-exports this constant for its +/// server-side callers, so the `b'c'` byte is written in exactly one place. +pub const COMPACTED_ADDRESS_BALANCES_KEY_U8: u8 = b'c'; + /// Path to the compacted address-balance subtree under `SavedBlockTransactions`. /// -/// Lives here (a `verify`-available module) so the **server-side** storage/fetch -/// path and the **verify-side** proof verifier share one definition — the -/// subtree location is part of the proof contract and must not drift between -/// them. The byte must stay identical to -/// `saved_block_transactions::queries::COMPACTED_ADDRESS_BALANCES_KEY_U8` -/// (which is `server`-gated and so not referenceable from the verifier). +/// Shared by the server-side storage/fetch path and the verify-side proof +/// verifier; both sides reach it through this one helper. See +/// [`COMPACTED_ADDRESS_BALANCES_KEY_U8`] for why the byte lives in this module. pub(crate) fn compacted_address_balances_path() -> Vec> { vec![ vec![crate::drive::RootTree::SavedBlockTransactions as u8], - vec![b'c'], // COMPACTED_ADDRESS_BALANCES_KEY_U8 + vec![COMPACTED_ADDRESS_BALANCES_KEY_U8], ] } From 89b873fe01a4f2b6f2fcad5918b93f7ee83dc149 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 5 Jun 2026 15:51:31 +0200 Subject: [PATCH 11/11] refactor(drive): derive compacted-address-balance key slice from the single-sourced u8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the prior single-sourcing commit: the adjacent byte-slice form `COMPACTED_ADDRESS_BALANCES_KEY: &[u8; 1] = b"c"` still wrote the subtree byte independently of the now-single-sourced `COMPACTED_ADDRESS_BALANCES_KEY_U8`, partially undoing the goal — the slice could drift or be picked up by future server code with no compiler-enforced linkage to the u8 (and it has no consumer in-repo today). Derive the slice from the constant: `&[COMPACTED_ADDRESS_BALANCES_KEY_U8]`. The `b'c'` byte is now written in exactly one place (`util::common`); both the u8 re-export and the slice form resolve to it, so they cannot drift. Also corrects the slice's stale doc comment ("address balances" -> "compacted address balances"). Server-side only (the const lives in the `server`-gated `saved_block_transactions::queries`); no behavior change. Server and verify-only builds both compile. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/drive/saved_block_transactions/queries.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/rs-drive/src/drive/saved_block_transactions/queries.rs b/packages/rs-drive/src/drive/saved_block_transactions/queries.rs index 6722e213235..0e726fdda5b 100644 --- a/packages/rs-drive/src/drive/saved_block_transactions/queries.rs +++ b/packages/rs-drive/src/drive/saved_block_transactions/queries.rs @@ -7,8 +7,12 @@ pub const ADDRESS_BALANCES_KEY: &[u8; 1] = b"m"; /// The subtree key for address balances storage as u8 pub const ADDRESS_BALANCES_KEY_U8: u8 = b'm'; -/// The subtree key for address balances storage -pub const COMPACTED_ADDRESS_BALANCES_KEY: &[u8; 1] = b"c"; +/// The subtree key for compacted address balances storage. +/// +/// Derived from [`COMPACTED_ADDRESS_BALANCES_KEY_U8`] so the `b'c'` byte is +/// written in exactly one place (the `verify`-available `crate::util::common`) +/// and the slice form cannot drift from the u8. +pub const COMPACTED_ADDRESS_BALANCES_KEY: &[u8; 1] = &[COMPACTED_ADDRESS_BALANCES_KEY_U8]; /// The subtree key for compacted address balances storage as u8. ///