feat: Update goblin translator circuit builder (Goblin Translator part 1) #2764

Rumata888 · 2023-10-10T15:05:00Z

This PR updates the goblin translator circuit builder to the latest version needed for Goblin Translator proofs. Most of the code is moved from the header to the cpp files and all constraints are being checked in check_circuit

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).

ledwards2225

Overall looks great. Most of my comments are minor suggestions around naming/comments. I went ahead and clicked Approve rather than request changes simply because I'll be out soon and don't want this to get hung up. Nice work

...berg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.cpp

ledwards2225 · 2023-10-11T17:40:03Z

...berg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.cpp

+    insert_pair_into_wire(
+        RELATION_WIDE_LIMBS_RANGE_CONSTRAINT_3, low_relation_microlimbs[3], high_relation_microlimbs[3]);
+
+    // Next ones go into top P_x and P_y, current accumulator and quotient unused microlimbs


Is there a typo or a missing word here?

Could you elaborate?

...berg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.hpp

ledwards2225 · 2023-10-11T19:29:05Z

...berg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.hpp

+    static constexpr auto MAX_MICRO_LIMB_SIZE = (uint256_t(1) << MICRO_LIMB_BITS) - 1;
+
+    // To range constrain a limb to 68 bits we need 6 limbs: 5 for 14-bit windowed chunks and 1 to range constrain the
+    // highest to a more restrictive range (0 <= a < 2¹⁴ && 0 <= 4*a < 2¹⁴ ) ~ ( 0 <= a < 2¹² )


The comment makes sense to me but the comparison expressions are confusing. Is there a typo here? I cant make sense of constraing a to 14 bits then also 4a to 14 bits

Imagine, you have a range constraint that ensures all values are in range {0,1,2,3,4,5,6,7}, but you want to range constrain to {0,1}. What you do is range constrain a to [0-7] and then range constrain 4*a to [0-7]. Then a can only be {0,1}

...berg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.hpp

ledwards2225 · 2023-10-11T19:49:15Z

...berg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.cpp

    auto [quotient_0, quotient_1, quotient_2, quotient_3, quotient_prime] = uint512_t_to_bigfield(quotient);
-    std::array<Fr, 5> quotient_witnesses = { quotient_0, quotient_1, quotient_2, quotient_3, quotient_prime };
+    std::array<Fr, NUM_BINARY_LIMBS> quotient_witnesses = { quotient_0, quotient_1, quotient_2, quotient_3 };

    // We will divide by shift_2 instantly in the relation itself, but first we need to compute the low part (0*0) and
    // the high part (0*1, 1*0) multiplied by a signle limb shift


I think what these expressions are doing is not so complicated but they're fairly hard to read/check. I wonder if some combination of breaking them up into smaller components plus adding more comments would be helpful.

I added a bit more in places, but I am not sure how to expand this

...berg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.cpp

AztecBot · 2023-10-12T13:57:06Z

Benchmark results

All benchmarks are run on txs on the Benchmarking contract on the repository. Each tx consists of a batch call to create_note and increment_balance, which guarantees that each tx has a private call, a nested private call, a public call, and a nested public call, as well as an emitted private note, an unencrypted log, and public storage read and write.

This benchmark source data is available in JSON format on S3 here.

Values are compared against data from master at commit 5622b506 and shown if the difference exceeds 1%.

L2 block published to L1

Each column represents the number of txs on an L2 block published to L1.

Metric	8 txs	32 txs	128 txs
l1_rollup_calldata_size_in_bytes	45,444	179,588	716,132
l1_rollup_calldata_gas	222,912	868,232	3,449,468
l1_rollup_execution_gas	841,999	3,595,340	22,204,837
l2_block_processing_time_in_ms	1,036 (+2%)	3,897 (+2%)	15,343 (+1%)
note_successful_decrypting_time_in_ms	326 (+3%)	992 (+3%)	3,647 (+3%)
note_trial_decrypting_time_in_ms	⚠️ 28.0 (+33%)	⚠️ 101 (+42%)	138 (+1%)
l2_block_building_time_in_ms	8,966 (+1%)	36,001 (+2%)	149,822 (+1%)
l2_block_rollup_simulation_time_in_ms	6,666 (+2%)	26,801 (+2%)	105,750 (+1%)
l2_block_public_tx_process_time_in_ms	2,256 (+1%)	9,073 (+2%)	43,397 (+1%)

L2 chain processing

Each column represents the number of blocks on the L2 chain where each block has 16 txs.

Metric	5 blocks	10 blocks
node_history_sync_time_in_ms	14,108 (-3%)	31,284 (-2%)
note_history_successful_decrypting_time_in_ms	2,314 (-2%)	4,763
note_history_trial_decrypting_time_in_ms	123 (+1%)	148
node_database_size_in_bytes	1,650,087	1,194,182
pxe_database_size_in_bytes	27,188	54,187

Circuits stats

Stats on running time and I/O sizes collected for every circuit run across all benchmarks.

Circuit	circuit_simulation_time_in_ms	circuit_input_size_in_bytes	circuit_output_size_in_bytes
private-kernel-init	43.4	56,577	14,745
private-kernel-ordering	21.5 (+1%)	20,137	8,089
base-rollup	852	631,605	811
root-rollup	37.7 (-1%)	4,072	1,097
private-kernel-inner	36.0 (+1%)	72,288	14,745
public-kernel-private-input	46.6 (-1%)	37,359	14,745
public-kernel-non-first-iteration	27.7 (-1%)	37,401	14,745
merge-rollup	0.884 (+3%)	2,592	873

Miscellaneous

Transaction sizes based on how many contracts are deployed in the tx.

Metric	0 deployed contracts	1 deployed contracts
tx_size_in_bytes	8,723	27,094

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-packages: 0.8.11</summary> ## [0.8.11](aztec-packages-v0.8.10...aztec-packages-v0.8.11) (2023-10-13) ### Features * **archiver:** Use registry to fetch searchStartBlock ([#2830](#2830)) ([e5bc067](e5bc067)) * Configure sandbox for network ([#2818](#2818)) ([d393a59](d393a59)) * **docker-sandbox:** Allow forks in sandbox ([#2831](#2831)) ([ed8431c](ed8431c)), closes [#2726](#2726) * Goblin Translator Decomposition relation (Goblin Translator part 4) ([#2802](#2802)) ([3c3cd9f](3c3cd9f)) * Goblin Translator GenPermSort relation (Goblin Translator part 3) ([#2795](#2795)) ([b36fdc4](b36fdc4)) * Goblin translator opcode constraint and accumulator transfer relations (Goblin Translator part 5) ([#2805](#2805)) ([b3d1f28](b3d1f28)) * Goblin Translator Permutation relation (Goblin Translator part 2) ([#2790](#2790)) ([9a354c9](9a354c9)) * Integrate ZeroMorph into Honk ([#2774](#2774)) ([ea86869](ea86869)) * Purge non native token + reorder params in token portal ([#2723](#2723)) ([447dade](447dade)) * Throw compile error if read/write public state from private ([#2804](#2804)) ([a3649df](a3649df)) * Unencrypted log filtering ([#2600](#2600)) ([7ae554a](7ae554a)), closes [#1498](#1498) [#1500](#1500) * Update goblin translator circuit builder (Goblin Translator part 1) ([#2764](#2764)) ([32c69ae](32c69ae)) ### Bug Fixes * Outdated `noir:clean` ([#2821](#2821)) ([2ea199f](2ea199f)) ### Miscellaneous * Benchmark tx sizes in p2p pool ([#2810](#2810)) ([f63219c](f63219c)) * Change acir_tests branch to point to master ([#2815](#2815)) ([73f229d](73f229d)) * Fix typo ([#2839](#2839)) ([5afdf91](5afdf91)) * From < genesis allowed in getBlocks ([#2816](#2816)) ([5622b50](5622b50)) * Remove Ultra Grumpkin flavor ([#2825](#2825)) ([bde77b8](bde77b8)) * Remove work queue from honk ([#2814](#2814)) ([bca7d12](bca7d12)) * Spell check ([#2817](#2817)) ([4777a11](4777a11)) ### Documentation * Slight changes to update portal page ([#2799](#2799)) ([eb65819](eb65819)) * Update aztec_connect_sunset.mdx ([#2808](#2808)) ([5f659a7](5f659a7)) </details> <details><summary>barretenberg.js: 0.8.11</summary> ## [0.8.11](barretenberg.js-v0.8.10...barretenberg.js-v0.8.11) (2023-10-13) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>barretenberg: 0.8.11</summary> ## [0.8.11](barretenberg-v0.8.10...barretenberg-v0.8.11) (2023-10-13) ### Features * Goblin Translator Decomposition relation (Goblin Translator part 4) ([#2802](#2802)) ([3c3cd9f](3c3cd9f)) * Goblin Translator GenPermSort relation (Goblin Translator part 3) ([#2795](#2795)) ([b36fdc4](b36fdc4)) * Goblin translator opcode constraint and accumulator transfer relations (Goblin Translator part 5) ([#2805](#2805)) ([b3d1f28](b3d1f28)) * Goblin Translator Permutation relation (Goblin Translator part 2) ([#2790](#2790)) ([9a354c9](9a354c9)) * Integrate ZeroMorph into Honk ([#2774](#2774)) ([ea86869](ea86869)) * Update goblin translator circuit builder (Goblin Translator part 1) ([#2764](#2764)) ([32c69ae](32c69ae)) ### Miscellaneous * Change acir_tests branch to point to master ([#2815](#2815)) ([73f229d](73f229d)) * Remove Ultra Grumpkin flavor ([#2825](#2825)) ([bde77b8](bde77b8)) * Remove work queue from honk ([#2814](#2814)) ([bca7d12](bca7d12)) * Spell check ([#2817](#2817)) ([4777a11](4777a11)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-packages: 0.8.11</summary> ## [0.8.11](AztecProtocol/aztec-packages@aztec-packages-v0.8.10...aztec-packages-v0.8.11) (2023-10-13) ### Features * **archiver:** Use registry to fetch searchStartBlock ([#2830](AztecProtocol/aztec-packages#2830)) ([e5bc067](AztecProtocol/aztec-packages@e5bc067)) * Configure sandbox for network ([#2818](AztecProtocol/aztec-packages#2818)) ([d393a59](AztecProtocol/aztec-packages@d393a59)) * **docker-sandbox:** Allow forks in sandbox ([#2831](AztecProtocol/aztec-packages#2831)) ([ed8431c](AztecProtocol/aztec-packages@ed8431c)), closes [#2726](AztecProtocol/aztec-packages#2726) * Goblin Translator Decomposition relation (Goblin Translator part 4) ([#2802](AztecProtocol/aztec-packages#2802)) ([3c3cd9f](AztecProtocol/aztec-packages@3c3cd9f)) * Goblin Translator GenPermSort relation (Goblin Translator part 3) ([#2795](AztecProtocol/aztec-packages#2795)) ([b36fdc4](AztecProtocol/aztec-packages@b36fdc4)) * Goblin translator opcode constraint and accumulator transfer relations (Goblin Translator part 5) ([#2805](AztecProtocol/aztec-packages#2805)) ([b3d1f28](AztecProtocol/aztec-packages@b3d1f28)) * Goblin Translator Permutation relation (Goblin Translator part 2) ([#2790](AztecProtocol/aztec-packages#2790)) ([9a354c9](AztecProtocol/aztec-packages@9a354c9)) * Integrate ZeroMorph into Honk ([#2774](AztecProtocol/aztec-packages#2774)) ([ea86869](AztecProtocol/aztec-packages@ea86869)) * Purge non native token + reorder params in token portal ([#2723](AztecProtocol/aztec-packages#2723)) ([447dade](AztecProtocol/aztec-packages@447dade)) * Throw compile error if read/write public state from private ([#2804](AztecProtocol/aztec-packages#2804)) ([a3649df](AztecProtocol/aztec-packages@a3649df)) * Unencrypted log filtering ([#2600](AztecProtocol/aztec-packages#2600)) ([7ae554a](AztecProtocol/aztec-packages@7ae554a)), closes [#1498](AztecProtocol/aztec-packages#1498) [#1500](AztecProtocol/aztec-packages#1500) * Update goblin translator circuit builder (Goblin Translator part 1) ([#2764](AztecProtocol/aztec-packages#2764)) ([32c69ae](AztecProtocol/aztec-packages@32c69ae)) ### Bug Fixes * Outdated `noir:clean` ([#2821](AztecProtocol/aztec-packages#2821)) ([2ea199f](AztecProtocol/aztec-packages@2ea199f)) ### Miscellaneous * Benchmark tx sizes in p2p pool ([#2810](AztecProtocol/aztec-packages#2810)) ([f63219c](AztecProtocol/aztec-packages@f63219c)) * Change acir_tests branch to point to master ([#2815](AztecProtocol/aztec-packages#2815)) ([73f229d](AztecProtocol/aztec-packages@73f229d)) * Fix typo ([#2839](AztecProtocol/aztec-packages#2839)) ([5afdf91](AztecProtocol/aztec-packages@5afdf91)) * From < genesis allowed in getBlocks ([#2816](AztecProtocol/aztec-packages#2816)) ([5622b50](AztecProtocol/aztec-packages@5622b50)) * Remove Ultra Grumpkin flavor ([#2825](AztecProtocol/aztec-packages#2825)) ([bde77b8](AztecProtocol/aztec-packages@bde77b8)) * Remove work queue from honk ([#2814](AztecProtocol/aztec-packages#2814)) ([bca7d12](AztecProtocol/aztec-packages@bca7d12)) * Spell check ([#2817](AztecProtocol/aztec-packages#2817)) ([4777a11](AztecProtocol/aztec-packages@4777a11)) ### Documentation * Slight changes to update portal page ([#2799](AztecProtocol/aztec-packages#2799)) ([eb65819](AztecProtocol/aztec-packages@eb65819)) * Update aztec_connect_sunset.mdx ([#2808](AztecProtocol/aztec-packages#2808)) ([5f659a7](AztecProtocol/aztec-packages@5f659a7)) </details> <details><summary>barretenberg.js: 0.8.11</summary> ## [0.8.11](AztecProtocol/aztec-packages@barretenberg.js-v0.8.10...barretenberg.js-v0.8.11) (2023-10-13) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>barretenberg: 0.8.11</summary> ## [0.8.11](AztecProtocol/aztec-packages@barretenberg-v0.8.10...barretenberg-v0.8.11) (2023-10-13) ### Features * Goblin Translator Decomposition relation (Goblin Translator part 4) ([#2802](AztecProtocol/aztec-packages#2802)) ([3c3cd9f](AztecProtocol/aztec-packages@3c3cd9f)) * Goblin Translator GenPermSort relation (Goblin Translator part 3) ([#2795](AztecProtocol/aztec-packages#2795)) ([b36fdc4](AztecProtocol/aztec-packages@b36fdc4)) * Goblin translator opcode constraint and accumulator transfer relations (Goblin Translator part 5) ([#2805](AztecProtocol/aztec-packages#2805)) ([b3d1f28](AztecProtocol/aztec-packages@b3d1f28)) * Goblin Translator Permutation relation (Goblin Translator part 2) ([#2790](AztecProtocol/aztec-packages#2790)) ([9a354c9](AztecProtocol/aztec-packages@9a354c9)) * Integrate ZeroMorph into Honk ([#2774](AztecProtocol/aztec-packages#2774)) ([ea86869](AztecProtocol/aztec-packages@ea86869)) * Update goblin translator circuit builder (Goblin Translator part 1) ([#2764](AztecProtocol/aztec-packages#2764)) ([32c69ae](AztecProtocol/aztec-packages@32c69ae)) ### Miscellaneous * Change acir_tests branch to point to master ([#2815](AztecProtocol/aztec-packages#2815)) ([73f229d](AztecProtocol/aztec-packages@73f229d)) * Remove Ultra Grumpkin flavor ([#2825](AztecProtocol/aztec-packages#2825)) ([bde77b8](AztecProtocol/aztec-packages@bde77b8)) * Remove work queue from honk ([#2814](AztecProtocol/aztec-packages#2814)) ([bca7d12](AztecProtocol/aztec-packages@bca7d12)) * Spell check ([#2817](AztecProtocol/aztec-packages#2817)) ([4777a11](AztecProtocol/aztec-packages@4777a11)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Rumata888 added the crypto cryptography label Oct 10, 2023

Rumata888 self-assigned this Oct 10, 2023

Rumata888 requested a review from ledwards2225 October 10, 2023 16:31

Rumata888 force-pushed the is/updated_goblin_translator_circuit_builder branch from 08f2ace to 0d146e8 Compare October 10, 2023 16:33

ledwards2225 approved these changes Oct 11, 2023

View reviewed changes

Rumata888 and others added 7 commits October 12, 2023 13:21

Final goblin translator circuit buidler

b37ffda

Fuzzer build fixed

b263a30

Remove header that confused wasm

15ecd5f

Added comments

6e91c51

Test rename

07e61bb

correct several minor typos found during review

be2a6dc

Addressed Luke's comments

2f7d21a

Rumata888 force-pushed the is/updated_goblin_translator_circuit_builder branch from 4b0b196 to 2f7d21a Compare October 12, 2023 13:22

Rumata888 merged commit 32c69ae into master Oct 12, 2023
2 checks passed

Rumata888 deleted the is/updated_goblin_translator_circuit_builder branch October 12, 2023 13:59

AztecBot mentioned this pull request Oct 12, 2023

chore(master): Release 0.8.11 #2811

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Update goblin translator circuit builder (Goblin Translator part 1) #2764

feat: Update goblin translator circuit builder (Goblin Translator part 1) #2764

