refactor: No Translator composer #5202

codygunton · 2024-03-14T01:06:49Z

Get rid of the Translator composer. The eventual goal will be a a uniform interface across all proving systems (prover, verifier, keys all constructed in the same way, also sharing more code).

AztecBot · 2024-03-14T01:39:22Z

Benchmark results

Metrics with a significant change:

note_trial_decrypting_time_in_ms (64): 128 (+79%)

Detailed 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 c34a299e 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	64 txs
l1_rollup_calldata_size_in_bytes	5,668	18,820	36,356
l1_rollup_calldata_gas	66,280	239,080	469,904
l1_rollup_execution_gas	666,961	953,990	1,337,261
l2_block_processing_time_in_ms	1,342 (+1%)	4,984 (+3%)	9,386 (-1%)
note_successful_decrypting_time_in_ms	230 (+2%)	524 (-5%)	997 (+6%)
note_trial_decrypting_time_in_ms	35.6 (+9%)	40.6 (-44%)	⚠️ 128 (+79%)
l2_block_building_time_in_ms	18,499 (-1%)	70,089 (+1%)	137,028
l2_block_rollup_simulation_time_in_ms	8,232 (-1%)	29,650 (+2%)	56,831
l2_block_public_tx_process_time_in_ms	10,239 (-1%)	40,379	80,097

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,303 (+3%)	26,420 (-1%)
note_history_successful_decrypting_time_in_ms	1,234 (+4%)	2,452 (-1%)
note_history_trial_decrypting_time_in_ms	103 (+27%)	160 (-7%)
node_database_size_in_bytes	19,042,384	35,631,184 (-1%)
pxe_database_size_in_bytes	29,859	59,414

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	284	44,370	27,700
private-kernel-ordering	218	52,324	14,326
base-parity	1,901 (+5%)	128	311
base-rollup	730 (+1%)	177,083	925
root-parity	1,458 (-8%)	1,244	311
root-rollup	68.4 (-1%)	4,487	789
private-kernel-inner	641	73,229	27,700
public-kernel-app-logic	451	35,262	28,215
public-kernel-tail	176 (-1%)	40,926	28,215
merge-rollup	8.23	2,696	925

Tree insertion stats

The duration to insert a fixed batch of leaves into each tree type.

Metric	1 leaves	16 leaves	64 leaves	128 leaves	512 leaves	1024 leaves	2048 leaves	4096 leaves	32 leaves
batch_insert_into_append_only_tree_16_depth_ms	10.0	15.8 (-1%)	N/A	N/A	N/A	N/A	N/A	N/A	N/A
batch_insert_into_append_only_tree_16_depth_hash_count	16.8	31.6	N/A	N/A	N/A	N/A	N/A	N/A	N/A
batch_insert_into_append_only_tree_16_depth_hash_ms	0.586 (+1%)	0.490 (-1%)	N/A	N/A	N/A	N/A	N/A	N/A	N/A
batch_insert_into_append_only_tree_32_depth_ms	N/A	N/A	45.9 (-2%)	72.2	231	438 (-1%)	869	1,721 (+1%)	N/A
batch_insert_into_append_only_tree_32_depth_hash_count	N/A	N/A	96.0	159	543	1,055	2,079	4,127	N/A
batch_insert_into_append_only_tree_32_depth_hash_ms	N/A	N/A	0.471 (-2%)	0.445	0.420	0.410 (-1%)	0.413	0.413 (+1%)	N/A
batch_insert_into_indexed_tree_20_depth_ms	N/A	N/A	54.5 (-1%)	106 (-1%)	337 (-1%)	650 (-2%)	1,306 (-1%)	2,596	N/A
batch_insert_into_indexed_tree_20_depth_hash_count	N/A	N/A	104	207	691	1,363	2,707	5,395	N/A
batch_insert_into_indexed_tree_20_depth_hash_ms	N/A	N/A	0.485 (-1%)	0.478 (-1%)	0.458 (-1%)	0.448 (-2%)	0.454	0.451	N/A
batch_insert_into_indexed_tree_40_depth_ms	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	60.8 (-1%)
batch_insert_into_indexed_tree_40_depth_hash_count	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	109
batch_insert_into_indexed_tree_40_depth_hash_ms	N/A	N/A	N/A	N/A	N/A	N/A	N/A	N/A	0.532 (-1%)

Miscellaneous

Transaction sizes based on how many contract classes are registered in the tx.

Metric	0 registered classes
tx_size_in_bytes	22,014

Transaction processing duration by data writes.

Metric	0 new note hashes	1 new note hashes
tx_pxe_processing_time_ms	3,246 (-2%)	1,754

Metric	0 public data writes	1 public data writes
tx_sequencer_processing_time_ms	12.2 (+6%)	1,259

codygunton · 2024-03-14T16:48:45Z

barretenberg/cpp/src/barretenberg/flavor/goblin_translator.hpp


        // Expose constructors on the base class
        using Base = ProvingKey_<PrecomputedEntities<Polynomial>, WitnessEntities<Polynomial>, CommitmentKey>;
        using Base::Base;

+        ProvingKey() = default;
+        ProvingKey(const CircuitBuilder& builder)


Core change: the keys and their constructors, partially defined in the flavors, are now fully defined there. This means it makes sense to move the auxiliary functions, constants etc into the flavors too.

codygunton · 2024-03-14T16:54:51Z

barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_verifier.cpp

-{}
-
-GoblinTranslatorVerifier& GoblinTranslatorVerifier::operator=(GoblinTranslatorVerifier&& other) noexcept
-{


Doesn't seem like this is needed? Also, if you're move assigning, why clear the maps? They will just be default initialized, seems like that's fine? In any case: removing breaks nothing

codygunton · 2024-03-14T16:55:45Z

barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_verifier.cpp

@@ -268,7 +259,7 @@ bool GoblinTranslatorVerifier::verify_proof(const HonkProof& proof)
                                                                    commitments.get_concatenation_groups(),
                                                                    claimed_evaluations.get_concatenated_constraints());

-    auto verified = pcs_verification_key->pairing_check(pairing_points[0], pairing_points[1]);
+    auto verified = key->pcs_verification_key->pairing_check(pairing_points[0], pairing_points[1]);


From work on Ultra composer removal, the pcs_verification_key is already available through the verification key.

codygunton · 2024-03-14T16:58:32Z

barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_verifier.hpp

    std::shared_ptr<Transcript> transcript;
    RelationParameters<FF> relation_parameters;

    GoblinTranslatorVerifier(const std::shared_ptr<VerificationKey>& verifier_key = nullptr,
                             const std::shared_ptr<Transcript>& transcript = std::make_shared<Transcript>());
-    GoblinTranslatorVerifier(GoblinTranslatorVerifier&& other) noexcept;


Without custom special member functions, no reason to explicitly do any of this, right? Why delete copy constructor?

I don't actually remember why I did this

Rumata888 · 2024-03-19T14:36:31Z

barretenberg/cpp/src/barretenberg/honk/proof_system/permutation_library.hpp

@@ -1,6 +1,7 @@
 #pragma once
 #include "barretenberg/common/ref_vector.hpp"
 #include "barretenberg/polynomials/polynomial.hpp"
+#include "barretenberg/relations/relation_parameters.hpp"


Why did you add this header?

Yeah I just double checked and it is needed. I guess it was an implicit dependency that was satisfied through one of the deleted files? This is defined in a header and the error happens when build relation_objects, so it's a little hard to tell the cause.

Rumata888

