refactor(sdk): shared wire-request decode and pure DPNS/DashPay document builders

[PastaPastaPasta]

Issue being fixed or feature implemented

Third slice of the feat/transport-free-embedder-core series (#4335; after #4344, #4345, and #4388): gives transport-free embedders the remaining pieces they need to construct and verify Platform interactions without reimplementing SDK logic — the drift-prone code Dash Core's Platform GUI (PastaPastaPasta/dash#67, dashpay/dash#7512) currently hand-builds in C++.

Stacked on #4388 (base branch refactor/dash-platform-queries); will be retargeted to v4.2-dev when that merges. Only the last four commits are new.

What was done?

• Wire-request decode: DocumentQuery::try_from_request decodes a wire-format GetDocumentsRequest back into a rich DocumentQuery — the inverse of request encoding — by lifting drive-abci's server-side proto conversions into shared client code, so the bytes the server decodes and the client verifies go through the same code. drive-abci now consumes the shared conversions (deduplicated, −382 lines in its conversions.rs). Round-trip coverage in tests/document_query_wire_roundtrip.rs.
• Request-driven proof verification: documents::verify_documents_response delegating to drive-proof-verifier's FromProof, keyed on the exact request bytes sent. It binds the proof to the whole request, not just the part that survives lowering: GroveDB/Tenderdash proofs authenticate the state and the resolved DriveDocumentQuery, and the DocumentQuery → DriveDocumentQuery lowering drops group_by, having, offset and prove — so each of those is rejected up front, mirroring rs-drive-abci's validate_and_route and reject_offset_off_the_ranked_path. Without that, an untrusted transport could pair a request the real server would have refused (SELECT DOCUMENTS … GROUP BY age) with a genuine proof for the narrower query it lowers to. The limit conversion is likewise checked (u16::try_from) rather than an as cast, which silently turned a request for 65537 documents into a 1-document query.
• Pure document builders: build_dpns_preorder_and_domain_documents (salted-domain-hash preorder/domain pair) and dashpay::build_contact_request_document, extracted from rs-sdk's networked flows. Crypto material is supplied by the caller — ECDH/key custody stays out of this crate.
• Consensus-aligned DPNS validation: split is_consensus_valid_label (matches the DPNS contract regex; gates the builders) from is_valid_username (stricter client-side policy, e.g. consecutive-hyphen rejection) so builders cannot reject labels the contract accepts.
• rs-sdk re-exports everything at its old paths; no consumer changes imports.

How Has This Been Tested?

• cargo test -p dash-platform-queries (41 unit + 15 integration tests, including wire round-trip, request-binding rejections, and builder/validation coverage); cargo check for dash-sdk and drive-abci; cargo fmt --check; clippy clean.
• The equivalent code (as part of the full series branch) is exercised end-to-end by feat(qt): back Dash Platform GUI internals with real grovedb/drive/dpp crates PastaPastaPasta/dash#67 including live-testnet E2E (username registration, contact requests).

Breaking Changes

None beyond those already declared by the base PR #4388. Moved items remain importable at their previous dash_sdk paths; drive-abci's request decoding behavior is unchanged (same conversions, now shared).

One behavior change worth calling out: TryFrom<&DocumentQuery> for DriveDocumentQuery now returns an error for a limit above u16::MAX instead of wrapping it. The server rejects such limits outright (QuerySyntaxError::InvalidLimit), so the previous as cast could only ever produce a query the caller did not ask for. No existing test depended on the truncation.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9e56606a-cecc-4a76-99bf-b5f9f4c73632

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

⛔ Blockers found — Opus deferred (commit 5435934)
Canonical validated blockers: 2

[thepastaclaw]

Preliminary review — Codex only

The shared decoders and extracted builders largely preserve the existing behavior, but the new request-driven document verifier does not bind verification to every semantically relevant request field. A malicious transport can therefore substitute a valid proof for a different query, so this trust-boundary issue must be fixed before merging.
Source: reviewer backend model gpt-5.6-sol; final verifier backend model gpt-5.6-sol. Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol (not reviewer evidence).

Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 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/dash-platform-queries/src/documents/document_query.rs`:
- [BLOCKING] packages/dash-platform-queries/src/documents/document_query.rs:665-678: Reject request fields that are discarded before proof verification
  Validating only the `select` projection does not ensure that the proof corresponds to the wire request. The subsequent `TryFrom<&DocumentQuery> for DriveDocumentQuery` conversion discards `group_by` and `having`, even though the server rejects both for `SELECT DOCUMENTS`; consequently, an untrusted transport can pair a request such as `SELECT DOCUMENTS GROUP BY age` with a valid proof for the corresponding plain document query and this function will accept it. The conversion also narrows `request.limit` with `as u16` at lines 1127-1129, so a wire limit of 65537 becomes 1 even though the server rejects limits above `u16::MAX`. Plain-document `offset` and a false `prove` flag are additional request shapes that cannot produce this proved response from the real server but are not rejected here. GroveDB and Tenderdash proofs authenticate the state and resolved Drive query, not the discarded request envelope. Before delegating, reject every field incompatible with a proved plain-document request (`group_by`, `having`, `offset`, and `prove == false`) and use a checked `u16::try_from` conversion for the limit so no request information is silently changed.

[thepastaclaw]

🔴 Blocking: Reject request fields that are discarded before proof verification

Validating only the select projection does not ensure that the proof corresponds to the wire request. The subsequent TryFrom<&DocumentQuery> for DriveDocumentQuery conversion discards group_by and having, even though the server rejects both for SELECT DOCUMENTS; consequently, an untrusted transport can pair a request such as SELECT DOCUMENTS GROUP BY age with a valid proof for the corresponding plain document query and this function will accept it. The conversion also narrows request.limit with as u16 at lines 1127-1129, so a wire limit of 65537 becomes 1 even though the server rejects limits above u16::MAX. Plain-document offset and a false prove flag are additional request shapes that cannot produce this proved response from the real server but are not rejected here. GroveDB and Tenderdash proofs authenticate the state and resolved Drive query, not the discarded request envelope. Before delegating, reject every field incompatible with a proved plain-document request (group_by, having, offset, and prove == false) and use a checked u16::try_from conversion for the limit so no request information is silently changed.

_{source: ['codex']}

[PastaPastaPasta]

Verified against rs-drive-abci and fixed — this was a real hole.

Server rules confirmed in packages/rs-drive-abci/src/query/document_query/v1/mod.rs:

• validate_and_route, SelectFunction::Documents arm: non-empty group_by → InvalidArgument.
• validate_and_route, ahead of the per-function gates: non-empty having with a non-aggregate SELECT → not_yet_implemented("HAVING clause").
• reject_offset_off_the_ranked_path: offset.is_some() on any non-Ranked decision → not_yet_implemented("OFFSET pagination …"). A documents fetch never routes to Ranked.
• query_documents_typed (v0/mod.rs, shared by both wire versions): limit > u16::MAX → QuerySyntaxError::InvalidLimit.

So all four are shapes an honest server refuses, and all four are dropped by the DocumentQuery → DriveDocumentQuery lowering. prove == false isn't a server rejection but has the same effect: the server answers without a proof, so a proved response cannot belong to that request.

Changes:

• New reject_request_the_server_would_not_have_proved(&query, prove) gate in verify_documents_response, run before delegating to FromProof. It rejects prove == false, non-documents projections (the pre-existing check, moved in), non-empty having, non-empty group_by, and any offset. prove is read off the wire request before decode, since DocumentQuery has no such field.
• TryFrom<&DocumentQuery> for DriveDocumentQuery now uses u16::try_from for the limit. You were right that this one is objectively wrong independent of the security argument — 65537 wrapped to 1. It now matches the offset conversion immediately below it, which already refused rather than truncated.
• Five tests in tests/document_query_wire_roundtrip.rs (verify_binds_the_whole_request::*) plus limit_above_u16_max_is_rejected_not_truncated. The tests pass a ContextProvider that panics on every method, so they also pin that the rejection fires before any proof machinery runs.

try_from_request itself is unchanged — it still decodes these fields faithfully, which is what makes it a true inverse of encoding. Its "scope caveat" doc now points at the entry point that closes the gap.

No existing test expectations changed.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[thepastaclaw]

Resolved in this update — Reject request fields that are discarded before proof verification no longer present.

_{Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.}

[thepastaclaw]

Preliminary review — Codex only

The shared proto conversions and extracted document builders generally preserve existing behavior and add useful deterministic coverage. However, the request-driven verifier still reduces the wire request to a narrower Drive query without validating every field that the server uses for routing, allowing a fabricated response to verify against a request the server would reject.
Source: reviewer backend model gpt-5.6-sol; final verifier backend model gpt-5.6-sol. Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol (not reviewer evidence).

Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 1 blocking

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

🤖 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/dash-platform-queries/src/documents/document_query.rs`:
- [BLOCKING] packages/dash-platform-queries/src/documents/document_query.rs:681-694: Reject request fields that are discarded before proof verification
  (existing thread: https://github.com/dashpay/platform/pull/4389#discussion_r3771773859)
  Checking only that `select` is the documents projection does not bind verification to the complete wire request. The subsequent `TryFrom<&DocumentQuery> for DriveDocumentQuery` conversion discards `group_by` and `having`, although the server rejects both when used with `SELECT DOCUMENTS`. It also narrows a nonzero `u32` limit with `as u16`, so a request for 65537 documents is verified as a limit-1 query even though the server rejects limits above `u16::MAX`. A plain-document `offset` is lowered into a Drive query despite being rejected by the server's routing layer, and `prove == false` is ignored even though an honest server cannot return this proved response for such a request. Consequently, an untrusted transport can pair one of these server-invalid requests with a valid proof for the reduced Drive query and this function accepts it. Before delegating, reject nonempty `group_by` or `having`, any plain-document `offset`, and `prove == false`, and convert the limit with `u16::try_from` so no request information is silently changed.

[thepastaclaw]

Preliminary review — Codex only

The prior request-envelope binding issue is fixed: discarded fields are now rejected before proof verification, and limit narrowing is checked. Two server-parity gaps remain in the new request-driven verifier: it accepts wire versions disabled by the supplied PlatformVersion and explicit document limits above the server's canonical cap, allowing proofs to authenticate request/response pairings that an honest server could not produce.
Source: reviewer backend model gpt-5.6-sol; final verifier backend model gpt-5.6-sol.
Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol (not reviewer evidence).

Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 2 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/dash-platform-queries/src/documents/document_query.rs`:
- [BLOCKING] packages/dash-platform-queries/src/documents/document_query.rs:761-767: Reject document wire versions unavailable at the target platform version
  The verifier dispatches on the request oneof without checking whether that wire version is enabled by `platform_version.drive_abci.query.document_query`. The server performs this check in `rs-drive-abci/src/query/document_query/mod.rs` before decoding or executing the request. For example, PlatformVersions 1–11 have bounds `min_version = 0, max_version = 0`, so their servers reject every V1 request with `UnsupportedQueryVersion`; this verifier instead decodes the V1 request and can verify a genuine V0 proof for the equivalent lowered `DriveDocumentQuery`. An untrusted transport can therefore attach a valid proof to a request that an honest server at the supplied PlatformVersion could not have answered. Derive the request feature version (`V0 = 0`, `V1 = 1`) and reject it unless the supplied version bounds accept it before decoding or delegating to `FromProof`.
- [BLOCKING] packages/dash-platform-queries/src/documents/document_query.rs:1234-1245: Enforce the server's document limit cap during verification
  The checked `u16` conversion prevents wrapping, but it still accepts explicit limits from 101 through 65535. The server passes the converted value to `DriveDocumentQuery::from_typed_clauses`, which rejects any value above `DriveConfig::default().default_query_limit` (`DEFAULT_QUERY_LIMIT`, currently 100). The verifier bypasses that constructor and creates a raw `DriveDocumentQuery` with values such as `limit = Some(101)`. If the matching range contains fewer documents than either limit, a genuine proof for a server-valid query can also satisfy the larger path query, so verification accepts a fabricated request/response pairing that an honest server would reject. Validate explicit plain-document limits against the same canonical limit used by proof verification, rather than only checking whether they fit in `u16`.

[thepastaclaw]

🔴 Blocking: Reject document wire versions unavailable at the target platform version

The verifier dispatches on the request oneof without checking whether that wire version is enabled by platform_version.drive_abci.query.document_query. The server performs this check in rs-drive-abci/src/query/document_query/mod.rs before decoding or executing the request. For example, PlatformVersions 1–11 have bounds min_version = 0, max_version = 0, so their servers reject every V1 request with UnsupportedQueryVersion; this verifier instead decodes the V1 request and can verify a genuine V0 proof for the equivalent lowered DriveDocumentQuery. An untrusted transport can therefore attach a valid proof to a request that an honest server at the supplied PlatformVersion could not have answered. Derive the request feature version (V0 = 0, V1 = 1) and reject it unless the supplied version bounds accept it before decoding or delegating to FromProof.

_{source: ['codex']}

[thepastaclaw]

🔴 Blocking: Enforce the server's document limit cap during verification

The checked u16 conversion prevents wrapping, but it still accepts explicit limits from 101 through 65535. The server passes the converted value to DriveDocumentQuery::from_typed_clauses, which rejects any value above DriveConfig::default().default_query_limit (DEFAULT_QUERY_LIMIT, currently 100). The verifier bypasses that constructor and creates a raw DriveDocumentQuery with values such as limit = Some(101). If the matching range contains fewer documents than either limit, a genuine proof for a server-valid query can also satisfy the larger path query, so verification accepts a fabricated request/response pairing that an honest server would reject. Validate explicit plain-document limits against the same canonical limit used by proof verification, rather than only checking whether they fit in u16.

_{source: ['codex']}