docs(rs-sdk): fix rustdoc inaccuracies and resolve all cargo doc warnings#3161
Open
docs(rs-sdk): fix rustdoc inaccuracies and resolve all cargo doc warnings#3161
Conversation
…ings
- Fix crate-level docs: correct type name references (DriveQuery -> DriveDocumentQuery),
fix grammar ("can be executing" -> "can be executed"), update test file paths,
remove outdated reference to C bindings, fix crate name (dash-platform-sdk -> SDK)
- Fix 32 cargo doc warnings:
- Resolve broken intra-doc links for ProTxHash, VotePollsByDocumentTypeQuery,
ContestedDocumentVotePollDriveQuery, ContestedDocumentVotePollVotesDriveQuery,
ContestedResourceVotesGivenByIdentityQuery, VotePollsByEndDateDriveQuery,
KeysInPath, ProposerBlockCounts, TransportRequest, sync_address_balances,
AddressProvider::last_sync_height
- Remove references to non-existent MockRequest trait and SdkBuilder::with_wallet()
- Replace BroadcastRequestForNewIdentity references with BroadcastRequestForStateTransition
- Fix unclosed HTML tags in Vec<Identifier> doc comments
- Fix redundant explicit link targets for Identifier in query.rs
- Escape regex pattern in dpns_usernames to prevent markdown parsing
- Update mock.rs test file path references
https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
✅ gRPC Query Coverage Report |
Contributor
📝 WalkthroughWalkthroughThis PR mainly updates Rust SDK docs and examples, but also includes targeted API changes: renamed broadcast trait and its methods for state transitions, changed proposed blocks query signature, switched BlockInfo.epoch to an Epoch type, added builder methods for token config updates, and adjusted an internal sync worker response channel type. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rs-sdk/src/platform/transition/broadcast_request.rs (1)
23-36:⚠️ Potential issue | 🟡 MinorDocs still describe removed identity-specific API behavior.
The trait is now state-transition generic, but this block still describes
broadcast_new_identity(...)-style semantics and parameters that the current method no longer takes.✏️ Suggested doc correction
-/// Trait implemented by objects that can be used to broadcast new identity state transitions. +/// Trait implemented by objects that can be used to broadcast state transitions. @@ -/// To broadcast a new [Identity](dpp::prelude::Identity) state transition, you would typically -/// create an [IdentityCreateTransition](dpp::state_transition::identity_create_transition::IdentityCreateTransition), -/// sign it, and use the `broadcast_new_identity` method provided by this trait: +/// To broadcast a state transition, construct it and call +/// [`BroadcastRequestForStateTransition::broadcast_request_for_state_transition`]. @@ - /// * `asset_lock_proof` - The proof that locks the asset which is used to create the identity. - /// * `asset_lock_proof_private_key` - The private key associated with the asset lock proof. - /// * `signer` - The signer to be used for signing the state transition. - /// * `platform_version` - The version of Platform for which the state transition is intended. + /// This method does not take additional arguments; it converts `self` + /// into a broadcast request. @@ - /// On success, this method yields an instance of the `TransportRequest` type (`T`), which can be used to broadcast the new identity state transition to Platform. + /// On success, this method yields a request that can be sent to Platform.Also applies to: 73-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/transition/broadcast_request.rs` around lines 23 - 36, The docs for the BroadcastRequestForStateTransition trait still describe identity-specific API (e.g., broadcast_new_identity, IdentityCreateTransition); update the docblock to describe the trait as a generic state-transition broadcaster: remove identity-specific examples and parameters, explain that implementors should prepare, sign and broadcast an arbitrary StateTransition (or the trait's generic parameter), and replace the example with a neutral snippet referencing a generic StateTransition and the trait's actual method(s) used for signing/broadcasting; ensure all identity-specific wording is removed (also update the repeated section around the later block).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rs-sdk/src/platform/fetch_many.rs`:
- Line 436: Update the doc comments that currently say "can be and [Option]" to
"can be an [Option]" for the ProTxHash parameter description; locate the
occurrences in packages/rs-sdk/src/platform/fetch_many.rs where ProTxHash is
documented (both occurrences around the ProTxHash/Option lines) and correct the
wording to "can be an [Option]".
---
Outside diff comments:
In `@packages/rs-sdk/src/platform/transition/broadcast_request.rs`:
- Around line 23-36: The docs for the BroadcastRequestForStateTransition trait
still describe identity-specific API (e.g., broadcast_new_identity,
IdentityCreateTransition); update the docblock to describe the trait as a
generic state-transition broadcaster: remove identity-specific examples and
parameters, explain that implementors should prepare, sign and broadcast an
arbitrary StateTransition (or the trait's generic parameter), and replace the
example with a neutral snippet referencing a generic StateTransition and the
trait's actual method(s) used for signing/broadcasting; ensure all
identity-specific wording is removed (also update the repeated section around
the later block).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/rs-sdk/src/lib.rspackages/rs-sdk/src/mock.rspackages/rs-sdk/src/platform/address_sync/types.rspackages/rs-sdk/src/platform/delegate.rspackages/rs-sdk/src/platform/dpns_usernames/mod.rspackages/rs-sdk/src/platform/fetch.rspackages/rs-sdk/src/platform/fetch_many.rspackages/rs-sdk/src/platform/query.rspackages/rs-sdk/src/platform/transition/broadcast_request.rspackages/rs-sdk/src/platform/types/proposed_blocks.rspackages/rs-sdk/src/sdk.rs
Switch from pinned Rust 1.92 to stable (currently 1.93.1) to work around an index error. https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
Revert toolchain change. Will use `cargo +stable doc` instead to workaround the index error only for doc generation. https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
- lib.rs: Replace misleading "CRUD interface" with accurate description of query traits (Fetch, FetchMany, FetchUnproved, FetchCurrent) and state transition traits (PutIdentity, PutContract, PutDocument, etc.) - put_contract.rs: Fix doc saying "document" instead of "contract" - put_document.rs: Fix doc saying "identity" instead of "document" - vote.rs: Fix docs saying "identity" instead of "vote" (two places) - query.rs: Fix duplicate "epoch records" doc on DEFAULT_NODES_VOTING_LIMIT - waitable.rs: Fix typos "a wait" -> "a way", "conveniance" -> "convenience" - tokens/mod.rs: Fix wrong doc on token_info module (was "Identity token balances queries", now "Token info queries") - total_credits_in_platform.rs: Fix module doc saying "Epoch-related" - fetch_current_no_parameters.rs: Broaden FetchCurrent doc from epoch-specific to generic (also used for TotalCreditsInPlatform) All changes verified with `RUSTDOCFLAGS="-D warnings" cargo doc` — zero warnings and no broken intra-doc links. https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
Comprehensive review of all rustdoc comments in rs-sdk found and fixed
issues across 32 files including:
- Copy-paste errors (wrong object types in trait docs, wrong parameter
names in argument lists)
- Factual inaccuracies (wrong method names, wrong return type docs,
incorrect trait bound claims, wrong default value references)
- Misleading descriptions (broadcast_request.rs claimed signing/identity
operations but trait only serializes state transitions)
- Terminology errors ("request" vs "transition" in token builders,
code identifiers like "config_update" in prose text)
- Typos and grammar (double spaces, missing prepositions, missing
plurals, misspellings, non-breaking space characters)
- Incorrect doc links (wrong type names, singular vs plural, redundant
explicit link targets)
- Wrong/copy-pasted supported query type lists in FetchMany impls
- Missing parameter documentation
Verified with RUSTDOCFLAGS="-D warnings" cargo doc -- zero warnings.
https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
The previous commit removed an inaccurate example that referenced BroadcastNewIdentity/broadcast_new_identity (a different trait). Restore a corrected example that accurately demonstrates the BroadcastRequestForStateTransition trait and its actual API. https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
Add a two-step example showing how to serialize a state transition into a request and then execute it via DAPI. Also point users to the higher-level BroadcastStateTransition trait for the common case with retry logic and proof verification. https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
…_wait Replace the low-level DAPI example with a concise broadcast_and_wait snippet and link to the BroadcastStateTransition trait docs. https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
Remove redundant explicit link target for StateTransition that caused a rustdoc warning with -D warnings. https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
lklimek
commented
Feb 26, 2026
lklimek
added
the
ready for final review
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/rs-sdk/src/platform/types/total_credits_in_platform.rs (1)
28-31: Consider renamingepochvariable for consistency with the doc fix.The variable
epochon line 28 appears to be a copy-paste remnant from epoch-related code. Since this PR is fixing documentation inaccuracies, consider renaming it tototal_creditsortotal_credits_on_platformfor consistency with the other methods in this file.🔧 Suggested fix
async fn fetch_current_with_metadata_and_proof( sdk: &Sdk, ) -> Result<(Self, ResponseMetadata, Proof), Error> { - let (epoch, metadata, proof) = + let (total_credits, metadata, proof) = Self::fetch_with_metadata_and_proof(sdk, NoParamQuery {}, None).await?; - Ok((epoch.ok_or(Error::TotalCreditsNotFound)?, metadata, proof)) + Ok((total_credits.ok_or(Error::TotalCreditsNotFound)?, metadata, proof)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/platform/types/total_credits_in_platform.rs` around lines 28 - 31, Rename the misleading local variable `epoch` to `total_credits` (or `total_credits_on_platform`) in the response unpacking line where you call Self::fetch_with_metadata_and_proof(sdk, NoParamQuery {}, None).await? so the returned tuple reads (total_credits, metadata, proof), and update the Ok return to use total_credits.ok_or(Error::TotalCreditsNotFound)? to match other methods and the doc fix; ensure references to `epoch` in this function are replaced accordingly.packages/rs-sdk/src/sync.rs (1)
93-98: Consider removing commented-out dead code.The doc update on line 93 correctly reflects that the worker now uses an mpsc channel. However, line 97 contains a commented-out
oneshot::Senderreference that appears to be leftover from the previous implementation. This dead code adds no value and slightly reduces readability.🧹 Suggested cleanup
/// Worker function that runs the provided future and sends the result back to the caller using mpsc channel. #[cfg(not(target_arch = "wasm32"))] async fn worker<F: Future>( fut: F, - // response: oneshot::Sender<F::Output>, response: std::sync::mpsc::Sender<F::Output>, ) -> Result<(), AsyncError> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rs-sdk/src/sync.rs` around lines 93 - 98, Remove the dead commented-out oneshot sender reference in the worker function: delete the commented line "// response: oneshot::Sender<F::Output>," so the signature only documents the actual parameter `response: std::sync::mpsc::Sender<F::Output>` in the `worker` async fn (the symbol to edit is `worker` and the commented `oneshot::Sender` reference).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rs-sdk/src/mock/provider.rs`:
- Around line 254-256: Update the comment for the struct field `inner:
std::sync::RwLock<lru::LruCache<K, Arc<V>>>` to clarify that `RwLock` is used
for interior mutability to allow `&self` methods (e.g., the `get()` method) to
access and mutate the LRU cache, and not to provide concurrent read performance
— note that `get()` takes a write lock to update recency so operations are
serialized; reference `inner`, `RwLock`, `lru::LruCache`, and `get()` in the
comment to make this explicit.
---
Nitpick comments:
In `@packages/rs-sdk/src/platform/types/total_credits_in_platform.rs`:
- Around line 28-31: Rename the misleading local variable `epoch` to
`total_credits` (or `total_credits_on_platform`) in the response unpacking line
where you call Self::fetch_with_metadata_and_proof(sdk, NoParamQuery {},
None).await? so the returned tuple reads (total_credits, metadata, proof), and
update the Ok return to use total_credits.ok_or(Error::TotalCreditsNotFound)? to
match other methods and the doc fix; ensure references to `epoch` in this
function are replaced accordingly.
In `@packages/rs-sdk/src/sync.rs`:
- Around line 93-98: Remove the dead commented-out oneshot sender reference in
the worker function: delete the commented line "// response:
oneshot::Sender<F::Output>," so the signature only documents the actual
parameter `response: std::sync::mpsc::Sender<F::Output>` in the `worker` async
fn (the symbol to edit is `worker` and the commented `oneshot::Sender`
reference).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
packages/rs-sdk/src/core/dash_core_client.rspackages/rs-sdk/src/error.rspackages/rs-sdk/src/lib.rspackages/rs-sdk/src/mock/provider.rspackages/rs-sdk/src/mock/sdk.rspackages/rs-sdk/src/platform/block_info_from_metadata.rspackages/rs-sdk/src/platform/delegate.rspackages/rs-sdk/src/platform/documents/document_query.rspackages/rs-sdk/src/platform/documents/transitions/create.rspackages/rs-sdk/src/platform/dpns_usernames/contested_queries.rspackages/rs-sdk/src/platform/fetch.rspackages/rs-sdk/src/platform/fetch_current_no_parameters.rspackages/rs-sdk/src/platform/fetch_many.rspackages/rs-sdk/src/platform/group_actions.rspackages/rs-sdk/src/platform/query.rspackages/rs-sdk/src/platform/tokens/builders/burn.rspackages/rs-sdk/src/platform/tokens/builders/config_update.rspackages/rs-sdk/src/platform/tokens/builders/destroy.rspackages/rs-sdk/src/platform/tokens/builders/emergency_action.rspackages/rs-sdk/src/platform/tokens/builders/freeze.rspackages/rs-sdk/src/platform/tokens/builders/mint.rspackages/rs-sdk/src/platform/tokens/builders/purchase.rspackages/rs-sdk/src/platform/tokens/builders/set_price.rspackages/rs-sdk/src/platform/tokens/builders/transfer.rspackages/rs-sdk/src/platform/tokens/builders/unfreeze.rspackages/rs-sdk/src/platform/tokens/mod.rspackages/rs-sdk/src/platform/tokens/transitions/emergency_action.rspackages/rs-sdk/src/platform/tokens/transitions/set_price_for_direct_purchase.rspackages/rs-sdk/src/platform/tokens/transitions/transfer.rspackages/rs-sdk/src/platform/tokens/transitions/unfreeze.rspackages/rs-sdk/src/platform/transition/broadcast_request.rspackages/rs-sdk/src/platform/transition/put_contract.rspackages/rs-sdk/src/platform/transition/put_document.rspackages/rs-sdk/src/platform/transition/transfer.rspackages/rs-sdk/src/platform/transition/vote.rspackages/rs-sdk/src/platform/transition/waitable.rspackages/rs-sdk/src/platform/types/evonode.rspackages/rs-sdk/src/platform/types/proposed_blocks.rspackages/rs-sdk/src/platform/types/total_credits_in_platform.rspackages/rs-sdk/src/sdk.rspackages/rs-sdk/src/sync.rs
✅ Files skipped from review due to trivial changes (17)
- packages/rs-sdk/src/platform/transition/put_contract.rs
- packages/rs-sdk/src/platform/tokens/builders/transfer.rs
- packages/rs-sdk/src/platform/tokens/builders/unfreeze.rs
- packages/rs-sdk/src/core/dash_core_client.rs
- packages/rs-sdk/src/platform/types/evonode.rs
- packages/rs-sdk/src/platform/documents/transitions/create.rs
- packages/rs-sdk/src/platform/tokens/builders/freeze.rs
- packages/rs-sdk/src/platform/tokens/transitions/set_price_for_direct_purchase.rs
- packages/rs-sdk/src/platform/tokens/builders/destroy.rs
- packages/rs-sdk/src/platform/transition/waitable.rs
- packages/rs-sdk/src/platform/tokens/builders/mint.rs
- packages/rs-sdk/src/platform/tokens/builders/set_price.rs
- packages/rs-sdk/src/error.rs
- packages/rs-sdk/src/platform/transition/transfer.rs
- packages/rs-sdk/src/platform/dpns_usernames/contested_queries.rs
- packages/rs-sdk/src/mock/sdk.rs
- packages/rs-sdk/src/platform/documents/document_query.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/rs-sdk/src/platform/fetch.rs
- packages/rs-sdk/src/lib.rs
- packages/rs-sdk/src/sdk.rs
- packages/rs-sdk/src/platform/delegate.rs
Comment on lines
+254
to
256
| // We use a RwLock to allow access to the cache when we don't have mutable &self | ||
| // And we use Arc to allow multiple threads to access the cache without having to clone it | ||
| inner: std::sync::RwLock<lru::LruCache<K, Arc<V>>>, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/rs-sdk/src/mock/provider.rs"
rg -n -C2 'fn get\(|inner\.(read|write)\(' "$FILE"Repository: dashpay/platform
Length of output: 522
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/rs-sdk/src/mock/provider.rs"
sed -n '250,280p' "$FILE" | cat -nRepository: dashpay/platform
Length of output: 1411
Clarify that lock usage doesn't leverage RwLock concurrency.
The current comment implies RwLock enables concurrent reads, but get() uses a write lock (line 270) to update LRU recency, so all operations are serialized. The RwLock is chosen for interior mutability to support &self methods, not for read-concurrency. Suggest updating the comment:
Recommended comment update
- // We use a RwLock to allow access to the cache when we don't have mutable &self
- // And we use Arc to allow multiple threads to access the cache without having to clone it
+ // We use interior mutability so cache operations can be called via `&self`.
+ // Note: `get` updates LRU recency and currently takes a write lock.
+ // Arc allows sharing cached values across threads without cloning payloads.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // We use a RwLock to allow access to the cache when we don't have mutable &self | |
| // And we use Arc to allow multiple threads to access the cache without having to clone it | |
| inner: std::sync::RwLock<lru::LruCache<K, Arc<V>>>, | |
| // We use interior mutability so cache operations can be called via `&self`. | |
| // Note: `get` updates LRU recency and currently takes a write lock. | |
| // Arc allows sharing cached values across threads without cloning payloads. | |
| inner: std::sync::RwLock<lru::LruCache<K, Arc<V>>>, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rs-sdk/src/mock/provider.rs` around lines 254 - 256, Update the
comment for the struct field `inner: std::sync::RwLock<lru::LruCache<K,
Arc<V>>>` to clarify that `RwLock` is used for interior mutability to allow
`&self` methods (e.g., the `get()` method) to access and mutate the LRU cache,
and not to provide concurrent read performance — note that `get()` takes a write
lock to update recency so operations are serialized; reference `inner`,
`RwLock`, `lru::LruCache`, and `get()` in the comment to make this explicit.
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.
fix grammar ("can be executing" -> "can be executed"), update test file paths,
remove outdated reference to C bindings, fix crate name (dash-platform-sdk -> SDK)
ContestedDocumentVotePollDriveQuery, ContestedDocumentVotePollVotesDriveQuery,
ContestedResourceVotesGivenByIdentityQuery, VotePollsByEndDateDriveQuery,
KeysInPath, ProposerBlockCounts, TransportRequest, sync_address_balances,
AddressProvider::last_sync_height
https://claude.ai/code/session_01HdTH5NBEVTmxbW63RCkS9J
Summary by CodeRabbit
New Features
Documentation
Breaking Changes