Skip to content

fix(dpp): add toJSON() serialization to TokenContractInfoWasm#3089

Open
thepastaclaw wants to merge 5 commits intodashpay:v3.1-devfrom
thepastaclaw:fix-token-contract-info-tojson
Open

fix(dpp): add toJSON() serialization to TokenContractInfoWasm#3089
thepastaclaw wants to merge 5 commits intodashpay:v3.1-devfrom
thepastaclaw:fix-token-contract-info-tojson

Conversation

@thepastaclaw
Copy link
Contributor

@thepastaclaw thepastaclaw commented Feb 16, 2026

Summary

TokenContractInfoWasm was missing toJSON() support, causing it to serialize as an empty object {} when called from JavaScript.

Changes

Added a manual toJSON() method to TokenContractInfoWasm that returns:

  • contractId — base58-encoded string
  • tokenContractPosition — numeric position

Why manual instead of serde?

The inner TokenContractInfo type from rs-dpp doesn't derive serde Serialize/Deserialize, so we can't use the serialization::to_json() helper or the impl_wasm_conversions! macro. Instead, we build the JSON object manually using the existing getters, similar to the pattern used elsewhere in the codebase.

Fixes #3027

Validation

Build verification

  • cargo check -p dpp — confirms the conditional serde::Serialize/serde::Deserialize derives on TokenContractInfo and TokenContractInfoV0 compile correctly under the fixtures-and-mocks and state-transition-serde-conversion features
  • cargo check -p wasm-dpp2 — confirms the new to_json() method on TokenContractInfoWasm compiles and the serialization::to_json integration is wired up properly

Test verification

  • cargo test -p dpp — ensures existing rs-dpp tests still pass with the new serde derives (the cfg_attr gating means production builds without the feature flags are unaffected)
  • cargo test -p wasm-dpp2 — validates the wasm-dpp2 package tests pass with the new toJSON() method

CI

PR CI runs the full platform test matrix including cargo check, cargo test, clippy, and formatting across all packages — no regressions expected since the serde derives are feature-gated and the WASM method follows the existing serialization::to_json pattern used by other types.

Known pre-existing CI failure: JS packages (@dashevo/dapi) / Tests

The JS packages (@dashevo/dapi) / Tests check fails on this PR with a NaN v2 / Node.js 24 incompatibility in ssh2@npm:1.11.0:

ssh2@npm:1.11.0 STDERR error: no matching function for call to
  'Nan::FunctionCallbackInfo<v8::Value>::FunctionCallbackInfo(
    const v8::FunctionCallbackInfo<v8::Value>&, v8::Local<v8::Data>)'

This failure is pre-existing and unrelated to this PR. This PR only modifies wasm-dpp2. The dapi test job is triggered whenever wasm-dpp is changed (CI matrix dependency), but the failure originates from a NaN v2 native addon build error in @dashevo/dapi's transitive ssh2 dependency against Node 24. PRs that don't touch wasm-dpp show this check as skipping rather than failing.

Summary by CodeRabbit

  • New Features
    • Added TypeScript/wasm interop types so token contract info can be passed as plain objects or JSON strings in JS tooling.
    • Exposed JSON-friendly export for token contract info to simplify retrieval of contract ID and token position in JS.
    • Added optional serde (de)serialization and camelCase renaming behind feature flags for improved cross-language compatibility.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 16, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f12983f and 2fcbef0.

📒 Files selected for processing (1)
  • packages/wasm-dpp2/src/tokens/contract_info.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/wasm-dpp2/src/tokens/contract_info.rs

📝 Walkthrough

Walkthrough

Adds serde-based JSON serialization and TypeScript interop for TokenContractInfo in the wasm bindings, and conditionally enables serde derives (with camelCase rename for v0) for TokenContractInfo types in rs-dpp behind feature flags.

Changes

Cohort / File(s) Summary
WASM bindings — TokenContractInfo
packages/wasm-dpp2/src/tokens/contract_info.rs
Adds TypeScript plain-object and JSON extern types (TokenContractInfoObjectJs, TokenContractInfoJSONJs), derives Serialize/Deserialize on TokenContractInfoWasm (serde(transparent)), and wires conversion/serialization via impl_wasm_conversions! so toJSON() returns proper JSON.
Core enum serde derive
packages/rs-dpp/src/tokens/contract_info/mod.rs
Applies conditional cfg_attr to derive serde::Serialize/Deserialize and serde(untagged) for TokenContractInfo when fixtures-and-mocks or state-transition-serde-conversion features are enabled.
V0 struct serde derive & rename
packages/rs-dpp/src/tokens/contract_info/v0/mod.rs
Applies conditional cfg_attr to derive serde::Serialize/Deserialize and serde(rename_all = "camelCase") for TokenContractInfoV0 under the same feature flags.

Sequence Diagram(s)

sequenceDiagram
    participant JS as Client (JS)
    participant WB as wasm-bindgen glue
    participant R as Rust TokenContractInfoWasm
    participant S as serde::to_string / conversions

    JS->>WB: call obj.toJSON()
    WB->>R: invoke to_json() (wasm wrapper)
    R->>S: serialize inner TokenContractInfo -> TokenContractInfoJSONJs
    S-->>R: TokenContractInfoJSONJs
    R-->>WB: WasmDppResult<TokenContractInfoJSONJs>
    WB-->>JS: return JSON object
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nibbled bytes and stitched a key,
ContractId now shines for you to see,
Position hops in camelCase attire,
No more empty objects — JSON's on fire,
A tiny hop, and bridges wire.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: adding toJSON() serialization support to TokenContractInfoWasm, which directly addresses the core issue.
Linked Issues check ✅ Passed The PR fulfills all coding requirements from #3027: adds serde derives to TokenContractInfo and TokenContractInfoV0, implements conversion macros with proper camelCase serialization, and provides toJSON() support via impl_wasm_conversions!.
Out of Scope Changes check ✅ Passed All changes are within scope: TypeScript type definitions and serde derives enable the toJSON() serialization fix; no unrelated modifications to other components.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@thepastaclaw thepastaclaw changed the title fix(wasm-dpp2): add toJSON() serialization to TokenContractInfoWasm fix(dpp): add toJSON() serialization to TokenContractInfoWasm Feb 16, 2026
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.

🤖 Fix all issues with AI agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.

In `@packages/wasm-dpp2/src/tokens/contract_info.rs`:
- Line 53: Replace the direct cast to f64 in the JsValue conversion by using the
From conversion: instead of using "self.token_contract_position() as f64" (where
token_contract_position() returns TokenContractPosition/u16), call
f64::from(self.token_contract_position()) so clippy's cast_lossless lint is
satisfied; update the expression that constructs JsValue (the line with
&JsValue::from_f64(...)) accordingly.
🧹 Nitpick comments (1)
🤖 Fix all nitpicks with AI agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.

In `@packages/wasm-dpp2/src/tokens/contract_info.rs`:
- Line 53: Replace the direct cast to f64 in the JsValue conversion by using the
From conversion: instead of using "self.token_contract_position() as f64" (where
token_contract_position() returns TokenContractPosition/u16), call
f64::from(self.token_contract_position()) so clippy's cast_lossless lint is
satisfied; update the expression that constructs JsValue (the line with
&JsValue::from_f64(...)) accordingly.
packages/wasm-dpp2/src/tokens/contract_info.rs (1)

53-53: Replace as f64 with f64::from() to comply with clippy's cast_lossless lint.

Since token_contract_position() returns TokenContractPosition (which is u16), the cast to f64 is a lossless widening operation. Clippy's cast_lossless lint prefers explicit From conversions for such casts.

