feat(sdk): wasm drive verify optimization#2683
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the WASM build pipeline for wasm-drive-verify by moving package metadata into its root package.json, applying Cargo and Binaryen optimizations in the build script, and removing redundant profile settings from Cargo.toml.
- Consolidated and simplified package metadata in
packages/wasm-drive-verify/package.json(addedmain/types, formatted collaborators). - Enhanced
build.shto usewasm-snip, refinedwasm-bindgeninvocation, and integrated extensivewasm-optoptimizations. - Removed local
[profile.release]inCargo.tomland registered the new package in the workspace’s rootpackage.json.
Reviewed Changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/wasm-drive-verify/package.json | Defined entry points (main, types) and formatted metadata |
| packages/wasm-drive-verify/build.sh | Updated build steps: Cargo --config flags, wasm-snip/opt |
| packages/wasm-drive-verify/Cargo.toml | Removed [profile.release] section in favor of CLI overrides |
| package.json | Added packages/wasm-drive-verify to workspace packages |
Files not reviewed (1)
- .pnp.cjs: Language not supported
Comments suppressed due to low confidence (2)
packages/wasm-drive-verify/package.json:18
- The
exportsfield still points to./dist/index.js, but the package entry points are now inpkg/. Update this to referencepkg/wasm_drive_verify.js(and correspondingrequirepath) to match the newmain.
"import": "./dist/index.js",
packages/wasm-drive-verify/build.sh:38
- The
wasm-bindgencommand is passing the input file as an argument to the--omit-default-module-pathflag. Move the input path to the end of the command and keep--omit-default-module-pathstandalone, e.g.:wasm-bindgen ../../target/.../wasm_drive_verify.wasm --out-dir pkg --target web --omit-default-module-path.
--omit-default-module-path ../../target/wasm32-unknown-unknown/release/wasm_drive_verify.wasm
| transitions = [] | ||
| debug_logs = [] | ||
|
|
||
| [profile.release] |
There was a problem hiding this comment.
Can we move these settings to .cargo/config.toml and .cargo/config-release.toml instead of adding them to the build script?
Copilot suggests (but I didn't test):
[target.wasm32-unknown-unknown.profile.release]
opt-level = "z"
lto = true
codegen-units = 1
strip = true
panic = "abort"There was a problem hiding this comment.
doesn't work.
3.6 mb instead of 3 mb.
And if you put it in .cargo/config.toml, it throws an error
error: expected a string, but found a table for `release` in /Users/bebra/Documents/Dash/verify/platform/packages/wasm-drive-verify/.cargo/config.toml
QuantumExplorer
left a comment
There was a problem hiding this comment.
Thanks, this is great.
Resolves merge conflicts by: - Accepting upstream Cargo.toml changes that properly fix LTO configuration at workspace level (PR #2683) - Accepting upstream .pnp.cjs and yarn.lock changes - Removing temporary local workarounds for WASM build issues 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Issue being fixed or feature implemented
This PR contains a fix for a build process that results in an unoptimized js package
Current size of build: 3 Mb
What was done?
How Has This Been Tested?
build by
yarn buildand then test in this projectBreaking Changes
Checklist:
For repository code-owners and collaborators only