Skip to content
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

Upgrade slang to 0.18.3 #1102

Merged
merged 7 commits into from
Nov 25, 2024
Merged

Conversation

ericglau
Copy link
Member

@ericglau ericglau commented Nov 20, 2024

Continuation of #1100

The upgrades-core package has a makeNamespacedInput function, where near the end of its processing, it uses Slang to try to sanitize the source code further to remove non-struct NatSpec. This previously used Slang 0.17.

Slang 0.18.x supports WASM and must be imported using await import().

To avoid converting the makeNamespacedInput function to async which would result in breaking changes, this PR instead removes the sanitization step from makeNamespacedInput and exposes an async trySanitizeNatSpec function that can be called separately. We then call this new function in the hardhat-upgrades plugin.

OmarTawfik and others added 2 commits November 14, 2024 20:57
The newest version includes many performance improvements, and adds WASM support. It also adds a bunch of requested public APIs.

This allows importing Slang from any WASM-compatible environment (NodeJS), at the cost of using ESM (or using `await import()` from CJS).
That turns the calling utility `makeNamespacedInput()` to be `async`.

Please let me know if I need to verify/run anything else.
Copy link

socket-security bot commented Nov 20, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@nomicfoundation/slang@0.18.3 None +1 2.78 MB nomic-foundation-publisher

🚮 Removed packages: npm/@nomicfoundation/slang@0.17.0

View full report↗︎

@ericglau ericglau marked this pull request as ready for review November 20, 2024 18:57
@ericglau ericglau requested a review from a team November 20, 2024 18:57
Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll keep reviewing

packages/plugin-hardhat/src/index.ts Show resolved Hide resolved
packages/plugin-hardhat/src/index.ts Outdated Show resolved Hide resolved
packages/core/src/utils/make-namespaced.ts Outdated Show resolved Hide resolved
packages/core/src/utils/make-namespaced.ts Show resolved Hide resolved
@ernestognw
Copy link
Member

Should this be released under a minor?

Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nits, I don't think they're a blocker

@ericglau ericglau enabled auto-merge (squash) November 25, 2024 22:05
@ericglau ericglau merged commit 9d3112a into OpenZeppelin:master Nov 25, 2024
11 checks passed
@ericglau ericglau deleted the upgrade-slang branch November 25, 2024 22:06
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.

3 participants