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

feat: Sync from aztec-packages #4494

Merged
merged 32 commits into from
Mar 13, 2024
Merged

feat: Sync from aztec-packages #4494

merged 32 commits into from
Mar 13, 2024

Conversation

AztecBot
Copy link
Collaborator

@AztecBot AztecBot commented Mar 6, 2024

Automated pull of Noir development from aztec-packages.
BEGIN_COMMIT_OVERRIDE
chore!: Remove open keyword from Noir (AztecProtocol/aztec-packages#4967)
chore: aztec-macros refactor (AztecProtocol/aztec-packages#5127)
feat: make brillig-gen more AVM-friendly (AztecProtocol/aztec-packages#5091)
feat: Integrated native ACVM (AztecProtocol/aztec-packages#4903)
END_COMMIT_OVERRIDE

AztecBot and others added 3 commits March 6, 2024 18:44
## Overview
Works around brillig blowup issue by altering the read and write opcodes
to take in arrays of data. This is potentially just a short term fix.

- Reading and writing to storage now take in arrays, code will not
compile without this change, due to an ssa issue ->[ ISSUE
](AztecProtocol/aztec-packages#4979)

- Tag checks on memory now just make sure the tag is LESS than uint64,
rather than asserting that the memory tag is uint32, this should be
fine.

- We had to blow up the memory space of the avm to u64 as the entire
noir compiler works with u64s. Arrays will not work unless we either
    - Make the avm 64 bit addressable ( probably fine )
- Make noir 32 bit addressable ( requires alot of buy in )
AztecProtocol/aztec-packages#4814

---------

Co-authored-by: sirasistant <[email protected]>
This PR creates a cli entrypoint for the ACVM simulator, builds it and
makes it available for use in e2e tests both locally and on CI.

This native simulator is used to execute sequencer-side protocol
circuits, in parallel where possible.
This PR creates a cli entrypoint for the ACVM simulator, builds it and
makes it available for use in e2e tests both locally and on CI.

This native simulator is used to execute sequencer-side protocol
circuits, in parallel where possible.
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 6, 2024
Copy link
Contributor

github-actions bot commented Mar 6, 2024

AztecBot and others added 17 commits March 7, 2024 02:12
Fixes #4844.

Purges calldata hash and txs hash to replace both with txs effects hash.

Also moves the compute tx effects hash function from the base rollup and
into the components as was the intention.
Fixes #4844.

Purges calldata hash and txs hash to replace both with txs effects hash.

Also moves the compute tx effects hash function from the base rollup and
into the components as was the intention.
## Overview
Works around brillig blowup issue by altering the read and write opcodes
to take in arrays of data. This is potentially just a short term fix.

- Reading and writing to storage now take in arrays, code will not
compile without this change, due to an ssa issue ->[ ISSUE
](AztecProtocol/aztec-packages#4979)

- Tag checks on memory now just make sure the tag is LESS than uint64,
rather than asserting that the memory tag is uint32, this should be
fine.

- We had to blow up the memory space of the avm to u64 as the entire
noir compiler works with u64s. Arrays will not work unless we either
    - Make the avm 64 bit addressable ( probably fine )
- Make noir 32 bit addressable ( requires alot of buy in )
AztecProtocol/aztec-packages#4814

---------

Co-authored-by: sirasistant <[email protected]>
## Overview
Works around brillig blowup issue by altering the read and write opcodes
to take in arrays of data. This is potentially just a short term fix.

- Reading and writing to storage now take in arrays, code will not
compile without this change, due to an ssa issue ->[ ISSUE
](AztecProtocol/aztec-packages#4979)

- Tag checks on memory now just make sure the tag is LESS than uint64,
rather than asserting that the memory tag is uint32, this should be
fine.

- We had to blow up the memory space of the avm to u64 as the entire
noir compiler works with u64s. Arrays will not work unless we either
    - Make the avm 64 bit addressable ( probably fine )
- Make noir 32 bit addressable ( requires alot of buy in )
AztecProtocol/aztec-packages#4814

---------

Co-authored-by: sirasistant <[email protected]>
…tec-packages#5065)

It isn't clear after some of the recursion cleanup in
AztecProtocol/aztec-packages#4221 why
`double_verify_proof` is failing the solidity verifier.

`double_verify_proof` was being used as a recursive proof itself to be
verified inside of `double_verify_nested_proof`. I have renamed this
test to `double_verify_proof_recursive` to note that its proof should be
used as input to another circuit.

I have also included a new test `double_verify_proof` where we accept
two non-nested proofs and use the Keccak prover. This is what we were
previously expecting for `double_verify_proof`. I also brought back
`arretenberg-acir-tests-bb-sol` for a few tests.

---------

Co-authored-by: Maddiaa <[email protected]>
…tec-packages#5065)

It isn't clear after some of the recursion cleanup in
AztecProtocol/aztec-packages#4221 why
`double_verify_proof` is failing the solidity verifier.

`double_verify_proof` was being used as a recursive proof itself to be
verified inside of `double_verify_nested_proof`. I have renamed this
test to `double_verify_proof_recursive` to note that its proof should be
used as input to another circuit.

I have also included a new test `double_verify_proof` where we accept
two non-nested proofs and use the Keccak prover. This is what we were
previously expecting for `double_verify_proof`. I also brought back
`arretenberg-acir-tests-bb-sol` for a few tests.

---------

Co-authored-by: Maddiaa <[email protected]>
…l/aztec-packages#5086)

Closes #4520.

I did some work adding tests for this, but ultimately decided to go back
on it. We don't really have any tests for other parts of the `addNotes`
flow (such as the checks for note existence, inclusion in the provided
tx, non-nullification, etc.), and this didn't feel like the right place
to work on those.

We definitely should however. Part of the problem is that writing a
contract that allows for manual note creation, deletion and retrieval
from a test is quite annoying - I did some of this in `TestContract` for
#4238. For this we'd also need to have different functions for different
note types, and even then some of these notes can only be added
automatically via broadcast due to the random values in the note.
…l/aztec-packages#5086)

Closes #4520.

I did some work adding tests for this, but ultimately decided to go back
on it. We don't really have any tests for other parts of the `addNotes`
flow (such as the checks for note existence, inclusion in the provided
tx, non-nullification, etc.), and this didn't feel like the right place
to work on those.

We definitely should however. Part of the problem is that writing a
contract that allows for manual note creation, deletion and retrieval
from a test is quite annoying - I did some of this in `TestContract` for
#4238. For this we'd also need to have different functions for different
note types, and even then some of these notes can only be added
automatically via broadcast due to the random values in the note.
This PR enables the stack of 6 PRs on top.
\
While working on external calls, we came across several problems with Brillig. I made some changes to fix them. Some Brillig changes:
* **Truncation**: Brillig was using AND of Fields (actually, AND on Ints of 254 bits). This is not supported by the VM. Truncation was changed to be done without ANDing, and using `CAST` instead, which truncates to the required bit size.
* **Array.get**/**Array.set**: Calculation of the `arrayBase+index` was done using field arithmetic (or field sizes). Now it's done using ints.
* **Reference counting**: Checking `refCount==1` was done using field arithmetic (or field sizes). Now it's done with ints.

These changes seem to solve all the comparison w/different bit sizes, and unexpected uses of field arithmetic. (That we've found with the current test contract).

NOTE: I had to recreate the contract snapshots with `yarn workspace @aztec/protocol-contracts test -u`, then modify
* noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr
* noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr

Relates to #4313, #4127.
This PR enables the stack of 6 PRs on top.
\
While working on external calls, we came across several problems with Brillig. I made some changes to fix them. Some Brillig changes:
* **Truncation**: Brillig was using AND of Fields (actually, AND on Ints of 254 bits). This is not supported by the VM. Truncation was changed to be done without ANDing, and using `CAST` instead, which truncates to the required bit size.
* **Array.get**/**Array.set**: Calculation of the `arrayBase+index` was done using field arithmetic (or field sizes). Now it's done using ints.
* **Reference counting**: Checking `refCount==1` was done using field arithmetic (or field sizes). Now it's done with ints.

These changes seem to solve all the comparison w/different bit sizes, and unexpected uses of field arithmetic. (That we've found with the current test contract).

NOTE: I had to recreate the contract snapshots with `yarn workspace @aztec/protocol-contracts test -u`, then modify
* noir-projects/noir-contracts/contracts/app_subscription_contract/src/main.nr
* noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr

Relates to #4313, #4127.
@sirasistant sirasistant requested a review from TomAFrench March 11, 2024 12:27
Copy link
Contributor

@sirasistant sirasistant left a comment

Choose a reason for hiding this comment

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

The brillig and macros changes look fine to me, what do you think about the acvm cli changes @TomAFrench ? They come from this PR AztecProtocol/aztec-packages#4903

sirasistant and others added 6 commits March 11, 2024 12:56
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: generalise `FunctionVisibility` to `ItemVisibility`
(#4495)
fix: Dynamic assert messages in brillig
(#4531)
chore: organize the `blackbox_solver` crate
(#4519)
fix(acir_gen): More granular element sizes array check
(#4528)
chore: Release Noir(0.25.0)
(#4352)
chore: document big integers
(#4487)
fix: Add `follow_bindings` to follow `Type::Alias` links
(#4521)
fix: Fix brillig slowdown when assigning arrays in loops
(#4472)
chore: Move `check_method_signatures` to type checking phase
(#4516)
chore(ci): fix JS publishing workflow checking out inconsistent commits
(#4493)
fix(ssa): Handle mergers of slices returned from calls
(#4496)
chore: Add HashMap docs (#4457)
chore: custom hash for eddsa
(#4440)
chore: update various dependencies
(#4513)
fix: Allow type aliases in main
(#4505)
chore: add `ModuleDeclaration` struct
(#4512)
fix: Force src impl for == on slices
(#4507)
chore: pass `import_directive` by reference
(#4511)
feat: Track stack frames and their variables in the debugger
(#4188)
chore: add regression test for issue 4449
(#4503)
chore: pass macro processors by reference
(#4501)
chore: bump bb to 0.26.3 (#4488)
fix: handling of gh deps in noir_wasm
(#4499)
fix: iterative flattening pass
(#4492)
chore: Move templated code for assert_message into the stdlib
(#4475)
chore: pull out separate function for compiling and running a test
chore: update cargo deny config
(#4486)
feat: run tests in parallel in `nargo test`
(#4484)
END_COMMIT_OVERRIDE

---------

Co-authored-by: TomAFrench <[email protected]>
Co-authored-by: Tom French <[email protected]>
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: generalise `FunctionVisibility` to `ItemVisibility`
(#4495)
fix: Dynamic assert messages in brillig
(#4531)
chore: organize the `blackbox_solver` crate
(#4519)
fix(acir_gen): More granular element sizes array check
(#4528)
chore: Release Noir(0.25.0)
(#4352)
chore: document big integers
(#4487)
fix: Add `follow_bindings` to follow `Type::Alias` links
(#4521)
fix: Fix brillig slowdown when assigning arrays in loops
(#4472)
chore: Move `check_method_signatures` to type checking phase
(#4516)
chore(ci): fix JS publishing workflow checking out inconsistent commits
(#4493)
fix(ssa): Handle mergers of slices returned from calls
(#4496)
chore: Add HashMap docs (#4457)
chore: custom hash for eddsa
(#4440)
chore: update various dependencies
(#4513)
fix: Allow type aliases in main
(#4505)
chore: add `ModuleDeclaration` struct
(#4512)
fix: Force src impl for == on slices
(#4507)
chore: pass `import_directive` by reference
(#4511)
feat: Track stack frames and their variables in the debugger
(#4188)
chore: add regression test for issue 4449
(#4503)
chore: pass macro processors by reference
(#4501)
chore: bump bb to 0.26.3 (#4488)
fix: handling of gh deps in noir_wasm
(#4499)
fix: iterative flattening pass
(#4492)
chore: Move templated code for assert_message into the stdlib
(#4475)
chore: pull out separate function for compiling and running a test
chore: update cargo deny config
(#4486)
feat: run tests in parallel in `nargo test`
(#4484)
END_COMMIT_OVERRIDE

---------

Co-authored-by: TomAFrench <[email protected]>
Co-authored-by: Tom French <[email protected]>
sirasistant and others added 3 commits March 13, 2024 10:40
…ztec-packages#5144)

The responsibility to check that the initialization arguments used when
calling the initializer as correct is responsibility of the initializer
itself. Without this check, a deployer could commit to a set of init
args in the address preimage, but then call the initializer using
something totally different.

Fixes #5154

---------

Co-authored-by: Alex Gherghisan <[email protected]>
…ztec-packages#5144)

The responsibility to check that the initialization arguments used when
calling the initializer as correct is responsibility of the initializer
itself. Without this check, a deployer could commit to a set of init
args in the address preimage, but then call the initializer using
something totally different.

Fixes #5154

---------

Co-authored-by: Alex Gherghisan <[email protected]>
@TomAFrench
Copy link
Member

I'm gonna look through this and merge sometime today.

what do you think about the acvm cli changes

It's not entirely ideal having it where it is and syncing out into this repo but it's self-contained enough that it's probably not worth the effort to hoist it somewhere else inside aztec-packages.

TomAFrench and others added 3 commits March 13, 2024 12:05
* master:
  feat: Add checks for bit size consistency on brillig gen (#4542)
@TomAFrench TomAFrench enabled auto-merge March 13, 2024 12:22
@TomAFrench TomAFrench added this pull request to the merge queue Mar 13, 2024
Copy link
Contributor

FYI @noir-lang/developerrelations on Noir doc changes.

Merged via the queue into master with commit a6016b4 Mar 13, 2024
44 checks passed
TomAFrench added a commit that referenced this pull request Mar 13, 2024
* master:
  feat: Sync from aztec-packages (#4494)
  feat: Add checks for bit size consistency on brillig gen (#4542)
  fix: Allow non-integer globals to reference struct methods (#4490)
TomAFrench added a commit that referenced this pull request Mar 13, 2024
* master: (48 commits)
  feat: Visible aliases for nargo commands (#4453)
  feat: Sync from aztec-packages (#4494)
  feat: Add checks for bit size consistency on brillig gen (#4542)
  fix: Allow non-integer globals to reference struct methods (#4490)
  chore: generalise `FunctionVisibility` to `ItemVisibility` (#4495)
  fix: Dynamic assert messages in brillig (#4531)
  chore: organize the `blackbox_solver` crate (#4519)
  fix(acir_gen): More granular element sizes array check (#4528)
  chore: Release Noir(0.25.0) (#4352)
  chore: document big integers (#4487)
  fix: Add `follow_bindings` to follow `Type::Alias` links (#4521)
  fix: Fix brillig slowdown when assigning arrays in loops (#4472)
  chore: Move `check_method_signatures` to type checking phase (#4516)
  chore(ci): fix JS publishing workflow checking out inconsistent commits (#4493)
  fix(ssa): Handle mergers of slices returned from calls (#4496)
  chore: Add HashMap docs (#4457)
  chore: custom hash for eddsa (#4440)
  chore: update various dependencies (#4513)
  fix: Allow type aliases in main (#4505)
  chore: add `ModuleDeclaration` struct (#4512)
  ...
TomAFrench added a commit that referenced this pull request Mar 13, 2024
* master: (36 commits)
  fix: Substitute generics when checking the field count of a type (#4547)
  feat: optimize sha2 implementation (#4441)
  chore: allow setting namespace visibility on functions (#4510)
  feat: Visible aliases for nargo commands (#4453)
  feat: Sync from aztec-packages (#4494)
  feat: Add checks for bit size consistency on brillig gen (#4542)
  fix: Allow non-integer globals to reference struct methods (#4490)
  chore: generalise `FunctionVisibility` to `ItemVisibility` (#4495)
  fix: Dynamic assert messages in brillig (#4531)
  chore: organize the `blackbox_solver` crate (#4519)
  fix(acir_gen): More granular element sizes array check (#4528)
  chore: Release Noir(0.25.0) (#4352)
  chore: document big integers (#4487)
  fix: Add `follow_bindings` to follow `Type::Alias` links (#4521)
  fix: Fix brillig slowdown when assigning arrays in loops (#4472)
  chore: Move `check_method_signatures` to type checking phase (#4516)
  chore(ci): fix JS publishing workflow checking out inconsistent commits (#4493)
  fix(ssa): Handle mergers of slices returned from calls (#4496)
  chore: Add HashMap docs (#4457)
  chore: custom hash for eddsa (#4440)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants