-
Notifications
You must be signed in to change notification settings - Fork 39
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
build: bump wasm-bindgen to 0.2.99 #2395
Conversation
WalkthroughThis pull request focuses on updating the Changes
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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.
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 (1)
packages/wasm-dpp/src/identity/factory_utils.rs (1)
49-52
: LGTM! Type inference simplification looks good.The removal of explicit type annotation for
public_key
is safe as Rust's type inference system can correctly determine the type from thegeneric_of_js_val
function call.Consider adding a brief comment explaining the type being inferred for better code maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (11)
.devcontainer/Dockerfile
(1 hunks).github/workflows/release.yml
(1 hunks).github/workflows/tests-build-js.yml
(1 hunks)Dockerfile
(1 hunks)README.md
(1 hunks)packages/wasm-dpp/Cargo.toml
(2 hunks)packages/wasm-dpp/README.md
(1 hunks)packages/wasm-dpp/src/identity/factory_utils.rs
(2 hunks)packages/wasm-dpp/src/identity/state_transition/asset_lock_proof/mod.rs
(1 hunks)packages/wasm-dpp/src/identity/state_transition/identity_create_transition/identity_create_transition.rs
(2 hunks)packages/wasm-dpp/src/identity/state_transition/identity_update_transition/identity_update_transition.rs
(3 hunks)
🔇 Additional comments (14)
packages/wasm-dpp/Cargo.toml (2)
52-52
: LGTM: Compatible futures version
The wasm-bindgen-futures version 0.4.49 is the compatible version for wasm-bindgen 0.2.99.
18-18
: LGTM: Version pin looks good
The exact version pin (=0.2.99) is appropriate for WASM builds to ensure reproducible builds.
Let's verify version consistency across the codebase:
✅ Verification successful
LGTM: Version is consistent across the codebase
The version pin wasm-bindgen = "=0.2.99"
is the only instance of wasm-bindgen version specification in the codebase, which is appropriate since it's the only WASM build configuration. The extensive use of wasm-bindgen macros throughout the codebase confirms this is the main WASM binding configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining old versions
rg "0\.2\.86" --type rust --type toml
# Check for any mismatched wasm-bindgen versions
rg "wasm-bindgen.*0\.2\.[0-9]+" --type rust --type toml
Length of output: 165
Script:
#!/bin/bash
# Let's check for any other wasm-bindgen dependencies in other Cargo.toml files
fd "Cargo.toml" --exec rg "wasm-bindgen" {}
# Also check for any wasm-bindgen macro usage in Rust files that might need version alignment
rg "#\[wasm_bindgen" --type rust
Length of output: 65925
packages/wasm-dpp/README.md (1)
42-42
: LGTM: Documentation updated correctly
Installation instructions correctly updated to match the version in Cargo.toml. The note about version matching is particularly helpful.
.devcontainer/Dockerfile (1)
Line range hint 69-73
: LGTM: Secure and reproducible installation
The installation command is well-configured with appropriate security and reproducibility flags:
- Version matches Cargo.toml
- Uses --locked for reproducible builds
- Includes security-conscious flags
.github/workflows/tests-build-js.yml (1)
53-53
: LGTM! Version update aligns with PR objectives.
The update to wasm-bindgen-cli@0.2.99 should resolve the Alpine 3.21 build issues mentioned in #2074.
Let's verify version compatibility across the project:
✅ Verification successful
Version update is consistent across the codebase
The wasm-bindgen version 0.2.99 is properly aligned in:
- GitHub workflow file (.github/workflows/tests-build-js.yml)
- Cargo dependency (packages/wasm-dpp/Cargo.toml)
- The build script (packages/wasm-dpp/scripts/build-wasm.sh) uses the CLI without hardcoding the version
The update from 0.2.86 to 0.2.99 is consistently applied where needed, and related dependencies (wasm-bindgen-futures 0.4.49) are compatible with this version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check wasm-bindgen version consistency across the project
# Look for wasm-bindgen version specifications in Cargo.toml files and other config files
echo "Checking wasm-bindgen versions in Cargo.toml files:"
rg -A 1 "wasm-bindgen.*=" -g "Cargo.toml"
echo -e "\nChecking wasm-bindgen-cli versions in workflow files:"
rg "wasm-bindgen-cli@" -g "*.yml"
Length of output: 793
Script:
#!/bin/bash
# Let's check for any wasm-bindgen dependencies and their versions in a more comprehensive way
echo "Checking wasm-bindgen dependencies in all Cargo.toml files:"
fd Cargo.toml --exec cat {} | grep -A 1 "wasm-bindgen"
echo -e "\nChecking workflow files for wasm-bindgen-cli:"
fd "\.yml$" .github/workflows --exec cat {} | grep "wasm-bindgen-cli"
echo -e "\nChecking for any wasm-bindgen version references in package.json files:"
fd package.json --exec cat {} | grep -A 1 "wasm-bindgen"
Length of output: 1250
Script:
#!/bin/bash
# Let's check the build script mentioned in the comment
echo "Checking wasm-bindgen version in build-wasm.sh:"
cat packages/wasm-dpp/scripts/build-wasm.sh | grep -A 2 "wasm-bindgen"
# Also check for any other potential references
echo -e "\nChecking for any other wasm-bindgen version references:"
rg "0\.2\.(86|99)" --type-not toml --type-not yml
Length of output: 1051
packages/wasm-dpp/src/identity/factory_utils.rs (1)
75-76
: LGTM! Type inference simplification is consistent.
Similar to the previous change, removing explicit type annotation for public_key_wasm
maintains code consistency while relying on Rust's type inference.
README.md (1)
64-64
: LGTM! Documentation updated correctly.
The wasm-bindgen-cli version update in the installation instructions matches the changes in the codebase.
Let's verify if all relevant documentation is updated:
✅ Verification successful
Let me verify if these references in CHANGELOG.md are historical entries by checking their context.
The documentation update is correct and complete
The references to version 0.2.86 found in CHANGELOG.md are historical entries documenting past updates, which is the expected behavior for a changelog. These entries should remain unchanged as they represent the history of changes. The README.md correctly reflects the current required version (0.2.99) for new installations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to old version
# Look for potential missed documentation updates
echo "Checking for any remaining references to old version 0.2.86:"
rg "0\.2\.86" --type md
Length of output: 537
Script:
#!/bin/bash
# Get context around the matches in CHANGELOG.md
echo "Checking context of version references in CHANGELOG.md:"
rg -B 2 -A 2 "0\.2\.86" CHANGELOG.md
Length of output: 632
.github/workflows/release.yml (1)
73-73
: LGTM: Version update matches PR objective.
The wasm-bindgen-cli version update to 0.2.99 aligns with the PR's goal of fixing build issues on Alpine 3.21.
packages/wasm-dpp/src/identity/state_transition/identity_create_transition/identity_create_transition.rs (1)
103-106
: LGTM: Type handling simplification.
The removal of explicit Ref
type usage simplifies the code while maintaining the same functionality. This change aligns with the wasm-bindgen update and improves code readability.
Also applies to: 121-124
packages/wasm-dpp/src/identity/state_transition/identity_update_transition/identity_update_transition.rs (2)
Line range hint 1-28
: LGTM: Import organization looks good.
The imports are well-organized and properly grouped. The changes maintain good code organization practices.
88-91
: LGTM: Simplified type conversion logic.
The code simplifies the conversion of JavaScript values to IdentityPublicKeyInCreation
instances using the generic_of_js_val
helper function.
Dockerfile (1)
316-316
: LGTM: Version update aligns with PR objective.
The update of wasm-bindgen-cli to version 0.2.99 directly addresses the build issues on Alpine 3.21 mentioned in issue #2074.
Let's verify if this version is used consistently across the repository:
✅ Verification successful
Let me verify if there are any other files that might reference wasm-bindgen-cli version to ensure complete consistency.
Based on the search results, I can now generate the final response:
Version update is consistent across the codebase
The update of wasm-bindgen-cli to version 0.2.99 is consistent with all other references in the codebase:
- Matches wasm-bindgen dependency version in Cargo.lock (0.2.99)
- Aligns with installation instructions in README.md and packages/wasm-dpp/README.md
- Matches the version specified in packages/wasm-dpp/Cargo.toml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old version
# and confirm the new version is used consistently
# Search for any remaining references to the old version
echo "Checking for old version references..."
rg "0\.2\.86" --type-add 'docker:*.{dockerfile,Dockerfile}' --type docker
# Search for new version references to ensure consistency
echo "Checking new version references..."
rg "0\.2\.99" --type-add 'docker:*.{dockerfile,Dockerfile}' --type docker
Length of output: 391
Script:
#!/bin/bash
# Search for wasm-bindgen version references in all files
echo "Checking for wasm-bindgen version references..."
rg "wasm-bindgen" -A 1 -B 1
# Also check any cargo.toml files specifically
echo "Checking Cargo.toml files..."
fd "Cargo.toml" --exec cat {}
Length of output: 46867
packages/wasm-dpp/src/identity/state_transition/asset_lock_proof/mod.rs (2)
146-149
: LGTM: Clean type conversion using generic_of_js_val
The code effectively leverages improved type conversion utilities, making the implementation more maintainable while maintaining the same functionality.
152-155
: LGTM: Consistent implementation with InstantAssetLockProof
The code maintains a consistent pattern with the InstantAssetLockProof implementation, ensuring symmetrical handling of different asset lock proof types.
Issue being fixed or feature implemented
wasm-bindgen-cli 0.2.86 doesn't build correctly on Alpine 3.21 #2074
What was done?
Updated wasm-bindgen-cli to 0.2.99
Fixed build issues
How Has This Been Tested?
GHA
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
wasm-bindgen-cli
from version0.2.86
to0.2.99
across various components.Bug Fixes
sign_by_private_key
method to provide descriptive messages when a BLS adapter is missing.verify_signature
method.Documentation
wasm-bindgen-cli
.Refactor
IdentityUpdateTransitionWasm
class.