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 seven commits are new (the original four plus the trust-boundary hardening described below).

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 request envelope is bound the same way: the wire version (V0/V1 oneof arm) is checked against platform_version.drive_abci.query.document_query's feature-version bounds before anything is decoded — the same check_version gate the server's query_documents dispatch runs — and the query limit is capped at the server's compile-time default (100) rather than only u16::try_from, since limits 101..=65535 fit a u16 but the server refuses them with InvalidLimit. The COUNT/SUM/AVG proof verifiers share the same cap through one aggregate_limit gate that runs before any proof or context-provider machinery.
• 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 + 20 integration tests, including wire round-trip, request-binding rejections — wire-version bounds, over-cap limits on both the documents and aggregate verify paths, each pinned to fire before proof machinery via a panicking ContextProvider — and builder/validation coverage); cargo test -p dash-sdk --lib; 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).

Two behavior changes worth calling out:

• TryFrom<&DocumentQuery> for DriveDocumentQuery now rejects any limit above the server's cap (100, DEFAULT_QUERY_LIMIT) instead of only limits above u16::MAX, and over-cap limits surface as Error::Drive(QuerySyntaxError::InvalidLimit) — the server's own error — instead of the previous Error::Config. No in-tree caller matched on Error::Config, and the server refuses every such limit before answering, so the previous behaviors could only ever produce a query the server would not have served.
• The verifier caps limits at the compile-time config defaults (DEFAULT_QUERY_LIMIT / DEFAULT_MAX_QUERY_LIMIT, both 100). A node operator who raises max_query_limit above 100 serves limits this verifier will refuse. That trade-off is accepted deliberately: verification parity targets the canonical configuration, the SDK cannot observe per-operator tuning, and proof bytes must not depend on it.

Follow-ups deliberately not in this PR

• The rich-object path (FromProof with a caller-constructed DocumentQuery) still routes through the same conversion that silently drops group_by/having for plain document fetches. The new pre-proof rejections cover only the wire-request entry points; a caller who hand-builds a server-invalid rich object and verifies against a malicious node's proof retains the (pre-existing, misuse-conditional) parity gap. The limit cap, by contrast, was fixed in the shared conversion and covers both paths.

• Wire-version gating for the aggregate verify entry points (COUNT/SUM/AVG and the split variants): they take an already-decoded DocumentQuery with no wire-request wrapper, so there is no oneof arm to check yet; adding wrappers analogous to verify_documents_response is tracked as follow-up work.

• Mode-aware limit-absence enforcement: the server's CountMode::accepts_limit() rejects any Some(limit) for the Aggregate (and GroupByIn) modes, while the verifier currently applies only the numeric cap — so a limit: 50 aggregate request verifies against a proof produced for limit: None today. Closing that requires a mode-aware check, not a numeric cap.

[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: 95f2ae9a-b76e-454d-a9bd-43f1c72492f8

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 39cfc28)
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]

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]

Final validation — Codex/Sol only (Phase 2 disabled)

The two prior trust-boundary blockers are fixed: document verification now rejects unsupported wire versions before decoding or provider access, and plain-document limits above the canonical server cap are rejected before proof verification. No blocking issue remains, but the new embedder API needs security guidance, the shared decoder exposes unnecessary implementation details, and the aggregate-limit hardening needs stronger boundary coverage. Tests could not be rerun because cargo is unavailable in this environment.
Source: reviewer backend model gpt-5.6-sol (general, security-auditor, rust-quality); final verifier backend model gpt-5.6-sol. Orchestration-only: openclaw-agent/cliproxy/gpt-5.6-sol (not reviewer evidence).

Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.

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 — verifier
• Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
• Secondary pass: disabled (temporary_phase2_sonnet_disable)

🟡 3 suggestion(s)

🤖 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/dpns_usernames.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/dpns_usernames.rs:29-34: Document the DPNS salt secrecy and reveal-order requirements
  This new transport-free builder intentionally delegates randomness and submission ordering to its caller, but its public contract only says that the caller supplies a salt. The existing networked SDK generates a fresh salt from `StdRng::from_entropy()` and waits for the preorder response before submitting the domain document. Embedders need the same security requirements: a predictable or reused salt allows observers to dictionary-test likely labels against `sha256d(salt || normalized_label || ".dash")`, while publishing the domain document before preorder confirmation reveals the commitment preimage early. Document that every registration requires a fresh CSPRNG-generated salt and that the salt, label, and domain document must remain private until the preorder is confirmed.

In `packages/dash-platform-queries/src/documents/proto_conversions.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/documents/proto_conversions.rs:75-298: Keep internal proto conversion primitives out of the public API
  The extraction widened `where_operator_from_proto`, `value_from_proto`, `where_clause_from_proto`, `order_clause_from_proto`, and `having_clause_from_proto` from the old server module's `pub(super)` visibility to public crate API. Repository-wide callers only use these singular helpers inside this module; cross-crate consumers require `DecodeError`, the three plural request-level decoders, and `select_from_proto`. Every unnecessary `pub` function becomes downstream semver surface and constrains future changes to depth limits, validation, and error classification. Keep the singular helpers private while retaining public visibility for the actual cross-crate facade.

In `packages/dash-platform-queries/tests/document_query_wire_roundtrip.rs`:
- [SUGGESTION] packages/dash-platform-queries/tests/document_query_wire_roundtrip.rs:576-609: Exercise all aggregate limit paths and sentinel translations
  The new aggregate-cap test invokes only `DocumentCount`, and every case exits from `check_within_server_cap` before either centralized walk conversion runs. The current implementation correctly calls the gate from COUNT, SUM, and AVG, but this test would not catch a future omission from the SUM or AVG entry point, nor would it catch swapping the proof-sensitive sentinel translations (`0` to `DEFAULT_QUERY_LIMIT` for distinct walks and `0` to `None` for carrier walks). Add direct boundary coverage for `0`, `1`, `DEFAULT_MAX_QUERY_LIMIT`, and cap-plus-one, and exercise the over-cap rejection through `DocumentCount`, `DocumentSum`, and `DocumentAverage`.

[thepastaclaw]

Preliminary review — Codex only

The shared decoders and extracted builders preserve the intended ownership boundaries, and all three prior suggestions are fixed at the exact head. Two in-scope proof-binding defects remain: omitted plain-document limits are reconstructed as unbounded queries, and COUNT range-outer carrier proofs use the wrong default and cap; both can make verification diverge from the server. Tests could not be rerun because cargo is unavailable in this environment.
Source: reviewer backend model gpt-5.6-sol (general, security-auditor, rust-quality); 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), gpt-5.6-sol — security-auditor (completed), gpt-5.6-sol — rust-quality (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 2 blocking | 🟡 2 suggestion(s)

🤖 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/aggregate_limit.rs`:
- [BLOCKING] packages/dash-platform-queries/src/documents/aggregate_limit.rs:76-87: Preserve the range-outer carrier limit during COUNT verification
  `carrier_walk_limit` treats every carrier proof as the In-outer shape, but COUNT also uses `RangeAggregateCarrierProof` for the G8 range-outer shape: `GROUP BY` one range field with two range clauses on distinct fields. `DriveDocumentCountQuery::detect_mode_versioned` routes that query to the carrier mode, while the server converts an omitted limit to `Some(MAX_CARRIER_AGGREGATE_OUTER_RANGE_LIMIT)` (currently 10). The verifier passes `None` from `count_proof_helpers.rs:280`, so `verify_carrier_aggregate_count_proof` reconstructs a different proof-sensitive `SizedQuery::limit` and rejects an honest server proof. The shared 100-item cap also permits explicit G8 limits from 11 through 100 even though the server rejects them above the carrier-specific cap of 10. Make COUNT carrier translation shape-aware: range-outer queries must use the compile-time range-outer default for limit 0 and enforce that carrier-specific cap, while In-outer queries retain `None` for an omitted limit.
- [SUGGESTION] packages/dash-platform-queries/src/documents/aggregate_limit.rs:45-87: Encode aggregate-limit validation in the returned type
  `check_within_server_cap` returns `()`, while `distinct_walk_limit` and `carrier_walk_limit` independently accept the original `u32` and narrow it with `as u16`. Every current COUNT, SUM, and AVG caller runs the check first, but Rust does not encode that ordering invariant, and the `debug_assert!` disappears from release builds. A future internal caller that skips the separate gate can therefore truncate an over-wide value and reconstruct the wrong proof query. Return a private validated-limit newtype whose methods perform the walk-specific conversions, or make each conversion validate and return `Result`; callers can still create the validated value at function entry to preserve pre-provider rejection ordering.

In `packages/dash-platform-queries/src/documents/document_query.rs`:
- [BLOCKING] packages/dash-platform-queries/src/documents/document_query.rs:1301: Apply the server default when lowering an omitted document limit
  The `0` sentinel represents an omitted wire limit, but this branch constructs a raw `DriveDocumentQuery` with `limit: None`. Nothing subsequently applies the server default: `DriveDocumentQuery::construct_path_query` forwards this field directly into `SizedQuery`, where `None` is unbounded. The server instead maps V0 `limit = 0` and V1 `limit = None` to `Some(self.config.drive.default_query_limit)` before calling `from_typed_clauses`, which stores the concrete limit. Under the canonical configuration, an honest request therefore proves at most 100 documents while this verifier accepts an authenticated proof for every matching document. A malicious full node can exploit that mismatch to return more documents than the request could legitimately produce and control excess proof processing. Lower the sentinel to the canonical compile-time default and update the test that currently asserts `None`.

