Skip to content

Use published acvm v0.1.0#623

Closed
phated wants to merge 2 commits intomasterfrom
bb/acvm-version
Closed

Use published acvm v0.1.0#623
phated wants to merge 2 commits intomasterfrom
bb/acvm-version

Conversation

@phated
Copy link
Copy Markdown
Contributor

@phated phated commented Jan 9, 2023

Related issue(s)

Resolves #596

Description

Summary of changes

This PR swaps out the avcm dependency from the noir-lang/noir repo for the v0.1.0 version published on crates.io - The 0.1.0 version should be compatible with the current version of noir, but it is published from a separate repository.

Dependency additions / changes

The published acvm crate.

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.

Additional context

This PR is needed in combination with noir-lang/acvm-backend-barretenberg#36 to successfully install nargo with a completely fresh clone.

@phated phated requested review from kevaundray and vezenovm January 9, 2023 23:46
Copy link
Copy Markdown
Contributor

@guipublic guipublic left a comment

Choose a reason for hiding this comment

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

Before switching to the new acvm, we should first:
merge current PRs that modify acvm
review the new acvm changes
update the new acvm with the internal acvm

@kevaundray
Copy link
Copy Markdown
Contributor

  • The main PR open on this repo that modifies acvm is Binary solver for array equality constraints #520. We can migrate those changes over to the standalone repo.

  • Happy to jump on a call to speak about the new acvm changes. You can also check the CHANGELOG for a quick summary.

  • I think we can do this in two PRs. First merge the 0.1 version of acvm without any changes and then merge 0.3 which does have changes and will require modifications in other places. The first is easier to do and will stop any new PRs from modifying ACVM in this repo.

@phated
Copy link
Copy Markdown
Contributor Author

phated commented Jan 10, 2023

Now that I have that native backend building on Mac, I tried to see debug the CI problems locally and none of them appear when I run cargo check and cargo clippy 🤔

@phated
Copy link
Copy Markdown
Contributor Author

phated commented Jan 10, 2023

I had not updated the lockfile, so I tried doing that. It still points at some acir and acvm github references (assumed from the dep graph)

@phated
Copy link
Copy Markdown
Contributor Author

phated commented Jan 18, 2023

Replaced by #647

@phated phated closed this Jan 18, 2023
@phated phated deleted the bb/acvm-version branch May 11, 2023 20:18
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.

Switch to crates.io version of acvm

3 participants