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

feat!: Implement CommonReferenceString trait #139

Merged
merged 21 commits into from
May 16, 2023

Conversation

phated
Copy link
Contributor

@phated phated commented Apr 26, 2023

This is a draft implementation of noir-lang/acvm#231 to see how the API worked out.

Closes #160
Closes #162

constraint_system: &ConstraintSystem,
witness: WitnessAssignments,
proving_key: &[u8],
) -> Vec<u8> {
let circuit_size = self.get_circuit_size(constraint_system);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do like this switch away from having to serialize a circuit to/from bberg for every composer method in order to fetch the circuit size and thus the data from the CRS. For methods such as create proof we have been forced to serialize the entire circuit to bberg to fetch the circuit size, and then once again to perform the main action of the method.

Is there anyway to persist the reference string in the Barretenberg struct once we call get_reference_string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't like the idea of persisting it on the struct because then you have to do like Option<CRS> and check to see if you have it in each function. The idea behind get_reference_string is that it won't be called by nargo if Nargo already has a cached version that is large enough for the circuit, so we'd have to check/cache it in each function, which seems messy.

acvm_backend_barretenberg/src/composer.rs Outdated Show resolved Hide resolved
acvm_backend_barretenberg/src/composer.rs Outdated Show resolved Hide resolved
@phated phated changed the base branch from master to phated/fallible May 2, 2023 22:53
@phated phated linked an issue May 5, 2023 that may be closed by this pull request
1 task
@phated phated mentioned this pull request May 5, 2023
1 task
Base automatically changed from phated/fallible to master May 10, 2023 10:25
@phated phated changed the base branch from master to acvm-0.12.0 May 11, 2023 00:06
@phated phated changed the title feat!: Implement ReferenceString trait on the backend feat!: Implement CommonReferenceString trait May 11, 2023
src/crs.rs Outdated Show resolved Hide resolved
@phated
Copy link
Contributor Author

phated commented May 11, 2023

I'm see a lot of DNS failures on Linux. Do we know why or any way to avoid them? Is there a misconfiguration on the AWS bucket where the CRS lives? Or is it a problem with the Reqwest library (searching the internet for the error shows many similar errors from Reqwest...)?

We wouldn't have seen these errors with the old implementation because we were caching in CI and then we were using Nix to fetch.

@phated phated marked this pull request as ready for review May 12, 2023 15:36
cspell.json Outdated Show resolved Hide resolved
flake.nix Show resolved Hide resolved
@phated phated requested a review from kobyhallx May 12, 2023 23:33
@phated phated linked an issue May 15, 2023 that may be closed by this pull request
1 task
@TomAFrench TomAFrench merged commit 04c9d0f into acvm-0.12.0 May 16, 2023
@TomAFrench TomAFrench deleted the phated/acvm-ref-string branch May 16, 2023 05:03
TomAFrench added a commit that referenced this pull request May 19, 2023
* feat!: update to target acvm-84b5d18d

* chore: update to use new black box solver interface

* use patch syntax

* update to latest changes

* feat!: update to acvm with non-homogeneous bb calls (#169)

* feat: update acvm

* feat!: updated acvm to latest master

* chore: update cargo toml

* Update src/barretenberg_structures.rs

Co-authored-by: Tom French <[email protected]>

---------

Co-authored-by: Tom French <[email protected]>

* fix bad rebase

* feat!: Update backend to rely on WitnessMap (#152)

Co-authored-by: Tom French <[email protected]>

* update acvm

* feat!: Implement CommonReferenceString trait (#139)

* chore!: Remove filesystem access and utilize async fetch with range requests

Co-authored-by: Tom French <[email protected]>

* chore: Switch to tokio test macro for async function (#191)

* chore: Remove `sled` & `tempfile` dev-dependencies (#190)

* chore: Remove sled & tempfile dev-dependencies

* chore: space out functions

---------

Co-authored-by: Tom French <[email protected]>

* chore: bump to crates.io release

* Update src/crs.rs

* chore: Remove streaming CRS download & indicator (#194)

* Update src/acvm_interop/common_reference_string.rs

* code review

---------

Co-authored-by: Blaine Bublitz <[email protected]>
Co-authored-by: Álvaro Rodríguez <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CRS should fetch only what is needed Remove filesystem operations from the backend
4 participants