-
Notifications
You must be signed in to change notification settings - Fork 22
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
Expand mention to suitability of the builder #70
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Nice!
Not-a-blocker note that might be worth a follow-up PR
SHORT VERSION:
I'd suggest adding something along the lines of:
For native builds, run:
- cd contracts/my-contract && RUSTFLAGS="-C link-arg=-s" cargo build --release --lib --target=wasm32-unknown-unknown
- wasm-opt -Os --signext-lowering "target/wasm32-unknown-unknown/release/my-contract.wasm" -o "artifacts/my-contract.wasm"
LONG VERSION / MOTIVATION
For local dev what many people do is avoid the docker image entirely, and rather just use native cargo
and wasm-opt
, like the comment and script linked earlier: CosmWasm/optimizer#152 (comment)
I have another example in Taskfile syntax here: https://github.com/dakom/ibc-chat/blob/ff384141fc58018d6ca0b55bf1fbd579451c0e9a/Taskfile.yml#L72
Tbh I got more into that habit on apple silicon a long time ago where the Docker translation layer was a massive slowdown... at that time it wasn't just a "nice way to save a few milliseconds", it was crucial - but since then there were arm images and now it "just works" on arm.. maybe it's less of an issue.
Many people don't understand that the docker optimizer is not necessary at all for development or optimization, it's only really necessary for reproducibility - in other words, people think if they use cargo/wasm-opt directly it's "less optimized" because they didn't use the cosmwasm tool - it's actually a fairly common misconception!
@dakom Added your suggestion. Also I moved everything into a collapsible because the callout got a little too long |
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!
Added a couple suggestions in comments that are purely subjective nitpicks. Absolutely feel free to ignore or accept at your discretion :)
Co-authored-by: David Komer <[email protected]>
Co-authored-by: David Komer <[email protected]>
Closes #66