fix: set client ivc test vkeys properly by lucasxia01 · Pull Request #5230 · AztecProtocol/aztec-packages

lucasxia01 · 2024-03-14T22:13:38Z

This fix sets the verification key in client_ivc.test.cpp correctly for function circuits. Previously, the verification key was not being updated properly for the new function circuits.


--------------------------------------------------------------------------------
Benchmark                      Time             CPU   Iterations UserCounters...
--------------------------------------------------------------------------------
ClientIVCBench/Full/6      27297 ms        22546 ms            1 Decider::construct_proof=1 Decider::construct_proof(t)=776.493M ECCVMComposer::compute_commitment_key=1 ECCVMComposer::compute_commitment_key(t)=3.78121M ECCVMComposer::compute_witness=1 ECCVMComposer::compute_witness(t)=1.77129G ECCVMComposer::create_prover=1 ECCVMComposer::create_prover(t)=3.47306G ECCVMComposer::create_proving_key=1 ECCVMComposer::create_proving_key(t)=1.69774G ECCVMProver::construct_proof=1 ECCVMProver::construct_proof(t)=1.78864G Goblin::merge=11 Goblin::merge(t)=130.126M GoblinTranslatorCircuitBuilder::constructor=1 GoblinTranslatorCircuitBuilder::constructor(t)=59.0199M GoblinTranslatorComposer::create_prover=1 GoblinTranslatorComposer::create_prover(t)=119.07M GoblinTranslatorProver::construct_proof=1 GoblinTranslatorProver::construct_proof(t)=934.749M ProtoGalaxyProver_::accumulator_update_round=10 ProtoGalaxyProver_::accumulator_update_round(t)=720.581M ProtoGalaxyProver_::combiner_quotient_round=10 ProtoGalaxyProver_::combiner_quotient_round(t)=7.38002G ProtoGalaxyProver_::perturbator_round=10 ProtoGalaxyProver_::perturbator_round(t)=1.35437G ProtoGalaxyProver_::preparation_round=10 ProtoGalaxyProver_::preparation_round(t)=4.22303G ProtogalaxyProver::fold_instances=10 ProtogalaxyProver::fold_instances(t)=13.678G ProverInstance(Circuit&)=11 ProverInstance(Circuit&)(t)=1.98771G batch_mul_with_endomorphism=30 batch_mul_with_endomorphism(t)=569.263M commit=425 commit(t)=4.05584G compute_combiner=10 compute_combiner(t)=7.3778G compute_perturbator=9 compute_perturbator(t)=1.35396G compute_univariate=48 compute_univariate(t)=1.45556G construct_circuits=6 construct_circuits(t)=4.32967G
Benchmarking lock deleted.
client_ivc_bench.json                  100% 4008   109.7KB/s   00:00    
function                                        ms     % sum
construct_circuits(t)                         4330    15.91%
ProverInstance(Circuit&)(t)                   1988     7.30%
ProtogalaxyProver::fold_instances(t)         13678    50.25%
Decider::construct_proof(t)                    776     2.85%
ECCVMComposer::create_prover(t)               3473    12.76%
GoblinTranslatorComposer::create_prover(t)     119     0.44%
ECCVMProver::construct_proof(t)               1789     6.57%
GoblinTranslatorProver::construct_proof(t)     935     3.43%
Goblin::merge(t)                               130     0.48%

Total time accounted for: 27218ms/27297ms = 99.71%

Major contributors:
function                                        ms    % sum
commit(t)                                     4056   14.90%
compute_combiner(t)                           7378   27.11%
compute_perturbator(t)                        1354    4.97%
compute_univariate(t)                         1456    5.35%

Breakdown of ECCVMProver::create_prover:
ECCVMComposer::compute_witness(t)             1771    51.00%
ECCVMComposer::create_proving_key(t)          1698    48.88%

Breakdown of ProtogalaxyProver::fold_instances:
ProtoGalaxyProver_::preparation_round(t)           4223    30.87%
ProtoGalaxyProver_::perturbator_round(t)           1354     9.90%
ProtoGalaxyProver_::combiner_quotient_round(t)     7380    53.96%
ProtoGalaxyProver_::accumulator_update_round(t)     721     5.27%

Please read contributing guidelines and remove this line.

lucasxia01 · 2024-03-14T22:14:34Z

barretenberg/cpp/src/barretenberg/protogalaxy/protogalaxy_verifier.cpp

            transcript->template receive_from_prover<FF>(domain_separator + "_public_input_" + std::to_string(i));
-        inst->verification_key->public_inputs.emplace_back(public_input_i);
+        public_inputs.emplace_back(public_input_i);
+        ASSERT(public_input_i == inst->verification_key->public_inputs[i]);


actually check that the public inputs match the verification key.

this uncovered the ivc test bug

lucasxia01 · 2024-03-14T22:15:46Z

barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp

        // Accumulate function circuit
        Builder function_circuit = create_mock_circuit(ivc);
        FoldProof function_fold_proof = ivc.accumulate(function_circuit);
+        function_vk_with_merge = std::make_shared<VerificationKey>(ivc.prover_instance->proving_key);


we used to never reset function_vk_with_merge, so we were using the same vk for all function circuits, which worked generally since all of our function circuits were the same except for the public inputs and the witness values

I believe there is no need to reset the vk? the witness values and public inputs are not relevant for the vk. Am I missing something?

oh, right, the VerificationKey contains the public inputs.. but still the number of those should not change across function circuits in this test, hmm.. Also are we certain we want the PIs to be part of the VK?

lucasxia01 · 2024-03-14T22:16:27Z

barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp

        ivc.prover_fold_output.accumulator, kernel_fold_proof, foo_verifier_instance, kernel_vk);

-    VerifierFoldData kernel_fold_output = { kernel_fold_proof, function_vk_with_merge };
+    VerifierFoldData kernel_fold_output = { kernel_fold_proof, kernel_vk };


kernel_vk is the same as function_vk_with_merge here but its the correct one name-wise

AztecBot · 2024-03-14T22:45:07Z

Benchmark results

Metrics with a significant change:

tx_sequencer_processing_time_ms (0): 15.2 (+33%)
note_successful_decrypting_time_in_ms (8): 132 (-41%)

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,340	239,080	469,868
l1_rollup_execution_gas	667,021	953,990	1,337,225
l2_block_processing_time_in_ms	1,329	4,809 (-1%)	8,883 (-7%)
note_successful_decrypting_time_in_ms	⚠️ 132 (-41%)	488 (-12%)	988 (+5%)
note_trial_decrypting_time_in_ms	39.4 (+21%)	101 (+39%)	119 (+66%)
l2_block_building_time_in_ms	18,333 (-2%)	69,361	137,683 (+1%)
l2_block_rollup_simulation_time_in_ms	8,167 (-2%)	28,926	56,852
l2_block_public_tx_process_time_in_ms	10,145 (-2%)	40,379	80,729 (+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,164 (+2%)	27,726 (+4%)
note_history_successful_decrypting_time_in_ms	1,253 (+6%)	2,462 (-1%)
note_history_trial_decrypting_time_in_ms	109 (+34%)	152 (-12%)
node_database_size_in_bytes	19,075,152	35,651,664 (-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	282 (-1%)	44,370	27,700
private-kernel-ordering	217	52,324	14,326
base-parity	1,884 (+4%)	128	311
base-rollup	723	177,083	925
root-parity	1,462 (-7%)	1,244	311
root-rollup	68.7	4,487	789
private-kernel-inner	636 (-1%)	73,229	27,700
public-kernel-app-logic	448 (-1%)	35,262	28,215
public-kernel-tail	176 (-1%)	40,926	28,215
merge-rollup	8.26	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.1 (+1%)	16.0	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.590 (+1%)	0.497 (+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	46.3 (-1%)	72.0	230	447 (+1%)	865	1,719	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.475 (-1%)	0.445	0.421	0.418 (+1%)	0.413	0.412	N/A
batch_insert_into_indexed_tree_20_depth_ms	N/A	N/A	54.7	106 (-1%)	336 (-2%)	660	1,302 (-1%)	2,606	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.489	0.480 (-1%)	0.458 (-1%)	0.455	0.453 (-1%)	0.453	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	61.5
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.539 (+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,248 (-2%)	1,739 (-1%)

Metric	0 public data writes	1 public data writes
tx_sequencer_processing_time_ms	⚠️ 15.2 (+33%)	1,252

maramihali · 2024-03-15T10:19:49Z

barretenberg/cpp/src/barretenberg/ultra_honk/ultra_verifier.cpp

        auto public_input_i = transcript->template receive_from_prover<FF>("public_input_" + std::to_string(i));
        public_inputs.emplace_back(public_input_i);
+        if (public_input_i != key->public_inputs[i]) {
+            return false;


can you please add a comment here?

maramihali · 2024-03-15T10:21:35Z

barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp

        // Accumulate function circuit
        Builder function_circuit = create_mock_circuit(ivc);
        FoldProof function_fold_proof = ivc.accumulate(function_circuit);
+        function_vk_with_merge = std::make_shared<VerificationKey>(ivc.prover_instance->proving_key);


I believe there is no need to reset the vk? the witness values and public inputs are not relevant for the vk. Am I missing something?

maramihali · 2024-03-15T10:23:36Z

barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp

        // Accumulate function circuit
        Builder function_circuit = create_mock_circuit(ivc);
        FoldProof function_fold_proof = ivc.accumulate(function_circuit);
+        function_vk_with_merge = std::make_shared<VerificationKey>(ivc.prover_instance->proving_key);


oh, right, the VerificationKey contains the public inputs.. but still the number of those should not change across function circuits in this test, hmm.. Also are we certain we want the PIs to be part of the VK?

lucasxia01 · 2024-03-25T16:23:19Z

Replaced by #5315. Instead of comparing verification key public inputs with transcript public inputs, we just use the transcript ones. The verification key public inputs shouldn't exist.

fix client ivc test

835ad78

lucasxia01 commented Mar 14, 2024

View reviewed changes

maramihali self-requested a review March 15, 2024 10:23

maramihali assigned lucasxia01 Mar 15, 2024

maramihali added the crypto cryptography label Mar 15, 2024

maramihali reviewed Mar 15, 2024

View reviewed changes

lucasxia01 closed this Mar 25, 2024

ludamad deleted the lx/client-ivc-test-vkey-fix branch August 22, 2024 15:22

Conversation

lucasxia01 commented Mar 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucasxia01 Mar 14, 2024

Choose a reason for hiding this comment

Uh oh!

lucasxia01 Mar 14, 2024

Choose a reason for hiding this comment

Uh oh!

lucasxia01 Mar 14, 2024

Choose a reason for hiding this comment

Uh oh!

maramihali Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

maramihali Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucasxia01 Mar 14, 2024

Choose a reason for hiding this comment

Uh oh!

AztecBot commented Mar 14, 2024

Benchmark results

L2 block published to L1

L2 chain processing

Circuits stats

Tree insertion stats

Miscellaneous

Uh oh!

maramihali Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

maramihali Mar 15, 2024

Choose a reason for hiding this comment

Uh oh!

maramihali Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucasxia01 commented Mar 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lucasxia01 commented Mar 14, 2024 •

edited

Loading

maramihali Mar 15, 2024 •

edited

Loading

maramihali Mar 15, 2024 •

edited

Loading