Skip to content

Conversation

shumkov
Copy link
Collaborator

@shumkov shumkov commented Oct 1, 2025

Issue being fixed or feature implemented

Some of the methods are failing befoe wasm is initiated

What was done?

  • Added lazy wasm initialization to all methods which using it

How Has This Been Tested?

With tests

Breaking Changes

None

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • Refactor

    • Transitioned DPNS, Tokens, and Wallet APIs to asynchronous interfaces. Affected methods now return Promises and should be awaited, improving initialization reliability and consistency.
  • New Features

    • Exposed DataContract type to SDK consumers.
  • Tests

    • Updated unit and functional tests to use async/await with the new asynchronous APIs.

@shumkov shumkov self-assigned this Oct 1, 2025
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

Several SDK methods were converted from synchronous to asynchronous, adding await wasm.ensureInitialized() before invoking wasm.WasmSdk. A type export (DataContract) was added. Corresponding unit and functional tests were updated to await the new async interfaces.

Changes

Cohort / File(s) Summary
DPNS facade async conversion
packages/js-evo-sdk/src/dpns/facade.ts
Made convertToHomographSafe, isValidUsername, and isContestedUsername async; each awaits wasm.ensureInitialized() before delegating to wasm.WasmSdk. Updated return types to Promise.
Tokens facade async conversion
packages/js-evo-sdk/src/tokens/facade.ts
Made calculateId async; added await wasm.ensureInitialized(); return type changed to Promise.
Wallet functions async conversion
packages/js-evo-sdk/src/wallet/functions.ts
Converted all exported wallet helpers to async; each awaits wasm.ensureInitialized(); preserved parameters (with nullish handling) and changed return types to Promise.
WASM re-exports
packages/js-evo-sdk/src/wasm.ts
Added type export: DataContract from @dashevo/wasm-sdk/compressed.
Functional tests updated to async
packages/js-evo-sdk/tests/functional/tokens.spec.mjs, packages/js-evo-sdk/tests/functional/wallet.spec.mjs
Updated tests to await async SDK methods (tokens.calculateId, wallet mnemonic/seed/keys flows).
Unit tests updated to async
packages/js-evo-sdk/tests/unit/facades/dpns.spec.mjs, packages/js-evo-sdk/tests/unit/facades/tokens.spec.mjs
Switched to async tests invoking client facades; expectations updated to await results.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as App Code
  participant Facade as SDK Facade
  participant Wasm as wasm.ensureInitialized()
  participant Core as wasm.WasmSdk

  App->>Facade: call method(...)
  activate Facade
  Facade->>Wasm: ensureInitialized()
  activate Wasm
  Wasm-->>Facade: resolved
  deactivate Wasm
  Facade->>Core: underlyingOperation(...)
  activate Core
  Core-->>Facade: result
  deactivate Core
  Facade-->>App: Promise<result>
  deactivate Facade

  note over Facade,Core: Methods now return Promises and guard with ensureInitialized()
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

js-sdk

Suggested reviewers

  • QuantumExplorer

Poem

A hop, a wait, a promise kept,
I twitch my ears—init prepped.
From seeds to keys, IDs in line,
Async burrows now align.
With every await, I softly thrum—
The wasm woods are warm—come! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and clearly communicates that the pull request addresses missing WebAssembly initialization in SDK methods, directly reflecting the primary change of adding lazy wasm initialization to affected functions, and it follows conventional commit formatting for clarity.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/sdk/wasm-not-initialized

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9d09e7 and 891e99e.

📒 Files selected for processing (8)
  • packages/js-evo-sdk/src/dpns/facade.ts (1 hunks)
  • packages/js-evo-sdk/src/tokens/facade.ts (1 hunks)
  • packages/js-evo-sdk/src/wallet/functions.ts (2 hunks)
  • packages/js-evo-sdk/src/wasm.ts (1 hunks)
  • packages/js-evo-sdk/tests/functional/tokens.spec.mjs (1 hunks)
  • packages/js-evo-sdk/tests/functional/wallet.spec.mjs (1 hunks)
  • packages/js-evo-sdk/tests/unit/facades/dpns.spec.mjs (1 hunks)
  • packages/js-evo-sdk/tests/unit/facades/tokens.spec.mjs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/**/tests/**

📄 CodeRabbit inference engine (AGENTS.md)

Place unit and integration tests alongside each package in packages//tests

Files:

  • packages/js-evo-sdk/tests/unit/facades/tokens.spec.mjs
  • packages/js-evo-sdk/tests/functional/tokens.spec.mjs
  • packages/js-evo-sdk/tests/unit/facades/dpns.spec.mjs
  • packages/js-evo-sdk/tests/functional/wallet.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/tokens/facade.ts
  • packages/js-evo-sdk/src/dpns/facade.ts
  • packages/js-evo-sdk/src/wasm.ts
  • packages/js-evo-sdk/src/wallet/functions.ts
🧠 Learnings (1)
📚 Learning: 2025-09-24T05:16:54.422Z
Learnt from: shumkov
PR: dashpay/platform#2784
File: packages/js-evo-sdk/src/wallet/functions.ts:24-26
Timestamp: 2025-09-24T05:16:54.422Z
Learning: The WASM SDK methods like `deriveKeyFromSeedWithExtendedPath` and `deriveKeyFromSeedWithPath` already handle mnemonic-to-seed conversion internally, so the JS wrapper functions can safely pass mnemonic strings directly without needing to convert them to seeds first.

Applied to files:

  • packages/js-evo-sdk/src/wallet/functions.ts
🧬 Code graph analysis (4)
packages/js-evo-sdk/src/tokens/facade.ts (1)
packages/js-evo-sdk/src/sdk.ts (1)
  • wasm (65-68)
packages/js-evo-sdk/tests/unit/facades/dpns.spec.mjs (1)
packages/js-evo-sdk/tests/unit/facades/tokens.spec.mjs (1)
  • client (6-6)
packages/js-evo-sdk/src/dpns/facade.ts (1)
packages/js-evo-sdk/src/sdk.ts (1)
  • wasm (65-68)
packages/js-evo-sdk/src/wallet/functions.ts (2)
packages/js-evo-sdk/src/sdk.ts (1)
  • wasm (65-68)
packages/js-evo-sdk/tests/functional/wallet.spec.mjs (2)
  • mnemonic (5-5)
  • mnemonic (11-11)
⏰ 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 Docker images (Drive, drive, drive-abci) / Build Drive image
  • GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
  • GitHub Check: Build JS packages / Build JS
  • GitHub Check: Rust crates security audit
🔇 Additional comments (14)
packages/js-evo-sdk/src/wasm.ts (1)

16-16: LGTM: Explicit type export improves type clarity.

The explicit export type for DataContract is good practice even though line 14's wildcard export already includes it. This makes the type export explicit for better documentation and IDE support.

packages/js-evo-sdk/tests/unit/facades/tokens.spec.mjs (1)

48-51: LGTM: Test correctly updated to async pattern.

The test properly reflects the async signature change of calculateId. The await is correctly placed and the assertion remains valid.

packages/js-evo-sdk/tests/unit/facades/dpns.spec.mjs (1)

25-32: LGTM: Tests correctly updated for async DPNS methods.

All three DPNS utility methods (convertToHomographSafe, isValidUsername, isContestedUsername) are properly awaited, and the type assertions correctly verify boolean return types for the validation methods.

packages/js-evo-sdk/tests/functional/tokens.spec.mjs (1)

13-16: LGTM: Functional test correctly updated.

The functional test properly uses await for the async calculateId method and validates the expected token ID.

packages/js-evo-sdk/src/tokens/facade.ts (1)

12-15: LGTM: Proper async initialization added.

The method correctly initializes wasm before calling the static calculateTokenIdFromContract. The async conversion properly addresses the issue where wasm was not initialized before use.

packages/js-evo-sdk/tests/functional/wallet.spec.mjs (3)

4-8: LGTM! Test correctly updated for async wallet functions.

The test properly awaits both generateMnemonic and validateMnemonic calls, with appropriate assertions for the async results.


10-17: LGTM! Comprehensive async test for seed and derivation functions.

All wallet function calls are properly awaited, and the test flow correctly validates the asynchronous seed generation and key derivation operations.


19-24: LGTM! Key utility tests correctly handle async operations.

Both generateKeyPair and generateKeyPairs are properly awaited, with appropriate type assertions for the returned values.

packages/js-evo-sdk/src/wallet/functions.ts (5)

4-17: LGTM! Mnemonic functions correctly converted to async.

All three mnemonic-related functions (generateMnemonic, validateMnemonic, mnemonicToSeed) properly await wasm initialization and return appropriate Promise types.


19-45: LGTM! Key derivation functions correctly converted to async.

All key derivation functions properly await wasm initialization before delegating to WASM SDK methods. The functions correctly pass mnemonic strings directly, as WASM SDK handles internal conversion (based on learnings).


47-75: LGTM! Derivation path functions correctly converted to async.

All six derivation path functions (BIP44, DIP9, DIP13 for mainnet and testnet) properly await wasm initialization and correctly delegate to WASM SDK methods.


77-90: LGTM! Key manipulation functions correctly converted to async.

Functions for deriving child public keys, converting extended keys, and generating key pairs properly await wasm initialization with appropriate Promise return types.


92-120: LGTM! Remaining wallet utilities correctly converted to async.

All remaining wallet functions (key pair generation/conversion, address operations, message signing) properly await wasm initialization and return appropriate Promise types.

packages/js-evo-sdk/src/dpns/facade.ts (1)

11-14: Verify initialization pattern consistency in DPNS and tokens facades.

In packages/js-evo-sdk/src/dpns/facade.ts (and similarly in tokens/facade.ts), methods like convertToHomographSafe use

await wasm.ensureInitialized();
return wasm.WasmSdk.dpnsConvertToHomographSafe(input);

while the rest of the façade methods call

const w = await this.sdk.getWasmSdkConnected();

Confirm whether those standalone direct WASM calls are intentional; if not, unify all façade methods to use this.sdk.getWasmSdkConnected() for consistent initialization.


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.

@shumkov shumkov added this to the v2.1.0 milestone Oct 1, 2025
@shumkov shumkov merged commit 32441bf into v2.1-dev Oct 3, 2025
18 checks passed
@shumkov shumkov deleted the fix/sdk/wasm-not-initialized branch October 3, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant