Merge 067cbb0 into 62fc917

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

@@ -173,14 +173,14 @@ void ClientIVC::accumulate(ClientCircuit& circuit,
     circuit.add_pairing_point_accumulator(stdlib::recursion::init_default_agg_obj_indices<ClientCircuit>(circuit));
 
     // Construct the proving key for circuit
-    std::shared_ptr<DeciderProvingKey> proving_key;
-    if (!initialized) {
-        proving_key = std::make_shared<DeciderProvingKey>(circuit, trace_settings);
-        trace_usage_tracker = ExecutionTraceUsageTracker(trace_settings);
-    } else {
-        proving_key = std::make_shared<DeciderProvingKey>(circuit, trace_settings);
-    }
+    std::shared_ptr<DeciderProvingKey> proving_key = std::make_shared<DeciderProvingKey>(circuit, trace_settings);
 
+    // The commitment key is initialised with the number of points determined by the trace_settings' dyadic size. If a
+    // circuit overflows past the dyadic size the commitment key will not have enough points so we need to increase it
+    if (proving_key->proving_key.circuit_size > trace_settings.dyadic_size()) {
+        bn254_commitment_key = std::make_shared<CommitmentKey<curve::BN254>>(proving_key->proving_key.circuit_size);
+        goblin.commitment_key = bn254_commitment_key;
+    }
     proving_key->proving_key.commitment_key = bn254_commitment_key;
 
     vinfo("getting honk vk... precomputed?: ", precomputed_vk);
@@ -194,7 +194,8 @@ void ClientIVC::accumulate(ClientCircuit& circuit,
     }
     vinfo("set honk vk metadata");
 
-    // If this is the first circuit in the IVC, use oink to complete the decider proving key and generate an oink proof
+    // If this is the first circuit in the IVC, use oink to complete the decider proving key and generate an oink
+    // proof
     if (!initialized) {
         OinkProver<Flavor> oink_prover{ proving_key };
         vinfo("computing oink proof...");
@@ -212,8 +213,8 @@ void ClientIVC::accumulate(ClientCircuit& circuit,
 
         initialized = true;
     } else { // Otherwise, fold the new key into the accumulator
+        vinfo("computing folding proof");
         FoldingProver folding_prover({ fold_output.accumulator, proving_key }, trace_usage_tracker);
-        vinfo("constructed folding prover");
         fold_output = folding_prover.prove();
         vinfo("constructed folding proof");
 

barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp

@@ -136,7 +136,8 @@ class ClientIVC {
     bool initialized = false; // Is the IVC accumulator initialized
 
     ClientIVC(TraceSettings trace_settings = {}, bool auto_verify_mode = false)
-        : trace_settings(trace_settings)
+        : trace_usage_tracker(trace_settings)
+        , trace_settings(trace_settings)
         , auto_verify_mode(auto_verify_mode)
         , bn254_commitment_key(trace_settings.structure.has_value()
                                    ? std::make_shared<CommitmentKey<curve::BN254>>(trace_settings.dyadic_size())

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

@@ -2,9 +2,9 @@
 #include "barretenberg/client_ivc/test_bench_shared.hpp"
 #include "barretenberg/goblin/goblin.hpp"
 #include "barretenberg/goblin/mock_circuits.hpp"
+#include "barretenberg/protogalaxy/folding_test_utils.hpp"
 #include "barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp"
 #include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp"
-
 #include <gtest/gtest.h>
 
 using namespace bb;
@@ -403,5 +403,62 @@ TEST_F(ClientIVCTests, StructuredTraceOverflow)
         log2_num_gates += 1;
     }
 
+    EXPECT_TRUE(ivc.prove_and_verify());
+};
+
+/**
+ * @brief Test dynamic structured trace overflow block mechanism
+ * @details Tests the case where the required overflow capacity is not known until runtime. Accumulates two circuits,
+ * the second of which overflows the trace but not enough to change the dyadic circuit size and thus there is no need
+ * for a virtual size increase of the first key.
+ *
+ */
+TEST_F(ClientIVCTests, DynamicOverflow)
+{
+    // Define trace settings with zero overflow capacity
+    ClientIVC ivc{ { SMALL_TEST_STRUCTURE_FOR_OVERFLOWS, /*overflow_capacity=*/0 } };
+
+    MockCircuitProducer circuit_producer;
+
+    const size_t NUM_CIRCUITS = 2;
+
+    // define parameters for two circuits; the first fits within the structured trace, the second overflows
+    const std::vector<size_t> log2_num_arith_gates = { 14, 16 };
+    // Accumulate
+    for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
+        auto circuit = circuit_producer.create_next_circuit(ivc, log2_num_arith_gates[idx]);
+        ivc.accumulate(circuit);
+    }
+
+    EXPECT_EQ(check_accumulator_target_sum_manual(ivc.fold_output.accumulator), true);
+    EXPECT_TRUE(ivc.prove_and_verify());
+};
+
+/**
+ * @brief Test dynamic trace overflow where the dyadic circuit size also increases
+ * @details Accumulates two circuits, the second of which overflows the trace structure and leads to an increased dyadic
+ * circuit size. This requires the virtual size of the polynomials in the first key to be increased accordingly which
+ * should be handled automatically in PG/ClientIvc.
+ *
+ */
+TEST_F(ClientIVCTests, DynamicOverflowCircuitSizeChange)
+{
+    uint32_t overflow_capacity = 0;
+    // uint32_t overflow_capacity = 1 << 1;
+    ClientIVC ivc{ { SMALL_TEST_STRUCTURE_FOR_OVERFLOWS, overflow_capacity } };
+
+    MockCircuitProducer circuit_producer;
+
+    const size_t NUM_CIRCUITS = 2;
+
+    // define parameters for two circuits; the first fits within the structured trace, the second overflows
+    const std::vector<size_t> log2_num_arith_gates = { 14, 18 };
+    // Accumulate
+    for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
+        auto circuit = circuit_producer.create_next_circuit(ivc, log2_num_arith_gates[idx]);
+        ivc.accumulate(circuit);
+    }
+
+    EXPECT_EQ(check_accumulator_target_sum_manual(ivc.fold_output.accumulator), true);
     EXPECT_TRUE(ivc.prove_and_verify());
 };

barretenberg/cpp/src/barretenberg/commitment_schemes/kzg/kzg.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "../claim.hpp"
+#include "barretenberg/commitment_schemes/claim.hpp"
 #include "barretenberg/commitment_schemes/commitment_key.hpp"
 #include "barretenberg/commitment_schemes/utils/batch_mul_native.hpp"
 #include "barretenberg/commitment_schemes/verification_key.hpp"

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

@@ -18,6 +18,22 @@
 
 namespace bb {
 
+/**
+ * @brief An arbitrary but small-ish structuring that can be used for testing with non-trivial circuits in cases when
+ * they overflow
+ */
+static constexpr TraceStructure SMALL_TEST_STRUCTURE_FOR_OVERFLOWS{ .ecc_op = 1 << 14,
+                                                                    .pub_inputs = 1 << 14,
+                                                                    .busread = 1 << 14,
+                                                                    .arithmetic = 1 << 15,
+                                                                    .delta_range = 1 << 14,
+                                                                    .elliptic = 1 << 14,
+                                                                    .aux = 1 << 14,
+                                                                    .poseidon2_external = 1 << 14,
+                                                                    .poseidon2_internal = 1 << 15,
+                                                                    .lookup = 1 << 14,
+                                                                    .overflow = 0 };
+
 class GoblinMockCircuits {
   public:
     using Curve = curve::BN254;

barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/execution_trace_usage_tracker.hpp

@@ -31,6 +31,21 @@ struct ExecutionTraceUsageTracker {
     size_t max_databus_size = 0;
     size_t max_tables_size = 0;
 
+    // For printing only. Must match the order of the members in the arithmetization
+    static constexpr std::array<std::string_view, 13> block_labels{ "ecc_op",
+                                                                    "pub_inputs",
+                                                                    "busread",
+                                                                    "arithmetic",
+                                                                    "delta_range",
+                                                                    "elliptic",
+                                                                    "aux",
+                                                                    "poseidon2_external",
+                                                                    "poseidon2_internal",
+                                                                    "lookup",
+                                                                    "overflow",
+                                                                    "databus_table_data",
+                                                                    "lookup_table_data" };
+
     TraceSettings trace_settings;
 
     ExecutionTraceUsageTracker(const TraceSettings& trace_settings = TraceSettings{})
@@ -72,8 +87,12 @@ struct ExecutionTraceUsageTracker {
         }
 
         // The active ranges must also include the rows where the actual databus and lookup table data are stored.
-        // (Note: lookup tables are constructed at the end of the trace; databus data is constructed at the start).
-        size_t dyadic_circuit_size = fixed_sizes.get_structured_dyadic_size();
+        // (Note: lookup tables are constructed at the end of the trace; databus data is constructed at the start) so we
+        // need to determine the dyadic size for this. We call the size function on the current circuit which will have
+        // the same fixed block sizes but might also have an overflow block potentially influencing the dyadic circuit
+        // size.
+        // TODO(https://github.com/AztecProtocol/barretenberg/issues/1160)
+        const size_t dyadic_circuit_size = circuit.blocks.get_structured_dyadic_size();
 
         // TODO(https://github.com/AztecProtocol/barretenberg/issues/1152): should be able to use simply Range{ 0,
         // max_databus_size } but this breaks for certain choices of num_threads.
@@ -98,24 +117,13 @@ struct ExecutionTraceUsageTracker {
         });
     }
 
-    // For printing only. Must match the order of the members in the arithmetization
-    std::vector<std::string> block_labels{ "ecc_op",
-                                           "pub_inputs",
-                                           "busread",
-                                           "arithmetic",
-                                           "delta_range",
-                                           "elliptic",
-                                           "aux",
-                                           "poseidon2_external",
-                                           "poseidon2_internal",
-                                           "lookup",
-                                           "overflow" };
-
     void print()
     {
         info("Minimum required block sizes for structured trace: ");
-        for (auto [label, max_size] : zip_view(block_labels, max_sizes.get())) {
-            std::cout << std::left << std::setw(20) << (label + ":") << max_size << std::endl;
+        size_t idx = 0;
+        for (auto max_size : max_sizes.get()) {
+            std::cout << std::left << std::setw(20) << block_labels[idx] << ": " << max_size << std::endl;
+            idx++;
         }
         info("");
     }
@@ -124,8 +132,18 @@ struct ExecutionTraceUsageTracker {
     {
         info("Active regions of accumulator: ");
         for (auto [label, range] : zip_view(block_labels, active_ranges)) {
-            std::cout << std::left << std::setw(20) << (label + ":") << "(" << range.first << ", " << range.second
-                      << ")" << std::endl;
+            std::cout << std::left << std::setw(20) << label << ": (" << range.first << ", " << range.second << ")"
+                      << std::endl;
+        }
+        info("");
+    }
+
+    void print_previous_active_ranges()
+    {
+        info("Active regions of previous accumulator: ");
+        for (auto [label, range] : zip_view(block_labels, previous_active_ranges)) {
+            std::cout << std::left << std::setw(20) << label << ": (" << range.first << ", " << range.second << ")"
+                      << std::endl;
         }
         info("");
     }

barretenberg/cpp/src/barretenberg/plonk_honk_shared/execution_trace/mega_execution_trace.hpp

@@ -30,10 +30,17 @@ template <typename T> struct MegaTraceBlockData {
 
     std::vector<std::string_view> get_labels() const
     {
-        return { "ecc_op",     "pub_inputs",         "busread",
-                 "arithmetic", "delta_range",        "elliptic",
-                 "aux",        "poseidon2_external", "poseidon2_internal",
-                 "lookup" };
+        return { "ecc_op",
+                 "pub_inputs",
+                 "busread",
+                 "arithmetic",
+                 "delta_range",
+                 "elliptic",
+                 "aux",
+                 "poseidon2_external",
+                 "poseidon2_internal",
+                 "lookup",
+                 "overflow" };
     }
 
     auto get()
@@ -220,10 +227,10 @@ class MegaExecutionTraceBlocks : public MegaTraceBlockData<MegaTraceBlock> {
         info("");
     }
 
-    size_t get_structured_dyadic_size()
+    size_t get_structured_dyadic_size() const
     {
         size_t total_size = 1; // start at 1 because the 0th row is unused for selectors for Honk
-        for (auto block : this->get()) {
+        for (const auto& block : this->get()) {
             total_size += block.get_fixed_size();
         }
 

barretenberg/cpp/src/barretenberg/plonk_honk_shared/library/grand_product_delta.hpp

@@ -48,6 +48,9 @@ typename Flavor::FF compute_public_input_delta(std::span<const typename Flavor::
     // Note: The public inputs may be offset from the 0th index of the wires, for example due to the inclusion of an
     // initial zero row or Goblin-stlye ECC op gates. Accordingly, the indices i in the above formulas are given by i =
     // [0, m-1] + offset, i.e. i = offset, 1 + offset, …, m - 1 + offset.
+
+    // TODO(https://github.com/AztecProtocol/barretenberg/issues/1158): Ensure correct construction of public input
+    // delta in the face of increases to virtual size caused by execution trace overflow
     Field numerator_acc = gamma + (beta * Field(domain_size + offset));
     Field denominator_acc = gamma - beta * Field(1 + offset);
 

barretenberg/cpp/src/barretenberg/plonk_honk_shared/proving_key_inspector.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "barretenberg/common/assert.hpp"
 #include "barretenberg/common/log.hpp"
 
 namespace bb::proving_key_inspector {

barretenberg/cpp/src/barretenberg/plonk_honk_shared/relation_checker.cpp

@@ -0,0 +1,3 @@
+#include "relation_checker.hpp"
+
+// Hack to make the module compile.