-
Notifications
You must be signed in to change notification settings - Fork 44
fix(wasm-sdk): enable proofs for getContestedResourceVotersForIdentity #2732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…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>
WalkthroughAdded 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
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
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
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 unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
: Moveindex_values
to the end of the wasm_bindgen signature and make it optionalAdding 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 asOption<Vec<JsValue>>
, and update the conversion logic tounwrap_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.
📒 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
toGetContestedResourceVotersForIdentityRequestV0
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 acceptsindex_values
.This will exercise the proof path in UI tests for getContestedResourceVotersForIdentity.
832-832
: No changes needed:indexValues
injection is already supportedThe
ParameterInjector
mapping (utils/parameter-injector.js:93) includes'indexValues'
, and your test fixtures (fixtures/test-data.js:414–422) supplyindexValues
forgetContestedResourceVotersForIdentity
. TheinjectParameters
method (utils/parameter-injector.js:15–22) will pick up and populate that field automatically.
#2732) Co-authored-by: Claude <noreply@anthropic.com>
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?
index_values
parameter toget_contested_resource_voters_for_identity_with_proof_info
functiongetContestedResourceVotersForIdentity
queryHow Has This Been Tested?
Breaking Changes
None - this is a backward compatible addition that adds an optional parameter to enhance query capabilities.
Checklist:
Summary by CodeRabbit
New Features
Tests