-
Notifications
You must be signed in to change notification settings - Fork 699
feat: add defaults and public functions in clarity-vm for better tooling #6268
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
feat: add defaults and public functions in clarity-vm for better tooling #6268
Conversation
I just saw #6239, which is super promising for clarinet. The two PRs enable similar things (compile to wasm), but also, this specific one (6268) add some changes needed specifically for clarinet. |
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.
🎉
7ece404
to
94c13c9
Compare
94c13c9
to
101a3bb
Compare
@obycode |
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.
👍
Thanks @Jiloc! |
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.
lgtm!
Codecov ReportAttention: Patch coverage is
❌ Your project status has failed because the head coverage (64.82%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #6268 +/- ##
===========================================
- Coverage 67.41% 64.82% -2.60%
===========================================
Files 545 545
Lines 347403 347411 +8
Branches 323 323
===========================================
- Hits 234210 225217 -8993
- Misses 113185 122186 +9001
Partials 8 8
... and 374 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
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.
👍
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.
I tested it on my end and unfortunately cargo check -p clarity --no-default-features --target wasm32-unknown-unknown
is currently now working. #6238 made into develop before this PR and removed all unnecessary default-features of the crates for supporting both wasm32 in javascript and deterministic environments.
In the comments I laid out a possible fix, which is to propagate the wasm-web feature from the clarity create to the stacks-common one and use the following command to built it:
cargo check -p clarity --no-default-features --features wasm-web --target wasm32-unknown-unknown
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.
Great, LGTM!
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.
lgtm
7c07804
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Fix: #5891
With the PR, the clarity-vm can be compiled to wasm32-unknown-unknown, as it is being using the clarinet JS SDK.
It mostly hide a few things behind compilation flags.
This PR also includes a few changes that are need for clarinet. Mostly by making public certain functions.
WIth this PR, this command passes:
Once this is merged, this check should be added to the CI.
Checklist
docs/rpc/openapi.yaml
andrpc-endpoints.md
for v2 endpoints,event-dispatcher.md
for new events)clarity-benchmarking
repobitcoin-tests.yml