wasm32 support for twenty-first by dan-da · Pull Request #265 · Neptune-Crypto/twenty-first

dan-da · 2025-06-19T18:20:02Z

Adds wasm32 support. See included README files:

README-wasm32.md --> usage instructions, aimed at users of twenty-first.
README-wasm32-dev.md --> details of the changes to enable wasm32 support, aimed at twenty-first devs/contributors.

This PR is a series of commits. The very first commit only changes Cargo.toml and is sufficient to enable wasm32 support for cargo build meaning that crates that depend on twenty-first can use it in their wasm32 builds. So it can be considered the "bare minimum" for wasm32 support.

The remainder of the commits mainly tackle the challenge of getting our unit tests and especially proptests working in the wasm32 environment, which is substantially more complicated.

There are also a couple of commits that address failing test cases. One of these failing tests found a problem with twenty-first when executing in a 32-bit environment. This is addressed in 19a11a4, and should be carefully reviewed for correctness. This is the only change that was made to non-test library code in any of the commits, afaik.

This PR may be easiest to grok by:

reading the two README files.
reviewing each commit, in sequence from oldest to newest.

note: the README files were generated by Gemini LLM, with coaching and minor edits. Also the LLM was very helpful for navigating through the complex issues getting the tests to build and execute.

makes twenty-first "wasm-friendly" such that it builds when target_arch is wasm32. This enables twenty-first to be used inside a web-browser's web-assembly environment. Tests do not presently build for wasm32 target. Addressing that is more complicated and will be the focus of subsequent commits.

This adds the necessary Cargo.toml magic for `cargo test` to build tests correctly, both #[test] and #[proptest]. However the wasm test harness ignores #[test] and #[proptest] so tests are not actually run in that environment. The source files will need to be modified to make the tests visible to wasm test harness in a future commit.

annotates tests to indicate they should be compiled for wasm32 target by decorating with #[wasm_bindgen_test] when target_arch = wasm32.

It remains 32 on 64 bit systems. This avoids isize::MAX overflow for 32 bit systems and fixes a failing test case

The test uses assert_snapshot() which performs wasm32 disallowed operation(s)

Sword-Smith · 2025-06-19T20:04:51Z

👀 Very interesting! Would be really cool if we could get our dependencies to all run on wasm-32.

What I've seen looks good, with minimal changes/decorations to get this to work. I'll let others weigh in though, as devops is not my forté.

jan-ferdinand

Thanks for taking this on. I've left a number of comments inline. Additionally:

I can't wasm-pack build because apparently, “crate-type must be cdylib to compile to wasm32-unknown-unknown”. The suggestion is to add crate-type = ["cdylib", "rlib"] to the Cargo.toml's [lib] section. I don't know what the implications of this are. Do you?
Running wasm-pack test --node (which works) results in one test failure, in particular, ntt_on_empty_input. Running cargo t ntt_on_empty_input results in a pass. Seems like we need to do some debugging.
Before merging, I'd like to have a CI script that builds twenty-first for a wasm target and executes the full test suite successfully.

jan-ferdinand · 2025-06-25T10:55:10Z

-proptest-arbitrary-interop = "0.1"
 test-strategy = "0.4"
 trybuild = "1.0"
+proptest-arbitrary-interop = {git="https://github.com/dan-da/proptest-arbitrary-interop.git", version = "0.1", rev="d9fcf5b"}


Let's hope that your PR on the upstream crate gets merged, but I'm not holding my breath. In order to publish twenty-first on crates.io, we must not have git dependencies per their policy. There are two obvious ways forward, and maybe there are more:

We (or you) publish your fork as a new crate to crates.io, twenty-first then depends on that crate.

We copy the entire content of your fork into twenty-first and remove the dependency. (We should publicly expose this for downstream usage.)

I don't like either method too much. I think I slightly favor the latter approach. Perhaps you see a better option still?

yeah, I don't really like either option. I just added a comment to that PR asking if its likely to be merged soon. If no reply by end of week, I suppose we should move forward with one of the two options.

dan-da · 2025-06-25T17:26:22Z

Thanks for taking this on. I've left a number of comments inline. Additionally:

I can't wasm-pack build because apparently, “crate-type must be cdylib to compile to wasm32-unknown-unknown”. The suggestion is to add crate-type = ["cdylib", "rlib"] to the Cargo.toml's [lib] section. I don't know what the implications of this are. Do you?

well, I just learned:

I've been building a Rust-internal Wasm library (rlib) with cargo build --target wasm32-unknown-unknown. This is usable by the test harness.
You were trying to build a web-ready package with wasm-pack. That requires cdylib format, a stable ABI that a javascript runtime can load.

Apparently adding crate-type = ["cdylib", "rlib"] is the standard way to tell Cargo to produce both formats: the rlib needed for cargo test to work, and the cdylib needed for wasm-pack to build the final product.

Hopefully we can make this specific to the wasm32 build.

Running wasm-pack test --node (which works) results in one test failure, in particular, ntt_on_empty_input. Running cargo t ntt_on_empty_input results in a pass. Seems like we need to do some debugging.

I observed same/similar. Though I was using cargo build --target wasm32-unknown-unknown.

However I also saw multiple test failures running cargo test against unmodified master branch on my machine. The exact observations were:

some tests would fail when run in parallel with cargo test
Those same tests would succeed when run individually.
Some or all of them would also fail when run with cargo test -- --test-threads 1.

This is novel behavior and I concluded that perhaps there is some shared state happening between tests. But since it was happening on master I put it firmly in the "not my problem" box. If anything, the wasm32 tests behaved better under the wasm test harness, because only one of them failed when run with cargo test --target wasm32-unknown-unknown.

Regardless, my position is that:

the very first commit introduces basic wasm support, and is sufficient to enable crates that depend on twenty-first to build wasm32 target. So if only that commit gets merged, it is already a win and all I really need for wallet purposes.
getting tests to work with wasm32 was already a substantial effort, outside my present area of focus. I did that work to "help out", but I am not inclined to spend much more effort on it nor to debug a single flaky test (that passes when run by itself) further. I don't think that should be a showstopper for this PR, but that's not my call. Stated differently, I don't think it is demonstrated that the (very mechanical) wasm32 changes cause the test failure; rather the test suite itself may have issues.

Before merging, I'd like to have a CI script that builds twenty-first for a wasm target and executes the full test suite successfully.

I consider CI scripts to be outside scope of this PR. Anyway, it is not a skill I possess. I would suggest that could be done in a followup and should not block merging this PR.

jan-ferdinand · 2025-06-26T09:11:05Z

As per discussion with @dan-da, we're splitting this PR in two. The PR's first commit is merged into master via 1aeb2a2, the remaining commits will be merged (in due time) via #269.

Thanks, @dan-da, for getting tests to work on wasm32-unknown-unknown!

dan-da added 6 commits June 19, 2025 11:03

test: tests run in wasm32 test harness

8dfb17d

annotates tests to indicate they should be compiled for wasm32 target by decorating with #[wasm_bindgen_test] when target_arch = wasm32.

fix: set NUM_DOMAINS to 29 on 32 bit systems

da31c1c

It remains 32 on 64 bit systems. This avoids isize::MAX overflow for 32 bit systems and fixes a failing test case

test: disable tip5_hasher_trait_test for wasm32

953b922

The test uses assert_snapshot() which performs wasm32 disallowed operation(s)

docs: add README files for wasm32

d960e06

dan-da force-pushed the wasm_feature_with_tests branch from 9caacb6 to d960e06 Compare June 19, 2025 18:47

dan-da requested a review from jan-ferdinand June 20, 2025 15:26

jan-ferdinand requested changes Jun 25, 2025

View reviewed changes

jan-ferdinand force-pushed the master branch from f650d1b to 17a5683 Compare June 25, 2025 12:34

jan-ferdinand mentioned this pull request Jun 26, 2025

Run tests on wasm32 architecture #269

Merged

5 tasks

jan-ferdinand closed this Jun 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

wasm32 support for twenty-first#265

wasm32 support for twenty-first#265
dan-da wants to merge 6 commits into
Neptune-Crypto:masterfrom
dan-da:wasm_feature_with_tests

dan-da commented Jun 19, 2025 •

edited

Loading

Uh oh!

Sword-Smith commented Jun 19, 2025 •

edited

Loading

Uh oh!

jan-ferdinand left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jan-ferdinand Jun 25, 2025

Uh oh!

dan-da Jun 25, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dan-da commented Jun 25, 2025

Uh oh!

jan-ferdinand commented Jun 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dan-da commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sword-Smith commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jan-ferdinand left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jan-ferdinand Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

dan-da Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dan-da commented Jun 25, 2025

Uh oh!

jan-ferdinand commented Jun 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dan-da commented Jun 19, 2025 •

edited

Loading

Sword-Smith commented Jun 19, 2025 •

edited

Loading