Skip to content

Conversation

thephez
Copy link
Collaborator

@thephez thephez commented Aug 20, 2025

Issue being fixed or feature implemented

The wasm-sdk package was generating >60 compiler warnings during build, consisting of unused imports, unused variables,
unused constants, and an ambiguous glob re-export. These warnings cluttered build output and indicated potential dead code
in the codebase.

What was done?

Cleaned up the wasm-sdk codebase to eliminate all compiler warnings:

  • Removed 43+ unused imports across query and state transition modules using cargo fix
  • Fixed ambiguous glob re-export in lib.rs by explicitly naming conflicting exports
  • Prefixed unused variables with underscore to indicate intentionally unused parameters
  • Prefixed unused constants with underscore in DIP14 implementation for future reference

How Has This Been Tested?

  • Verified build completes with zero warnings from wasm-sdk package
  • Ran mjs test suite to confirm functionality is preserved
  • All existing functionality remains intact with clean compilation

Breaking Changes

None - this is purely a code cleanup that removes warnings without changing functionality.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • New Features

    • Proof-enabled voting queries now consistently return proof metadata by default in “with_proof_info” variants.
  • Refactor

    • Simplified public APIs: removed documentTypeName from identity key queries; removed resultType and allowIncludeLockedAndAbstainingVoteTally from contested resources queries.
    • Streamlined exports and aliases for query and state transition modules.
    • Broad cleanup of unused imports for smaller bundle/noise reduction.
  • Documentation

    • Updated guides, examples, and API reference to match simplified parameters.
  • Tests

    • Adjusted UI automation fixtures to reflect updated query parameters.

Remove 65 compiler warnings from wasm-sdk package by:
- Removing unused imports across query and state transition modules
- Prefixing unused variables with underscore
- Fixing ambiguous glob re-export in lib.rs
- Prefixing unused constants with underscore

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Contributor

coderabbitai bot commented Aug 20, 2025

Walkthrough

This PR primarily cleans up unused imports across the WASM SDK, refactors public re-exports to be explicit, adjusts several public function signatures (identity and voting queries), updates docs and API definitions to match, and refactors token last-claim retrieval to a direct gRPC flow with response parsing.

Changes

Cohort / File(s) Summary of changes
Explicit re-exports
packages/wasm-sdk/src/lib.rs
Replaced wildcard re-exports with explicit ones; aliased queries::identity as query_identity and state_transitions::identity as state_transition_identity.
Identity query signatures
packages/wasm-sdk/src/queries/identity.rs
Removed document_type_name parameter from get_identities_contract_keys and its proof variant; minor import cleanups and unused param renames.
Voting queries interface & proof behavior
packages/wasm-sdk/src/queries/voting.rs
Removed result_type and allowIncludeLockedAndAbstainingVoteTally from contested-resources methods; set proof-info variants to force prove: true; adjusted parameter placeholders.
Token last-claim retrieval
packages/wasm-sdk/src/queries/token.rs
Switched to direct gRPC request for last-claim; decode PaidAt variants (TimestampMs/BlockHeight/Epoch/RawBytes); import cleanups elsewhere.
Query import cleanups
packages/wasm-sdk/src/queries/data_contract.rs, .../document.rs, .../dpns.rs, .../epoch.rs, .../group.rs, .../protocol.rs, .../system.rs
Removed unused imports (e.g., ResponseMetadata, ProofInfo, miscellaneous local types); no logic changes.
SDK/state transition cleanups
packages/wasm-sdk/src/sdk.rs, .../state_transitions/identity/mod.rs, .../state_transitions/tokens/mod.rs, packages/wasm-sdk/src/dpns.rs
Trimmed unused imports; no behavioral changes.
Wallet tweaks
packages/wasm-sdk/src/wallet/dip14.rs, .../wallet/key_derivation.rs
Removed unused imports; prefixed unused constants with _; marked unused public params with _ in signatures.
Docs & API definitions
packages/wasm-sdk/AI_REFERENCE.md, .../api-definitions.json, .../docs.html, .../index.html, .../docs_manifest.json
Updated signatures/examples to remove dropped parameters; refreshed manifest timestamp.
Tests
packages/wasm-sdk/test/ui-automation/fixtures/test-data.js
Updated test data to match removed parameters (identity keys, contested resources).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant WASM_SDK as WASM SDK (token.rs)
  participant gRPC as Platform gRPC
  participant Parser as PaidAt Parser

  Caller->>WASM_SDK: get_token_perpetual_distribution_last_claim(token_id)
  WASM_SDK->>gRPC: GetTokenPerpetualDistributionLastClaimRequest { prove: false }
  gRPC-->>WASM_SDK: Response { last_claim?: PaidAt }
  alt last_claim present
    WASM_SDK->>Parser: Parse PaidAt (TimestampMs|BlockHeight|Epoch|RawBytes)
    Parser-->>WASM_SDK: { ts_ms?, block_height? }
    WASM_SDK-->>Caller: LastClaimResponse { last_claim_timestamp_ms?, last_claim_block_height? }
  else not found
    WASM_SDK-->>Caller: LastClaimResponse { nulls }
  end
  note over WASM_SDK,Parser: RawBytes path validates/decodes 8 bytes → seconds→ms + height
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–60 minutes

Possibly related PRs

Suggested reviewers

  • QuantumExplorer
  • pauldelucia

Poem

A nibble of imports, trimmed just right,
Hop! goes the rabbit through docs so bright.
Keys and votes now lighter to claim,
Last-claim hops straight via gRPC lane.
With whiskers twitching, APIs align—
Thump-thump! Clean code, delightful design. 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1676554 and 8e564ce.

📒 Files selected for processing (5)
  • packages/wasm-sdk/AI_REFERENCE.md (1 hunks)
  • packages/wasm-sdk/api-definitions.json (0 hunks)
  • packages/wasm-sdk/docs.html (2 hunks)
  • packages/wasm-sdk/index.html (0 hunks)
  • packages/wasm-sdk/test/ui-automation/fixtures/test-data.js (0 hunks)
💤 Files with no reviewable changes (3)
  • packages/wasm-sdk/index.html
  • packages/wasm-sdk/api-definitions.json
  • packages/wasm-sdk/test/ui-automation/fixtures/test-data.js
✅ Files skipped from review due to trivial changes (1)
  • packages/wasm-sdk/AI_REFERENCE.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/wasm-sdk/docs.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build JS packages / Build JS
  • GitHub Check: build-wasm-sdk
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch wasm-sdk-fix-build-warnings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@thephez
Copy link
Collaborator Author

thephez commented Aug 20, 2025

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Aug 20, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)
packages/wasm-sdk/src/state_transitions/tokens/mod.rs (1)

931-936: Prevent u64 overflow when computing total purchase price

price_per_token * token_amount can overflow u64 (TokenAmount is u64), silently wrapping in release builds and corrupting payment math. Use checked math and surface an error.

Apply this diff:

-                let price_per_token = price_str.parse::<u64>()
-                    .map_err(|e| JsValue::from_str(&format!("Invalid current price format: {}", e)))?;
-                
-                price_per_token * token_amount
+                let price_per_token = price_str.parse::<u64>()
+                    .map_err(|e| JsValue::from_str(&format!("Invalid current price format: {}", e)))?;
+
+                let total = price_per_token
+                    .checked_mul(token_amount)
+                    .ok_or_else(|| JsValue::from_str("Price overflow: price_per_token * amount exceeds u64"))?;
+                total
packages/wasm-sdk/src/wallet/dip14.rs (1)

