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

chore: Moves add gate out of aux #8541

Merged
merged 8 commits into from
Sep 16, 2024
Merged

Conversation

lucasxia01
Copy link
Contributor

@lucasxia01 lucasxia01 commented Sep 13, 2024

Resolves AztecProtocol/barretenberg#913.

Moves the add gate in aux to the arithmetic block instead and adds a dummy gate in aux as replacement. Adds some tests to check the partitioning of selectors into their blocks.

This is needed to complete the sorting work that @ledwards2225 did to finally partition gates into their separate blocks. Therefore, only the q_arith selector can be nonzero in the arithmetic block and so on...

This used to be blocked by the recursion limit in Plonk (as in this change would've bumped us over the limit), but we don't
care about that anymore (because it's double_verify_proof already goes over 2^19).

@@ -646,7 +646,77 @@ TEST(ultra_circuit_constructor, non_native_field_multiplication)
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, rom)
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this test copies the previous test and uses the function that previously, but now doesn't, added a gate with q_arith turned on in the aux block. It checks that the aux block does not have any other selector turned on besides the aux one

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to have this, but you're only checking one block.

@@ -15,7 +15,7 @@ namespace bb {
using plookup::ColumnIdx;
using plookup::MultiTableId;

TEST(ultra_circuit_constructor, copy_constructor)
TEST(UltraCircuitConstructor, CopyConstructor)
Copy link
Contributor Author

@lucasxia01 lucasxia01 Sep 13, 2024

Choose a reason for hiding this comment

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

was testing out ai editor to do these formatting changes

// selectors are zero get the trace blocks
for (auto& block : builder.blocks.get()) {
for (size_t i = 0; i < block.size(); ++i) {
if (&block != &builder.blocks.arithmetic) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

using the ptr is a little weird, but it works. Could change to using an enum or something instead

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems fine

TEST(MegaCircuitBuilder, CompleteSelectorPartitioningCheck)
{
auto builder = MegaCircuitBuilder();
GoblinMockCircuits::construct_simple_circuit(builder);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure if this is the best circuit to use, but it seemed pretty diverse

@@ -83,14 +83,14 @@ template <typename FF_> class MegaArith {
{
this->ecc_op = 1 << 10;
this->pub_inputs = 1 << 7;
this->arithmetic = 187000;
this->arithmetic = 201000;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to update this with the change, and also reduce some other gates to prevent increasing the circuit size to 2^20.

@lucasxia01 lucasxia01 self-assigned this Sep 14, 2024
}
check_selector_length_consistency();
++this->num_gates;

// TODO(https://github.com/AztecProtocol/barretenberg/issues/879): Originally this was a single arithmetic gate.
// With trace sorting, we must add a dummy gate since the add gate would otherwise try to read into an aux gate that
// has been sorted out of sequence. (Note: temporarily disabled in favor of manual arith gate in aux block above).
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this comment be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, great catch!

Copy link
Contributor

github-actions bot commented Sep 16, 2024

Changes to circuit sizes

Generated at commit: f24be6aa57644fdb17306d01b2c5cc7dba38c188, compared to commit: 0c5422446de40a5824daa5af33cad1ec015589a0

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
parity_root 0 ➖ 0.00% +87,732 ❌ +1.63%
rollup_block_root 0 ➖ 0.00% +65,799 ❌ +1.62%
rollup_merge 0 ➖ 0.00% +43,866 ❌ +1.62%
rollup_root 0 ➖ 0.00% +43,866 ❌ +1.60%
rollup_block_merge 0 ➖ 0.00% +43,866 ❌ +1.60%
public_kernel_app_logic 0 ➖ 0.00% +21,933 ❌ +1.12%
public_kernel_teardown 0 ➖ 0.00% +21,933 ❌ +1.12%
public_kernel_setup 0 ➖ 0.00% +21,933 ❌ +1.12%
public_kernel_tail 0 ➖ 0.00% +21,933 ❌ +0.81%
rollup_base 0 ➖ 0.00% +21,933 ❌ +0.61%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
parity_root 374 (0) 0.00% 5,481,623 (+87,732) +1.63%
rollup_block_root 803 (0) 0.00% 4,117,756 (+65,799) +1.62%
rollup_merge 319 (0) 0.00% 2,748,043 (+43,866) +1.62%
rollup_root 18,188 (0) 0.00% 2,785,852 (+43,866) +1.60%
rollup_block_merge 18,204 (0) 0.00% 2,785,884 (+43,866) +1.60%
public_kernel_app_logic 246,381 (0) 0.00% 1,983,861 (+21,933) +1.12%
public_kernel_teardown 247,012 (0) 0.00% 1,984,645 (+21,933) +1.12%
public_kernel_setup 246,841 (0) 0.00% 1,987,064 (+21,933) +1.12%
public_kernel_tail 259,775 (0) 0.00% 2,726,963 (+21,933) +0.81%
rollup_base 347,688 (0) 0.00% 3,629,648 (+21,933) +0.61%

Copy link
Contributor

@codygunton codygunton left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -646,7 +646,77 @@ TEST(ultra_circuit_constructor, non_native_field_multiplication)
EXPECT_EQ(result, true);
}

TEST(ultra_circuit_constructor, rom)
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to have this, but you're only checking one block.

@lucasxia01 lucasxia01 enabled auto-merge (squash) September 16, 2024 18:56
@lucasxia01 lucasxia01 added the C-barretenberg Component: barretenberg cryptography library label Sep 16, 2024
@lucasxia01 lucasxia01 merged commit c3ad163 into master Sep 16, 2024
145 of 149 checks passed
@lucasxia01 lucasxia01 deleted the lx/move-add-gate-out-of-aux branch September 16, 2024 22:01
spypsy pushed a commit that referenced this pull request Sep 17, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.55.1</summary>

##
[0.55.1](aztec-package-v0.55.0...aztec-package-v0.55.1)
(2024-09-17)


### Features

* CI deploy on sepolia
([#8514](#8514))
([54f0344](54f0344))
* Tx verification & peer scoring on p2p layer. bot support for
EasyPrivateToken
([#8298](#8298))
([beb651f](beb651f))


### Miscellaneous

* Remove ARCHIVER_L1_START_BLOCK
([#8554](#8554))
([bc8d461](bc8d461))
</details>

<details><summary>barretenberg.js: 0.55.1</summary>

##
[0.55.1](barretenberg.js-v0.55.0...barretenberg.js-v0.55.1)
(2024-09-17)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>aztec-packages: 0.55.1</summary>

##
[0.55.1](aztec-packages-v0.55.0...aztec-packages-v0.55.1)
(2024-09-17)


### Features

* `TXE::store_note_in_cache` --&gt; `TXE::add_note`
([#8547](#8547))
([5a6aaeb](5a6aaeb))
* Add a `comptime` string type for string handling at compile-time
(noir-lang/noir#6026)
([cd7983a](cd7983a))
* CI deploy on sepolia
([#8514](#8514))
([54f0344](54f0344))
* Default to outputting witness with file named after package
(noir-lang/noir#6031)
([cd7983a](cd7983a))
* Let LSP suggest trait impl methods as you are typing them
(noir-lang/noir#6029)
([cd7983a](cd7983a))
* NFT with "transient" storage shield flow
([#8129](#8129))
([578f67c](578f67c))
* Optimize allocating immediate amounts of memory
([#8579](#8579))
([e0185e7](e0185e7))
* Spartan iac
([#8455](#8455))
([16fba46](16fba46))
* Sync from aztec-packages (noir-lang/noir#6028)
([cd7983a](cd7983a))
* Tx verification & peer scoring on p2p layer. bot support for
EasyPrivateToken
([#8298](#8298))
([beb651f](beb651f))
* Unconstraining keys in unconstrained encryption
([#7912](#7912))
([eb9275a](eb9275a))
* Update args hash to be a flat poseidon
([#8571](#8571))
([0c54224](0c54224))
* Use poseidon for fn selectors
([#8239](#8239))
([41891db](41891db))


### Bug Fixes

* Disable side-effects for no_predicates functions
(noir-lang/noir#6027)
([cd7983a](cd7983a))
* Native world state test issues
([#8546](#8546))
([aab8773](aab8773))
* Remove special case for epoch 0
([#8549](#8549))
([b035d01](b035d01))
* Serialize AvmVerificationKeyData
([#8529](#8529))
([78c94a4](78c94a4))


### Miscellaneous

* 7791: Disable world_state test suite
([#8594](#8594))
([ee21583](ee21583))
* Add jq to aztec image
([#8542](#8542))
([a7fb791](a7fb791))
* Add sync suite
([#8550](#8550))
([ce0a9db](ce0a9db))
* **ci:** Action to redo typo PRs
([#8553](#8553))
([3ed5879](3ed5879))
* **ci:** Fix master
([#8534](#8534))
([47c368f](47c368f))
* **ci:** Fix redo-typo-pr.yml
([abf9802](abf9802))
* **ci:** Fix redo-typo-pr.yml
([#8555](#8555))
([7f1673c](7f1673c))
* **ci:** Hotfix
([ffd31aa](ffd31aa))
* **ci:** Hotfix arm ci
([979f267](979f267))
* **ci:** Optimize disk usage in arm run
([#8564](#8564))
([33e6aa4](33e6aa4))
* **ci:** Use labels and if branch=master to control jobs
([#8508](#8508))
([68a2226](68a2226))
* GitHub Actions Deployments to Amazon EKS
([#8563](#8563))
([6fae8f0](6fae8f0))
* Moves add gate out of aux
([#8541](#8541))
([c3ad163](c3ad163))
* Protogalaxy verifier matches prover 2
([#8477](#8477))
([58882b1](58882b1))
* Redo typo PR by ankushgoel27
([#8595](#8595))
([7ca6d24](7ca6d24))
* Redo typo PR by Ocheretovich
([#8559](#8559))
([c4296ba](c4296ba))
* Redo typo PR by Olexandr88
([#8560](#8560))
([e35d148](e35d148))
* Redo typo PR by skaunov
([#8557](#8557))
([8a1e7c3](8a1e7c3))
* Release Noir(0.34.0) (noir-lang/noir#5692)
([cd7983a](cd7983a))
* Remove ARCHIVER_L1_START_BLOCK
([#8554](#8554))
([bc8d461](bc8d461))
* Remove redundant e2e tests and organize
([#8561](#8561))
([de2b775](de2b775))
* Remove unused imports
([#8556](#8556))
([e11242e](e11242e))
* Replace relative paths to noir-protocol-circuits
([2336986](2336986))
* Replace relative paths to noir-protocol-circuits
([9668ed5](9668ed5))
</details>

<details><summary>barretenberg: 0.55.1</summary>

##
[0.55.1](barretenberg-v0.55.0...barretenberg-v0.55.1)
(2024-09-17)


### Bug Fixes

* Native world state test issues
([#8546](#8546))
([aab8773](aab8773))


### Miscellaneous

* 7791: Disable world_state test suite
([#8594](#8594))
([ee21583](ee21583))
* Moves add gate out of aux
([#8541](#8541))
([c3ad163](c3ad163))
* Protogalaxy verifier matches prover 2
([#8477](#8477))
([58882b1](58882b1))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Sep 18, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.55.1</summary>

##
[0.55.1](AztecProtocol/aztec-packages@aztec-package-v0.55.0...aztec-package-v0.55.1)
(2024-09-17)


### Features

* CI deploy on sepolia
([#8514](AztecProtocol/aztec-packages#8514))
([54f0344](AztecProtocol/aztec-packages@54f0344))
* Tx verification & peer scoring on p2p layer. bot support for
EasyPrivateToken
([#8298](AztecProtocol/aztec-packages#8298))
([beb651f](AztecProtocol/aztec-packages@beb651f))


### Miscellaneous

* Remove ARCHIVER_L1_START_BLOCK
([#8554](AztecProtocol/aztec-packages#8554))
([bc8d461](AztecProtocol/aztec-packages@bc8d461))
</details>

<details><summary>barretenberg.js: 0.55.1</summary>

##
[0.55.1](AztecProtocol/aztec-packages@barretenberg.js-v0.55.0...barretenberg.js-v0.55.1)
(2024-09-17)


### Miscellaneous

* **barretenberg.js:** Synchronize aztec-packages versions
</details>

<details><summary>aztec-packages: 0.55.1</summary>

##
[0.55.1](AztecProtocol/aztec-packages@aztec-packages-v0.55.0...aztec-packages-v0.55.1)
(2024-09-17)


### Features

* `TXE::store_note_in_cache` --&gt; `TXE::add_note`
([#8547](AztecProtocol/aztec-packages#8547))
([5a6aaeb](AztecProtocol/aztec-packages@5a6aaeb))
* Add a `comptime` string type for string handling at compile-time
(noir-lang/noir#6026)
([cd7983a](AztecProtocol/aztec-packages@cd7983a))
* CI deploy on sepolia
([#8514](AztecProtocol/aztec-packages#8514))
([54f0344](AztecProtocol/aztec-packages@54f0344))
* Default to outputting witness with file named after package
(noir-lang/noir#6031)
([cd7983a](AztecProtocol/aztec-packages@cd7983a))
* Let LSP suggest trait impl methods as you are typing them
(noir-lang/noir#6029)
([cd7983a](AztecProtocol/aztec-packages@cd7983a))
* NFT with "transient" storage shield flow
([#8129](AztecProtocol/aztec-packages#8129))
([578f67c](AztecProtocol/aztec-packages@578f67c))
* Optimize allocating immediate amounts of memory
([#8579](AztecProtocol/aztec-packages#8579))
([e0185e7](AztecProtocol/aztec-packages@e0185e7))
* Spartan iac
([#8455](AztecProtocol/aztec-packages#8455))
([16fba46](AztecProtocol/aztec-packages@16fba46))
* Sync from aztec-packages (noir-lang/noir#6028)
([cd7983a](AztecProtocol/aztec-packages@cd7983a))
* Tx verification & peer scoring on p2p layer. bot support for
EasyPrivateToken
([#8298](AztecProtocol/aztec-packages#8298))
([beb651f](AztecProtocol/aztec-packages@beb651f))
* Unconstraining keys in unconstrained encryption
([#7912](AztecProtocol/aztec-packages#7912))
([eb9275a](AztecProtocol/aztec-packages@eb9275a))
* Update args hash to be a flat poseidon
([#8571](AztecProtocol/aztec-packages#8571))
([0c54224](AztecProtocol/aztec-packages@0c54224))
* Use poseidon for fn selectors
([#8239](AztecProtocol/aztec-packages#8239))
([41891db](AztecProtocol/aztec-packages@41891db))


### Bug Fixes

* Disable side-effects for no_predicates functions
(noir-lang/noir#6027)
([cd7983a](AztecProtocol/aztec-packages@cd7983a))
* Native world state test issues
([#8546](AztecProtocol/aztec-packages#8546))
([aab8773](AztecProtocol/aztec-packages@aab8773))
* Remove special case for epoch 0
([#8549](AztecProtocol/aztec-packages#8549))
([b035d01](AztecProtocol/aztec-packages@b035d01))
* Serialize AvmVerificationKeyData
([#8529](AztecProtocol/aztec-packages#8529))
([78c94a4](AztecProtocol/aztec-packages@78c94a4))


### Miscellaneous

* 7791: Disable world_state test suite
([#8594](AztecProtocol/aztec-packages#8594))
([ee21583](AztecProtocol/aztec-packages@ee21583))
* Add jq to aztec image
([#8542](AztecProtocol/aztec-packages#8542))
([a7fb791](AztecProtocol/aztec-packages@a7fb791))
* Add sync suite
([#8550](AztecProtocol/aztec-packages#8550))
([ce0a9db](AztecProtocol/aztec-packages@ce0a9db))
* **ci:** Action to redo typo PRs
([#8553](AztecProtocol/aztec-packages#8553))
([3ed5879](AztecProtocol/aztec-packages@3ed5879))
* **ci:** Fix master
([#8534](AztecProtocol/aztec-packages#8534))
([47c368f](AztecProtocol/aztec-packages@47c368f))
* **ci:** Fix redo-typo-pr.yml
([abf9802](AztecProtocol/aztec-packages@abf9802))
* **ci:** Fix redo-typo-pr.yml
([#8555](AztecProtocol/aztec-packages#8555))
([7f1673c](AztecProtocol/aztec-packages@7f1673c))
* **ci:** Hotfix
([ffd31aa](AztecProtocol/aztec-packages@ffd31aa))
* **ci:** Hotfix arm ci
([979f267](AztecProtocol/aztec-packages@979f267))
* **ci:** Optimize disk usage in arm run
([#8564](AztecProtocol/aztec-packages#8564))
([33e6aa4](AztecProtocol/aztec-packages@33e6aa4))
* **ci:** Use labels and if branch=master to control jobs
([#8508](AztecProtocol/aztec-packages#8508))
([68a2226](AztecProtocol/aztec-packages@68a2226))
* GitHub Actions Deployments to Amazon EKS
([#8563](AztecProtocol/aztec-packages#8563))
([6fae8f0](AztecProtocol/aztec-packages@6fae8f0))
* Moves add gate out of aux
([#8541](AztecProtocol/aztec-packages#8541))
([c3ad163](AztecProtocol/aztec-packages@c3ad163))
* Protogalaxy verifier matches prover 2
([#8477](AztecProtocol/aztec-packages#8477))
([58882b1](AztecProtocol/aztec-packages@58882b1))
* Redo typo PR by ankushgoel27
([#8595](AztecProtocol/aztec-packages#8595))
([7ca6d24](AztecProtocol/aztec-packages@7ca6d24))
* Redo typo PR by Ocheretovich
([#8559](AztecProtocol/aztec-packages#8559))
([c4296ba](AztecProtocol/aztec-packages@c4296ba))
* Redo typo PR by Olexandr88
([#8560](AztecProtocol/aztec-packages#8560))
([e35d148](AztecProtocol/aztec-packages@e35d148))
* Redo typo PR by skaunov
([#8557](AztecProtocol/aztec-packages#8557))
([8a1e7c3](AztecProtocol/aztec-packages@8a1e7c3))
* Release Noir(0.34.0) (noir-lang/noir#5692)
([cd7983a](AztecProtocol/aztec-packages@cd7983a))
* Remove ARCHIVER_L1_START_BLOCK
([#8554](AztecProtocol/aztec-packages#8554))
([bc8d461](AztecProtocol/aztec-packages@bc8d461))
* Remove redundant e2e tests and organize
([#8561](AztecProtocol/aztec-packages#8561))
([de2b775](AztecProtocol/aztec-packages@de2b775))
* Remove unused imports
([#8556](AztecProtocol/aztec-packages#8556))
([e11242e](AztecProtocol/aztec-packages@e11242e))
* Replace relative paths to noir-protocol-circuits
([2336986](AztecProtocol/aztec-packages@2336986))
* Replace relative paths to noir-protocol-circuits
([9668ed5](AztecProtocol/aztec-packages@9668ed5))
</details>

<details><summary>barretenberg: 0.55.1</summary>

##
[0.55.1](AztecProtocol/aztec-packages@barretenberg-v0.55.0...barretenberg-v0.55.1)
(2024-09-17)


### Bug Fixes

* Native world state test issues
([#8546](AztecProtocol/aztec-packages#8546))
([aab8773](AztecProtocol/aztec-packages@aab8773))


### Miscellaneous

* 7791: Disable world_state test suite
([#8594](AztecProtocol/aztec-packages#8594))
([ee21583](AztecProtocol/aztec-packages@ee21583))
* Moves add gate out of aux
([#8541](AztecProtocol/aztec-packages#8541))
([c3ad163](AztecProtocol/aztec-packages@c3ad163))
* Protogalaxy verifier matches prover 2
([#8477](AztecProtocol/aztec-packages#8477))
([58882b1](AztecProtocol/aztec-packages@58882b1))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-barretenberg Component: barretenberg cryptography library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trace sorting: Move add gate out of aux block
3 participants