Skip to content
This repository has been archived by the owner on Aug 22, 2023. It is now read-only.

fix: WASM provided by nix build of barretenberg #24

Merged
merged 20 commits into from
Apr 28, 2023
Merged

fix: WASM provided by nix build of barretenberg #24

merged 20 commits into from
Apr 28, 2023

Conversation

kobyhallx
Copy link
Contributor

Related issue(s)

#23

Resolves #

Description

Switching Noir to barretenberg upstream introduced requirement for Barretenberg WASM to be located at $BARRETENBERG_BIN_DIR. Absence of this variable breaks builds.

Summary of changes

libbarretenberg-wasm32 is built by nix from upstream AztecProtocol/barretenberg, uploaded to artifacts and later downloaded and reused by workflows.

Dependency additions / changes

libbarretenberg-wasm32

Test additions / changes

N/A

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

Documentation needs

  • This PR requires documentation updates when merged.

Additional context

@kobyhallx kobyhallx requested a review from phated April 26, 2023 17:45
phated
phated suggested changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

This is a great solution. We need to figure out how to pin bb to the commit noir uses though, perhaps using the lockfile?

.github/workflows/publish-apple-darwin-wasm.yml Outdated Show resolved Hide resolved
.github/workflows/publish-apple-darwin-wasm.yml Outdated Show resolved Hide resolved
@TomAFrench TomAFrench mentioned this pull request Apr 26, 2023
Merged
fix: barretenberg pinning
d09e35f 
- Barretenberg pinned in nix-barretenberg.yml
- Barretenberg artfcats uploaded to be shared across workflows
- system specific workflows triggered by workflow_run - sucess
@kobyhallx kobyhallx requested a review from phated April 28, 2023 15:51
phated
phated suggested changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

Question about your choice about an unofficial action when an official one exists. Maybe there is a reason we can't use official version?

.github/workflows/publish-apple-darwin-wasm.yml Show resolved Hide resolved
phated
phated suggested changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

Let's add this comment to each workflow.

.github/workflows/publish-apple-darwin-wasm.yml Show resolved Hide resolved
.github/workflows/publish-linux.yml Outdated Show resolved Hide resolved
.github/workflows/publish-x86_64-pc-windows-wasm.yml Show resolved Hide resolved
phated
phated approved these changes Apr 28, 2023
View reviewed changes
Copy link
Contributor

@phated phated left a comment

Choose a reason for hiding this comment

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

We need to update Noir to dispatch to this new workflow.

@phated phated merged commit c72bb8d into master Apr 28, 2023
@phated phated deleted the kh-wasm-from-nix branch April 28, 2023 17:10
@phated phated mentioned this pull request Apr 28, 2023
Merged
6 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TomAFrench TomAFrench TomAFrench left review comments

@phated phated phated approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kobyhallx @phated @TomAFrench