refactor(drive-abci): share the v1 document-query wire decoders and bound dpp document field reads - #4618
Open
PastaPastaPasta wants to merge 2 commits into
Open
Conversation
…atform-query-wire
Pure move: the wire-proto -> drive-type decoders for the v1 getDocuments surface now live in a new packages/rs-platform-query-wire micro-crate (crate name platform-query-wire) behind a neutral DecodeError { InvalidArgument, Unsupported }, and drive-abci's v1/conversions.rs becomes a thin adapter mapping DecodeError onto its QueryError surface with the exact same message strings (including the aggregate ORDER BY arm, which maps Unsupported onto QuerySyntaxError::Unsupported). No behavior change to server request decoding.
The decode of a wire request into a rich query is an equivalence contract at a trust boundary: a client-side proof verifier must interpret a request exactly as the server does, or a proof could verify against a different query than the server answered. Hosting the shared decoders in a neutral crate lets both rs-drive-abci and (in a follow-up) the client-side SDK decoders run the same functions, without making the consensus server depend on SDK-branded code: the new crate's dependencies (dapi-grpc platform+client without transport, dpp, drive/verify, thiserror) are a strict subset of what drive-abci already carries.
Document deserialization read a varint length from the (untrusted) serialized bytes and immediately allocated a Vec of that size, for both variable-size string/byte-array fields and nested object payloads. A proved getDocuments response is decoded before its quorum signature is checked, so a self-consistent GroveDB proof carrying a document whose length prefix claims gigabytes made every client that verifies proofs (rs-sdk, wasm-sdk, and the Dash Core embedder) abort on allocation failure from ~70 bytes of input. Both sites now check the declared length against the bytes remaining in the reader and reject with CorruptedSerialization before allocating. A regression test feeds a 2^62 length prefix to both the string and object arms.
Contributor
|
Warning Review limit reachedNext included review available in 29 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
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 |
PastaPastaPasta
changed the title
This was referenced Sep 8, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4618 +/- ##
============================================
- Coverage 87.72% 85.35% -2.37%
============================================
Files 2762 2793 +31
Lines 360146 371086 +10940
============================================
+ Hits 315922 316737 +815
- Misses 44224 54349 +10125
🚀 New features to boost your workflow:
|
PastaPastaPasta
added
the
ready for final review
Collaborator
|
🕓 Queued for automated review — 50th in line, estimated start in ~69 h (commit ffb791d)
|
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
First of three PRs replacing the transport-free-embedder series (#4464, #4478, #4389, #4433, #4416), rebuilt after a review of that stack found it duplicating code the workspace already has and carrying two remotely triggerable aborts in the C++ bridge. This PR is the server-side and dpp half; the SDK verification/builders and the C++ crate follow in stacked PRs.
Two independent changes that both sit below the client crates:
getDocumentswire decode becomes a shared crate. A client that verifies a proof against the request it actually sent must reconstruct the query exactly as the server did. Today that decode lives only insiders-drive-abci, so the earlier client-side PR carried a byte-for-byte copy with a "keep in lockstep" comment. Supersedes refactor(drive-abci): extract v1 document-query wire decoders into platform-query-wire #4464, re-derived from the current server file (which had grown the time-range decoders since refactor(drive-abci): extract v1 document-query wire decoders into platform-query-wire #4464 was cut).Vecof that size, for variable-size string/byte-array fields and for nested object payloads. A proved response's documents are decoded before its quorum signature is checked (the root hash only exists after replay), so ~70 bytes of self-consistent proof carrying a document whose length prefix claims 2^62 bytes aborted every proof-verifying client (rs-sdk, wasm-sdk, the Dash Core embedder) on allocation failure.What was done?
packages/rs-platform-query-wire(platform-query-wire): the wire-proto → drive-type decoders forgetDocumentsv1 (where / order-by / having / select / time-range clauses), moved verbatim fromrs-drive-abci/src/query/document_query/v1/conversions.rs, with a neutralDecodeError { InvalidArgument, Unsupported }. Its dependency graph (dapi-grpc,dpp,drive) is a strict subset of what drive-abci already carries, so the consensus server does not gain client-flavoured dependencies.rs-drive-abci'sv1/conversions.rsis now a thin adapter mappingDecodeErrorontoQueryError, preserving every existing message string.rs-dppDocumentPropertyType::read_varint_valueand theObjectarm check the declared length against the bytes remaining in the reader before allocating, and reject withCorruptedSerializationotherwise.COPY --parents, package filters, and the transport-free CI cut gain the new crate.How Has This Been Tested?
cargo test -p drive-abci --lib -- query::document_query::v1: 97 passed (the server's own decode tests now exercise the shared crate through the adapter).cargo test -p dpp --lib -- read_optionally_from: all pass, including the newtest_read_optionally_from_rejects_length_prefix_longer_than_input, which feeds a 2^62 varint to both the string and object arms and asserts the rejection message.cargo check -p platform-query-wire --lockedandcargo clippy --all-targets --all-features -- -D warningson the touched crates: clean.cargo machete: clean.Breaking Changes
None. Server error strings are unchanged; the dpp change only turns an abort into an error.
Checklist:
For repository code-owners and collaborators only