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: Update backend dependency containing updated pk write fix #956

Merged
merged 6 commits into from
Mar 13, 2023

Conversation

vezenovm
Copy link
Contributor

@vezenovm vezenovm commented Mar 6, 2023

Related issue(s)

Resolves #905

Description

The context for this PR can be found in the issue and the downstream noir-lang/aztec-connect PR: noir-lang/aztec-connect#26. Once this is approved and merged I can merge the aztec_backend PR linked below, and then finally this PR.

I also made the relevant change to upstream bb: AztecProtocol/barretenberg#204.

Backend PR: noir-lang/acvm-backend-barretenberg#78

Summary of changes

All changes can be found in the downstream PRs.

Dependency additions / changes

The aztec_backend dependency needs to be updated to reflect the changes to how we write the proving key to buffer in C++.

Test additions / changes

We are testing checking whether the Noir CI still passes as usual.

I also tested on the poseidonsponge_x5_254 test example linked in issue #905 that this PR resolves. It is currently no longer giving a buffer serialization error on nargo prove p, but proof verification is returning false. This looks like it may be a separate error with the impl or a bug in Noir though.

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

@vezenovm vezenovm requested a review from guipublic March 6, 2023 23:36
@kevaundray
Copy link
Contributor

We probably want to make this change to the upstream barretenberg now and not the noir-lang/aztec-connect branch

We are waiting for this PR to merge which is blocked by a PR to remove the lagrange SRS

@kevaundray
Copy link
Contributor

You could possibly merge into that branch and wait for it to get merged

@vezenovm
Copy link
Contributor Author

vezenovm commented Mar 7, 2023

We probably want to make this change to the upstream barretenberg now and not the noir-lang/aztec-connect branch

We are waiting for this PR to merge which is blocked by a PR to remove the lagrange SRS

Just to follow on discussion from scurm: I also submitted the same changes I made to noir-lang/aztec-connect in upstream bb. This is branched off of the PR you linked: AztecProtocol/barretenberg#204. I did both as I wanted to get this fix in and the upstream bb PRs are currently blocked from being merged. We will see the time-delay with getting the upstream bb PRs merged before merging this fix using the noir-lang/aztec-connect fork.

@kevaundray
Copy link
Contributor

Since this stops users from making circuits above 100K and there are no breaking changes as of yet, we can push another release once this is merged -- apologise for missing this as it was you brought this to my attention a week ago

@vezenovm vezenovm added this pull request to the merge queue Mar 13, 2023
Merged via the queue into master with commit 5d627a7 Mar 13, 2023
@vezenovm vezenovm deleted the mv/fix-pk-write branch March 13, 2023 21:33
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.

Buffer overflow serializing proving key
2 participants