In `packages/dash-platform-queries/tests/document_query_wire_roundtrip.rs`:
- [SUGGESTION] packages/dash-platform-queries/tests/document_query_wire_roundtrip.rs:340-346: Pin the InvalidLimit error variant in the regression test
  This PR intentionally changes over-cap lowering from `Error::Config` to `Error::Drive(QuerySyntaxError::InvalidLimit)`, but this regression test checks only a fragment of the rendered message. Returning the old error category with the same text would still pass. Match the typed variant and its payload so the documented error-surface change remains covered.

[thepastaclaw]

🔴 Blocking: Preserve the range-outer carrier limit during COUNT verification

carrier_walk_limit treats every carrier proof as the In-outer shape, but COUNT also uses RangeAggregateCarrierProof for the G8 range-outer shape: GROUP BY one range field with two range clauses on distinct fields. DriveDocumentCountQuery::detect_mode_versioned routes that query to the carrier mode, while the server converts an omitted limit to Some(MAX_CARRIER_AGGREGATE_OUTER_RANGE_LIMIT) (currently 10). The verifier passes None from count_proof_helpers.rs:280, so verify_carrier_aggregate_count_proof reconstructs a different proof-sensitive SizedQuery::limit and rejects an honest server proof. The shared 100-item cap also permits explicit G8 limits from 11 through 100 even though the server rejects them above the carrier-specific cap of 10. Make COUNT carrier translation shape-aware: range-outer queries must use the compile-time range-outer default for limit 0 and enforce that carrier-specific cap, while In-outer queries retain None for an omitted limit.

_{source: ['codex']}

[thepastaclaw]

Resolved in this update — Preserve the range-outer carrier limit during COUNT 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]

🔴 Blocking: Apply the server default when lowering an omitted document limit

The 0 sentinel represents an omitted wire limit, but this branch constructs a raw DriveDocumentQuery with limit: None. Nothing subsequently applies the server default: DriveDocumentQuery::construct_path_query forwards this field directly into SizedQuery, where None is unbounded. The server instead maps V0 limit = 0 and V1 limit = None to Some(self.config.drive.default_query_limit) before calling from_typed_clauses, which stores the concrete limit. Under the canonical configuration, an honest request therefore proves at most 100 documents while this verifier accepts an authenticated proof for every matching document. A malicious full node can exploit that mismatch to return more documents than the request could legitimately produce and control excess proof processing. Lower the sentinel to the canonical compile-time default and update the test that currently asserts None.

Suggested change

	0 => None,
	0 => Some(DEFAULT_QUERY_LIMIT),

_{source: ['codex']}

[thepastaclaw]

🟡 Suggestion: Encode aggregate-limit validation in the returned type

check_within_server_cap returns (), while distinct_walk_limit and carrier_walk_limit independently accept the original u32 and narrow it with as u16. Every current COUNT, SUM, and AVG caller runs the check first, but Rust does not encode that ordering invariant, and the debug_assert! disappears from release builds. A future internal caller that skips the separate gate can therefore truncate an over-wide value and reconstruct the wrong proof query. Return a private validated-limit newtype whose methods perform the walk-specific conversions, or make each conversion validate and return Result; callers can still create the validated value at function entry to preserve pre-provider rejection ordering.

_{source: ['codex']}

[thepastaclaw]

🟡 Suggestion: Pin the InvalidLimit error variant in the regression test

This PR intentionally changes over-cap lowering from Error::Config to Error::Drive(QuerySyntaxError::InvalidLimit), but this regression test checks only a fragment of the rendered message. Returning the old error category with the same text would still pass. Match the typed variant and its payload so the documented error-surface change remains covered.

Suggested change

	for limit in [101u32, 65_535, 65_537, u32::MAX] {
	let error = DriveDocumentQuery::try_from(&query(limit))
	.expect_err("a limit the server refuses must not reach a DriveDocumentQuery");
	assert!(
	error.to_string().contains("greater than max limit 100"),
	"unexpected error for limit {limit}: {error}"
	);
	assert!(
	matches!(
	&error,
	Error::Drive(drive::error::Error::Query(
	drive::error::query::QuerySyntaxError::InvalidLimit(message)
	)) if message.contains("greater than max limit 100")
	),
	"unexpected error for limit {limit}: {error}"
	);

_{source: ['codex']}

[thepastaclaw]

Preliminary review — Codex only

The shared wire decoding and extracted builders largely preserve the intended trust boundaries, but two proof-sensitive limit translations still diverge from server behavior: omitted plain-document limits become unbounded, and COUNT range-outer carrier queries use the In-outer limit rules. Two additional suggestions strengthen the aggregate-limit invariant and pin the intentional typed error contract; tests could not be rerun because cargo is unavailable in the environment.
Source: reviewer backends gpt-5.6-sol (general), gpt-5.6-sol (security-auditor), and gpt-5.6-sol (rust-quality); final verifier backend 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), gpt-5.6-sol — security-auditor (completed), gpt-5.6-sol — rust-quality (completed)
• Verifier: gpt-5.6-sol — verifier
• Sonnet: not run (deferred by blocker gate)

🔴 2 blocking

4 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/aggregate_limit.rs`:
- [BLOCKING] packages/dash-platform-queries/src/documents/aggregate_limit.rs:76-87: Preserve the range-outer carrier limit during COUNT verification
  (existing thread: https://github.com/dashpay/platform/pull/4389#discussion_r3801736937)
  `carrier_walk_limit` always applies the In-outer carrier semantics, but COUNT uses `RangeAggregateCarrierProof` for two different shapes. `DriveDocumentCountQuery::detect_mode_versioned` also selects this mode for the G8 range-outer shape—`GROUP BY` on one range field with two range clauses on distinct fields. The server detects that shape by counting two range clauses, maps an omitted limit to `Some(MAX_CARRIER_AGGREGATE_OUTER_RANGE_LIMIT)` (currently 10), and rejects explicit limits above 10. The verifier instead passes `None` for an omitted limit and allows explicit values through the shared 100-item cap. Because the limit is part of the proof-sensitive `SizedQuery`, an honest omitted-limit proof is reconstructed differently, while an untrusted full node can produce a proof for a broader query than the server permits. Make COUNT carrier translation shape-aware: preserve `None` only for In-outer carriers, and for range-outer carriers apply the compile-time limit of 10 when omitted and reject explicit values above that limit.
- [SUGGESTION] packages/dash-platform-queries/src/documents/aggregate_limit.rs:45-87: Encode aggregate-limit validation in the returned type
  (existing thread: https://github.com/dashpay/platform/pull/4389#discussion_r3801736948)
  `check_within_server_cap` returns `()`, while `distinct_walk_limit` and `carrier_walk_limit` independently receive the original `u32` and narrow it with `as u16`. Every current COUNT, SUM, and AVG caller invokes the gate first, so there is no present truncation through those entry points, but that required ordering is represented only by documentation and a release-disabled `debug_assert!`. A future crate-internal caller can skip the gate and silently turn an over-wide request into a different proof query. Return a private validated-limit newtype from the cap check and expose conversions through it, or make each conversion validate and return `Result`, while retaining validation before provider or proof access.

In `packages/dash-platform-queries/src/documents/document_query.rs`:
- [BLOCKING] packages/dash-platform-queries/src/documents/document_query.rs:1301: Apply the server default when lowering an omitted document limit
  (existing thread: https://github.com/dashpay/platform/pull/4389#discussion_r3801736939)
  The `0` sentinel represents an omitted V0 limit or V1 `None`, but this branch constructs a raw `DriveDocumentQuery` with `limit: None`. `DriveDocumentQuery::construct_path_query` forwards that value directly into `SizedQuery`, where it is unbounded. By contrast, `query_documents_typed` maps both omitted forms to `Some(self.config.drive.default_query_limit)` before calling `from_typed_clauses`, so a canonical server proves at most `DEFAULT_QUERY_LIMIT` documents. The verifier therefore reconstructs a broader query than the server executed and can accept an authenticated proof containing every matching document from an untrusted full node. Lower the sentinel to the canonical compile-time default so proof verification uses the same bounded path query as the server.

In `packages/dash-platform-queries/tests/document_query_wire_roundtrip.rs`:
- [SUGGESTION] packages/dash-platform-queries/tests/document_query_wire_roundtrip.rs:340-346: Pin the InvalidLimit error variant in the regression test
  (existing thread: https://github.com/dashpay/platform/pull/4389#discussion_r3801736954)
  This PR intentionally changes over-cap lowering from `Error::Config` to `Error::Drive(QuerySyntaxError::InvalidLimit)`, but the regression test checks only the rendered message. Restoring the old error category while preserving the same text would still pass, leaving the documented error-surface contract untested. Match the typed variant and its payload.