refactor(sdk): share document transition preparation with embedders - #4433
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
|
✅ Final review complete — no blockers (commit 3f00ac2) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
At exact head ca2afbb, the helpers are a behavior-preserving relocation: the SDK retains property sanitization, entropy/document-ID validation, signing and broadcast ordering, and its existing Generic error classification through the query-error conversion. The moved tests cover matching and mismatched entropy and verify sanitization without mutating the caller's document; no in-scope defects were confirmed.
Source: reviewer backend model gpt-5.6-sol (general, security-auditor, rust-quality, and ffi-engineer lanes); final verifier backend model gpt-5.6-sol. openclaw-agent/cliproxy/gpt-5.6-sol was orchestration-only and is 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),gpt-5.6-sol— ffi-engineer (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)
ca2afbb to
acb258b
Compare
acb258b to
3f00ac2
Compare
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The helper extraction preserves the SDK's existing property sanitization, entropy/document-ID validation, call ordering, and Generic error classification while exposing the logic through the transport-free query crate. The relocated tests cover matching and mismatched entropy as well as non-mutating property normalization; no in-scope defects were confirmed.
Source: reviewer backend model gpt-5.6-sol (general and rust-quality lanes); final verifier backend model gpt-5.6-sol. openclaw-agent/cliproxy/gpt-5.6-sol was orchestration-only and is 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— 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)
3f00ac2 to
1aa8081
Compare
1aa8081 to
970dd23
Compare
…rked flows register_dpns_name and create_contact_request were interleaving document assembly (id derivation, salted-domain-hash commitment, property maps, size validation) with fetching, ECDH, and broadcasting. The assembly halves become pure functions - build_dpns_preorder_and_domain_documents and build_contact_request_document - that take caller-supplied entropy/salt/ciphertexts and touch no network or randomness. The networked flows now call them; ids, properties, size-validation bounds, and error messages are unchanged.
Copies the wire-proto -> drive-type decoders for the v1 getDocuments surface from rs-drive-abci's query/document_query/v1/conversions.rs into dash-platform-queries::documents::proto_conversions, verbatim except for a neutral DecodeError replacing the server's QueryError with the exact same message strings. The server is untouched. This is a client-side mirror kept in lockstep by doc contract, the same convention the v0 path uses where CBOR clause decoding mirrors query_documents_v0. Hosting a single shared decode crate that both sides consume is proposed separately; this PR deliberately avoids adding any drive-abci dependency. Upcoming client-side wire decoding (DocumentQuery::try_from_request) consumes these functions; until that commit lands the module carries a temporary allow(dead_code).
…policy is_consensus_valid_label matches exactly the DPNS contract's label schema pattern (consecutive hyphens allowed); is_valid_username is recomposed as that pattern plus the stricter client-side consecutive-hyphen rejection. Its acceptance set is unchanged - the pre-existing test vectors pass as-is - but the consensus check is now available on its own so document builders cannot reject labels the contract accepts.
…re assembly The pure DPNS builder now rejects labels that fail the DPNS contract's consensus pattern (is_consensus_valid_label) before assembling. The previous flow did no label validation locally and let the network reject invalid labels; failing locally with a clear message is strictly earlier, and using the consensus pattern (not the stricter client policy) means the builder cannot reject labels the contract accepts.
…-platform-queries File move of the pure builders introduced in the previous commit, unchanged except for the error type: they now return dash_platform_queries::Error::InvalidInput, which dash-sdk maps back to Error::Generic with identical messages, so the SDK surface is byte-for-byte the same. rs-sdk re-exports the builders at their previous paths. This makes the document-assembly half of DPNS registration and DashPay contact requests reachable without the SDK's transport stack; crypto material and randomness stay with the caller.
…d client code DocumentQuery::try_from_request decodes a wire GetDocumentsRequest back into a rich DocumentQuery - the inverse of request encoding. V1 typed clauses go through the same proto_conversions functions the server's v1 handler runs; V0 CBOR where/order_by fields are decoded exactly as the server's query_documents_v0 does. Multi-projection selects and limit Some(0) are rejected, mirroring the server's contracts.
…d embedders verify_documents_response verifies a proved GetDocumentsResponse directly against the wire request that produced it: the wire version (V0/V1 oneof arm) is checked against the platform version's document_query feature bounds (the server's own dispatch gate), prove=false requests are rejected (an honest server answers them unproved), and the request decodes through the shared try_from_request before delegating to FromProof. The query-shape gates (HAVING, GROUP BY, OFFSET, non-documents SELECT - every field the DocumentQuery -> DriveDocumentQuery lowering drops) run inside the shared FromProof<DocumentQuery> impl itself rather than only at the wire entry point. dash-sdk's document fetches verify through that impl, and the SDK talks to the same untrusted evonodes an embedder's transport does, so both paths now reject request shapes no honest server would have proved before any proof machinery runs.
Extracts the document create/replace preparation out of dash-sdk's PutDocument broadcast path into dash-platform-queries: property sanitization for the transition (prepare_document_for_transition) and the entropy/document-id consistency check (ensure_entropy_matches_document_id) that surfaces an id/entropy drift locally instead of after the broadcast has paid a bumped identity-contract nonce. dash-sdk delegates to the shared helpers with unchanged behavior; transport-free embedders (packages/rs-platform-cxx) assemble their own transitions through the same code instead of reimplementing it in C++. Split out of #4389 to keep that PR to its declared decode/builders/verification scope.
970dd23 to
f036c83
Compare
2 participants
Issue being fixed or feature implemented
Transport-free embedders (the Dash Core Platform GUI via
packages/rs-platform-cxx, #4416) need the same document create/replace preparationdash-sdk'sPutDocumentbroadcast path performs — property sanitization and the entropy/document-id consistency check — or they reimplement it in C++ and drift.Split out of #4389 on scope grounds: it is neither the wire-request decode nor the DPNS/DashPay builders that PR declares. Stacked on #4389 (base
refactor/document-query-decode-builders); will be retargeted tov4.2-devwhen it merges.What was done?
dash-platform-queries::transition::put_documentgainsprepare_document_for_transition(property sanitization for the transition, e.g. integer arrays coerced back into byte arrays after a WASM boundary crossing) andensure_entropy_matches_document_id(Drive recomputes the id from the entropy duringadvanced_structurevalidation and rejects a mismatch — this surfaces the drift locally, before the broadcast has paid a bumped identity-contract nonce).dash-sdk'sPutDocumentpath delegates to the shared helpers; behavior unchanged.How Has This Been Tested?
cargo test -p dash-platform-queriesand a fulldash-sdkbuild on the stacked branch. The helpers are exercised end to end byrs-platform-cxx's transition tests in #4416.Breaking Changes
None.
Checklist: