Skip to content

Conversation

thephez
Copy link
Collaborator

@thephez thephez commented Aug 19, 2025

Issue being fixed or feature implemented

Add support for the index_values parameter to the contested resource voters query in the WASM SDK to enable proofs for that query.

What was done?

  • Added index_values parameter to get_contested_resource_voters_for_identity_with_proof_info function
  • Implemented proper serialization of index values using bincode for compatibility with the underlying query system
  • Updated UI automation tests to enable proof support for the getContestedResourceVotersForIdentity query

How Has This Been Tested?

  • Updated existing UI automation test suite to verify the new parameter functionality
  • Tested proof generation with index values through the automated test framework
  • Manual verification through the WASM SDK web interface

Breaking Changes

None - this is a backward compatible addition that adds an optional parameter to enhance query capabilities.

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

Summary by CodeRabbit

  • New Features

    • Added support to provide index values when querying contested-resource voters, enabling targeted filtering instead of querying all resources.
    • Improved input validation with clear errors when index values are not strings.
    • Response format remains unchanged and continues to include proof information.
  • Tests

    • Enabled proof-info test path for the contested-resource voters query to increase coverage and reliability.

…rs query

- Add index_values parameter to get_contested_resource_voters_for_identity_with_proof_info
- Implement proper serialization of index values using bincode
- Enable proof support for getContestedResourceVotersForIdentity in tests

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

Co-Authored-By: Claude <noreply@anthropic.com>
@thephez thephez added this to the v2.1 milestone Aug 19, 2025
Copy link
Contributor

coderabbitai bot commented Aug 19, 2025

Walkthrough

Added index_values parameter to get_contested_resource_voters_for_identity_with_proof_info, serializing provided JsValue strings into bytes and including them in the gRPC request. Updated UI automation test to enable hasProofSupport for this query. Error handling added for non-string values and serialization failures; existing request/response structure unchanged.

Changes

Cohort / File(s) Summary
WASM SDK: voting query index_values support
packages/wasm-sdk/src/queries/voting.rs
Added index_values: Vec to public API; validated JsValue as strings; serialized via BINCODE_CONFIG into index_values_bytes; passed to GetContestedResourceVotersForIdentityRequest; preserved existing flow, prove=true, and response shaping; added error paths for invalid types/serialization.
UI automation: enable proof path for contested voters query
packages/wasm-sdk/test/ui-automation/tests/query-execution.spec.js
Set hasProofSupport: true for getContestedResourceVotersForIdentity test entry, enabling proof-info test execution.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller (JS)
  participant W as Wasm SDK (voting.rs)
  participant S as Serializer (bincode)
  participant G as gRPC Client
  participant P as Platform

  C->>W: get_contested_resource_voters_for_identity_with_proof_info(..., index_values)
  W->>S: For each JsValue: validate string and serialize to bytes
  alt Any value not string or serialization error
    S-->>W: Error
    W-->>C: JsError("Index values must be strings" / serialization error)
  else All values serialized
    S-->>W: index_values_bytes
    W->>G: GetContestedResourceVotersForIdentityRequest{..., index_values: bytes, prove: true}
    G->>P: Request
    P-->>G: Response (proof + data)
    G-->>W: Response
    W-->>C: JSON-serializable result
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • QuantumExplorer
  • pauldelucia

Poem

A rabbit taps keys with lively delight,
Index seeds packed snugly, ready for flight.
Bytes hop down gRPC’s bright lane,
Proofs now enabled—no longer in vain.
With stringy snacks and serialized cheer,
Queries contest, and answers appear. 🐇✨

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ 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-feat-add-proofs

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 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.

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: 0

Caution

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

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

460-465: Move index_values to the end of the wasm_bindgen signature and make it optional

Adding a positional parameter in the middle of a wasm_bindgen-exported function is a breaking change. To preserve backwards compatibility, remove index_values from its current spot, append it at the end as Option<Vec<JsValue>>, and update the conversion logic to unwrap_or_default().

