-
Notifications
You must be signed in to change notification settings - Fork 44
perf(rs-sdk): optimize wasm-sdk bundle size #2783
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
WalkthroughSwitches consumers to ES module artifacts and a compressed wasm SDK: js-evo-sdk now points its main/exports to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as Application
participant Evo as js-evo-sdk (evo-sdk.module.js)
participant WasmPkg as @dashevo/wasm-sdk/compressed
participant Loader as Compressed Loader
participant Env as Node:zlib / Browser DecompressionStream
participant Wasm as WebAssembly
App->>Evo: import APIs / ensureInitialized()
Evo->>WasmPkg: re-export compressed APIs
App->>WasmPkg: init(moduleOrPath)
WasmPkg->>Loader: load base64 gzip payload
Loader->>Env: decode + decompress
Env-->>Loader: wasm bytes
Loader->>Wasm: compile/instantiate
Wasm-->>WasmPkg: instance ready
WasmPkg-->>App: exports available
note right of Env: fallback paths for Node / Browser / Worker
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
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/scripts/bundle.cjs (3)
61-61
: Consider extracting Worker source code as a constant for better maintainability.The Worker source code in the compressed wrapper is a long inline string that's difficult to read and maintain. Consider extracting it as a separate constant or template literal for improved readability.
Example approach:
+const DECOMPRESSION_WORKER_SRC = ` +self.onmessage = async (event) => { + try { + const data = event.data || {}; + let bytes = data.compressed; + const compression = data.compression || null; + // ... rest of worker logic + } catch(err) { + self.postMessage({ok:0, err:String(err)}) + } +}`; - const src = "self.onmessage=async(event)=>{try{const data=event.data||{};let bytes=data.compressed;const compression=data.compression||null;if(!(bytes instanceof Uint8Array)){bytes=bytes?new Uint8Array(bytes):new Uint8Array();}if(compression){if(typeof Blob==='function'&&typeof Response==='function'&&typeof DecompressionStream==='function'){const res=new Response(new Blob([bytes]).stream().pipeThrough(new DecompressionStream(compression)));const buf=await res.arrayBuffer();bytes=new Uint8Array(buf);}else{throw new Error('DecompressionStream not available');}}const mod=await WebAssembly.compile(bytes);self.postMessage({ok:1,mod});}catch(err){self.postMessage({ok:0,err:String(err)})}}";\n const blob = new Blob([src], { type: 'application/javascript' }); + const minified = DECOMPRESSION_WORKER_SRC.replace(/\s+/g, ' ').trim(); + const blob = new Blob([minified], { type: 'application/javascript' });
75-77
: Consider adding percentage reduction to the output.The logging could be enhanced by showing the percentage reduction achieved through compression.
-console.log(`gzip(sdk.js): ${baseGzipSize} bytes | gzip(sdk.compressed.js): ${compressedGzipSize} bytes`); +const reduction = ((baseGzipSize - compressedGzipSize) / baseGzipSize * 100).toFixed(1); +console.log(`gzip(sdk.js): ${baseGzipSize} bytes | gzip(sdk.compressed.js): ${compressedGzipSize} bytes (${reduction}% reduction)`);
61-61
: Add Content Security Policy considerations to documentation.The dynamic Worker creation using Blob URLs may be blocked by strict Content Security Policy (CSP) settings. Consider documenting that users may need to adjust their CSP to include
worker-src blob:
or provide a fallback mechanism.Would you like me to help create documentation about CSP requirements for the compressed SDK or implement a CSP-friendly fallback mechanism?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
packages/js-evo-sdk/package.json
(1 hunks)packages/js-evo-sdk/src/wasm.ts
(2 hunks)packages/js-evo-sdk/tsconfig.json
(1 hunks)packages/js-evo-sdk/webpack.config.cjs
(1 hunks)packages/wasm-sdk/package.json
(1 hunks)packages/wasm-sdk/scripts/bundle.cjs
(2 hunks)packages/wasm-sdk/tests/functional/dpns.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/epochs-blocks.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/groups.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/identities.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/protocol.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/status.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/system.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/token-pricing.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/tokens.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/transitions.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/utilities.spec.mjs
(1 hunks)packages/wasm-sdk/tests/functional/voting.spec.mjs
(1 hunks)packages/wasm-sdk/tests/unit/address-validation.spec.mjs
(1 hunks)packages/wasm-sdk/tests/unit/api-availability.spec.mjs
(1 hunks)packages/wasm-sdk/tests/unit/builder.spec.mjs
(1 hunks)packages/wasm-sdk/tests/unit/derivation.spec.mjs
(1 hunks)packages/wasm-sdk/tests/unit/dpns-utils.spec.mjs
(1 hunks)packages/wasm-sdk/tests/unit/errors.spec.mjs
(1 hunks)packages/wasm-sdk/tests/unit/extended-keys.spec.mjs
(1 hunks)packages/wasm-sdk/tests/unit/key-generation.spec.mjs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/functional/dpns.spec.mjs
packages/wasm-sdk/tests/functional/system.spec.mjs
packages/wasm-sdk/tests/unit/errors.spec.mjs
packages/wasm-sdk/tests/unit/dpns-utils.spec.mjs
packages/wasm-sdk/tests/functional/groups.spec.mjs
packages/wasm-sdk/tests/functional/transitions.spec.mjs
packages/wasm-sdk/tests/unit/builder.spec.mjs
packages/wasm-sdk/tests/functional/token-pricing.spec.mjs
packages/wasm-sdk/tests/functional/tokens.spec.mjs
packages/wasm-sdk/tests/functional/epochs-blocks.spec.mjs
packages/wasm-sdk/tests/functional/status.spec.mjs
packages/wasm-sdk/tests/unit/extended-keys.spec.mjs
packages/wasm-sdk/tests/functional/utilities.spec.mjs
packages/wasm-sdk/package.json
packages/wasm-sdk/tests/functional/identities.spec.mjs
packages/wasm-sdk/tests/functional/protocol.spec.mjs
packages/wasm-sdk/tests/unit/derivation.spec.mjs
packages/wasm-sdk/tests/unit/address-validation.spec.mjs
packages/wasm-sdk/tests/functional/voting.spec.mjs
packages/wasm-sdk/tests/unit/key-generation.spec.mjs
packages/wasm-sdk/tests/unit/api-availability.spec.mjs
packages/wasm-sdk/scripts/bundle.cjs
packages/**/tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place unit and integration tests alongside each package in packages//tests
Files:
packages/wasm-sdk/tests/functional/dpns.spec.mjs
packages/wasm-sdk/tests/functional/system.spec.mjs
packages/wasm-sdk/tests/unit/errors.spec.mjs
packages/wasm-sdk/tests/unit/dpns-utils.spec.mjs
packages/wasm-sdk/tests/functional/groups.spec.mjs
packages/wasm-sdk/tests/functional/transitions.spec.mjs
packages/wasm-sdk/tests/unit/builder.spec.mjs
packages/wasm-sdk/tests/functional/token-pricing.spec.mjs
packages/wasm-sdk/tests/functional/tokens.spec.mjs
packages/wasm-sdk/tests/functional/epochs-blocks.spec.mjs
packages/wasm-sdk/tests/functional/status.spec.mjs
packages/wasm-sdk/tests/unit/extended-keys.spec.mjs
packages/wasm-sdk/tests/functional/utilities.spec.mjs
packages/wasm-sdk/tests/functional/identities.spec.mjs
packages/wasm-sdk/tests/functional/protocol.spec.mjs
packages/wasm-sdk/tests/unit/derivation.spec.mjs
packages/wasm-sdk/tests/unit/address-validation.spec.mjs
packages/wasm-sdk/tests/functional/voting.spec.mjs
packages/wasm-sdk/tests/unit/key-generation.spec.mjs
packages/wasm-sdk/tests/unit/api-availability.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/wasm.ts
🧠 Learnings (8)
📚 Learning: 2025-09-03T14:42:29.958Z
Learnt from: thephez
PR: dashpay/platform#2754
File: packages/wasm-sdk/docs.html:1970-1971
Timestamp: 2025-09-03T14:42:29.958Z
Learning: In packages/wasm-sdk/, the docs.html file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in docs.html, as manual changes to docs.html would be overwritten during regeneration.
Applied to files:
packages/wasm-sdk/tests/functional/dpns.spec.mjs
packages/wasm-sdk/tests/unit/errors.spec.mjs
packages/wasm-sdk/tests/functional/groups.spec.mjs
packages/wasm-sdk/tests/functional/token-pricing.spec.mjs
packages/wasm-sdk/tests/functional/tokens.spec.mjs
packages/wasm-sdk/tests/functional/utilities.spec.mjs
packages/wasm-sdk/tests/functional/protocol.spec.mjs
packages/wasm-sdk/tests/unit/address-validation.spec.mjs
packages/wasm-sdk/tests/unit/key-generation.spec.mjs
📚 Learning: 2025-09-07T22:18:50.883Z
Learnt from: CR
PR: dashpay/platform#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T22:18:50.883Z
Learning: Applies to packages/wasm-sdk/** : Keep WASM SDK docs in sync (run generate_docs.py) when updating the WASM SDK
Applied to files:
packages/wasm-sdk/tests/functional/dpns.spec.mjs
packages/wasm-sdk/tests/unit/errors.spec.mjs
packages/wasm-sdk/tests/functional/groups.spec.mjs
📚 Learning: 2025-09-03T14:41:16.196Z
Learnt from: thephez
PR: dashpay/platform#2754
File: packages/wasm-sdk/AI_REFERENCE.md:766-766
Timestamp: 2025-09-03T14:41:16.196Z
Learning: In packages/wasm-sdk/, the AI_REFERENCE.md file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in AI_REFERENCE.md, as manual changes to AI_REFERENCE.md would be overwritten during regeneration.
Applied to files:
packages/wasm-sdk/tests/functional/dpns.spec.mjs
packages/wasm-sdk/tests/unit/errors.spec.mjs
packages/wasm-sdk/tests/functional/groups.spec.mjs
packages/wasm-sdk/tests/functional/tokens.spec.mjs
packages/wasm-sdk/tests/unit/address-validation.spec.mjs
📚 Learning: 2025-02-10T11:26:36.709Z
Learnt from: lklimek
PR: dashpay/platform#2405
File: packages/wasm-sdk/src/verify.rs:26-68
Timestamp: 2025-02-10T11:26:36.709Z
Learning: In the wasm-sdk package, empty vectors and placeholder values are intentionally used in verification functions during the proof-of-concept stage to ensure proper compilation and type checking.
Applied to files:
packages/wasm-sdk/tests/unit/errors.spec.mjs
📚 Learning: 2025-09-02T13:30:17.703Z
Learnt from: thephez
PR: dashpay/platform#2739
File: packages/wasm-sdk/test/ui-automation/tests/state-transitions.spec.js:1-171
Timestamp: 2025-09-02T13:30:17.703Z
Learning: In packages/wasm-sdk/index.html, state transition definitions are loaded dynamically from api-definitions.json via the loadApiDefinitions() function that fetches './api-definitions.json'. The UI doesn't have hardcoded transition definitions - instead it populates categories, types, inputs, and labels from this JSON configuration file at runtime.
Applied to files:
packages/wasm-sdk/tests/functional/transitions.spec.mjs
📚 Learning: 2025-09-02T13:30:17.703Z
Learnt from: thephez
PR: dashpay/platform#2739
File: packages/wasm-sdk/test/ui-automation/tests/state-transitions.spec.js:1-171
Timestamp: 2025-09-02T13:30:17.703Z
Learning: In packages/wasm-sdk/index.html, state transition definitions are loaded dynamically from api-definitions.json rather than being hardcoded in the HTML file. The UI loads transition categories, types, inputs, and labels from this JSON configuration file.
Applied to files:
packages/wasm-sdk/tests/functional/transitions.spec.mjs
📚 Learning: 2024-11-06T07:27:01.722Z
Learnt from: shumkov
PR: dashpay/platform#2314
File: packages/wallet-contract/test/bootstrap.js:14-16
Timestamp: 2024-11-06T07:27:01.722Z
Learning: In `packages/wallet-contract/test/bootstrap.js`, for Mocha tests in Node.js, async functions like `loadWasmDpp` can be assigned directly to `beforeAll` without wrapping them in another async function.
Applied to files:
packages/wasm-sdk/tests/unit/builder.spec.mjs
📚 Learning: 2025-07-28T20:00:08.502Z
Learnt from: QuantumExplorer
PR: dashpay/platform#2711
File: packages/wasm-sdk/AI_REFERENCE.md:771-783
Timestamp: 2025-07-28T20:00:08.502Z
Learning: In packages/wasm-sdk/AI_REFERENCE.md, the documentation correctly shows the actual SDK method signatures (including identityCreate and identityTopUp with their full parameter lists), which may differ from the UI inputs shown in fixed_definitions.json. The UI may collect fewer parameters from users while handling additional requirements internally.
Applied to files:
packages/wasm-sdk/tests/functional/tokens.spec.mjs
packages/wasm-sdk/tests/functional/identities.spec.mjs
packages/wasm-sdk/tests/functional/protocol.spec.mjs
packages/wasm-sdk/tests/unit/derivation.spec.mjs
⏰ 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) / Formatting
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI 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 (31)
packages/wasm-sdk/tests/unit/dpns-utils.spec.mjs (1)
1-1
: Import path updated to compressed build — verification requiredThe import in packages/wasm-sdk/tests/unit/dpns-utils.spec.mjs was changed to '../../dist/sdk.compressed.js' (matches PR intent). Verification couldn't complete because packages/wasm-sdk/dist/ is missing in the sandbox. Confirm locally that packages/wasm-sdk/dist/sdk.compressed.js exists and that no other tests import '../../dist/sdk.js'. Recommended local checks: fd 'sdk.compressed.js' packages/wasm-sdk/dist/ && rg -n --type=js 'dist/sdk.js' packages/wasm-sdk/tests/.
packages/wasm-sdk/tests/functional/protocol.spec.mjs (1)
1-1
: LGTM! Successfully migrated to compressed build.The test now imports the compressed wasm-sdk build correctly. Based on the research, WASM typically compresses to less than 50% its uncompressed size with gzip, which aligns with the PR's optimization goal for reducing bundle size.
packages/wasm-sdk/tests/functional/groups.spec.mjs (1)
1-1
: LGTM! Consistent migration to compressed build.The import path has been correctly updated to use the compressed SDK build, maintaining consistency with other functional tests in this PR.
packages/wasm-sdk/tests/functional/utilities.spec.mjs (1)
1-1
: LGTM! Clean migration to compressed build.The import has been correctly updated to use the compressed build while preserving all test functionality.
packages/wasm-sdk/tests/unit/api-availability.spec.mjs (1)
1-1
: LGTM! API availability test correctly updated.The test now uses the compressed build while maintaining the same API surface validation, which is crucial for ensuring the compressed build maintains functional parity.
packages/wasm-sdk/tests/functional/system.spec.mjs (1)
1-1
: LGTM! System test updated to compressed build.The import has been correctly updated to use the compressed SDK build, maintaining consistency with the broader test migration in this PR.
packages/wasm-sdk/tests/unit/errors.spec.mjs (1)
1-1
: LGTM! Error handling test preserved with compressed build.The import correctly targets the compressed build while maintaining all error handling validation logic intact.
packages/wasm-sdk/tests/functional/dpns.spec.mjs (1)
1-1
: LGTM! DPNS functional test migrated successfully.The test correctly imports the compressed build, ensuring DPNS functionality testing continues with the optimized SDK.
packages/wasm-sdk/tests/unit/extended-keys.spec.mjs (1)
1-1
: LGTM! Extended keys test updated to compressed build.The import has been correctly updated while preserving all cryptographic key functionality tests.
packages/wasm-sdk/tests/unit/derivation.spec.mjs (1)
1-1
: Import path successfully updated to compressed SDK.The change from
'../../dist/sdk.js'
to'../../dist/sdk.compressed.js'
aligns with the PR's optimization objective. WASM tends to compress very well, typically shrinking to less than 50% its uncompressed size, and always serve compressed WASM in production is a well-established best practice.packages/wasm-sdk/tests/unit/key-generation.spec.mjs (1)
1-1
: Import path correctly updated to compressed build.The change aligns with the PR's optimization strategy and follows WASM compression best practices.
packages/wasm-sdk/tests/functional/token-pricing.spec.mjs (1)
1-1
: Import path appropriately updated for compressed SDK.The functional test now uses the compressed build, which is consistent with the PR's optimization goals.
packages/wasm-sdk/tests/unit/address-validation.spec.mjs (1)
1-1
: Import path correctly switched to compressed build.The unit test change is consistent with the broader PR migration to the compressed WASM SDK.
packages/wasm-sdk/tests/functional/epochs-blocks.spec.mjs (1)
1-1
: Import path updated appropriately to compressed build.This functional test correctly adopts the compressed SDK import path.
packages/wasm-sdk/tests/functional/voting.spec.mjs (1)
1-1
: Import path properly updated for compressed SDK.The test file successfully migrates to the compressed build import.
packages/js-evo-sdk/webpack.config.cjs (1)
32-32
: ES module output filename appropriately renamed.The change from
'sdk.js'
to'evo-sdk.module.js'
provides better naming clarity and aligns with the project's packaging strategy for the compressed SDK optimization.packages/wasm-sdk/tests/functional/tokens.spec.mjs (1)
1-1
: Import path correctly updated to compressed build.The functional test now uses the compressed SDK, maintaining consistency across the test suite.
packages/wasm-sdk/tests/functional/identities.spec.mjs (1)
1-1
: Confirmed import path update for compressed build.The test now imports from the compressed build (
sdk.compressed.js
) instead of the standard build. This is consistent with the PR objective to optimize wasm-sdk size by switching to a compressed distribution. Gzip compression of WASM files typically achieves 60-75% size reductions compared to uncompressed ones, making this a valuable optimization for web delivery.packages/wasm-sdk/tests/unit/builder.spec.mjs (1)
1-1
: Import path updated to compressed build.The test correctly imports from
sdk.compressed.js
maintaining the same API surface while benefiting from the compressed distribution.packages/wasm-sdk/tests/functional/transitions.spec.mjs (1)
1-1
: Import path updated to compressed build.The test correctly switches to the compressed build import path, maintaining all existing functionality while reducing bundle size.
packages/wasm-sdk/package.json (1)
12-15
: New compressed export path added correctly.The new
"./compressed"
export provides access to the compressed build while maintaining the same TypeScript declarations. This follows the established pattern of the existing exports and enables consumers to opt-in to the compressed version.packages/wasm-sdk/tests/functional/status.spec.mjs (1)
1-1
: Import path updated to compressed build.The test correctly imports from
sdk.compressed.js
, consistent with the broader migration to the compressed distribution across the test suite.packages/js-evo-sdk/tsconfig.json (1)
15-15
: TypeScript path alias added for compressed build.The new path alias
"@dashevo/wasm-sdk/compressed"
correctly maps to the same TypeScript declarations as the base wasm-sdk, enabling proper type resolution for the compressed import path.packages/js-evo-sdk/src/wasm.ts (1)
2-2
: Switched to compressed wasm-sdk exports.The exports now use the compressed build path (
@dashevo/wasm-sdk/compressed
) while maintaining the same API surface. This change enables the EVO SDK to benefit from the compressed distribution automatically.Also applies to: 14-15
packages/js-evo-sdk/package.json (1)
5-5
: Verify build produces dist/evo-sdk.module.js and preserves public API
- package.json main/exports now point to "./dist/evo-sdk.module.js" and types -> "./dist/sdk.d.ts" (packages/js-evo-sdk/package.json).
- webpack config emits 'evo-sdk.module.js' (packages/js-evo-sdk/webpack.config.cjs:32).
- packages/js-evo-sdk/dist/ is missing in the repo; the built artifact and types cannot be validated here.
Build the package (CI or locally) and confirm packages/js-evo-sdk/dist/evo-sdk.module.js and packages/js-evo-sdk/dist/sdk.d.ts are produced and that the exported API matches the previous sdk.js, then re-run the verification script above.
packages/wasm-sdk/scripts/bundle.cjs (6)
16-16
: LGTM! Addition of zlib import for compression support.The zlib module is appropriately imported to enable gzip compression functionality used later in the script.
53-57
: Well-implemented compression with optimal settings.The code correctly:
- Reads the WASM bytes
- Creates both base64 and gzip-compressed versions
- Uses
Z_BEST_COMPRESSION
for maximum size reduction- Encodes the compressed data as base64 for embedding
64-71
: Good telemetry for size comparisons.The code provides useful metrics by comparing:
- Raw wrapper size vs compressed wrapper size
- Gzipped sizes of both wrappers
This helps validate the compression benefits.
61-61
: Ensure error handling consistency between wrapper implementations.Both wrappers handle Worker compilation failures by falling back to main thread compilation. However, the compressed wrapper's error handling in
__compileInWorker
rejects the promise on error, while the original wrapper resolves with a fallback. Verify this behavioral difference is intentional.The compressed wrapper's
__compileInWorker
:
- Rejects on worker errors
- Has explicit error handling with cleanup
The original wrapper's
__compileInWorker
:
- Always resolves (falls back to main thread on any issue)
- No explicit worker error handler
Is this difference intentional? The compressed version seems more robust but changes the error handling behavior.
61-61
: Node dynamic import ('node:zlib') is compatible with the declared engine.packages/wasm-sdk/package.json declares "engines.node": ">=18.18", which supports the node: protocol and the dynamic import used — no change required.
61-61
: DecompressionStream support confirmed — adjust only if you must support older browsersAs of Sept 19, 2025, DecompressionStream is supported in Chrome/Edge/Opera (v80+), Safari/iOS Safari (v16.4+), and Firefox (desktop & Android) since v113. If your minimum supported browsers include versions older than those, add a fallback/polyfill; otherwise no change required.
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: 5
🧹 Nitpick comments (9)
packages/js-evo-sdk/tests/functional/documents.spec.mjs (3)
21-21
: Use Chai’s property assertion, not a function call.
exist
is a property in Chai; calling it as a function can throw. Use.to.exist
instead of.to.exist()
.- expect(res).to.exist(); + expect(res).to.exist;Also applies to: 30-30
9-12
: Add teardown to disconnect the SDK after tests.Avoid leaking network resources; add an
after
hook to disconnect.before(async () => { sdk = EvoSDK.testnetTrusted(); await sdk.connect(); }); + + after(async () => { + await sdk?.disconnect?.(); + });Also applies to: 32-32
2-2
: ESM artifact import consistent — no stale ../../dist/sdk.js imports found.Occurrences of '../../dist/evo-sdk.module.js' found in:
packages/js-evo-sdk/tests/unit/sdk.spec.mjs
packages/js-evo-sdk/tests/functional/dpns.spec.mjs
packages/js-evo-sdk/tests/functional/system.spec.mjs
packages/js-evo-sdk/tests/functional/documents.spec.mjs
packages/js-evo-sdk/tests/functional/contracts.spec.mjs
packages/js-evo-sdk/tests/functional/voting.spec.mjs
packages/js-evo-sdk/tests/functional/tokens.spec.mjs
packages/js-evo-sdk/tests/functional/protocol.spec.mjs
packages/js-evo-sdk/tests/functional/identities.spec.mjs
packages/js-evo-sdk/tests/functional/group.spec.mjs
packages/js-evo-sdk/tests/functional/epoch.spec.mjs
Prefer switching tests to import from the package entry (package.json "exports" or main) to avoid brittle dist path churn.
packages/js-evo-sdk/tests/functional/group.spec.mjs (1)
2-2
: Import rename verified — no stale dist/sdk.js; optional test-import helper recommended
- No occurrences of dist/sdk.js in packages. packages/js-evo-sdk/package.json (main/exports) and packages/js-evo-sdk/webpack.config.cjs reference ./dist/evo-sdk.module.js.
- Optional: add packages/js-evo-sdk/tests/helpers/sdk-import.mjs that re-exports '../../dist/evo-sdk.module.js' and update tests to import from that helper.
packages/js-evo-sdk/tests/functional/protocol.spec.mjs (1)
2-2
: Reduce brittleness by importing via the package entry instead of a dist filename.To decouple tests from build artifact names, import through the package’s exports (if configured) rather than a hardcoded dist path.
Example:
-import { EvoSDK } from '../../dist/evo-sdk.module.js'; +import { EvoSDK } from '@dashevo/js-evo-sdk';If these tests intentionally exercise the built dist directly, ignore this suggestion.
packages/js-evo-sdk/tests/functional/system.spec.mjs (1)
2-2
: Optional: import via the package entry point to reduce churn.
This makes tests resilient to future filename changes and validates the exported entrypoint. Keep the current import if you specifically want to exercise the artifact path.-import { EvoSDK } from '../../dist/evo-sdk.module.js'; +import { EvoSDK } from '../..';packages/js-evo-sdk/tests/functional/tokens.spec.mjs (2)
16-16
: Chai assertion style: use the property.exist
(not a function) unless a plugin overrides it.Vanilla Chai exposes
exist
as a chainable property. Calling it asexist()
can throw unless you rely on a custom plugin. If no plugin is intended, switch to the property form:- expect(res).to.exist(); + expect(res).to.exist;If a plugin is in place that intentionally provides
exist()
, ignore this suggestion.Also applies to: 21-21, 26-26, 33-33, 38-38
6-6
: Consider a higher timeout for network + wasm init to reduce flakiness.The suite hits testnet and initializes WASM; bumping to 120s is safer.
- this.timeout(60000); + this.timeout(120000);packages/js-evo-sdk/tests/unit/sdk.spec.mjs (1)
12-19
: Add teardown to avoid leaking connections in the connect testGuard against resource leaks and improve test isolation by disconnecting in finally.
- const sdk = new EvoSDK(); - expect(sdk.isConnected).to.equal(false); - await sdk.connect(); - expect(sdk.isConnected).to.equal(true); + const sdk = new EvoSDK(); + try { + expect(sdk.isConnected).to.equal(false); + await sdk.connect(); + expect(sdk.isConnected).to.equal(true); + } finally { + if (sdk && typeof sdk.disconnect === 'function') { + await sdk.disconnect(); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/js-evo-sdk/tests/functional/contracts.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/functional/documents.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/functional/dpns.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/functional/epoch.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/functional/group.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/functional/identities.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/functional/protocol.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/functional/system.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/functional/tokens.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/functional/voting.spec.mjs
(1 hunks)packages/js-evo-sdk/tests/unit/sdk.spec.mjs
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/js-evo-sdk/tests/functional/epoch.spec.mjs
🧰 Additional context used
📓 Path-based instructions (1)
packages/**/tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place unit and integration tests alongside each package in packages//tests
Files:
packages/js-evo-sdk/tests/functional/tokens.spec.mjs
packages/js-evo-sdk/tests/functional/protocol.spec.mjs
packages/js-evo-sdk/tests/functional/identities.spec.mjs
packages/js-evo-sdk/tests/functional/contracts.spec.mjs
packages/js-evo-sdk/tests/functional/group.spec.mjs
packages/js-evo-sdk/tests/unit/sdk.spec.mjs
packages/js-evo-sdk/tests/functional/dpns.spec.mjs
packages/js-evo-sdk/tests/functional/system.spec.mjs
packages/js-evo-sdk/tests/functional/voting.spec.mjs
packages/js-evo-sdk/tests/functional/documents.spec.mjs
⏰ 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). (6)
- 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 JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (3)
packages/js-evo-sdk/tests/functional/protocol.spec.mjs (1)
2-2
: Import path update looks good — confirm ESM named export.
rg shows 11 tests import { EvoSDK } from '../../dist/evo-sdk.module.js' and no imports of '../../dist/sdk.js' remain; verify that packages/js-evo-sdk/dist/evo-sdk.module.js exports EvoSDK as a named export.packages/js-evo-sdk/tests/functional/system.spec.mjs (1)
2-2
: Legacy dist/sdk.js imports found — update unit facade tests to use evo-sdk.module.js and ensure build artifact exists.Found these files importing '../../../dist/sdk.js'; change the import to '../../../dist/evo-sdk.module.js' (or the correct relative path) and confirm packages/js-evo-sdk/dist/evo-sdk.module.js is produced before running tests:
- packages/js-evo-sdk/tests/unit/facades/voting.spec.mjs
- packages/js-evo-sdk/tests/unit/facades/identities.spec.mjs
- packages/js-evo-sdk/tests/unit/facades/group.spec.mjs
- packages/js-evo-sdk/tests/unit/facades/dpns.spec.mjs
- packages/js-evo-sdk/tests/unit/facades/documents.spec.mjs
- packages/js-evo-sdk/tests/unit/facades/contracts.spec.mjs
- packages/js-evo-sdk/tests/unit/facades/system.spec.mjs
- packages/js-evo-sdk/tests/unit/facades/tokens.spec.mjs
- packages/js-evo-sdk/tests/unit/facades/protocol.spec.mjs
- packages/js-evo-sdk/tests/unit/facades/epoch.spec.mjs
⛔ Skipped due to learnings
Learnt from: CR PR: dashpay/platform#0 File: AGENTS.md:0-0 Timestamp: 2025-09-12T13:18:08.661Z Learning: Applies to packages/**/tests/**/*.{js,ts,jsx,tsx,rs} : Unit/integration tests must not perform network calls; mock dependencies instead
Learnt from: shumkov PR: dashpay/platform#2249 File: packages/dashmate/test/unit/status/scopes/platform.spec.js:466-467 Timestamp: 2024-10-18T15:37:21.329Z Learning: In test files for the dashmate project, such as `packages/dashmate/test/unit/status/scopes/platform.spec.js`, prefer to keep mocks explicitly in tests rather than consolidating them, to increase readability.
Learnt from: shumkov PR: dashpay/platform#2249 File: packages/dashmate/test/unit/status/scopes/platform.spec.js:77-78 Timestamp: 2024-10-18T15:37:36.387Z Learning: In `packages/dashmate/test/unit/status/scopes/platform.spec.js`, prefer to keep mocks explicitly in tests to increase readability, rather than refactoring them into shared `beforeEach` blocks or helper functions.
Learnt from: CR PR: dashpay/platform#0 File: AGENTS.md:0-0 Timestamp: 2025-09-12T13:18:08.661Z Learning: Applies to packages/platform-test-suite/** : Keep all E2E tests exclusively in packages/platform-test-suite
packages/js-evo-sdk/tests/functional/contracts.spec.mjs (1)
1-1
: Confirm distribution directory and stale imports
Thepackages/js-evo-sdk/dist
folder wasn’t found—ensure your build emitsevo-sdk.module.js
there (or adjust the test’s../../dist
path to where the ESM artifact actually lives). Then verify no remaining imports of the oldsdk.js
across your tests.
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/wasm-sdk/scripts/bundle.cjs (1)
46-51
: Fail fast if wasm_bindgen URL sanitization pattern changesThe regex is brittle; if it doesn’t match, bundlers may still emit a .wasm asset.
-const sanitizedJs = rawJs.replace(defaultUrlRegex, "if (typeof module_or_path === 'undefined') { }"); +const sanitizedJs = rawJs.replace(defaultUrlRegex, "if (typeof module_or_path === 'undefined') { }"); +if (!defaultUrlRegex.test(rawJs)) { + console.warn('Warning: wasm URL sentinel not found; downstream bundlers may still emit the .wasm file.'); +} +if (sanitizedJs === rawJs) { + throw new Error('Sanitization failed: expected to strip import.meta.url wasm path'); +}Longer‑term: consider a more resilient transform (AST or multiple tolerant regexes).
🧹 Nitpick comments (5)
packages/wasm-sdk/scripts/bundle.cjs (5)
53-57
: Make gzip output deterministic (set mtime=0)Ensures reproducible builds and stable base64 payloads for caching and diffing.
Apply:
-const wasmGzip = zlib.gzipSync(wasmBytes, { level: zlib.constants.Z_BEST_COMPRESSION }); +const wasmGzip = zlib.gzipSync(wasmBytes, { + level: zlib.constants.Z_BEST_COMPRESSION, + mtime: 0, // reproducible gzip header +});
61-61
: Avoid extra copy: transfer ArrayBuffer to the workerSaves a copy and reduces main-thread memory spike.
- try { - worker.postMessage({ compressed: bytes, compression: __WASM_COMPRESSION }); + try { + worker.postMessage( + { compressed: bytes.buffer, compression: __WASM_COMPRESSION }, + [bytes.buffer] // transfer ownership + );The worker already reconstructs a Uint8Array if it receives a non-Uint8Array; no further changes required.
64-71
: Deterministic gzip stats; optionally add brotli statsSet mtime=0 for size metrics to make CI diffs stable.
-const baseGzipSize = zlib.gzipSync(fs.readFileSync(sdkJsPath)).length; -const compressedGzipSize = zlib.gzipSync(fs.readFileSync(sdkCompressedPath)).length; +const baseGzipSize = zlib.gzipSync(fs.readFileSync(sdkJsPath), { mtime: 0 }).length; +const compressedGzipSize = zlib.gzipSync(fs.readFileSync(sdkCompressedPath), { mtime: 0 }).length;
75-77
: Log relative savings for clarityPrinting percent deltas makes the improvement immediately obvious.
-console.log(`gzip(sdk.js): ${baseGzipSize} bytes | gzip(sdk.compressed.js): ${compressedGzipSize} bytes`); +const pct = ((1 - compressedGzipSize / baseGzipSize) * 100).toFixed(1); +console.log(`gzip(sdk.js): ${baseGzipSize} bytes | gzip(sdk.compressed.js): ${compressedGzipSize} bytes (${pct}% smaller)`);
22-22
: Remove unused variabletypesDir
Not referenced.
-const typesDir = path.join(root, 'types');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/wasm-sdk/scripts/bundle.cjs
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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/scripts/bundle.cjs
🧠 Learnings (1)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2783
File: packages/js-evo-sdk/tests/functional/dpns.spec.mjs:2-2
Timestamp: 2025-09-22T05:59:31.119Z
Learning: The dist/evo-sdk.module.js file in packages/js-evo-sdk is a generated build artifact created during the build process, not a source file that should exist in the repository.
⏰ 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). (8)
- 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 (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (3)
packages/wasm-sdk/scripts/bundle.cjs (3)
16-16
: LGTM: add zlib for build-time compressionNo issues with requiring zlib in this build script.
61-61
: Pass raw bytes/module to initSync (not an object)wasm-bindgen initSync expects a BufferSource or WebAssembly.Module; calling it with an object will fail at runtime.
- packages/wasm-sdk/scripts/bundle.cjs — in both wrapper templates, replace:
- return rawInitSync({ module: bytes }); + return rawInitSync(bytes);- packages/wasm-dpp/lib/index.ts — replace:
- dpp_module.initSync({ module: bytes }); + dpp_module.initSync(bytes);
62-62
: "./compressed" export present — update docs & regenerate WASM SDK docs
- packages/wasm-sdk/package.json already exposes "./compressed" -> types: ./dist/sdk.d.ts, import: ./dist/sdk.compressed.js (verified).
- Update usage/API docs and regenerate the WASM SDK docs from packages/wasm-sdk/api-definitions.json by running packages/wasm-sdk/generate_docs.py (do not edit generated files like docs.html/AI_REFERENCE.md directly); commit the regenerated docs.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr.yml (1)
16-16
: Pin the third-party action to a commit SHA (security best practice, especially with pull_request_target).Using
pull_request_target
exposes repo secrets; pinning avoids supply‑chain risk from a moving tag.Apply this diff (replace COMMIT-SHA with the v5 commit SHA from the action’s releases):
- uses: amannn/action-semantic-pull-request@v5 + uses: amannn/action-semantic-pull-request@COMMIT-SHA
🧹 Nitpick comments (2)
.github/workflows/pr.yml (2)
33-53
: Preemptively allow evo SDK package scopes to avoid false negatives.Given active work across multiple SDKs, consider explicitly allowing
evo-sdk
and/orjs-evo-sdk
as scopes so titles likeperf(evo-sdk): …
pass without relying on the genericsdk
.Apply this diff:
scopes: | sdk + evo-sdk + js-evo-sdk dapi platform drive drive-abci dpp dashmate contract rs-sdk rs-sdk-ffi swift-sdk strategy-tests simple-signer wallet-lib test-suite bench-suite release wasm-sdk
54-54
: Optional: enforce scopes for consistency.If you want stricter hygiene across many packages, consider
requireScope: true
. Not blocking for this PR.- requireScope: false + requireScope: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr.yml
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: shumkov
PR: dashpay/platform#2783
File: packages/js-evo-sdk/tests/functional/dpns.spec.mjs:2-2
Timestamp: 2025-09-22T05:59:31.119Z
Learning: The dist/evo-sdk.module.js file in packages/js-evo-sdk is a generated build artifact created during the build process, not a source file that should exist in the repository.
📚 Learning: 2025-09-12T13:18:08.661Z
Learnt from: CR
PR: dashpay/platform#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:18:08.661Z
Learning: Follow Conventional Commits for PR titles and commit messages (e.g., feat(sdk): add identity fetch), using ! for breaking changes
Applied to files:
.github/workflows/pr.yml
🔇 Additional comments (1)
.github/workflows/pr.yml (1)
35-35
: Switching scope from “ci” to “sdk” is correct per Conventional Commits.“ci” is a type, not a scope. Keeping it in
types
and removing it fromscopes
aligns with the spec. Approved.
Issue being fixed or feature implemented
Wasm SDK is quite big for browser: ~10 Mb
What was done?
How Has This Been Tested?
With existing unit and functional tests
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit