feat(sdk): expose data contract from json #2791
Conversation
WalkthroughReplaces explicit wasm re-exports with a wildcard export in js-evo-sdk, exposes DataContractWasm as Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as JS App
participant SDK as js-evo-sdk (export *)
participant WASM as wasm DataContract (DataContract)
participant PV as PlatformVersion Resolver
participant DC as Core DataContract
App->>SDK: import { DataContract } (via export * from './wasm.js')
App->>WASM: DataContract.from_json(json, platform_version)
WASM->>PV: resolve(platform_version)
alt valid version
PV-->>WASM: PlatformVersion
WASM->>DC: parse JSON & construct DataContract
DC-->>WASM: DataContract internal
WASM-->>App: DataContract instance (WASM wrapper)
App->>WASM: instance.toJSON()
WASM-->>App: contract JSON
else invalid version or parse error
WASM-->>App: WasmSdkError (invalid version / parse failure)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (16)
📒 Files selected for processing (3)
⏰ 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). (7)
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/wasm-sdk/src/dpp.rs (1)
312-328: Well-structured constructor with proper error handling.The
from_jsonmethod correctly:
- Resolves platform version with error mapping
- Deserializes JSON to the internal format
- Creates a DataContract instance
- Wraps errors appropriately
Minor note: The
json.clone()on line 320 may be unnecessary sinceserde_wasm_bindgen::from_valuelikely doesn't need ownership. This is a minor optimization opportunity but not incorrect.Consider removing the clone if not required:
- let data_contract = DataContract::from_json(serde_wasm_bindgen::from_value(json.clone()).map_err(|e| { + let data_contract = DataContract::from_json(serde_wasm_bindgen::from_value(json).map_err(|e| {packages/wasm-sdk/tests/unit/data-contract.spec.mjs (2)
11-22: Expand test coverage for the new API.The test validates basic functionality but could be more comprehensive. Consider adding tests for:
- Invalid JSON input (malformed, missing required fields)
- Invalid platform version
- Error handling and error messages
- Memory leak verification with multiple allocations/frees
- Edge cases in the fixture data
14-15: Use more descriptive assertion messages.The test uses
expect(contract).to.be.ok()without a failure message. Consider adding descriptive messages to aid debugging when tests fail.- expect(contract).to.be.ok(); - expect(contract.id()).to.equal(contractFixture.id); + expect(contract, 'DataContract should be created from JSON').to.be.ok(); + expect(contract.id(), 'Contract ID should match fixture').to.equal(contractFixture.id);packages/wasm-sdk/tests/unit/fixtures/data-contract-crypto-card-game.mjs (1)
1-96: Consider extracting fixture to a JSON file.The fixture is defined as a JavaScript object. For better maintainability and reusability, consider storing it as a JSON file that can be imported. This would also make it easier to validate against schemas and use in other contexts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/js-evo-sdk/src/sdk.ts(1 hunks)packages/wasm-sdk/src/dpp.rs(2 hunks)packages/wasm-sdk/tests/unit/data-contract.spec.mjs(1 hunks)packages/wasm-sdk/tests/unit/fixtures/data-contract-crypto-card-game.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/wasm-sdk/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK
Files:
packages/wasm-sdk/tests/unit/fixtures/data-contract-crypto-card-game.mjspackages/wasm-sdk/src/dpp.rspackages/wasm-sdk/tests/unit/data-contract.spec.mjs
packages/**/tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place unit and integration tests alongside each package in packages//tests
Files:
packages/wasm-sdk/tests/unit/fixtures/data-contract-crypto-card-game.mjspackages/wasm-sdk/tests/unit/data-contract.spec.mjs
packages/**/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/**/**/*.{js,ts,jsx,tsx}: Adhere to ESLint with Airbnb/TypeScript configs for JS/TS code
Use camelCase for JS/TS variables and functions
Use PascalCase for JS/TS classes
Prefer kebab-case filenames within JS packages
Files:
packages/js-evo-sdk/src/sdk.ts
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Format Rust code with cargo fmt
Run Clippy linter for Rust code
Files:
packages/wasm-sdk/src/dpp.rs
🧬 Code graph analysis (3)
packages/wasm-sdk/tests/unit/fixtures/data-contract-crypto-card-game.mjs (1)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs (1)
contract(12-12)
packages/wasm-sdk/src/dpp.rs (1)
packages/wasm-sdk/src/error.rs (2)
invalid_argument(73-75)serialization(77-79)
packages/wasm-sdk/tests/unit/data-contract.spec.mjs (1)
packages/wasm-sdk/tests/unit/fixtures/data-contract-crypto-card-game.mjs (1)
contract(1-96)
⏰ 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). (9)
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (wasm-sdk) / Formatting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (7)
packages/wasm-sdk/src/dpp.rs (3)
297-298: LGTM! Clean binding structure.The
js_nameattribute correctly exposesDataContractWasmasDataContractto JavaScript, and the tuple struct wrapper is a clean pattern for WASM bindings.
306-311: LGTM! Correct JS class binding.The
js_classattribute properly associates the implementation block with the JavaScriptDataContractclass, and theid()method correctly encodes the identifier as Base58.
330-343: toJSON serialization is correct
The use ofPlatformVersion::first()intoJSONacross both wasm-dpp and wasm-sdk is intentional—serializations always emit the current first protocol version, whilefrom_jsonaccepts a version to support legacy formats. No changes required.packages/js-evo-sdk/src/sdk.ts (1)
148-148: LGTM! Clean export addition.The DataContract export follows the established pattern and properly exposes the new functionality to SDK consumers.
packages/wasm-sdk/tests/unit/fixtures/data-contract-crypto-card-game.mjs (2)
74-93: Ignore incorrect index property fix suggestionThe existing index definitions (
{ name: 'asc' }and{ rarity: 'asc' }) already map property names to sort order correctly—quoting the keys is optional and doesn’t change behavior.Likely an incorrect or invalid review comment.
64-72: Verify$createdAtand$updatedAtinrequired
Therequiredlist includes$createdAtand$updatedAt, but neither is defined underproperties. Confirm per the Data Contract specification that system‐generated fields may be listed here, or remove them if they’re not intended.packages/wasm-sdk/tests/unit/data-contract.spec.mjs (1)
4-4: Avoid hardcoded PLATFORM_VERSION in test
Replace the literal1with the SDK’s version helper (e.g. callgetLatestVersionNumber()or expose agetFirstVersion()method) and verify it matchesPlatformVersion::first().protocol_version.
There was a problem hiding this comment.
What do you think about using a contract that includes tokens and groups also in the tests? Like https://testnet.platform-explorer.com/dataContract/Afk9QSj9UDE14K1y9y3iSx6kUSm5LLmhbdAvPvWL4P2i (schema below). Really tests should include an example of all contract version so any issues are caught, right?
v1 contract with documents, tokens, and groups
There was a problem hiding this comment.
Sounds good. Let's do it in a separate PR though.
Issue being fixed or feature implemented
We need to create DataContract from JSON definition
What was done?
DataContract.fromJSONmethod and exportDataContractHow Has This Been Tested?
Added unit test
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Tests
Chores