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

chore: Remove usage of acvm::default_is_opcode_supported #1366

Merged
merged 16 commits into from
May 23, 2023
Merged

chore: Remove usage of acvm::default_is_opcode_supported #1366

merged 16 commits into from
May 23, 2023

Conversation

vezenovm
Copy link
Contributor

Related issue(s)

Resolves #

Description

I need some of the most recent updates from acvm 0.12.0 (noir-lang/acvm-backend-barretenberg#165) for my work. This PR simply updates to most recent commit in barretenberg backend PR (4358d3b9e8cd98d88a78dda3337e80e90668378e) and also upgrades to ACVM 0.12.0. This requires removing any usage of IsOpcodeSupported, which I simply replaced with a closure.

Summary of changes

Dependency additions / changes

Test additions / changes

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

phated and others added 12 commits May 15, 2023 13:49
@phated
feat(nargo): Only download as much CRS as you need
da2525d
@phated
update references
b6b2c6c
@phated
code review
7439e39
@vezenovm
remove usage of IsOpcodeSupported
cb05ac2
@vezenovm
switch to using dyn
6822b47
@vezenovm
use Box new
7e07bf9
@TomAFrench
chore: remove usage of Box<dyn> with impl
60e0a79
@TomAFrench
Merge branch 'master' into mv/opcode_supported
d1ec8d2 
* master:
  fix: Fix modulo operator for comptime values (#1361)
  chore: clarify that `verify_signature` takes a hashed message (#1365)
  feat: pass in closure to `Driver` to signal backend opcode support (#1349)
  feat(nargo)!: retire print-acir in favour of flag (#1328)
  chore(ssa): enable cse for assert (#1350)
  chore(ssa refactor): Add basic instruction simplification (#1329)
  chore(noir): Release 0.6.0 (#1279)
@TomAFrench
chore: switch to using backend.supports_opcode where possible
0c7eae6
@TomAFrench
Merge branch 'master' into phated/ref-string
2ac6352 
* master:
  fix: Fix modulo operator for comptime values (#1361)
  chore: clarify that `verify_signature` takes a hashed message (#1365)
  feat: pass in closure to `Driver` to signal backend opcode support (#1349)
  feat(nargo)!: retire print-acir in favour of flag (#1328)
  chore(ssa): enable cse for assert (#1350)
  chore(ssa refactor): Add basic instruction simplification (#1329)
  chore(noir): Release 0.6.0 (#1279)
@TomAFrench
Merge branch 'phated/acvm-0.12.0' into phated/ref-string
2cf7e16 
* phated/acvm-0.12.0:
@TomAFrench
Merge branch 'phated/ref-string' into mv/opcode_supported
f719117 
* phated/ref-string:
Base automatically changed from phated/ref-string to phated/acvm-0.12.0 May 22, 2023 15:08
@TomAFrench
Merge branch 'phated/acvm-0.12.0' into mv/opcode_supported
4aba46e 
* phated/acvm-0.12.0:
  fix compilation issue
  switch to published acvm and backend
  feat(nargo): Consume CommonReferenceString functions & manage caching (#1348)
  fix(stdlib): Workaround for Field comparison error in EdDSA signature verification (#1372)
  feat!: remove concept of noir fallbacks for foreign functions (#1371)
  feat(ssa refactor): mem2reg opt pass (#1363)
  feat(stdlib): EdDSA sig verification (#1313)
@TomAFrench TomAFrench marked this pull request as ready for review May 23, 2023 07:18
@TomAFrench TomAFrench changed the title chore!: Remove usage of IsOpcodeSupported chore: Remove usage of acvm::default_is_opcode_supported May 23, 2023
@TomAFrench
Copy link
Member

None of these cargo.lock changes are necessary and just come about because of the messy merge. Going to revert these.

@TomAFrench
Copy link
Member

I've left some TODOs related to cleaning up the quick fix to the backend lifetime issues (it's unfortunate that we need to create a whole new backend instance just to get opcode support).

This can be addressed through #1102. If we separate noirc_evaluator creating the circuit and acvm::compile optimizing it for a particular backend then we don't need to keep this long-lived closure around rather than just passing it in directly to where it's used.

@TomAFrench TomAFrench merged commit 8ca4ec6 into phated/acvm-0.12.0 May 23, 2023
@TomAFrench TomAFrench deleted the mv/opcode_supported branch May 23, 2023 08:05
@Savio-Sou Savio-Sou mentioned this pull request May 24, 2023
Merged
6 tasks
TomAFrench added a commit that referenced this pull request May 24, 2023
@phated @sirasistant
@TomAFrench @vezenovm
chore!: Update to ACVM 0.12.0 (#1339)
b938c7e 
* chore!: Update to ACVM 0.12.0

* feat: adapted to heterogeneous bb calls

* chore: update cargo tomls

* test: re enabled sort test

* fix: improve variable resolution

* feat: use dummy constructor for bb call

* updates for latest

* feat!: Move WitnessMap type into ACVM to avoid leaking BTreeMap type

* feat(nargo): Consume CommonReferenceString functions & manage caching (#1348)

* switch to published acvm and backend

* fix compilation issue

* chore: Remove usage of `acvm::default_is_opcode_supported` (#1366)

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
Co-authored-by: Tom French <tom@tomfren.ch>

* add issue numbers to TODOs

* chore: deduplicate driver setup logic

* chore: clippy

---------

Co-authored-by: sirasistant <sirasistant@gmail.com>
Co-authored-by: Tom French <tom@tomfren.ch>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
vezenovm added a commit that referenced this pull request Jun 1, 2023
@TomAFrench @phated
@sirasistant @vezenovm @kevaundray @guipublic
chore!: update to ACVM 0.13.0 (#1393)
22dee75 
* chore!: Update to ACVM 0.12.0

* feat: adapted to heterogeneous bb calls

* chore: update cargo tomls

* test: re enabled sort test

* fix: improve variable resolution

* feat: use dummy constructor for bb call

* updates for latest

* feat!: Move WitnessMap type into ACVM to avoid leaking BTreeMap type

* feat(nargo): Consume CommonReferenceString functions & manage caching (#1348)

* switch to published acvm and backend

* fix compilation issue

* chore: Remove usage of `acvm::default_is_opcode_supported` (#1366)

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
Co-authored-by: Tom French <tom@tomfren.ch>

* add issue numbers to TODOs

* chore: update to ACVM 0.13.0

* chore: fix broken import

* chore: bump commit

* chore: Update Acvm 0.13.0 branch  (#1465)

have acvm-backend-bberg patch reference new branch with additional crate-type

* Update Cargo.toml

* Update Cargo.toml

* update flake.lock

* feat!: var message size for keccak in stdlib (#1481)

* Var message size for keccak in stdlib

* fix the build:
remove aes blackbox
add domain separator for pedersen

* pedersen with domain separator

* chore: update pedersen test case for domain separator (#1482)

pedersen with domain separator

* chore: add pedersen hash with domain separator in stdlib (#1483)

* pedersen with domain separator

* separator for pedersen

* update to acvm 0.13.0 and new acvm-backend-bberg

* update cargo lock

* update MockBackend in sort test

* update merkle_insert and simple_shield for updated pedersen in bberg

* try ubuntu-large

* remove patch and update to acvm-backend-bberg 0.3.0

* cargo.lock

---------

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
Co-authored-by: sirasistant <sirasistant@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: guipublic <47281315+guipublic@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomAFrench TomAFrench TomAFrench 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
@vezenovm @TomAFrench @phated