Merge 55f1dec into 10ef970

barretenberg/cpp/src/barretenberg/circuit_checker/circuit_checker.cpp

@@ -34,34 +34,90 @@ template <typename Builder> bool CircuitChecker::check(const Builder& builder_in
     TagCheckData tag_data;
     MemoryCheckData memory_data{ builder };
 
+    bool result = true;
+    // TODO(https://github.com/AztecProtocol/barretenberg/issues/870): Currently we check all relations for each block.
+    // Once sorting is complete, is will be sufficient to check only the relevant relation(s) per block.
+    size_t block_idx = 0;
+    for (auto& block : builder.blocks.get()) {
+        result = result && check_block(builder, block, tag_data, memory_data, lookup_hash_table);
+        if (result == false) {
+            info("Failed at block idx = ", block_idx);
+            return false;
+        }
+        block_idx++;
+    }
+
+    // Tag check is only expected to pass after entire execution trace (all blocks) have been processed
+    result = result && check_tag_data(tag_data);
+    if (result == false) {
+        info("Failed tag check.");
+        return false;
+    }
+
+    return result;
+};
+
+template <typename Builder>
+bool CircuitChecker::check_block(Builder& builder,
+                                 auto& block,
+                                 TagCheckData& tag_data,
+                                 MemoryCheckData& memory_data,
+                                 LookupHashTable& lookup_hash_table)
+{
     // Initialize empty AllValues of the correct Flavor based on Builder type; for input to Relation::accumulate
     auto values = init_empty_values<Builder>();
     Params params;
     params.eta = memory_data.eta; // used in Auxiliary relation for RAM/ROM consistency
 
-    // TODO(https://github.com/AztecProtocol/barretenberg/issues/867): Once we sort gates into their respective blocks
-    // we'll need to either naively run this on all blocks or run only the relevant checks on each block.
-    auto& block = builder.blocks.main;
-
     // Perform checks on each gate defined in the builder
     bool result = true;
     for (size_t idx = 0; idx < block.size(); ++idx) {
+
         populate_values(builder, block, values, tag_data, memory_data, idx);
 
         result = result && check_relation<Arithmetic>(values, params);
+        if (result == false) {
+            info("Failed Arithmetic relation at row idx = ", idx);
+            return false;
+        }
         result = result && check_relation<Elliptic>(values, params);
+        if (result == false) {
+            info("Failed Elliptic relation at row idx = ", idx);
+            return false;
+        }
         result = result && check_relation<Auxiliary>(values, params);
+        if (result == false) {
+            info("Failed Auxiliary relation at row idx = ", idx);
+            return false;
+        }
         result = result && check_relation<GenPermSort>(values, params);
+        if (result == false) {
+            info("Failed GenPermSort relation at row idx = ", idx);
+            return false;
+        }
         result = result && check_lookup(values, lookup_hash_table);
+        if (result == false) {
+            info("Failed Lookup check relation at row idx = ", idx);
+            return false;
+        }
         if constexpr (IsGoblinBuilder<Builder>) {
             result = result && check_relation<PoseidonInternal>(values, params);
+            if (result == false) {
+                info("Failed PoseidonInternal relation at row idx = ", idx);
+                return false;
+            }
             result = result && check_relation<PoseidonExternal>(values, params);
+            if (result == false) {
+                info("Failed PoseidonExternal relation at row idx = ", idx);
+                return false;
+            }
+        }
+        if (result == false) {
+            info("Failed at row idx = ", idx);
+            return false;
         }
     }
 
-    // Tag check is only expected to pass after all gates have been processed
-    result = result && check_tag_data(tag_data);
-
     return result;
 };
 
