feat: build command to build optimized wasm binaries

[willemneal]

What

This PR adds a build subcommand that aims to hide the complexity from new users. It uses clap-cargo-extra which provides most of the cargo args that users are familiar with. Currently this PR relies on an unreleased version of the crate (I'm the author).

Added to the usual features is an optimize flag, which will aim to run the nightly version of cargo if it is installed. It also applies wasm-opt for size on the generated cargo binaries adding _opt.wasm suffix.

wasm-opt is provided from a dependency binaryen, which is added as optional, since the C++ building slowed things down and seemed to break rust-analyzer. Furthermore it seems to be an old version of binaryen and so the optimized binary generated differed from the most recent release. The crate looks unmaintained, however, a fork and a solution like https://www.riff.sh/, might help solve the issues above.

ctor is added as a new dependency so that a function to build is triggered before any tests. This doesn't seem to slow things down if the the test contracts aren't updated.

Lastly, the tests are reorganized so that a singe test binary is built instead of individual ones. As mentioned here.

Why

Currently the build step for optimized contract binaries is awkward for new devs and most examples use a Makefile to simplify this. It also requires that you have external binaries installed.

Known limitations

Currently depends on #222 to be merged.

Currently clap-cargo-extra doesn't handle profiles, however, this could be addressed in a future PR.

[willemneal]

While this PR is ready to be tested, as mentioned relies on an unreleased branch of clap-cargo-extra, which is why it's marked as a draft.

Furthermore, wanted to add profiles to clap-cargo-extra or perhaps again hide this detail internally and perhaps move more logic from here into that crate.

[leighmcculloch]

This is really interesting and definitely something I've been wanting explore. A couple things for us to consider:

• Portability – I think largest challenge with this command will be operating system portability. The addition of this is probably the point where we need to add back the CI builds for mac and windows that we removed a little while ago. The goal of the builds would be to ensure that the binaryen bindings and the shell-outs work consistently with a variety of Rust installations (rustup, simple binary installs, nix installs, ubuntu installs, etc) on these systems.

• Necessity – This encourages use of nightly, but nightly is something we're only using to get a specific type of optimization (removing panic strings from panic!() calls), and we have another effort in progress we're experimenting with (panic_error!()) that very well could remove the need for that optimization. We're also finding we often don't need these optimizations, definitely at the moment with futurenet. So we've got a reasonable amount of time before an average developer actually needs this functionality. If we're encouraging developers to go deep on these optimization early their experimentation, we should probably stop doing that.

We should definitely keep exploring this though.

(Related stellar/soroban-docs#205)

[willemneal]

• portability - Agreed, however, for now it also doesn't prevent power users from building their own way. The only tricky aspect is that is expects a cargo in the path that is a proxy that then calls a stable bin by default or +nightly etc.

• necessity - the use of the optimizations is already used in the example repos, and while I agree that it's overkill (premature optimization is the root of all evil as they say), especially binaryen, it will be needed in the future as contract sizes grow. The optimization is optional and as the test example shows it already provides the functionality of ensuring the the fixtures are rebuilt before each test run.

Also aptly, it just failed CI since nightly wasn't installed, so just made it a feature and make a separate test for it.

[leighmcculloch]

I think if we can use the wasm-opt crate that we're adding in #236 in this PR, it should make the build process simpler. I think @willemneal is already planning that given comments on #236.

Before we commit to adding a build command to the CLI, I'd like to explore if we can remove the need for rust nightly all together, which it appears is the main benefit we get out of a build command.

The reason I'm concerned about our use of nightly:

• We're encouraging using stable for testing and nightly for releasing, which in a production environment could be harmful. Tests and releases should ideally be the same as much as possible. So if we're going to stay using nightly we probably need to encourage its use for all development.
• In general running on bleeding edge Rust could be risky, as there could be changes that haven't had anytime to go through the beta process or had bugs found.
• Running on bleeding edge Rust runs the risk of causing developer problems, for the same reasons as above, if we're unlucky enough to see a bug that affects a build process.

For those reasons we have motivation to get off nightly, or to find another way to provide the same optimizations without needing nightly.

Getting developers to use soroban to build also has a cost of moving developers away from the natural Rust build tools. So far we've tried very hard to provide a "you're just writing Rust" experience. I don't think this is a hard requirement, but I'm cautious.

That all said, if we can't get rid of nightly, a build subcommand would make things much simpler for new developers, especially because we could do things potentially like run a specific version of nightly that aligns with the current stable release to reduce some risks.

Definitely interested in everyones thoughts on this.

(cc @graydon I would be very interested in picking your brain about alternative ways we can achieve the same panic_immediate_abort optimizations without nightly, and/or if you think my concerns about nightly are unwarranted.)

[graydon]

@leighmcculloch I'm not super worried about users using nightly for their production wasm builds. It's obviously not ideal, and I think we should make it clear what users are trading off (size vs. stability) but:

• WASM is sandboxed. A miscompiled contract can't hurt the host or any other unrelated contracts, the worst it can do is make a logic error. Of course this could have financial consequences to the contract or its users, but so could any bug in contracts, or bugs in the SDK, or bugs in libraries or other contracts they depend on. The stability of the compiler they use is just one factor.
• Rust nightly is -- certainly by the standards of "stuff people throw into production on blockchains" -- a fairly stable and fairly high quality piece of software. Lots of rust ecosystem libraries depend on nightly features, and all the release channels ultimately derive from the same repo, so any serious problem in a nightly build is fairly quickly noticed and reverted. The main difference between nightly and non-nightly isn't quality so much as whether users can access unstable features that have no long-term stability guarantee, i.e. whether code you compiled with nightly today might still compile with nightly tomorrow. And that depends entirely on which nightly-gated features or APIs you use.
• We're targeting a fairly small, conservative, non-bleeding-edge subset of the rust language feature-space and standard library API-space with soroban contracts. We're not using fancy new stuff in the traits or async system, or fancy new APIs in std -- we build most contracts no_std entirely! So my sense is that there's a fairly high intersection between "the language subset we're using" and "the language subset that has fairly high coverage with basic pre-landing integration tests, and therefore literally isn't allowed to regress at all".

In summary: I don't think it's something we need to lose sleep over users using, and they're probably going to do so by choice whether or not we pave the path for them / make it simple / build it into a command.

I would recommend we make it an option they have to choose, and maybe even default to stable, because there is a tradeoff here and users might wish to play it super safe. Same with a variety of codegen options! You can get smaller contracts if you turn off overflow checks. Some users will. Should we make it the default? Probably not. But we can't stop them, or even really make it especially hard.

In terms of whether we can achieve the same panic-shrinking without building std with panic_immediate_abort -- we tried this and it was .. disagreeable. IIRC I was actually the proponent! But also it felt bad. I'm willing to try again, but I think it's largely on "your" side (the SDK) not "mine" (the host)! The host is written not to panic at all, except during local testing. The question is one of developer UX. When we avoided the panic!() macro, IIRC we found:

• Common rust idioms (unwrap / expect / panic) had to be avoided, which felt bad from a training perspective.
• It was easy to get an obscure / confusing build error due to misconfiguring feature flags and targets.
• We struggled to use any common (even no_std) libraries that did panics in certain cases anyways.

Personally I can .. also maybe live with these, especially if the nightly concern is strongly felt. Maybe with the recent SDK-side changes to using Result more extensively, you don't feel as inclined to panic!() or unwrap everywhere. Maybe you can configure the SDK or soroban build command in such a way that nobody gets the obscure missing-panic-handler or duplicate-panic-handler errors. I'm not sure. I feel like it's .. a bit more your call.

[leighmcculloch]

the worst it can do is make a logic error

This is my concern.

any serious problem in a nightly build is fairly quickly noticed and reverted

This is exactly my concern. When using nightly a developer might be using the release from yesterday that hasn't had time to "simmer" where bugs are found quickly and patched before the release makes it to beta/stable.

When we avoided the panic!() macro

I don't want to avoid the panic macro. As you said we went there, and it wasn't great. What I'm curious about is if we can find another way to reduce the code generated by panic without requiring nightly.

I agree whatever we do, we probably shouldn't require nightly, and also, we shouldn't prevent people from using it. I'll think about this some more on how we can present these options well in docs, etc, and if we can do anything differently.

[paulbellamy]

Closing this, because we now have soroban contract optimize command.