Suggested fix
-            &JsValue::from_f64(self.token_contract_position() as f64),
+            &JsValue::from_f64(f64::from(self.token_contract_position())),
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@packages/wasm-dpp2/src/tokens/contract_info.rs` at line 53, Replace the
direct cast to f64 in the JsValue conversion by using the From conversion:
instead of using "self.token_contract_position() as f64" (where
token_contract_position() returns TokenContractPosition/u16), call
f64::from(self.token_contract_position()) so clippy's cast_lossless lint is
satisfied; update the expression that constructs JsValue (the line with
&JsValue::from_f64(...)) accordingly.

@thepastaclaw
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 16, 2026

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

@PastaPastaPasta
Copy link
Member

Can you confirm we shouldn't add this lower in the stack? we generally want to make changes like in the rs-sdk if possible, and then just expose them in the higher levels.

If this is relevant here, please make the change

@thepastaclaw
Copy link
Contributor Author

Good point. TokenContractInfo in rs-dpp doesn't derive Serialize/Deserialize, so the WASM layer couldn't use the standard serialization::to_json() helper. That's why I went with a manual toJSON() method.

The cleaner approach would be to add serde derives to TokenContractInfo in rs-dpp, then the WASM layer could use the standard impl_wasm_conversions! macro like other types. I can make that change — it would be a small addition to rs-dpp (Serialize/Deserialize derives on the struct) and then simplify the WASM code to use the standard pattern.

Want me to update this PR to do it that way?

@thepastaclaw
Copy link
Contributor Author

Good question. I looked into this — TokenContractInfoV0 in rs-dpp only derives Encode/Decode (bincode), no Serialize/serde. The toJSON() method is specifically a JavaScript concern — it constructs a js_sys::Object with Reflect::set to expose contractId (base58) and tokenContractPosition to JS consumers.

This is the correct layer (wasm-dpp2) since it's the JS binding surface. The Rust SDK doesn't need JSON serialization here — it accesses the fields directly via the accessor trait.

That said, if you'd prefer adding #[derive(Serialize)] + serde to the Rust struct and then using serde_wasm_bindgen in the wasm layer instead of manual Reflect::set, I can refactor it that way. But the manual approach matches the existing pattern in other wasm-dpp2 types.

@thepastaclaw
Copy link
Contributor Author

Good question — I looked into this and the fix is correctly scoped to the wasm layer. The underlying Rust type (TokenContractInfo) already has proper serialization via serde derives in rs-sdk/dpp. The issue was specifically that the wasm_bindgen wrapper (TokenContractInfoWasm) didn't implement toJSON(), which is a JavaScript-specific concern — it controls how JSON.stringify() handles the object. Without it, JS sees an opaque wasm object and serializes it as {}.

The rs-sdk layer doesn't (and shouldn't) know about JavaScript's toJSON() protocol. Other wasm wrapper types in wasm-dpp2 follow this same pattern — they add toJSON() in the wasm layer to bridge Rust serialization to JS.

So no changes needed lower in the stack here.

@thepastaclaw
Copy link
Contributor Author

Applied the cast_lossless fix — switched to f64::from(self.token_contract_position()) in cfecaaf. Thanks for the catch!

Copy link
Collaborator

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

You should look on how we inmplement conversion methods for other dpp2 entities and do the same way.d

@thepastaclaw
Copy link
Contributor Author

Thanks @shumkov — will look at the existing dpp2 entity conversion patterns and align TokenContractInfoWasm with the same approach. Will push an updated implementation.

@thepastaclaw
Copy link
Contributor Author

Refactored per review feedback (commit b7c2b0c):

Changed from manual js_sys::Reflect::set approach to the standard dpp2 conversion pattern:

#[wasm_bindgen(js_name = "toJSON")]
pub fn to_json(&self) -> WasmDppResult<TokenContractInfoJSONJs> {
    let js_value = serialization::to_json(&self.0)?;
    Ok(js_value.into())
}

This required adding feature-gated serde derives to the underlying types:

  • TokenContractInfoV0: Serialize/Deserialize + #[serde(rename_all = "camelCase")]
  • TokenContractInfo enum: Serialize/Deserialize
  • Both gated behind fixtures-and-mocks or state-transition-serde-conversion features (matching how other dpp types handle this)

Also added proper TypeScript type definition (TokenContractInfoJSON interface) following the pattern from other wasm-dpp2 entities.

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/rs-dpp/src/tokens/contract_info/mod.rs`:
- Around line 27-36: The TokenContractInfo enum's serde attributes lack
untagged, causing JSON to be wrapped with a "V0" key; update the cfg_attr on
TokenContractInfo (the same attribute block that currently derives
serde::Serialize and serde::Deserialize) to include serde(untagged) so that
TokenContractInfo serializes to the flat shape expected by
TokenContractInfoJSON; follow the same pattern used in state_transition/mod.rs
when adding serde(untagged).