Please check if the header that was added is not needed. Other than that LGTM

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.30.1</summary> ## [0.30.1](aztec-package-v0.30.0...aztec-package-v0.30.1) (2024-03-20) ### Miscellaneous * **aztec-package:** Synchronize aztec-packages versions </details> <details><summary>barretenberg.js: 0.30.1</summary> ## [0.30.1](barretenberg.js-v0.30.0...barretenberg.js-v0.30.1) (2024-03-20) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>aztec-cli: 0.30.1</summary> ## [0.30.1](aztec-cli-v0.30.0...aztec-cli-v0.30.1) (2024-03-20) ### Miscellaneous * **aztec-cli:** Synchronize aztec-packages versions </details> <details><summary>aztec-packages: 0.30.1</summary> ## [0.30.1](aztec-packages-v0.30.0...aztec-packages-v0.30.1) (2024-03-20) ### Features * Add CMOV instruction to brillig and brillig gen ([#5308](#5308)) ([208abbb](208abbb)) * **avm:** Indirect memory support for arithmetic/bitwise opcodes ([#5328](#5328)) ([d5ffa17](d5ffa17)), closes [#5273](#5273) * **avm:** Indirect memory support for MOV ([#5257](#5257)) ([10ef970](10ef970)), closes [#5205](#5205) * Merge SMT Terms in one class ([#5254](#5254)) ([f5c9b0f](f5c9b0f)) * Sorted execution trace ([#5252](#5252)) ([a216759](a216759)) ### Bug Fixes * Fix recursion tests and reinstate in CI ([#5300](#5300)) ([96c6f21](96c6f21)) * Skip uniswap l1 tests ([#5334](#5334)) ([7a56941](7a56941)) * Update smt_verification README.md ([#5332](#5332)) ([46b15e3](46b15e3)) ### Miscellaneous * Avm team as generated codeowners ([#5325](#5325)) ([06d2786](06d2786)) * No Translator composer ([#5202](#5202)) ([c8897ca](c8897ca)) * Remove toy vm files ([#5326](#5326)) ([d940356](d940356)) * Replace relative paths to noir-protocol-circuits ([ea2ac09](ea2ac09)) </details> <details><summary>barretenberg: 0.30.1</summary> ## [0.30.1](barretenberg-v0.30.0...barretenberg-v0.30.1) (2024-03-20) ### Features * Add CMOV instruction to brillig and brillig gen ([#5308](#5308)) ([208abbb](208abbb)) * **avm:** Indirect memory support for arithmetic/bitwise opcodes ([#5328](#5328)) ([d5ffa17](d5ffa17)), closes [#5273](#5273) * **avm:** Indirect memory support for MOV ([#5257](#5257)) ([10ef970](10ef970)), closes [#5205](#5205) * Merge SMT Terms in one class ([#5254](#5254)) ([f5c9b0f](f5c9b0f)) * Sorted execution trace ([#5252](#5252)) ([a216759](a216759)) ### Bug Fixes * Fix recursion tests and reinstate in CI ([#5300](#5300)) ([96c6f21](96c6f21)) * Update smt_verification README.md ([#5332](#5332)) ([46b15e3](46b15e3)) ### Miscellaneous * No Translator composer ([#5202](#5202)) ([c8897ca](c8897ca)) * Remove toy vm files ([#5326](#5326)) ([d940356](d940356)) </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-package: 0.30.1</summary> ## [0.30.1](AztecProtocol/aztec-packages@aztec-package-v0.30.0...aztec-package-v0.30.1) (2024-03-20) ### Miscellaneous * **aztec-package:** Synchronize aztec-packages versions </details> <details><summary>barretenberg.js: 0.30.1</summary> ## [0.30.1](AztecProtocol/aztec-packages@barretenberg.js-v0.30.0...barretenberg.js-v0.30.1) (2024-03-20) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>aztec-cli: 0.30.1</summary> ## [0.30.1](AztecProtocol/aztec-packages@aztec-cli-v0.30.0...aztec-cli-v0.30.1) (2024-03-20) ### Miscellaneous * **aztec-cli:** Synchronize aztec-packages versions </details> <details><summary>aztec-packages: 0.30.1</summary> ## [0.30.1](AztecProtocol/aztec-packages@aztec-packages-v0.30.0...aztec-packages-v0.30.1) (2024-03-20) ### Features * Add CMOV instruction to brillig and brillig gen ([#5308](AztecProtocol/aztec-packages#5308)) ([208abbb](AztecProtocol/aztec-packages@208abbb)) * **avm:** Indirect memory support for arithmetic/bitwise opcodes ([#5328](AztecProtocol/aztec-packages#5328)) ([d5ffa17](AztecProtocol/aztec-packages@d5ffa17)), closes [#5273](AztecProtocol/aztec-packages#5273) * **avm:** Indirect memory support for MOV ([#5257](AztecProtocol/aztec-packages#5257)) ([10ef970](AztecProtocol/aztec-packages@10ef970)), closes [#5205](AztecProtocol/aztec-packages#5205) * Merge SMT Terms in one class ([#5254](AztecProtocol/aztec-packages#5254)) ([f5c9b0f](AztecProtocol/aztec-packages@f5c9b0f)) * Sorted execution trace ([#5252](AztecProtocol/aztec-packages#5252)) ([a216759](AztecProtocol/aztec-packages@a216759)) ### Bug Fixes * Fix recursion tests and reinstate in CI ([#5300](AztecProtocol/aztec-packages#5300)) ([96c6f21](AztecProtocol/aztec-packages@96c6f21)) * Skip uniswap l1 tests ([#5334](AztecProtocol/aztec-packages#5334)) ([7a56941](AztecProtocol/aztec-packages@7a56941)) * Update smt_verification README.md ([#5332](AztecProtocol/aztec-packages#5332)) ([46b15e3](AztecProtocol/aztec-packages@46b15e3)) ### Miscellaneous * Avm team as generated codeowners ([#5325](AztecProtocol/aztec-packages#5325)) ([06d2786](AztecProtocol/aztec-packages@06d2786)) * No Translator composer ([#5202](AztecProtocol/aztec-packages#5202)) ([c8897ca](AztecProtocol/aztec-packages@c8897ca)) * Remove toy vm files ([#5326](AztecProtocol/aztec-packages#5326)) ([d940356](AztecProtocol/aztec-packages@d940356)) * Replace relative paths to noir-protocol-circuits ([ea2ac09](AztecProtocol/aztec-packages@ea2ac09)) </details> <details><summary>barretenberg: 0.30.1</summary> ## [0.30.1](AztecProtocol/aztec-packages@barretenberg-v0.30.0...barretenberg-v0.30.1) (2024-03-20) ### Features * Add CMOV instruction to brillig and brillig gen ([#5308](AztecProtocol/aztec-packages#5308)) ([208abbb](AztecProtocol/aztec-packages@208abbb)) * **avm:** Indirect memory support for arithmetic/bitwise opcodes ([#5328](AztecProtocol/aztec-packages#5328)) ([d5ffa17](AztecProtocol/aztec-packages@d5ffa17)), closes [#5273](AztecProtocol/aztec-packages#5273) * **avm:** Indirect memory support for MOV ([#5257](AztecProtocol/aztec-packages#5257)) ([10ef970](AztecProtocol/aztec-packages@10ef970)), closes [#5205](AztecProtocol/aztec-packages#5205) * Merge SMT Terms in one class ([#5254](AztecProtocol/aztec-packages#5254)) ([f5c9b0f](AztecProtocol/aztec-packages@f5c9b0f)) * Sorted execution trace ([#5252](AztecProtocol/aztec-packages#5252)) ([a216759](AztecProtocol/aztec-packages@a216759)) ### Bug Fixes * Fix recursion tests and reinstate in CI ([#5300](AztecProtocol/aztec-packages#5300)) ([96c6f21](AztecProtocol/aztec-packages@96c6f21)) * Update smt_verification README.md ([#5332](AztecProtocol/aztec-packages#5332)) ([46b15e3](AztecProtocol/aztec-packages@46b15e3)) ### Miscellaneous * No Translator composer ([#5202](AztecProtocol/aztec-packages#5202)) ([c8897ca](AztecProtocol/aztec-packages@c8897ca)) * Remove toy vm files ([#5326](AztecProtocol/aztec-packages#5326)) ([d940356](AztecProtocol/aztec-packages@d940356)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Get rid of the ECCVM composer, following the model of #5202 --------- Co-authored-by: ledwards2225 <[email protected]>

