feat: ranked aggregate indexes with provable top-K queries (protocol v14) - #4266
Merged
QuantumExplorer merged 41 commits intoAug 4, 2026
Merged
Conversation
…l v14
Contracts can now declare that an index's groups are rankable by an
aggregate (rankedCountable / rankedSummable / rankedAverageable, each
requiring its range* counterpart), upgrading the index's terminal
property-name tree to a GroveDB indexed tree (ProvableCountIndexedTree /
ProvableSumIndexedTree / ProvableCountProvableSumIndexedTree) whose
per-axis ordered secondaries are maintained on every document write.
Queries of the form
SELECT AVG(grade) GROUP BY restaurantId HAVING AVG(grade) IN TOP(5)
are served in O(log n + k) with proofs, over the existing
GetDocumentsRequestV1 wire surface (the only proto change is the
additive RankedEntries response message).
- rs-dpp: meta-schema v3 grammar + validation (single-property,
non-unique indexes only; flags immutable on contract update), gated
on document_type_schema >= 3 so pre-v14 contracts cannot declare it
- rs-drive: indexed-tree construction at contract registration,
secondary maintenance through the normal batch write path, cost
estimation, the drive_document_ranked_query family and
verify/document_ranked
- rs-drive-abci: HAVING ranking routing, feature-gated so protocol v13
nodes keep rejecting HAVING and mixed-version networks agree
- rs-drive-proof-verifier + rs-sdk: DocumentRankedEntries::fetch with
proof verification bound to the signed app hash
- rs-platform-version: protocol v14, behaviorally identical to v13
until a contract uses the new grammar
- grovedb pinned to develop 4c6720b, which includes the indexed-tree
family (grovedb #657) plus the subset-verification regression fix and
verify-only proof gating shipped via grovedb #781
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
requested review from
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
August 2, 2026 15:08
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughProtocol v14 adds ranked aggregate indexes and ChangesRanked aggregate contracts and versioning
Ranked index storage
Ranked query execution
Proofs and SDK integration
Compatibility and generated support
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Collaborator
|
🕓 Ready for review — 10 ahead in queue (commit 745581d) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
packages/rs-drive/src/query/drive_document_ranked_query/tests.rs (1)
1234-1280: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConfirm the exhaustive bit sweep keeps an acceptable test runtime.
The loop runs
8 * proof.len()full proof verifications. For a proof of a few hundred bytes this is several thousand verifications, and debug-profile CI amplifies the cost. If the measured runtime is high, reduce the sweep to a deterministic sample of byte offsets and keep the "some mutations verify with a diverged root" sanity check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/query/drive_document_ranked_query/tests.rs` around lines 1234 - 1280, Measure the runtime of the exhaustive mutation loop in a_tampered_proof_never_verifies_to_the_honest_root_hash; if it is excessive, replace the full 8 * proof.len() sweep with a deterministic sample of byte offsets and bit positions. Preserve tampering coverage and the verified_at_all > 0 sanity check so at least some mutations must verify to a divergent root.packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs (1)
44-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
DriveError::UnknownVersionMismatchfor the unknown method version.The unknown-version arm returns
QuerySyntaxError::Unsupported. In rs-drive-abci that maps to a client-visible query rejection, so a node misconfiguration is reported as a malformed request. The sibling ranked dispatcherDriveDocumentRankedQuery::verify_ranked_top_k_proofusesDriveError::UnknownVersionMismatchfor the same condition. Align the two so version faults stay internal errors.♻️ Proposed change
0 => detect_ranked_mode_v0(select, group_by, having, where_clauses, pagination), - version => Err(Error::Query(QuerySyntaxError::Unsupported(format!( - "detect_ranked_mode: unknown method version {version}; only 0 is supported" - )))), + version => Err(Error::Drive(DriveError::UnknownVersionMismatch { + method: "detect_ranked_mode".to_string(), + known_versions: vec![0], + received: version, + })),Add
use crate::error::drive::DriveError;to the imports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs` around lines 44 - 56, Update the unknown-version arm in the ranked-mode dispatcher to return DriveError::UnknownVersionMismatch instead of QuerySyntaxError::Unsupported, matching DriveDocumentRankedQuery::verify_ranked_top_k_proof. Add the required crate::error::drive::DriveError import and preserve the supported version-0 dispatch.packages/rs-drive/src/query/mod.rs (1)
886-898: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve the underlying
OrderClauseparse error.
QuerySyntaxError::InvalidOrderByPropertiesonly takes&'static str, butOrderClause::from_componentsreturns detailed failures such as non-text fields or invalidasc/descvalues. Add aStringpayload variant here so each rejected clause can report its actual reason and index.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/query/mod.rs` around lines 886 - 898, Update the order_by parsing flow around OrderClause::from_components to preserve its detailed parse error, including the rejected clause’s reason and index, instead of replacing it with a static InvalidOrderByProperties message. Add a String-carrying QuerySyntaxError variant and map the underlying error into it while keeping the existing non-array validation unchanged.packages/dapi-grpc/protos/platform/v0/platform.proto (1)
1349-1357: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the ranked shapes to the
GetDocumentsResponseV1wire-shape table.The message-level docstring lists one row per
select×group_by×provecombination. It has no row for the ranked path. Add the ranked rows so the table stays the single reference for response routing.📝 Suggested docstring rows
// - `having=RANKING` (no prove) → `result.data.ranked.entries`. // - `having=RANKING` (prove) → `result.proof`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dapi-grpc/protos/platform/v0/platform.proto` around lines 1349 - 1357, Update the GetDocumentsResponseV1 message-level wire-shape docstring to add rows for having=RANKING without prove routing to result.data.ranked.entries and having=RANKING with prove routing to result.proof. Keep the existing table entries unchanged and place the new rows with the other select/group_by/prove response combinations.packages/rs-drive/src/fees/op.rs (1)
1095-1102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider canonicalizing
ranked_axesbefore building the TLV.The mapped list is passed to grovedb exactly as the caller supplied it. grovedb's
validate_pcpsit_axesthen rejects an unsorted or duplicated list with an error. Sorting by tag and de-duplicating here would make the helper tolerant of caller ordering, at negligible cost for a list of at most three entries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/fees/op.rs` around lines 1095 - 1102, Canonicalize the axes in the TreeType::ProvableCountProvableSumIndexedTree branch before passing them to Self::for_known_path_key_empty_provable_count_provable_sum_indexed_tree: sort the mapped entries by axis tag and remove duplicate tags. Preserve the existing None values and storage_flags while ensuring the resulting ranked_axes list is ordered and unique for grovedb validation.packages/rs-drive/src/drive/contract/update/update_contract/v0/mod.rs (1)
484-537: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the generic indexed-tree helper to remove the duplicated dispatch.
The existing-doctype branch above (Lines 358-368) already resolves the same pair and delegates to
batch_insert_empty_index_tree_if_not_exists, which handles every tree variant and carriesranked_axes. This branch repeats the whole match, and it is a near-copy of the dispatch inpackages/rs-drive/src/drive/contract/insert/insert_contract/v0/mod.rs. Three copies of the same table must stay in lock-step, and a missed arm produces an on-disk layout that differs between fresh insert and contract update. Consider collapsing this branch onto the same helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/drive/contract/update/update_contract/v0/mod.rs` around lines 484 - 537, Replace the TreeType match in the new-doctype branch with the existing generic indexed-tree helper used by the existing-doctype path, `batch_insert_empty_index_tree_if_not_exists`. Pass through `type_path`, `KeyRef(index_bytes)`, `ranked_axes`, storage flags, batch operations, and `drive_version`, preserving the helper’s handling for every tree variant.packages/rs-drive/src/drive/contract/insert/insert_contract/v0/mod.rs (1)
440-455: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider routing the single-axis arms through the validating dispatcher.
The
ProvableCountIndexedTreeandProvableSumIndexedTreearms discardranked_axes. The element constructors do not need the TLV, so the written tree is correct today. HoweverLowLevelDriveOperation::for_known_path_key_empty_indexed_treeadditionally asserts that the resolved axis set matches the variant ([IndexAxis::Count]/[IndexAxis::Sum]). Calling the per-variant helpers directly skips that assertion, so a future resolver change that pairs, for example,ProvableCountIndexedTreewith[IndexAxis::Avg]would silently create a tree whose secondary is keyed on the wrong aggregate. Routing all three arms through the dispatcher keeps the invariant enforced in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/drive/contract/insert/insert_contract/v0/mod.rs` around lines 440 - 455, Update the ProvableCountIndexedTree and ProvableSumIndexedTree arms in the tree insertion dispatch to call LowLevelDriveOperation::for_known_path_key_empty_indexed_tree, passing ranked_axes and the existing insertion context. Route all three indexed-tree variants through this validating dispatcher, preserving the current per-variant construction behavior while enforcing axis-to-variant validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/rs-dpp/schema/meta_schemas/document/v3/document-meta.json`:
- Around line 3-4: Update the v3 meta-schema header’s $comment to identify it as
the v3 document meta-schema, state that it activates with protocol v14 (the
corresponding contract version), and correct the release-specific freeze
wording. Verify the $id compatibility requirement separately; preserve the
existing v1 URL if it is intentionally frozen, and document that it is a
compatibility identifier rather than the file path.
In `@packages/rs-dpp/src/validation/meta_validators/mod.rs`:
- Around line 247-254: Correct the stale v3 document meta-schema comments in
packages/rs-dpp/src/validation/meta_validators/mod.rs (lines 247-254) and
packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v6.rs
(line 54) so they accurately describe the existing rankedCountable,
rankedSummable, rankedAverageable, and dependentRequired validation; remove the
incorrect v6 claim rather than changing schema behavior.
In `@packages/rs-drive-proof-verifier/src/lib.rs`:
- Around line 17-23: Update the rustdoc link in the public re-export
documentation for DocumentRankedEntries to reference the public
verify_ranked_top_k_proof re-export rather than the private
proof::document_ranked path, using the existing public symbol without changing
the surrounding documentation.
In
`@packages/rs-drive/src/drive/contract/insert/insert_contract/v0/tests/ranked_index_e2e_tests.rs`:
- Around line 232-238: Update expected_avg_fixed_point to use Euclidean division
via div_euclid on the scaled sum and count, matching the documented
toward-negative-infinity rounding for negative sums while preserving the
existing calculation and types.
In `@packages/rs-drive/src/drive/document/mod.rs`:
- Around line 56-58: Gate all imports or nearest parent module declarations that
reference ranked_index_tree_type behind the server feature, including
estimated_sum_trees_for_value_tree_type, non-server contract index
insert/update/delete code, and
batch_insert_empty_provable_count_provable_sum_indexed_tree. Ensure non-server
feature builds no longer compile these dependent paths while preserving their
existing server-only behavior.
In
`@packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs`:
- Around line 495-503: Update the re-materialization logic around
batch_insert_empty_index_tree_if_not_exists to call
property_name_tree_type_and_ranked_axes(current_index_level) instead of
property_name_tree_type_for_index_level. Ensure the terminal property-name level
of a single-property ranked index is recreated with its indexed tree type and
ranked_axes, while preserving normal-tree behavior for non-ranked levels.
In `@packages/rs-drive/src/query/drive_document_ranked_query/execute_top_k.rs`:
- Around line 135-167: Update execute_top_k_with_proof to detect grovedb’s
dedicated empty-tree error variant, when available, and map it to the
appropriate typed Drive-specific error before the generic Error::GroveDB
conversion. Preserve generic grovedb failures through Error::GroveDB, and keep
the existing empty-ranking limitation documentation without matching
error-message text.
In `@packages/rs-platform-version/Cargo.toml`:
- Line 14: Update the grovedb-version dependency revision in Cargo.toml to a
valid commit or tag resolvable from the configured GroveDB repository, while
retaining the intended GroveDB changes.
---
Nitpick comments:
In `@packages/dapi-grpc/protos/platform/v0/platform.proto`:
- Around line 1349-1357: Update the GetDocumentsResponseV1 message-level
wire-shape docstring to add rows for having=RANKING without prove routing to
result.data.ranked.entries and having=RANKING with prove routing to
result.proof. Keep the existing table entries unchanged and place the new rows
with the other select/group_by/prove response combinations.
In `@packages/rs-drive/src/drive/contract/insert/insert_contract/v0/mod.rs`:
- Around line 440-455: Update the ProvableCountIndexedTree and
ProvableSumIndexedTree arms in the tree insertion dispatch to call
LowLevelDriveOperation::for_known_path_key_empty_indexed_tree, passing
ranked_axes and the existing insertion context. Route all three indexed-tree
variants through this validating dispatcher, preserving the current per-variant
construction behavior while enforcing axis-to-variant validation.
In `@packages/rs-drive/src/drive/contract/update/update_contract/v0/mod.rs`:
- Around line 484-537: Replace the TreeType match in the new-doctype branch with
the existing generic indexed-tree helper used by the existing-doctype path,
`batch_insert_empty_index_tree_if_not_exists`. Pass through `type_path`,
`KeyRef(index_bytes)`, `ranked_axes`, storage flags, batch operations, and
`drive_version`, preserving the helper’s handling for every tree variant.
In `@packages/rs-drive/src/fees/op.rs`:
- Around line 1095-1102: Canonicalize the axes in the
TreeType::ProvableCountProvableSumIndexedTree branch before passing them to
Self::for_known_path_key_empty_provable_count_provable_sum_indexed_tree: sort
the mapped entries by axis tag and remove duplicate tags. Preserve the existing
None values and storage_flags while ensuring the resulting ranked_axes list is
ordered and unique for grovedb validation.
In `@packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs`:
- Around line 44-56: Update the unknown-version arm in the ranked-mode
dispatcher to return DriveError::UnknownVersionMismatch instead of
QuerySyntaxError::Unsupported, matching
DriveDocumentRankedQuery::verify_ranked_top_k_proof. Add the required
crate::error::drive::DriveError import and preserve the supported version-0
dispatch.
In `@packages/rs-drive/src/query/drive_document_ranked_query/tests.rs`:
- Around line 1234-1280: Measure the runtime of the exhaustive mutation loop in
a_tampered_proof_never_verifies_to_the_honest_root_hash; if it is excessive,
replace the full 8 * proof.len() sweep with a deterministic sample of byte
offsets and bit positions. Preserve tampering coverage and the verified_at_all >
0 sanity check so at least some mutations must verify to a divergent root.
In `@packages/rs-drive/src/query/mod.rs`:
- Around line 886-898: Update the order_by parsing flow around
OrderClause::from_components to preserve its detailed parse error, including the
rejected clause’s reason and index, instead of replacing it with a static
InvalidOrderByProperties message. Add a String-carrying QuerySyntaxError variant
and map the underlying error into it while keeping the existing non-array
validation unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6654c4f5-c86c-499b-b898-399c5707474d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (94)
packages/dapi-grpc/protos/platform/v0/platform.protopackages/rs-dpp/Cargo.tomlpackages/rs-dpp/schema/meta_schemas/document/v3/document-meta.jsonpackages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rspackages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v1/mod.rspackages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v2/mod.rspackages/rs-dpp/src/data_contract/document_type/index/mod.rspackages/rs-dpp/src/data_contract/document_type/index/random_index.rspackages/rs-dpp/src/data_contract/document_type/index_level/find_first_change.rspackages/rs-dpp/src/data_contract/document_type/index_level/mod.rspackages/rs-dpp/src/data_contract/document_type/methods/validate_update/v0/mod.rspackages/rs-dpp/src/data_contract/factory/v0/mod.rspackages/rs-dpp/src/data_contract/methods/registration_cost/v1/mod.rspackages/rs-dpp/src/validation/meta_validators/mod.rspackages/rs-drive-abci/Cargo.tomlpackages/rs-drive-abci/src/query/document_query/v1/compute_aggregate_mode_and_check_limit/mod.rspackages/rs-drive-abci/src/query/document_query/v1/compute_aggregate_mode_and_check_limit/v0/mod.rspackages/rs-drive-abci/src/query/document_query/v1/compute_aggregate_mode_and_check_limit/v1/mod.rspackages/rs-drive-abci/src/query/document_query/v1/mod.rspackages/rs-drive-abci/src/query/document_query/v1/tests.rspackages/rs-drive-proof-verifier/src/lib.rspackages/rs-drive-proof-verifier/src/proof.rspackages/rs-drive-proof-verifier/src/proof/document_ranked.rspackages/rs-drive/Cargo.tomlpackages/rs-drive/src/cache/system_contracts.rspackages/rs-drive/src/drive/contract/estimation_costs/mod.rspackages/rs-drive/src/drive/contract/insert/insert_contract/v0/mod.rspackages/rs-drive/src/drive/contract/insert/insert_contract/v0/tests/mod.rspackages/rs-drive/src/drive/contract/insert/insert_contract/v0/tests/ranked_index_e2e_tests.rspackages/rs-drive/src/drive/contract/update/update_contract/v0/mod.rspackages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v1/mod.rspackages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v1/mod.rspackages/rs-drive/src/drive/document/estimation_costs/estimated_sum_trees_for_value_tree_type.rspackages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v1/mod.rspackages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v1/mod.rspackages/rs-drive/src/drive/document/mod.rspackages/rs-drive/src/drive/document/ranked_index_tree_type.rspackages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rspackages/rs-drive/src/fees/op.rspackages/rs-drive/src/query/drive_document_count_query/tests.rspackages/rs-drive/src/query/drive_document_ranked_query/drive_dispatcher.rspackages/rs-drive/src/query/drive_document_ranked_query/execute_top_k.rspackages/rs-drive/src/query/drive_document_ranked_query/executors/mod.rspackages/rs-drive/src/query/drive_document_ranked_query/executors/top_k_no_proof.rspackages/rs-drive/src/query/drive_document_ranked_query/executors/top_k_proof.rspackages/rs-drive/src/query/drive_document_ranked_query/index_picker.rspackages/rs-drive/src/query/drive_document_ranked_query/mod.rspackages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rspackages/rs-drive/src/query/drive_document_ranked_query/path.rspackages/rs-drive/src/query/drive_document_ranked_query/tests.rspackages/rs-drive/src/query/drive_document_sum_query/tests.rspackages/rs-drive/src/query/mod.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_provable_count_indexed_tree/mod.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_provable_count_indexed_tree/v0/mod.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_provable_count_provable_sum_indexed_tree/mod.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_provable_count_provable_sum_indexed_tree/v0/mod.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_provable_sum_indexed_tree/mod.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_provable_sum_indexed_tree/v0/mod.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_tree_if_not_exists/mod.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_tree_if_not_exists/v0/mod.rspackages/rs-drive/src/util/grove_operations/grove_insert_empty_tree/v0/mod.rspackages/rs-drive/src/util/grove_operations/mod.rspackages/rs-drive/src/verify/document_ranked/mod.rspackages/rs-drive/src/verify/document_ranked/verify_ranked_top_k_proof/mod.rspackages/rs-drive/src/verify/document_ranked/verify_ranked_top_k_proof/v0/mod.rspackages/rs-drive/src/verify/mod.rspackages/rs-drive/tests/supporting_files/contract/restaurants/restaurants-contract.jsonpackages/rs-platform-version/Cargo.tomlpackages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/mod.rspackages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v6.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/mod.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/v2.rspackages/rs-platform-version/src/version/drive_versions/drive_document_method_versions/mod.rspackages/rs-platform-version/src/version/drive_versions/drive_document_method_versions/v1.rspackages/rs-platform-version/src/version/drive_versions/drive_document_method_versions/v2.rspackages/rs-platform-version/src/version/drive_versions/drive_document_method_versions/v3.rspackages/rs-platform-version/src/version/drive_versions/drive_document_method_versions/v4.rspackages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions/mod.rspackages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions/v1.rspackages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rspackages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rspackages/rs-platform-version/src/version/drive_versions/mod.rspackages/rs-platform-version/src/version/drive_versions/v9.rspackages/rs-platform-version/src/version/mod.rspackages/rs-platform-version/src/version/protocol_version.rspackages/rs-platform-version/src/version/v14.rspackages/rs-platform-wallet/Cargo.tomlpackages/rs-sdk-ffi/src/system/queries/path_elements.rspackages/rs-sdk/Cargo.tomlpackages/rs-sdk/src/mock/requests.rspackages/rs-sdk/src/platform/documents/document_ranked_entries.rspackages/rs-sdk/src/platform/documents/mod.rspackages/rs-sdk/src/platform/documents/ranked_proof_helpers.rspackages/wasm-sdk/src/queries/system.rs
thepastaclaw
suggested changes
Aug 2, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The ranked storage and proof paths are extensively implemented, but three correctness issues block merge: MAX/MIN omit tied extrema, explicit false ranked flags are rejected by the meta-schema, and the committed generated gRPC clients cannot decode ranked responses. The WASM projection also needs two compatibility fixes, while several smaller documentation and test-helper issues remain.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed),gpt-5.6-sol— rust-quality (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 3 blocking | 🟡 2 suggestion(s) | 💬 3 nitpick(s)
2 additional finding(s) omitted (not in diff).
🤖 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_ranked_query/mode_detection.rs`:
- [BLOCKING] packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs:187-205: MAX and MIN omit groups tied at the extreme
The wire and Drive contracts define `HAVING COUNT(*) EQ MAX` as selecting every group whose aggregate equals the maximum scalar. This branch instead converts MAX/MIN into a directional top-K read with `k = 1`. The indexed secondary breaks equal aggregates by group key, so only one tied group is returned and the other groups satisfying the predicate are silently omitted. Reject MAX/MIN until a tie-aware primitive is available, or retrieve and prove every entry tied at the extreme.
In `packages/dapi-grpc/protos/platform/v0/platform.proto`:
- [BLOCKING] packages/dapi-grpc/protos/platform/v0/platform.proto:1349-1357: Regenerate the committed gRPC clients for ranked results
The protobuf adds `RankedEntry`, `RankedEntries`, and oneof field 5, but the maintained bindings under `packages/dapi-grpc/clients/platform/v0/` contain no ranked symbols. For example, the Node decoder's oneof still lists only `documents|counts|sums|averages` and skips field 5 as unknown, while the Web, Python, and Objective-C APIs expose no ranked accessor. A non-proof ranked response therefore loses its result when decoded by the shipped clients. Run the existing dapi-grpc generation script and commit every maintained binding.
In `packages/rs-dpp/schema/meta_schemas/document/v3/document-meta.json`:
- [BLOCKING] packages/rs-dpp/schema/meta_schemas/document/v3/document-meta.json:486-493: Explicit false ranked flags incorrectly require range axes
JSON Schema's `dependentRequired` is triggered by property presence rather than its boolean value. Consequently, `"rankedCountable": false` still requires `rangeCountable`, and `"rankedAverageable": false` still requires `rangeAverageable`. This contradicts the fields' “When true” semantics and `Index::try_from_value_map`, which parses false as an ordinary opt-out and only enforces the range prerequisite when the resolved ranked flag is true. Replace these dependencies with value-sensitive `if`/`then` conditions, or declare the optional ranked properties as `const: true` if explicit false is intentionally unsupported.
In `packages/wasm-sdk/src/queries/system.rs`:
- [SUGGESTION] packages/wasm-sdk/src/queries/system.rs:861-875: Expose indexed-tree sums through the WASM PathElement API
`PathElementWasm::from_element` obtains its JavaScript `sum` property through this function. Both `ProvableSumIndexedTree` and `ProvableCountProvableSumIndexedTree` carry an `i64` sum, but they currently fall through to `None`, so JavaScript sees a recognized indexed element type with an unexpectedly undefined aggregate. Project these sums in the same way as the corresponding non-indexed variants.
- [SUGGESTION] packages/wasm-sdk/src/queries/system.rs:20-68: Add indexed-tree runtime values to the WASM TypeScript union
The `elementType` getter now emits six values absent from `GroveElementType`: `provableSumIndexedTree`, `provableCountIndexedTree`, `provableCountProvableSumIndexedTree`, and their three `nonCounted` forms. JavaScript receives these strings, but generated TypeScript declarations claim they are impossible, making exhaustive switches and validators unsound. Add all six strings to the custom TypeScript section.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4266 +/- ##
============================================
+ Coverage 87.54% 87.57% +0.03%
============================================
Files 2679 2700 +21
Lines 341312 344254 +2942
============================================
+ Hits 298798 301493 +2695
- Misses 42514 42761 +247
🚀 New features to boost your workflow:
|
- Reject MAX / MIN rankings instead of mapping them to TOP(1) / BOTTOM(1): '= MAX' selects every group tied at the extreme, which the axis secondary cannot prove (ties break by group key), so a k = 1 read would silently omit tied groups. Positional TOP(1) / BOTTOM(1) remain the single-best-ranked forms. - Meta-schema v3: the three ranked prerequisite rules are now value-sensitive if/then conditionals, so an explicit 'rankedCountable: false' no longer demands its range axis, matching the structural parser's opt-out semantics. - wasm-sdk: project the i64 sum of ProvableSumIndexedTree / ProvableCountProvableSumIndexedTree path elements and add the six indexed element-type strings to the TypeScript union. - Average fixed-point test helper now mirrors grovedb's euclidean floor; new signed-sum doctype in the restaurants fixture exercises negative averages where truncating division would disagree. - Stale v3 meta-schema header and version-table comments corrected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…657-integration-b6f426 # Conflicts: # packages/rs-platform-version/src/version/v14.rs
5 tasks
Pulls in the terminal non-Merk proof-binding fix (dashpay/grovedb#782): CommitmentTree / MmrTree / BulkAppendTree / DenseTree elements reported as a query's final result are now bound to the parent value_hash, so a malicious node can no longer serve forged aggregates (e.g. a wrong shielded note count) under a genuine root hash. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…657-integration-b6f426 # Conflicts: # packages/rs-drive/src/util/grove_operations/batch_insert_empty_tree_if_not_exists/mod.rs # packages/rs-drive/src/util/grove_operations/batch_insert_empty_tree_if_not_exists/v0/mod.rs # packages/rs-platform-version/src/version/drive_versions/drive_document_method_versions/v4.rs # packages/rs-platform-version/src/version/drive_versions/v9.rs # packages/rs-platform-version/src/version/v14.rs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Regenerated with the repo's pinned docker toolchain (yarn workspace @dashevo/dapi-grpc build): RankedEntry / RankedEntries and the ResultData.ranked oneof arm now exist in the Node, Web, Python and Objective-C bindings; drive_pbjs.js picks the messages up through its platform.proto import. Diff is additive — the only removals are the four-variant oneof lists becoming five-variant and proto doc comments flowing into generated jsdoc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs (1)
136-178: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove
MAX/MINfrom these error messages.Both messages still advertise
MAXandMINas accepted ranking operands. Lines 191-204 now reject both kinds. A user who follows either message gets a second rejection.
- Line 142: "(
IN TOP(n),IN BOTTOM(n),EQ MAXorEQ MIN)".- Line 173: "(
TOP(n)/BOTTOM(n)/MAX/MIN)".📝 Proposed message fixes
- "ranked queries require exactly one `having` clause carrying the ranking \ - (`IN TOP(n)`, `IN BOTTOM(n)`, `EQ MAX` or `EQ MIN`); got {}. Multiple \ + "ranked queries require exactly one `having` clause carrying the ranking \ + (`IN TOP(n)` or `IN BOTTOM(n)`); got {}. Multiple \- (`TOP(n)` / `BOTTOM(n)` / `MAX` / `MIN`). Thresholds need a range walk \ + (`TOP(n)` / `BOTTOM(n)`). Thresholds need a range walk \🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs` around lines 136 - 178, Update the two error messages in the ranked-query validation around the single-clause check and ranking operand check to advertise only the supported TOP(n) and BOTTOM(n) ranking forms; remove MAX and MIN references while preserving the existing validation behavior.packages/rs-drive/src/fees/op.rs (1)
655-694: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winResolve the contradiction about indexed parents between the two dispatchers.
wrap_in_non_aggregated_for_parent_tree_typenow acceptsProvableCountIndexedTree,ProvableSumIndexedTree, andProvableCountProvableSumIndexedTreeas aggregating parents. The new zero-contribution dispatcher states the opposite at Line 865-882: indexed trees are property-name trees, never value trees, so they cannot host zero-contributing children, and it rejects them explicitly.Both functions answer the same question for the same parent role. Only one answer can be right.
If indexed parents are structurally impossible, remove the three indexed variants from these arms and let them fall to the
_arm. If they are reachable, correct the rejection text and reasoning infor_known_path_key_empty_tree_contributing_zero_to_parent. Note also that this function is described as the frozen v0 dispatcher, and pre-v14 contracts never produce indexed trees, so the added arms are unreachable for v0/v1 walkers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/fees/op.rs` around lines 655 - 694, Resolve the inconsistent indexed-parent handling between wrap_in_non_aggregated_for_parent_tree_type and for_known_path_key_empty_tree_contributing_zero_to_parent. Treat ProvableCountIndexedTree, ProvableSumIndexedTree, and ProvableCountProvableSumIndexedTree consistently in both dispatchers; because the frozen v0/v1 path cannot produce indexed trees and the zero-contribution dispatcher rejects them, remove these variants from the aggregating arms so they fall through to the existing fallback handling.
🧹 Nitpick comments (2)
packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs (1)
191-253: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFold the direction and
kresolution into one match to drop theunreachable!.The second match repeats the
Max | Minarm only to panic. The arm is unreachable today, because the first match returns early. A future edit that moves or removes the early return turns a validation error into a panic on a request-handling path.One match over
ranking.kindthat yields(descending, k)removes the panic and keeps exhaustiveness, which is the stated goal of the current arm.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs` around lines 191 - 253, Combine the two ranking.kind matches into one exhaustive match that returns both descending and k. Preserve the existing Max/Min validation error, Top/Bottom n validation, operator checks, and limit checks, while removing the unreachable! arm and destructuring the resulting tuple for downstream use.packages/rs-drive/src/fees/op.rs (1)
2439-2617: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the matrix test to pin
wrap_in_non_aggregated_for_parent_tree_typeas well.The test pins every cell of
for_known_path_key_empty_tree_contributing_zero_to_parent, including the indexed rejections. It does not cover the v0 dispatcher, whose indexed-parent arms changed in this PR. Add a small loop that asserts the intended result forwrap_in_non_aggregated_for_parent_tree_typewith each indexed parent, once the contradiction noted at Line 655-694 is resolved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-drive/src/fees/op.rs` around lines 2439 - 2617, Extend zero_contribution_dispatcher_full_matrix with a focused loop covering wrap_in_non_aggregated_for_parent_tree_type for every indexed parent type, asserting the intended result for each. Resolve the existing contradiction around the indexed-parent behavior first, then use the established dispatch/assertion patterns to pin the v0 dispatcher’s expected acceptance or rejection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/rs-drive-abci/Cargo.toml`:
- Line 85: Update the protocol v14 version mapping centered on DRIVE_VERSION_V9
so its grove_version uses GROVE_V4 rather than GROVE_V3, and update any related
protocol-v14 configuration or mapping consistently. Ensure PLATFORM_V14.drive
selects GroveDB v4 behavior at activation while leaving other protocol versions
unchanged.
In `@packages/rs-drive/src/query/having.rs`:
- Around line 146-151: Update the cross-group ranking documentation near
HavingRankingKind to state that only Equal works with Top(1)/Bottom(1), while In
works with Top(N)/Bottom(N); remove the broader scalar-comparison claim and
preserve the existing Min/Max rejection note.
---
Outside diff comments:
In `@packages/rs-drive/src/fees/op.rs`:
- Around line 655-694: Resolve the inconsistent indexed-parent handling between
wrap_in_non_aggregated_for_parent_tree_type and
for_known_path_key_empty_tree_contributing_zero_to_parent. Treat
ProvableCountIndexedTree, ProvableSumIndexedTree, and
ProvableCountProvableSumIndexedTree consistently in both dispatchers; because
the frozen v0/v1 path cannot produce indexed trees and the zero-contribution
dispatcher rejects them, remove these variants from the aggregating arms so they
fall through to the existing fallback handling.
In `@packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs`:
- Around line 136-178: Update the two error messages in the ranked-query
validation around the single-clause check and ranking operand check to advertise
only the supported TOP(n) and BOTTOM(n) ranking forms; remove MAX and MIN
references while preserving the existing validation behavior.
---
Nitpick comments:
In `@packages/rs-drive/src/fees/op.rs`:
- Around line 2439-2617: Extend zero_contribution_dispatcher_full_matrix with a
focused loop covering wrap_in_non_aggregated_for_parent_tree_type for every
indexed parent type, asserting the intended result for each. Resolve the
existing contradiction around the indexed-parent behavior first, then use the
established dispatch/assertion patterns to pin the v0 dispatcher’s expected
acceptance or rejection.
In `@packages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rs`:
- Around line 191-253: Combine the two ranking.kind matches into one exhaustive
match that returns both descending and k. Preserve the existing Max/Min
validation error, Top/Bottom n validation, operator checks, and limit checks,
while removing the unreachable! arm and destructuring the resulting tuple for
downstream use.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b07c4bf9-773d-4b85-987f-6748f7652dd5
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (50)
packages/dapi-grpc/clients/drive/v0/nodejs/drive_pbjs.jspackages/dapi-grpc/clients/platform/v0/nodejs/platform_pbjs.jspackages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.jspackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.hpackages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.mpackages/dapi-grpc/clients/platform/v0/python/platform_pb2.pypackages/dapi-grpc/clients/platform/v0/web/platform_pb.d.tspackages/dapi-grpc/clients/platform/v0/web/platform_pb.jspackages/dapi-grpc/protos/platform/v0/platform.protopackages/rs-dpp/Cargo.tomlpackages/rs-dpp/schema/meta_schemas/document/v3/document-meta.jsonpackages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v2/mod.rspackages/rs-drive-abci/Cargo.tomlpackages/rs-drive-abci/src/query/document_query/v1/compute_aggregate_mode_and_check_limit/mod.rspackages/rs-drive-abci/src/query/document_query/v1/compute_aggregate_mode_and_check_limit/v1/mod.rspackages/rs-drive-abci/src/query/document_query/v1/conversions.rspackages/rs-drive-abci/src/query/document_query/v1/mod.rspackages/rs-drive-abci/src/query/document_query/v1/tests.rspackages/rs-drive-proof-verifier/src/lib.rspackages/rs-drive-proof-verifier/src/proof.rspackages/rs-drive-proof-verifier/src/proof/document_ranked.rspackages/rs-drive/Cargo.tomlpackages/rs-drive/src/drive/contract/insert/insert_contract/v0/tests/ranked_index_e2e_tests.rspackages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v2/mod.rspackages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v2/mod.rspackages/rs-drive/src/drive/document/index_level_tree_types.rspackages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v2/mod.rspackages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v2/mod.rspackages/rs-drive/src/drive/document/mod.rspackages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v1/mod.rspackages/rs-drive/src/fees/op.rspackages/rs-drive/src/query/drive_document_ranked_query/drive_dispatcher.rspackages/rs-drive/src/query/drive_document_ranked_query/mod.rspackages/rs-drive/src/query/drive_document_ranked_query/mode_detection.rspackages/rs-drive/src/query/drive_document_ranked_query/tests.rspackages/rs-drive/src/query/having.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_tree_if_not_exists/mod.rspackages/rs-drive/src/util/grove_operations/batch_insert_empty_tree_if_not_exists/v0/mod.rspackages/rs-drive/tests/supporting_files/contract/restaurants/restaurants-contract.jsonpackages/rs-platform-version/Cargo.tomlpackages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v6.rspackages/rs-platform-version/src/version/drive_versions/drive_document_method_versions/v4.rspackages/rs-platform-version/src/version/drive_versions/v9.rspackages/rs-platform-version/src/version/v14.rspackages/rs-platform-wallet/Cargo.tomlpackages/rs-sdk/Cargo.tomlpackages/rs-sdk/src/platform/documents/document_query.rspackages/rs-sdk/src/platform/documents/document_ranked_entries.rspackages/rs-sdk/src/platform/documents/ranked_proof_helpers.rspackages/wasm-sdk/src/queries/system.rs
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/rs-drive/Cargo.toml
- packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v6.rs
- packages/rs-drive/src/drive/document/mod.rs
- packages/rs-platform-version/src/version/drive_versions/v9.rs
- packages/rs-platform-version/Cargo.toml
- packages/rs-sdk/Cargo.toml
- packages/rs-platform-wallet/Cargo.toml
- packages/rs-drive-abci/src/query/document_query/v1/compute_aggregate_mode_and_check_limit/v1/mod.rs
- packages/rs-drive-abci/src/query/document_query/v1/compute_aggregate_mode_and_check_limit/mod.rs
- packages/rs-sdk/src/platform/documents/ranked_proof_helpers.rs
- packages/rs-dpp/Cargo.toml
- packages/rs-drive-proof-verifier/src/lib.rs
- packages/rs-drive-proof-verifier/src/proof/document_ranked.rs
- packages/dapi-grpc/protos/platform/v0/platform.proto
- packages/rs-drive-abci/src/query/document_query/v1/mod.rs
- packages/rs-drive-abci/src/query/document_query/v1/tests.rs
- packages/rs-sdk/src/platform/documents/document_ranked_entries.rs
GROVE_V4 gates the indexed-tree batch cleanup behaviors (overwrite inspection + delete-tree actual-type cleanup namespaces). Indexed trees only exist from protocol v14, so staying on GROVE_V3 would let a batch overwrite of a ranked index orphan its per-axis secondary storage. The gate charges one extra stored-element read per overwrite-capable op, so the latest-version fee-constant tests move up by that read while the first-version tests keep the released values — pinning that the cost change activates exactly at v14. Also narrows the HavingRightOperand ranking-operator doc to what evaluation actually accepts (Equal with Top(1)/Bottom(1), In with Top(N)/Bottom(N)). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/rs-platform-version/src/version/drive_versions/v9.rs (1)
145-151: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftAdd a regression test for the Grove version gate.
DRIVE_VERSION_V9now selectsGROVE_V4, which changes cleanup behavior for ranked-index secondary storage. Add an integration test that overwrites a ranked index in a batch and verifies that obsolete secondary storage is deleted. Also verify that the protocol v13 version does not selectGROVE_V4.As per coding guidelines, the pull request must include tests. The PR objectives state that live protocol v14 tests remain pending.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-platform-version/src/version/drive_versions/v9.rs` around lines 145 - 151, Add an integration regression test covering the DRIVE_VERSION_V9 Grove-version gate: perform a batch overwrite of a ranked index and verify obsolete per-axis secondary storage is removed. Also assert that the protocol v13 version selects a Grove version earlier than GROVE_V4, while DRIVE_VERSION_V9 selects GROVE_V4; keep live protocol v14 coverage unchanged or pending.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/rs-drive/src/drive/identity/update/mod.rs`:
- Line 62: Align the processing_fee expectation near the identity update test
with its explanation: either update the nearby comment to account for the full
4,040-credit increase, remove the insufficient explanation, or correct
processing_fee if the increase is unintended.
---
Nitpick comments:
In `@packages/rs-platform-version/src/version/drive_versions/v9.rs`:
- Around line 145-151: Add an integration regression test covering the
DRIVE_VERSION_V9 Grove-version gate: perform a batch overwrite of a ranked index
and verify obsolete per-axis secondary storage is removed. Also assert that the
protocol v13 version selects a Grove version earlier than GROVE_V4, while
DRIVE_VERSION_V9 selects GROVE_V4; keep live protocol v14 coverage unchanged or
pending.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 13e93bf3-4f42-4286-9369-de1333490a08
📒 Files selected for processing (5)
packages/rs-drive/src/drive/identity/balance/update.rspackages/rs-drive/src/drive/identity/update/mod.rspackages/rs-drive/src/drive/tokens/balance/update.rspackages/rs-drive/src/query/having.rspackages/rs-platform-version/src/version/drive_versions/v9.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-drive/src/query/having.rs
The GROVE_V4 cleanup gates charge one stored-element read per gated batch op from protocol v14. Latest-version fee assertions across drive-abci's execution suites (check_tx, address funds transfer, document deletion/transfer/nft/dpns, token burn/direct selling, identity create/top-up) move by exactly 2000-per-seek + 20-per-byte multiples; storage fees are unchanged everywhere and first-version tests keep the released values. Fresh-key inserts show no delta — the inspection only reads on overwrites — pinned by the untouched 1-input fee-regression cases next to the moved multi-input ones. Also documents the two components of the identity-update delta. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The latest-protocol-version chain run pays one extra seek (2000 credits) from the grove v4 cleanup gates; the balance pin moves accordingly. A new protocol-version-13 sibling pins the released value on the other side, so the pair proves the cost change activates exactly at the v13 -> v14 boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6 tasks
The generation-1 try_from_schema entry point can never run on a meta-schema-v3 platform version, so reading document_type_schema >= 3 inside it was dead logic that implied otherwise. The v1 entry now hardcodes false into an inner try_from_schema_with_ranked_aggregates, and the v2 wrapper — the only caller that can be on protocol v14 — computes the flag and threads it through. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A generation-1 document type never carries ranked aggregates, so a function named with_ranked_aggregates on DocumentTypeV1 implied a capability the generation does not have. The parameter is a grammar switch owned by the caller; the shared core is now named neutrally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Protocol v14 contracts now parse through a new try_from_schema/v3 generation (CONTRACT_VERSIONS_V6.try_from_schema: 3) where the ranked index keywords are admitted as a constant fact of the generation — Index::try_from_value_map(.., true) — instead of a version-table gate threaded through the shipped parsers. Generations 0/1/2 revert to byte-identical upstream outside their test modules. Their unit tests pinned PlatformVersion::latest() while calling their generation directly, which only worked because the shipped files carried a meta-schema arm for a version they can never serve; each generation's tests now pin the highest protocol version that actually selects it (v0→PV8, v1→PV11, v2→PV13), and v1's meta-schema-axis tests pin PV12, the one version pairing their generation with document_type_schema 1 — under latest() they had drifted onto meta-schema v2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
force-pushed
the
claude/grovedb-pr657-integration-b6f426
branch
from
August 3, 2026 09:23
164c785 to
d30f6ee
Compare
document-ranked-trees.md (storage concepts: grammar, indexed-tree variants, sort keys, hash binding, grove v4 fee shape, shared-prefix interaction) and ranked-index-examples.md (worked queries over the restaurants fixture, wire shapes, SDK example, rejection table), registered in SUMMARY.md and cross-linked from the Index field docs like their count/sum/average siblings. Also corrects comments the chapters exposed as stale: the proto's having field doc still described the pre-v14 blanket rejection (and called the shape repeated WhereClause), and three v3 parser comments attributed the ranked prerequisites to dependentRequired when the schema implements them as value-sensitive if/then conditionals. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The version tables pair each try_from_schema generation with its document_type_schema value, so a table value of 3 already means generation 3 is running — the flag only guarded a pairing no shipped table can produce, and the grammar a generation admits is gated by its own ParserGeneration flags (admit_ranked and the key-length hook), not by which meta-schema validated the JSON. select_document_meta_schema is now a total function of the table value with known_versions [0, 1, 2, 3], and validate_against_meta_schema_and_compile loses the threaded parameter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
force-pushed
the
claude/grovedb-pr657-integration-b6f426
branch
from
August 4, 2026 09:55
a8ef33f to
a77cbe4
Compare
Three test comments still described the per-overwrite stored-element read surcharge from the pre-#790 grovedb gates. The fee reversion restored the released constants (PV14 fees equal PV13; the gates are cost-neutral), so the prose contradicted the values beside it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
approved these changes
Aug 4, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Codex + Sonnet
All three cumulative prior findings are fixed at the current head: ranked key ceilings are enforced during schema parsing, ranked indexes reject nullSearchable=false, and the protobuf documentation now matches protocol-v14 HAVING behavior. No prior finding remains active, and the latest-delta review found no new actionable defect; the only newly reported suggestion is refuted by the parser invariant and contract-insertion call order.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— final-verifier (fallback) - Sonnet reviewers:
claude-sonnet-5— general (completed),claude-sonnet-5— security-auditor (completed),claude-sonnet-5— rust-quality (completed)
grovedb #791: the sum axis's secondary Merk (PSIT's lone secondary and PCPSIT's sum axis) is now a ProvableCountProvableSumTree, so positional proofs against sum rankings become possible, and the indexed-axis paginated proofs gain count-bound offsets on all three axes (O(log n + k) at any offset, attested skip, provable offset-past-end) plus a rank-of-key variant. Platform's query surface is unchanged in this commit — the top-k call sites compiled untouched and the new primitives are additive, to be exposed in a future protocol version. No fee movement: the secondary node-shape change only affects ranked writes, which exist only at PV14 with no exact-fee pins, and grovedb's estimators derive from the same NodeType mapping so the >=-actual estimation contracts hold without constant changes. Full battery green: drive 3316/0, dpp 3848/0, drive-abci 2642/0 (PV13/PV14 parity pairs unchanged), proof-verifier, fmt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…OFFSET
The ranking grammar leaves `having` and becomes the SQL surface the
wire already carried: group_by + a single order_by naming the selected
aggregate ("$count" sentinel for COUNT(*)) + limit as k + the
previously always-rejected offset field, now consumed in ranked mode
as a count-attested skip (grovedb a2791bbd paginated proofs:
O(log n + k) at any offset, so offset is uncapped and documented as
such). "5th best by average grade" is ORDER BY avg DESC LIMIT 1
OFFSET 4, with the starting rank cryptographically attested via the
returned skipped count. Non-ranked paths keep the verbatim
Unsupported rejection for offset.
having loses the Ranking operand entirely (with the wire-stable-but-
rejected MAX/MIN variants and their tie-semantics error): it is a
boolean per-group predicate again, and it cannot yet combine with an
aggregate ordering. The pagination-limit-must-equal-n redundancy is
structurally gone.
Also: the paginated prover handles an empty axis secondary (guaranteed
empty range), so the empty-ranking prove gap is closed — reads and
proofs of an empty ranking both succeed and the tests that pinned the
old limitation now pin the capability.
drive 3322/0 (+6: offset paging, attested-rank binding, $count
sentinel, three offset e2e cases incl. offset-past-end population
proof), drive-abci 2645/0 (+3 incl. the verbatim off-path offset
rejection pin), clippy/fmt clean. rs-sdk adaptation and the
RankedEntries.skipped wire field follow in the next commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Completes the ranked-grammar rework: the HavingRanking message and its
oneof arm leave the proto entirely (removed before release rather than
deprecated), RankedEntries gains the attested skipped field (starting
rank base; equals the attested total population when the offset runs
past the end), and the offset request field's contract changes from
always-rejected to consumed-in-ranked-mode. Clients regenerated with
the pinned toolchain — surgical diff, no generator drift.
SDK: ranking_having is replaced by order_by_selected_aggregate +
with_offset ("5th best by average grade" = select avg + group_by +
Descending + limit 1 + offset 4); DocumentRankedEntries carries the
starting rank; encode_v0 refuses a set offset instead of dropping it;
the empty-ranking rejection helper is deleted because the paginated
prover closed that gap. Book chapters rewritten to the ORDER BY
grammar with a new ranks-and-offsets section; the empty-ranking
caveat is flipped to a capability.
dash-sdk 191/0 (+6 incl. the fifth-best wire round-trip and the
uncapped-deep-offset pin), proof-verifier 262/0 (+4 rank decoding),
drive-abci 2644/0, workspace check + clippy + fmt clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
force-pushed
the
claude/grovedb-pr657-integration-b6f426
branch
from
August 4, 2026 19:15
5fc92fe to
8ebd879
Compare
…657-integration-b6f426 # Conflicts: # packages/rs-platform-wallet/src/wallet/identity/network/encrypted_document.rs
Review follow-up: v3's tests used PlatformVersion::latest(), which silently retargets them onto a different parser generation and meta-schema whenever LATEST moves — the same hazard already fixed for generations 0/1/2. Adds the pv14() helper mirroring pv13() and pins all nine uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Conflicts resolved in 0bbe28e (v4.2-dev's txMetadata revert deleted a file this branch had only mechanically touched — the deletion wins) and CodeRabbit's PV14-pin nitpick is addressed in 94cd599: v3's tests now pin 🤖 Addressed by Claude Code |
RankedEntry's avg_fixed_point (16-byte BE i128) becomes double avg at the same field number: the RankedEntry wire messages are populated only on the no-proof quick-answer path — a proof-verifying client reconstructs entries from the proof itself, where the exact committed fixed point lives regardless — so the byte-exactness rationale never applied to this surface, and JS clients are spared 128-bit big-endian decoding. Ranking order stays exact (ordering happens over the i128 before conversion); the comment states the f64 precision bound and where exactness lives. The no-proof decode reconstructs the internal fixed point best-effort (documented as such at type, method, and re-export level) and rejects non-finite or out-of-range doubles rather than saturating — f64 as i128 maps NaN to 0, which would decode a malformed value as a confident average of zero. Clients regenerated surgically with the pinned toolchain. One test fixture moved from i128::MIN (unreachable, non-round-tripping) to the largest-magnitude average the axis can produce; every other fixture round-trips bit-exactly and comparisons stay ==. drive-abci 2644/0, proof-verifier 262/0, dash-sdk 191/0, workspace check + clippy + fmt clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
added a commit
that referenced
this pull request
Aug 5, 2026
One textual conflict and one semantic one. `.github/workflows/tests-rs-workspace.yml`: #4287 collapsed the macOS job and the Ubuntu backup job into a single `[self-hosted, rust-ci]` job and dropped the cdylib->rlib strip step entirely. Took that structure, so this branch's edits to the strip step fall away with the step itself (the surviving job already runs cdylib crates such as rs-sdk-ffi unstripped). Kept this branch's addition of platform-encryption and rs-unified-sdk-jni to the package list; the resolved file is now exactly the base plus those two lines. The wallet workflow's rs-unified-sdk-jni entry merged cleanly. `DocumentQuery` gained an `offset` field in #4266 (ranked aggregate indexes, protocol v14), so the three txMetadata query builders no longer compiled. All three set `offset: None`: the encrypted-document scan pages by insertion-order cursor, and `offset` is served only on the ranked surface — a skip-count page would silently drop documents whenever the owner writes between round-trips. The two test builders mirror the production query, so they take the same value. cargo test -p platform-wallet -p platform-wallet-ffi -p rs-unified-sdk-jni: 710 / 267+26+6+9 / 52 passed, 0 failed. cargo fmt --all -- --check and clippy --all-targets on the three crates: clean.
QuantumExplorer
added a commit
that referenced
this pull request
Aug 5, 2026
The ranked aggregate index grammar (#4266) only exists at protocol v14, where validate_update now dispatches to v1, so the rankedAverageable update tests added to the v0 module assert the v1 rejection shape ("changed index 'byRestaurant'") and belong in the v1 module. The ranked flags ride the same name-keyed index-definition comparison as every other index property. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 5, 2026
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 5, 2026
…b into v41-keystore-qa5 Reconstruction of PR dashpay#4301 (feat/shielded-two-note-invites @d6610262b0) on top of qa5's tip. dashpay#4301 is based on a current v4.2-dev point; merging it directly would have dragged in 9 unrelated commits (dashpay#4287, dashpay#4266, dashpay#4279, dashpay#4276, dashpay#4278, dashpay#4277, and duplicate dashpay#4183/dashpay#4191/dashpay#4251), including dashpay#4277's competing encrypted-txMetadata implementation that collides with qa5's dashpay#4186. The reconstruction cherry-picks ONLY dashpay#4301's own commit; the resulting delta is byte-identical to the original (1256 insertions, 8 deletions across the same 11 files) — only hunk offsets differ. Verified: platform-wallet 774/774, platform-wallet-ffi 284/284, dpp shielded 216/216 (incl. both multi/single-output fee-vs-action-count tests), the four dashpay#4204 security tests, and dashpay#4301's three note-selection tests all pass. rustfmt clean; no new clippy warnings.
This was referenced Aug 12, 2026
Draft
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Platform's 4.0 aggregates can compute counts, sums and averages over document groups, but answering "which groups rank highest?" (e.g. top 5 restaurants by average grade) requires scanning every group —
O(n)work with anO(n)proof, so it was never offered.GroveDB's indexed-tree family (dashpay/grovedb#657, follow-ups in dashpay/grovedb#781) pairs each aggregate-bearing tree with per-axis ordered secondaries, making provable top-K reads
O(log n + k). This PR integrates that at the contract level and activates it at protocol v14.What was done?
Contract grammar (rs-dpp, meta-schema v3,
document_type_schema >= 3)rankedCountable/rankedSummable/rankedAverageable, each requiring itsrange*counterpart; parsed intoIndex/IndexLevelTypeInfofind_first_ranked_change); pre-v14 contracts get the same "unexpected property name" rejection as beforeStorage (rs-drive)
ProvableCountIndexedTree/ProvableSumIndexedTree/ProvableCountProvableSumIndexedTree(axes)— a byte-compatible mirror of the tree it replaces, so existing range-aggregate queries keep working on itdrive/document/ranked_index_tree_type.rsused by contract insert/update and all document index walkers; three newbatch_insert_empty_provable_*_indexed_treegrove-op helpers; estimation maps indexed trees to their non-indexed mirror weights (estimated ≥ actual verified per variant)Queries (rs-drive → rs-drive-abci → wire)
query/drive_document_ranked_query/: validation (detect_ranked_mode, versioned), executors overindexed_{count,sum,avg}_top_kandprove_indexed_axis_top_k,MAX_RANKED_LIMIT = 100;verify/document_ranked/wrapsGroveDb::verify_indexed_axis_top_kdocument_query/v1:compute_aggregate_mode_and_check_limitv1 routes a singleHAVINGranking clause (TOP(n)/BOTTOM(n)/MAX/MIN) todispatch_ranked_v1; protocol v13 keeps feature version 0 and still rejects everyHAVING, so mixed-version networks agreeRankedEntry/RankedEntriesmessages and aResultData.rankedarm; requests reuse the existingselects/group_by/havingfields ofGetDocumentsRequestV1. Avg entries carry the grovedb fixed-point i128 (scaleAVG_FIXED_POINT_SCALE, currently 10^19) as 16 BE bytes; clients divideClients (rs-drive-proof-verifier, rs-sdk)
DocumentRankedEntries: Fetchwith proof verification bound to the signed app hash viaverify_tenderdash_proof;ranking_havingconstructor; empty-ranking-with-proof surfaces as an explicit error (grovedb has no absence envelope for indexed-axis proofs)Versioning (rs-platform-version)
CONTRACT_VERSIONS_V6(meta-schema v3),DRIVE_VERSION_V9(detect_ranked_mode+ grove/verify method slots),DRIVE_ABCI_QUERY_VERSIONS_V2(ranked routing gate). Behaviorally identical to v13 until a contract uses the new grammarv5.0.1→ develop4c6720b(contains chore(drive): comprehensive logging for same block execution #657 + feat(dashmate): pack release script #781; feat(dashmate): pack release script #781 also fixes a subset-verification regression that chore(drive): comprehensive logging for same block execution #657 introduced for pre-existing shielded proofs)How Has This Been Tested?
rs-dppindex grammar/immutability/gating tests (28) — parsing, desugaring, per-flag rejections, PV13 vs PV14 acceptancers-driveinsert_contract/v0/tests/ranked_index_e2e_tests.rs— all three variants through real batches: registration shape (axes TLV), ranking across inserts/updates/deletes,verify_grovedbintegrity after every test;restaurantsfixture addedrs-drivedrive_document_ranked_query/tests.rs(28) — validation matrix, all axes/kinds, prove→verify round-trips against the live root hash, tamper rejection, tie ordering, multi-index prover/verifier index agreementrs-drive-abcidocument_query/v1/tests.rsranked module (13) — wire-level requests per axis, proof responses, PV13 rejection, empty-ranking-prove clean error, malformed-shape rejectionsrs-drive-proof-verifier(12) +rs-sdk(7 unit + doctest) — wire round-trip against the abci ground-truth shape, 16-byte avg decode incl. negatives, order preservationcargo test -p drive --lib3300 ·-p dpp --lib3834 ·-p drive-abci --lib query::604 ·-p platform-version13 ·-p drive-proof-verifier257 ·-p dash-sdk --lib/--doc181/12 — all 0 failures;cargo check --workspace --all-targets,cargo check -p drive --no-default-features --features verify, clippy clean on all touched crates,cargo fmtcleanNot yet covered (needs a live PV14 network, which doesn't exist): recorded SDK
tests/vectorsfixtures and live e2e; JS/Java/ObjC/Python generated gRPC clients still need regeneration (yarn workspace @dashevo/dapi-grpc build, docker).Breaking Changes
None for existing networks or clients: the new consensus behavior is entirely gated behind protocol v14 (a v13→v14 upgrade is a no-op until a contract uses the new grammar), the wire change is additive, and existing aggregate queries are unaffected. Merging does make nodes signal v14 as their desired version.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
HAVING TOP(n)andBOTTOM(n)for count, sum, and average results.MIN/MAXlimitations.