Skip to content

feat(platform): add GetDocumentsCount and GetDocumentsSplitCount queries - #3435

Merged
QuantumExplorer merged 26 commits into
v3.1-devfrom
feat/documents-count-query
May 9, 2026
Merged

feat(platform): add GetDocumentsCount and GetDocumentsSplitCount queries#3435
QuantumExplorer merged 26 commits into
v3.1-devfrom
feat/documents-count-query

Conversation

@QuantumExplorer

@QuantumExplorerQuantumExplorer commented Apr 4, 2026

Copy link
Copy Markdown
Member

Summary

Adds two new gRPC endpoints for O(1)/O(log n) document counts on countable indexes (the documentsCountable / rangeCountable flags introduced in #2516):

  • GetDocumentsCount — total count of documents matching where clauses
  • GetDocumentsSplitCount — per-key counts split by an index property

Full stack from .proto → drive-abci handlers → rs-drive count machinery → proof verifier → rs-sdk Fetch traits → wasm-sdk + rs-sdk-ffi bindings, plus a book chapter explaining the design.

Layers touched

LayerPackageWhat
Protodapi-grpcMessage definitions + RPC service entries
Codegendapi-grpc/build.rsRegister versioned request/response types
Server proxyrs-dapidrive_method! passthrough for both endpoints
Query handlersrs-drive-abciVersion dispatch + v0 implementations (no-prove fast path + prove path)
Drive queryrs-driveDriveDocumentCountQuery — count-tree walker, supports Equal and In operators
Proof verifierrs-drive-proof-verifierFromProof implementations for DocumentCount and DocumentSplitCounts
Rust SDKrs-sdkDocumentCountQuery / DocumentSplitCountQuery + Fetch impls
WASM SDKwasm-sdkgetDocumentsCount / getDocumentsSplitCount (+ proof-info variants)
FFIrs-sdk-ffiC bindings for iOS / Swift
Versioningrs-platform-versionQuery version bounds
Docsbook/New "Document Count Trees" chapter under Drive

Two paths under the hood

  • prove=false (fast path)DriveDocumentCountQuery walks the countable index level by level, doing point-lookups against CountTree elements. Single-prefix query → O(1); partial prefix → O(distinct values at the unbound levels). Supports Equal (one path) and In (cartesian fork over the listed values, deduped by serialized key); range operators are rejected upfront with a clear error since the current PathQuery model can't express the boundary walk.
  • prove=true (proof path) — drive-abci returns a standard DriveDocumentQuery proof of the matching documents themselves (no signed-count primitive on the wire today). The client verifies and aggregates locally: documents.len() as u64 for total counts; per-key serialize_value_for_key-bucketed counts for split. Capped at u16::MAX matching documents per request to keep response size bounded — beyond that, callers must use the no-prove fast path with a covering countable index.

Notable fixes during review

  • Split-count proof verifier was returning Some(BTreeMap::new()) for any verified result set. Replaced with a dedicated maybe_from_proof_with_split_property entry point that takes the split property explicitly and aggregates verified documents; the generic FromProof<DriveDocumentQuery> impl now errors loudly so a caller can't silently get an empty map.
  • In deduplicationage in [30, 30] would visit the same subtree twice; serialized keys are now tracked in a BTreeSet before forking.
  • SDK limit truncationDocumentCountQuery / DocumentSplitCountQueryDriveDocumentQuery now force limit = None. Fix applied at the SDK layer covers WASM (which defaulted DocumentQuery.limit to 100) plus FFI and direct rs-sdk callers.
  • Server prove path bounded — handler now caps DriveDocumentQuery.limit at u16::MAX rather than clearing it, so a prove=true request can't materialize an unbounded document set.

Test plan

  • cargo check --workspace clean
  • cargo clippy --workspace --all-features -- -D warnings clean (toolchain 1.92)
  • cargo fmt --all applied
  • 30 new tests across the stack:
  • All affected packages green (rs-drive lib: 3075 tests; drive-abci count + split-count handlers: 11 tests; drive-proof-verifier: 217 tests)
  • CI macOS run-time fits in 30-min budget (re-run pending after a file-rename caused a one-time cache invalidation)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added getDocumentsCount API endpoint to retrieve document counts with optional proof generation.
    • Added getDocumentsSplitCount API endpoint to retrieve document counts split by a specified property with optional proof generation.
    • Extended platform client libraries across Java, Objective-C, Python, and Web/TypeScript to support the new endpoints.
  • Tests

    • Added unit tests covering document count and split count queries with and without proof generation.

QuantumExplorerand others added 2 commits April 4, 2026 12:15
Add two new gRPC endpoints for querying document counts from countable
indices:
- GetDocumentsCount: returns a total count matching where clauses
- GetDocumentsSplitCount: returns per-key counts split by an index property
Both support proof responses for cryptographic verification.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t queries
Full stack implementation of two new gRPC endpoints for querying
document counts from countable indices:
- GetDocumentsCount: returns total count matching where clauses
- GetDocumentsSplitCount: returns per-key counts split by index property
Changes across all layers:
- dapi-grpc: register versioned request/response types
- rs-dapi: add drive_method proxy for both endpoints
- rs-drive-abci: query handlers with version dispatch
- rs-dapi-client: transport request mappings
- rs-drive-proof-verifier: FromProof implementations
- rs-platform-version: version bounds for both query types
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actionsgithub-actionsBot added this to the v3.1.0 milestone Apr 4, 2026
@coderabbitai

coderabbitaiBot commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds platform RPCs for document counting (per-query and split-by-property), proto messages, server transport bindings, dAPI service handlers, Drive ABCI query implementations (v0) with tests, proof-verifier types, platform-version bounds, and generated client bindings across Java, Objective‑C, Python, and Web; also adds getMostRecentShieldedAnchor RPC and minor proto field.

Changes

Cohort / File(s)Summary
Proto Definitions & Build
packages/dapi-grpc/protos/platform/v0/platform.proto, packages/dapi-grpc/build.rs
Added RPCs getDocumentsCount / getDocumentsSplitCount and their request/response messages; also added getMostRecentShieldedAnchor and a small proto field change; updated build-time versioned request/response arrays to include new message types.
Rust Transport & dAPI Service
packages/rs-dapi-client/src/transport/grpc.rs, packages/rs-dapi/src/services/platform_service/mod.rs
Added TransportRequest bindings and Platform service methods wiring new gRPC endpoints to drive-backed handlers.
Drive ABCI Query Modules
packages/rs-drive-abci/src/query/document_count_query/..., packages/rs-drive-abci/src/query/document_split_count_query/..., packages/rs-drive-abci/src/query/mod.rs, packages/rs-drive-abci/src/query/service.rs
Added query dispatchers and V0 implementations: version validation, contract/type resolution, CBOR where parsing, query execution (with/without proof), split-by-property aggregation, metadata handling, and unit tests; registered submodules and added QueryService endpoints.
Proof Verifier (Rust)
packages/rs-drive-proof-verifier/src/proof.rs, .../proof/document_count.rs, .../proof/document_split_count.rs, packages/rs-drive-proof-verifier/src/lib.rs
Added DocumentCount and DocumentSplitCounts types with FromProof implementations; re-exported new types from crate root.
Platform Versioning & Mocks
packages/rs-platform-version/src/version/.../mod.rs, .../v1.rs, .../mocks/v2_test.rs
Added document_count_query and document_split_count_query FeatureVersionBounds to version structs/constants and test mocks (initialized to 0).
Client Bindings — Java
packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java
Generated Java gRPC stubs/server skeleton additions for getDocumentsCount, getDocumentsSplitCount, getMostRecentShieldedAnchor; updated method descriptors, stubs, and service descriptor.
Client Bindings — Objective‑C
packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.{h,m}, .../Platform.pbrpc.{h,m}
Added Objective‑C protobuf classes and gRPC client/server methods for document count and split-count RPCs and most-recent-shielded-anchor; added proto message classes and updated doc annotations and method declarations.
Client Bindings — Python
packages/dapi-grpc/clients/platform/v0/python/platform_pb2_grpc.py
Added server and stub methods and server registration for new RPCs; updated docstrings.
Client Bindings — Web/TS/JS
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts, .../platform_pb_service.d.ts, .../platform_pb_service.js
Added TypeScript definitions and JS service/client methods/descriptors for new RPCs and updated web protobuf types (including split-count structures and small request field addition).

Sequence Diagram(s)

sequenceDiagram
participant Client
participant gRPCTransport as gRPC Transport
participant PlatformService
participant QueryHandler as Query Handler
participant Drive
participant Verifier as Proof Verifier
Client->>gRPCTransport: GetDocumentsCountRequest
gRPCTransport->>PlatformService: route request
PlatformService->>QueryHandler: query_documents_count()
QueryHandler->>QueryHandler: validate version & bounds
QueryHandler->>Drive: query_documents_count_v0()
Drive->>Drive: resolve contract/type, parse where
alt prove = true
Drive->>Drive: execute with proof
Drive->>QueryHandler: return proof + metadata
QueryHandler->>Verifier: verify proof & checkpoint
Verifier->>QueryHandler: verification result
else prove = false
Drive->>Drive: execute query, count results
Drive->>QueryHandler: return count + metadata
end
QueryHandler->>PlatformService: GetDocumentsCountResponse
PlatformService->>gRPCTransport: response
gRPCTransport->>Client: GetDocumentsCountResponse
Loading
sequenceDiagram
participant Client
participant gRPCTransport as gRPC Transport
participant PlatformService
participant QueryHandler as Query Handler
participant Drive
participant Verifier as Proof Verifier
Client->>gRPCTransport: GetDocumentsSplitCountRequest
gRPCTransport->>PlatformService: route request
PlatformService->>QueryHandler: query_documents_split_count()
QueryHandler->>QueryHandler: validate version & bounds
QueryHandler->>Drive: query_documents_split_count_v0()
Drive->>Drive: resolve contract/type, validate split property, parse where
alt prove = true
Drive->>Drive: execute with proof
Drive->>QueryHandler: return proof + metadata
QueryHandler->>Verifier: verify proof & checkpoint
Verifier->>QueryHandler: verification result
else prove = false
Drive->>Drive: execute query, fetch docs
Drive->>Drive: group counts by property value
Drive->>QueryHandler: return SplitCountEntry list + metadata
end
QueryHandler->>PlatformService: GetDocumentsSplitCountResponse
PlatformService->>gRPCTransport: response
gRPCTransport->>Client: GetDocumentsSplitCountResponse
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Poem

🐇 I counted bytes and grouped by key,
Hopped through proofs and proto tree.
From request to root, I stitched the chain,
Split counts and totals, all in my lane.
A happy hop — the queries reign!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 68.18% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly and specifically summarizes the main changes: adding two new gRPC query endpoints (GetDocumentsCount and GetDocumentsSplitCount) for document counting functionality, which is the primary objective of this pull request.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/documents-count-query

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thepastaclaw

thepastaclaw commented Apr 4, 2026

Copy link
Copy Markdown
Collaborator

Review Gate

Commit: 86b9bbc6

  • Debounce: 13m ago (need 30m)

  • CI checks: checks still running (1 pending)

  • CodeRabbit review: comment found

  • Off-peak hours: off-peak (06:27 AM PT Saturday)

  • Run review now (check to override)

@codecov

codecovBot commented Apr 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.01399% with 315 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.27%. Comparing base (ba3b05d) to head (86b9bbc).

Files with missing linesPatch %Lines
...-drive/src/query/drive_document_count_query/mod.rs84.05%70 Missing ⚠️
...e-proof-verifier/src/proof/document_split_count.rs20.77%61 Missing ⚠️
...bci/src/query/document_split_count_query/v0/mod.rs91.61%38 Missing ⚠️
...rive-abci/src/query/document_count_query/v0/mod.rs90.33%37 Missing ⚠️
...e-abci/src/query/document_split_count_query/mod.rs0.00%37 Missing ⚠️
...s-drive-abci/src/query/document_count_query/mod.rs0.00%32 Missing ⚠️
...s-drive-proof-verifier/src/proof/document_count.rs0.00%27 Missing ⚠️
...s-dpp/src/data_contract/document_type/index/mod.rs56.00%11 Missing ⚠️
..._for_index_level_for_contract_operations/v0/mod.rs50.00%2 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## v3.1-dev #3435 +/- ##
============================================
- Coverage 88.32% 88.27% -0.05% 
============================================
Files 2486 2493 +7 Lines 302964 304434 +1470 ============================================
+ Hits 267593 268750 +1157 - Misses 35371 35684 +313 
ComponentsCoverage Δ
dpp87.99% <75.55%> (-0.02%)⬇️
drive87.41% <83.89%> (-0.02%)⬇️
drive-abci90.18% <84.08%> (-0.07%)⬇️
sdk∅ <ø> (∅)
dapi-client∅ <ø> (∅)
platform-version∅ <ø> (∅)
platform-value92.17% <ø> (ø)
platform-wallet∅ <ø> (∅)
drive-proof-verifier54.22% <15.38%> (-1.45%)⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
packages/rs-drive-proof-verifier/src/proof/document_count.rs (1)

16-18: Remove the unnecessary Clone bound and request clone.
request is consumed by try_into and not reused, so Q: Clone and request.clone() can be dropped.

Refactor suggestion
 impl<'dq, Q> FromProof<Q> for DocumentCount
where
- Q: TryInto<DriveDocumentQuery<'dq>> + Clone + 'dq,+ Q: TryInto<DriveDocumentQuery<'dq>> + 'dq,
Q::Error: std::fmt::Display,
{
@@
- let request: DriveDocumentQuery<'dq> =- request- .clone()- .try_into()+ let request: DriveDocumentQuery<'dq> =+ request+ .try_into()
.map_err(|e: Q::Error| Error::RequestError {
error: e.to_string(),
})?;

Also applies to: 35-39

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rs-drive-proof-verifier/src/proof/document_count.rs` around lines 16
- 18, Remove the unnecessary Clone bound on the generic Q and stop cloning the
request before conversion: in the function signatures where you have Q:
TryInto<DriveDocumentQuery<'dq>> + Clone + 'dq (and the similar signature around
line 35-39), drop the + Clone requirement and remove any request.clone() calls;
call request.try_into() (or std::convert::TryInto::try_into(request)) directly
since try_into consumes request and no further clones are needed. Ensure the
Q::Error: std::fmt::Display bound remains if used for error formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rs-drive-abci/src/query/document_count_query/v0/mod.rs`:
- Around line 73-84: The count endpoint is incorrectly capped because
DriveDocumentQuery::from_decomposed_values is called with
Some(self.config.drive.default_query_limit) which embeds a size limit into the
query; change that argument to None so the constructed DriveDocumentQuery
(created in the drive_query variable in mod.rs) does not pass a limit into
SizedQuery and GroveDB, ensuring the subsequent count computed from
results.len() reflects all matching documents; update any related comments and,
if you prefer truncation semantics instead, add explicit response metadata
indicating the result was truncated rather than leaving the limit in place.
In `@packages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs`:
- Around line 101-112: The split count query is using a hard limit
(Some(self.config.drive.default_query_limit)) which truncates results and yields
incorrect split counts; update the DriveDocumentQuery::from_decomposed_values
call (the drive_query creation) to remove the default_query_limit by passing no
limit (e.g., None) so the query returns the full result set for accurate split
counting, ensuring you only change the limit argument and keep the other
parameters the same.
- Around line 164-172: The current code silently swallows CBOR serialization
failures by using value.to_cbor_buffer().unwrap_or_default(), which can collapse
distinct property values into the same empty key; change this to propagate the
error instead of defaulting: replace the unwrap_or_default call in the let key
assignment (where you access document.properties() and
split_count_by_index_property) with proper error handling (e.g., use
value.to_cbor_buffer()? or value.to_cbor_buffer().map_err(|e| /* wrap in the
function's error type */ )?) so that serialization failures return an error from
the surrounding function rather than producing an empty Vec<u8>.
In `@packages/rs-drive-proof-verifier/src/proof/document_split_count.rs`:
- Around line 49-63: The code currently discards the verified documents from
request.verify_proof and always returns an empty DocumentSplitCounts; change
verify_proof usage to keep the deserialized documents, compute split counts
using the request's split_count_by_index_property (or the equivalent field on
the DriveDocumentQuery) and a helper like split_count_by_index_property to group
documents and produce a BTreeMap, then return DocumentSplitCounts(populated_map)
along with mtd.clone() and proof.clone(); ensure you reference and use the
returned _documents from request.verify_proof and mirror the approach used in
document_count.rs where documents.len() is extracted from the verified proof.
In
`@packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs`:
- Around line 11-12: DriveAbciQueryVersions struct literals in the mocks need to
be updated to include the two new mandatory fields document_count_query and
document_split_count_query; locate the initializer in
packages/rs-platform-version/src/version/mocks/v2_test.rs (the mock
DriveAbciQueryVersions instance around Line ~170) and add those fields with
appropriate FeatureVersionBounds values (e.g., the same bounds used for similar
query fields or FeatureVersionBounds::default()) so the struct literal compiles;
ensure the field names exactly match
DriveAbciQueryVersions::document_count_query and ::document_split_count_query.
---
Nitpick comments:
In `@packages/rs-drive-proof-verifier/src/proof/document_count.rs`:
- Around line 16-18: Remove the unnecessary Clone bound on the generic Q and
stop cloning the request before conversion: in the function signatures where you
have Q: TryInto<DriveDocumentQuery<'dq>> + Clone + 'dq (and the similar
signature around line 35-39), drop the + Clone requirement and remove any
request.clone() calls; call request.try_into() (or
std::convert::TryInto::try_into(request)) directly since try_into consumes
request and no further clones are needed. Ensure the Q::Error: std::fmt::Display
bound remains if used for error formatting.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7d1a82c8-31c5-47f1-8fa3-922c9cbf1a3d

📥 Commits

Reviewing files that changed from the base of the PR and between 8a83981 and 0c84b98.

📒 Files selected for processing (15)
  • packages/dapi-grpc/build.rs
  • packages/dapi-grpc/protos/platform/v0/platform.proto
  • packages/rs-dapi-client/src/transport/grpc.rs
  • packages/rs-dapi/src/services/platform_service/mod.rs
  • packages/rs-drive-abci/src/query/document_count_query/mod.rs
  • packages/rs-drive-abci/src/query/document_count_query/v0/mod.rs
  • packages/rs-drive-abci/src/query/document_split_count_query/mod.rs
  • packages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs
  • packages/rs-drive-abci/src/query/mod.rs
  • packages/rs-drive-abci/src/query/service.rs
  • packages/rs-drive-proof-verifier/src/proof.rs
  • packages/rs-drive-proof-verifier/src/proof/document_count.rs
  • packages/rs-drive-proof-verifier/src/proof/document_split_count.rs
  • packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rs
  • packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v1.rs

Comment threadpackages/rs-drive-abci/src/query/document_count_query/v0/mod.rs Outdated
Comment threadpackages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs Outdated
Comment threadpackages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs Outdated
Comment threadpackages/rs-drive-proof-verifier/src/proof/document_split_count.rs Outdated
- Remove 100-document limit on count queries (set limit = None)
- Add 3 tests for GetDocumentsCount (no-prove, empty, with-prove)
- Add 4 tests for GetDocumentsSplitCount (no-prove, with-prove, errors)
- Fix missing version fields in mock v2_test.rs
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
packages/rs-drive-abci/src/query/document_count_query/v0/mod.rs (2)

73-90: Construct the unlimited count query directly.

Line 77 sets a default limit and Line 89 immediately removes it. Passing None for limit at construction avoids mutable post-fix state and reduces regression risk.

♻️ Proposed simplification
- let mut drive_query =+ let drive_query =
check_validation_result_with_data!(DriveDocumentQuery::from_decomposed_values(
where_clause,
None,
- Some(self.config.drive.default_query_limit),+ None,
None,
true,
None,
contract_ref,
document_type,
&self.config.drive,
));
-- // Remove the limit so we count ALL matching documents, not just up to the- // default query limit. A count query needs to return the total number of- // documents matching the where clause.- drive_query.limit = None;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rs-drive-abci/src/query/document_count_query/v0/mod.rs` around lines
73 - 90, The code currently constructs a DriveDocumentQuery with
Some(self.config.drive.default_query_limit) and then mutates drive_query.limit =
None; instead, call DriveDocumentQuery::from_decomposed_values with None for the
limit parameter so the query is constructed without any limit upfront; update
the call site (DriveDocumentQuery::from_decomposed_values) to pass None instead
of Some(self.config.drive.default_query_limit) and remove the subsequent
drive_query.limit = None mutation.

152-160: Use one platform version source per test.

These tests mix PlatformVersion::latest() with version from setup_platform. Using a single source (preferably version) makes tests less brittle across versioned behavior changes.

🧪 Example adjustment pattern
- let platform_version = PlatformVersion::latest();+ let platform_version = version;

Apply consistently in the three tests where documents/contracts are created and stored.

Also applies to: 171-175, 181-182, 213-221, 257-265, 276-280, 286-287

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rs-drive-abci/src/query/document_count_query/v0/mod.rs` around lines
152 - 160, The tests call PlatformVersion::latest() directly when creating
contracts/documents (e.g., in json_document_to_contract_with_ids) while the test
harness provides a platform version variable (commonly named version from
setup_platform); change all uses of PlatformVersion::latest() in those tests to
use the single source 'version' instead so the tests rely on the
setup_platform-provided version consistently (apply this replacement in the
calls to json_document_to_contract_with_ids and any other contract/document
creation or storage sites mentioned around the blocks using
PlatformVersion::latest()).
packages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs (1)

101-117: Consider passing None directly for the limit parameter.

The code passes Some(self.config.drive.default_query_limit) on line 105, then immediately removes the limit on line 117. This two-step approach is redundant - you could simplify by passing None directly.

♻️ Proposed simplification
 let mut drive_query =
check_validation_result_with_data!(DriveDocumentQuery::from_decomposed_values(
where_clause,
None,
- Some(self.config.drive.default_query_limit),+ None, // No limit - split count needs all matching documents
None,
true,
None,
contract_ref,
document_type,
&self.config.drive,
));
- // Remove the limit so we count ALL matching documents, not just up to the- // default query limit. A split count query needs to return complete counts- // across all values of the split property.- drive_query.limit = None;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs` around
lines 101 - 117, DriveDocumentQuery::from_decomposed_values is being called with
Some(self.config.drive.default_query_limit) and then immediately overwritten by
setting drive_query.limit = None, which is redundant; update the call to
DriveDocumentQuery::from_decomposed_values to pass None for the limit parameter
(the argument currently supplying default_query_limit) and remove the subsequent
drive_query.limit = None line so the query is created without a limit from the
start.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/rs-drive-abci/src/query/document_count_query/v0/mod.rs`:
- Around line 73-90: The code currently constructs a DriveDocumentQuery with
Some(self.config.drive.default_query_limit) and then mutates drive_query.limit =
None; instead, call DriveDocumentQuery::from_decomposed_values with None for the
limit parameter so the query is constructed without any limit upfront; update
the call site (DriveDocumentQuery::from_decomposed_values) to pass None instead
of Some(self.config.drive.default_query_limit) and remove the subsequent
drive_query.limit = None mutation.
- Around line 152-160: The tests call PlatformVersion::latest() directly when
creating contracts/documents (e.g., in json_document_to_contract_with_ids) while
the test harness provides a platform version variable (commonly named version
from setup_platform); change all uses of PlatformVersion::latest() in those
tests to use the single source 'version' instead so the tests rely on the
setup_platform-provided version consistently (apply this replacement in the
calls to json_document_to_contract_with_ids and any other contract/document
creation or storage sites mentioned around the blocks using
PlatformVersion::latest()).
In `@packages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs`:
- Around line 101-117: DriveDocumentQuery::from_decomposed_values is being
called with Some(self.config.drive.default_query_limit) and then immediately
overwritten by setting drive_query.limit = None, which is redundant; update the
call to DriveDocumentQuery::from_decomposed_values to pass None for the limit
parameter (the argument currently supplying default_query_limit) and remove the
subsequent drive_query.limit = None line so the query is created without a limit
from the start.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 960f7945-798d-456b-b54c-622573fc270f

📥 Commits

Reviewing files that changed from the base of the PR and between 0c84b98 and b878c55.

📒 Files selected for processing (3)
  • packages/rs-drive-abci/src/query/document_count_query/v0/mod.rs
  • packages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs
  • packages/rs-platform-version/src/version/mocks/v2_test.rs

QuantumExplorerand others added 4 commits April 4, 2026 13:30
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… lint
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@thepastaclawthepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Well-structured new query endpoints following existing patterns. One confirmed bug: the DocumentSplitCountsFromProof implementation verifies the GroveDB proof but returns BTreeMap::new() instead of extracting the actual count data. Any client using prove=true for split-count queries receives a valid-looking but empty result. Convergent finding from two independent codex agents (rust-quality + security-auditor), confirmed by verifier.

Reviewed commit: 99686a3

🔴 1 blocking

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-proof-verifier/src/proof/document_split_count.rs`:
- [BLOCKING] line 60: DocumentSplitCounts FromProof always returns empty BTreeMap
The `FromProof` implementation for `DocumentSplitCounts` verifies the GroveDB proof and Tenderdash proof but discards the verified documents (`_documents` on line 49) and returns `BTreeMap::new()` on line 60. The `prove=false` server-side path (`document_split_count_query/v0/mod.rs:154-180`) correctly deserializes documents and groups counts by the split property, but this client-side proof verification path never performs the equivalent aggregation.
Any client using `prove=true` for split-count queries receives `Some(DocumentSplitCounts(BTreeMap::new()))` — a valid-looking but empty result. The verified documents need to be grouped by the split property and counted, matching the server-side logic.

Comment threadpackages/rs-drive-proof-verifier/src/proof/document_split_count.rs Outdated
QuantumExplorerand others added 2 commits April 4, 2026 20:05
…mment
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
libz-sys 1.1.26 removed bundled zlib sources, breaking compilation
on macOS. Pin to 1.1.25 which matches v3.1-dev.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@thepastaclawthepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This push is an incremental re-review of PR #3435 at commit 8ffbf55a. The new commit only regenerates dapi-grpc client artifacts and removes a stale // Force rebuild comment from build.rs, so the previously reported proof-verifier bug is the main question. That blocker is still present in the current snapshot: the DocumentSplitCounts proof path verifies the proof but still discards the verified documents and returns an empty map.

Reviewed commit: 8ffbf55

🔴 1 blocking

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-proof-verifier/src/proof/document_split_count.rs`:
- [BLOCKING] lines 49-60: prove=true split-count verification still returns an empty result
This re-review commit does not touch the split-count proof verifier, and the same bug is still present in the current snapshot. `verify_proof()` returns the verified documents into `_documents` on lines 49-51, but the implementation immediately discards them and returns `DocumentSplitCounts(BTreeMap::new())` on line 60. The server-side non-proof path in `rs-drive-abci` groups matching documents by the split property and returns counts, so `prove=true` and `prove=false` still diverge semantically: a valid proof-backed split-count query succeeds but always produces an empty map. This means the new generated client bindings now expose an endpoint whose proof-verification path still cannot return correct split counts.

Comment threadpackages/rs-drive-proof-verifier/src/proof/document_split_count.rs Outdated

@thepastaclawthepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Incremental re-review of PR #3435 at commit 8c5d2b4f. This push only updates Cargo.lock to pin libz-sys (plus lockfile-adjacent version churn such as semver and arc-swap), so the key question is whether the previously reported split-count proof bug was fixed elsewhere. It was not: the DocumentSplitCounts proof verifier still discards the verified documents and returns an empty map, so prove=true split-count queries are still incorrect in the current tip.

Reviewed commit: 8c5d2b4

🔴 1 blocking

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-proof-verifier/src/proof/document_split_count.rs`:
- [BLOCKING] lines 49-60: The split-count proof verifier still drops the verified result set
This incremental push does not touch the split-count verifier, and the same blocker remains in the current tip. `request.verify_proof()` returns the verified documents into `_documents` on lines 49–51, but the implementation then ignores them and returns `DocumentSplitCounts(BTreeMap::new())` on line 60. The non-proof server path in `rs-drive-abci/src/query/document_split_count_query/v0/mod.rs` groups matching documents by the split property and returns actual counts, so `prove=true` and `prove=false` still diverge semantically: a valid proof-backed split-count query succeeds but always yields an empty map. The lockfile pin in this commit may help CI, but it does not make the new split-count endpoint correct.

Comment threadpackages/rs-drive-proof-verifier/src/proof/document_split_count.rs Outdated
QuantumExplorerand others added 2 commits April 4, 2026 21:28
…l documents
Rewrites both count query handlers to read counts directly from
CountTree elements in GroveDB instead of fetching and counting
all matching documents:
- Count query: navigates index path using where clause values,
fetches CountTree element at terminal key [0], returns
count_value_or_default()
- Split count: iterates split property values at the appropriate
index level, sums CountTree counts for each value
- Both now require a countable index — returns error if none found
- Prove path unchanged (still uses DriveDocumentQuery for proofs)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduces DriveDocumentCountQuery in rs-drive with the CountTree
counting logic. ABCI handlers are now thin wrappers that parse gRPC
requests and delegate to Drive.
- New: DriveDocumentCountQuery struct with execute_no_proof()
- Supports total count and split-by-property count
- 5 Drive-level tests + 7 ABCI integration tests
- ABCI handlers simplified to parse + delegate pattern
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@thepastaclawthepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This push rewrites the non-proof count paths to use CountTree-based aggregation, but it introduces a correctness regression: the fast path only applies equality predicates and silently ignores all other where operators. That makes prove=false return broader counts than prove=true for range/prefix-style queries. The previously reported DocumentSplitCounts proof-verifier bug is also still present, so both the proof and non-proof split-count paths remain incorrect in the current tip.

Reviewed commit: e7c98f8

🔴 3 blocking

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-abci/src/query/document_count_query/v0/mod.rs`:
- [BLOCKING] lines 152-165: The CountTree fast path silently drops non-equality filters
The new `prove=false` path chooses a countable index from `all_where_clauses`, but `find_countable_index_for_where_clauses()` only looks at clauses whose operator is `WhereOperator::Equal` (lines 198-202), and `count_from_count_tree()` likewise only serializes equality clauses into the path (lines 275-289). Any other predicate in the request (`>`, `>=`, `<`, `startsWith`, etc.) is neither validated nor applied. As a result, a query like `where = [["age", ">", 18]]` can still hit the CountTree path and return the count for the entire prefix / whole index instead of the filtered subset, while the `prove=true` path above still evaluates the full `DriveDocumentQuery`. `prove=false` and `prove=true` therefore diverge semantically for the same request. The no-proof path needs to reject unsupported operators or fall back to the full query engine whenever any non-equality clause is present.
In `packages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs`:
- [BLOCKING] lines 175-189: The split-count CountTree path also ignores non-equality where clauses
The same issue exists in the split-count refactor. `find_countable_index_for_split()` builds its prefix using only `WhereOperator::Equal` clauses (lines 221-225, 232-249), and `split_count_from_count_tree()` only pushes equality values into the CountTree path (lines 290-303). Any non-equality predicate in the request is silently ignored rather than rejected or evaluated. That means `prove=false` split counts can now be computed over the entire split subtree even when the original query included additional range/pattern filters, while the `prove=true` path still uses the full `DriveDocumentQuery`. This breaks correctness for non-equality split-count queries and makes proof/no-proof results disagree.
In `packages/rs-drive-proof-verifier/src/proof/document_split_count.rs`:
- [BLOCKING] lines 49-60: The proof-backed split-count verifier still returns an empty map
The previously reported proof-verifier bug is still present in this tip. `request.verify_proof()` returns the verified documents into `_documents` on lines 49-51, but the implementation immediately discards them and returns `DocumentSplitCounts(BTreeMap::new())` on line 60. So even after this refactor, a valid `prove=true` split-count query still verifies the proof and then returns an empty result instead of grouping the verified documents by the split property. This keeps the proof-backed endpoint incorrect even if the non-proof path were fixed.

Comment threadpackages/rs-drive-abci/src/query/document_count_query/v0/mod.rs Outdated
Comment threadpackages/rs-drive-abci/src/query/document_split_count_query/v0/mod.rs Outdated
Comment threadpackages/rs-drive-proof-verifier/src/proof/document_split_count.rs Outdated

@thepastaclawthepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Incremental re-review of PR #3435 at commit 1c23a28c. This push moves the CountTree counting logic from rs-drive-abci into a shared rs-drive::query::DriveDocumentCountQuery, but it does not fix the correctness issues from the previous review. The new shared implementation still only honors equality clauses in the no-proof path, so unsupported operators are silently ignored, and the proof-backed split-count verifier still returns an empty map.

Reviewed commit: 1c23a28

🔴 3 blocking

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive/src/query/drive_document_count_query.rs`:
- [BLOCKING] lines 69-111: The shared CountTree total-count path still ignores non-equality filters
`find_countable_index_for_where_clauses()` now lives in `rs-drive`, but it still builds `equality_fields` by discarding every where clause whose operator is not `WhereOperator::Equal` (lines 73-76). `execute_total_count()` then only serializes equality clauses into the index path (lines 199-218) and never validates that there were additional predicates in the original query. That means a request with mixed filters like `["age", ">", 18]` or `["name", "startsWith", "A"]` can still take the no-proof CountTree fast path and return the broader prefix count / full index count, while the `prove=true` path above still uses the full `DriveDocumentQuery`. So the refactor preserved the bug where `prove=false` silently ignores non-equality constraints instead of rejecting them or falling back to the full query engine.
- [BLOCKING] lines 118-145: The shared split-count path still drops non-equality where clauses
`find_countable_index_for_split()` has the same problem in the shared implementation: it only considers `WhereOperator::Equal` clauses when choosing an index (lines 121-124), and `execute_split_count()` only pushes equality values into the path before enumerating split buckets (lines 263-285). Any other predicate in the original query is silently ignored rather than enforced. As a result, `prove=false` split-count queries can still return counts for the whole split subtree even when the request included additional range/pattern filters, while `prove=true` uses the full query semantics. Moving the code from `rs-drive-abci` into `rs-drive` did not fix the correctness gap; it just centralized it.
In `packages/rs-drive-proof-verifier/src/proof/document_split_count.rs`:
- [BLOCKING] lines 49-60: The proof-backed split-count verifier still returns an empty map
The old proof-verifier bug is unchanged in this tip. `request.verify_proof()` still returns the verified documents into `_documents` on lines 49-51, but the implementation immediately discards them and returns `DocumentSplitCounts(BTreeMap::new())` on line 60. So even after the CountTree refactor, a valid `prove=true` split-count query still verifies the proof and then returns an empty result instead of grouping the verified documents by the split property.

Comment threadpackages/rs-drive-proof-verifier/src/proof/document_split_count.rs Outdated
@thephezthephez added the dapi-endpoint DAPI endpoint addition or modification label Apr 6, 2026
Adds proof generation method using GroveDB's get_proved_path_query.
Each test now verifies both execute_no_proof and execute_with_proof.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@thepastaclawthepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The workflow change is syntactically fine, but it only partially fixes the intended test-trigger problem because the new Dashmate-specific gate is narrower than the existing integration trigger surface for the same jobs.

Reviewed commit: 998fdbb

🔴 1 blocking

1 additional finding

🔴 blocking: New `dashmate-changed` gate still leaves the same integration jobs blind to non-Dashmate trigger paths

.github/workflows/tests.yml (lines 128-168)

Validated against the checked-out workflow plus .github/package-filters/test-suite-triggers.yml: the new dashmate-changed output only matches packages/dashmate/**, but it now gates build-images, dashmate-e2e-tests, test-suite, and test-functional. Those same jobs are still part of the broader integration surface already described by test-suite-triggers.yml, which includes paths such as packages/platform-test-suite/**, .github/actions/local-network/**, scripts/**, and multiple platform package directories. After this patch, a PR that changes those integration-critical paths but not packages/dashmate/** will still skip these jobs, so the workflow blind spot remains and the fix is incomplete.

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.github/workflows/tests.yml`:
- [BLOCKING] lines 128-168: New `dashmate-changed` gate still leaves the same integration jobs blind to non-Dashmate trigger paths
Validated against the checked-out workflow plus `.github/package-filters/test-suite-triggers.yml`: the new `dashmate-changed` output only matches `packages/dashmate/**`, but it now gates `build-images`, `dashmate-e2e-tests`, `test-suite`, and `test-functional`. Those same jobs are still part of the broader integration surface already described by `test-suite-triggers.yml`, which includes paths such as `packages/platform-test-suite/**`, `.github/actions/local-network/**`, `scripts/**`, and multiple platform package directories. After this patch, a PR that changes those integration-critical paths but not `packages/dashmate/**` will still skip these jobs, so the workflow blind spot remains and the fix is incomplete.

Resolves conflicts in two auto-generated client files:
- packages/dapi-grpc/clients/platform/v0/java/.../PlatformGrpc.java
- packages/dapi-grpc/clients/platform/v0/python/platform_pb2.py
Both sides expose the same set of underlying RPC methods; this branch's
side inserts METHODID_GET_DOCUMENTS_COUNT (15) and
METHODID_GET_DOCUMENTS_SPLIT_COUNT (16) and bumps the subsequent IDs by
2, while v3.1-dev kept the original numbering. Take the count-query
branch's numbering since it adds the new endpoints these client
bindings need.
Brings in PR #3457 (documentsCountable / rangeCountable feature) so this
branch's count-query handlers can read CountTree / ProvableCountTree
primary key trees end-to-end.
Test results post-merge:
- 3 documents_count tests pass
- 4 documents_split_count tests pass
- 5 drive count-query tests pass
- 10 countable e2e tests pass (incl. v3.1-dev's new
range_countable_primary_key_tree_supports_trunk_proof)
- cargo check --workspace clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@QuantumExplorer
QuantumExplorer marked this pull request as ready for review May 8, 2026 15:04
Adds the SDK convenience layer that was missing on PR #3435: until now a
consumer had to hand-construct a GetDocumentsCountRequest and call the
rs-dapi-client transport directly. After this commit, the count queries
are reachable through the standard rs-sdk Fetch trait:
let count: DocumentCount =
DocumentCount::fetch(&sdk, DocumentCountQuery::new(contract, "type")?)
.await?
.unwrap_or(DocumentCount(0));
let splits: DocumentSplitCounts =
DocumentSplitCounts::fetch(
&sdk,
DocumentSplitCountQuery::new(contract, "type", "indexProp")?,
)
.await?
.unwrap_or_default();
Plumbing:
- packages/rs-sdk/src/platform/documents/document_count_query.rs
New SDK request type DocumentCountQuery wrapping DocumentQuery; impls
TryFrom<&DocumentCountQuery> for DriveDocumentQuery, TryFrom<DocumentCountQuery>
for GetDocumentsCountRequest, TransportRequest (targets the count
endpoint), FromProof<DocumentCountQuery> for DocumentCount (delegates
to the existing FromProof<DriveDocumentQuery>), Fetch for DocumentCount.
- packages/rs-sdk/src/platform/documents/document_split_count_query.rs
Same shape with an extra split_property field; Fetch for DocumentSplitCounts.
- packages/rs-sdk/src/mock/requests.rs
MockResponse for DocumentCount (bincode u64) and DocumentSplitCounts
(bincode Vec<(Vec<u8>, u64)> round-trip into BTreeMap).
- packages/rs-sdk/src/mock/sdk.rs
load_expectations match arms for DocumentCountQuery,
DocumentSplitCountQuery, GetDocumentsCountRequest,
GetDocumentsSplitCountRequest.
Tests: 6 new mock-based integration tests under tests/fetch/. They cover
the present-with-value, empty/zero, and not-found paths for both
endpoints, asserting the SDK-side query → mock-DAPI → MockResponse
round trip works end-to-end. Live-devnet test vectors are still
required for the full proof-verification round trip; that's a follow-up
once the platform-test-suite has a SDK_TEST_DATA hook for these
endpoints (out of scope here).
Test results:
- 6 new tests in fetch::document_count / fetch::document_split_count
- 120 fetch integration tests pass
- 117 dash-sdk lib tests pass
- cargo check --workspace clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t queries
Mirrors the rs-sdk Fetch impls landed in 5bb3ce6 into the language
bindings so browser and iOS consumers can call the count endpoints
without dropping down to gRPC.
wasm-sdk
- getDocumentsCount(query) → bigint
- getDocumentsCountWithProofInfo(query) → ProofMetadataResponseTyped<bigint>
- getDocumentsSplitCount(query, splitProperty) → Map<string, bigint>
- getDocumentsSplitCountWithProofInfo(query, splitProperty)
→ ProofMetadataResponseTyped<Map<string, bigint>>
All four reuse the existing parse_documents_query plumbing, so the
same DocumentsQuery shape (data contract id, document type, where
clauses, etc.) carries through. Split-count keys are hex-encoded so
callers can match them against the platform-value-encoded property
bytes returned by proofs.
rs-sdk-ffi
- dash_sdk_document_count(sdk, contract, type, where_json)
→ JSON {"count": <number>}
- dash_sdk_document_split_count(sdk, contract, type, split_property, where_json)
→ JSON {"counts": {"<hex-key>": <number>, ...}}
Both wrap the rs-sdk Fetch flow; where_json is the same JSON shape
the existing dash_sdk_document_search FFI accepts so iOS callers can
reuse their where-clause encoding.
Workspace check clean. Existing rs-sdk count integration tests still
pass (6/6).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

github-actionsBot commented May 8, 2026

Copy link
Copy Markdown
Contributor

✅ DashSDKFFI.xcframework built for this PR.

SwiftPM (host the zip at a stable URL, then use):

.binaryTarget(
name:"DashSDKFFI",
url:"https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum:"b894d1b6daab5bb0fc91757e9cfb8d522ee8b3338001b24e6473e3126fa70f8b")

Xcode manual integration:

  • Download 'DashSDKFFI.xcframework' artifact from the run link above.
  • Drag it into your app target (Frameworks, Libraries & Embedded Content) and set Embed & Sign.
  • If using the Swift wrapper package, point its binaryTarget to the xcframework location or add the package and place the xcframework at the expected path.

QuantumExplorerand others added 2 commits May 8, 2026 23:26
…roperty
Before this commit the FromProof impl for DocumentSplitCounts verified
the GroveDB and Tenderdash proofs, threw away the verified documents,
and returned `Some(DocumentSplitCounts(BTreeMap::new()))`. Any caller
using `prove=true` for a split-count query received a valid-looking but
empty result — flagged as blocking by thepastaclaw on every successive
review of #3435 and again by coderabbitai. My own SDK additions
inherited the bug because they delegated through the generic
FromProof<DriveDocumentQuery>.
Root cause: split aggregation needs the split-property name, but
`DriveDocumentQuery` does not carry it. The generic
`FromProof<Q where Q: TryInto<DriveDocumentQuery>>` impl literally
cannot do the work; previously it papered over this with an empty map.
Fix:
drive-proof-verifier
- Replace the generic FromProof impl with one that returns an explicit
error pointing callers at the new entry point. Silent emptiness is a
consensus-layer footgun; loud failure surfaces it immediately.
- Add `DocumentSplitCounts::maybe_from_proof_with_split_property` —
same behavior as before for proof verification, plus actual
aggregation: group the verified documents by
`document_type.serialize_value_for_key(split_property, value, ...)`,
yielding the same byte-keyed BTreeMap the no-prove CountTree path
already returns. Documents missing the property are skipped.
- Implement Default on DocumentSplitCounts (needed by Fetch trait
bounds; previously implicit via the broken generic impl).
rs-sdk
- Update FromProof<DocumentSplitCountQuery> for DocumentSplitCounts
to call `maybe_from_proof_with_split_property` directly, threading
the SDK-side split_property through. No more delegation to the
erroring generic impl.
Tests
- Regression test in tests/fetch/document_split_count.rs that pins
the new error path: invoking
`<DocumentSplitCounts as FromProof<DriveDocumentQuery>>::…` returns
an Err mentioning "split-property" rather than silently returning
`Some(empty)`.
- Existing 6 mock-based integration tests still pass; the new
regression brings the count to 7.
Aggregation unit tests live above the proof-verifier crate (in rs-sdk
and rs-drive-abci) because drive-proof-verifier's feature surface
doesn't expose dpp's test fixtures (DataContractFactory, random_document).
A note in the source points readers at those higher-level tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the documentation gap on documentsCountable / rangeCountable, the
v11→v12 strip migration, and the GetDocumentsCount /
GetDocumentsSplitCount endpoints — none of which existed in the book
prior to this commit. Single new chapter covers:
- Why count trees exist (avoid O(n) walks for count(*) queries).
- The three primary-key tree variants (NormalTree / CountTree /
ProvableCountTree) and how primary_key_tree_type() selects between
them as the single source of truth shared across contract insert /
update, document insert / delete, and cost-estimation paths.
- Storage-layout invariants and why the immutability guards in
validate_config are load-bearing.
- The v11→v12 strip migration: why it has to run, why
ALLOWED_TRANSITION_TO_DOCUMENT_SCHEMA_V1_PROPERTIES deliberately
excludes the v12 flags, and the end-to-end test that pins the
smuggling defense.
- The two query endpoints, both code paths (no-prove CountTree fast
lookup vs prove path = verify-then-aggregate-by-property), the
byte-key encoding callers see, and the design choice to error
loudly on the generic FromProof for split counts (replacing the
earlier silent-empty-map regression).
- Known limitation: the no-prove fast path silently drops
non-equality where clauses; flagged with a workaround.
- SDK access at three layers (rs-sdk Fetch, wasm-sdk getDocumentsCount{,SplitCount},
rs-sdk-ffi dash_sdk_document_{count,split_count}) with code samples.
- Test coverage table + the one big remaining gap (live-devnet proof
round trip via the platform-test-suite).
Registered in SUMMARY.md under the existing # Drive section after the
Finalize Tasks chapter.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

github-actionsBot commented May 8, 2026

Copy link
Copy Markdown
Contributor

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-05-09T13:14:07.841Z

The no-prove count and split-count handlers previously dropped any
non-Equal where clause silently, returning the equality-prefix count
instead of the filtered count. This adds explicit support for `In`
(cartesian fork over the listed values) and rejects range operators
upfront with a clear `InvalidArgument` error rather than producing a
wrong number.
Range operators (`>`, `<`, `between*`, `startsWith`) still need a
boundary walk that the current count `PathQuery` model cannot express;
those are deferred to a follow-up PR. Callers that need range counts
should fall back to `prove=true` and aggregate client-side.
Tests: 5 new drive-level tests (In on total / split + picker rejection +
has_unsupported_operator helper), 4 new handler tests (In and range
rejection on each endpoint). Extended the drive-abci copy of the family
countable contract so split-by-lastName under an `In firstName` filter
is reachable from the handler tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@QuantumExplorerQuantumExplorer left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m Codex, posting the review findings I found while checking this PR.

Findings

  1. [P1] Count proof path is unbounded

    packages/rs-drive-abci/src/query/document_count_query/v0/mod.rs:104

    The prove=true path builds a normal DriveDocumentQuery, then clears its limit before generating the proof. That makes a public count request materialize/prove every matching document instead of proving the count tree, bypassing max-return limits and making empty-where count requests potentially huge. The split-count handler does the same. Use the CountTree proof path, keep a hard bound, or reject prove=true until count-tree proof verification exists.

  2. [P1] WASM count inherits document limit

    packages/wasm-sdk/src/queries/document.rs:462

    The WASM count methods reuse parse_documents_query, which sets a default document limit of 100. The server proof is generated with no limit, but the SDK verifier converts this same request back into a DriveDocumentQuery and DocumentCount counts the verified documents, so getDocumentsCount / getDocumentsSplitCount can be capped or mismatched by the document-query limit. Count queries should build a base query with limit=0 / None and ignore any document pagination fields.

  3. [P2] Duplicate IN values double-count

    packages/rs-drive/src/query/drive_document_count_query.rs:361

    For IN clauses the count code iterates every value and sums each branch. A query like age in [30, 30] will count the same subtree twice, and split-count does the same when merging branches. IN semantics are set-membership, so serialize/deduplicate the branch keys before summing.

  4. [P2] Unique countable indexes return zero

    packages/rs-drive/src/query/drive_document_count_query.rs:624

    fetch_count_at_path assumes key [0] is a CountTree and reads count_value_or_default(). For countable unique indexes, existing index insertion stores the terminal reference directly at key [0] inside the count tree, so this returns 0 for a matching unique value instead of 1. Handle unique indexes separately or read the parent count tree count rather than the terminal reference.

Three real fixes from Codex review on PR #3435:
- **In dedup**: `expand_paths_and_count` and `expand_split_prefix_paths`
now serialize each `In` value to the canonical index key and dedupe
before forking. Previously `age in [30, 30]` would visit and sum the
same subtree twice; same shape on the split path.
- **SDK count not capped at document limit**: `DocumentCountQuery` and
`DocumentSplitCountQuery` `TryFrom<&Self>` for `DriveDocumentQuery`
now force `limit = None`. The proof verifier counts documents in the
verified proof, so any limit on the wrapped `DocumentQuery` would
silently truncate the count — most visibly in the WASM SDK, where
`parse_documents_query` defaults `DocumentQuery.limit` to 100.
- **Server prove path bound**: both count handlers now cap the
underlying `DriveDocumentQuery` at `u16::MAX` matching documents
rather than `None`. Previously the server materialized and proved
every matching document, making empty-where count requests an
unbounded resource sink. Until count-tree proof verification is
implemented, callers needing exact counts on larger result sets
should use `prove=false` with a covering countable index.
Codex finding #4 (unique countable index allegedly returning 0) was
checked and is not a bug: `Element::count_value_or_default()` returns 1
for non-CountTree elements, so the Reference stored at key [0] under a
unique countable index reads as 1 correctly. Added a regression test
to lock that behavior in.
Tests added:
- `test_count_query_in_operator_dedupes_duplicate_values`
- `test_count_query_unique_countable_index_returns_correct_count`
Book chapter updated to note the prove-path cap and the SDK-side
limit-clearing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@QuantumExplorer

Copy link
Copy Markdown
MemberAuthor

Thanks Codex — reviewed all four findings, fixed three, dismissed one as a non-issue. Addressed in commit 4ef1e40.

[P1] Count proof path is unbounded — fixed (defensive bound)

The handlers now cap the prove-path DriveDocumentQuery at u16::MAX matching documents rather than clearing the limit:

drive_query.limit = Some(u16::MAX);

This is a defensive bound, not the proper fix. The proper fix is count-tree proof verification (no signed-count primitive exists on the wire today), which is out of scope for this PR. Callers that need exact counts above u16::MAX use prove=false with a covering countable index. Documented in book/src/drive/document-count-trees.md.

[P1] WASM count inherits document limit — fixed at the SDK layer

Both DocumentCountQuery and DocumentSplitCountQueryTryFrom<&Self> for DriveDocumentQuery now force limit = None:

letmut drive_query:DriveDocumentQuery = (&query.document_query).try_into()?;
drive_query.limit = None;Ok(drive_query)

The proof verifier counts documents from the verified proof, so any limit on the wrapped DocumentQuery would truncate the count. Fixing it in the rs-sdk conversion layer covers WASM, FFI, and direct rs-sdk callers in one place rather than each entry point setting limit = 0 defensively.

[P2] Duplicate IN values double-count — fixed

expand_paths_and_count and expand_split_prefix_paths now serialize each In value to the canonical index key and dedupe via a BTreeSet<Vec<u8>> before forking:

letmut seen_keys:BTreeSet<Vec<u8>> = BTreeSet::new();for v in values {let serialized = self.document_type.serialize_value_for_key(...)?;if !seen_keys.insert(serialized.clone()){continue;}// ... recurse / collect}

Regression test: test_count_query_in_operator_dedupes_duplicate_values (asserts age in [30, 30, 30] over 2 docs with age=30 returns 2, not 6).

[P2] Unique countable indexes return zero — verified, not a bug

I wrote a test (test_count_query_unique_countable_index_returns_correct_count) that inserts 3 distinct (firstName, middleName, lastName) tuples into a contract whose 3-property index has unique: true, countable: true, then queries for an exact match. The test passes — count is 1, not 0.

The reason: grovedb-element's Element::count_value_or_default() returns 1 for any non-CountTree variant (the _ => 1 arm of the match). The Reference stored at key [0] under a unique countable index falls into that arm, so fetch_count_at_path returns 1 correctly. From grovedb-element/src/element/helpers.rs:

pubfncount_value_or_default(&self) -> u64{matchself{Element::CountTree(_, count_value, _)
| Element::CountSumTree(_, count_value, ..)
| Element::ProvableCountTree(_, count_value, _)
| Element::ProvableCountSumTree(_, count_value, ..) => *count_value,
_ => 1,}}

Kept the test as a regression guard so this stays correct.

cc @QuantumExplorer

QuantumExplorerand others added 7 commits May 9, 2026 04:09
CI's clippy 1.92 catches two lints my prior pushes missed:
- `doc_overindented_list_items` on the `expand_paths_and_count` doc
comment — reformat the bullet list with the canonical 1-space gutter.
- `cloned_ref_to_slice_refs` on a few `&[clause.clone()]` test sites —
switch to `std::slice::from_ref(&clause)`.
- `unnecessary_to_owned` on two `BTreeMap::get(&b"…".to_vec())` lookups
in the rs-sdk split-count fetch test — switch to
`b"…".as_slice()`.
No semantic changes; existing tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the unit tests out of the inline `mod tests { ... }` block and into
a sibling `tests.rs` so the production code in this module stays
focused. Same module shape the rest of the rs-drive crate uses
(util/operations, state_transition_action/batch).
No behavior change; same 12 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a focused test that documents and locks in the grovedb semantic
the count fast path depends on: when a non-CountTree element (empty
Tree, Item, Reference, etc.) is inserted as a child of a CountTree,
it contributes 1 to the parent's aggregated count. Only nested
CountTree/ProvableCountTree/CountSumTree/ProvableCountSumTree children
contribute their own stored count_value.
This matters because the picker walks countable indexes assuming each
intermediate value-level subtree (which is a NormalTree, not a
CountTree) is *not* swallowed into 0. A regression in grovedb's
`count_value_or_default` defaults would corrupt every fast-path count.
Test inserts an empty CountTree (count=0), then progressively adds an
empty Tree (count=1), a second empty Tree (count=2), and an Item
(count=3) and asserts each step.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…CountableAllowingOffset)
Replaces the boolean `Index.countable` field with a three-way enum so an
index can opt into the provable-count-tree variant alongside the plain
count-tree variant:
- `NotCountable` (default) → `NormalTree`. No count fast path.
- `Countable` → `CountTree`. O(1) totals at the root.
- `CountableAllowingOffset` → `ProvableCountTree`. O(1) totals plus
per-node sub-counts that will enable future O(log n) range / offset
queries on the index.
JSON schema accepts both the legacy boolean form (`true` → Countable,
`false` → NotCountable) and the new camelCase string form
(`"notCountable"` / `"countable"` / `"countableAllowingOffset"`). v0
contracts continue to load via the bool fallback; the v1 meta-schema is
extended with `oneOf [bool, string-enum]`.
Reaches:
- `Index` / `IndexLevelTypeInfo` field type changes from bool to enum.
- Insert / delete tree-type selection switches from a 2-way `if` to a
3-way `match`, mapping `CountableAllowingOffset` to
`TreeType::ProvableCountTree`.
- Count query pickers replace `!index.countable` with
`!index.countable.is_countable()` so both countable variants are
accepted.
- v1 try_from_schema gate uses `is_countable()` so either variant
requires v12+.
Tests:
- `test_countable_allowing_offset_variant_end_to_end` builds a contract
via `DataContract::from_json` with the new string form, asserts the
enum parses correctly, applies the contract, inserts documents, and
verifies the picker + fast-path total count work against the
ProvableCountTree-backed index.
- All 14 existing drive count tests still pass — the bool back-compat
in the parser keeps `family-contract-countable.json`'s `"countable":
true` working unchanged.
Book chapter updated with a table covering all three variants and a
note that the bool form is back-compat only; new contracts should use
the explicit string variants.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…idating
Earlier I considered rejecting `unique: true` combined with any countable
variant at schema validation, on the assumption the flag is always a
no-op for unique indexes. That's wrong: the insert path takes the
count-tree branch when *any* indexed field is null
(`!is_unique() || any_fields_null`), so on a unique index where at least
one indexed property is optional, `countable` *does* affect storage for
null-bearing entries. Rejecting the combination would block a legitimate
case.
Drop the validation idea and document the nuance instead:
- `IndexCountability` doc comment now explains when the flag actually
does work on unique indexes (null-bearing paths) vs. when it's a no-op
(all-non-null exact matches).
- Book chapter has a matching note in the index-level flag bullet list.
- The `test_count_query_unique_countable_index_returns_correct_count`
test (which exercises the all-non-null path) gets a more accurate
doc comment explaining why the count comes back correct (Reference's
`count_value_or_default = 1`, not via a count tree).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reference chapter covering:
- Each `Index` field (name, properties, unique, null_searchable,
contested_index, countable) with what it does and when to use it.
- The `IndexLevel` trie compiled from a flat list of indexes — how
shared prefixes get merged and how an index "terminates" at a level.
- The on-disk GroveDB path shape:
`[DataContractDocuments, contract_id, 1, doc_type, prop, value, …, 0]`
- Tree-type-at-the-terminal matrix: when the `[0]` slot holds a bare
Reference vs an empty NormalTree / CountTree / ProvableCountTree
containing per-doc references.
- How `any_fields_null` / `all_fields_null` accumulate down the index
walk and why a unique index can land different documents in different
storage shapes depending on which of their indexed fields are null.
- The three-step insert flow (top-level → recursive → terminal) and
where each step lives in the source.
- Brief note on query traversal with a pointer to the count-tree
chapter for count-specific path picking.
Linked from `Document Count Trees`. Sits in SUMMARY.md between
`Finalize Tasks` and `Document Count Trees` so the count-trees chapter
can lean on the index-fundamentals reference.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three additions to `book/src/drive/indexes.md`:
1. **Tree vs reference distinction in the existing layout / terminal-shape
diagrams.** Tree-type elements stay as dark rectangles, count trees
stay amber, and `Reference` elements are now drawn as rounded
stadium nodes filled green so the leaf-pointer-vs-tree distinction
is obvious before reading any text.
2. **Shared-prefix layout diagram.** A second on-disk diagram showing
`byColor` + `byColorShape` together with three documents
(A: red,circle / B: red,square / C: blue,square). Two things made
explicit: (a) `[0]` (the byColor terminal) and `'shape'` (the
byColorShape continuation) are siblings under each color value,
(b) the same document is stored as a separate `Reference` under
every index path that matches it.
3. **Range-Countable Indexes design section.** New `range_countable`
per-index property in design — additive to `countable`, makes range
queries on the indexed property O(log n). Documents the constraints
(requires `countable`, non-unique-or-null-bearing only, siblings
must use `NonCounted<*>`), the mechanism table (property-name level
→ `ProvableCountTree`, value level → `CountTree`, siblings →
`NonCounted<NormalTree>`), and includes a layout diagram with new
color coding (purple = ProvableCountTree, dashed lavender =
NonCounted, amber = CountTree, green = Reference). Walks through
how the counts aggregate cleanly with `NonCounted` siblings, what
would mis-aggregate without them, and how a `BETWEEN` query
resolves on the `ProvableCountTree`. Marked as design-only — depends
on a parallel grovedb change adding `NonCounted<ElementType>`
element variants whose count values do not propagate to a parent
count tree.
No code changes — design draft only for the range_countable feature.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@QuantumExplorer

Copy link
Copy Markdown
MemberAuthor

There will need to be a follow up to this PR, but for now am merging it in.

@QuantumExplorer
QuantumExplorer merged commit a28c298 into v3.1-devMay 9, 2026
39 checks passed
@QuantumExplorer
QuantumExplorer deleted the feat/documents-count-query branch May 9, 2026 13:30
QuantumExplorer added a commit that referenced this pull request May 11, 2026
…metric rejection
Replaces the prove-path materialize-and-count fallback (capped at
`u16::MAX` matching docs, scaled with document count not branch count)
with a CountTree element proof. Symmetric with the no-proof
`Total` / `PerInValue` modes: requires a covering `countable: true`
index, rejects partial coverage with a clear error instead of
silently materializing every matching document.
## Background
PR #3435 shipped the unified `GetDocumentsCount` endpoint with
`DriveDocumentQuery::execute_with_proof` (the regular document-fetch
proof) as the prove path for Equal/`In` counts: drive returned full
document bytes for every match, the SDK deserialized each and
counted them client-side. The `u16::MAX` cap was a defensive bound
on response size — and a hard ceiling that any contract with > 65,535
docs in a single Equal/In bucket couldn't escape.
This PR replaces that with a count-aware proof shape:
- **Server**: drive builds a `PathQuery` targeting `CountTree`
elements at `[..., last_field, last_value, 0]` for each covered
branch. Calls `get_proved_path_query` and returns proof bytes.
- **Verifier**: rebuilds the same path query (shared builder under
`cfg(any(server, verify))`), calls `GroveDb::verify_query`,
extracts `count_value_or_default()` from each verified element.
Each `count` is bound to the merk root via `node_hash_with_count`
— same forge-resistance guarantee the range-distinct path uses.
Proof size becomes O(k × log n) where k is the number of covered
branches and n is the tree depth — one merk path per CountTree
element, regardless of how many docs sit in each bucket. The
`u16::MAX` cap is gone.
## Symmetric rejection contract
The no-proof `Total` / `PerInValue` modes already require a covering
`countable: true` index — calls without one fail with
`WhereClauseOnNonIndexedProperty`. The prove path now matches:
partial coverage, non-`Equal`/`In` operators, or `In` on a non-last
property all return the same class of error, pointing the caller at
the index-design fix. Contract authors who want fast prove counts
have to define an appropriate countable index, same as for no-proof
counts. No silent fallback.
Supported shapes (this PR):
- Equal on every index property, fully covered.
- Equal on every property except the last + `In` on the last.
Future work (separate PR): partial coverage via subquery enumeration
of uncovered levels — the no-proof side handles it via
`count_recursive`; the prove-side equivalent is a more complex
subquery construction that's not blocking this PR's release.
## Per-layer changes
- **`packages/rs-drive/src/query/drive_document_count_query/path_query.rs`**:
new `point_lookup_count_path_query` builder. Shared between the
prover and verifier so path-query bytes match byte-for-byte. For
the In-on-last case, sorts In keys lex-ascending before insertion
(same convention as `distinct_count_path_query`) so pushed-limit /
direction semantics are meaningful.
- **`packages/rs-drive/src/query/drive_document_count_query/execute_point_lookup.rs`**:
the dead `execute_with_proof` method (only reached from two unit
tests, never from the dispatcher in production) is replaced with
`execute_point_lookup_count_with_proof`. Calls `get_proved_path_query`
on the new builder's output.
- **`packages/rs-drive/src/verify/document_count/verify_point_lookup_count_proof/`**:
new verifier module mirroring the prove-distinct one. v0 walks
`verify_query`'s `(path, key, element)` triples and extracts each
element's `count_value_or_default()`.
- **`packages/rs-drive-proof-verifier/src/proof/document_count.rs`**:
new `verify_point_lookup_count_proof` wrapper composing the drive
verifier with `verify_tenderdash_proof`.
- **`packages/rs-drive/src/query/drive_document_count_query/drive_dispatcher.rs`**:
`execute_document_count_point_lookup_proof` rewired. Signature
changes from `(where_clause: Value, order_by: Value, contract, ...)`
to `(contract_id, document_type, document_type_name, where_clauses:
Vec<WhereClause>, ...)` — no longer goes through
`DriveDocumentQuery::from_decomposed_values`, so the raw `Value`
shapes are unnecessary. Calls `find_countable_index_for_where_clauses`
up-front and rejects with `WhereClauseOnNonIndexedProperty` when no
covering index exists.
- **`packages/rs-sdk/src/platform/documents/document_count_query.rs`**:
- `FromProof<DocumentCountQuery> for DocumentCount`: no-range
branch now routes through `verify_point_lookup_count_proof` and
sums the per-branch entries. No more
`<DocumentCount as FromProof<DriveDocumentQuery>>::maybe_from_proof_with_metadata`
materialize fallback.
- `FromProof<DocumentCountQuery> for DocumentSplitCounts`: no-range
branch routes through the same verifier and returns its entries
directly. For Equal-only fully-covered queries, the verifier may
return zero entries (CountTree absent or count=0); we re-emit a
single empty-key zero-count entry so callers can structurally
distinguish "verified zero" from "no proof returned" without
inspecting the variant.
- The `maybe_from_proof_with_split_property` fallback that
aggregated documents by an In field's property value is no
longer reachable from the SDK FromProof flow. The function and
its `aggregate_documents_by_property` helper are deleted from
rs-drive-proof-verifier as dead code; the generic
`FromProof<Q> for DocumentSplitCounts` footgun-guard impl is
kept and its docstring updated.
- **`packages/rs-platform-version/.../drive_verify_method_versions/`**:
new `verify_point_lookup_count_proof: FeatureVersion` field on
`DriveVerifyDocumentCountMethodVersions`; v1 dispatch table sets
it to 0.
- **`book/src/drive/document-count-trees.md`**: prove-path section
rewritten. Drops the materialize-and-count + `u16::MAX` cap
description; describes the CountTree element proof + symmetric
rejection contract. Implementation reference cross-links to all
three layers.
## Tests
New / updated:
- `test_documents_count_with_prove_and_covering_equal` (drive-abci):
positive end-to-end. Builds a contract with `countable: true` on
`firstName`, inserts docs at distinct firstName values, sends
`firstName == "Alice"` + `prove: true`, asserts the response
carries a non-empty `Proof` variant.
- `test_documents_count_prove_without_covering_index_returns_clear_error`
(drive-abci): negative end-to-end. Empty where clauses against a
contract whose indexes don't fully cover the request → asserts
`InvalidWhereClauseComponents` error with "countable" in the
message. Pins the symmetric-rejection contract at the API
boundary.
- `test_count_query_total_count_with_documents` /
`test_count_query_total_count_empty` (drive): the trailing proof
assertions (`.execute_with_proof(...).expect(...)`) become
rejection assertions (`.expect_err(...)`). The tests' main intent
(no-proof count behavior) is unaffected.
- `test_documents_count_with_prove` (drive-abci): deleted. Was a
smoke test for "prove path doesn't crash" with empty where; under
the new contract that's exactly the case that rejects. The two
new tests above cover the positive and negative paths.
Verified:
- 33 drive `query::drive_document_count_query` unit tests pass
- 27 drive `range_countable_index_e2e_tests` pass (including the
`aggregate_count_proof_*` and `distinct_count_proof_*` end-to-end
prover/verifier roundtrip suites — unaffected by the
point-lookup rewrite)
- 9 drive-abci `query::document_count_query` end-to-end tests pass
- 225 drive-proof-verifier tests pass
- `cargo clippy -p drive -p drive-abci -p dash-sdk -p drive-proof-verifier --lib --tests --features=server,verify -- -D warnings` clean
- `cargo fmt --check` clean
## Breaking changes
Source-API on rs-drive-proof-verifier:
- `DocumentSplitCounts::maybe_from_proof_with_split_property` deleted
(was dead code post-rewrite). Callers should use the rs-sdk Fetch
impl on `DocumentCountQuery`, which routes to the correct proof
shape internally.
Wire-format compatibility: the prove path's response variant is
still `Proof(grovedb_proof_bytes)` — the SDK now verifies the bytes
as a CountTree element proof instead of a document proof. A pre-PR
client running against a post-PR server would receive a proof shape
its old verifier can't decode; a post-PR client running against a
pre-PR server would get a document proof and fail to extract counts.
The endpoint is pre-testnet so no real-world clients are affected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dapi-endpointDAPI endpoint addition or modification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@QuantumExplorer@thepastaclaw@thephez