Get rid of the ECCVM composer, following the model of AztecProtocol/aztec-packages#5202 --------- Co-authored-by: ledwards2225 <[email protected]>

codygunton self-assigned this Mar 14, 2024

codygunton added 6 commits March 14, 2024 13:59

Remove create_prover and move related.

dcfa8e2

Nix composer; IOU key construction

573e739

Fix "composer" test

a6b2290

Delete compute_proving_key

ddfe3e4

Initialize key in prover

5f424c2

Fix fuzzer build

fd41719

codygunton force-pushed the cg/translator_composer_nix_1 branch from cd52021 to fd41719 Compare March 14, 2024 14:00

codygunton added 11 commits March 14, 2024 14:02

Actually fix Translator fuzzer

1220c6c

Remove compute_vk and compute vk

d19aa98

Header cleanup

b502558

Simplify verifier construction interface

e38e318

Fix fuzzing build again

eed7669

Remove unneeded function argument

f851b2b

Comment already resolved

cc24252

Resolve remaining todos

a0f2663

Reinstate and fix test

b6f94c5

Give better name

5002b62

Spawn and ref issue

be48921

codygunton commented Mar 14, 2024

View reviewed changes

codygunton marked this pull request as ready for review March 14, 2024 17:16

codygunton requested a review from Rumata888 March 14, 2024 17:16

Merge branch 'master' into cg/translator_composer_nix_1

c302749

Rumata888 reviewed Mar 19, 2024

View reviewed changes

Rumata888 approved these changes Mar 19, 2024

View reviewed changes

codygunton merged commit c8897ca into master Mar 19, 2024
97 checks passed

codygunton deleted the cg/translator_composer_nix_1 branch March 19, 2024 20:43

AztecBot mentioned this pull request Mar 19, 2024

chore(master): Release 0.30.1 #5322

Merged

codygunton mentioned this pull request Apr 5, 2024

refactor: Get rid of ECCVM composer #5562

Merged

ledwards2225 added a commit that referenced this pull request Apr 8, 2024

refactor: Get rid of ECCVM composer (#5562)

43ed901

Get rid of the ECCVM composer, following the model of #5202 --------- Co-authored-by: ledwards2225 <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: No Translator composer #5202

refactor: No Translator composer #5202

codygunton commented Mar 14, 2024 •

edited

Loading

AztecBot commented Mar 14, 2024 •

edited

Loading

L2 block published to L1

L2 chain processing

Circuits stats

Tree insertion stats

Miscellaneous

codygunton Mar 14, 2024

codygunton Mar 14, 2024

codygunton Mar 14, 2024

codygunton Mar 14, 2024

Rumata888 Mar 19, 2024

Rumata888 Mar 19, 2024

codygunton Mar 19, 2024

Rumata888 left a comment

refactor: No Translator composer #5202

refactor: No Translator composer #5202

Conversation

codygunton commented Mar 14, 2024 • edited Loading

AztecBot commented Mar 14, 2024 • edited Loading

Benchmark results

L2 block published to L1

L2 chain processing

Circuits stats

Tree insertion stats

Miscellaneous

codygunton Mar 14, 2024

Choose a reason for hiding this comment

codygunton Mar 14, 2024

Choose a reason for hiding this comment

codygunton Mar 14, 2024

Choose a reason for hiding this comment

codygunton Mar 14, 2024

Choose a reason for hiding this comment

Rumata888 Mar 19, 2024

Choose a reason for hiding this comment

Rumata888 Mar 19, 2024

Choose a reason for hiding this comment

codygunton Mar 19, 2024

Choose a reason for hiding this comment

Rumata888 left a comment

Choose a reason for hiding this comment

codygunton commented Mar 14, 2024 •

edited

Loading

AztecBot commented Mar 14, 2024 •

edited

Loading