Rumata888 commented Oct 10, 2023 •

edited

Loading

ledwards2225 left a comment

ledwards2225 Oct 11, 2023

Rumata888 Oct 12, 2023

ledwards2225 Oct 11, 2023

Rumata888 Oct 12, 2023

ledwards2225 Oct 11, 2023

Rumata888 Oct 12, 2023

AztecBot commented Oct 12, 2023

feat: Update goblin translator circuit builder (Goblin Translator part 1) #2764

feat: Update goblin translator circuit builder (Goblin Translator part 1) #2764

Conversation

Rumata888 commented Oct 10, 2023 • edited Loading

Checklist:

ledwards2225 left a comment

Choose a reason for hiding this comment

ledwards2225 Oct 11, 2023

Choose a reason for hiding this comment

Rumata888 Oct 12, 2023

Choose a reason for hiding this comment

ledwards2225 Oct 11, 2023

Choose a reason for hiding this comment

Rumata888 Oct 12, 2023

Choose a reason for hiding this comment

ledwards2225 Oct 11, 2023

Choose a reason for hiding this comment

Rumata888 Oct 12, 2023

Choose a reason for hiding this comment

AztecBot commented Oct 12, 2023

Benchmark results

L2 block published to L1

L2 chain processing

Circuits stats

Miscellaneous

Rumata888 commented Oct 10, 2023 •

edited

Loading