From 0fb96e3ca977f5bcef78641fa4d9b61bdb026ff7 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 18 Jun 2026 08:24:20 +0300 Subject: [PATCH 1/5] feat(sdk): implement document sum/average aggregation FFI (DOC-13/14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dash_sdk_document_sum` and `dash_sdk_document_average` were stubs returning NotImplemented, citing "blocked on grovedb PR 670". That block is stale — the pinned grovedb (fc81498) already exposes verify_aggregate_sum_query / verify_aggregate_count_and_sum_query, and the rs-drive sum/average query + verifiers and the rs-sdk DocumentSplitSums / DocumentSplitAverages `Fetch` impls are complete. Implement both FFI functions as mirrors of the working `dash_sdk_document_count`: build the DocumentQuery from the JSON params, select `SelectProjection::sum(field)` / `avg(field)`, fetch via DocumentSplitSums / DocumentSplitAverages, fold with the overflow-safe `try_into_flat_map`, and serialize: - sum -> {"sums":{"":}} - average -> {"averages":{"":{"count":,"sum":}}} Empty `sum_property` is rejected with InvalidParameter (the rs-sdk select assertions require a non-empty field). Three where/order/limit parsing helpers in count.rs are promoted to `pub(super)` and reused to keep cross-mode parsing identical. C ABI unchanged (signatures already existed); only doc comments changed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/document/queries/average.rs | 179 ++++++++++++++---- .../rs-sdk-ffi/src/document/queries/count.rs | 8 +- .../rs-sdk-ffi/src/document/queries/mod.rs | 7 +- .../rs-sdk-ffi/src/document/queries/sum.rs | 166 ++++++++++++---- 4 files changed, 276 insertions(+), 84 deletions(-) diff --git a/packages/rs-sdk-ffi/src/document/queries/average.rs b/packages/rs-sdk-ffi/src/document/queries/average.rs index f90f4591022..ddb5f862db4 100644 --- a/packages/rs-sdk-ffi/src/document/queries/average.rs +++ b/packages/rs-sdk-ffi/src/document/queries/average.rs @@ -7,30 +7,69 @@ //! exposes the raw pair (not a pre-divided average) so iOS/Swift //! callers can pick their own precision representation. //! -//! **Status**: skeleton — same gating as `dash_sdk_document_sum`. The -//! actual call into rs-sdk's -//! [`drive_proof_verifier::DocumentSplitAverages::fetch`] depends on -//! grovedb PR 670 landing `verify_aggregate_count_and_sum_query` and -//! the rs-drive executor bodies being filled in. Until then this entry -//! returns a typed `NotImplemented` error so iOS / Swift callers can -//! encode against the stable API and see a clear "feature not yet -//! shipped" rather than a crash. -//! -//! Once those dependencies land: -//! 1. Replace the `NotImplemented` error with a body mirroring -//! `dash_sdk_document_sum`. -//! 2. Substitute `DocumentSplitSums::fetch` → -//! `DocumentSplitAverages::fetch`. -//! 3. Keep the `sum_property` parameter (averages aggregate the same -//! integer property; the divisor is the implicit count). -//! 4. Return JSON of `{"averages": {"": {"count": u64, -//! "sum": i64}, ...}}` — callers divide. +//! Wraps the rs-sdk +//! [`drive_proof_verifier::DocumentSplitAverages::fetch`] flow. The +//! where / order_by / group_by / limit parameter handling is shared +//! verbatim with the count and sum surfaces (see [`super::count`]); the +//! only average-specific piece is the `(count, sum)` pair carried per +//! key in place of a single scalar. -use crate::{DashSDKError, DashSDKErrorCode, DashSDKResult, DataContractHandle, SDKHandle}; +use std::collections::BTreeMap; +use std::ffi::{CStr, CString}; use std::os::raw::c_char; +use dash_sdk::drive::query::SelectProjection; +use dash_sdk::platform::Fetch; +use drive_proof_verifier::DocumentSplitAverages; +use serde::Serialize; + +use super::count::{build_base_query, decode_ffi_limit, parse_group_by_json}; +use crate::sdk::SDKWrapper; +use crate::{ + DashSDKError, DashSDKErrorCode, DashSDKResult, DataContractHandle, FFIError, SDKHandle, +}; +use dash_sdk::dpp::prelude::DataContract; + +/// The verified `(count, sum)` pair for a single key. Callers divide +/// `sum / count` locally to obtain the average at their chosen +/// precision. `count` is unsigned (`u64`); `sum` is signed (`i64`) to +/// match grovedb's `SumValue = i64`. +#[derive(Debug, Serialize)] +struct AverageEntryJson { + count: u64, + sum: i64, +} + +#[derive(Debug, Serialize)] +struct DocumentAverageResult { + /// Per-key `(count, sum)` pairs. Keys are hex-encoded so iOS + /// callers can match them against the corresponding + /// platform-value-encoded property bytes. For aggregate-average + /// requests (empty / null `group_by_json`) this is a one-entry map + /// with an empty key. + averages: BTreeMap, +} + /// `SELECT AVG()` over a where clause + optional group_by. /// +/// Returns a JSON string of shape +/// `{"averages": {"": {"count": , "sum": }, ...}}`. +/// The raw `(count, sum)` pair is returned rather than a pre-divided +/// average so callers can pick their own precision representation +/// (`sum / count`). Hex keys correspond to the platform-value-encoded +/// property values from the underlying count+sum tree path. For +/// aggregate averages (empty/null `group_by_json`) the result is a +/// one-entry map with an empty key — `averages[""]` is the total +/// `(count, sum)`. +/// +/// Per-key result shapes mirror [`super::count::dash_sdk_document_count`] +/// exactly. Compound `(in_key, key)` entries are collapsed into a flat +/// map by summing each In-fork's `count` and `sum` at the same +/// terminator key; both axes use checked arithmetic (`u64` for count, +/// `i64` for sum) and surface overflow as an error rather than wrapping. +/// Callers needing the unmerged per-branch shape should use a richer +/// binding. +/// /// # Parameters /// - `sdk_handle`, `data_contract_handle`: valid non-null pointers. /// - `document_type`: NUL-terminated C string naming the document type. @@ -44,11 +83,17 @@ use std::os::raw::c_char; /// - `order_by_json`: NUL-terminated JSON `[{field, direction}]` or /// null. /// - `group_by_json`: NUL-terminated JSON `["", ...]` or null. -/// - `limit`: -1 for server default, >= 0 for explicit cap. +/// - `limit`: sentinel-encoded `int64` — `-1` for server default, +/// `> 0` for an explicit cap, `0` / `< -1` rejected. See +/// [`super::count::dash_sdk_document_count`] for the full contract. /// /// # Safety /// Same contract as [`super::sum::dash_sdk_document_sum`]. All /// pointers must be valid for the duration of the call. +/// - `sdk_handle` and `data_contract_handle` must be valid, non-null pointers. +/// - `document_type` and `sum_property` must be NUL-terminated C strings valid for the duration of the call. +/// - `where_json`, `order_by_json`, and `group_by_json` may be null; if non-null they must be NUL-terminated JSON strings. +/// - On success, returns a heap-allocated C string pointer; caller must free it using SDK routines. #[allow(clippy::too_many_arguments)] #[no_mangle] pub unsafe extern "C" fn dash_sdk_document_average( @@ -61,23 +106,79 @@ pub unsafe extern "C" fn dash_sdk_document_average( group_by_json: *const c_char, limit: i64, ) -> DashSDKResult { - let _ = ( - sdk_handle, - data_contract_handle, - document_type, - sum_property, - where_json, - order_by_json, - group_by_json, - limit, - ); - DashSDKResult::error(DashSDKError::new( - DashSDKErrorCode::NotImplemented, - "dash_sdk_document_average: not yet implemented. Waits on grovedb PR 670 (\ - verify_aggregate_count_and_sum_query) and the rs-drive count+sum executor \ - bodies in drive_document_sum_query/executors/. Same gating as \ - dash_sdk_document_sum — see the rs-drive `grovedb_pr_670` catalog module \ - for the full dependency list." - .to_string(), - )) + if sdk_handle.is_null() + || data_contract_handle.is_null() + || document_type.is_null() + || sum_property.is_null() + { + return DashSDKResult::error(DashSDKError::new( + DashSDKErrorCode::InvalidParameter, + "SDK handle, data contract handle, document type, or sum property is null".to_string(), + )); + } + + let wrapper = &*(sdk_handle as *const SDKWrapper); + let data_contract = &*(data_contract_handle as *const DataContract); + + let result: Result = wrapper.runtime.block_on(async { + let base_query = build_base_query(data_contract, document_type, where_json, order_by_json)?; + + let sum_property_str = CStr::from_ptr(sum_property) + .to_str() + .map_err(FFIError::from)?; + if sum_property_str.is_empty() { + return Err(FFIError::InvalidParameter( + "sum_property must name the integer property to average; got an empty string" + .to_string(), + )); + } + + let limit_u32 = decode_ffi_limit(limit)?; + + // `group_by_json` mirrors the wire's `repeated string` field + // one-to-one (see `dash_sdk_document_count`). The AVG + // projection carries the field directly — `SelectProjection::avg` + // is the average-side analog of count's `count_star`. + let group_by = parse_group_by_json(group_by_json)?; + let average_query = base_query + .with_select(SelectProjection::avg(sum_property_str)) + .with_group_by_fields(group_by) + .with_limit(limit_u32); + + // `DocumentSplitAverages::fetch` handles every average mode — + // for aggregate-average requests the result is a one-entry map + // with an empty key (so `result.averages[""]` is the total). + // `try_into_flat_map` collapses any compound (in_key + key) + // entries by summing each In-fork's count + sum at the same + // terminator key with checked arithmetic; callers needing the + // unmerged shape should use a richer binding. + let flat_averages = DocumentSplitAverages::fetch(&wrapper.sdk, average_query) + .await + .map_err(|e| FFIError::InternalError(format!("Failed to fetch average: {}", e)))? + .map(|s| s.try_into_flat_map()) + .transpose() + .map_err(|e| { + FFIError::InternalError(format!("Failed to flatten average result: {}", e)) + })? + .unwrap_or_default(); + + let averages: BTreeMap = flat_averages + .into_iter() + .map(|(k, (count, sum))| (hex::encode(k), AverageEntryJson { count, sum })) + .collect(); + + serde_json::to_string(&DocumentAverageResult { averages }) + .map_err(|e| FFIError::InternalError(format!("Failed to serialize result: {}", e))) + }); + + match result { + Ok(json) => match CString::new(json) { + Ok(s) => DashSDKResult::success_string(s.into_raw()), + Err(e) => DashSDKResult::error(DashSDKError::new( + DashSDKErrorCode::InternalError, + format!("Failed to create CString: {}", e), + )), + }, + Err(e) => DashSDKResult::error(e.into()), + } } diff --git a/packages/rs-sdk-ffi/src/document/queries/count.rs b/packages/rs-sdk-ffi/src/document/queries/count.rs index 366eb41449f..6b8223fcc1f 100644 --- a/packages/rs-sdk-ffi/src/document/queries/count.rs +++ b/packages/rs-sdk-ffi/src/document/queries/count.rs @@ -158,7 +158,9 @@ fn json_to_platform_value(json: serde_json::Value) -> Result { /// `GetDocumentsRequestV1` directly — no implicit translation, /// no transform, no SDK-internal helper between FFI and wire. #[allow(clippy::result_large_err)] -unsafe fn parse_group_by_json(group_by_json: *const c_char) -> Result, FFIError> { +pub(super) unsafe fn parse_group_by_json( + group_by_json: *const c_char, +) -> Result, FFIError> { if group_by_json.is_null() { return Ok(Vec::new()); } @@ -173,7 +175,7 @@ unsafe fn parse_group_by_json(group_by_json: *const c_char) -> Result Result { +pub(super) fn decode_ffi_limit(limit: i64) -> Result { match limit { -1 => Ok(0), // SDK-internal "unset" sentinel; maps to `None` on the V1 wire. n if n < -1 => Err(FFIError::InternalError(format!( diff --git a/packages/rs-sdk-ffi/src/document/queries/mod.rs b/packages/rs-sdk-ffi/src/document/queries/mod.rs index ceb4f742e57..71d0a2fabce 100644 --- a/packages/rs-sdk-ffi/src/document/queries/mod.rs +++ b/packages/rs-sdk-ffi/src/document/queries/mod.rs @@ -1,13 +1,14 @@ //! Document query operations -/// Average-side FFI entry. Same gating as `sum` — lights up alongside -/// grovedb PR 670's `AggregateCountAndSumOnRange`. +/// Average-side FFI entry (`dash_sdk_document_average`). Wraps the +/// rs-sdk `DocumentSplitAverages::fetch` flow. pub mod average; pub mod count; pub mod fetch; pub mod info; pub mod search; -/// Sum-side FFI entry. Skeleton — lights up alongside grovedb PR 670. +/// Sum-side FFI entry (`dash_sdk_document_sum`). Wraps the rs-sdk +/// `DocumentSplitSums::fetch` flow. pub mod sum; pub use count::dash_sdk_document_count; diff --git a/packages/rs-sdk-ffi/src/document/queries/sum.rs b/packages/rs-sdk-ffi/src/document/queries/sum.rs index c52907b7d7e..7160812da9f 100644 --- a/packages/rs-sdk-ffi/src/document/queries/sum.rs +++ b/packages/rs-sdk-ffi/src/document/queries/sum.rs @@ -2,31 +2,58 @@ //! for the sum surface — `SELECT SUM(sum_property)` over a where //! clause + optional group_by. //! -//! **Status**: skeleton. The actual call into rs-sdk's -//! [`drive_proof_verifier::DocumentSplitSums::fetch`] depends on -//! grovedb PR 670 landing `verify_aggregate_sum_query` and the -//! rs-drive executor bodies being filled in. Until then this entry -//! returns a typed `NotImplemented` error so iOS / Swift callers can -//! encode against the stable API and see a clear "feature not yet -//! shipped" rather than a crash. -//! -//! Once those dependencies land: -//! 1. Replace the `NotImplemented` error with a body mirroring -//! `dash_sdk_document_count` (see ~250 lines in `count.rs`). -//! 2. Substitute `DocumentSplitCounts::fetch` → -//! `DocumentSplitSums::fetch`. -//! 3. Add a `sum_property` parameter alongside `where_json` / -//! `order_by_json` / `group_by_json` — the property name to -//! aggregate (matches the `Select::field` in -//! `GetDocumentsRequestV1`). -//! 4. Return JSON of `{"sums": {"": , ...}}` -//! (signed to match grovedb's `SumValue = i64`). +//! Wraps the rs-sdk [`drive_proof_verifier::DocumentSplitSums::fetch`] +//! flow so callers can obtain document sums without constructing +//! `GetDocumentsRequest` v1 payloads directly. The where / order_by / +//! group_by / limit parameter handling is shared verbatim with the +//! count surface (see [`super::count`]); the only sum-specific pieces +//! are the `sum_property` field naming the integer property to +//! aggregate and the signed (`i64`) result values. -use crate::{DashSDKError, DashSDKErrorCode, DashSDKResult, DataContractHandle, SDKHandle}; +use std::collections::BTreeMap; +use std::ffi::{CStr, CString}; use std::os::raw::c_char; +use dash_sdk::drive::query::SelectProjection; +use dash_sdk::platform::Fetch; +use drive_proof_verifier::DocumentSplitSums; +use serde::Serialize; + +use super::count::{build_base_query, decode_ffi_limit, parse_group_by_json}; +use crate::sdk::SDKWrapper; +use crate::{ + DashSDKError, DashSDKErrorCode, DashSDKResult, DataContractHandle, FFIError, SDKHandle, +}; +use dash_sdk::dpp::prelude::DataContract; + +#[derive(Debug, Serialize)] +struct DocumentSumResult { + /// Per-key sums. Keys are hex-encoded so iOS callers can match + /// them against the corresponding platform-value-encoded property + /// bytes. For aggregate-sum requests (empty / null `group_by_json`) + /// this is a one-entry map with an empty key. Values are signed + /// (`i64`) to match grovedb's `SumValue = i64`. + sums: BTreeMap, +} + /// `SELECT SUM()` over a where clause + optional group_by. /// +/// Returns a JSON string of shape +/// `{"sums": {"": , ...}}`. Hex keys correspond +/// to the platform-value-encoded property values from the underlying +/// sum-tree path; iOS callers should hex-decode them and decode against +/// the contract's index-property type if they need a typed key. For +/// aggregate sums (empty/null `group_by_json`) the result is a one-entry +/// map with an empty key — `sums[""]` is the total. +/// +/// Per-key result shapes mirror [`super::count::dash_sdk_document_count`] +/// exactly (aggregate / per-`in_field` / per-`range_field` / +/// compound `(in_field, range_field)`). Compound `(in_key, key)` entries +/// are collapsed into a flat map by summing each In-fork's contribution +/// at the same terminator key; the fold uses checked `i64` arithmetic and +/// surfaces overflow as an error rather than wrapping. Callers needing +/// the unmerged per-branch shape should use a richer binding. +/// /// # Parameters /// - `sdk_handle`, `data_contract_handle`: valid non-null pointers. /// - `document_type`: NUL-terminated C string naming the document type. @@ -39,11 +66,17 @@ use std::os::raw::c_char; /// - `order_by_json`: NUL-terminated JSON `[{field, direction}]` or /// null. /// - `group_by_json`: NUL-terminated JSON `["", ...]` or null. -/// - `limit`: -1 for server default, >= 0 for explicit cap. +/// - `limit`: sentinel-encoded `int64` — `-1` for server default, +/// `> 0` for an explicit cap, `0` / `< -1` rejected. See +/// [`super::count::dash_sdk_document_count`] for the full contract. /// /// # Safety /// Same contract as [`super::count::dash_sdk_document_count`]. All /// pointers must be valid for the duration of the call. +/// - `sdk_handle` and `data_contract_handle` must be valid, non-null pointers. +/// - `document_type` and `sum_property` must be NUL-terminated C strings valid for the duration of the call. +/// - `where_json`, `order_by_json`, and `group_by_json` may be null; if non-null they must be NUL-terminated JSON strings. +/// - On success, returns a heap-allocated C string pointer; caller must free it using SDK routines. #[allow(clippy::too_many_arguments)] #[no_mangle] pub unsafe extern "C" fn dash_sdk_document_sum( @@ -56,22 +89,77 @@ pub unsafe extern "C" fn dash_sdk_document_sum( group_by_json: *const c_char, limit: i64, ) -> DashSDKResult { - let _ = ( - sdk_handle, - data_contract_handle, - document_type, - sum_property, - where_json, - order_by_json, - group_by_json, - limit, - ); - DashSDKResult::error(DashSDKError::new( - DashSDKErrorCode::NotImplemented, - "dash_sdk_document_sum: not yet implemented. Waits on grovedb PR 670 (\ - verify_aggregate_sum_query) and the rs-drive executor bodies in \ - drive_document_sum_query/executors/. See the rs-drive `grovedb_pr_670` \ - catalog module for the full dependency list." - .to_string(), - )) + if sdk_handle.is_null() + || data_contract_handle.is_null() + || document_type.is_null() + || sum_property.is_null() + { + return DashSDKResult::error(DashSDKError::new( + DashSDKErrorCode::InvalidParameter, + "SDK handle, data contract handle, document type, or sum property is null".to_string(), + )); + } + + let wrapper = &*(sdk_handle as *const SDKWrapper); + let data_contract = &*(data_contract_handle as *const DataContract); + + let result: Result = wrapper.runtime.block_on(async { + let base_query = build_base_query(data_contract, document_type, where_json, order_by_json)?; + + let sum_property_str = CStr::from_ptr(sum_property) + .to_str() + .map_err(FFIError::from)?; + if sum_property_str.is_empty() { + return Err(FFIError::InvalidParameter( + "sum_property must name the integer property to sum; got an empty string" + .to_string(), + )); + } + + let limit_u32 = decode_ffi_limit(limit)?; + + // `group_by_json` mirrors the wire's `repeated string` field + // one-to-one (see `dash_sdk_document_count`). The SUM + // projection carries the field directly — `SelectProjection::sum` + // is the sum-side analog of count's `count_star`. + let group_by = parse_group_by_json(group_by_json)?; + let sum_query = base_query + .with_select(SelectProjection::sum(sum_property_str)) + .with_group_by_fields(group_by) + .with_limit(limit_u32); + + // `DocumentSplitSums::fetch` handles every sum mode — for + // aggregate-sum requests the result is a one-entry map with an + // empty key (so `result.sums[""]` is the total). + // `try_into_flat_map` collapses any compound (in_key + key) + // entries by summing over `in_key` with checked `i64` + // arithmetic; callers needing the unmerged shape should use a + // richer binding. + let flat_sums = DocumentSplitSums::fetch(&wrapper.sdk, sum_query) + .await + .map_err(|e| FFIError::InternalError(format!("Failed to fetch sum: {}", e)))? + .map(|s| s.try_into_flat_map()) + .transpose() + .map_err(|e| FFIError::InternalError(format!("Failed to flatten sum result: {}", e)))? + .unwrap_or_default(); + + let sums: BTreeMap = flat_sums + .into_iter() + .map(|(k, v)| (hex::encode(k), v)) + .collect(); + + serde_json::to_string(&DocumentSumResult { sums }) + .map_err(|e| FFIError::InternalError(format!("Failed to serialize result: {}", e))) + }); + + match result { + Ok(json) => match CString::new(json) { + Ok(s) => DashSDKResult::success_string(s.into_raw()), + Err(e) => DashSDKResult::error(DashSDKError::new( + DashSDKErrorCode::InternalError, + format!("Failed to create CString: {}", e), + )), + }, + Err(e) => DashSDKResult::error(e.into()), + } } From 074c91bc55be068b3f7b6b5e746b3dc415804b63 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 18 Jun 2026 08:25:25 +0300 Subject: [PATCH 2/5] =?UTF-8?q?docs(swift-example-app):=20DOC-13/14=20?= =?UTF-8?q?=F0=9F=9A=AB=E2=86=92=F0=9F=94=8C=20(sum/avg=20FFI=20implemente?= =?UTF-8?q?d)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "blocked on grovedb PR 670" note was stale. With dash_sdk_document_sum / _average now implemented, DOC-13/14 are FFI-available (no app UI yet), so reclassify 🚫 → 🔌 and record the result shapes + the summable-index fixture requirement. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md b/packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md index e08a190cdf0..c4ae9a48608 100644 --- a/packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md +++ b/packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md @@ -215,8 +215,8 @@ The app is a full multi-wallet client: `PlatformWalletManager` holds N wallets c | DOC-10 | Aggregation — count documents (total) | Platform | Uncommon | 🧪 | **Count Documents** read view → Swift wrapper over FFI `dash_sdk_document_count` (proof-verified). Total count is `counts[""]` in the `{counts:{hexKey:u64}}` result. Requires a contract whose doc type sets `documentsCountable: true` (e.g. the `countable` QA fixture). | | DOC-11 | Aggregation — count documents, filtered (`where`) | Platform | Uncommon | 🧪 | Same Count view with a `where` clause → `dash_sdk_document_count(where_json=…)`. The filtered field must be a `countable` index. | | DOC-12 | Aggregation — count documents, grouped (`group_by`) | Platform | Uncommon | 🧪 | Same Count view with a `group_by` field → `dash_sdk_document_count(group_by_json=…)`; returns one count per group (hex-encoded group key → `u64`). | -| DOC-13 | Aggregation — sum of a numeric property | Platform | Uncommon | 🚫 | FFI `dash_sdk_document_sum` returns `NotImplemented` — blocked upstream on grovedb PR 670 (range/sum aggregate). Will need a `summable` index once unblocked. | -| DOC-14 | Aggregation — average of a numeric property | Platform | Uncommon | 🚫 | FFI `dash_sdk_document_average` returns `NotImplemented` — blocked upstream on grovedb PR 670. Will need a `summable` index once unblocked. | +| DOC-13 | Aggregation — sum of a numeric property | Platform | Uncommon | 🔌 | FFI `dash_sdk_document_sum` **implemented** (the grovedb PR 670 aggregate-sum capability is present; wraps rs-sdk `DocumentSplitSums::fetch`, proof-verified) → `{sums:{hexKey:i64}}`. No app UI yet; needs a contract doc type with a `summable` index on a numeric property. | +| DOC-14 | Aggregation — average of a numeric property | Platform | Uncommon | 🔌 | FFI `dash_sdk_document_average` **implemented** (wraps rs-sdk `DocumentSplitAverages::fetch`) → `{averages:{hexKey:{count,sum}}}` (caller divides). No app UI yet; needs a `countable`+`summable` index. | ### 4.8 Tokens — `Domain=Token` From 762f68d00cd5a61d4e76d261a3f289e48e85f47b Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 19 Jun 2026 00:00:43 +0300 Subject: [PATCH 3/5] fix(sdk): preserve typed SDK errors + test sum/average FFI (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address thepastaclaw review on the sum/average aggregation FFI: - Route the `.fetch(...)` errors through `FFIError::from` (the existing `SDKError(#[from] dash_sdk::Error)` classifier) instead of collapsing to InternalError, so Swift sees NetworkError/Timeout/etc. Applied to sum, average, AND count (aligning all three to avoid divergence). The `try_into_flat_map` overflow error stays InternalError (a data error, not transport). - Extract `validate_aggregation_property` (pure, testable) for the empty-field guard, used by both sum and average, and add unit tests: empty-field rejection + DocumentSumResult / DocumentAverageResult JSON shape round-trips (mirrors count.rs's decode_ffi_limit test style). - Drop the redundant "Same contract as […]" Safety prose, keeping the per-parameter bullets (cbindgen headers don't follow intra-doc links). C ABI unchanged. cargo build/test/clippy/fmt -p rs-sdk-ffi all clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/document/queries/average.rs | 35 ++++++--- .../rs-sdk-ffi/src/document/queries/count.rs | 2 +- .../rs-sdk-ffi/src/document/queries/sum.rs | 73 ++++++++++++++++--- 3 files changed, 91 insertions(+), 19 deletions(-) diff --git a/packages/rs-sdk-ffi/src/document/queries/average.rs b/packages/rs-sdk-ffi/src/document/queries/average.rs index ddb5f862db4..342cc96e4d3 100644 --- a/packages/rs-sdk-ffi/src/document/queries/average.rs +++ b/packages/rs-sdk-ffi/src/document/queries/average.rs @@ -24,6 +24,7 @@ use drive_proof_verifier::DocumentSplitAverages; use serde::Serialize; use super::count::{build_base_query, decode_ffi_limit, parse_group_by_json}; +use super::sum::validate_aggregation_property; use crate::sdk::SDKWrapper; use crate::{ DashSDKError, DashSDKErrorCode, DashSDKResult, DataContractHandle, FFIError, SDKHandle, @@ -88,8 +89,6 @@ struct DocumentAverageResult { /// [`super::count::dash_sdk_document_count`] for the full contract. /// /// # Safety -/// Same contract as [`super::sum::dash_sdk_document_sum`]. All -/// pointers must be valid for the duration of the call. /// - `sdk_handle` and `data_contract_handle` must be valid, non-null pointers. /// - `document_type` and `sum_property` must be NUL-terminated C strings valid for the duration of the call. /// - `where_json`, `order_by_json`, and `group_by_json` may be null; if non-null they must be NUL-terminated JSON strings. @@ -126,12 +125,7 @@ pub unsafe extern "C" fn dash_sdk_document_average( let sum_property_str = CStr::from_ptr(sum_property) .to_str() .map_err(FFIError::from)?; - if sum_property_str.is_empty() { - return Err(FFIError::InvalidParameter( - "sum_property must name the integer property to average; got an empty string" - .to_string(), - )); - } + validate_aggregation_property(sum_property_str)?; let limit_u32 = decode_ffi_limit(limit)?; @@ -154,7 +148,7 @@ pub unsafe extern "C" fn dash_sdk_document_average( // unmerged shape should use a richer binding. let flat_averages = DocumentSplitAverages::fetch(&wrapper.sdk, average_query) .await - .map_err(|e| FFIError::InternalError(format!("Failed to fetch average: {}", e)))? + .map_err(FFIError::from)? .map(|s| s.try_into_flat_map()) .transpose() .map_err(|e| { @@ -182,3 +176,26 @@ pub unsafe extern "C" fn dash_sdk_document_average( Err(e) => DashSDKResult::error(e.into()), } } + +#[cfg(test)] +mod tests { + //! Unit test for the average-side FFI wire shape. The empty-property + //! rejection is shared with the sum surface via + //! [`super::super::sum::validate_aggregation_property`] and tested + //! there; here we pin the `(count, sum)`-pair JSON shape iOS decodes. + + use super::*; + + /// The `DocumentAverageResult` wire shape iOS decodes is exactly + /// `{"averages": {"": {"count": , "sum": }, ...}}`. + /// The raw `(count, sum)` pair is returned un-divided so callers pick + /// their own precision; field order (`count` then `sum`) must stay + /// stable so the contract iOS decodes against doesn't drift. + #[test] + fn document_average_result_serializes_to_expected_shape() { + let averages = BTreeMap::from([("61".to_string(), AverageEntryJson { count: 3, sum: 30 })]); + let json = serde_json::to_string(&DocumentAverageResult { averages }) + .expect("DocumentAverageResult must serialize"); + assert_eq!(json, r#"{"averages":{"61":{"count":3,"sum":30}}}"#); + } +} diff --git a/packages/rs-sdk-ffi/src/document/queries/count.rs b/packages/rs-sdk-ffi/src/document/queries/count.rs index 6b8223fcc1f..b8508049863 100644 --- a/packages/rs-sdk-ffi/src/document/queries/count.rs +++ b/packages/rs-sdk-ffi/src/document/queries/count.rs @@ -394,7 +394,7 @@ pub unsafe extern "C" fn dash_sdk_document_count( // unmerged shape should use a richer binding. let split_counts = DocumentSplitCounts::fetch(&wrapper.sdk, count_query) .await - .map_err(|e| FFIError::InternalError(format!("Failed to fetch count: {}", e)))? + .map_err(FFIError::from)? .map(|s| s.into_flat_map()) .unwrap_or_default(); diff --git a/packages/rs-sdk-ffi/src/document/queries/sum.rs b/packages/rs-sdk-ffi/src/document/queries/sum.rs index 7160812da9f..e95a49588fa 100644 --- a/packages/rs-sdk-ffi/src/document/queries/sum.rs +++ b/packages/rs-sdk-ffi/src/document/queries/sum.rs @@ -26,6 +26,26 @@ use crate::{ }; use dash_sdk::dpp::prelude::DataContract; +/// Reject an empty aggregation-property name. +/// +/// The sum / average FFI entry points both require a non-empty +/// `sum_property` naming the integer property to aggregate; an empty +/// string is malformed input the server would reject. Extracted from +/// the async call sites so the rejection can be unit-tested without +/// standing up an SDK / data contract / runtime (mirrors +/// [`super::count::decode_ffi_limit`]). +#[allow(clippy::result_large_err)] +pub(super) fn validate_aggregation_property(prop: &str) -> Result<(), FFIError> { + if prop.is_empty() { + return Err(FFIError::InvalidParameter( + "aggregation property must name the integer property to \ + aggregate; got an empty string" + .to_string(), + )); + } + Ok(()) +} + #[derive(Debug, Serialize)] struct DocumentSumResult { /// Per-key sums. Keys are hex-encoded so iOS callers can match @@ -71,8 +91,6 @@ struct DocumentSumResult { /// [`super::count::dash_sdk_document_count`] for the full contract. /// /// # Safety -/// Same contract as [`super::count::dash_sdk_document_count`]. All -/// pointers must be valid for the duration of the call. /// - `sdk_handle` and `data_contract_handle` must be valid, non-null pointers. /// - `document_type` and `sum_property` must be NUL-terminated C strings valid for the duration of the call. /// - `where_json`, `order_by_json`, and `group_by_json` may be null; if non-null they must be NUL-terminated JSON strings. @@ -109,12 +127,7 @@ pub unsafe extern "C" fn dash_sdk_document_sum( let sum_property_str = CStr::from_ptr(sum_property) .to_str() .map_err(FFIError::from)?; - if sum_property_str.is_empty() { - return Err(FFIError::InvalidParameter( - "sum_property must name the integer property to sum; got an empty string" - .to_string(), - )); - } + validate_aggregation_property(sum_property_str)?; let limit_u32 = decode_ffi_limit(limit)?; @@ -137,7 +150,7 @@ pub unsafe extern "C" fn dash_sdk_document_sum( // richer binding. let flat_sums = DocumentSplitSums::fetch(&wrapper.sdk, sum_query) .await - .map_err(|e| FFIError::InternalError(format!("Failed to fetch sum: {}", e)))? + .map_err(FFIError::from)? .map(|s| s.try_into_flat_map()) .transpose() .map_err(|e| FFIError::InternalError(format!("Failed to flatten sum result: {}", e)))? @@ -163,3 +176,45 @@ pub unsafe extern "C" fn dash_sdk_document_sum( Err(e) => DashSDKResult::error(e.into()), } } + +#[cfg(test)] +mod tests { + //! Unit tests for the sum-side FFI surface that don't need an SDK / + //! data contract / runtime: the empty-property rejection (extracted + //! into [`validate_aggregation_property`]) and the exact JSON wire + //! shape iOS callers decode. Mirrors the `decode_ffi_limit` test + //! style at the bottom of [`super::super::count`]. + + use super::*; + + /// An empty aggregation property is malformed input and must be + /// rejected as [`FFIError::InvalidParameter`] (maps to + /// `DashSDKErrorCode::InvalidParameter` at the FFI boundary); a + /// non-empty name passes. + #[test] + fn validate_aggregation_property_rejects_empty_accepts_named() { + assert!( + matches!( + validate_aggregation_property(""), + Err(FFIError::InvalidParameter(_)) + ), + "empty property must be rejected as InvalidParameter" + ); + assert!( + validate_aggregation_property("amount").is_ok(), + "a named property must be accepted" + ); + } + + /// The `DocumentSumResult` wire shape iOS decodes is exactly + /// `{"sums": {"": , ...}}`. The empty key + /// (aggregate total) and a signed value must round-trip verbatim; + /// `BTreeMap` ordering keeps the key order deterministic. + #[test] + fn document_sum_result_serializes_to_expected_shape() { + let sums = BTreeMap::from([("".to_string(), 42i64), ("61".to_string(), -5i64)]); + let json = serde_json::to_string(&DocumentSumResult { sums }) + .expect("DocumentSumResult must serialize"); + assert_eq!(json, r#"{"sums":{"":42,"61":-5}}"#); + } +} From 71440d7d118f68c5fb7c5a791d890cee80972009 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 19 Jun 2026 00:08:20 +0300 Subject: [PATCH 4/5] docs(swift-example-app): fix stale DOC-13/14 cross-refs (CodeRabbit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After reclassifying DOC-13/14 🚫→🔌, two cross-references still said "blocked on grovedb PR 670": - Appendix A getDocuments RPC row: status ✅/🧪/🚫 → ✅/🧪/🔌, and "SUM/AVG are upstream-blocked (grovedb PR 670)" → "FFI-available, no app UI yet". - Appendix B: moved DOC-13/14 out of "🚫 Not implemented anywhere" into "🔌 SDK-only (FFI/wrapper exists, no UI)" (GRP-04 stays under 🚫). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md b/packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md index c4ae9a48608..7a816e041d5 100644 --- a/packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md +++ b/packages/swift-sdk/SwiftExampleApp/TEST_PLAN.md @@ -391,7 +391,7 @@ The complete Platform read surface, mapped to where each RPC is exercised in the ### Document | RPC | Tier | Status | Where | |---|---|---|---| -| getDocuments (incl. V1 COUNT/SUM/AVG, group_by, having) | Common | ✅ / 🧪 / 🚫 | `DocumentsView` / catalog. COUNT (total/`where`/`group_by`) now has a **Count Documents** read view — `DOC-10/11/12`. SUM/AVG are upstream-blocked (grovedb PR 670) — `DOC-13/14`. `having` is not exposed by the FFI. | +| getDocuments (incl. V1 COUNT/SUM/AVG, group_by, having) | Common | ✅ / 🧪 / 🔌 | `DocumentsView` / catalog. COUNT (total/`where`/`group_by`) now has a **Count Documents** read view — `DOC-10/11/12`. SUM/AVG are FFI-available (`DOC-13/14`) — no app UI yet. `having` is not exposed by the FFI. | | getDocumentHistory | Thorough | ✅ | catalog | ### Token @@ -476,10 +476,10 @@ For completeness (the "everything gRPC + Core can do" requirement), these exist **🔌 SDK-only (FFI/wrapper exists, no UI):** - `ADDR-05` address balance-change history (recent / compacted / branch / trunk) - `SH-11` create identity from shielded pool (Type 20) +- `DOC-13` document SUM aggregation (FFI `dash_sdk_document_sum`) +- `DOC-14` document AVERAGE aggregation (FFI `dash_sdk_document_average`) **🚫 Not implemented anywhere:** -- `DOC-13` document SUM aggregation — FFI stub returns `NotImplemented` (blocked on grovedb PR 670) -- `DOC-14` document AVERAGE aggregation — FFI stub returns `NotImplemented` (blocked on grovedb PR 670) - `GRP-04` standalone group lifecycle management - `getConsensusParams` (served via Tenderdash RPC, not the SDK) From d72375712383aee0cf3c5176627b89a9a924f4cf Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 19 Jun 2026 00:55:27 +0300 Subject: [PATCH 5/5] fix(sdk): route ungrouped sum/average through aggregate types (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit thepastaclaw (blocking): the FFI promised a single empty-key total for ungrouped sum/average, but always called DocumentSplitSums/Averages — the per-group surface (SplitSumEntry is one entry per matched group). For an `in`/range-carrier `where` with no group_by, that returns per-key entries, not the documented `{sums:{"":total}}`. Branch on group_by: - empty → DocumentSum::fetch / DocumentAverage::fetch (fold all verified entries to one total) → {"sums":{"":}} / {"averages":{"":{"count","sum"}}}; absent aggregate → 0 / (0,0). - non-empty → DocumentSplitSums/Averages::fetch + try_into_flat_map → per-group hex keys (unchanged). Aggregate fetch also uses .map_err(FFIError::from). Doc-comments now describe both shapes; 4 ungrouped-shape unit tests added. count.rs left as-is (its analogous behavior is pre-existing / out of scope). C ABI unchanged. cargo build/test (293)/clippy/fmt -p rs-sdk-ffi clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/document/queries/average.rs | 182 ++++++++++++----- .../rs-sdk-ffi/src/document/queries/sum.rs | 183 +++++++++++++----- 2 files changed, 262 insertions(+), 103 deletions(-) diff --git a/packages/rs-sdk-ffi/src/document/queries/average.rs b/packages/rs-sdk-ffi/src/document/queries/average.rs index 342cc96e4d3..865df2d2d59 100644 --- a/packages/rs-sdk-ffi/src/document/queries/average.rs +++ b/packages/rs-sdk-ffi/src/document/queries/average.rs @@ -7,12 +7,25 @@ //! exposes the raw pair (not a pre-divided average) so iOS/Swift //! callers can pick their own precision representation. //! -//! Wraps the rs-sdk -//! [`drive_proof_verifier::DocumentSplitAverages::fetch`] flow. The -//! where / order_by / group_by / limit parameter handling is shared -//! verbatim with the count and sum surfaces (see [`super::count`]); the -//! only average-specific piece is the `(count, sum)` pair carried per -//! key in place of a single scalar. +//! As with the sum surface, the result shape — and the rs-sdk type the +//! call routes to — depends on whether `group_by` is requested: +//! +//! - **Ungrouped** (empty / null `group_by_json`): the aggregate +//! [`drive_proof_verifier::DocumentAverage`] (a single folded +//! `(count, sum)`), surfaced as a one-entry map with the empty (`""`) +//! key — the documented "single total" contract. `DocumentAverage` +//! folds every verified branch (including an `in` / range fork in the +//! `where` clause) into one pair. (The per-group +//! `DocumentSplitAverages` view returns one entry *per matched +//! group/key* and is the wrong type for an ungrouped total.) +//! - **Grouped** (non-empty `group_by_json`): the per-group +//! [`drive_proof_verifier::DocumentSplitAverages`] view — one +//! hex-keyed entry per matched group, flattened by `try_into_flat_map`. +//! +//! Wraps those rs-sdk `Fetch` flows. The where / order_by / group_by / +//! limit parameter handling is shared verbatim with the count and sum +//! surfaces (see [`super::count`]); the only average-specific piece is +//! the `(count, sum)` pair carried per key in place of a single scalar. use std::collections::BTreeMap; use std::ffi::{CStr, CString}; @@ -20,7 +33,7 @@ use std::os::raw::c_char; use dash_sdk::drive::query::SelectProjection; use dash_sdk::platform::Fetch; -use drive_proof_verifier::DocumentSplitAverages; +use drive_proof_verifier::{DocumentAverage, DocumentSplitAverages}; use serde::Serialize; use super::count::{build_base_query, decode_ffi_limit, parse_group_by_json}; @@ -43,33 +56,45 @@ struct AverageEntryJson { #[derive(Debug, Serialize)] struct DocumentAverageResult { - /// Per-key `(count, sum)` pairs. Keys are hex-encoded so iOS - /// callers can match them against the corresponding - /// platform-value-encoded property bytes. For aggregate-average - /// requests (empty / null `group_by_json`) this is a one-entry map - /// with an empty key. + /// Average `(count, sum)` results keyed by group. + /// + /// - **Ungrouped** (empty / null `group_by_json`): exactly one + /// entry under the empty (`""`) key — the aggregate + /// `(count, sum)` folded across every matched branch. An absent / + /// `None` aggregate yields `{"": {"count": 0, "sum": 0}}` so the + /// "single empty-key total" contract always holds. + /// - **Grouped** (non-empty `group_by_json`): one entry per matched + /// group, hex-encoded so iOS callers can match them against the + /// corresponding platform-value-encoded property bytes. averages: BTreeMap, } /// `SELECT AVG()` over a where clause + optional group_by. /// /// Returns a JSON string of shape -/// `{"averages": {"": {"count": , "sum": }, ...}}`. -/// The raw `(count, sum)` pair is returned rather than a pre-divided -/// average so callers can pick their own precision representation -/// (`sum / count`). Hex keys correspond to the platform-value-encoded -/// property values from the underlying count+sum tree path. For -/// aggregate averages (empty/null `group_by_json`) the result is a -/// one-entry map with an empty key — `averages[""]` is the total -/// `(count, sum)`. +/// `{"averages": {"": {"count": , "sum": }, ...}}`. The +/// raw `(count, sum)` pair is returned rather than a pre-divided average +/// so callers can pick their own precision representation +/// (`sum / count`). The map's shape depends on whether `group_by_json` +/// is requested: /// -/// Per-key result shapes mirror [`super::count::dash_sdk_document_count`] -/// exactly. Compound `(in_key, key)` entries are collapsed into a flat -/// map by summing each In-fork's `count` and `sum` at the same -/// terminator key; both axes use checked arithmetic (`u64` for count, -/// `i64` for sum) and surface overflow as an error rather than wrapping. -/// Callers needing the unmerged per-branch shape should use a richer -/// binding. +/// - **Ungrouped** (empty/null `group_by_json`): exactly one entry under +/// the empty (`""`) key — `averages[""]` is the aggregate total +/// `(count, sum)`. This routes to the aggregate +/// [`drive_proof_verifier::DocumentAverage`], which folds every +/// verified branch (including any `in` / range fork in the `where` +/// clause) into one `(count, sum)`. An absent / `None` aggregate is +/// reported as `{"count": 0, "sum": 0}` so the single empty-key total +/// contract always holds. +/// - **Grouped** (non-empty `group_by_json`): one entry per matched +/// group, keyed by the hex-encoded platform-value-encoded property +/// value from the underlying count+sum tree path. This routes to the +/// per-group [`drive_proof_verifier::DocumentSplitAverages`] view. +/// Compound `(in_key, key)` entries are collapsed into a flat map by +/// summing each In-fork's `count` and `sum` at the same terminator +/// key; both axes use checked arithmetic (`u64` for count, `i64` for +/// sum) and surface overflow as an error rather than wrapping. Callers +/// needing the unmerged per-branch shape should use a richer binding. /// /// # Parameters /// - `sdk_handle`, `data_contract_handle`: valid non-null pointers. @@ -134,32 +159,57 @@ pub unsafe extern "C" fn dash_sdk_document_average( // projection carries the field directly — `SelectProjection::avg` // is the average-side analog of count's `count_star`. let group_by = parse_group_by_json(group_by_json)?; - let average_query = base_query - .with_select(SelectProjection::avg(sum_property_str)) - .with_group_by_fields(group_by) - .with_limit(limit_u32); - - // `DocumentSplitAverages::fetch` handles every average mode — - // for aggregate-average requests the result is a one-entry map - // with an empty key (so `result.averages[""]` is the total). - // `try_into_flat_map` collapses any compound (in_key + key) - // entries by summing each In-fork's count + sum at the same - // terminator key with checked arithmetic; callers needing the - // unmerged shape should use a richer binding. - let flat_averages = DocumentSplitAverages::fetch(&wrapper.sdk, average_query) - .await - .map_err(FFIError::from)? - .map(|s| s.try_into_flat_map()) - .transpose() - .map_err(|e| { - FFIError::InternalError(format!("Failed to flatten average result: {}", e)) - })? - .unwrap_or_default(); - - let averages: BTreeMap = flat_averages - .into_iter() - .map(|(k, (count, sum))| (hex::encode(k), AverageEntryJson { count, sum })) - .collect(); + + // Route on whether grouping was requested. The per-group + // `DocumentSplitAverages` view yields one entry *per matched + // group/key* — the wrong type for an ungrouped total (it would + // surface per-key entries for an `in` / range `where` clause + // instead of one folded `(count, sum)`). The aggregate + // `DocumentAverage` folds every verified branch into a single + // `(count, sum)`. + let averages: BTreeMap = if group_by.is_empty() { + // Ungrouped: aggregate `(count, sum)` under the empty (`""`) + // key. `DocumentAverage` carries named `count: u64` / `sum: + // i64` fields; a `None` fetch (queried-but-absent) reports + // the zero pair so the documented single empty-key total + // always holds. + let average_query = base_query + .with_select(SelectProjection::avg(sum_property_str)) + .with_limit(limit_u32); + + let DocumentAverage { count, sum } = + DocumentAverage::fetch(&wrapper.sdk, average_query) + .await + .map_err(FFIError::from)? + .unwrap_or(DocumentAverage { count: 0, sum: 0 }); + + BTreeMap::from([(String::new(), AverageEntryJson { count, sum })]) + } else { + // Grouped: one hex-keyed entry per matched group. + // `try_into_flat_map` collapses any compound (in_key + key) + // entries by summing each In-fork's count + sum at the same + // terminator key with checked arithmetic; callers needing + // the unmerged shape should use a richer binding. + let average_query = base_query + .with_select(SelectProjection::avg(sum_property_str)) + .with_group_by_fields(group_by) + .with_limit(limit_u32); + + let flat_averages = DocumentSplitAverages::fetch(&wrapper.sdk, average_query) + .await + .map_err(FFIError::from)? + .map(|s| s.try_into_flat_map()) + .transpose() + .map_err(|e| { + FFIError::InternalError(format!("Failed to flatten average result: {}", e)) + })? + .unwrap_or_default(); + + flat_averages + .into_iter() + .map(|(k, (count, sum))| (hex::encode(k), AverageEntryJson { count, sum })) + .collect() + }; serde_json::to_string(&DocumentAverageResult { averages }) .map_err(|e| FFIError::InternalError(format!("Failed to serialize result: {}", e))) @@ -198,4 +248,30 @@ mod tests { .expect("DocumentAverageResult must serialize"); assert_eq!(json, r#"{"averages":{"61":{"count":3,"sum":30}}}"#); } + + /// The ungrouped (aggregate) branch always emits a single + /// empty-string-keyed `(count, sum)` total. This pins the exact + /// wire shape the `group_by.is_empty()` path produces — + /// `{"averages":{"":{"count":..,"sum":..}}}` — so a regression that + /// reintroduced per-key entries for an ungrouped `in` / range query + /// would change the serialized bytes iOS decodes against. + #[test] + fn document_average_result_ungrouped_is_single_empty_key_total() { + let averages = BTreeMap::from([(String::new(), AverageEntryJson { count: 3, sum: 30 })]); + let json = serde_json::to_string(&DocumentAverageResult { averages }) + .expect("DocumentAverageResult must serialize"); + assert_eq!(json, r#"{"averages":{"":{"count":3,"sum":30}}}"#); + } + + /// An absent / `None` aggregate (queried-but-empty) is reported as + /// the zero `(count, sum)` pair under the empty key, never an empty + /// map, so the documented "single empty-key total" contract always + /// holds. + #[test] + fn document_average_result_ungrouped_absent_is_zero_pair() { + let averages = BTreeMap::from([(String::new(), AverageEntryJson { count: 0, sum: 0 })]); + let json = serde_json::to_string(&DocumentAverageResult { averages }) + .expect("DocumentAverageResult must serialize"); + assert_eq!(json, r#"{"averages":{"":{"count":0,"sum":0}}}"#); + } } diff --git a/packages/rs-sdk-ffi/src/document/queries/sum.rs b/packages/rs-sdk-ffi/src/document/queries/sum.rs index e95a49588fa..6bcde8b5efb 100644 --- a/packages/rs-sdk-ffi/src/document/queries/sum.rs +++ b/packages/rs-sdk-ffi/src/document/queries/sum.rs @@ -2,13 +2,28 @@ //! for the sum surface — `SELECT SUM(sum_property)` over a where //! clause + optional group_by. //! -//! Wraps the rs-sdk [`drive_proof_verifier::DocumentSplitSums::fetch`] -//! flow so callers can obtain document sums without constructing -//! `GetDocumentsRequest` v1 payloads directly. The where / order_by / -//! group_by / limit parameter handling is shared verbatim with the -//! count surface (see [`super::count`]); the only sum-specific pieces -//! are the `sum_property` field naming the integer property to -//! aggregate and the signed (`i64`) result values. +//! The result shape depends on whether `group_by` is requested, and +//! the call routes to a *different* rs-sdk type for each: +//! +//! - **Ungrouped** (empty / null `group_by_json`): the aggregate +//! [`drive_proof_verifier::DocumentSum`] (a single folded `i64`), +//! surfaced as a one-entry map with the empty (`""`) key — the +//! documented "single total" contract. This holds even when the +//! `where` clause carries an `in` / range fork: `DocumentSum` folds +//! every verified branch into one total. (The per-group +//! `DocumentSplitSums` view returns one entry *per matched group/key* +//! and is the wrong type for an ungrouped total.) +//! - **Grouped** (non-empty `group_by_json`): the per-group +//! [`drive_proof_verifier::DocumentSplitSums`] view — one hex-keyed +//! entry per matched group, flattened by `try_into_flat_map`. +//! +//! Wraps those rs-sdk `Fetch` flows so callers can obtain document +//! sums without constructing `GetDocumentsRequest` v1 payloads +//! directly. The where / order_by / group_by / limit parameter +//! handling is shared verbatim with the count surface (see +//! [`super::count`]); the only sum-specific pieces are the +//! `sum_property` field naming the integer property to aggregate and +//! the signed (`i64`) result values. use std::collections::BTreeMap; use std::ffi::{CStr, CString}; @@ -16,7 +31,7 @@ use std::os::raw::c_char; use dash_sdk::drive::query::SelectProjection; use dash_sdk::platform::Fetch; -use drive_proof_verifier::DocumentSplitSums; +use drive_proof_verifier::{DocumentSplitSums, DocumentSum}; use serde::Serialize; use super::count::{build_base_query, decode_ffi_limit, parse_group_by_json}; @@ -48,31 +63,49 @@ pub(super) fn validate_aggregation_property(prop: &str) -> Result<(), FFIError> #[derive(Debug, Serialize)] struct DocumentSumResult { - /// Per-key sums. Keys are hex-encoded so iOS callers can match - /// them against the corresponding platform-value-encoded property - /// bytes. For aggregate-sum requests (empty / null `group_by_json`) - /// this is a one-entry map with an empty key. Values are signed - /// (`i64`) to match grovedb's `SumValue = i64`. + /// Sum results keyed by group. + /// + /// - **Ungrouped** (empty / null `group_by_json`): exactly one + /// entry under the empty (`""`) key — the aggregate total folded + /// across every matched branch. An absent / `None` aggregate + /// yields `{"": 0}` so the "single empty-key total" contract + /// always holds. + /// - **Grouped** (non-empty `group_by_json`): one entry per matched + /// group, hex-encoded so iOS callers can match them against the + /// corresponding platform-value-encoded property bytes. + /// + /// Values are signed (`i64`) to match grovedb's `SumValue = i64`. sums: BTreeMap, } /// `SELECT SUM()` over a where clause + optional group_by. /// /// Returns a JSON string of shape -/// `{"sums": {"": , ...}}`. Hex keys correspond -/// to the platform-value-encoded property values from the underlying -/// sum-tree path; iOS callers should hex-decode them and decode against -/// the contract's index-property type if they need a typed key. For -/// aggregate sums (empty/null `group_by_json`) the result is a one-entry -/// map with an empty key — `sums[""]` is the total. +/// `{"sums": {"": , ...}}`. The map's shape depends +/// on whether `group_by_json` is requested: +/// +/// - **Ungrouped** (empty/null `group_by_json`): exactly one entry under +/// the empty (`""`) key — `sums[""]` is the aggregate total. This +/// routes to the aggregate [`drive_proof_verifier::DocumentSum`], +/// which folds every verified branch (including any `in` / range fork +/// in the `where` clause) into one `i64`. An absent / `None` +/// aggregate is reported as `{"": 0}` so the single empty-key total +/// contract always holds. +/// - **Grouped** (non-empty `group_by_json`): one entry per matched +/// group, keyed by the hex-encoded platform-value-encoded property +/// value from the underlying sum-tree path; iOS callers should +/// hex-decode them and decode against the contract's index-property +/// type if they need a typed key. This routes to the per-group +/// [`drive_proof_verifier::DocumentSplitSums`] view. Compound +/// `(in_key, key)` entries are collapsed into a flat map by summing +/// each In-fork's contribution at the same terminator key; the fold +/// uses checked `i64` arithmetic and surfaces overflow as an error +/// rather than wrapping. Callers needing the unmerged per-branch shape +/// should use a richer binding. /// -/// Per-key result shapes mirror [`super::count::dash_sdk_document_count`] -/// exactly (aggregate / per-`in_field` / per-`range_field` / -/// compound `(in_field, range_field)`). Compound `(in_key, key)` entries -/// are collapsed into a flat map by summing each In-fork's contribution -/// at the same terminator key; the fold uses checked `i64` arithmetic and -/// surfaces overflow as an error rather than wrapping. Callers needing -/// the unmerged per-branch shape should use a richer binding. +/// (The analogous ungrouped-vs-grouped distinction exists for +/// [`super::count::dash_sdk_document_count`], which routes both modes +/// through `DocumentSplitCounts`.) /// /// # Parameters /// - `sdk_handle`, `data_contract_handle`: valid non-null pointers. @@ -136,30 +169,55 @@ pub unsafe extern "C" fn dash_sdk_document_sum( // projection carries the field directly — `SelectProjection::sum` // is the sum-side analog of count's `count_star`. let group_by = parse_group_by_json(group_by_json)?; - let sum_query = base_query - .with_select(SelectProjection::sum(sum_property_str)) - .with_group_by_fields(group_by) - .with_limit(limit_u32); - - // `DocumentSplitSums::fetch` handles every sum mode — for - // aggregate-sum requests the result is a one-entry map with an - // empty key (so `result.sums[""]` is the total). - // `try_into_flat_map` collapses any compound (in_key + key) - // entries by summing over `in_key` with checked `i64` - // arithmetic; callers needing the unmerged shape should use a - // richer binding. - let flat_sums = DocumentSplitSums::fetch(&wrapper.sdk, sum_query) - .await - .map_err(FFIError::from)? - .map(|s| s.try_into_flat_map()) - .transpose() - .map_err(|e| FFIError::InternalError(format!("Failed to flatten sum result: {}", e)))? - .unwrap_or_default(); - - let sums: BTreeMap = flat_sums - .into_iter() - .map(|(k, v)| (hex::encode(k), v)) - .collect(); + + // Route on whether grouping was requested. The per-group + // `DocumentSplitSums` view yields one entry *per matched + // group/key* — so it is the wrong type for an ungrouped total + // (it would surface per-key entries for an `in` / range `where` + // clause instead of one folded total). The aggregate + // `DocumentSum` folds every verified branch into a single `i64`. + let sums: BTreeMap = if group_by.is_empty() { + // Ungrouped: aggregate total under the empty (`""`) key. + // `DocumentSum` is a tuple struct over a single folded `i64`; + // a `None` fetch (queried-but-absent) reports the zero total + // so the documented single empty-key total always holds. + let sum_query = base_query + .with_select(SelectProjection::sum(sum_property_str)) + .with_limit(limit_u32); + + let total = DocumentSum::fetch(&wrapper.sdk, sum_query) + .await + .map_err(FFIError::from)? + .map(|s| s.0) + .unwrap_or(0); + + BTreeMap::from([(String::new(), total)]) + } else { + // Grouped: one hex-keyed entry per matched group. + // `try_into_flat_map` collapses any compound (in_key + key) + // entries by summing over `in_key` with checked `i64` + // arithmetic; callers needing the unmerged shape should use + // a richer binding. + let sum_query = base_query + .with_select(SelectProjection::sum(sum_property_str)) + .with_group_by_fields(group_by) + .with_limit(limit_u32); + + let flat_sums = DocumentSplitSums::fetch(&wrapper.sdk, sum_query) + .await + .map_err(FFIError::from)? + .map(|s| s.try_into_flat_map()) + .transpose() + .map_err(|e| { + FFIError::InternalError(format!("Failed to flatten sum result: {}", e)) + })? + .unwrap_or_default(); + + flat_sums + .into_iter() + .map(|(k, v)| (hex::encode(k), v)) + .collect() + }; serde_json::to_string(&DocumentSumResult { sums }) .map_err(|e| FFIError::InternalError(format!("Failed to serialize result: {}", e))) @@ -217,4 +275,29 @@ mod tests { .expect("DocumentSumResult must serialize"); assert_eq!(json, r#"{"sums":{"":42,"61":-5}}"#); } + + /// The ungrouped (aggregate) branch always emits a single + /// empty-string-keyed total. This pins the exact wire shape the + /// `group_by.is_empty()` path produces — `{"sums":{"":}}` — + /// so a regression that reintroduced per-key entries for an + /// ungrouped `in` / range query would change the serialized bytes + /// iOS decodes against. + #[test] + fn document_sum_result_ungrouped_is_single_empty_key_total() { + let sums = BTreeMap::from([(String::new(), 42i64)]); + let json = serde_json::to_string(&DocumentSumResult { sums }) + .expect("DocumentSumResult must serialize"); + assert_eq!(json, r#"{"sums":{"":42}}"#); + } + + /// An absent / `None` aggregate (queried-but-empty) is reported as + /// the zero total under the empty key, never an empty map, so the + /// documented "single empty-key total" contract always holds. + #[test] + fn document_sum_result_ungrouped_absent_is_zero_total() { + let sums = BTreeMap::from([(String::new(), 0i64)]); + let json = serde_json::to_string(&DocumentSumResult { sums }) + .expect("DocumentSumResult must serialize"); + assert_eq!(json, r#"{"sums":{"":0}}"#); + } }