fix(sdk): toJSON returns empty object#2995
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds JS→JSON normalization utilities and tests (Map/toJSON/BigInt/Uint8Array handling), implements Serialize for PlatformAddressWasm with bech32m/hex parsing fallback, updates many wasm serde conversion macro usages to pair wrapper types with internal Rust types, and extends NetworkLike to accept numeric literals. Changes
Sequence Diagram(s)sequenceDiagram
participant JS as JavaScript
participant WASM as WASM Module
participant Norm as Normalizer
JS->>WASM: testJsValueToJson(jsValue)
WASM->>Norm: js_value_to_json(jsValue)
Norm->>Norm: detect type (Map, BigInt, Uint8Array, Array, Object, primitive)
alt has toJSON()
Norm->>WASM: try_call_to_json(obj)
Norm->>Norm: normalize returned value recursively
else Map
Norm->>Norm: map_key_to_string() for each key
Norm->>Norm: normalize_map_for_json() values recursively
else BigInt
Norm->>Norm: convert to string
else Uint8Array
Norm->>Norm: convert to Array of numbers
else Array/Object
Norm->>Norm: recursively normalize elements/properties
end
Norm-->>WASM: normalized JS-friendly structure
WASM-->>JS: return normalized JsValue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/wasm-dpp2/src/core/network.rs`:
- Around line 45-57: The code that handles numeric enum values (using
value.as_f64() and casting to u32) silently truncates fractional inputs (e.g.,
1.9 becomes 1); update the logic in the numeric branch that returns
NetworkWasm::Mainnet/Testnet/Devnet/Regtest so that it first verifies the f64 is
an integer and non-negative (e.g., check num.fract() == 0.0 and num >= 0.0 and
within u32 range) before casting to u32, and if the value is not an integer
return a WasmDppError::invalid_argument with a message rejecting non-integer
network values; keep the existing match on the validated integer to return the
appropriate NetworkWasm::* variant.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/wasm-dpp2/src/core/network.rspackages/wasm-dpp2/src/platform_address/address.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Rust code must passcargo clippy --workspacelinter checks
Rust code must be formatted usingcargo fmt --all
**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/wasm-dpp2/src/platform_address/address.rspackages/wasm-dpp2/src/core/network.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use WASM bindings to connect Rust and JavaScript code
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use WebAssembly as a bridge between Rust and JavaScript implementations
📚 Learning: 2025-11-25T13:10:23.481Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use WASM bindings to connect Rust and JavaScript code
Applied to files:
packages/wasm-dpp2/src/platform_address/address.rspackages/wasm-dpp2/src/core/network.rs
📚 Learning: 2025-11-25T13:10:23.481Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use WebAssembly as a bridge between Rust and JavaScript implementations
Applied to files:
packages/wasm-dpp2/src/platform_address/address.rs
🧬 Code graph analysis (1)
packages/wasm-dpp2/src/core/network.rs (1)
packages/wasm-sdk/src/error.rs (1)
invalid_argument(75-77)
🔇 Additional comments (3)
packages/wasm-dpp2/src/core/network.rs (2)
6-16: LGTM! TypeScript type extension is well-documented.The
NetworkLiketype alias correctly extends to include numeric literals 0-3, aligning with theNetworkWasmenum discriminants. Documentation clearly describes acceptable input formats.
59-61: LGTM! Error message correctly reflects the expanded input options.The updated error message accurately describes both string and numeric input options available to the caller.
packages/wasm-dpp2/src/platform_address/address.rs (1)
8-9: LGTM!The imports are correctly added to support the new
Serializeimplementation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Issue being fixed or feature implemented
Multiple issues with
toJSONmethods in Evo SDKWhat was done?
How Has This Been Tested?
Implemented tests for JS value to JSON value conversion
Breaking Changes
Identity queries now return
IdentityPublicKeyinstead ofIdentityKeyInfoChecklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Tests
New Features
Refactor
API
✏️ Tip: You can customize this high-level summary in your review settings.