@thepastaclaw
Copy link
Contributor Author

Good catch on the missing serde(untagged) — fixed in 03208e7. Without it, serialization::to_json() would produce {"V0": {"contractId": ..., "tokenContractPosition": 0}} instead of the flat structure the TS interface expects. Now matches the pattern used by other versioned enums in rs-dpp.

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK

@thepastaclaw
Copy link
Contributor Author

CI failures here are all base branch (v3.1-dev) issues — security audits, clippy on base changes, etc. The base branch itself has been consistently failing CI over the last several runs. My changes (serde untagged annotation on TokenContractInfo) are unrelated.

Ready to merge once base CI stabilizes.

@thepastaclaw
Copy link
Contributor Author

Fix: Swift SDK build — missing SPV symbols

Root cause: rust-dashcore is a Cargo workspace. When cargo build runs from the dash-spv-ffi/ subdirectory, output goes to the workspace root rust-dashcore/target/, not dash-spv-ffi/target/. The build script (packages/rs-sdk-ffi/build_ios.sh) was looking for libdash_spv_ffi.a at the crate-level target dir ($SPV_CRATE_PATH/target/), which doesn't exist.

This meant the SPV library was being built successfully but never found during the merge step. The XCFramework was created with only librs_sdk_ffi.a (no SPV symbols), causing the symbol verification to fail.

Fix: Changed all 6 $SPV_CRATE_PATH/target/ references to $RUST_DASHCORE_PATH/target/ (the workspace root target directory).

This is a platform-wide issue affecting all PRs, not just this one.

Commit: 4240f62

@thepastaclaw thepastaclaw force-pushed the fix-token-contract-info-tojson branch from 4240f62 to 1b23e6b Compare February 20, 2026 23:03
@thepastaclaw
Copy link
Contributor Author

Dropped the swift-sdk build_ios.sh commit — that's a pre-existing issue already fixed by #3107 (lklimek). Shouldn't have been bundled here.

thepastaclaw and others added 3 commits February 21, 2026 12:29
TokenContractInfoWasm was missing toJSON() support, causing it to
serialize as an empty object {}. Add a manual toJSON() method that
returns contractId (as base58 string) and tokenContractPosition.

Fixes dashpay#3027
…nContractInfo.toJSON()

Replace manual js_sys::Reflect::set approach with serialization::to_json(),
matching the pattern used by Identity, DataContract, and other dpp2 types.
Add serde Serialize/Deserialize derives to TokenContractInfo and
TokenContractInfoV0 behind feature flags.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…alization

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…kenContractInfo

Use impl_wasm_conversions! macro with #[serde(transparent)] wrapper
following the same pattern as ExtendedEpochInfoWasm and other dpp2
entities. This provides toObject/fromObject/toJSON/fromJSON via the
standard serialization infrastructure instead of a manual toJSON() only.

Addresses review feedback from shumkov.
@thepastaclaw
Copy link
Contributor Author

Refactored to follow the standard dpp2 conversion conventions (modeled after ExtendedEpochInfoWasm):

  • Replaced manual to_json() with impl_wasm_conversions! macro — now provides all 4 standard methods: toObject, fromObject, toJSON, fromJSON
  • Added #[derive(serde::Serialize, serde::Deserialize)] + #[serde(transparent)] to the wrapper
  • Added TokenContractInfoObject TypeScript interface + extern type
  • Removed unused imports

Verification: cargo check/clippy -p wasm-dpp2 ✅, cargo test -p dpp ✅ (481 tests pass).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for final review Ready for the final review. If AI was involved in producing this PR, it has already had a reviewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TokenContractInfoWasm missing toJSON() serialization - returns empty object

3 participants