-
Notifications
You must be signed in to change notification settings - Fork 44
feat(sdk)!: provide all getStatus info #2729
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
WalkthroughReplaces PlatformStatus with a nested StatusResponse sourced from a dAPI gRPC v0 GetStatusRequest; adds multiple wasm-bindgen system endpoints and proof-info variants; marks several queries as not supporting proofs in api-definitions; updates the UI to show a “no proof” note and adjusts tests to expect the structured status JSON. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WasmSDK as Wasm SDK (wasm entry)
participant Dapi as dAPI gRPC (GetStatus v0)
Client->>WasmSDK: get_status()
WasmSDK->>Dapi: GetStatusRequest (Version::V0)
alt v0 response present
Dapi-->>WasmSDK: GetStatusResponse { inner.version, node, chain, network, state_sync, time }
WasmSDK->>WasmSDK: map -> StatusResponse (version,node,chain,network,stateSync,time)
note right of WasmSDK #D6EAF8: serialize via serde_wasm_bindgen\nhex-encode IDs, string heights
WasmSDK-->>Client: JsValue(JSON StatusResponse)
else missing version / error
Dapi-->>WasmSDK: error / missing version
WasmSDK-->>Client: JsError
end
sequenceDiagram
participant Client
participant WasmSDK
participant Store
Client->>WasmSDK: get_path_elements_with_proof_info(path, keys)
WasmSDK->>Store: query path elements (with optional proof)
alt elements + proof
Store-->>WasmSDK: elements bytes + proof metadata
WasmSDK->>WasmSDK: base64-encode elements, wrap in ProofMetadataResponse
WasmSDK-->>Client: JsValue(ProofMetadataResponse)
else elements only
Store-->>WasmSDK: elements
WasmSDK-->>Client: JsValue(elements)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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
🔭 Outside diff range comments (2)
packages/wasm-sdk/src/queries/system.rs (2)
99-101
: Serialize with JSON-compatible serializer to guaranteenull
forOption::None
.This ensures
version
,block_height
, andcore_height
becomenull
(notundefined
/omitted) when unavailable, matching the PR’s stated behavior and keeping consistency with other functions in this file that already usejson_compatible
.Apply:
- serde_wasm_bindgen::to_value(&status) - .map_err(|e| JsError::new(&format!("Failed to serialize response: {}", e))) + let serializer = serde_wasm_bindgen::Serializer::json_compatible(); + status + .serialize(&serializer) + .map_err(|e| JsError::new(&format!("Failed to serialize response: {}", e)))
11-15
: Makeversion: Option<u32>
serialize to JS null (use json_compatible in wasm-sdk queries/system.rs)get_status (and a few other serializers in this file) use serde_wasm_bindgen::to_value(...) — switch to serde_wasm_bindgen::Serializer::json_compatible() so Option::None becomes JSON null (not omitted/undefined).
Files/locations to change:
- packages/wasm-sdk/src/queries/system.rs
- line 99: serde_wasm_bindgen::to_value(&status)
- line 169: serde_wasm_bindgen::to_value(&info)
- line 178: serde_wasm_bindgen::to_value(&info)
- line 313: serde_wasm_bindgen::to_value(&result)
- line 378: serde_wasm_bindgen::to_value(&elements)
Suggested replacement (example for get_status):
- Replace:
serde_wasm_bindgen::to_value(&status)
.map_err(|e| JsError::new(&format!("Failed to serialize response: {}", e)))- With:
let serializer = serde_wasm_bindgen::Serializer::json_compatible();
status.serialize(&serializer)
.map_err(|e| JsError::new(&format!("Failed to serialize response: {}", e)))Apply the same pattern to the other to_value(...) calls listed above. Note: the repo already uses json_compatible in many wasm-sdk serializers — make this file consistent.
Also verify JS/TS consumers that read
.version
handle a present null value (search for.version
usage and update callers if they expect the field to be missing/undefined).
🧹 Nitpick comments (3)
packages/wasm-sdk/src/queries/system.rs (3)
77-85
: Confirm that “first_*_block_height” matches the intended semantics for getStatus heights.ExtendedEpochInfo exposes “first block” heights of the current epoch. If getStatus is expected to report the current/latest platform/core heights, using epoch “first” heights may be misleading. If the intended semantics are epoch-first heights, consider documenting this in the response or renaming fields in a future breaking change; otherwise, fetch the appropriate “current/latest” heights.
81-83
: Inconsistent casting; double-check underlying types.
first_block_height()
is used asu64
directly, whilefirst_core_block_height()
is cast tou64
. If both getters already return the same integral width, keep casting consistent (either both cast or neither). Iffirst_core_block_height()
is narrower (e.g.,u32
), consideru64::from(...)
for clarity.
86-89
: Swallowing fetch errors hides context; optionally add a trace for debugging.Returning
(None, None, None)
is aligned with the PR goal, but it can be hard to diagnose issues. Consider logging the error (behind a debug flag) to aid troubleshooting without changing the API.Would you like a minimal
log::warn!
-based snippet that compiles to no-op in release unless logging is enabled?
📜 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 (1)
packages/wasm-sdk/src/queries/system.rs
(2 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/system.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 : For WASM builds, fix 'time not implemented on this platform' errors by using js_sys::Date::now().
Applied to files:
packages/wasm-sdk/src/queries/system.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). (3)
- GitHub Check: Build JS packages / Build JS
- GitHub Check: build-and-test-wasm-sdk
- GitHub Check: build-wasm-sdk
🔇 Additional comments (1)
packages/wasm-sdk/src/queries/system.rs (1)
93-97
: Using the network protocol version from epoch info instead of the SDK’s compiled version is spot on.This aligns the API with actual network state and resolves the misleading value problem.
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
🧹 Nitpick comments (6)
packages/wasm-sdk/index.html (1)
4328-4344
: Proof gating logic is correct; consider resetting the proof toggle to avoid stale checked stateThe gating uses definition?.supportsProof !== false, which correctly treats unspecified as supported. One minor UX refinement: when a non-proof endpoint is selected after a proof-enabled one, the hidden checkbox can remain checked. Resetting it avoids later confusion if the user returns to a proof-capable endpoint.
Apply this small tweak inside the else branch:
- } else { + } else { proofToggleContainer.style.display = 'none'; noProofInfoContainer.style.display = 'block'; + const proofToggle = document.getElementById('proofToggle'); + if (proofToggle) proofToggle.checked = false; }packages/wasm-sdk/src/queries/system.rs (5)
179-308
: Accurate field mapping; minor readability nit on Option chainingThe field-by-field mapping from the v0 response is thorough and safely handles missing substructures. One small readability nit: prefer and_then over map(...).flatten() for nested Option access to keep the style uniform (as you do elsewhere in this function).
Example (no behavior change):
- dapi: v0_response.version.as_ref() - .map(|v| v.software.as_ref()) - .flatten() - .map(|s| s.dapi.clone()) + dapi: v0_response.version.as_ref() + .and_then(|v| v.software.as_ref()) + .map(|s| s.dapi.clone()) .unwrap_or_else(|| "unknown".to_string()),
314-392
: Reasonable interim approach to quorum typing; add a note to revisit once type info is exposedThe heuristic quorum_type inference by member_count is a practical placeholder and you mark TODOs. Once the platform exposes the actual quorum type, this block should be tightened to avoid mislabeling.
If helpful, I can open a follow-up to swap the heuristic with concrete types once the SDK surface exposes them.
145-151
: PathElement shape is ambiguous; include the actual path and the key explicitlyCurrently, PathElement { path: Vec, value } is instantiated with path: vec![key], which overloads “path” to actually carry the key. This can confuse consumers expecting the real path components. Recommend carrying both the full path and the key explicitly.
Apply these diffs:
-struct PathElement { - path: Vec<String>, - value: Option<String>, -} +struct PathElement { + // The path segments provided by the caller + path: Vec<String>, + // The key requested at that path + key: String, + // Base64-encoded value (if any) + value: Option<String>, +}And update both constructors below (see next comments).
569-586
: Populate PathElement with the actual path and a separate key fieldUse the provided path (cloned) and add the key field so UIs can render unambiguously.
- let elements: Vec<PathElement> = keys.into_iter() + let elements: Vec<PathElement> = keys.into_iter() .map(|key| { // Check if this key exists in the result let value = path_elements_result.get(key.as_bytes()) .and_then(|element_opt| element_opt.as_ref()) .and_then(|element| { // Element can contain different types, we'll serialize it as base64 element.as_item_bytes().ok().map(|bytes| { use base64::Engine; base64::engine::general_purpose::STANDARD.encode(bytes) }) }); PathElement { - path: vec![key], + path: path.clone(), + key, value, } }) .collect();
704-719
: Mirror the PathElement shape change in the proof-enabled variantKeep both variants aligned so frontends can consume a single shape regardless of proof selection.
- let elements: Vec<PathElement> = keys.into_iter() + let elements: Vec<PathElement> = keys.into_iter() .map(|key| { let value = path_elements_result.get(key.as_bytes()) .and_then(|element_opt| element_opt.as_ref()) .and_then(|element| { element.as_item_bytes().ok().map(|bytes| { use base64::Engine; base64::engine::general_purpose::STANDARD.encode(bytes) }) }); PathElement { - path: vec![key], + path: path.clone(), + key, value, } }) .collect();
📜 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 (3)
packages/wasm-sdk/api-definitions.json
(4 hunks)packages/wasm-sdk/index.html
(5 hunks)packages/wasm-sdk/src/queries/system.rs
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/wasm-sdk/index.html
📄 CodeRabbit Inference Engine (CLAUDE.md)
Test the WASM SDK using the web interface at 'index.html' in 'packages/wasm-sdk'
Files:
packages/wasm-sdk/index.html
packages/wasm-sdk/**/index.html
📄 CodeRabbit Inference Engine (packages/wasm-sdk/CLAUDE.md)
packages/wasm-sdk/**/index.html
: When adding new queries or state transitions, update the definitions in index.html.
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/index.html
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/system.rs
🧠 Learnings (3)
📚 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/api-definitions.json
packages/wasm-sdk/index.html
📚 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/index.html
📚 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/system.rs
🧬 Code Graph Analysis (1)
packages/wasm-sdk/src/queries/system.rs (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GetStatusRequestV0
(5068-5077)GetStatusRequest
(5046-5061)
⏰ 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-and-test-wasm-sdk
🔇 Additional comments (5)
packages/wasm-sdk/api-definitions.json (1)
851-852
: Proof support flags are correctly annotated and align with the UI gating and Rust bindings
- Marking supportsProof: false for:
- epoch.getEvonodesProposedEpochBlocksByIds / ByRange
- system.getStatus
- system.getCurrentQuorumsInfo
- system.waitForStateTransitionResult
matches the absence of corresponding with_proof_info bindings and the UI logic that hides the proof toggle and shows an explanatory note. This keeps UX consistent and avoids implying proofs where they’re unavailable.Also applies to: 871-872, 1126-1127, 1132-1133, 1174-1175
packages/wasm-sdk/index.html (2)
164-167
: Nice UX: clear, non-intrusive note for non-proof endpointsThe addition of noProofInfoContainer is compact and clear; it communicates capability limitations without cluttering the UI.
4133-4134
: Consistent state reset of proof UI across mode/category changesHiding both proofToggleContainer and noProofInfoContainer on mode/category reset prevents stale UI. This is the right call to avoid confusing states when switching operations.
Also applies to: 4153-4154, 4428-4429
packages/wasm-sdk/src/queries/system.rs (2)
11-105
: Well-structured, camelCase JSON schema for getStatus outputDefining StatusResponse and its nested types with serde(rename_all = "camelCase") sets a clean, stable contract for JS consumers. Field choices (stringifying large counters/hashes, Option for potentially absent values) are pragmatic and JS-friendly.
154-170
: Correct gRPC v0 request construction and error mappingBuilding a V0 GetStatusRequest and mapping transport errors into JsError is appropriate for WASM bindings. This keeps the error channel consistent with other SDK functions.
Previously returned SDK's configured version (10) instead of network's actual protocol version (9). Now extracts protocol version from ExtendedEpochInfo to provide accurate, provable network data.
Replace ExtendedEpochInfo-based getStatus with direct gRPC call to platform getStatus endpoint. Returns comprehensive status including version info, node details, chain state, network info, sync progress, and timestamps. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Add visual indicators for queries that don't support cryptographic proof verification, improving user understanding of query capabilities. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
6b3901c
to
c467b7c
Compare
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
🧹 Nitpick comments (3)
packages/wasm-sdk/test/ui-automation/tests/query-execution.spec.js (3)
499-501
: Strengthen getStatus structure check for clearer failures and single parseCurrent assertion relies on Object.keys(JSON.parse(...)) and parses inline. Parse once, assert properties, and add a quick JSON-parse guard for more actionable failure messages.
- expect(Object.keys(JSON.parse(result))).toEqual(expect.arrayContaining([ - 'version', 'node', 'chain', 'network', 'stateSync', 'time' - ])); + expect(() => JSON.parse(result)).not.toThrow(); + const parsed = JSON.parse(result); + ['version', 'node', 'chain', 'network', 'stateSync', 'time'].forEach((k) => { + expect(parsed).toHaveProperty(k); + });
1079-1081
: Align mainnet getStatus assertion with new structured JSONGiven getStatus now returns a full JSON object, mirror the earlier structural check instead of a substring match.
- // Should contain status data with version info - expect(result.result).toContain('version'); + // Should contain structured status data with expected top-level keys + expect(() => JSON.parse(result.result)).not.toThrow(); + const status = JSON.parse(result.result); + ['version', 'node', 'chain', 'network', 'stateSync', 'time'].forEach((k) => { + expect(status).toHaveProperty(k); + });
491-586
: Add UI assertions for the no-proof path of getStatusPR states the UI hides the proof toggle and shows a “no proof” note when proofs aren’t supported. We don’t currently validate that for system.getStatus. Add a targeted UI test to prevent regressions.
}); + // Additional UI assertions for system.getStatus (no proof support) + test('system.getStatus hides proof toggle and shows no-proof note', async () => { + await wasmSdkPage.setupQuery('system', 'getStatus'); + // The proof toggle should not be present for queries that don't support proofs + const proofToggle = wasmSdkPage.page.locator('#proofToggle'); + await expect(proofToggle).toHaveCount(0); + // A "no proof info" note should be visible + const noProofInfo = wasmSdkPage.page.locator('#noProofInfoContainer'); // adjust selector if different + await expect(noProofInfo).toBeVisible(); + });If the selector for the note differs from
#noProofInfoContainer
, please update it to match index.html.
📜 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 (1)
packages/wasm-sdk/test/ui-automation/tests/query-execution.spec.js
(1 hunks)
🧰 Additional context used
🧠 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/**/index.html : When adding new queries or state transitions, update the definitions in index.html.
Applied to files:
packages/wasm-sdk/test/ui-automation/tests/query-execution.spec.js
⏰ 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). (4)
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
- GitHub Check: build-and-test-wasm-sdk
- GitHub Check: build-wasm-sdk
@QuantumExplorer this should be good to go now |
Co-authored-by: Claude <noreply@anthropic.com>
Issue being fixed or feature implemented
The getStatus function was returning a limited amount of information rather than the full getStatus endpoint's available data.
What was done?
How Has This Been Tested?
Checked locally
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Chores / API
UI
Tests