@@ -132,27 +188,36 @@ void CircuitChecker::populate_values(
     values.w_l = builder.get_variable(block.w_l()[idx]);
     values.w_r = builder.get_variable(block.w_r()[idx]);
     values.w_o = builder.get_variable(block.w_o()[idx]);
-    if (memory_data.read_record_gates.contains(idx)) {
+    // Note: memory_data contains indices into the block to which RAM/ROM gates were added so we need to check that we
+    // are indexing into the correct block before updating the w_4 value.
+    if (block.has_ram_rom && memory_data.read_record_gates.contains(idx)) {
         values.w_4 = compute_memory_record_term(values.w_l, values.w_r, values.w_o, memory_data.eta);
-    } else if (memory_data.write_record_gates.contains(idx)) {
+    } else if (block.has_ram_rom && memory_data.write_record_gates.contains(idx)) {
         values.w_4 = compute_memory_record_term(values.w_l, values.w_r, values.w_o, memory_data.eta) + FF::one();
     } else {
         values.w_4 = builder.get_variable(block.w_4()[idx]);
     }
 
     // Set shifted wire values. Again, wire 4 is treated specially. On final row, set shift values to zero
-    values.w_l_shift = idx < block.size() - 1 ? builder.get_variable(block.w_l()[idx + 1]) : 0;
-    values.w_r_shift = idx < block.size() - 1 ? builder.get_variable(block.w_r()[idx + 1]) : 0;
-    values.w_o_shift = idx < block.size() - 1 ? builder.get_variable(block.w_o()[idx + 1]) : 0;
-    if (memory_data.read_record_gates.contains(idx + 1)) {
-        values.w_4_shift =
-            compute_memory_record_term(values.w_l_shift, values.w_r_shift, values.w_o_shift, memory_data.eta);
-    } else if (memory_data.write_record_gates.contains(idx + 1)) {
-        values.w_4_shift =
-            compute_memory_record_term(values.w_l_shift, values.w_r_shift, values.w_o_shift, memory_data.eta) +
-            FF::one();
+    if (idx < block.size() - 1) {
+        values.w_l_shift = builder.get_variable(block.w_l()[idx + 1]);
+        values.w_r_shift = builder.get_variable(block.w_r()[idx + 1]);
+        values.w_o_shift = builder.get_variable(block.w_o()[idx + 1]);
+        if (block.has_ram_rom && memory_data.read_record_gates.contains(idx + 1)) {
+            values.w_4_shift =
+                compute_memory_record_term(values.w_l_shift, values.w_r_shift, values.w_o_shift, memory_data.eta);
+        } else if (block.has_ram_rom && memory_data.write_record_gates.contains(idx + 1)) {
+            values.w_4_shift =
+                compute_memory_record_term(values.w_l_shift, values.w_r_shift, values.w_o_shift, memory_data.eta) +
+                FF::one();
+        } else {
+            values.w_4_shift = builder.get_variable(block.w_4()[idx + 1]);
+        }
     } else {
-        values.w_4_shift = idx < block.size() - 1 ? builder.get_variable(block.w_4()[idx + 1]) : 0;
+        values.w_l_shift = 0;
+        values.w_r_shift = 0;
+        values.w_o_shift = 0;
+        values.w_4_shift = 0;
     }
 
     // Update tag check data

barretenberg/cpp/src/barretenberg/circuit_checker/circuit_checker.hpp

@@ -64,8 +64,28 @@ class CircuitChecker {
     }
 
   private:
-    struct TagCheckData;
-    struct MemoryCheckData;
+    struct TagCheckData;           // Container for data pertaining to generalized permutation tag check
+    struct MemoryCheckData;        // Container for data pertaining to RAM/RAM record check
+    using Key = std::array<FF, 4>; // Key type for lookup table hash table
+    struct HashFunction;           // Custom hash function for lookup table hash table
+    using LookupHashTable = std::unordered_set<Key, HashFunction>;
+
+    /**
+     * @brief Checks that the provided witness satisfies all gates contained in a single execution trace block
+     *
+     * @tparam Builder
+     * @param builder
+     * @param block
+     * @param tag_data
+     * @param memory_data
+     * @param lookup_hash_table
+     */
+    template <typename Builder>
+    static bool check_block(Builder& builder,
+                            auto& block,
+                            TagCheckData& tag_data,
+                            MemoryCheckData& memory_data,
+                            LookupHashTable& lookup_hash_table);
 
     /**
      * @brief Check that a given relation is satisfied for the provided inputs corresponding to a single row
@@ -151,8 +171,7 @@ class CircuitChecker {
         }
     };
 
-    // Define a hash table for efficiently checking if lookups are present in the set of tables used by the circuit
-    using Key = std::array<FF, 4>; // key value is the four wire inputs for a lookup gates
+    // Hash for lookups hash table for efficiently checking if lookups are present in set of tables used by circuit
     struct HashFunction {
         const FF mult_const = FF(uint256_t(0x1337, 0x1336, 0x1335, 0x1334));
         const FF mc_sqr = mult_const.sqr();
@@ -164,6 +183,5 @@ class CircuitChecker {
             return static_cast<size_t>(result.reduce_once().data[0]);
         }
     };
-    using LookupHashTable = std::unordered_set<Key, HashFunction>;
 };
 } // namespace bb

barretenberg/cpp/src/barretenberg/examples/join_split/join_split.test.cpp

@@ -701,7 +701,7 @@ TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change)
     // The below part detects any changes in the join-split circuit
     constexpr size_t DYADIC_CIRCUIT_SIZE = 1 << 16;
 
-    constexpr uint256_t CIRCUIT_HASH("0x3792ae05102a73979a20d1962e30720ea083f87341a79f7714f356adbe670222");
+    constexpr uint256_t CIRCUIT_HASH("0x470358e4d91c4c5296ef788b1165b2c439cd498f49c3f99386b002753ca3d0ee");
 
     const uint256_t circuit_hash = circuit.hash_circuit();
     // circuit is finalized now

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

@@ -62,7 +62,7 @@ class GoblinMockCircuits {
     {
         // Determine number of times to execute the below operations that constitute the mock circuit logic. Note that
         // the circuit size does not scale linearly with number of iterations due to e.g. amortization of lookup costs
-        const size_t NUM_ITERATIONS_LARGE = 13; // results in circuit size 2^19 (521327 gates)
+        const size_t NUM_ITERATIONS_LARGE = 12; // results in circuit size 2^19 (502238 gates)
         const size_t NUM_ITERATIONS_MEDIUM = 3; // results in circuit size 2^17 (124843 gates)
         const size_t NUM_ITERATIONS = large ? NUM_ITERATIONS_LARGE : NUM_ITERATIONS_MEDIUM;
 

barretenberg/cpp/src/barretenberg/proof_system/arithmetization/arithmetization.hpp

@@ -151,16 +151,26 @@ template <typename FF_> class UltraArith {
     struct TraceBlocks {
         UltraTraceBlock pub_inputs;
         UltraTraceBlock arithmetic;
-        UltraTraceBlock sort;
+        UltraTraceBlock delta_range;
         UltraTraceBlock elliptic;
         UltraTraceBlock aux;
         UltraTraceBlock lookup;
-        UltraTraceBlock main;
 
-        // TODO(https://github.com/AztecProtocol/barretenberg/issues/867): update to aux.has_ram_rom = true
-        TraceBlocks() { main.has_ram_rom = true; }
+        TraceBlocks() { aux.has_ram_rom = true; }
 
-        auto get() { return RefArray{ pub_inputs, arithmetic, sort, elliptic, aux, lookup, main }; }
+        auto get() { return RefArray{ pub_inputs, arithmetic, delta_range, elliptic, aux, lookup }; }
+
+        void summarize()
+        {
+            info("Gate blocks summary:");
+            info("pub inputs:\t", pub_inputs.size());
+            info("arithmetic:\t", arithmetic.size());
+            info("delta range:\t", delta_range.size());
+            info("elliptic:\t", elliptic.size());
+            info("auxiliary:\t", aux.size());
+            info("lookups:\t", lookup.size());
+            info("");
+        }
 
         bool operator==(const TraceBlocks& other) const = default;
     };
@@ -244,21 +254,37 @@ template <typename FF_> class UltraHonkArith {
         UltraHonkTraceBlock ecc_op;
         UltraHonkTraceBlock pub_inputs;
         UltraHonkTraceBlock arithmetic;
-        UltraHonkTraceBlock sort;
+        // TODO(https://github.com/AztecProtocol/barretenberg/issues/919): Change: GenPermSort --> DeltaRangeConstraint
+        UltraHonkTraceBlock delta_range;
         UltraHonkTraceBlock elliptic;
         UltraHonkTraceBlock aux;
         UltraHonkTraceBlock lookup;
         UltraHonkTraceBlock busread;
         UltraHonkTraceBlock poseidon_external;
         UltraHonkTraceBlock poseidon_internal;
-        UltraHonkTraceBlock main;
 
-        TraceBlocks() { main.has_ram_rom = true; }
+        TraceBlocks() { aux.has_ram_rom = true; }
 
         auto get()
         {
-            return RefArray{ ecc_op,  pub_inputs,        arithmetic,        sort, elliptic, aux, lookup,
-                             busread, poseidon_external, poseidon_internal, main };
+            return RefArray{ ecc_op, pub_inputs, arithmetic, delta_range,       elliptic,
+                             aux,    lookup,     busread,    poseidon_external, poseidon_internal };
+        }
+
+        void summarize()
+        {
+            info("Gate blocks summary:");
+            info("goblin ecc op:\t", ecc_op.size());
+            info("pub inputs:\t", pub_inputs.size());
+            info("arithmetic:\t", arithmetic.size());
+            info("delta range:\t", delta_range.size());
+            info("elliptic:\t", elliptic.size());
+            info("auxiliary:\t", aux.size());
+            info("lookups:\t", lookup.size());
+            info("busread:\t", busread.size());
+            info("poseidon ext:\t", poseidon_external.size());
+            info("poseidon int:\t", poseidon_internal.size());
+            info("");
         }
 
         bool operator==(const TraceBlocks& other) const = default;