chore: reorganise translator proving key construction #10853

maramihali · 2024-12-18T16:37:09Z

Translator ProverPolynomials were constructed in three different files:

permutation_lib.hpp which was part of the honk target
the witness was constructed in the TranslatorProver class
some polynomials were constructed in TranslatorFlavor

This PR introduces a TranslatorProvingKey aimed to unify the logic in a manner similar to DeciderProvingKey, in an attempt to make navigating state less confusing.

fcarreiro · 2024-12-18T16:50:50Z

barretenberg/cpp/src/barretenberg/vm/avm/generated/prover.cpp

@@ -6,7 +6,6 @@
 #include "barretenberg/common/constexpr_utils.hpp"
 #include "barretenberg/common/thread.hpp"
 #include "barretenberg/honk/proof_system/logderivative_library.hpp"
-#include "barretenberg/honk/proof_system/permutation_library.hpp"


Could you make this change in the prover.cpp.hbs file as well? 🙏

maramihali · 2024-12-19T14:35:34Z

barretenberg/cpp/src/barretenberg/translator_vm/translator_proving_key.hpp

+        // come as is, since they have to reflect the structure of polynomials in the first 4 wires, which we've
+        // commited to
+        for (auto [wire_poly, wire] : zip_view(proving_key->polynomials.get_wires(), circuit.wires)) {
+            parallel_for_range(circuit.num_gates, [&](size_t start, size_t end) {


This was added when merging master, a modification made as part of the CI work which I didn't want to disrupt in case it's intended for some reason

…ng-key

barretenberg/cpp/src/barretenberg/goblin/goblin.hpp

codygunton · 2024-12-19T15:29:58Z

barretenberg/cpp/src/barretenberg/translator_vm/relation_correctness.test.cpp

@@ -59,8 +58,10 @@ TEST_F(TranslatorRelationCorrectnessTests, Permutation)
    params.gamma = gamma;

    // Create storage for polynomials
-    auto proving_key = std::make_shared<ProvingKey>();
-
+    auto proving_key = std::make_shared<Flavor::ProvingKey>(full_circuit_size);


Please make and use an appropriate constructor.

codygunton · 2024-12-19T15:31:30Z

barretenberg/cpp/src/barretenberg/translator_vm/translator_flavor.hpp

-            , batching_challenge_v(builder.batching_challenge_v)
-            , evaluation_input_x(builder.evaluation_input_x)
+        ProvingKey(const size_t dyadic_circuit_size, std::shared_ptr<CommitmentKey> commitment_key = nullptr)
+            : Base(dyadic_circuit_size, 0, std::move(commitment_key))


Why move the commitment key?

If a shared_ptr is passed as value to the constructor (as it is here) moving it is the right thing to do. Also, passing by value and moving is the right pattern for smart pointers (clangd-format suggests it).

Note that I'm not judging IF the constructor should take a commitment key or not! That is, whether the addition makes sense.

barretenberg/cpp/src/barretenberg/translator_vm/translator_prover.cpp

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.68.1</summary> ## [0.68.1](aztec-package-v0.68.0...aztec-package-v0.68.1) (2024-12-23) ### Miscellaneous * Configurable parallelism in bootstrap ([#10909](#10909)) ([5260f1e](5260f1e)) </details> <details><summary>barretenberg.js: 0.68.1</summary> ## [0.68.1](barretenberg.js-v0.68.0...barretenberg.js-v0.68.1) (2024-12-23) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>aztec-packages: 0.68.1</summary> ## [0.68.1](aztec-packages-v0.68.0...aztec-packages-v0.68.1) (2024-12-23) ### Features * Add a warning when using unsafe blocks without safety comments (noir-lang/noir#6860) ([84a4005](84a4005)) * Add limit to unique contract call ([#10640](#10640)) ([d340f0b](d340f0b)) * Configurable external check failures (noir-lang/noir#6810) ([84a4005](84a4005)) * **docs:** Add aztec-wallet proving ([#10847](#10847)) ([3efae86](3efae86)) * Flatten nested if-else statements with equivalent conditions (noir-lang/noir#6875) ([84a4005](84a4005)) * **p2p:** Timeout peers, disconnect from badly scored peers ([#10907](#10907)) ([76a23eb](76a23eb)) * Replace `eval_global_as_array_length` with type/interpreter evaluation (noir-lang/noir#6469) ([84a4005](84a4005)) * Revamped sequencer timetable and tx processing timeout ([#10870](#10870)) ([145122b](145122b)) * Sync from aztec-packages (noir-lang/noir#6824) ([84a4005](84a4005)) * Warn on unnecessary unsafe blocks (noir-lang/noir#6867) ([84a4005](84a4005)) ### Bug Fixes * Add devcoin to faucet after deployment ([#10903](#10903)) ([6aa5369](6aa5369)) * CI kind test fix ([#10932](#10932)) ([bda1ac7](bda1ac7)) * **ci:** Tester/builder start race conditions ([#10893](#10893)) ([4250782](4250782)) * Conditionally deploy deterministic deployment proxy ([#10936](#10936)) ([48624b7](48624b7)) * Degrade libp2p crypto package ([#10876](#10876)) ([9293f38](9293f38)) * Detect cycles in globals (noir-lang/noir#6859) ([84a4005](84a4005)) * Don't deduplicate binary math of unsigned types (noir-lang/noir#6848) ([84a4005](84a4005)) * Double alias in path (noir-lang/noir#6855) ([84a4005](84a4005)) * Implement `as_field` and `from_field` in the interpreter (noir-lang/noir#6829) ([84a4005](84a4005)) * Install Yarn 4.5.2 to build WASM ([#10940](#10940)) ([2a76380](2a76380)) * Removed Sepolia stuff from devnet deploy action ([#10916](#10916)) ([fbf120b](fbf120b)) ### Miscellaneous * **avm:** Check that slice read/write are not out of memory range ([#10879](#10879)) ([ab3f318](ab3f318)), closes [#7385](#7385) * Avoid duplicate Not instructions during flattening (noir-lang/noir#6886) ([84a4005](84a4005)) * **ci:** Add non determinism check and fixes (noir-lang/noir#6847) ([84a4005](84a4005)) * **ci:** Display times in compilation and execution reports only with seconds (noir-lang/noir#6880) ([84a4005](84a4005)) * **ci:** Execution time report (noir-lang/noir#6827) ([84a4005](84a4005)) * **ci:** Take averages for compilation and execution report of small programs (noir-lang/noir#6874) ([84a4005](84a4005)) * Clean up gates reports script (noir-lang/noir#6896) ([84a4005](84a4005)) * Configurable parallelism in bootstrap ([#10909](#10909)) ([5260f1e](5260f1e)) * **docs:** Updating noirjs tutorial for 1.0.0 (noir-lang/noir#6792) ([84a4005](84a4005)) * **docs:** Updating the solidity contract how-to guide (noir-lang/noir#6804) ([84a4005](84a4005)) * Fix warnings (noir-lang/noir#6863) ([84a4005](84a4005)) * Have rust-analyzer use the stable toolchain (noir-lang/noir#6825) ([84a4005](84a4005)) * Move constant creation out of loop (noir-lang/noir#6836) ([84a4005](84a4005)) * Move empty programs to `compile_success_empty` (noir-lang/noir#6891) ([84a4005](84a4005)) * New default resource values for GKE ([#10928](#10928)) ([18e38d3](18e38d3)) * Quick docs fix for [#6839](#6839) (noir-lang/noir#6840) ([84a4005](84a4005)) * Refactor `DataFlowGraph.insert_instruction_and_results` (noir-lang/noir#6823) ([84a4005](84a4005)) * Remove get registered account from pxe ([#10479](#10479)) ([ee568ff](ee568ff)) * Remove malformed functions from brillig reports (noir-lang/noir#6898) ([84a4005](84a4005)) * Remove the `as_field` and `from_field` built-ins (noir-lang/noir#6845) ([84a4005](84a4005)) * Reorganise translator proving key construction ([#10853](#10853)) ([5da4d1b](5da4d1b)) * Replace relative paths to noir-protocol-circuits ([b9f9875](b9f9875)) * Update gates diff action ([#10917](#10917)) ([57439a7](57439a7)) * Use smallvec for instruction results (noir-lang/noir#6877) ([84a4005](84a4005)) * Use Vec for callstacks (noir-lang/noir#6821) ([84a4005](84a4005)) </details> <details><summary>barretenberg: 0.68.1</summary> ## [0.68.1](barretenberg-v0.68.0...barretenberg-v0.68.1) (2024-12-23) ### Features * Add limit to unique contract call ([#10640](#10640)) ([d340f0b](d340f0b)) ### Miscellaneous * **avm:** Check that slice read/write are not out of memory range ([#10879](#10879)) ([ab3f318](ab3f318)), closes [#7385](#7385) * Reorganise translator proving key construction ([#10853](#10853)) ([5da4d1b](5da4d1b)) </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.68.1</summary> ## [0.68.1](AztecProtocol/aztec-packages@aztec-package-v0.68.0...aztec-package-v0.68.1) (2024-12-23) ### Miscellaneous * Configurable parallelism in bootstrap ([#10909](AztecProtocol/aztec-packages#10909)) ([5260f1e](AztecProtocol/aztec-packages@5260f1e)) </details> <details><summary>barretenberg.js: 0.68.1</summary> ## [0.68.1](AztecProtocol/aztec-packages@barretenberg.js-v0.68.0...barretenberg.js-v0.68.1) (2024-12-23) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>aztec-packages: 0.68.1</summary> ## [0.68.1](AztecProtocol/aztec-packages@aztec-packages-v0.68.0...aztec-packages-v0.68.1) (2024-12-23) ### Features * Add a warning when using unsafe blocks without safety comments (noir-lang/noir#6860) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Add limit to unique contract call ([#10640](AztecProtocol/aztec-packages#10640)) ([d340f0b](AztecProtocol/aztec-packages@d340f0b)) * Configurable external check failures (noir-lang/noir#6810) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * **docs:** Add aztec-wallet proving ([#10847](AztecProtocol/aztec-packages#10847)) ([3efae86](AztecProtocol/aztec-packages@3efae86)) * Flatten nested if-else statements with equivalent conditions (noir-lang/noir#6875) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * **p2p:** Timeout peers, disconnect from badly scored peers ([#10907](AztecProtocol/aztec-packages#10907)) ([76a23eb](AztecProtocol/aztec-packages@76a23eb)) * Replace `eval_global_as_array_length` with type/interpreter evaluation (noir-lang/noir#6469) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Revamped sequencer timetable and tx processing timeout ([#10870](AztecProtocol/aztec-packages#10870)) ([145122b](AztecProtocol/aztec-packages@145122b)) * Sync from aztec-packages (noir-lang/noir#6824) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Warn on unnecessary unsafe blocks (noir-lang/noir#6867) ([84a4005](AztecProtocol/aztec-packages@84a4005)) ### Bug Fixes * Add devcoin to faucet after deployment ([#10903](AztecProtocol/aztec-packages#10903)) ([6aa5369](AztecProtocol/aztec-packages@6aa5369)) * CI kind test fix ([#10932](AztecProtocol/aztec-packages#10932)) ([bda1ac7](AztecProtocol/aztec-packages@bda1ac7)) * **ci:** Tester/builder start race conditions ([#10893](AztecProtocol/aztec-packages#10893)) ([4250782](AztecProtocol/aztec-packages@4250782)) * Conditionally deploy deterministic deployment proxy ([#10936](AztecProtocol/aztec-packages#10936)) ([48624b7](AztecProtocol/aztec-packages@48624b7)) * Degrade libp2p crypto package ([#10876](AztecProtocol/aztec-packages#10876)) ([9293f38](AztecProtocol/aztec-packages@9293f38)) * Detect cycles in globals (noir-lang/noir#6859) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Don't deduplicate binary math of unsigned types (noir-lang/noir#6848) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Double alias in path (noir-lang/noir#6855) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Implement `as_field` and `from_field` in the interpreter (noir-lang/noir#6829) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Install Yarn 4.5.2 to build WASM ([#10940](AztecProtocol/aztec-packages#10940)) ([2a76380](AztecProtocol/aztec-packages@2a76380)) * Removed Sepolia stuff from devnet deploy action ([#10916](AztecProtocol/aztec-packages#10916)) ([fbf120b](AztecProtocol/aztec-packages@fbf120b)) ### Miscellaneous * **avm:** Check that slice read/write are not out of memory range ([#10879](AztecProtocol/aztec-packages#10879)) ([ab3f318](AztecProtocol/aztec-packages@ab3f318)), closes [#7385](AztecProtocol/aztec-packages#7385) * Avoid duplicate Not instructions during flattening (noir-lang/noir#6886) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * **ci:** Add non determinism check and fixes (noir-lang/noir#6847) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * **ci:** Display times in compilation and execution reports only with seconds (noir-lang/noir#6880) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * **ci:** Execution time report (noir-lang/noir#6827) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * **ci:** Take averages for compilation and execution report of small programs (noir-lang/noir#6874) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Clean up gates reports script (noir-lang/noir#6896) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Configurable parallelism in bootstrap ([#10909](AztecProtocol/aztec-packages#10909)) ([5260f1e](AztecProtocol/aztec-packages@5260f1e)) * **docs:** Updating noirjs tutorial for 1.0.0 (noir-lang/noir#6792) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * **docs:** Updating the solidity contract how-to guide (noir-lang/noir#6804) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Fix warnings (noir-lang/noir#6863) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Have rust-analyzer use the stable toolchain (noir-lang/noir#6825) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Move constant creation out of loop (noir-lang/noir#6836) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Move empty programs to `compile_success_empty` (noir-lang/noir#6891) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * New default resource values for GKE ([#10928](AztecProtocol/aztec-packages#10928)) ([18e38d3](AztecProtocol/aztec-packages@18e38d3)) * Quick docs fix for [#6839](AztecProtocol/aztec-packages#6839) (noir-lang/noir#6840) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Refactor `DataFlowGraph.insert_instruction_and_results` (noir-lang/noir#6823) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Remove get registered account from pxe ([#10479](AztecProtocol/aztec-packages#10479)) ([ee568ff](AztecProtocol/aztec-packages@ee568ff)) * Remove malformed functions from brillig reports (noir-lang/noir#6898) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Remove the `as_field` and `from_field` built-ins (noir-lang/noir#6845) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Reorganise translator proving key construction ([#10853](AztecProtocol/aztec-packages#10853)) ([5da4d1b](AztecProtocol/aztec-packages@5da4d1b)) * Replace relative paths to noir-protocol-circuits ([b9f9875](AztecProtocol/aztec-packages@b9f9875)) * Update gates diff action ([#10917](AztecProtocol/aztec-packages#10917)) ([57439a7](AztecProtocol/aztec-packages@57439a7)) * Use smallvec for instruction results (noir-lang/noir#6877) ([84a4005](AztecProtocol/aztec-packages@84a4005)) * Use Vec for callstacks (noir-lang/noir#6821) ([84a4005](AztecProtocol/aztec-packages@84a4005)) </details> <details><summary>barretenberg: 0.68.1</summary> ## [0.68.1](AztecProtocol/aztec-packages@barretenberg-v0.68.0...barretenberg-v0.68.1) (2024-12-23) ### Features * Add limit to unique contract call ([#10640](AztecProtocol/aztec-packages#10640)) ([d340f0b](AztecProtocol/aztec-packages@d340f0b)) ### Miscellaneous * **avm:** Check that slice read/write are not out of memory range ([#10879](AztecProtocol/aztec-packages#10879)) ([ab3f318](AztecProtocol/aztec-packages@ab3f318)), closes [#7385](AztecProtocol/aztec-packages#7385) * Reorganise translator proving key construction ([#10853](AztecProtocol/aztec-packages#10853)) ([5da4d1b](AztecProtocol/aztec-packages@5da4d1b)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

create translator proving key

5389a89

maramihali requested review from jeanmon, IlyasRidhuan and fcarreiro as code owners December 18, 2024 16:37

fcarreiro reviewed Dec 18, 2024

View reviewed changes

cleanup and fix segfault; add modification to avm generated files

f9bfe44

maramihali requested a review from Maddiaa0 as a code owner December 19, 2024 14:22

maramihali commented Dec 19, 2024

View reviewed changes

Merge remote-tracking branch 'origin/master' into mm/translator-provi…

c088d12

…ng-key

codygunton approved these changes Dec 19, 2024

View reviewed changes

maramihali added 2 commits December 20, 2024 11:30

modifications from PR review

98e0945

fix gcc build

5383e10

maramihali merged commit 5da4d1b into master Dec 23, 2024
49 checks passed

maramihali deleted the mm/translator-proving-key branch December 23, 2024 14:58

AztecBot mentioned this pull request Dec 23, 2024

chore(master): Release 0.68.1 #10918

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: reorganise translator proving key construction #10853

chore: reorganise translator proving key construction #10853

maramihali commented Dec 18, 2024 •

edited by codygunton

Loading

fcarreiro Dec 18, 2024

maramihali Dec 19, 2024 •

edited

Loading

codygunton Dec 19, 2024

codygunton Dec 19, 2024

fcarreiro Dec 19, 2024

chore: reorganise translator proving key construction #10853

chore: reorganise translator proving key construction #10853

Conversation

maramihali commented Dec 18, 2024 • edited by codygunton Loading

fcarreiro Dec 18, 2024

Choose a reason for hiding this comment

maramihali Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

codygunton Dec 19, 2024

Choose a reason for hiding this comment

codygunton Dec 19, 2024

Choose a reason for hiding this comment

fcarreiro Dec 19, 2024

Choose a reason for hiding this comment

maramihali commented Dec 18, 2024 •

edited by codygunton

Loading

maramihali Dec 19, 2024 •

edited

Loading