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.

Sign up for GitHub

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

Jump to bottom

fix!: Barretenberg binaries now take in the encoded circuit instead of a json file #1618

Merged
merged 8 commits into from
Aug 18, 2023
Merged

fix!: Barretenberg binaries now take in the encoded circuit instead of a json file #1618

merged 8 commits into from
Aug 18, 2023

Conversation

kevaundray
Copy link
Contributor

@kevaundray kevaundray commented Aug 17, 2023 •
edited
Loading

PreprocessedProgram is a Noir structure while backends do not have awareness of Noir. The circuit structure is an ACVM structure that the backend does have awareness for. This should not change much in terms of observable behaviour, the code was previously extracting the circuit structure anyways and only using that.

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

  • If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
  • I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
  • Every change is related to the PR description.
  • I have linked this pull request to relevant issues (if any exist).

@kevaundray
Copy link
Contributor Author

kevaundray commented Aug 17, 2023 •
edited
Loading

Unfortunately, this soft blocks us from making a release because:

  • This change needs to also be reflected on the bb.js binary API
  • The bb.js binary does not seem to be running any tests at the moment and is not installable

This PR is useful as it allows us to essentially move away from the forked branch in the old barretenberg sooner. We do not lose functionality since bb.js is not working right now anyways. Fixing this has been delegated to Koby, I'll open up an issue for this

EDIT: Tests run on the CI but not locally -- I'll debug using CI

@TomAFrench TomAFrench linked an issue Aug 18, 2023 that may be closed by this pull request
Update bb binary to accept Circuits rather than PreprocessedPrograms #1647
Closed
@TomAFrench TomAFrench self-assigned this Aug 18, 2023
@TomAFrench TomAFrench mentioned this pull request Aug 18, 2023
Closed
10 tasks
@kevaundray
Copy link
Contributor Author

This should still fail, since ts/bin-test/target is still using main.json.

Since this PR is already breaking, we should modify the flag for what was previously jsonPath to -b for bytecode and change all references of jsonPath to bytecode

@kevaundray
Copy link
Contributor Author

This should still fail, since ts/bin-test/target is still using main.json.

Since this PR is already breaking, we should modify the flag for what was previously jsonPath to -b for bytecode and change all references of jsonPath to bytecode

This should be done now -- there was a mixture of camel and snake case in places that I chose to not break and leave for another PR, since this is likely in quite a few places outside the scope of this PR's domain

@kevaundray kevaundray marked this pull request as ready for review August 18, 2023 21:15
@kevaundray kevaundray merged commit 4bc551e into master Aug 18, 2023
1 check passed
@kevaundray kevaundray deleted the kw/bytecode-only-artifacts branch August 18, 2023 21:47
@github-actions github-actions bot mentioned this pull request Sep 8, 2023
Merged
codygunton pushed a commit that referenced this pull request Jan 23, 2024
@PhilWindle @joss-aztec
Pw/new bridge subsidy (#1618)
f981c3e 
* Initial work on new subsidy model

* WIP

* WIP

* WIP

* WIP

* Updated metrics and blockchain status

* WIP

* WIP

* WIP

* Reverted bridge clients update

* Merge fixes

* Merge fixes

* Merge fixes

* Merge fixes

* WIP

* Test fixes

* Test fix

* Test fix

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* Ignore virtual assets. Gas override should be optional

* WIP

* Cache data provided by the data provider to prevent constant requests

* Fix virtual assets

* Test fix

* WIP

* Cleanup

* Fixed issue around the timing out of bridge interactions

* Review changes. Tests around the BridgeDataProvider

* New e2e test for bridge subsidies

* Added e2e test to circle ci config

* Minor test change

* Fixed comment

* Use of criteria in bridge data provider

* Updates to bridge subsidy provider

* Bug fix

* Added ability to query bridge publish stats

* Test fix

* Deployment work

* On frontent display batch times as:
- if fasttrack -> next rollup
- otherwise -> average timeout

* accidental debugging config commit

* Fixed merge error

* Updated SDK/Falafel versions

* Fix compiler error

* Fix

* Update comment

* Updated Terraform for the data provider in Falafel

Co-authored-by: joss <joss@aztecprotocol.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vezenovm vezenovm vezenovm approved these changes

Assignees

@TomAFrench TomAFrench

Labels
None yet
Projects
A3
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update bb binary to accept Circuits rather than PreprocessedPrograms
3 participants
@kevaundray @vezenovm @TomAFrench