• In packages/wasm-sdk/src/queries/voting.rs (around line 455):

  • Remove the existing
    index_values: Vec<JsValue>,
  • Append
    index_values: Option<Vec<JsValue>>,

at the end of the parameter list.

• Update the conversion block below the signature from something like:

- let values: Vec<_> = index_values.into_iter()
    .map(|v| /* … */)
    .collect();

to:

+ let values: Vec<_> = index_values
+     .unwrap_or_default()
    .into_iter()
    .map(|v| /* … */)
    .collect();

Revised signature snippet:

 pub async fn get_contested_resource_voters_for_identity_with_proof_info(
     sdk: &WasmSdk,
     data_contract_id: &str,
     document_type_name: &str,
     index_name: &str,
-    index_values: Vec<JsValue>,
     contestant_id: &str,
     start_at_identifier_info: Option<String>,
     count: Option<u32>,
     order_ascending: Option<bool>,
+    index_values: Option<Vec<JsValue>>,
 ) -> Result<JsValue, JsError> {
🧹 Nitpick comments (1)
packages/wasm-sdk/src/queries/voting.rs (1)

479-493: Harden serialization: pre-allocate capacity, better errors, and support optional param.

Minor ergonomics/perf and debuggability improvements. If you make index_values optional (per prior comment), unwrap it; also improve error text and pre-allocate.

Apply:

-    // Convert JsValue index values to Vec<Vec<u8>> using bincode serialization
-    let mut index_values_bytes: Vec<Vec<u8>> = Vec::new();
-    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"));
-        }
-    }
+    // Convert JsValue index values to Vec<Vec<u8>> using bincode serialization
+    // If signature is `Option<Vec<JsValue>>`, default to empty; otherwise remove this unwrap.
+    let index_values = index_values.unwrap_or_default();
+    let mut index_values_bytes: Vec<Vec<u8>> = Vec::with_capacity(index_values.len());
+    for (i, value) in index_values.into_iter().enumerate() {
+        if let Some(s) = value.as_string() {
+            let platform_value = Value::Text(s);
+            let serialized = bincode::encode_to_vec(&platform_value, BINCODE_CONFIG)
+                .map_err(|e| JsError::new(&format!(
+                    "Failed to serialize index value at position {} for index '{}': {}",
+                    i, index_name, e
+                )))?;
+            index_values_bytes.push(serialized);
+        } else {
+            return Err(JsError::new(&format!(
+                "Index values for '{}' must be strings (invalid at position {})",
+                index_name, i
+            )));
+        }
+    }
📜 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 5ab180e and be43b55.

📒 Files selected for processing (2)
  • packages/wasm-sdk/src/queries/voting.rs (3 hunks)
  • packages/wasm-sdk/test/ui-automation/tests/query-execution.spec.js (1 hunks)
🧰 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/voting.rs
🧠 Learnings (1)
📚 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/voting.rs
🔇 Additional comments (3)
packages/wasm-sdk/src/queries/voting.rs (1)

524-529: LGTM: index_values correctly propagated into the gRPC request.

Passing the bincode-encoded index_values_bytes to GetContestedResourceVotersForIdentityRequestV0 aligns with the existing pattern in sibling functions and should enable proofs for this query.

packages/wasm-sdk/test/ui-automation/tests/query-execution.spec.js (2)

832-832: Enabling proof support here makes sense given the SDK now accepts index_values.

This will exercise the proof path in UI tests for getContestedResourceVotersForIdentity.


832-832: No changes needed: indexValues injection is already supported

The ParameterInjector mapping (utils/parameter-injector.js:93) includes 'indexValues', and your test fixtures (fixtures/test-data.js:414–422) supply indexValues for getContestedResourceVotersForIdentity. The injectParameters method (utils/parameter-injector.js:15–22) will pick up and populate that field automatically.

@QuantumExplorer QuantumExplorer merged commit cfd9b57 into v2.1-dev Aug 20, 2025
22 of 23 checks passed
@QuantumExplorer QuantumExplorer deleted the wasm-sdk-feat-add-proofs branch August 20, 2025 04:27
lklimek pushed a commit that referenced this pull request Sep 1, 2025
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