136-146: Remove logs that exfiltrate private key material

Console-logging parent private key bytes, IL tweak, and child key bytes leaks secrets to JS consoles and logs. This is a serious security bug for wallet code.

Apply this diff to remove sensitive logs:

-        // Debug: log parent and IL values before addition
-        web_sys::console::log_1(&format!("Parent key: {}", hex::encode(&self.private_key.secret_bytes())).into());
-        web_sys::console::log_1(&format!("IL (tweak): {}", hex::encode(&il_scalar_bytes)).into());
@@
-        // Debug: log result after addition
-        web_sys::console::log_1(&format!("Child key after addition: {}", hex::encode(&child_key.secret_bytes())).into());

If you need diagnostics, gate safer, non-sensitive telemetry behind a debug-only feature without printing raw key material.

packages/wasm-sdk/src/queries/voting.rs (1)

151-161: Bug: startIdentifier is treated as ASCII, not as base58 Identifier bytes.

The code copies startIdentifier as raw string bytes. dpp/Dash APIs expect Identifier bytes (Base58 decoded), not ASCII of the string. This will produce incorrect pagination and potentially empty/incorrect results.

Apply the following fix in all affected places:

@@
-        if let (Some(start_id), Some(included)) = (info.get("startIdentifier"), info.get("startIdentifierIncluded")) {
-            let start_identifier = start_id.as_str()
-                .ok_or_else(|| JsError::new("startIdentifier must be a string"))?
-                .as_bytes()
-                .to_vec();
+        if let (Some(start_id), Some(included)) = (info.get("startIdentifier"), info.get("startIdentifierIncluded")) {
+            let start_identifier = Identifier::from_string(
+                start_id.as_str().ok_or_else(|| JsError::new("startIdentifier must be a string"))?,
+                dash_sdk::dpp::platform_value::string_encoding::Encoding::Base58,
+            )?
+            .to_vec();
             let start_identifier_included = included.as_bool().unwrap_or(true);

Locations:

  • Lines 151–161: get_contested_resource_voters_for_identity
  • Lines 371–381: get_contested_resource_vote_state_with_proof_info
  • Lines 499–510: get_contested_resource_voters_for_identity_with_proof_info
  • Lines 724–734: get_contested_resource_vote_state

Also applies to: 371-381, 499-510, 724-734

packages/wasm-sdk/src/queries/token.rs (3)

26-29: Fix JS example: wrong function name and call style

The example uses calculateTokenId(...), but the exported name here is calculate_token_id_from_contract. As written, consumers will copy/paste a broken snippet.

Apply this doc fix:

-/// const tokenId = await sdk.calculateTokenId("Hqyu8WcRwXCTwbNxdga4CN5gsVEGc67wng4TFzceyLUv", 0);
+/// // Top-level export (wasm_bindgen) – not a method on sdk
+/// const tokenId = await calculate_token_id_from_contract(
+///   "Hqyu8WcRwXCTwbNxdga4CN5gsVEGc67wng4TFzceyLUv",
+///   0
+/// );

66-72: Fix JS example: method vs free function and duplicate sdk param

The snippet calls sdk.getTokenPriceByContract(sdk, ...), which both implies a method and passes sdk twice. This will confuse users and is likely to fail at runtime.

Apply this doc fix:

-/// const priceInfo = await sdk.getTokenPriceByContract(
-///     sdk,
-///     "Hqyu8WcRwXCTwbNxdga4CN5gsVEGc67wng4TFzceyLUv",
-///     0
-/// );
+/// // Top-level export (wasm_bindgen) – pass sdk as the first argument
+/// const priceInfo = await get_token_price_by_contract(
+///   sdk,
+///   "Hqyu8WcRwXCTwbNxdga4CN5gsVEGc67wng4TFzceyLUv",
+///   0
+/// );

568-649: Tighten RawBytes decoding to strict 8-byte layout only

The current fallback decoding of any ≥4 bytes can silently produce bogus heights—let’s remove that and treat any non-8-byte payload as “unknown”.

• File: packages/wasm-sdk/src/queries/token.rs
– Lines 584–607: the PaidAt::RawBytes(bytes) arm

Proposed diff:

-                        Some(...::PaidAt::RawBytes(bytes)) => {
-                            // Raw bytes format specification (confirmed via server trace logs):
-                            // - Total length: 8 bytes (big-endian encoding)
-                            if bytes.len() >= 8 {
+                        Some(...::PaidAt::RawBytes(bytes)) => {
+                            // Only accept the exact, documented 8-byte format; otherwise treat as unknown.
+                            if bytes.len() == 8 {
                                 let timestamp = u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]) as u64;
                                 let block_height = u32::from_be_bytes([bytes[4], bytes[5], bytes[6], bytes[7]]) as u64;
 
                                 // Validate timestamp: must be 0 (unset) or >= Jan 1, 2021
                                 let validated_timestamp = if timestamp != 0 && timestamp < 1609459200 {
                                     web_sys::console::warn_1(&format!("Invalid timestamp in raw bytes: {} (too early)", timestamp).into());
                                     0
                                 } else {
                                     timestamp
                                 };
 
                                 // Validate block height: must be ≥ 1
                                 let validated_block_height = if block_height == 0 {
                                     web_sys::console::warn_1(&"Invalid block height: 0".into());
                                     1
                                 } else {
                                     block_height
                                 };
 
                                 Some((validated_timestamp * 1000, validated_block_height))
-                            } else if bytes.len() >= 4 {
-                                // Fallback: decode only the last 4 bytes as block height
-
-                                Some((0, validated_block_height))
-                            } else {
-                                web_sys::console::warn_1(&format!("Insufficient raw bytes length: {} (expected 8 or 4)", bytes.len()).into());
-                                Some((0, 0))
+                            } else {
+                                web_sys::console::warn_1(&format!("Unrecognized RawBytes layout: {} bytes", bytes.len()).into());
+                                None
                             }
                         },

Follow-up: once the RawBytes schema is formalized in the public API, we can replace this with a strongly-typed variant upstream and drop the entire branch.

🧹 Nitpick comments (15)
packages/wasm-sdk/src/wallet/dip14.rs (1)

53-56: Underscored DIP14 version constants silence warnings; consider documenting intent

Prefixing with _ avoids warnings while keeping the bytes handy. Add a brief comment (or #[allow(dead_code)]) indicating they’re reserved for serialization/interop to aid future readers.

packages/wasm-sdk/src/state_transitions/documents/mod.rs (1)

1276-1284: Remove unused entropy generation block to avoid unnecessary runtime work

let _entropy_bytes = { ... } allocates and fills randomness but isn’t used. It adds cost and introduces nondeterminism for no benefit. Prefer removing it until needed.

Apply this diff:

-        // Generate entropy for the state transition
-        let _entropy_bytes = {
-            let mut entropy = [0u8; 32];
-            if let Some(window) = web_sys::window() {
-                if let Ok(crypto) = window.crypto() {
-                    let _ = crypto.get_random_values_with_u8_array(&mut entropy);
-                }
-            }
-            entropy
-        };
packages/wasm-sdk/src/queries/protocol.rs (2)

41-51: Pick the lowest “next” protocol version instead of the first encountered.

upgrade_result is iterated in insertion order; the “first > current” may not be the smallest eligible next version. Select the minimum version greater than the current to avoid skipping an earlier activation candidate.

Apply this refactor in both places:

-    // The result is an IndexMap<u32, Option<u64>> where u32 is version and Option<u64> is activation height
-    for (version, height_opt) in upgrade_result.iter() {
-        if *version > current_version {
-            next_version = Some(*version);
-            activation_height = *height_opt;
-            // TODO: Get actual vote count and threshold from platform
-            vote_count = None;
-            threshold_reached = height_opt.is_some();
-            break;
-        }
-    }
+    // Select the smallest version strictly greater than the current version
+    if let Some((version, height_opt)) = upgrade_result
+        .iter()
+        .filter(|(v, _)| **v > current_version)
+        .min_by_key(|(v, _)| *v)
+    {
+        next_version = Some(*version);
+        activation_height = *height_opt;
+        // TODO: Get actual vote count and threshold from platform
+        vote_count = None;
+        threshold_reached = height_opt.is_some();
+    }

Also applies to: 125-133


61-63: Optional: Use json_compatible serializer for consistency.

Elsewhere in wasm-sdk query APIs you serialize with Serializer::json_compatible(). Not required here (no maps), but aligning keeps responses consistent.

-    serde_wasm_bindgen::to_value(&state)
-        .map_err(|e| JsError::new(&format!("Failed to serialize response: {}", e)))
+    let serializer = serde_wasm_bindgen::Serializer::json_compatible();
+    state.serialize(&serializer)
+        .map_err(|e| JsError::new(&format!("Failed to serialize response: {}", e)))
packages/wasm-sdk/src/queries/group.rs (1)

530-531: Remove redundant local imports of ProofMetadataResponse.

ProofMetadataResponse is already imported at the module top (Line 5). The local use lines inside these functions are unnecessary and can be dropped to reduce repetition.

-    use crate::queries::ProofMetadataResponse;
...
-    use crate::queries::ProofMetadataResponse;

Also applies to: 570-571

packages/wasm-sdk/src/queries/document.rs (1)

170-181: Make Identifier detection robust by attempting parse instead of length heuristic.

Relying on len()==44 && is_alphanumeric() is brittle. Simply try parsing as Base58 first; fall back to text on failure. This improves correctness without changing behavior for non-identifiers.

-        JsonValue::String(s) => {
-            // Check if it's an identifier (base58 encoded)
-            if s.len() == 44 && s.chars().all(|c| c.is_alphanumeric()) {
-                // Try to parse as identifier
-                match Identifier::from_string(s, dash_sdk::dpp::platform_value::string_encoding::Encoding::Base58) {
-                    Ok(id) => Ok(platform_value!(id)),
-                    Err(_) => Ok(Value::Text(s.clone())),
-                }
-            } else {
-                Ok(Value::Text(s.clone()))
-            }
-        },
+        JsonValue::String(s) => {
+            // Try to parse as Identifier; fallback to plain text
+            match Identifier::from_string(
+                s,
+                dash_sdk::dpp::platform_value::string_encoding::Encoding::Base58
+            ) {
+                Ok(id) => Ok(platform_value!(id)),
+                Err(_) => Ok(Value::Text(s.clone())),
+            }
+        },
packages/wasm-sdk/src/queries/voting.rs (1)

132-144: Index value serialization only accepts strings. Consider broader Value support.

The code currently requires every index_values entry to be a string and serializes as Value::Text. Some indices are numeric/bytes. Consider accepting numbers/booleans and serializing accordingly.

Example refactor for one block (replicate to others):

-    for value in index_values {
-        if let Some(s) = value.as_string() {
-            // Create a platform Value from the string
-            let platform_value = Value::Text(s);
-            // Serialize using bincode
-            let serialized = bincode::encode_to_vec(&platform_value, BINCODE_CONFIG)
-                .map_err(|e| JsError::new(&format!("Failed to serialize index value: {}", e)))?;
-            index_values_bytes.push(serialized);
-        } else {
-            return Err(JsError::new("Index values must be strings"));
-        }
-    }
+    for value in index_values {
+        let platform_value = if let Some(s) = value.as_string() {
+            Value::Text(s)
+        } else if let Some(n) = value.as_f64() {
+            // Prefer i64/u64 when integral; fallback to f64 otherwise
+            if n.fract() == 0.0 && n >= 0.0 {
+                Value::U64(n as u64)
+            } else if n.fract() == 0.0 {
+                Value::I64(n as i64)
+            } else {
+                Value::F64(n)
+            }
+        } else if let Some(b) = value.as_bool() {
+            Value::Bool(b)
+        } else {
+            return Err(JsError::new("Unsupported index value type; expected string/number/bool"));
+        };
+        let serialized = bincode::encode_to_vec(&platform_value, BINCODE_CONFIG)
+            .map_err(|e| JsError::new(&format!("Failed to serialize index value: {}", e)))?;
+        index_values_bytes.push(serialized);
+    }

Also applies to: 389-401, 480-492

packages/wasm-sdk/src/queries/epoch.rs (1)

188-193: Unify ProTxHash conversions for clarity.

Two different patterns are used:

  • by-ids: ProTxHash::from_byte_array(hash_array)
  • by-range: sha256d::Hash::from_slice(&bytes) -> ProTxHash::from_raw_hash(hash)

Prefer a single approach (ideally ProTxHash::from_slice or from_byte_array) if the underlying bytes are already the raw 32-byte ProTxHash to avoid unnecessary hashing semantics.

Example tweak in the range path:

-            let hash = dash_sdk::dpp::dashcore::hashes::sha256d::Hash::from_slice(&bytes).unwrap();
-            let pro_tx_hash = ProTxHash::from_raw_hash(hash);
+            let arr: [u8; 32] = bytes.try_into().unwrap();
+            let pro_tx_hash = ProTxHash::from_byte_array(arr);

Also applies to: 241-245

packages/wasm-sdk/src/queries/identity.rs (2)

698-705: Remove redundant early parsing of identity IDs.

let _identity_ids: Vec<Identifier> = ... validates IDs but you re-parse inside the loop, doubling work. Either use this parsed vector in the loop or drop it entirely.

Minimal change: delete the unused pre-parse.

-    // Convert string IDs to Identifiers
-    let _identity_ids: Vec<Identifier> = identities_ids
-        .iter()
-        .map(|id| Identifier::from_string(
-            id,
-            dash_sdk::dpp::platform_value::string_encoding::Encoding::Base58,
-        ))
-        .collect::<Result<Vec<_>, _>>()?;

1331-1340: Avoid emitting “dummy” metadata/proof; error on empty input instead.

In get_identities_contract_keys_with_proof_info, you default to zeroed ResponseMetadata/ProofInfo when identities_ids is empty. That produces a proof-shaped response with no real proof.

  • Return an error if identities_ids is empty.
  • Populate metadata/proof from the first successful fetch (already done), and don’t synthesize defaults.

Example early guard:

 pub async fn get_identities_contract_keys_with_proof_info(
@@
 ) -> Result<JsValue, JsError> {
+    if identities_ids.is_empty() {
+        return Err(JsError::new("identities_ids must not be empty"));
+    }
@@
-    let mut combined_metadata: Option<ResponseMetadata> = None;
-    let mut combined_proof: Option<ProofInfo> = None;
+    let mut combined_metadata: Option<ResponseMetadata> = None;
+    let mut combined_proof: Option<ProofInfo> = None;
@@
-    let response = ProofMetadataResponse {
-        data: all_responses,
-        metadata: combined_metadata.unwrap_or_else(|| ResponseMetadata { /* zeros */ }),
-        proof: combined_proof.unwrap_or_else(|| ProofInfo { /* zeros */ }),
-    };
+    let (metadata, proof) = combined_metadata.zip(combined_proof)
+        .ok_or_else(|| JsError::new("Failed to obtain proof/metadata for any identity"))?;
+    let response = ProofMetadataResponse { data: all_responses, metadata, proof };

Also applies to: 1387-1405

packages/wasm-sdk/src/queries/token.rs (5)

522-566: Non-verified data path (prove=false) should be documented

This function intentionally bypasses proof verification via a direct gRPC call (prove: false). Please document that results are not cryptographically verified and depend on the selected node, so callers can make an informed choice.

Apply this doc addition above the function:

+/// Get last claim info without proofs (non-verified).
+/// Security/Trust: prove=false, data is not verified on the client and depends on the responding node.
+/// Prefer the `_with_proof_info` variant when you need verifiable results.
 #[wasm_bindgen]
 pub async fn get_token_perpetual_distribution_last_claim(

Can you confirm this is the intended trust model for the wasm SDK in browser contexts?


371-371: Trim a few unnecessary clones

In several queries the vectors are not reused after building the query; cloning can be dropped to reduce allocations.

Examples:

-    let statuses_result: TokenStatuses = TokenStatus::fetch_many(sdk.as_ref(), tokens.clone())
+    let statuses_result: TokenStatuses = TokenStatus::fetch_many(sdk.as_ref(), tokens)

-    let query = IdentitiesTokenBalancesQuery {
-        identity_ids: identities.clone(),
+    let query = IdentitiesTokenBalancesQuery {
+        identity_ids: identities,
         token_id: token_identifier,
     };

Apply similarly where tokens.clone() / identities.clone() are used only to populate the query and not referenced later.

Also applies to: 162-165, 234-237, 309-311, 730-733, 892-895, 967-970


119-121: Serialization consistency: prefer json_compatible for JS interop

Some functions use serde_wasm_bindgen::to_value, others use the json_compatible serializer. For predictable JS shapes (especially with u64/BigInt concerns), prefer the json_compatible serializer everywhere for responses exported to JS.

Example change:

-    serde_wasm_bindgen::to_value(&responses)
-        .map_err(|e| JsError::new(&format!("Failed to serialize response: {}", e)))
+    let serializer = serde_wasm_bindgen::Serializer::json_compatible();
+    responses.serialize(&serializer)
+        .map_err(|e| JsError::new(&format!("Failed to serialize response: {}", e)))

Also applies to: 192-194, 395-397, 465-466, 768-770, 819-821, 855-857, 937-939, 1081-1083, 1127-1129, 1195-1197


74-81: API surface: exported as free functions vs methods on WasmSdk

Per prior learnings, token-related calls are expected as WasmSdk methods rather than top-level functions. Here they’re exported as free functions that accept &WasmSdk.

If aligning the API is desirable, consider:

#[wasm_bindgen]
impl WasmSdk {
    #[wasm_bindgen]
    pub async fn get_token_price_by_contract(
        &self,
        contract_id: &str,
        token_position: u16,
    ) -> Result<JsValue, JsError> {
        // body identical, replace sdk.as_ref() with self.as_ref()
    }
}

If the current style is intentional for backward compatibility, please add a brief note in AI_REFERENCE.md to prevent future churn.

Also applies to: 137-144, 203-211, 283-290, 357-360, 407-414, 475-482, 522-527, 672-679, 861-869, 941-947, 1016-1023, 1085-1092, 1131-1136


1163-1177: Comment vs implementation mismatch: “return value and type”

The comment states “we'll return the moment value and type”, but the LastClaimResponse only contains timestamp_ms and block_height. Epoch-based moments are currently written into block_height, which is ambiguous for consumers.

Two options:

  • Add a discriminant: moment_type: "time" | "block" | "epoch" and keep the current fields.
  • Or return a tagged enum-like shape for JS.

If you prefer not to change the API now, amend the comment to reflect the actual behavior.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cfd9b57 and 0fbb0c5.

📒 Files selected for processing (18)
  • packages/wasm-sdk/src/dpns.rs (0 hunks)
  • packages/wasm-sdk/src/lib.rs (1 hunks)
  • packages/wasm-sdk/src/queries/data_contract.rs (1 hunks)
  • packages/wasm-sdk/src/queries/document.rs (3 hunks)
  • packages/wasm-sdk/src/queries/dpns.rs (1 hunks)
  • packages/wasm-sdk/src/queries/epoch.rs (3 hunks)
  • packages/wasm-sdk/src/queries/group.rs (2 hunks)
  • packages/wasm-sdk/src/queries/identity.rs (2 hunks)
  • packages/wasm-sdk/src/queries/protocol.rs (1 hunks)
  • packages/wasm-sdk/src/queries/system.rs (3 hunks)
  • packages/wasm-sdk/src/queries/token.rs (4 hunks)
  • packages/wasm-sdk/src/queries/voting.rs (6 hunks)
  • packages/wasm-sdk/src/sdk.rs (1 hunks)
  • packages/wasm-sdk/src/state_transitions/documents/mod.rs (3 hunks)
  • packages/wasm-sdk/src/state_transitions/identity/mod.rs (0 hunks)
  • packages/wasm-sdk/src/state_transitions/tokens/mod.rs (2 hunks)
  • packages/wasm-sdk/src/wallet/dip14.rs (2 hunks)
  • packages/wasm-sdk/src/wallet/key_derivation.rs (3 hunks)
💤 Files with no reviewable changes (2)
  • packages/wasm-sdk/src/state_transitions/identity/mod.rs
  • packages/wasm-sdk/src/dpns.rs
🧰 Additional context used
🧠 Learnings (15)
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : Token functions are methods on WasmSdk, not standalone functions; avoid importing them as standalone.

Applied to files:

  • packages/wasm-sdk/src/sdk.rs
  • packages/wasm-sdk/src/queries/data_contract.rs
  • packages/wasm-sdk/src/state_transitions/tokens/mod.rs
  • packages/wasm-sdk/src/wallet/dip14.rs
  • packages/wasm-sdk/src/lib.rs
  • packages/wasm-sdk/src/queries/dpns.rs
  • packages/wasm-sdk/src/queries/protocol.rs
  • packages/wasm-sdk/src/queries/document.rs
  • packages/wasm-sdk/src/queries/epoch.rs
  • packages/wasm-sdk/src/queries/system.rs
  • packages/wasm-sdk/src/queries/group.rs
  • packages/wasm-sdk/src/queries/identity.rs
  • packages/wasm-sdk/src/wallet/key_derivation.rs
  • packages/wasm-sdk/src/state_transitions/documents/mod.rs
  • packages/wasm-sdk/src/queries/token.rs
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : When implementing WASM SDK functionality, always refer to AI_REFERENCE.md first for accurate method signatures and examples.

Applied to files:

  • packages/wasm-sdk/src/sdk.rs
  • packages/wasm-sdk/src/queries/data_contract.rs
  • packages/wasm-sdk/src/state_transitions/tokens/mod.rs
  • packages/wasm-sdk/src/lib.rs
  • packages/wasm-sdk/src/queries/dpns.rs
  • packages/wasm-sdk/src/queries/protocol.rs
  • packages/wasm-sdk/src/queries/document.rs
  • packages/wasm-sdk/src/queries/epoch.rs
  • packages/wasm-sdk/src/queries/system.rs
  • packages/wasm-sdk/src/queries/group.rs
  • packages/wasm-sdk/src/queries/identity.rs
  • packages/wasm-sdk/src/state_transitions/documents/mod.rs
  • packages/wasm-sdk/src/queries/voting.rs
  • packages/wasm-sdk/src/queries/token.rs
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : For WASM builds, fix 'time not implemented on this platform' errors by using js_sys::Date::now().

Applied to files:

  • packages/wasm-sdk/src/sdk.rs
  • packages/wasm-sdk/src/state_transitions/tokens/mod.rs
  • packages/wasm-sdk/src/lib.rs
  • packages/wasm-sdk/src/queries/protocol.rs
  • packages/wasm-sdk/src/queries/epoch.rs
  • packages/wasm-sdk/src/queries/system.rs
📚 Learning: 2024-10-18T15:39:51.172Z
Learnt from: lklimek
PR: dashpay/platform#2254
File: packages/rs-sdk/src/sdk.rs:585-585
Timestamp: 2024-10-18T15:39:51.172Z
Learning: The 'platform' project uses Rust version 1.80, so code in 'packages/rs-sdk' can use features available in Rust 1.80, such as the `abs_diff()` method.

Applied to files:

  • packages/wasm-sdk/src/sdk.rs
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
PR: dashpay/platform#2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.

Applied to files:

  • packages/wasm-sdk/src/state_transitions/tokens/mod.rs
  • packages/wasm-sdk/src/queries/identity.rs
  • packages/wasm-sdk/src/state_transitions/documents/mod.rs
📚 Learning: 2025-07-28T20:00:08.502Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/AI_REFERENCE.md:771-783
Timestamp: 2025-07-28T20:00:08.502Z
Learning: In packages/wasm-sdk/AI_REFERENCE.md, the documentation correctly shows the actual SDK method signatures (including identityCreate and identityTopUp with their full parameter lists), which may differ from the UI inputs shown in fixed_definitions.json. The UI may collect fewer parameters from users while handling additional requirements internally.

Applied to files:

  • packages/wasm-sdk/src/state_transitions/tokens/mod.rs
  • packages/wasm-sdk/src/queries/identity.rs
  • packages/wasm-sdk/src/state_transitions/documents/mod.rs
📚 Learning: 2025-01-20T16:20:59.791Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2432
File: packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs:222-225
Timestamp: 2025-01-20T16:20:59.791Z
Learning: In the Dash Platform codebase, TokenAmount from crate::balances::credits is compatible with u64 when used for token base supply.

Applied to files:

  • packages/wasm-sdk/src/state_transitions/tokens/mod.rs
📚 Learning: 2025-01-20T16:20:59.791Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2432
File: packages/rs-dpp/src/data_contract/associated_token/token_configuration/v0/mod.rs:222-225
Timestamp: 2025-01-20T16:20:59.791Z
Learning: In the Dash Platform codebase, TokenAmount is defined as `type TokenAmount = u64` in balances/credits.rs, making it directly compatible with u64 values without any conversion needed.

Applied to files:

  • packages/wasm-sdk/src/state_transitions/tokens/mod.rs
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : The WASM SDK now fully supports where and orderBy clauses for document queries; use the specified JSON array formats and supported operators.

Applied to files:

  • packages/wasm-sdk/src/lib.rs
  • packages/wasm-sdk/src/queries/dpns.rs
  • packages/wasm-sdk/src/queries/protocol.rs
  • packages/wasm-sdk/src/queries/document.rs
  • packages/wasm-sdk/src/queries/epoch.rs
  • packages/wasm-sdk/src/queries/system.rs
  • packages/wasm-sdk/src/queries/identity.rs
  • packages/wasm-sdk/src/queries/token.rs
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/**/index.html : When adding new queries or state transitions, update the definitions in index.html.

Applied to files:

  • packages/wasm-sdk/src/lib.rs
📚 Learning: 2025-08-05T13:55:39.147Z
Learnt from: thephez
PR: dashpay/platform#2718
File: packages/wasm-sdk/index.html:0-0
Timestamp: 2025-08-05T13:55:39.147Z
Learning: The get_identity_keys_with_proof_info function in the Rust WASM bindings does not support the "search" key request type and lacks the searchPurposeMap parameter. When proof mode is enabled with keyRequestType === 'search', the implementation falls back to the non-proof version (get_identity_keys) to maintain functionality.

Applied to files:

  • packages/wasm-sdk/src/queries/dpns.rs
  • packages/wasm-sdk/src/queries/document.rs
  • packages/wasm-sdk/src/queries/epoch.rs
  • packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-02-10T11:26:36.709Z
Learnt from: lklimek
PR: dashpay/platform#2405
File: packages/wasm-sdk/src/verify.rs:26-68
Timestamp: 2025-02-10T11:26:36.709Z
Learning: In the wasm-sdk package, empty vectors and placeholder values are intentionally used in verification functions during the proof-of-concept stage to ensure proper compilation and type checking.

Applied to files:

  • packages/wasm-sdk/src/queries/dpns.rs
  • packages/wasm-sdk/src/queries/document.rs
📚 Learning: 2025-07-23T08:31:05.082Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:05.082Z
Learning: Use WASM bindings to connect Rust and JavaScript code for cross-language integration

Applied to files:

  • packages/wasm-sdk/src/queries/protocol.rs
  • packages/wasm-sdk/src/queries/system.rs
  • packages/wasm-sdk/src/queries/group.rs
  • packages/wasm-sdk/src/queries/token.rs
📚 Learning: 2025-07-28T20:04:48.458Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/index.html:4360-4416
Timestamp: 2025-07-28T20:04:48.458Z
Learning: In packages/wasm-sdk, the wallet helper `derive_key_from_seed_with_path` (Rust function in src/wallet/key_derivation.rs) is synchronous; its JS wrapper returns a value immediately, so `await` is unnecessary.

Applied to files:

  • packages/wasm-sdk/src/wallet/key_derivation.rs
📚 Learning: 2025-07-28T20:00:24.323Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.

Applied to files:

  • packages/wasm-sdk/src/state_transitions/documents/mod.rs
🔇 Additional comments (23)
packages/wasm-sdk/src/state_transitions/tokens/mod.rs (2)

7-7: Import cleanup aligns with actual usage

Switching to use dash_sdk::dpp::platform_value::{Identifier, string_encoding::Encoding}; is correct and narrows the surface to what’s used in this module.


909-910: Good: localize js_sys imports to reduce global unused-import warnings

Moving use js_sys::{Reflect, Array}; inside the function avoids top-level unused warnings and limits scope. Nice.

packages/wasm-sdk/src/wallet/dip14.rs (1)

12-12: Import narrowing is correct

Keeping only sha256 and Hash here is accurate; ripemd160 is referenced via its fully qualified path below. No issues.

packages/wasm-sdk/src/wallet/key_derivation.rs (2)

462-472: Underscoring unused parameters is fine; API shape remains the same

Renaming to _xpub and _index cleanly silences warnings without affecting the wasm-bindgen ABI. No action needed until implementation is added.


476-479: Consistent parameter underscore for unused xprv

Same here—good cleanup with no ABI impact.

packages/wasm-sdk/src/state_transitions/documents/mod.rs (1)

517-517: OK to underscore unused key_id parameters; retain for API stability

Renaming key_id_key_id across document transitions is a pragmatic way to keep the parameter for forward compatibility while avoiding warnings. Ensure external docs/examples don’t imply it’s currently used.

Also applies to: 801-801, 928-928, 1043-1044, 1189-1190

packages/wasm-sdk/src/queries/data_contract.rs (1)

3-3: Import trim looks good

Switching to ProofMetadataResponse only matches actual usage in this module and eliminates unused import warnings.

packages/wasm-sdk/src/queries/protocol.rs (2)

110-110: Import narrowing to ProofMetadataResponse is correct.

This aligns with the PR goal to remove unused imports and resolves any ambiguity from glob re-exports. No functional changes introduced here.


70-86: No truncation risk for count

MasternodeProtocolVote::fetch_votes is defined as

async fn fetch_votes(
    sdk: &Sdk,
    start_protxhash: Option<ProTxHash>,
    limit: Option<u32>,
) -> Result<, Error>

so wrapping your u32 count in Some(count) matches its Option<u32> parameter exactly. No cast, clamp, or change is required here.

packages/wasm-sdk/src/sdk.rs (1)

17-17: Import cleanup looks good.

Consolidating to dash_sdk::dpp::version::PlatformVersion eliminates unused imports and keeps the module surface tight. No behavior changes.

packages/wasm-sdk/src/queries/document.rs (3)

2-2: Import narrowing to only ProofMetadataResponse is correct.

Matches the PR intent to remove unused ResponseMetadata/ProofInfo here. Keeps the module lean.


326-328: Good: dropped unused inner import(s) around proof-enabled documents query.

Less surface area; no behavior change.


672-672: Good: removed superfluous imports in DPNS proof query.

Keeps the function scope minimal and warning-free.

packages/wasm-sdk/src/queries/dpns.rs (2)

2-2: Import trimming to just ProofMetadataResponse is on point.

Matches the PR’s warning cleanup theme.


6-6: Switching to {FetchMany, Document} is appropriate.

Keeps only what’s needed and avoids unused Fetch. Inference on Document::fetch_many return type remains clear.

packages/wasm-sdk/src/queries/voting.rs (1)

269-269: Import cleanup and forced-prove pattern look good.

Localizing use crate::queries::ProofMetadataResponse; inside the proof-info fns and enforcing prove: true aligns with the PR’s objective of removing warnings while making intent explicit.

Also applies to: 357-357, 466-466, 574-574, 637-637

packages/wasm-sdk/src/queries/epoch.rs (2)

2-2: Import narrowing matches the PR goals.

  • use crate::queries::ProofMetadataResponse; centralized where needed.
  • Switched to drive_proof_verifier::types::ProposerBlockCountById for the by-ids path.

No functional changes beyond warning cleanup. LGTM.

Also applies to: 168-169


220-224: Action Required: Verify Inclusive vs. Exclusive Pagination Semantics

I searched the TypeScript/JavaScript codebase and did not find any callers of getEvoNodesProposedEpochBlocksByRange nor any references to start_after or startIncluded in the UI code. It’s therefore unclear whether the front-end expects an exclusive range (start_included: false) or an inclusive one.

Please manually verify that the pagination behavior in:

packages/wasm-sdk/src/queries/epoch.rs
Lines 220–224

Some(QueryStartInfo {
    start_key: pro_tx_hash.to_byte_array().to_vec(),
    start_included: false,  // currently exclusive
})

matches the UI requirements. If the UI expects the first item after start_key to be included, change start_included to true (and update any docs/tests accordingly).

packages/wasm-sdk/src/lib.rs (1)

15-19: No re-export name collisions detected

After inspecting the public API of both modules, there are no overlapping identifiers:

• packages/wasm-sdk/src/dpns.rs exports
– RegisterDpnsNameResult
– dpns_convert_to_homograph_safe
– dpns_is_valid_username
– dpns_is_contested_username

• packages/wasm-sdk/src/queries/dpns.rs exports
– get_dpns_username_by_name
– get_dpns_username_by_name_with_proof_info

Since none of these names collide, the wildcard re-exports of queries::dpns::* and of dpns::* do not cause ambiguous imports. You can safely keep both re-exports.

Likely an incorrect or invalid review comment.

packages/wasm-sdk/src/queries/system.rs (1)

386-386: ProofMetadataResponse-only imports: good cleanup.

Switching to only ProofMetadataResponse (and narrowing grovedb types) removes unused imports cleanly without functional impact. Matches PR intent to eliminate warnings.

Also applies to: 419-419, 456-456, 458-458

packages/wasm-sdk/src/queries/identity.rs (1)

4-4: Import set narrowed appropriately.

Dropping FetchUnproved and keeping Fetch, FetchMany, Identifier, Identity matches usage and removes warnings.

packages/wasm-sdk/src/queries/token.rs (2)

5-5: Import surface cleanup looks right

Switching to the single ProofMetadataResponse import aligns with later usages (metadata.into(), proof.into()), and keeps Serialize/Deserialize available for local response types.


711-711: Minor: local import removals are fine

Noting the localized import adjustments to reduce warnings. No functional impact.

Also applies to: 774-774

QuantumExplorer and others added 8 commits September 3, 2025 23:13
…IdentitiesContractKeys

The documentTypeName parameter was unused in the implementation but still defined
in the API. This removes it from:
- Rust function signatures
- API definitions JSON
- Documentation examples
- HTML interface calls

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add documentation for unused _result_type parameters in voting queries:
- Document proper protobuf enum mapping for future implementation
- Explain current incorrect logic using allow_include_locked_and_abstaining_vote_tally
- Add GitHub issue placeholder for tracking implementation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…functions

Remove unused _result_type and _allow_include_locked_and_abstaining_vote_tally
parameters from both get_contested_resources and get_contested_resources_with_proof_info
functions as these fields do not exist in the GetContestedResourcesRequest protobuf.

This fixes build warnings and aligns function signatures with the actual protobuf schema.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
…meter removal

Update API documentation, examples, and definitions to reflect the removal of  unused _result_type and _allow_include_locked_and_abstaining_vote_tally  parameters from get_contested_resources functions.
@thephez thephez marked this pull request as ready for review September 4, 2025 13:53
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
packages/wasm-sdk/src/queries/voting.rs (4)

372-378: startIdentifier must be base58-decoded, not raw ASCII bytes

as_bytes() sends ASCII of the string, not the 32-byte Identifier required by the API. Decode using Identifier::from_string(..., Base58)?.to_vec().

-            let start_identifier = start_id.as_str()
-                .ok_or_else(|| JsError::new("startIdentifier must be a string"))?
-                .as_bytes()
-                .to_vec();
+            let start_identifier = Identifier::from_string(
+                start_id.as_str().ok_or_else(|| JsError::new("startIdentifier must be a string"))?,
+                dash_sdk::dpp::platform_value::string_encoding::Encoding::Base58,
+            )?.to_vec();

503-507: Base58 decode startIdentifier here as well

Same ASCII vs Identifier bug in this function.

-            let start_identifier = start_id.as_str()
-                .ok_or_else(|| JsError::new("startIdentifier must be a string"))?
-                .as_bytes()
-                .to_vec();
+            let start_identifier = Identifier::from_string(
+                start_id.as_str().ok_or_else(|| JsError::new("startIdentifier must be a string"))?,
+                dash_sdk::dpp::platform_value::string_encoding::Encoding::Base58,
+            )?.to_vec();

733-739: Base58 decode startIdentifier in non-proof variant

Same correctness issue here.

-            let start_identifier = start_id.as_str()
-                .ok_or_else(|| JsError::new("startIdentifier must be a string"))?
-                .as_bytes()
-                .to_vec();
+            let start_identifier = Identifier::from_string(
+                start_id.as_str().ok_or_else(|| JsError::new("startIdentifier must be a string"))?,
+                dash_sdk::dpp::platform_value::string_encoding::Encoding::Base58,
+            )?.to_vec();

341-358: Align docs and JS/TS bindings with result_type API

If retaining the _result_type parameter in get_contested_resource_vote_state_with_proof_info (packages/wasm-sdk/src/queries/voting.rs lines 341–358 & 704–721), update:

  • packages/wasm-sdk/AI_REFERENCE.md to document accepted values ("documents", "vote_tally", "documents_and_vote_tally") and their mapping to the Protobuf ResultType enum
  • JS/TS bindings under packages/dapi-grpc/clients/platform/v0 (platform_protoc.js, platform_pb.js) to include the result_type field and its semantics

Otherwise remove all result_type references from public APIs.

♻️ Duplicate comments (2)
packages/wasm-sdk/src/queries/voting.rs (2)

347-354: result_type param is ignored; wire it to protobuf enum or remove it

Leaving _result_type unused while deriving result_type from another flag changes API semantics. Map the string to the proto enum and use it, or drop the param and update bindings/docs. This is a repeat of an earlier concern.

Apply within this signature to use the parameter:

-    // TODO: Implement result_type parameter properly
-    // Currently unused - should map to protobuf ResultType enum:
-    // - "documents" -> 0 (DOCUMENTS)
-    // - "vote_tally" -> 1 (VOTE_TALLY)
-    // - "documents_and_vote_tally" -> 2 (DOCUMENTS_AND_VOTE_TALLY)
-    // See: https://github.com/dashpay/platform/issues/2760
-    _result_type: &str,
+    // result_type: "documents" | "vote_tally" | "documents_and_vote_tally"
+    result_type: &str,

Then before building the request:

+    // Validate/map result_type to protobuf enum
+    let result_type_code = parse_vote_state_result_type(result_type)?;

And in the request (see separate diff below), set result_type: result_type_code,.

Add this helper once in the module:

#[inline]
fn parse_vote_state_result_type(rt: &str) -> Result<i32, JsError> {
    use dapi_grpc::platform::v0::get_contested_resource_vote_state_request::get_contested_resource_vote_state_request_v0::ResultType as Rt;
    match rt.to_ascii_lowercase().as_str() {
        "documents" | "document" => Ok(Rt::Documents as i32),
        "vote_tally" | "tally" => Ok(Rt::VoteTally as i32),
        "documents_and_vote_tally" | "documents+vote_tally" | "full" => Ok(Rt::DocumentsAndVoteTally as i32),
        _ => Err(JsError::new("invalid result_type; expected 'documents', 'vote_tally', or 'documents_and_vote_tally'")),
    }
}

Script to find callers relying on result_type:

#!/bin/bash
rg -nP -C2 '\bget_contested_resource_vote_state(_with_proof_info)?\s*\(' --type=rust --type=ts --type=js

710-717: Repeat: result_type param unused; fix or remove

Non-proof variant also ignores result_type. Wire it to the request or remove and update bindings/docs.

-    // TODO: Implement result_type parameter properly
-    // Currently unused - should map to protobuf ResultType enum:
-    // - "documents" -> 0 (DOCUMENTS)
-    // - "vote_tally" -> 1 (VOTE_TALLY)
-    // - "documents_and_vote_tally" -> 2 (DOCUMENTS_AND_VOTE_TALLY)
-    // See: https://github.com/dashpay/platform/issues/2760
-    _result_type: &str,
+    // result_type: "documents" | "vote_tally" | "documents_and_vote_tally"
+    result_type: &str,

Before request:

+    let result_type_code = parse_vote_state_result_type(result_type)?;
🧹 Nitpick comments (7)
packages/wasm-sdk/src/queries/voting.rs (6)

265-265: Local use is fine; consider hoisting shared import

Importing ProofMetadataResponse inside each function is valid and avoids unused warnings. Optionally hoist once at module scope to deduplicate.


470-470: Same note on local import

Repeated use crate::queries::ProofMetadataResponse; across functions; optional hoist to module scope.


578-579: Local import OK; optional dedupe

Same as above; optional module-level import.


641-642: Local import OK; optional dedupe

Same as above.


328-332: Avoid unnecessary clones when constructing ProofMetadataResponse

metadata.clone().into() and proof.clone().into() can be metadata.into() and proof.into() since the values aren’t reused.

Example:

-    let response = ProofMetadataResponse {
-        data,
-        metadata: metadata.clone().into(),
-        proof: proof.clone().into(),
-    };
+    let response = ProofMetadataResponse {
+        data,
+        metadata: metadata.into(),
+        proof: proof.into(),
+    };

Also applies to: 447-450, 559-562, 621-624, 689-693


391-403: DRY: extract index_values serialization to a helper

The same JsValue->bincode serialization loop is repeated. Extract a small helper to reduce duplication and centralize error handling.

Proposed helper:

fn serialize_index_values(index_values: Vec<JsValue>) -> Result<Vec<Vec<u8>>, JsError> {
    let mut out = Vec::with_capacity(index_values.len());
    for value in index_values {
        if let Some(s) = value.as_string() {
            let pv = Value::Text(s);
            let ser = bincode::encode_to_vec(&pv, BINCODE_CONFIG)
                .map_err(|e| JsError::new(&format!("Failed to serialize index value: {}", e)))?;
            out.push(ser);
        } else {
            return Err(JsError::new("Index values must be strings"));
        }
    }
    Ok(out)
}

Then replace the loops with:

let index_values_bytes = serialize_index_values(index_values)?;

Also applies to: 484-496, 752-764

packages/wasm-sdk/src/queries/identity.rs (1)

697-697: Verify the unused variable naming follows Rust conventions

The renaming of identities_ids to _identity_ids indicates it's intentionally unused. However, there's an inconsistency in the variable naming - the original parameter is identities_ids (plural) but the underscore version is _identity_ids (singular). This could be confusing for future maintainers.

Consider using consistent naming:

-    let _identity_ids: Vec<Identifier> = identities_ids
+    let _identities_ids: Vec<Identifier> = identities_ids
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0fbb0c5 and 887ccb1.

📒 Files selected for processing (11)
  • packages/wasm-sdk/AI_REFERENCE.md (1 hunks)
  • packages/wasm-sdk/api-definitions.json (0 hunks)
  • packages/wasm-sdk/docs.html (2 hunks)
  • packages/wasm-sdk/docs_manifest.json (1 hunks)
  • packages/wasm-sdk/index.html (0 hunks)
  • packages/wasm-sdk/src/queries/identity.rs (2 hunks)
  • packages/wasm-sdk/src/queries/system.rs (3 hunks)
  • packages/wasm-sdk/src/queries/token.rs (1 hunks)
  • packages/wasm-sdk/src/queries/voting.rs (8 hunks)
  • packages/wasm-sdk/src/state_transitions/documents/mod.rs (2 hunks)
  • packages/wasm-sdk/test/ui-automation/fixtures/test-data.js (0 hunks)
💤 Files with no reviewable changes (3)
  • packages/wasm-sdk/test/ui-automation/fixtures/test-data.js
  • packages/wasm-sdk/index.html
  • packages/wasm-sdk/api-definitions.json
✅ Files skipped from review due to trivial changes (2)
  • packages/wasm-sdk/docs_manifest.json
  • packages/wasm-sdk/AI_REFERENCE.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/wasm-sdk/src/state_transitions/documents/mod.rs
  • packages/wasm-sdk/src/queries/system.rs
  • packages/wasm-sdk/src/queries/token.rs
🧰 Additional context used
📓 Path-based instructions (1)
packages/wasm-sdk/src/**/*.rs

📄 CodeRabbit inference engine (packages/wasm-sdk/CLAUDE.md)

packages/wasm-sdk/src/**/*.rs: When implementing WASM SDK functionality, always refer to AI_REFERENCE.md first for accurate method signatures and examples.
For WASM builds, fix 'time not implemented on this platform' errors by using js_sys::Date::now().
Token functions are methods on WasmSdk, not standalone functions; avoid importing them as standalone.
The WASM SDK now fully supports where and orderBy clauses for document queries; use the specified JSON array formats and supported operators.

Files:

  • packages/wasm-sdk/src/queries/identity.rs
  • packages/wasm-sdk/src/queries/voting.rs
🧠 Learnings (11)
📚 Learning: 2025-07-28T20:00:08.502Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/AI_REFERENCE.md:771-783
Timestamp: 2025-07-28T20:00:08.502Z
Learning: In packages/wasm-sdk/AI_REFERENCE.md, the documentation correctly shows the actual SDK method signatures (including identityCreate and identityTopUp with their full parameter lists), which may differ from the UI inputs shown in fixed_definitions.json. The UI may collect fewer parameters from users while handling additional requirements internally.

Applied to files:

  • packages/wasm-sdk/docs.html
  • packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/**/index.html : The WASM SDK now fully supports where and orderBy clauses for document queries; use the specified JSON array formats and supported operators.

Applied to files:

  • packages/wasm-sdk/docs.html
📚 Learning: 2025-09-03T14:41:16.196Z
Learnt from: thephez
PR: dashpay/platform#2754
File: packages/wasm-sdk/AI_REFERENCE.md:766-766
Timestamp: 2025-09-03T14:41:16.196Z
Learning: In packages/wasm-sdk/, the AI_REFERENCE.md file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in AI_REFERENCE.md, as manual changes to AI_REFERENCE.md would be overwritten during regeneration.

Applied to files:

  • packages/wasm-sdk/docs.html
📚 Learning: 2025-09-03T14:42:29.958Z
Learnt from: thephez
PR: dashpay/platform#2754
File: packages/wasm-sdk/docs.html:1970-1971
Timestamp: 2025-09-03T14:42:29.958Z
Learning: In packages/wasm-sdk/, the docs.html file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in docs.html, as manual changes to docs.html would be overwritten during regeneration.

Applied to files:

  • packages/wasm-sdk/docs.html
📚 Learning: 2025-07-28T20:00:24.323Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.

Applied to files:

  • packages/wasm-sdk/docs.html
  • packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-08-05T13:55:39.147Z
Learnt from: thephez
PR: dashpay/platform#2718
File: packages/wasm-sdk/index.html:0-0
Timestamp: 2025-08-05T13:55:39.147Z
Learning: The get_identity_keys_with_proof_info function in the Rust WASM bindings does not support the "search" key request type and lacks the searchPurposeMap parameter. When proof mode is enabled with keyRequestType === 'search', the implementation falls back to the non-proof version (get_identity_keys) to maintain functionality.

Applied to files:

  • packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : Token functions are methods on WasmSdk, not standalone functions; avoid importing them as standalone.

Applied to files:

  • packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
PR: dashpay/platform#2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.

Applied to files:

  • packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : When implementing WASM SDK functionality, always refer to AI_REFERENCE.md first for accurate method signatures and examples.

Applied to files:

  • packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-07-23T08:31:42.268Z
Learnt from: CR
PR: dashpay/platform#0
File: packages/wasm-sdk/CLAUDE.md:0-0
Timestamp: 2025-07-23T08:31:42.268Z
Learning: Applies to packages/wasm-sdk/src/**/*.rs : The WASM SDK now fully supports where and orderBy clauses for document queries; use the specified JSON array formats and supported operators.

Applied to files:

  • packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-09-03T15:44:33.889Z
Learnt from: thephez
PR: dashpay/platform#2754
File: packages/wasm-sdk/docs.html:0-0
Timestamp: 2025-09-03T15:44:33.889Z
Learning: In packages/wasm-sdk/docs.html, thephez prefers to keep realistic-looking private key placeholders in documentation examples rather than using obvious fake placeholders, despite secret scanner warnings.

Applied to files:

  • packages/wasm-sdk/src/queries/identity.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
  • GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
  • GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
  • GitHub Check: Build JS packages / Build JS
  • GitHub Check: JS NPM security audit
  • GitHub Check: Determine changed packages
🔇 Additional comments (3)
packages/wasm-sdk/src/queries/identity.rs (1)

4-4: LGTM on import cleanup

The removal of FetchUnproved from the import is consistent with the overall code cleanup effort to remove unused imports.

packages/wasm-sdk/docs.html (2)

620-620: Documentation aligned with code changes

The example code has been correctly updated to remove the documentTypeName parameter, aligning with the Rust implementation changes in the identity.rs file. This maintains consistency between the API documentation and the actual implementation.


1172-1172: Parameter reduction reflects API simplification

The example has been streamlined to remove unnecessary parameters, which aligns with the overall goal of cleaning up the API surface and removing unused functionality.

@QuantumExplorer QuantumExplorer merged commit 4b7124b into v2.1-dev Sep 7, 2025
19 of 20 checks passed
@QuantumExplorer QuantumExplorer deleted the wasm-sdk-fix-build-warnings branch September 7, 2025 18:42
@coderabbitai coderabbitai bot mentioned this pull request Sep 23, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants