feat: Encapsulated Goblin #3524

ledwards2225 · 2023-12-01T22:10:14Z

Adds a class to encapsulate Goblin. Uses this to thread a single transcript through the two VMs for proper challenge generation. The Goblin class has an interface via function: accumulate (construct a proof and a merge proof), prove (run the VM proofs) and verify (a testing function to natively verify those proofs. Rewrites the Goblin tests accordingly. Adds tests that use recursive verification of Honk proofs. When we have recursive merge verification we will have something worth measuring the assess the performance of Goblin proving.

Closes AztecProtocol/barretenberg#785
Closes AztecProtocol/barretenberg#786

AztecBot · 2023-12-01T22:43:48Z

Benchmark results

Metrics with a significant change:

node_database_size_in_bytes (10): 5,825,967 (+49%)
note_history_trial_decrypting_time_in_ms (5): 236 (+47%)
note_trial_decrypting_time_in_ms (8): 18.4 (-32%)

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 ceb2ed26 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,948	868,136	3,449,420
l1_rollup_execution_gas	842,035	3,595,244	22,204,789
l2_block_processing_time_in_ms	2,228 (-6%)	8,454	35,097
note_successful_decrypting_time_in_ms	309 (-3%)	908	3,334
note_trial_decrypting_time_in_ms	⚠️ 18.4 (-32%)	34.4 (-15%)	200
l2_block_building_time_in_ms	20,150	80,352	321,313
l2_block_rollup_simulation_time_in_ms	16,625	66,479	265,550
l2_block_public_tx_process_time_in_ms	3,493	13,803	55,518

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	24,611 (-1%)	46,984 (-1%)
note_history_successful_decrypting_time_in_ms	2,148 (-1%)	4,148 (-1%)
note_history_trial_decrypting_time_in_ms	⚠️ 236 (+47%)	352 (-5%)
node_database_size_in_bytes	3,630,607	⚠️ 5,825,967 (+49%)
pxe_database_size_in_bytes	29,748	59,307

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	199	43,109	20,441
private-kernel-ordering	114	25,833	9,689
base-rollup	2,952	667,692	873
root-rollup	84.1	4,072	881
private-kernel-inner	260	64,516	20,441
public-kernel-private-input	171	25,203	20,441
public-kernel-non-first-iteration	168	25,245	20,441
merge-rollup	10.8	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	10,323	25,938

ledwards2225

LG, just left a few comments about comments.

ledwards2225 · 2023-12-06T14:38:22Z

barretenberg/cpp/src/barretenberg/goblin/full_goblin_composer.test.cpp

-    // Add mock data to op queue to simulate interaction with a "first" circuit
-    perform_op_queue_interactions_for_mock_first_circuit(op_queue);
+    // Construct an initial circuit; its proof will be recursively verified by the first kernel
+    info("Initial circuit.");


I never intended these prints to stick around but no strong feelings

ledwards2225 · 2023-12-06T14:40:06Z

barretenberg/cpp/src/barretenberg/goblin/full_goblin_composer.test.cpp


-        // The same composer is used to manage Honk and Merge prover/verifier
-        proof_system::honk::GoblinUltraComposer composer;
+        // Construct a circuit with logic resembling that of the "kernel circuit"


Its a bit misleading to use the kernel terminology here since the point of this test is that there's no recursion. Not a big deal

ledwards2225 · 2023-12-06T14:41:25Z

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

+    };
+
+    /**
+     * @brief A full goblin proof


Would you still call this a full Goblin proof?

ledwards2225 · 2023-12-06T14:41:52Z

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

+    using TranslatorConsistencyData = barretenberg::TranslationEvaluations;
+
+    std::shared_ptr<OpQueue> op_queue = std::make_shared<OpQueue>();
+    bool verified{ true };


I think this is leftover and unused?

ledwards2225 · 2023-12-06T14:42:29Z

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

+        auto prover = composer.create_prover(instance);
+        auto ultra_proof = prover.construct_proof();
+
+        // Construct and verify op queue merge proof


Just construct, not verify

ledwards2225 · 2023-12-06T14:46:29Z

barretenberg/cpp/src/barretenberg/transcript/transcript.hpp

@@ -4,8 +4,16 @@
 #include "barretenberg/crypto/blake3s/blake3s.hpp"
 #include "barretenberg/crypto/pedersen_hash/pedersen.hpp"

+// #define LOG_CHALLENGES


Are these supposed to be commented out?

ludamad · 2023-12-06T16:35:26Z

barretenberg/cpp/src/barretenberg/stdlib/recursion/goblin/full_goblin_recursion.test.cpp

+ */
+TEST_F(GoblinRecursionTests, Pseudo)
+{
+    barretenberg::Goblin goblin;


Appreciate the X x; and X x {y}; constructor usage, I find it nice to only see auto when it's actually needed

…work

ludamad · 2023-12-06T16:39:12Z

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

+#include "barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp"
+#include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp"
+#include "barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp"
+#include "barretenberg/translator_vm/goblin_translator_composer.hpp"


Charlie says often that any 'utils' naming is an antipattern and I somewhat agree. I think I agree in that often the exercise of thinking 'what if I didn't name this utils' can be fruitful, but disagree in that sometimes 'bundle of utilities needed to make X work' is GoodEnough(tm)

As an exercise here, I guess I'd call it mock_circuits.hpp instead

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-packages: 0.16.7</summary> ## [0.16.7](aztec-packages-v0.16.6...aztec-packages-v0.16.7) (2023-12-06) ### Features * Encapsulated Goblin ([#3524](#3524)) ([2f08423](2f08423)) ### Bug Fixes * Extract whole archive instead of subset ([#3604](#3604)) ([cb000d8](cb000d8)) ### Documentation * **yellow-paper:** Note hash, nullifier, and public data trees ([#3518](#3518)) ([0e2db8b](0e2db8b)) </details> <details><summary>barretenberg.js: 0.16.7</summary> ## [0.16.7](barretenberg.js-v0.16.6...barretenberg.js-v0.16.7) (2023-12-06) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>barretenberg: 0.16.7</summary> ## [0.16.7](barretenberg-v0.16.6...barretenberg-v0.16.7) (2023-12-06) ### Features * Encapsulated Goblin ([#3524](#3524)) ([2f08423](2f08423)) </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.16.7</summary> ## [0.16.7](AztecProtocol/aztec-packages@aztec-packages-v0.16.6...aztec-packages-v0.16.7) (2023-12-06) ### Features * Encapsulated Goblin ([#3524](AztecProtocol/aztec-packages#3524)) ([2f08423](AztecProtocol/aztec-packages@2f08423)) ### Bug Fixes * Extract whole archive instead of subset ([#3604](AztecProtocol/aztec-packages#3604)) ([cb000d8](AztecProtocol/aztec-packages@cb000d8)) ### Documentation * **yellow-paper:** Note hash, nullifier, and public data trees ([#3518](AztecProtocol/aztec-packages#3518)) ([0e2db8b](AztecProtocol/aztec-packages@0e2db8b)) </details> <details><summary>barretenberg.js: 0.16.7</summary> ## [0.16.7](AztecProtocol/aztec-packages@barretenberg.js-v0.16.6...barretenberg.js-v0.16.7) (2023-12-06) ### Miscellaneous * **barretenberg.js:** Synchronize aztec-packages versions </details> <details><summary>barretenberg: 0.16.7</summary> ## [0.16.7](AztecProtocol/aztec-packages@barretenberg-v0.16.6...barretenberg-v0.16.7) (2023-12-06) ### Features * Encapsulated Goblin ([#3524](AztecProtocol/aztec-packages#3524)) ([2f08423](AztecProtocol/aztec-packages@2f08423)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

ledwards2225 added 6 commits November 30, 2023 18:10

codys baseline changes

7950c3c

updates to test structure; passing; no recursion

d0314a5

construct recursive ver directly from native vkey

2e8ced6

new test suite; passes with 2 layers of recusion

3ebd005

full test with recursion is passing through trans

cfdbc3b

remove pseudo from composer tests

178f646

ledwards2225 self-assigned this Dec 1, 2023

codygunton and others added 7 commits December 4, 2023 15:45

Merge branch 'master' into lde-cg/full_test_rework

fa68496

const&

6ef21ac

const&

91f3b8c

basic goblin prove and verify structure

466cbf0

Merge branch 'master' into lde-cg/full_test_rework

616fd40

It works!

d2db965

VM verification split out

385edee

codygunton self-assigned this Dec 5, 2023

codygunton added 8 commits December 5, 2023 15:00

Bring tests in line w/ e/o

b1df3ae

WIP

4e332f4

Fix Translator test

c02db5a

Share testing code

a4b5e2f

Cleanup

4f5d1ea

Cleanup

ba98a33

Spawn todos.

55ec020

Remove comment

e075d93

codygunton requested a review from ludamad December 6, 2023 07:17

Remove stupid alias.

9d9f735

codygunton marked this pull request as ready for review December 6, 2023 07:19

codygunton changed the title ~~feat: Full Goblin recursion tests~~ feat: Encapsulated Goblin Dec 6, 2023

ledwards2225 commented Dec 6, 2023

View reviewed changes

Adjust comments per review.

c90e3bf

ludamad approved these changes Dec 6, 2023

View reviewed changes

ludamad reviewed Dec 6, 2023

View reviewed changes

codygunton added 2 commits December 6, 2023 16:36

Better modeling of kernel

60d7f56

Merge remote-tracking branch 'origin/master' into lde-cg/full_test_re…

977021e

…work

ludamad reviewed Dec 6, 2023

View reviewed changes

Rename utils.hpp

791ff7d

codygunton enabled auto-merge (squash) December 6, 2023 16:44

codygunton merged commit 2f08423 into master Dec 6, 2023
77 checks passed

codygunton deleted the lde-cg/full_test_rework branch December 6, 2023 17:20

AztecBot mentioned this pull request Dec 6, 2023

chore(master): Release 0.16.7 #3603

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Encapsulated Goblin #3524

feat: Encapsulated Goblin #3524

ledwards2225 commented Dec 1, 2023 •

edited

Loading

AztecBot commented Dec 1, 2023 •

edited

Loading

L2 block published to L1

L2 chain processing

Circuits stats

Miscellaneous

ledwards2225 left a comment

ledwards2225 Dec 6, 2023

ledwards2225 Dec 6, 2023

ledwards2225 Dec 6, 2023

ledwards2225 Dec 6, 2023

ledwards2225 Dec 6, 2023

ledwards2225 Dec 6, 2023

ledwards2225 Dec 6, 2023

ludamad Dec 6, 2023

ludamad Dec 6, 2023 •

edited

Loading

codygunton Dec 6, 2023

feat: Encapsulated Goblin #3524

feat: Encapsulated Goblin #3524

Conversation

ledwards2225 commented Dec 1, 2023 • edited Loading

AztecBot commented Dec 1, 2023 • edited Loading

Benchmark results

L2 block published to L1

L2 chain processing

Circuits stats

Miscellaneous

ledwards2225 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ludamad Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ledwards2225 commented Dec 1, 2023 •

edited

Loading

AztecBot commented Dec 1, 2023 •

edited

Loading

ludamad Dec 6, 2023 •

edited

Loading