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

fix(nargo): Update acvm-backend-barretenberg to allow wasm backend compilation #1771

Merged
merged 2 commits into from
Jun 21, 2023

Conversation

phated
Copy link
Contributor

@phated phated commented Jun 20, 2023

Description

Problem*

Resolves

Summary*

This updates acvm-backend-barretenberg to a version that removes the wasm32 target. There seems to be conflicting dependencies between the backend and the noir_wasm crate. Since we aren't going to target wasm32 target with the backend, we can just remove it.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

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

@phated phated changed the title fix(nargo): Update acvm-backend-barretenberg to allow wasm backend co… fix(nargo): Update acvm-backend-barretenberg to allow wasm backend compilation Jun 20, 2023
@kevaundray
Copy link
Contributor

Somewhat related to this PR; perhaps this allows us to remove the hack that was placed here:

getrandom = { version = "*", features = ["js"] }

Not needed for this PR, just wanted to flag it

@phated
Copy link
Contributor Author

phated commented Jun 21, 2023

Somewhat related to this PR; perhaps this allows us to remove the hack that was placed here:

I'll try removing it. Might as well land that chore here if it isn't needed anymore.

@phated
Copy link
Contributor Author

phated commented Jun 21, 2023

getrandom workaround still needs to exist. The reason for this is similar to the reason we had to remove the wasm32 target in the backend: the noir_wasm crate is being built by cargo in the workspace, but it really should only be built by wasm-bindgen since it is not made to target anything but wasm32

@phated phated marked this pull request as ready for review June 21, 2023 17:30
@phated phated enabled auto-merge June 21, 2023 17:31
Copy link
Contributor

@kobyhallx kobyhallx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@phated phated added this pull request to the merge queue Jun 21, 2023
Merged via the queue into master with commit 97da745 Jun 21, 2023
@phated phated deleted the phated/backend-fix branch June 21, 2023 20:08
This was referenced Jun 21, 2023
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.

3 participants