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

feat(test): Enable the test fuzzer for Wasm #6835

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Dec 17, 2024

Description

Problem*

Resolves #6738

Summary*

  • Upgrades proptest to 1.6 to be able to use u128 and i128 strategies with Wasm
  • Creates the UIntStrategy with max 64 bit size, still returning u128
  • Creates the IntStrategy with max 64 bit size, still returning i128
  • Enables the FuzzedExecutor for #[test]s with inputs on Wasm
  • Disables the fork and timeout features, otherwise it doesn't seem to compile for Wasm (the compiler needs to be told if the target OS is unix or windows).

Additional Context

The ticket says:

we've since restricted the type system to only allow u64s as the maximum integer type.

I couldn't find where exactly this happened, it would be nice to include a link.

I started with using u64 instead of u128, and i64 instead of i128; the latter covers half the range as if we used u64 and mapped to negative or positive based on whether the random start value we picked was negative or positive. I did it that way because proptest enabled i128 and u128 for Wasm just last week, and I thought it would be easier to to go back to using i128 when it's released. Then I noticed that proptest 1.6 was actually just released yesterday, and it includes the above change, it just didn't have a tag in the repo 🎉

On a different note this shifting makes it look like we don't even want negative values for signed numbers, so maybe it would be easier to just use the UIntStrategy without fixtures and covert the returned value to i128.

Compilation

CI uses wasm-pack, but we can also compile it locally with:

cargo build -p noir_wasm --target wasm32-unknown-unknown

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

Copy link
Contributor

github-actions bot commented Dec 17, 2024

Peak Memory Sample

Program Peak Memory
keccak256 78.48M
workspace 123.64M
regression_4709 422.91M
ram_blowup_regression 1.58G
private-kernel-tail 201.52M
private-kernel-reset 716.79M
private-kernel-inner 291.60M
parity-root 171.90M

Copy link
Contributor

github-actions bot commented Dec 17, 2024

Compilation Sample

Program Compilation Time %
sha256_regression 1.392s 1%
regression_4709 0.797s 0%
ram_blowup_regression 15.927s 2%
rollup-base-public 114.219s 2%
rollup-base-private 98.511s 3%
private-kernel-tail 1.030s -4%
private-kernel-reset 7.634s 2%
private-kernel-inner 2.259s -1%
parity-root 0.734s -1%
noir-contracts 86.535s -3%

@aakoshh aakoshh marked this pull request as draft December 17, 2024 11:52
@aakoshh aakoshh marked this pull request as ready for review December 17, 2024 12:38
Copy link
Contributor

Execution Sample

Program Execution Time %
sha256_regression 0.621s -3%
regression_4709 0.405s 1%
ram_blowup_regression 4.414s -3%
rollup-base-public 21.175s -2%
rollup-base-private 19.359s -3%
private-kernel-tail 0.696s -3%
private-kernel-reset 1.483s -1%
private-kernel-inner 0.973s -4%
parity-root 0.521s 0%

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.

Support fuzz tests in wasm
2 participants