-
Notifications
You must be signed in to change notification settings - Fork 56
refactor(sdk)!: extract transport-free query core into dash-platform-queries #4388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v4.2-dev
Are you sure you want to change the base?
Changes from all commits
7277935
bdaa619
63e2e8a
7087bd2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| [package] | ||
| name = "dash-platform-queries" | ||
| description = "Transport-free query building and proof decoding core shared by Dash Platform SDK embedders" | ||
| version.workspace = true | ||
| edition = "2021" | ||
| rust-version.workspace = true | ||
| license = "MIT" | ||
|
|
||
| [features] | ||
| default = [] | ||
| mocks = [ | ||
| "dep:serde", | ||
| "dep:serde_json", | ||
| "dapi-grpc/mocks", | ||
| "drive/serde", | ||
| "dpp/serde-conversion", | ||
| ] | ||
|
|
||
| [dependencies] | ||
| dapi-grpc = { path = "../dapi-grpc", default-features = false, features = [ | ||
| "platform", | ||
| "client", | ||
| ] } | ||
| dash-context-provider = { path = "../rs-context-provider", default-features = false } | ||
| dash-platform-macros = { path = "../rs-dash-platform-macros" } | ||
| dpp = { path = "../rs-dpp", default-features = false, features = [ | ||
| "platform-value-cbor", | ||
| "state-transitions", | ||
| "state-transition-validation", | ||
| ] } | ||
| drive = { path = "../rs-drive", default-features = false, features = [ | ||
| "verify", | ||
| ] } | ||
| drive-proof-verifier = { path = "../rs-drive-proof-verifier", default-features = false } | ||
| hex = { version = "0.4.3" } | ||
| serde = { version = "1.0.219", default-features = false, features = [ | ||
| "rc", | ||
| ], optional = true } | ||
| serde_json = { version = "1.0", optional = true } | ||
| thiserror = "2.0.17" | ||
| tracing = { version = "0.1.41" } | ||
|
|
||
| [dev-dependencies] | ||
| dpp = { path = "../rs-dpp", default-features = false, features = [ | ||
| "fixtures-and-mocks", | ||
| ] } | ||
|
|
||
| [package.metadata.cargo-machete] | ||
| ignored = [ | ||
| # Used inside the `dash_platform_macros::Mockable` derive expansion under | ||
| # the `mocks` feature; machete cannot see through proc-macro output. | ||
| "serde_json", | ||
| ] | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,57 @@ | ||||||||||||||||
| # dash-platform-queries | ||||||||||||||||
|
|
||||||||||||||||
| Transport-free query core of the Dash Platform SDK. | ||||||||||||||||
|
|
||||||||||||||||
| This crate carries the pieces of `dash-sdk` that build queries, encode them | ||||||||||||||||
| onto the wire format, and decode/verify proved responses — with **no | ||||||||||||||||
| transport implementation**: no `rs-dapi-client` and no tonic native | ||||||||||||||||
| channel/TLS stack. Shared generated types and context-provider utilities | ||||||||||||||||
| remain dependencies. `dash-sdk` depends on it and re-exports everything at | ||||||||||||||||
| the historical paths, so SDK users need no changes. | ||||||||||||||||
|
Comment on lines
+8
to
+10
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: README incorrectly promises that SDK users need no changes The README says the historical re-exports mean SDK users need no changes, but this PR deliberately declares three source-incompatible SDK changes:
Suggested change
source: ['codex'] |
||||||||||||||||
|
|
||||||||||||||||
| ## Who this is for | ||||||||||||||||
|
|
||||||||||||||||
| Embedders that bring their own transport and trust context and only need the | ||||||||||||||||
| verification/query layer: | ||||||||||||||||
|
|
||||||||||||||||
| - **Dash Core's platform GUI** — fetches over its own gRPC-Web transport, | ||||||||||||||||
| serves quorum keys from its locally synced LLMQ state via a | ||||||||||||||||
| [`ContextProvider`](../rs-context-provider), and verifies every response | ||||||||||||||||
| proof with [`drive-proof-verifier`](../rs-drive-proof-verifier). | ||||||||||||||||
| - Block explorers, Electrum-style servers, hardware-wallet tooling — anything | ||||||||||||||||
| that talks to DAPI its own way but must not trust responses. | ||||||||||||||||
|
|
||||||||||||||||
| If you want networking, retries, and a managed connection pool, use | ||||||||||||||||
| `dash-sdk` — it consumes this crate internally. | ||||||||||||||||
|
|
||||||||||||||||
| ## What's here | ||||||||||||||||
|
|
||||||||||||||||
| - [`documents::DocumentQuery`] — rich document query builder with wire | ||||||||||||||||
| encoding for both request versions. | ||||||||||||||||
| - Aggregate proof helpers (count/sum/average/ranked) shared with `dash-sdk`. | ||||||||||||||||
| - DPNS username helpers — label normalization/validation and the | ||||||||||||||||
| convertibility/contested checks shared with `dash-sdk`. | ||||||||||||||||
| - `transition::validation` — structural validation for state transitions | ||||||||||||||||
| ahead of signing. | ||||||||||||||||
|
|
||||||||||||||||
| Wire-request decoding (`DocumentQuery::try_from_request`), request-driven | ||||||||||||||||
| proof verification, and pure DPNS/DashPay document builders arrive in the | ||||||||||||||||
| next slice of this series. | ||||||||||||||||
|
|
||||||||||||||||
| ## Feature flags | ||||||||||||||||
|
|
||||||||||||||||
| - `mocks` — serde support for the types used in dump/replay test vectors | ||||||||||||||||
| (forwarded by `dash-sdk`'s `mocks`). | ||||||||||||||||
|
|
||||||||||||||||
| The dependency tree is checked in CI to stay free of the transport stack | ||||||||||||||||
| (`hyper`, `rustls`, `tower`); see the "Check transport-free feature cuts" | ||||||||||||||||
| step in `.github/workflows/tests-rs-workspace.yml`. | ||||||||||||||||
|
|
||||||||||||||||
| "Transport-free" means no networking stack, not an async-runtime-free graph: | ||||||||||||||||
| `tokio` is still reachable on native targets through | ||||||||||||||||
| `dash-context-provider` → `dash-async`, exactly as it already was for | ||||||||||||||||
| `drive-proof-verifier` before this crate existed. `tonic` is present too, but | ||||||||||||||||
| only for `dapi-grpc`'s generated message/client types — its transport feature | ||||||||||||||||
| stays off, which is what the `hyper`/`rustls`/`tower` assertions prove. On | ||||||||||||||||
| `wasm32-unknown-unknown` none of that is pulled in; the wasm assertions in the | ||||||||||||||||
| same CI step also ban `mio`. | ||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| use crate::Error; | ||
| use dapi_grpc::platform::v0::ResponseMetadata; | ||
| use dpp::block::block_info::BlockInfo; | ||
| use dpp::block::epoch::MAX_EPOCH; | ||
| use drive::error::proof::ProofError; | ||
|
|
||
| /// Constructs a `BlockInfo` structure from the provided response metadata. This function | ||
| /// translates metadata received from a platform response into a format that is specific to the | ||
| /// application's needs, particularly focusing on block-related information. It ensures that | ||
| /// the epoch value from the metadata does not exceed `MAX_EPOCH`, | ||
| /// as this is a constraint for the `Epoch` type used in the `BlockInfo` structure. | ||
| /// | ||
| /// # Parameters | ||
| /// - `response_metadata`: A reference to `ResponseMetadata` obtained from a platform response. | ||
| /// This metadata includes various block-related information such as time in milliseconds, | ||
| /// height, core chain locked height, and epoch. | ||
| /// | ||
| /// # Returns | ||
| /// If successful, returns `Ok(BlockInfo)` where `BlockInfo` contains: | ||
| /// - `time_ms`: The timestamp of the block in milliseconds. | ||
| /// - `height`: The height of the block. | ||
| /// - `core_height`: The core chain locked height, indicating the height of the block in the core blockchain that is considered final and securely linked to this block. | ||
| /// - `epoch`: The epoch number, converted to an `Epoch` struct via a 16-bit number. | ||
| /// | ||
| /// # Errors | ||
| /// Returns an error if: | ||
| /// - The `epoch` value in the response metadata exceeds `MAX_EPOCH`. This is considered a data validity error as it indicates Platform returned an unexpectedly high epoch number. | ||
| /// | ||
| /// The function encapsulates errors into the application's own `Error` type, providing a unified interface for error handling across the application. | ||
| pub fn block_info_from_metadata(response_metadata: &ResponseMetadata) -> Result<BlockInfo, Error> { | ||
| if response_metadata.epoch > MAX_EPOCH as u32 { | ||
| return Err( | ||
| drive::error::Error::Proof(ProofError::InvalidMetadata(format!( | ||
| "platform returned an epoch {} that was higher than the maximum allowed epoch", | ||
| response_metadata.epoch | ||
| ))) | ||
| .into(), | ||
| ); | ||
| } | ||
|
|
||
| Ok(BlockInfo { | ||
| time_ms: response_metadata.time_ms, | ||
| height: response_metadata.height, | ||
| core_height: response_metadata.core_chain_locked_height, | ||
| epoch: (response_metadata.epoch as u16).try_into()?, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Blocking: The transport-free crate still pulls Tokio and native networking features
The PR description explicitly promises no Tokio anywhere in this crate's dependency graph, but
dash-context-provideris an unconditional normal dependency and itself unconditionally depends ondash-async. On native targets,dash-asyncenables Tokio'srt,rt-multi-thread,time, andnetfeatures, so the resulting normal graph includes Tokio plus its native networking support. Thedapi-grpccode-generation path also retains Tokio-related support dependencies. The dependency cut therefore does not meet the stated no-Tokio embedder requirement; the context-provider/async boundary must be split or feature-gated so this crate's normal graph excludes Tokio.source: ['codex']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency-graph facts here are correct and the PR description was overclaiming. Corrected.
Measured on the branch:
So
tokio,tokio-util,mioandtonicare all present.hyper,rustlsandtowerare absent.The part the finding does not account for:
drive-proof-verifierhas a byte-identical profile on the merge base — samedash-context-provider→dash-asyncedge, sametonicedge viadapi-grpc. This split introduces no new dependency; it inherits one that already existed. I checked both trees side by side.What was wrong was the claim, not the graph, so that is what I changed:
rs-dapi-client,hyper,rustls,tower— hence no tonic transport feature) and what is present transitively (tokioviadash-context-provider→dash-async,tonicfordapi-grpc's generated types), with the note that this matchesdrive-proof-verifierpre-split.hyper/rustls/towerassertions overdash-platform-queriesas well asdrive-proof-verifier(see the separate thread ontests-rs-workspace.yml).I did not assert
tokio/mioabsent on the native target, because they are not absent and a green CI check that lies is worse than no check.Declining the suggested remedy (splitting or feature-gating the
dash-context-provider→dash-asyncboundary) for this PR: it is a change to a crate this PR does not touch, it would alterdrive-proof-verifier's graph too, and it is not required by the extraction. It is a legitimate follow-up and the PR description now says so.🤖 Posted autonomously by Claude on behalf of pasta.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved in this update — The transport-free crate still pulls Tokio and native networking features 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.