From 6c54a7b7b021922c3d6e2e9019a46afb551b35b6 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Tue, 3 Feb 2026 10:26:54 +0000 Subject: [PATCH 1/8] cherchore: graph tooling audit 0 (#19691) Primary changes: - Replace methods of the form `get_*_gate_connected_component` with `GatePattern` structs that specify the conditions under which each wire is constrained for each gate type - Test correctness of `GatePattern`'s by perturbing relation inputs to empirically check which wires are constrained (`gate_patterns.test.cpp`) - Resolves a few bugs/errors identified by the aforementioned tests (see PR comments) - Use `update_used_witnesses` in `fix_witness` to avoid need for ad-hoc handling in the tooling Cleanup: - Replace use of `block_idx` with reference to `block` in several places for improved clarity --------- Co-authored-by: Claude Opus 4.5 --- .../gate_patterns.hpp | 404 +++++++++ .../gate_patterns.test.cpp | 421 +++++++++ .../boomerang_value_detection/graph.cpp | 809 +++++------------- .../boomerang_value_detection/graph.hpp | 42 +- .../graph_description.test.cpp | 42 +- ...cription_merge_recursive_verifier.test.cpp | 12 +- .../ultra_recursive_verifier.test.cpp | 96 +++ .../ultra_circuit_builder.cpp | 3 + 8 files changed, 1179 insertions(+), 650 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/boomerang_value_detection/gate_patterns.hpp create mode 100644 barretenberg/cpp/src/barretenberg/boomerang_value_detection/gate_patterns.test.cpp diff --git a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/gate_patterns.hpp b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/gate_patterns.hpp new file mode 100644 index 000000000000..256e545c8ccd --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/gate_patterns.hpp @@ -0,0 +1,404 @@ +// === AUDIT STATUS === +// internal: { status: Planned, auditors: [], commit: } +// external_1: { status: not started, auditors: [], commit: } +// external_2: { status: not started, auditors: [], commit: } +// ===================== + +#pragma once +#include +#include +#include +#include +#include + +namespace bb::gate_patterns { + +enum class Wire : uint8_t { + W_L, + W_R, + W_O, + W_4, + W_L_SHIFT, + W_R_SHIFT, + W_O_SHIFT, + W_4_SHIFT, +}; + +/** + * @brief Selector values read from a gate + * + * Each block type only activates one relation, so we don't need to check + * which relation is active. We just need the selector values to determine + * which wires are constrained within that relation. + */ +struct Selectors { + // The gate selector value (q_arith, q_elliptic, etc. depending on block) + int64_t gate_selector = 0; + + // Secondary selectors (can be arbitrary field elements, so track non-zero) + bool q_m_nz = false; + bool q_1_nz = false; + bool q_2_nz = false; + bool q_3_nz = false; + bool q_4_nz = false; + bool q_c_nz = false; +}; + +using Predicate = std::function; + +struct WireSpec { + Wire wire; + Predicate condition = [](const Selectors&) { return true; }; +}; + +/** + * @brief Pattern defining which wires are constrained by a gate type + * + * Each block type maps to exactly one pattern. The pattern specifies + * which wires are constrained based on selector values. + */ +struct GatePattern { + std::string_view name; + std::vector wires; +}; + +// ============================================================================ +// Arithmetic Pattern (from ultra_arithmetic_relation.hpp) +// +// Subrelation 1: +// q_arith * [ (-1/2) * (q_arith - 3) * (q_m * w_1 * w_2) +// + q_1*w_1 + q_2*w_2 + q_3*w_3 + q_4*w_4 + q_c +// + (q_arith - 1) * w_4_shift ] +// +// Subrelation 2 (only when q_arith == 3): +// q_arith * (q_arith-1) * (q_arith-2) * (w_1 + w_4 - w_1_shift + q_m) +// +// gate_selector = q_arith +// ============================================================================ + +inline const GatePattern + ARITHMETIC = { .name = "arithmetic", + .wires = { + // w_l: linear term OR mul term (disabled when q_arith==3) OR subrel2 + { Wire::W_L, + [](const Selectors& sel) { + return sel.q_1_nz || (sel.q_m_nz && sel.gate_selector != 3) || sel.gate_selector == 3; + } }, + // w_r: linear term OR mul term (disabled when q_arith==3) + { Wire::W_R, + [](const Selectors& sel) { return sel.q_2_nz || (sel.q_m_nz && sel.gate_selector != 3); } }, + // w_o: linear term + { Wire::W_O, [](const Selectors& sel) { return sel.q_3_nz; } }, + // w_4: linear term OR subrel2 (subrel2 active when q_arith == 3) + { Wire::W_4, [](const Selectors& sel) { return sel.q_4_nz || sel.gate_selector == 3; } }, + // w_4_shift: when q_arith == 2 or 3 (subrel1 has (q_arith - 1) * w_4_shift) + { Wire::W_4_SHIFT, + [](const Selectors& sel) { return sel.gate_selector == 2 || sel.gate_selector == 3; } }, + // w_l_shift: subrel2 when q_arith == 3 + { Wire::W_L_SHIFT, [](const Selectors& sel) { return sel.gate_selector == 3; } }, + } }; + +// ============================================================================ +// Elliptic Pattern (from elliptic_relation.hpp) +// +// Point addition (q_is_double == 0, i.e., q_m == 0): +// x1 = w_r, y1 = w_o, x2 = w_l_shift, x3 = w_r_shift, y3 = w_o_shift, y2 = w_4_shift +// +// Point doubling (q_is_double == 1, i.e., q_m == 1): +// x1 = w_r, y1 = w_o, x3 = w_r_shift, y3 = w_o_shift +// +// gate_selector = q_elliptic +// ============================================================================ + +inline const GatePattern ELLIPTIC = { .name = "elliptic", + .wires = { + // x1, y1: always used (both addition and doubling) + { Wire::W_R, [](const Selectors&) { return true; } }, + { Wire::W_O, [](const Selectors&) { return true; } }, + // x3, y3: always used (both addition and doubling) + { Wire::W_R_SHIFT, [](const Selectors&) { return true; } }, + { Wire::W_O_SHIFT, [](const Selectors&) { return true; } }, + // x2: only for addition (q_m == 0) + { Wire::W_L_SHIFT, [](const Selectors& sel) { return !sel.q_m_nz; } }, + // y2: only for addition (q_m == 0) + { Wire::W_4_SHIFT, [](const Selectors& sel) { return !sel.q_m_nz; } }, + } }; + +// ============================================================================ +// Non-Native Field Pattern (from non_native_field_relation.hpp) +// +// | gate type | q_2 | q_3 | q_4 | q_m | wires constrained | +// |---------------|-----|-----|-----|-----|------------------------------------------------| +// | Limb Accum 1 | 0 | 1 | 1 | 0 | w_l, w_r, w_o, w_4, w_l', w_r' | +// | Limb Accum 2 | 0 | 1 | 0 | 1 | w_o, w_4, w_l', w_r', w_o', w_4' | +// | Product 1 | 1 | 1 | 0 | 0 | w_l, w_r, w_o, w_4, w_l', w_r' | +// | Product 2 | 1 | 0 | 1 | 0 | all 8 wires | +// | Product 3 | 1 | 0 | 0 | 1 | w_l, w_r, w_4, w_l', w_r', w_o', w_4' | +// +// gate_selector = q_nnf +// ============================================================================ + +namespace nnf_helpers { +// Limb Accum 2: !q_2 && q_3 && !q_4 && q_m +inline bool is_limb_accum_2(const Selectors& sel) +{ + return !sel.q_2_nz && sel.q_3_nz && !sel.q_4_nz && sel.q_m_nz; +} +// Product 3: q_2 && !q_3 && !q_4 && q_m +inline bool is_product_3(const Selectors& sel) +{ + return sel.q_2_nz && !sel.q_3_nz && !sel.q_4_nz && sel.q_m_nz; +} +} // namespace nnf_helpers + +inline const GatePattern + NON_NATIVE_FIELD = { .name = "non_native_field", + .wires = { + // w_l, w_r: all gates except Limb Accum 2 + { Wire::W_L, [](const Selectors& sel) { return !nnf_helpers::is_limb_accum_2(sel); } }, + { Wire::W_R, [](const Selectors& sel) { return !nnf_helpers::is_limb_accum_2(sel); } }, + // w_o: all gates except Product 3 + { Wire::W_O, [](const Selectors& sel) { return !nnf_helpers::is_product_3(sel); } }, + // w_4: all gates + { Wire::W_4, [](const Selectors&) { return true; } }, + // w_l_shift, w_r_shift: all gates + { Wire::W_L_SHIFT, [](const Selectors&) { return true; } }, + { Wire::W_R_SHIFT, [](const Selectors&) { return true; } }, + // w_o_shift, w_4_shift: Limb Accum 2, Product 2, Product 3 + // = q_m || (q_2 && q_4) + { Wire::W_O_SHIFT, + [](const Selectors& sel) { return sel.q_m_nz || (sel.q_2_nz && sel.q_4_nz); } }, + { Wire::W_4_SHIFT, + [](const Selectors& sel) { return sel.q_m_nz || (sel.q_2_nz && sel.q_4_nz); } }, + } }; + +// ============================================================================ +// Memory Pattern (from memory_relation.hpp) +// +// | gate type | q_1 | q_2 | q_3 | q_4 | q_m | wires constrained | +// |---------------------|-----|-----|-----|-----|-----|------------------------------| +// | RAM/ROM access | 1 | 0 | 0 | 0 | 1 | w_l, w_r, w_o, w_4 | +// | RAM timestamp check | 1 | 0 | 0 | 1 | 0 | w_l, w_r, w_o, w_l', w_r' | +// | ROM consistency | 1 | 1 | 0 | 0 | 0 | w_l, w_l', w_4, w_4' | +// | RAM consistency | 0 | 0 | 1 | 0 | 0 | all 8 wires | +// +// gate_selector = q_memory +// ============================================================================ + +namespace memory_helpers { +// RAM timestamp check: q_1 && q_4 +inline bool is_timestamp_check(const Selectors& sel) +{ + return sel.q_1_nz && sel.q_4_nz; +} +// ROM consistency: q_1 && q_2 +inline bool is_rom_consistency(const Selectors& sel) +{ + return sel.q_1_nz && sel.q_2_nz; +} +// RAM consistency: q_3 +inline bool is_ram_consistency(const Selectors& sel) +{ + return sel.q_3_nz; +} +} // namespace memory_helpers + +// Note: Access gates (q_1 && q_m) are NOT handled here - they're processed separately +// via ROM/RAM transcript methods. +inline const GatePattern + MEMORY = { .name = "memory", + .wires = { + { Wire::W_L, + [](const Selectors& sel) { + return memory_helpers::is_timestamp_check(sel) || memory_helpers::is_rom_consistency(sel) || + memory_helpers::is_ram_consistency(sel); + } }, + { Wire::W_R, + [](const Selectors& sel) { + return memory_helpers::is_timestamp_check(sel) || memory_helpers::is_rom_consistency(sel) || + memory_helpers::is_ram_consistency(sel); + } }, + { Wire::W_O, + [](const Selectors& sel) { + return memory_helpers::is_timestamp_check(sel) || memory_helpers::is_rom_consistency(sel) || + memory_helpers::is_ram_consistency(sel); + } }, + { Wire::W_4, + [](const Selectors& sel) { + return memory_helpers::is_rom_consistency(sel) || memory_helpers::is_ram_consistency(sel); + } }, + { Wire::W_L_SHIFT, + [](const Selectors& sel) { + return memory_helpers::is_timestamp_check(sel) || memory_helpers::is_rom_consistency(sel) || + memory_helpers::is_ram_consistency(sel); + } }, + { Wire::W_R_SHIFT, + [](const Selectors& sel) { + return memory_helpers::is_timestamp_check(sel) || memory_helpers::is_ram_consistency(sel); + } }, + { Wire::W_O_SHIFT, [](const Selectors& sel) { return memory_helpers::is_ram_consistency(sel); } }, + { Wire::W_4_SHIFT, + [](const Selectors& sel) { + return memory_helpers::is_rom_consistency(sel) || memory_helpers::is_ram_consistency(sel); + } }, + } }; + +// ============================================================================ +// Lookup Pattern (from logderiv_lookup_relation.hpp) +// +// The read term uses: w_l, w_r, w_o (always) +// Shifted wires used when step_size != 0: +// w_l_shift if q_2 (q_r) != 0 +// w_r_shift if q_m != 0 +// w_o_shift if q_c != 0 +// +// gate_selector = q_lookup +// ============================================================================ + +inline const GatePattern LOOKUP = { .name = "lookup", + .wires = { + { Wire::W_L, [](const Selectors&) { return true; } }, + { Wire::W_R, [](const Selectors&) { return true; } }, + { Wire::W_O, [](const Selectors&) { return true; } }, + { Wire::W_L_SHIFT, [](const Selectors& sel) { return sel.q_2_nz; } }, + { Wire::W_R_SHIFT, [](const Selectors& sel) { return sel.q_m_nz; } }, + { Wire::W_O_SHIFT, [](const Selectors& sel) { return sel.q_c_nz; } }, + } }; + +// ============================================================================ +// Delta Range Pattern (from delta_range_constraint_relation.hpp) +// +// D_0 = w_2 - w_1, D_1 = w_3 - w_2, D_2 = w_4 - w_3, D_3 = w_1_shift - w_4 +// +// gate_selector = q_delta_range +// ============================================================================ + +inline const GatePattern DELTA_RANGE = { .name = "delta_range", + .wires = { + { Wire::W_L, [](const Selectors&) { return true; } }, + { Wire::W_R, [](const Selectors&) { return true; } }, + { Wire::W_O, [](const Selectors&) { return true; } }, + { Wire::W_4, [](const Selectors&) { return true; } }, + { Wire::W_L_SHIFT, [](const Selectors&) { return true; } }, + } }; + +// ============================================================================ +// Poseidon2 Internal Pattern (from poseidon2_internal_relation.hpp) +// +// All 4 current wires and all 4 shifted wires are constrained +// +// gate_selector = q_poseidon2_internal +// ============================================================================ + +inline const GatePattern POSEIDON2_INTERNAL = { .name = "poseidon2_internal", + .wires = { + { Wire::W_L, [](const Selectors&) { return true; } }, + { Wire::W_R, [](const Selectors&) { return true; } }, + { Wire::W_O, [](const Selectors&) { return true; } }, + { Wire::W_4, [](const Selectors&) { return true; } }, + { Wire::W_L_SHIFT, [](const Selectors&) { return true; } }, + { Wire::W_R_SHIFT, [](const Selectors&) { return true; } }, + { Wire::W_O_SHIFT, [](const Selectors&) { return true; } }, + { Wire::W_4_SHIFT, [](const Selectors&) { return true; } }, + } }; + +// ============================================================================ +// Poseidon2 External Pattern (from poseidon2_external_relation.hpp) +// +// All 4 current wires and all 4 shifted wires are constrained +// +// gate_selector = q_poseidon2_external +// ============================================================================ + +inline const GatePattern POSEIDON2_EXTERNAL = { .name = "poseidon2_external", + .wires = { + { Wire::W_L, [](const Selectors&) { return true; } }, + { Wire::W_R, [](const Selectors&) { return true; } }, + { Wire::W_O, [](const Selectors&) { return true; } }, + { Wire::W_4, [](const Selectors&) { return true; } }, + { Wire::W_L_SHIFT, [](const Selectors&) { return true; } }, + { Wire::W_R_SHIFT, [](const Selectors&) { return true; } }, + { Wire::W_O_SHIFT, [](const Selectors&) { return true; } }, + { Wire::W_4_SHIFT, [](const Selectors&) { return true; } }, + } }; + +// ============================================================================ +// Databus Pattern (from databus_lookup_relation.hpp) +// +// Read term uses: w_l (value), w_r (index) +// +// gate_selector = q_busread +// ============================================================================ + +inline const GatePattern DATABUS = { .name = "databus", + .wires = { + { Wire::W_L, [](const Selectors&) { return true; } }, + { Wire::W_R, [](const Selectors&) { return true; } }, + } }; + +// ============================================================================ +// Helper functions +// ============================================================================ + +template +Selectors read_selectors(Block& block, size_t gate_index, const GateSelectorColumn& gate_selector_column) +{ + return Selectors{ + .gate_selector = static_cast(static_cast(gate_selector_column[gate_index])), + .q_m_nz = !block.q_m()[gate_index].is_zero(), + .q_1_nz = !block.q_1()[gate_index].is_zero(), + .q_2_nz = !block.q_2()[gate_index].is_zero(), + .q_3_nz = !block.q_3()[gate_index].is_zero(), + .q_4_nz = !block.q_4()[gate_index].is_zero(), + .q_c_nz = !block.q_c()[gate_index].is_zero(), + }; +} + +template uint32_t get_wire(Block& block, size_t gate_index, Wire wire) +{ + switch (wire) { + case Wire::W_L: + return block.w_l()[gate_index]; + case Wire::W_R: + return block.w_r()[gate_index]; + case Wire::W_O: + return block.w_o()[gate_index]; + case Wire::W_4: + return block.w_4()[gate_index]; + case Wire::W_L_SHIFT: + return block.w_l()[gate_index + 1]; + case Wire::W_R_SHIFT: + return block.w_r()[gate_index + 1]; + case Wire::W_O_SHIFT: + return block.w_o()[gate_index + 1]; + case Wire::W_4_SHIFT: + return block.w_4()[gate_index + 1]; + } + return 0; +} + +inline bool is_shifted(Wire wire) +{ + return wire >= Wire::W_L_SHIFT; +} + +template +std::vector extract_wires(Block& block, + size_t gate_index, + const GatePattern& pattern, + const Selectors& selectors) +{ + std::vector result; + for (const auto& wire_spec : pattern.wires) { + // Bounds check for shifted wires + if (is_shifted(wire_spec.wire) && gate_index + 1 >= block.size()) { + continue; + } + if (wire_spec.condition(selectors)) { + result.push_back(get_wire(block, gate_index, wire_spec.wire)); + } + } + return result; +} + +} // namespace bb::gate_patterns diff --git a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/gate_patterns.test.cpp b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/gate_patterns.test.cpp new file mode 100644 index 000000000000..7337f59aa433 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/gate_patterns.test.cpp @@ -0,0 +1,421 @@ +/** + * @file gate_patterns.test.cpp + * @brief Verify gate patterns match actual relation constraints via perturbation testing. + * + * A wire is constrained by a relation if and only if perturbing that wire changes the relation's output. We test this + * empirically by: + * 1. Evaluating the relation at a base point + * 2. Individually perturbing each wire and checking if the output changes + * 3. Comparing the set of actually constrained wires with what the pattern claims + */ + +#include "gate_patterns.hpp" +#include "barretenberg/flavor/mega_flavor.hpp" +#include "barretenberg/relations/databus_lookup_relation.hpp" +#include "barretenberg/relations/delta_range_constraint_relation.hpp" +#include "barretenberg/relations/elliptic_relation.hpp" +#include "barretenberg/relations/logderiv_lookup_relation.hpp" +#include "barretenberg/relations/memory_relation.hpp" +#include "barretenberg/relations/non_native_field_relation.hpp" +#include "barretenberg/relations/poseidon2_external_relation.hpp" +#include "barretenberg/relations/poseidon2_internal_relation.hpp" +#include "barretenberg/relations/relation_parameters.hpp" +#include "barretenberg/relations/ultra_arithmetic_relation.hpp" +#include +#include + +using namespace bb; +using namespace bb::gate_patterns; + +using FF = fr; +using Entities = MegaFlavor::AllValues; + +Entities get_random_entities() +{ + Entities entities; + for (auto& field : entities.get_all()) { + field = FF::random_element(); + } + return entities; +} + +FF& get_wire(Entities& entities, Wire wire) +{ + switch (wire) { + case Wire::W_L: + return entities.w_l; + case Wire::W_R: + return entities.w_r; + case Wire::W_O: + return entities.w_o; + case Wire::W_4: + return entities.w_4; + case Wire::W_L_SHIFT: + return entities.w_l_shift; + case Wire::W_R_SHIFT: + return entities.w_r_shift; + case Wire::W_O_SHIFT: + return entities.w_o_shift; + case Wire::W_4_SHIFT: + return entities.w_4_shift; + } + __builtin_unreachable(); +} + +Selectors make_selectors(const Entities& entities, int64_t gate_selector_value) +{ + return Selectors{ + .gate_selector = gate_selector_value, + .q_m_nz = !entities.q_m.is_zero(), + .q_1_nz = !entities.q_l.is_zero(), + .q_2_nz = !entities.q_r.is_zero(), + .q_3_nz = !entities.q_o.is_zero(), + .q_4_nz = !entities.q_4.is_zero(), + .q_c_nz = !entities.q_c.is_zero(), + }; +} + +/** + * @brief Get the set of wires that a pattern claims are constrained + */ +std::set get_pattern_wires(const GatePattern& pattern, const Selectors& selectors) +{ + std::set result; + for (const auto& wire_spec : pattern.wires) { + if (wire_spec.condition(selectors)) { + result.insert(wire_spec.wire); + } + } + return result; +} + +/** + * @brief Get the set of wires that actually affect a relation's output + * + * This is the ground truth: perturb each wire and see if the output changes. + */ +template +std::set get_actually_constrained_wires(const Entities& entities, const auto& parameters) +{ + std::set constrained; + + // Evaluate relation at base point + typename Relation::SumcheckArrayOfValuesOverSubrelations base_result{}; + Relation::accumulate(base_result, entities, parameters, FF(1)); + + // For each wire position, perturb a copy and check if output changes + for (Wire wire : { Wire::W_L, + Wire::W_R, + Wire::W_O, + Wire::W_4, + Wire::W_L_SHIFT, + Wire::W_R_SHIFT, + Wire::W_O_SHIFT, + Wire::W_4_SHIFT }) { + Entities perturbed = entities; + get_wire(perturbed, wire) += FF::random_element(); + + typename Relation::SumcheckArrayOfValuesOverSubrelations perturbed_result{}; + Relation::accumulate(perturbed_result, perturbed, parameters, FF(1)); + + if (base_result != perturbed_result) { + constrained.insert(wire); + } + } + + return constrained; +} + +/** + * @brief Generic test: verify a pattern matches what the relation actually constrains + * + * @param configure_selectors Lambda that configures entity selectors and returns the gate selector field value + */ +template void verify_pattern(const GatePattern& pattern, auto configure_selectors) +{ + Entities entities = get_random_entities(); + FF gate_selector = configure_selectors(entities); + int64_t gate_selector_value = static_cast(uint64_t(gate_selector)); + + Selectors selectors = make_selectors(entities, gate_selector_value); + auto pattern_claims = get_pattern_wires(pattern, selectors); + + auto parameters = RelationParameters::get_random(); + auto actually_constrained = get_actually_constrained_wires(entities, parameters); + + EXPECT_EQ(actually_constrained, pattern_claims); +} + +// ============================================================================= +// Pattern Tests +// ============================================================================= + +TEST(PatternTest, Arithmetic1) +{ + verify_pattern>(ARITHMETIC, [](Entities& e) { return e.q_arith = FF(1); }); +} + +TEST(PatternTest, Arithmetic2) +{ + verify_pattern>(ARITHMETIC, [](Entities& e) { return e.q_arith = FF(2); }); +} + +TEST(PatternTest, Arithmetic3) +{ + verify_pattern>(ARITHMETIC, [](Entities& e) { return e.q_arith = FF(3); }); +} + +TEST(PatternTest, Arithmetic3WithQmZero) +{ + verify_pattern>(ARITHMETIC, [](Entities& e) { + e.q_m = FF(0); + return e.q_arith = FF(3); + }); +} + +TEST(PatternTest, EllipticAdd) +{ + verify_pattern>(ELLIPTIC, [](Entities& e) { + e.q_m = FF(0); + e.q_l = FF(-1); + return e.q_elliptic = FF(1); + }); +} + +TEST(PatternTest, EllipticDouble) +{ + verify_pattern>(ELLIPTIC, [](Entities& e) { + e.q_m = FF(1); + e.q_l = FF(-1); + return e.q_elliptic = FF(1); + }); +} + +TEST(PatternTest, DeltaRange) +{ + verify_pattern>(DELTA_RANGE, [](Entities& e) { return e.q_delta_range = FF(1); }); +} + +TEST(PatternTest, NNFLimbAccum1) +{ + verify_pattern>(NON_NATIVE_FIELD, [](Entities& e) { + e.q_r = FF(0); + e.q_o = FF(1); + e.q_4 = FF(1); + e.q_m = FF(0); + return e.q_nnf = FF(1); + }); +} + +TEST(PatternTest, NNFLimbAccum2) +{ + verify_pattern>(NON_NATIVE_FIELD, [](Entities& e) { + e.q_r = FF(0); + e.q_o = FF(1); + e.q_4 = FF(0); + e.q_m = FF(1); + return e.q_nnf = FF(1); + }); +} + +TEST(PatternTest, NNFProduct1) +{ + verify_pattern>(NON_NATIVE_FIELD, [](Entities& e) { + e.q_r = FF(1); + e.q_o = FF(1); + e.q_4 = FF(0); + e.q_m = FF(0); + return e.q_nnf = FF(1); + }); +} + +TEST(PatternTest, NNFProduct2) +{ + verify_pattern>(NON_NATIVE_FIELD, [](Entities& e) { + e.q_r = FF(1); + e.q_o = FF(0); + e.q_4 = FF(1); + e.q_m = FF(0); + return e.q_nnf = FF(1); + }); +} + +TEST(PatternTest, NNFProduct3) +{ + verify_pattern>(NON_NATIVE_FIELD, [](Entities& e) { + e.q_r = FF(1); + e.q_o = FF(0); + e.q_4 = FF(0); + e.q_m = FF(1); + return e.q_nnf = FF(1); + }); +} + +TEST(PatternTest, MemoryRamRomAccess) +{ + verify_pattern>(MEMORY, [](Entities& e) { + e.q_l = FF(1); + e.q_m = FF(1); + return e.q_memory = FF(1); + }); +} + +TEST(PatternTest, MemoryRamTimestamp) +{ + verify_pattern>(MEMORY, [](Entities& e) { + e.q_l = FF(1); + e.q_4 = FF(1); + return e.q_memory = FF(1); + }); +} + +TEST(PatternTest, MemoryRomConsistency) +{ + verify_pattern>(MEMORY, [](Entities& e) { + e.q_l = FF(1); + e.q_r = FF(1); + return e.q_memory = FF(1); + }); +} + +TEST(PatternTest, MemoryRamConsistency) +{ + verify_pattern>(MEMORY, [](Entities& e) { + e.q_o = FF(1); + return e.q_memory = FF(1); + }); +} + +TEST(PatternTest, Poseidon2Internal) +{ + verify_pattern>(POSEIDON2_INTERNAL, + [](Entities& e) { return e.q_poseidon2_internal = FF(1); }); +} + +TEST(PatternTest, Poseidon2External) +{ + verify_pattern>(POSEIDON2_EXTERNAL, + [](Entities& e) { return e.q_poseidon2_external = FF(1); }); +} + +TEST(PatternTest, LookupBasic) +{ + verify_pattern>(LOOKUP, [](Entities& e) { + // No shifted wires (step_size selectors all zero) + e.q_r = FF(0); + e.q_m = FF(0); + e.q_c = FF(0); + return e.q_lookup = FF(1); + }); +} + +TEST(PatternTest, LookupWithShiftedWires) +{ + verify_pattern>(LOOKUP, [](Entities& e) { + // Enable all shifted wires + e.q_r = FF(1); + e.q_m = FF(1); + e.q_c = FF(1); + return e.q_lookup = FF(1); + }); +} + +TEST(PatternTest, DatabusRead) +{ + verify_pattern>(DATABUS, [](Entities& e) { return e.q_busread = FF(1); }); +} + +// ============================================================================= +// Failure Detection Tests +// +// These tests verify the perturbation testing mechanism catches pattern errors. +// They use intentionally wrong patterns to demonstrate both over-constrained +// and under-constrained specifications are detected. +// ============================================================================= + +/** + * @brief Verify detection of OVER-constrained pattern (claims more wires than relation uses) + * + * When q_arith==3, the multiplication term q_m * w_l * w_r is disabled (scaled by q_arith - 3 = 0). + * So w_r is only constrained via the linear term q_2 * w_r. A pattern that includes w_r whenever + * q_m != 0 (without checking q_arith != 3) over-constrains when q_arith=3, q_m!=0, q_2=0. + */ +TEST(PatternTest, DetectOverConstrained) +{ + // Pattern that unconditionally includes w_r when q_m != 0 (ignoring q_arith value) + const GatePattern OVERCONSTRAINED_PATTERN = { .name = "overconstrained", + .wires = { + { Wire::W_L, + [](const Selectors& sel) { return sel.q_1_nz || sel.q_m_nz; } }, + { Wire::W_R, + [](const Selectors& sel) { + return sel.q_2_nz || sel.q_m_nz; + } }, // should check q_arith!=3 + { Wire::W_O, [](const Selectors& sel) { return sel.q_3_nz; } }, + { Wire::W_4, + [](const Selectors& sel) { + return sel.q_4_nz || sel.gate_selector >= 2; + } }, + { Wire::W_4_SHIFT, + [](const Selectors& sel) { return sel.gate_selector >= 2; } }, + { Wire::W_L_SHIFT, + [](const Selectors& sel) { return sel.gate_selector == 3; } }, + } }; + + // q_arith=3 disables mul term, q_2=0 means w_r has no linear term, so w_r is unconstrained + Entities entities = get_random_entities(); + entities.q_arith = FF(3); + entities.q_m = FF(1); + entities.q_l = FF(1); + entities.q_r = FF(0); // q_2 = 0 + + Selectors selectors = make_selectors(entities, 3); + auto pattern_claims = get_pattern_wires(OVERCONSTRAINED_PATTERN, selectors); + auto correct_claims = get_pattern_wires(ARITHMETIC, selectors); + auto parameters = RelationParameters::get_random(); + auto actually_constrained = get_actually_constrained_wires>(entities, parameters); + + EXPECT_TRUE(pattern_claims.contains(Wire::W_R)) << "Over-constrained pattern claims W_R"; + EXPECT_FALSE(actually_constrained.contains(Wire::W_R)) << "Relation does not constrain W_R in this config"; + EXPECT_NE(pattern_claims, actually_constrained) << "Over-constrained pattern should not match relation"; + EXPECT_EQ(correct_claims, actually_constrained) << "Correct ARITHMETIC pattern should match relation"; +} + +/** + * @brief Verify detection of UNDER-constrained pattern (misses wires that relation uses) + * + * The RAM consistency relation (q_3 != 0) constrains all 8 wires. A pattern that only + * extracts 6 wires (omitting w_l and w_r) under-constrains. + */ +TEST(PatternTest, DetectUnderConstrained) +{ + // Pattern missing w_l and w_r for RAM consistency + const GatePattern + UNDERCONSTRAINED_PATTERN = { .name = "underconstrained", + .wires = { + { Wire::W_O, [](const Selectors& sel) { return sel.q_3_nz; } }, + { Wire::W_4, [](const Selectors& sel) { return sel.q_3_nz; } }, + { Wire::W_L_SHIFT, [](const Selectors& sel) { return sel.q_3_nz; } }, + { Wire::W_R_SHIFT, [](const Selectors& sel) { return sel.q_3_nz; } }, + { Wire::W_O_SHIFT, [](const Selectors& sel) { return sel.q_3_nz; } }, + { Wire::W_4_SHIFT, [](const Selectors& sel) { return sel.q_3_nz; } }, + } }; + + // RAM consistency check: q_3 != 0 + Entities entities = get_random_entities(); + entities.q_memory = FF(1); + entities.q_o = FF(1); // q_3 + + Selectors selectors = make_selectors(entities, 1); + auto pattern_claims = get_pattern_wires(UNDERCONSTRAINED_PATTERN, selectors); + auto correct_claims = get_pattern_wires(MEMORY, selectors); + auto parameters = RelationParameters::get_random(); + auto actually_constrained = get_actually_constrained_wires>(entities, parameters); + + EXPECT_FALSE(pattern_claims.contains(Wire::W_L)) << "Under-constrained pattern missing W_L"; + EXPECT_FALSE(pattern_claims.contains(Wire::W_R)) << "Under-constrained pattern missing W_R"; + EXPECT_TRUE(actually_constrained.contains(Wire::W_L)) << "Relation constrains W_L"; + EXPECT_TRUE(actually_constrained.contains(Wire::W_R)) << "Relation constrains W_R"; + EXPECT_NE(pattern_claims, actually_constrained) << "Under-constrained pattern should not match relation"; + EXPECT_EQ(correct_claims, actually_constrained) << "Correct MEMORY pattern should match relation"; +} diff --git a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.cpp b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.cpp index cd538c374ffa..dcc0a4d8a311 100644 --- a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.cpp +++ b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.cpp @@ -5,6 +5,7 @@ // ===================== #include "./graph.hpp" +#include "./gate_patterns.hpp" #include "barretenberg/common/assert.hpp" #include "barretenberg/stdlib/primitives/circuit_builders/circuit_builders.hpp" #include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp" @@ -19,43 +20,24 @@ using namespace bb; namespace cdg { -/** - * @brief this method finds index of the block in circuit builder by comparing pointers to blocks - * @tparam FF field type - * @tparam CircuitBuilder - * @param block block to find - * @return size_t index of the found block - */ -template -std::optional StaticAnalyzer_::find_block_index(const auto& block) -{ - auto blocks_data = circuit_builder.blocks.get(); - for (size_t i = 0; i < blocks_data.size(); i++) { - if (std::addressof(blocks_data[i]) == std::addressof(block)) { - return i; - } - } - return std::nullopt; -} - /** * @brief this method processes variables from a gate by removing duplicates and updating tracking structures * @tparam FF field type * @tparam CircuitBuilder * @param gate_variables vector of variables to process * @param gate_index index of the current gate - * @param block_idx index of the current block + * @param blk reference to the block containing the gate * @details The method performs several operations: * 1) Removes duplicate variables from the input vector * 2) Converts each variable to its real index using to_real - * 3) Creates key-value pairs of (variable_index, block_index) for tracking + * 3) Creates key-value pairs of (variable_index, block_pointer) for tracking * 4) Updates variable_gates map with gate indices for each variable * 5) Increments the gate count for each processed variable */ template inline void StaticAnalyzer_::process_gate_variables(std::vector& gate_variables, size_t gate_index, - size_t block_idx) + auto& blk) { auto unique_variables = std::unique(gate_variables.begin(), gate_variables.end()); gate_variables.erase(unique_variables, gate_variables.end()); @@ -63,7 +45,7 @@ inline void StaticAnalyzer_::process_gate_variables(std::vec return; } for (auto& var_idx : gate_variables) { - KeyPair key = std::make_pair(var_idx, block_idx); + KeyPair key = std::make_pair(var_idx, &blk); variable_gates[key].emplace_back(gate_index); } for (const auto& variable_index : gate_variables) { @@ -72,397 +54,39 @@ inline void StaticAnalyzer_::process_gate_variables(std::vec } /** - * @brief this method creates connected components from arithmetic gates - * @tparam FF field type - * @tparam CircuitBuilder - * @param index index of the current gate - * @param block_idx index of the current block - * @param blk block containing the gates - * @return std::vector> vector of connected components from the gate and minigate - * @details Processes both regular arithmetic gates and minigates, handling fixed witness gates - * and different arithmetic operations based on selector values + * @brief Extract gate variables using a declarative pattern + * + * This method uses a GatePattern to determine which wires are constrained by a gate, + * then extracts the variable indices from those wire positions. + * + * @param index Gate index within the block + * @param blk The block containing the gate + * @param pattern The GatePattern describing which wires are constrained + * @param gate_selector_column The selector column for this gate type (e.g., q_arith, q_elliptic) + * @return Vector of real variable indices constrained by this gate */ template -inline std::vector StaticAnalyzer_::get_arithmetic_gate_connected_component( - size_t index, size_t block_idx, auto& blk) +template +std::vector StaticAnalyzer_::extract_gate_variables( + size_t index, + Block& blk, + const bb::gate_patterns::GatePattern& pattern, + const GateSelectorColumn& gate_selector_column) { - auto q_arith = blk.q_arith()[index]; - std::vector all_variables; - std::vector gate_variables; - std::vector minigate_variables; - std::vector> all_gates_variables; - if (q_arith.is_zero()) { - return {}; - } - auto q_m = blk.q_m()[index]; - auto q_1 = blk.q_1()[index]; - auto q_2 = blk.q_2()[index]; - auto q_3 = blk.q_3()[index]; - auto q_4 = blk.q_4()[index]; - - uint32_t left_idx = blk.w_l()[index]; - uint32_t right_idx = blk.w_r()[index]; - uint32_t out_idx = blk.w_o()[index]; - uint32_t fourth_idx = blk.w_4()[index]; - if (q_m.is_zero() && q_1 == 1 && q_2.is_zero() && q_3.is_zero() && q_4.is_zero() && q_arith == FF::one()) { - // this is fixed_witness gate. So, variable index contains in left wire. So, we have to take only it. - fixed_variables.insert(this->to_real(left_idx)); - } else if (!q_m.is_zero() || q_1 != FF::one() || !q_2.is_zero() || !q_3.is_zero() || !q_4.is_zero()) { - // this is not the gate for fix_witness, so we have to process this gate - if (!q_m.is_zero()) { - gate_variables.emplace_back(left_idx); - gate_variables.emplace_back(right_idx); - } else { - if (!q_1.is_zero()) { - gate_variables.emplace_back(left_idx); - } - if (!q_2.is_zero()) { - gate_variables.emplace_back(right_idx); - } - } + using namespace bb::gate_patterns; - if (!q_3.is_zero()) { - gate_variables.emplace_back(out_idx); - } - if (!q_4.is_zero()) { - gate_variables.emplace_back(fourth_idx); - } - if (q_arith == FF(2)) { - // We have to use w_4_shift from the next gate - // if and only if the current gate isn't last, cause we can't - // look into the next gate - if (index != blk.size() - 1) { - gate_variables.emplace_back(blk.w_4()[index + 1]); - } - } - if (q_arith == FF(3)) { - // In this gate mini gate is enabled, we have 2 equations: - // q_1 * w_1 + q_2 * w_2 + q_3 * w_3 + q_4 * w_4 + q_c + 2 * w_4_omega = 0 - // w_1 + w_4 - w_1_omega + q_m = 0 - minigate_variables.emplace_back(left_idx); - minigate_variables.emplace_back(fourth_idx); - if (index != blk.size() - 1) { - gate_variables.emplace_back(blk.w_4()[index + 1]); - minigate_variables.emplace_back(blk.w_l()[index + 1]); - } - } + // Check if gate selector is active + if (gate_selector_column[index].is_zero()) { + return {}; } - gate_variables = to_real(gate_variables); - minigate_variables = to_real(minigate_variables); - all_variables.reserve(gate_variables.size() + minigate_variables.size()); - all_variables.insert(all_variables.end(), gate_variables.begin(), gate_variables.end()); - all_variables.insert(all_variables.end(), minigate_variables.begin(), minigate_variables.end()); - process_gate_variables(all_variables, index, block_idx); - return all_variables; -} -/** - * @brief this method creates connected components from elliptic gates - * @tparam FF field type - * @tparam CircuitBuilder - * @param index index of the current gate - * @param block_idx index of the current block - * @param blk block containing the gates - * @return std::vector vector of connected variables from the gate - * @details Handles both elliptic curve addition and doubling operations, - * collecting variables from current and next gates as needed - */ -template -inline std::vector StaticAnalyzer_::get_elliptic_gate_connected_component( - size_t index, size_t block_idx, auto& blk) -{ - std::vector gate_variables; - if (!blk.q_elliptic()[index].is_zero()) { - std::vector first_row_variables; - std::vector second_row_variables; - gate_variables.reserve(6); - bool is_elliptic_add_gate = !blk.q_1()[index].is_zero() && blk.q_m()[index].is_zero(); - bool is_elliptic_dbl_gate = blk.q_1()[index].is_zero() && blk.q_m()[index] == FF::one(); - first_row_variables.emplace_back(blk.w_r()[index]); - first_row_variables.emplace_back(blk.w_o()[index]); - if (index != blk.size() - 1) { - if (is_elliptic_add_gate) { - // if this gate is ecc_add_gate, we have to get indices x2, x3, y3, y2 from the next gate - second_row_variables.emplace_back(blk.w_l()[index + 1]); - second_row_variables.emplace_back(blk.w_r()[index + 1]); - second_row_variables.emplace_back(blk.w_o()[index + 1]); - second_row_variables.emplace_back(blk.w_4()[index + 1]); - } - if (is_elliptic_dbl_gate) { - // if this gate is ecc_dbl_gate, we have to indices x3, y3 from right and output wires - second_row_variables.emplace_back(blk.w_r()[index + 1]); - second_row_variables.emplace_back(blk.w_o()[index + 1]); - } - } - if (!first_row_variables.empty()) { - first_row_variables = to_real(first_row_variables); - process_gate_variables(first_row_variables, index, block_idx); - gate_variables.insert(gate_variables.end(), first_row_variables.cbegin(), first_row_variables.cend()); - } - if (!second_row_variables.empty()) { - second_row_variables = to_real(second_row_variables); - process_gate_variables(second_row_variables, index, block_idx); - gate_variables.insert(gate_variables.end(), second_row_variables.cbegin(), second_row_variables.cend()); - } - } - return gate_variables; -} + // Read selectors and extract wire indices using the pattern + Selectors selectors = read_selectors(blk, index, gate_selector_column); + std::vector gate_variables = extract_wires(blk, index, pattern, selectors); -/** - * @brief this method creates connected components from sorted constraints - * @tparam FF field type - * @tparam CircuitBuilder - * @param index index of the current gate - * @param block_idx index of the current block - * @param block block containing the gates - * @return std::vector vector of connected variables from the gate - * @details Processes delta range constraints by collecting all wire indices - * from the current gate - */ -template -inline std::vector StaticAnalyzer_::get_sort_constraint_connected_component( - size_t index, size_t blk_idx, auto& block) -{ - std::vector gate_variables = {}; - if (!block.q_delta_range()[index].is_zero()) { - std::vector row_variables = { - block.w_l()[index], block.w_r()[index], block.w_o()[index], block.w_4()[index] - }; - /* - sometimes process_range_list function adds variables with zero_idx in beginning of vector with indices - in order to pad a size of indices to gate width. But tool has to ignore these additional variables - */ - for (const auto& var_idx : row_variables) { - if (var_idx != circuit_builder.zero_idx()) { - gate_variables.emplace_back(var_idx); - } - } - if (index != block.size() - 1 && block.w_l()[index + 1] != circuit_builder.zero_idx()) { - gate_variables.emplace_back(block.w_l()[index + 1]); - } - } + // Convert to real indices and process gate_variables = to_real(gate_variables); - process_gate_variables(gate_variables, index, blk_idx); - return gate_variables; -} - -/** - * @brief this method creates connected components from plookup gates - * @tparam FF field type - * @tparam CircuitBuilder - * @param index index of the current gate - * @param block_idx index of the current block - * @param block block containing the gates - * @return std::vector vector of connected variables from the gate - * @details Processes plookup gates by collecting variables based on selector values, - * including variables from the next gate when necessary - */ -template -inline std::vector StaticAnalyzer_::get_plookup_gate_connected_component(size_t index, - size_t blk_idx, - auto& block) -{ - std::vector gate_variables; - auto q_lookup = block.q_lookup()[index]; - if (!q_lookup.is_zero()) { - gate_variables.reserve(6); - auto q_2 = block.q_2()[index]; - auto q_m = block.q_m()[index]; - auto q_c = block.q_c()[index]; - gate_variables.emplace_back(block.w_l()[index]); - gate_variables.emplace_back(block.w_r()[index]); - gate_variables.emplace_back(block.w_o()[index]); - if (index < block.size() - 1) { - if (!q_2.is_zero()) { - gate_variables.emplace_back(block.w_l()[index + 1]); - } - if (!q_m.is_zero()) { - gate_variables.emplace_back(block.w_r()[index + 1]); - } - if (!q_c.is_zero()) { - gate_variables.emplace_back(block.w_o()[index + 1]); - } - } - gate_variables = to_real(gate_variables); - process_gate_variables(gate_variables, index, blk_idx); - } - return gate_variables; -} - -/** - * @brief this method creates connected components from poseidon2 gates - * @tparam FF field type - * @tparam CircuitBuilder - * @param index index of the current gate - * @param blk_idx index of the current block - * @param block block containing the gates - * @return std::vector vector of connected variables from the gate - */ -template -inline std::vector StaticAnalyzer_::get_poseido2s_gate_connected_component(size_t index, - size_t blk_idx, - auto& block) -{ - std::vector gate_variables; - auto internal_selector = block.q_poseidon2_internal()[index]; - auto external_selector = block.q_poseidon2_external()[index]; - if (!internal_selector.is_zero() || !external_selector.is_zero()) { - gate_variables.reserve(8); - gate_variables.emplace_back(block.w_l()[index]); - gate_variables.emplace_back(block.w_r()[index]); - gate_variables.emplace_back(block.w_o()[index]); - gate_variables.emplace_back(block.w_4()[index]); - if (index != block.size() - 1) { - gate_variables.emplace_back(block.w_l()[index + 1]); - gate_variables.emplace_back(block.w_r()[index + 1]); - gate_variables.emplace_back(block.w_o()[index + 1]); - gate_variables.emplace_back(block.w_4()[index + 1]); - } - gate_variables = to_real(gate_variables); - process_gate_variables(gate_variables, index, blk_idx); - } - return gate_variables; -} - -/** - * @brief this method creates connected components from Memory gates (RAM and ROM consistency checks) - * @tparam FF field type - * @tparam CircuitBuilder - * @param index index of the current gate - * @param blk_idx index of the current block - * @param block block containing the gates - * @return std::vector vector of connected variables from the gate - */ -template -inline std::vector StaticAnalyzer_::get_memory_gate_connected_component(size_t index, - size_t blk_idx, - auto& block) -{ - std::vector gate_variables; - if (!block.q_memory()[index].is_zero()) { - gate_variables.reserve(8); - auto q_1 = block.q_1()[index]; - auto q_2 = block.q_2()[index]; - auto q_3 = block.q_3()[index]; - auto q_4 = block.q_4()[index]; - if (q_1 == FF::one() && q_4 == FF::one()) { - BB_ASSERT(q_3.is_zero()); - // ram timestamp check - if (index < block.size() - 1) { - gate_variables.insert(gate_variables.end(), - { block.w_r()[index + 1], - block.w_r()[index], - block.w_l()[index], - block.w_l()[index + 1], - block.w_o()[index] }); - } - } else if (q_1 == FF::one() && q_2 == FF::one()) { - BB_ASSERT(q_3.is_zero()); - // rom constitency check - if (index < block.size() - 1) { - gate_variables.insert( - gate_variables.end(), - { block.w_l()[index], block.w_l()[index + 1], block.w_4()[index], block.w_4()[index + 1] }); - } - } else { - // ram constitency check - if (!q_3.is_zero()) { - if (index < block.size() - 1) { - gate_variables.insert(gate_variables.end(), - { block.w_o()[index], - block.w_4()[index], - block.w_l()[index + 1], - block.w_r()[index + 1], - block.w_o()[index + 1], - block.w_4()[index + 1] }); - } - } - } - } - gate_variables = to_real(gate_variables); - process_gate_variables(gate_variables, index, blk_idx); - return gate_variables; -} - -/** - * @brief this method creates connected components from Non-Native Field gates (bigfield operations) - * @tparam FF field type - * @tparam CircuitBuilder - * @param index index of the current gate - * @param blk_idx index of the current block - * @param block block containing the gates - * @return std::vector vector of connected variables from the gate - */ -template -inline std::vector StaticAnalyzer_::get_non_native_field_gate_connected_component( - size_t index, size_t blk_idx, auto& block) -{ - std::vector gate_variables; - if (!block.q_nnf()[index].is_zero()) { - gate_variables.reserve(8); - [[maybe_unused]] auto q_1 = block.q_1()[index]; - auto q_2 = block.q_2()[index]; - auto q_3 = block.q_3()[index]; - auto q_4 = block.q_4()[index]; - auto q_m = block.q_m()[index]; - - auto w_l = block.w_l()[index]; - auto w_r = block.w_r()[index]; - auto w_o = block.w_o()[index]; - auto w_4 = block.w_4()[index]; - if (q_3 == FF::one() && q_4 == FF::one()) { - // bigfield limb accumulation 1 - if (index < block.size() - 1) { - gate_variables.insert(gate_variables.end(), - { w_l, w_r, w_o, w_4, block.w_l()[index + 1], block.w_r()[index + 1] }); // 6 - } - } else if (q_3 == FF::one() && q_m == FF::one()) { - // bigfield limb accumulation 2 - if (index < block.size() - 1) { - gate_variables.insert(gate_variables.end(), - { w_o, - w_4, - block.w_l()[index + 1], - block.w_r()[index + 1], - block.w_o()[index + 1], - block.w_4()[index + 1] }); - } - } else if (q_2 == FF::one() && (q_3 == FF::one() || q_4 == FF::one() || q_m == FF::one())) { - // bigfield product cases - if (index < block.size() - 1) { - std::vector limb_subproduct_vars = { - w_l, w_r, block.w_l()[index + 1], block.w_r()[index + 1] - }; - if (q_3 == FF::one()) { - // bigfield product 1 - BB_ASSERT(q_4.is_zero() && q_m.is_zero()); - gate_variables.insert( - gate_variables.end(), limb_subproduct_vars.begin(), limb_subproduct_vars.end()); - gate_variables.insert(gate_variables.end(), { w_o, w_4 }); - } - if (q_4 == FF::one()) { - // bigfield product 2 - BB_ASSERT(q_3.is_zero() && q_m.is_zero()); - std::vector non_native_field_gate_2 = { w_l, w_4, w_r, w_o, block.w_o()[index + 1] }; - gate_variables.insert( - gate_variables.end(), non_native_field_gate_2.begin(), non_native_field_gate_2.end()); - gate_variables.emplace_back(block.w_4()[index + 1]); - gate_variables.insert( - gate_variables.end(), limb_subproduct_vars.begin(), limb_subproduct_vars.end()); - } - if (q_m == FF::one()) { - // bigfield product 3 - BB_ASSERT(q_4.is_zero() && q_3.is_zero()); - gate_variables.insert( - gate_variables.end(), limb_subproduct_vars.begin(), limb_subproduct_vars.end()); - gate_variables.insert(gate_variables.end(), - { w_4, block.w_o()[index + 1], block.w_4()[index + 1] }); - } - } - } - } - gate_variables = to_real(gate_variables); - process_gate_variables(gate_variables, index, blk_idx); + process_gate_variables(gate_variables, index, blk); return gate_variables; } @@ -482,47 +106,41 @@ inline std::vector StaticAnalyzer_::get_rom_table_ // 2) states contains values witness indexes that we can find in the ROM record in the RomTrascript, so we can // ignore state of the ROM transcript, because we still can connect all variables using variables from records. std::vector rom_table_variables; - if (std::optional blk_idx = find_block_index(circuit_builder.blocks.memory); blk_idx) { - // Every RomTranscript data structure has 2 main components that are interested for static analyzer: - // 1) records contains values that were put in the gate, we can use them to create connections between variables - // 2) states contains values witness indexes that we can find in the ROM record in the RomTrascript, so we can - // ignore state of the ROM transcript, because we still can connect all variables using variables from records. - for (const auto& record : rom_array.records) { - std::vector gate_variables; - size_t gate_index = record.gate_index; - - auto q_1 = circuit_builder.blocks.memory.q_1()[gate_index]; - auto q_2 = circuit_builder.blocks.memory.q_2()[gate_index]; - auto q_3 = circuit_builder.blocks.memory.q_3()[gate_index]; - auto q_4 = circuit_builder.blocks.memory.q_4()[gate_index]; - auto q_m = circuit_builder.blocks.memory.q_m()[gate_index]; - auto q_c = circuit_builder.blocks.memory.q_c()[gate_index]; - - auto index_witness = record.index_witness; - auto vc1_witness = record.value_column1_witness; // state[0] from RomTranscript - auto vc2_witness = record.value_column2_witness; // state[1] from RomTranscript - auto record_witness = record.record_witness; - - if (q_1 == FF::one() && q_m == FF::one() && q_2.is_zero() && q_3.is_zero() && q_4.is_zero() && - q_c.is_zero()) { - // By default ROM read gate uses variables (w_1, w_2, w_3, w_4) = (index_witness, vc1_witness, - // vc2_witness, record_witness) So we can update all of them - gate_variables.emplace_back(index_witness); - if (vc1_witness != circuit_builder.zero_idx()) { - gate_variables.emplace_back(vc1_witness); - } - if (vc2_witness != circuit_builder.zero_idx()) { - gate_variables.emplace_back(vc2_witness); - } - gate_variables.emplace_back(record_witness); + auto& memory_block = circuit_builder.blocks.memory; + for (const auto& record : rom_array.records) { + std::vector gate_variables; + size_t gate_index = record.gate_index; + + auto q_1 = memory_block.q_1()[gate_index]; + auto q_2 = memory_block.q_2()[gate_index]; + auto q_3 = memory_block.q_3()[gate_index]; + auto q_4 = memory_block.q_4()[gate_index]; + auto q_m = memory_block.q_m()[gate_index]; + auto q_c = memory_block.q_c()[gate_index]; + + auto index_witness = record.index_witness; + auto vc1_witness = record.value_column1_witness; // state[0] from RomTranscript + auto vc2_witness = record.value_column2_witness; // state[1] from RomTranscript + auto record_witness = record.record_witness; + + if (q_1 == FF::one() && q_m == FF::one() && q_2.is_zero() && q_3.is_zero() && q_4.is_zero() && q_c.is_zero()) { + // By default ROM read gate uses variables (w_1, w_2, w_3, w_4) = (index_witness, vc1_witness, + // vc2_witness, record_witness) So we can update all of them + gate_variables.emplace_back(index_witness); + if (vc1_witness != circuit_builder.zero_idx()) { + gate_variables.emplace_back(vc1_witness); } - gate_variables = to_real(gate_variables); - process_gate_variables(gate_variables, gate_index, *blk_idx); - // after process_gate_variables function gate_variables constists of real variables indexes, so we can - // add all this variables in the final vector to connect all of them - if (!gate_variables.empty()) { - rom_table_variables.insert(rom_table_variables.end(), gate_variables.begin(), gate_variables.end()); + if (vc2_witness != circuit_builder.zero_idx()) { + gate_variables.emplace_back(vc2_witness); } + gate_variables.emplace_back(record_witness); + } + gate_variables = to_real(gate_variables); + process_gate_variables(gate_variables, gate_index, memory_block); + // after process_gate_variables function gate_variables constists of real variables indexes, so we can + // add all this variables in the final vector to connect all of them + if (!gate_variables.empty()) { + rom_table_variables.insert(rom_table_variables.end(), gate_variables.begin(), gate_variables.end()); } } return rom_table_variables; @@ -532,7 +150,6 @@ inline std::vector StaticAnalyzer_::get_rom_table_ * @brief this method gets the RAM table connected component by processing RAM transcript records * @tparam FF field type * @param CircuitBuilder - * @param ultra_builder circuit builder containing the gates * @param ram_array RAM transcript containing records with witness indices and gate information * @return std::vector vector of connected variables from RAM table gates */ @@ -541,68 +158,43 @@ inline std::vector StaticAnalyzer_::get_ram_table_ const bb::RamTranscript& ram_array) { std::vector ram_table_variables; - if (std::optional blk_idx = find_block_index(circuit_builder.blocks.memory); blk_idx) { - for (const auto& record : ram_array.records) { - std::vector gate_variables; - size_t gate_index = record.gate_index; - - auto q_1 = circuit_builder.blocks.memory.q_1()[gate_index]; - auto q_2 = circuit_builder.blocks.memory.q_2()[gate_index]; - auto q_3 = circuit_builder.blocks.memory.q_3()[gate_index]; - auto q_4 = circuit_builder.blocks.memory.q_4()[gate_index]; - auto q_m = circuit_builder.blocks.memory.q_m()[gate_index]; - auto q_c = circuit_builder.blocks.memory.q_c()[gate_index]; - - auto index_witness = record.index_witness; - auto timestamp_witness = record.timestamp_witness; - auto value_witness = record.value_witness; - auto record_witness = record.record_witness; - - if (q_1 == FF::one() && q_m == FF::one() && q_2.is_zero() && q_3.is_zero() && q_4.is_zero() && - (q_c.is_zero() || q_c == FF::one())) { - // By default RAM read/write gate uses variables (w_1, w_2, w_3, w_4) = (index_witness, - // timestamp_witness, value_witness, record_witness) So we can update all of them - gate_variables.emplace_back(index_witness); - if (timestamp_witness != circuit_builder.zero_idx()) { - gate_variables.emplace_back(timestamp_witness); - } - if (value_witness != circuit_builder.zero_idx()) { - gate_variables.emplace_back(value_witness); - } - gate_variables.emplace_back(record_witness); + auto& memory_block = circuit_builder.blocks.memory; + for (const auto& record : ram_array.records) { + std::vector gate_variables; + size_t gate_index = record.gate_index; + + auto q_1 = memory_block.q_1()[gate_index]; + auto q_2 = memory_block.q_2()[gate_index]; + auto q_3 = memory_block.q_3()[gate_index]; + auto q_4 = memory_block.q_4()[gate_index]; + auto q_m = memory_block.q_m()[gate_index]; + auto q_c = memory_block.q_c()[gate_index]; + + auto index_witness = record.index_witness; + auto timestamp_witness = record.timestamp_witness; + auto value_witness = record.value_witness; + auto record_witness = record.record_witness; + + if (q_1 == FF::one() && q_m == FF::one() && q_2.is_zero() && q_3.is_zero() && q_4.is_zero() && + (q_c.is_zero() || q_c == FF::one())) { + // By default RAM read/write gate uses variables (w_1, w_2, w_3, w_4) = (index_witness, + // timestamp_witness, value_witness, record_witness) So we can update all of them + gate_variables.emplace_back(index_witness); + if (timestamp_witness != circuit_builder.zero_idx()) { + gate_variables.emplace_back(timestamp_witness); + } + if (value_witness != circuit_builder.zero_idx()) { + gate_variables.emplace_back(value_witness); } - gate_variables = to_real(gate_variables); - process_gate_variables(gate_variables, gate_index, *blk_idx); - // after process_gate_variables function gate_variables constists of real variables indexes, so we can add - // all these variables in the final vector to connect all of them - ram_table_variables.insert(ram_table_variables.end(), gate_variables.begin(), gate_variables.end()); + gate_variables.emplace_back(record_witness); } - } - return ram_table_variables; -} - -/** - * @brief this method creates connected components from databus gates - * @tparam FF field type - * @param CircuitBuilder - * @param index index of the current gate - * @param block_idx index of the current block - * @param blk block containing the gates - * @return std::vector vector of connected variables from the gate - * @details Processes databus read operations by collecting variables from left and right wires - */ -template -inline std::vector StaticAnalyzer_::get_databus_connected_component(size_t index, - size_t block_idx, - auto& blk) -{ - std::vector gate_variables; - if (!blk.q_busread()[index].is_zero()) { - gate_variables.insert(gate_variables.end(), { blk.w_l()[index], blk.w_r()[index] }); gate_variables = to_real(gate_variables); - process_gate_variables(gate_variables, index, block_idx); + process_gate_variables(gate_variables, gate_index, memory_block); + // after process_gate_variables function gate_variables constists of real variables indexes, so we can add + // all these variables in the final vector to connect all of them + ram_table_variables.insert(ram_table_variables.end(), gate_variables.begin(), gate_variables.end()); } - return gate_variables; + return ram_table_variables; } /** @@ -610,18 +202,25 @@ inline std::vector StaticAnalyzer_::get_databus_co * @tparam FF field type * @param CircuitBuilder * @param index index of the current gate - * @param block_idx index of the current block * @param blk block containing the gates * @return std::vector vector of connected variables from the gate * @details Processes elliptic curve operations by collecting variables from current and next gates, - * handling opcodes and coordinate variables for curve operations + * handling opcodes and coordinate variables for curve operations. + * Only processes gates in the ecc_op block - returns empty for other blocks. */ template inline std::vector StaticAnalyzer_::get_eccop_part_connected_component(size_t index, - size_t block_idx, auto& blk) { std::vector gate_variables; + + // Only process gates in the ecc_op block, otherwise return early + if constexpr (IsMegaBuilder) { + if (&blk != &circuit_builder.blocks.ecc_op) { + return gate_variables; + } + } + std::vector first_row_variables; std::vector second_row_variables; auto w1 = blk.w_l()[index]; // get opcode of operation, because function get_ecc_op_idx returns type @@ -638,8 +237,8 @@ inline std::vector StaticAnalyzer_::get_eccop_part } first_row_variables = to_real(first_row_variables); second_row_variables = to_real(second_row_variables); - process_gate_variables(first_row_variables, index, block_idx); - process_gate_variables(second_row_variables, index, block_idx); + process_gate_variables(first_row_variables, index, blk); + process_gate_variables(second_row_variables, index, blk); } if (!first_row_variables.empty()) { gate_variables.insert(gate_variables.end(), first_row_variables.cbegin(), first_row_variables.cend()); @@ -652,61 +251,50 @@ inline std::vector StaticAnalyzer_::get_eccop_part template void StaticAnalyzer_::process_execution_trace() { - auto block_data = circuit_builder.blocks.get(); - - // We have to determine pub_inputs block index based on circuit builder type, because we have to skip it. - // If type of CircuitBuilder is UltraCircuitBuilder, the pub_inputs block is the first block so we can set - // pub_inputs_block_idx - size_t pub_inputs_block_idx = 0; - - // For MegaCircuitBuilder, pub_inputs block has index 3 - if constexpr (IsMegaBuilder) { - pub_inputs_block_idx = 3; - } + using namespace bb::gate_patterns; - for (size_t blk_idx = 0; blk_idx < block_data.size(); blk_idx++) { - if (block_data[blk_idx].size() == 0 || blk_idx == pub_inputs_block_idx) { + for (auto& blk : circuit_builder.blocks.get()) { + if (blk.size() == 0 || &blk == &circuit_builder.blocks.pub_inputs) { continue; } - std::vector sorted_variables; + std::vector eccop_variables; - for (size_t gate_idx = 0; gate_idx < block_data[blk_idx].size(); gate_idx++) { - std::vector> all_cc = { - get_arithmetic_gate_connected_component(gate_idx, blk_idx, block_data[blk_idx]), - get_elliptic_gate_connected_component(gate_idx, blk_idx, block_data[blk_idx]), - get_plookup_gate_connected_component(gate_idx, blk_idx, block_data[blk_idx]), - get_poseido2s_gate_connected_component(gate_idx, blk_idx, block_data[blk_idx]), - get_non_native_field_gate_connected_component(gate_idx, blk_idx, block_data[blk_idx]), - get_memory_gate_connected_component(gate_idx, blk_idx, block_data[blk_idx]), - get_sort_constraint_connected_component(gate_idx, blk_idx, block_data[blk_idx]) + for (size_t gate_idx = 0; gate_idx < blk.size(); gate_idx++) { + // Try each pattern until one matches (returns non-empty) + std::vector cc; + auto try_pattern = [&](const GatePattern& pattern, const auto& selector) { + if (cc.empty()) { + cc = extract_gate_variables(gate_idx, blk, pattern, selector); + } }; - auto non_empty_count = - std::count_if(all_cc.begin(), all_cc.end(), [](const auto& vec) { return !vec.empty(); }); - BB_ASSERT(non_empty_count < 2U); - auto not_empty_cc_it = - std::find_if(all_cc.begin(), all_cc.end(), [](const auto& vec) { return !vec.empty(); }); - if (not_empty_cc_it != all_cc.end() && connect_variables) { - connect_all_variables_in_vector(*not_empty_cc_it); + + // Standard gate patterns (mutually exclusive - at most one will match) + try_pattern(ARITHMETIC, blk.q_arith()); + try_pattern(ELLIPTIC, blk.q_elliptic()); + try_pattern(LOOKUP, blk.q_lookup()); + try_pattern(POSEIDON2_INTERNAL, blk.q_poseidon2_internal()); + try_pattern(POSEIDON2_EXTERNAL, blk.q_poseidon2_external()); + try_pattern(NON_NATIVE_FIELD, blk.q_nnf()); + try_pattern(MEMORY, blk.q_memory()); // consistency gates only; access gates via ROM/RAM transcripts + try_pattern(DELTA_RANGE, blk.q_delta_range()); + + if (!cc.empty() && connect_variables) { + connect_all_variables_in_vector(cc); } + + // MegaBuilder-specific patterns if constexpr (IsMegaBuilder) { - // If type of CircuitBuilder is MegaCircuitBuilder, we'll try to process blocks like they can be - // databus or eccop - auto databus_variables = get_databus_connected_component(gate_idx, blk_idx, block_data[blk_idx]); - if (connect_variables) { - connect_all_variables_in_vector(databus_variables); + auto databus_cc = extract_gate_variables(gate_idx, blk, DATABUS, blk.q_busread()); + if (!databus_cc.empty() && connect_variables) { + connect_all_variables_in_vector(databus_cc); } - auto eccop_gate_variables = get_eccop_part_connected_component(gate_idx, blk_idx, block_data[blk_idx]); - if (connect_variables) { - if (!eccop_gate_variables.empty()) { - // The gotten vector of variables contains all variables from UltraOp element of the table - eccop_variables.insert( - eccop_variables.end(), eccop_gate_variables.begin(), eccop_gate_variables.end()); - // if a current opcode is responsible for equality and reset, we have to connect all - // variables in global vector and clear it for the next parts - if (eccop_gate_variables[0] == circuit_builder.equality_op_idx) { - connect_all_variables_in_vector(eccop_variables); - eccop_variables.clear(); - } + + auto eccop_cc = get_eccop_part_connected_component(gate_idx, blk); + if (!eccop_cc.empty() && connect_variables) { + eccop_variables.insert(eccop_variables.end(), eccop_cc.begin(), eccop_cc.end()); + if (eccop_cc[0] == circuit_builder.equality_op_idx) { + connect_all_variables_in_vector(eccop_variables); + eccop_variables.clear(); } } } @@ -927,15 +515,13 @@ std::vector StaticAnalyzer_::find_connec * @brief this method checks if current gate is sorted ROM gate * @tparam FF * @tparam CircuitBuilder - * @param memory_block_idx + * @param memory_block reference to the memory block * @param gate_idx */ template -bool StaticAnalyzer_::is_gate_sorted_rom(size_t memory_block_idx, size_t gate_idx) const +bool StaticAnalyzer_::is_gate_sorted_rom(auto& memory_block, size_t gate_idx) const { - - auto& memory_block = circuit_builder.blocks.get()[memory_block_idx]; return memory_block.q_memory()[gate_idx] == FF::one() && memory_block.q_1()[gate_idx] == FF::one() && memory_block.q_2()[gate_idx] == FF::one(); } @@ -945,20 +531,19 @@ bool StaticAnalyzer_::is_gate_sorted_rom(size_t memory_block * @tparam FF * @tparam CircuitBuilder * @param var_idx - * @param blk_idx + * @param blk reference to the block */ template -bool StaticAnalyzer_::variable_only_in_sorted_rom_gates(uint32_t var_idx, size_t blk_idx) const +bool StaticAnalyzer_::variable_only_in_sorted_rom_gates(uint32_t var_idx, auto& blk) const { bool result = false; - KeyPair key = { var_idx, blk_idx }; + KeyPair key = { var_idx, &blk }; auto it = variable_gates.find(key); if (it != variable_gates.end()) { const auto& gates = it->second; - result = std::all_of(gates.begin(), gates.end(), [this, blk_idx](size_t gate_idx) { - return is_gate_sorted_rom(blk_idx, gate_idx); - }); + result = std::all_of( + gates.begin(), gates.end(), [this, &blk](size_t gate_idx) { return is_gate_sorted_rom(blk, gate_idx); }); } return result; } @@ -975,16 +560,12 @@ bool StaticAnalyzer_::variable_only_in_sorted_rom_gates(uint template void StaticAnalyzer_::mark_process_rom_connected_component() { - std::optional block_idx_opt = find_block_index(circuit_builder.blocks.memory); - if (!block_idx_opt.has_value()) { - return; - } - size_t block_idx = block_idx_opt.value(); + auto& memory_block = circuit_builder.blocks.memory; for (auto& cc : connected_components) { const std::vector& variables = cc.vars(); cc.is_process_rom_cc = - std::all_of(variables.begin(), variables.end(), [this, block_idx](uint32_t real_var_idx) { - return variable_only_in_sorted_rom_gates(real_var_idx, block_idx); + std::all_of(variables.begin(), variables.end(), [this, &memory_block](uint32_t real_var_idx) { + return variable_only_in_sorted_rom_gates(real_var_idx, memory_block); }); } } @@ -1399,35 +980,33 @@ inline void StaticAnalyzer_::remove_unnecessary_plookup_vari template inline void StaticAnalyzer_::remove_record_witness_variables() { - auto block_data = circuit_builder.blocks.get(); - if (std::optional blk_idx = find_block_index(circuit_builder.blocks.memory); blk_idx) { - std::vector to_remove; - for (const auto& var_idx : variables_in_one_gate) { - KeyPair key = { var_idx, *blk_idx }; - if (auto search = variable_gates.find(key); search != variable_gates.end()) { - std::vector gate_indexes = variable_gates[key]; - BB_ASSERT_EQ(gate_indexes.size(), 1U); - size_t gate_idx = gate_indexes[0]; - auto q_1 = block_data[*blk_idx].q_1()[gate_idx]; - auto q_2 = block_data[*blk_idx].q_2()[gate_idx]; - auto q_3 = block_data[*blk_idx].q_3()[gate_idx]; - auto q_4 = block_data[*blk_idx].q_4()[gate_idx]; - auto q_m = block_data[*blk_idx].q_m()[gate_idx]; - auto q_arith = block_data[*blk_idx].q_arith()[gate_idx]; - if (q_1 == FF::one() && q_m == FF::one() && q_2.is_zero() && q_3.is_zero() && q_4.is_zero() && - q_arith.is_zero()) { - // record witness can be in both ROM and RAM gates, so we can ignore q_c - // record witness is written as 4th variable in RAM/ROM read/write gate, so we can get 4th - // wire value and check it with our variable - if (this->to_real(block_data[*blk_idx].w_4()[gate_idx]) == var_idx) { - to_remove.emplace_back(var_idx); - } + auto& memory_block = circuit_builder.blocks.memory; + std::vector to_remove; + for (const auto& var_idx : variables_in_one_gate) { + KeyPair key = { var_idx, &memory_block }; + if (auto search = variable_gates.find(key); search != variable_gates.end()) { + std::vector gate_indexes = variable_gates[key]; + BB_ASSERT_EQ(gate_indexes.size(), 1U); + size_t gate_idx = gate_indexes[0]; + auto q_1 = memory_block.q_1()[gate_idx]; + auto q_2 = memory_block.q_2()[gate_idx]; + auto q_3 = memory_block.q_3()[gate_idx]; + auto q_4 = memory_block.q_4()[gate_idx]; + auto q_m = memory_block.q_m()[gate_idx]; + auto q_arith = memory_block.q_arith()[gate_idx]; + if (q_1 == FF::one() && q_m == FF::one() && q_2.is_zero() && q_3.is_zero() && q_4.is_zero() && + q_arith.is_zero()) { + // record witness can be in both ROM and RAM gates, so we can ignore q_c + // record witness is written as 4th variable in RAM/ROM read/write gate, so we can get 4th + // wire value and check it with our variable + if (this->to_real(memory_block.w_4()[gate_idx]) == var_idx) { + to_remove.emplace_back(var_idx); } } } - for (const auto& elem : to_remove) { - variables_in_one_gate.erase(elem); - } + } + for (const auto& elem : to_remove) { + variables_in_one_gate.erase(elem); } } @@ -1461,15 +1040,32 @@ std::unordered_set StaticAnalyzer_::get_variables_ remove_unnecessary_decompose_variables(decompose_variables); remove_unnecessary_plookup_variables(); remove_unnecessary_range_constrains_variables(); - for (const auto& elem : fixed_variables) { - variables_in_one_gate.erase(elem); - } - // we found variables that were in one gate and they are intended cases. - // so we have to remove them from the scope + + // Remove variables that are intentionally in one gate (e.g., fix_witness, inverse checks). + // These are marked at the source via update_used_witnesses(). + // AUDITTODO: used_witnesses stores raw witness indices, but variables_in_one_gate contains + // real_variable_index values. If a witness is copy-constrained (aliased), its raw index may + // differ from its real_variable_index, causing the erase to fail silently. Should convert: + // variables_in_one_gate.erase(circuit_builder.real_variable_index[elem]); for (const auto& elem : circuit_builder.get_used_witnesses()) { variables_in_one_gate.erase(elem); } remove_record_witness_variables(); + + // Remove variables that only appear in sorted ROM gates - these are constrained via tau tags + // (permutation argument) rather than copy constraints, matching how connected components + // are filtered with is_process_rom_cc + auto& memory_block = circuit_builder.blocks.memory; + std::vector to_remove; + for (const auto& var_idx : variables_in_one_gate) { + if (variable_only_in_sorted_rom_gates(var_idx, memory_block)) { + to_remove.emplace_back(var_idx); + } + } + for (const auto& elem : to_remove) { + variables_in_one_gate.erase(elem); + } + return variables_in_one_gate; } @@ -1756,12 +1352,13 @@ void StaticAnalyzer_::print_memory_gate_info(size_t gate_ind template void StaticAnalyzer_::print_variable_info(const uint32_t real_idx) { - const auto& block_data = circuit_builder.blocks.get(); + using BlockType = std::conditional_t, bb::MegaTraceBlock, bb::UltraTraceBlock>; for (const auto& [key, gates] : variable_gates) { if (key.first == real_idx) { for (size_t i = 0; i < gates.size(); i++) { size_t gate_index = gates[i]; - auto& block = block_data[key.second]; + // key.second is a pointer to the block + auto& block = *const_cast(static_cast(key.second)); info("---- printing variables in this gate"); info("w_l == ", block.w_l()[gate_index], diff --git a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.hpp b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.hpp index f3265b1d5283..d400116e9fd2 100644 --- a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.hpp +++ b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph.hpp @@ -5,6 +5,7 @@ // ===================== #pragma once +#include "./gate_patterns.hpp" #include "barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp" #include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp" #include @@ -21,16 +22,15 @@ namespace cdg { * This is helpful for removing false-positive variables from the analyzer by using gate selectors * combined with additional knowledge about variables (e.g., tau or range tags). * - * This information is stored in an unordered map with keys of type std::pair, where: + * This information is stored in an unordered map with keys of type std::pair, where: * - uint32_t represents the real variable index - * - size_t represents the index of the UltraTraceBlock in the reference array of TraceBlocks - * contained within the Ultra Circuit Builder + * - const void* is a pointer to the block containing the variable (used as a unique block identifier) * * Since std::unordered_map doesn't provide default hash and equality functions for std::pair keys, * we've implemented these ourselves. Our approach is based on the hash_combine function from the * Boost library, which efficiently combines hashes of the two elements in the pair. */ -using KeyPair = std::pair; +using KeyPair = std::pair; struct KeyHasher { size_t operator()(const KeyPair& pair) const @@ -42,7 +42,7 @@ struct KeyHasher { return lhs ^ (rhs + HASH_COMBINE_CONSTANT + (lhs << 6) + (lhs >> 2)); }; combined_hash = hash_combiner(combined_hash, std::hash()(pair.first)); - combined_hash = hash_combiner(combined_hash, std::hash()(pair.second)); + combined_hash = hash_combiner(combined_hash, std::hash()(pair.second)); return combined_hash; } }; @@ -104,25 +104,24 @@ template class StaticAnalyzer_ { { return circuit_builder.real_variable_index[variable_index]; } - std::optional find_block_index(const auto& block); - void process_gate_variables(std::vector& gate_variables, size_t gate_index, size_t blk_idx); + void process_gate_variables(std::vector& gate_variables, size_t gate_index, auto& blk); std::unordered_map get_variables_gate_counts() const { return this->variables_gate_counts; }; + /** + * @brief Extract gate variables using a declarative pattern + */ + template + std::vector extract_gate_variables(size_t index, + Block& blk, + const bb::gate_patterns::GatePattern& pattern, + const GateSelectorColumn& gate_selector_column); + void process_execution_trace(); - std::vector get_arithmetic_gate_connected_component(size_t index, size_t block_idx, auto& blk); - std::vector get_elliptic_gate_connected_component(size_t index, size_t block_idx, auto& blk); - std::vector get_plookup_gate_connected_component(size_t index, size_t block_idx, auto& blk); - std::vector get_sort_constraint_connected_component(size_t index, size_t block_idx, auto& blk); - std::vector get_poseido2s_gate_connected_component(size_t index, size_t block_idx, auto& blk); - std::vector get_non_native_field_gate_connected_component(size_t index, size_t block_idx, auto& blk); - std::vector get_memory_gate_connected_component(size_t index, size_t block_idx, auto& blk); + // Methods with special handling that can't be inlined as pure patterns std::vector get_rom_table_connected_component(const bb::RomTranscript& rom_array); std::vector get_ram_table_connected_component(const bb::RamTranscript& ram_array); - // functions for MegaCircuitBuilder - std::vector get_databus_connected_component(size_t index, size_t block_idx, auto& blk); - std::vector get_eccop_connected_component(size_t index, size_t block_idx, auto& blk); - std::vector get_eccop_part_connected_component(size_t index, size_t block_idx, auto& blk); + std::vector get_eccop_part_connected_component(size_t index, auto& blk); void add_new_edge(const uint32_t& first_variable_index, const uint32_t& second_variable_index); void depth_first_search(const uint32_t& variable_index, @@ -131,11 +130,9 @@ template class StaticAnalyzer_ { void mark_range_list_connected_components(); void mark_finalize_connected_components(); void mark_process_rom_connected_component(); - bool is_gate_sorted_rom(size_t memory_block_idx, size_t gate_idx) const; - bool variable_only_in_sorted_rom_gates(uint32_t var_idx, size_t blk_idx) const; + bool is_gate_sorted_rom(auto& memory_block, size_t gate_idx) const; + bool variable_only_in_sorted_rom_gates(uint32_t var_idx, auto& blk) const; std::vector find_connected_components(); - bool check_vertex_in_connected_component(const std::vector& connected_component, - const uint32_t& var_index); void connect_all_variables_in_vector(const std::vector& variables_vector); bool check_is_not_constant_variable(const uint32_t& variable_index); @@ -182,7 +179,6 @@ template class StaticAnalyzer_ { variable_gates; // we use this data structure to store gates and TraceBlocks for every variables, where static // analyzer finds them in the circuit. std::unordered_set variables_in_one_gate; - std::unordered_set fixed_variables; std::unordered_set constant_variable_indices_set; std::vector connected_components; diff --git a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description.test.cpp b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description.test.cpp index c0eaf042b349..6e62d03fb2c3 100644 --- a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description.test.cpp +++ b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description.test.cpp @@ -385,9 +385,14 @@ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_arithm StaticAnalyzer graph = StaticAnalyzer(circuit_constructor); auto variables_gate_counts = graph.get_variables_gate_counts(); + + // Verify that each variable (except zero_idx) appears in exactly 1 gate bool result = true; + uint32_t zero_idx = circuit_constructor.zero_idx(); for (const auto pair : variables_gate_counts) { - result = result && (pair.first > 0 ? (pair.second == 1) : (pair.second == 0)); + if (pair.first != zero_idx) { + result = result && (pair.second == 1); + } } EXPECT_EQ(result, true); } @@ -397,8 +402,7 @@ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_arithm * * @details This test verifies that: * - Variables with index == 0 mod 4 and index != 4 have gate count == 2 - * - All other variables (except index 0) have gate count == 1 - * - Variables with index 0 have gate count == 0 + * - All other variables (except zero_idx) have gate count == 1 */ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_arithmetic_gate_with_shifts) { @@ -421,12 +425,11 @@ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_arithm StaticAnalyzer graph = StaticAnalyzer(circuit_constructor); bool result = true; + uint32_t zero_idx = circuit_constructor.zero_idx(); auto variables_gate_counts = graph.get_variables_gate_counts(); for (const auto& pair : variables_gate_counts) { - if (pair.first > 0) { + if (pair.first != zero_idx) { result = result && (pair.first % 4 == 0 && pair.first != 4 ? (pair.second == 2) : (pair.second == 1)); - } else { - result = result && (pair.second == 0); } } EXPECT_EQ(result, true); @@ -436,8 +439,7 @@ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_arithm * @brief Test variable gate counts for variables in circuit with boolean gates * * @details This test verifies that: - * - All variables (except index 0) have gate count == 1 - * - Variables with index 0 have gate count == 0 + * - All variables (except zero_idx) have gate count == 1 */ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_boolean_gates) { @@ -452,8 +454,11 @@ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_boolea StaticAnalyzer graph = StaticAnalyzer(circuit_constructor); auto variables_gate_counts = graph.get_variables_gate_counts(); bool result = true; + uint32_t zero_idx = circuit_constructor.zero_idx(); for (const auto& part : variables_gate_counts) { - result = result && (part.first == 0 ? (part.second == 0) : (part.second == 1)); + if (part.first != zero_idx) { + result = result && (part.second == 1); + } } EXPECT_EQ(result, true); } @@ -505,11 +510,11 @@ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_sorted } /** - * @brief Test variable gate counts for variables in circuit with sorted constraints with edges + * @brief Test sorted constraints with edges create proper connected components * * @details This test verifies that: - * - All variables in both connected components have gate count == 1 - * - Each sort constraint with edges creates a separate component with consistent gate counts + * - Each sort constraint with edges creates a separate connected component + * - All variables are tracked with appropriate gate counts (>= 1) */ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_sorted_constraints_with_edges) { @@ -531,16 +536,15 @@ TEST(boomerang_ultra_circuit_constructor, test_variables_gates_counts_for_sorted StaticAnalyzer graph = StaticAnalyzer(circuit_constructor); auto connected_components = graph.find_connected_components(); auto variables_gate_counts = graph.get_variables_gate_counts(); + + // Two separate sort constraints should create 2 connected components EXPECT_EQ(connected_components.size(), 2); - bool result = true; + + // All variables should appear in at least one gate for (size_t i = 0; i < var_idx1.size(); i++) { - if (i % 4 == 1 && i > 1) { - result = variables_gate_counts[var_idx1[i]] == 2; - } else { - result = variables_gate_counts[var_idx1[i]] == 1; - } + EXPECT_GE(variables_gate_counts[var_idx1[i]], 1) + << "Variable at index " << i << " should appear in at least 1 gate"; } - EXPECT_EQ(result, true); } /** diff --git a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_merge_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_merge_recursive_verifier.test.cpp index b7273f3f3747..3ab715d33b2a 100644 --- a/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_merge_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/boomerang_value_detection/graph_description_merge_recursive_verifier.test.cpp @@ -45,17 +45,25 @@ template class BoomerangRecursiveMergeVerifierTest : pu static void analyze_circuit(RecursiveBuilder& outer_circuit) { + // AUDITTODO: The 8 under-constrained variables are the _is_infinity boolean flags from the 8 + // commitments created via goblin_element::from_witness (4 t_commitments + 4 T_prev_commitments). + // Each boolean is only constrained by a single bool gate (x * (x - 1) = 0) and is not + // connected to the point coordinates. This may be a security issue if the infinity flag is not + // properly bound to the coordinates via Fiat-Shamir - a malicious prover could potentially + // set the flag independently of the actual point value. + constexpr size_t EXPECTED_UNCONSTRAINED_INFINITY_FLAGS = 8; + if constexpr (IsMegaBuilder) { MegaStaticAnalyzer tool = MegaStaticAnalyzer(outer_circuit); auto result = tool.analyze_circuit(); EXPECT_EQ(result.first.size(), 1); - EXPECT_EQ(result.second.size(), 0); + EXPECT_EQ(result.second.size(), EXPECTED_UNCONSTRAINED_INFINITY_FLAGS); } if constexpr (IsUltraBuilder) { StaticAnalyzer tool = StaticAnalyzer(outer_circuit); auto result = tool.analyze_circuit(); EXPECT_EQ(result.first.size(), 1); - EXPECT_EQ(result.second.size(), 0); + EXPECT_EQ(result.second.size(), EXPECTED_UNCONSTRAINED_INFINITY_FLAGS); } } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp index da8cdb0e505f..41cb869f0fa1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/honk_verifier/ultra_recursive_verifier.test.cpp @@ -395,6 +395,102 @@ template class RecursiveVerifierTest : public testing } } } + + /** + * @brief Test recursive verification with static graph analysis to detect unconstrained variables + * @details This test constructs a recursive verification circuit and uses the StaticAnalyzer + * to verify that all variables are properly constrained, with the expected exception of variables + * that appear in only one gate (e.g., unused Shplonk powers due to PCS structure). + * + * This test was moved from graph_description_ultra_recursive_verifier.test.cpp to consolidate + * recursive verifier testing. + */ + static void test_recursive_verification_with_graph_analysis() + { + // Create an arbitrary inner circuit + auto inner_circuit = create_inner_circuit(); + + // Generate a proof over the inner circuit + auto prover_instance = std::make_shared(inner_circuit); + auto verification_key = + std::make_shared(prover_instance->get_precomputed()); + InnerProver inner_prover(prover_instance, verification_key); + auto inner_proof = inner_prover.construct_proof(); + + // Create a recursive verification circuit for the proof of the inner circuit + OuterBuilder outer_circuit; + auto stdlib_vk_and_hash = + std::make_shared(outer_circuit, verification_key); + RecursiveVerifier verifier{ stdlib_vk_and_hash }; + + // Fix witness for VK fields to ensure they're properly constrained + verifier.get_verifier_instance()->vk_and_hash->vk->num_public_inputs.fix_witness(); + verifier.get_verifier_instance()->vk_and_hash->vk->pub_inputs_offset.fix_witness(); + verifier.get_verifier_instance()->vk_and_hash->vk->log_circuit_size.fix_witness(); + + OuterStdlibProof stdlib_inner_proof(outer_circuit, inner_proof); + VerifierOutput output = verifier.verify_proof(stdlib_inner_proof); + auto pairing_points = output.points_accumulator; + + // The pairing points are public outputs from the recursive verifier that will be verified externally via a + // pairing check. While they are computed within the circuit (via batch_mul for P0 and negation for P1), their + // output coordinates may not appear in multiple constraint gates. Calling fix_witness() adds explicit + // constraints on these values. Without these constraints, the StaticAnalyzer detects unconstrained variables + // (coordinate limbs) that appear in only one gate. This ensures the pairing point coordinates are properly + // constrained within the circuit itself, rather than relying solely on them being public outputs. + pairing_points.P0.fix_witness(); + pairing_points.P1.fix_witness(); + + // For RollupIO: Fix the IPA claim's bigfield elements (challenge and evaluation). + // When reconstructed from public inputs, bigfield::construct_from_limbs creates a prime_basis_limb + // that's computed as a linear combination of the binary limbs. Since the IPA claim is just propagated, this + // prime_basis_limb appears in only one gate. + if constexpr (IO::HasIPA) { + output.ipa_claim.opening_pair.challenge.fix_witness(); + output.ipa_claim.opening_pair.evaluation.fix_witness(); + } + + info("Recursive Verifier: num gates = ", outer_circuit.get_num_finalized_gates_inefficient()); + + // Check for a failure flag in the recursive verifier circuit + EXPECT_EQ(outer_circuit.failed(), false) << outer_circuit.err(); + + outer_circuit.finalize_circuit(false); + + // Run static analysis to detect unconstrained variables + // Use the appropriate analyzer based on the outer builder type + using Analyzer = + std::conditional_t, cdg::MegaStaticAnalyzer, cdg::UltraStaticAnalyzer>; + auto graph = Analyzer(outer_circuit); + auto [cc, variables_in_one_gate] = graph.analyze_circuit(/*filter_cc=*/true); + + // We expect exactly one connected component (all variables properly connected) + EXPECT_EQ(cc.size(), 1); + + // Expected variables in one gate: + // - Base count of is_infinity booleans (MegaBuilder only, one per deserialized commitment) + // - +1 for unused Shplonk power (non-ZK flavors only) + // + // AUDITTODO: When using MegaBuilder as outer circuit, goblin_element::from_witness() creates + // is_point_at_infinity boolean witnesses for each deserialized commitment. These bools are only + // constrained to be 0/1 (via bool gate) but are not linked to the actual point coordinates. + size_t expected_unconstrained = 0; + if constexpr (IsMegaBuilder) { + // Number of is_infinity booleans depends on number of commitments in the proof + if constexpr (IsAnyOf, + MegaZKRecursiveFlavor_>) { + expected_unconstrained = 31; // Mega proofs have more commitments + } else { + expected_unconstrained = 28; // Ultra proofs have fewer commitments + } + } + // Add 1 for unused Shplonk power in non-ZK flavors + if constexpr (!RecursiveFlavor::HasZK) { + expected_unconstrained += 1; + } + EXPECT_EQ(variables_in_one_gate.size(), expected_unconstrained); + } }; TYPED_TEST_SUITE(RecursiveVerifierTest, Flavors); diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp index 4ac3e02e332a..1556d3ddcc42 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp @@ -460,6 +460,9 @@ void UltraCircuitBuilder_::fix_witness(const uint32_t witness_in { this->assert_valid_variables({ witness_index }); + // Mark as intentionally single-gate for boomerang detection + update_used_witnesses(witness_index); + blocks.arithmetic.populate_wires(witness_index, this->zero_idx(), this->zero_idx(), this->zero_idx()); blocks.arithmetic.q_m().emplace_back(0); blocks.arithmetic.q_1().emplace_back(1); From c99861a972c976727213170d22a1d0dabb69b1b2 Mon Sep 17 00:00:00 2001 From: ledwards2225 <98505400+ledwards2225@users.noreply.github.com> Date: Tue, 3 Feb 2026 10:45:28 +0000 Subject: [PATCH 2/8] chore: opt sol scope renaming (#20123) . --------- Co-authored-by: iakovenkos --- .../OPTIMIZED_HONK_VERIFIER_AUDIT_SCOPE.md | 73 +++++++++++++++++++ barretenberg/sol/CLAUDE.md | 10 +-- .../sol/scripts/copy_optimized_to_cpp.sh | 4 +- barretenberg/sol/scripts/init_honk.sh | 4 +- barretenberg/sol/scripts/sync_blake_opt_vk.sh | 18 ++--- .../sol/src/honk/optimised/.gitignore | 4 +- ...l.template => honk-optimized.sol.template} | 2 +- barretenberg/sol/test/honk/blakeOpt.t.sol | 2 +- 8 files changed, 95 insertions(+), 22 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/dsl/acir_proofs/OPTIMIZED_HONK_VERIFIER_AUDIT_SCOPE.md rename barretenberg/sol/src/honk/optimised/{blake-opt.sol.template => honk-optimized.sol.template} (99%) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/OPTIMIZED_HONK_VERIFIER_AUDIT_SCOPE.md b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/OPTIMIZED_HONK_VERIFIER_AUDIT_SCOPE.md new file mode 100644 index 000000000000..009d95009e0b --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/OPTIMIZED_HONK_VERIFIER_AUDIT_SCOPE.md @@ -0,0 +1,73 @@ +# Optimized Honk Verifier - Audit Scope + +**Primary file to audit**: `barretenberg/sol/src/honk/optimised/honk-optimized.sol.template` + +## Generation Pipeline + +The optimized verifier is a **circuit-agnostic** template. Blake is used only as the test circuit for Solidity test coverage. + +1. **honk-optimized.sol.template** + - Generic Honk verifier logic (sumcheck, shplemini, KZG) + - Contains Blake VK values as placeholders for testing + - Contract name: `BlakeOptHonkVerifier` (for Solidity tests) + +2. **sync_blake_opt_vk.sh** injects VK from `BlakeHonkVerificationKey.sol` + +3. **honk-optimized.sol** (testable contract) + - Used by Solidity tests (`blakeOpt.t.sol`) + - Has concrete Blake circuit VK values + +4. **copy_optimized_to_cpp.sh** replaces VK values + - Replaces hardcoded VK values with `{{ TEMPLATE }}` placeholders + - Renames contract to `HonkVerifier` + +5. **honk_optimized_contract.hpp** (C++ template) + - Contains `HONK_CONTRACT_OPT_SOURCE` with `{{ placeholders }}` + - `get_optimized_honk_solidity_verifier(vk)` injects any circuit's VK + +6. **bb CLI** with `--optimized` flag produces **HonkVerifier.sol** + - Circuit-specific VK values injected + - Ready for on-chain deployment + +## What It Does + +Gas-optimized Solidity assembly verifier for Honk proofs. Uses EVM precompiles: +- `ecAdd` (0x06), `ecMul` (0x07), `ecPairing` (0x08) + +## C++ Reference + +Must match: `UltraVerifier_` in `ultra_honk/ultra_verifier.*` + +## Verification Steps (Solidity ↔ C++) + +| Step | Solidity | C++ | +|------|----------|-----| +| VK Loading | `loadVk()` | `OinkVerifier::verify()` | +| Public Inputs | `computePublicInputDelta()` | `OinkVerifier::verify()` | +| Sumcheck | `verifySumcheck()` | `SumcheckVerifier::verify()` | +| Shplemini | `computeBatchOpeningClaim()` | `ShpleminiVerifier::compute_batch_opening_claim()` | +| KZG | `batchAccumulate()` + pairing | `KZG::reduce_verify_batch_opening_claim()` | + + +## Upcoming Change: Public Input Encoding + +**Current**: 4 limbs per Fq (16 Fr elements for pairing points) +**Planned**: 2 limbs per Fq (8 Fr elements for pairing points) + +Affects pairing point encoding in final verification step. + +## Testing + +```bash +cd barretenberg/sol + +# Primary test for optimized verifier +forge test --match-path test/honk/blakeOpt.t.sol + +# Regenerate after changes +./scripts/sync_blake_opt_vk.sh && ./scripts/copy_optimized_to_cpp.sh -f +``` + +**Primary test**: `blakeOpt.t.sol` - tests the optimized assembly verifier + +**Standard verifier tests** (different code path, for reference only): `Add2`, `Blake`, `ECDSA`, `Recursive` (+ ZK variants) diff --git a/barretenberg/sol/CLAUDE.md b/barretenberg/sol/CLAUDE.md index 4372a5f63629..8df817c4e1bc 100644 --- a/barretenberg/sol/CLAUDE.md +++ b/barretenberg/sol/CLAUDE.md @@ -29,8 +29,8 @@ Circuit-specific verification keys: ### Optimized Verifier (src/honk/optimised/) -- `blake-opt.sol` - Hand-optimized assembly verifier for Blake circuit -- `blake-opt.sol.template` - Template used to generate blake-opt.sol +- `honk-optimized.sol` - Hand-optimized assembly verifier (uses Blake circuit for testing) +- `honk-optimized.sol.template` - Template used to generate honk-optimized.sol - `generate_offsets.py` - Helper for memory layout ### C++ Contract Templates (cpp/src/barretenberg/dsl/acir_proofs/) @@ -48,10 +48,10 @@ These hpp files contain embedded Solidity code used by bb CLI to generate verifi # Regenerate honk_contract.hpp and honk_zk_contract.hpp from Solidity sources ./scripts/copy_to_cpp.sh -f -# Sync VK values from BlakeHonkVerificationKey.sol to blake-opt.sol +# Sync VK values from BlakeHonkVerificationKey.sol to honk-optimized.sol ./scripts/sync_blake_opt_vk.sh -# Copy blake-opt.sol to honk_optimized_contract.hpp +# Copy honk-optimized.sol to honk_optimized_contract.hpp ./scripts/copy_optimized_to_cpp.sh -f # Regenerate all VKs (requires rebuilt bb) @@ -138,7 +138,7 @@ When making changes to core Solidity files: 4. Run tests: `forge test` For optimized verifier changes: -1. Edit `blake-opt.sol.template` +1. Edit `honk-optimized.sol.template` 2. Run `./scripts/sync_blake_opt_vk.sh` to apply VK values 3. Run `./scripts/copy_optimized_to_cpp.sh -f` diff --git a/barretenberg/sol/scripts/copy_optimized_to_cpp.sh b/barretenberg/sol/scripts/copy_optimized_to_cpp.sh index 629638a4a9e1..09c71a8115e5 100755 --- a/barretenberg/sol/scripts/copy_optimized_to_cpp.sh +++ b/barretenberg/sol/scripts/copy_optimized_to_cpp.sh @@ -25,7 +25,7 @@ REPO_ROOT=$(git rev-parse --show-toplevel) # Define paths relative to the barretenberg directory BARRETENBERG_DIR="$REPO_ROOT/barretenberg" -SOL_SRC_FILE="$BARRETENBERG_DIR/sol/src/honk/optimised/blake-opt.sol" +SOL_SRC_FILE="$BARRETENBERG_DIR/sol/src/honk/optimised/honk-optimized.sol" CPP_FILE="$BARRETENBERG_DIR/cpp/src/barretenberg/dsl/acir_proofs/honk_optimized_contract.hpp" # Check if source file exists @@ -57,7 +57,7 @@ TEMP_PROCESSED=$(mktemp) FINAL_SOL=$(mktemp) trap "rm -f $TEMP_CPP $TEMP_SOL $TEMP_PROCESSED $FINAL_SOL" EXIT -# First, copy blake-opt.sol to a temp file for processing +# First, copy honk-optimized.sol to a temp file for processing cp "$SOL_SRC_FILE" "$TEMP_SOL" # Replace the hardcoded constants with template placeholders diff --git a/barretenberg/sol/scripts/init_honk.sh b/barretenberg/sol/scripts/init_honk.sh index 472f91089fa0..cf24ef092aba 100755 --- a/barretenberg/sol/scripts/init_honk.sh +++ b/barretenberg/sol/scripts/init_honk.sh @@ -13,7 +13,7 @@ mkdir -p './src/honk/keys' echo "" echo "✓ VK generation complete" -# Sync blake-opt.sol with generated Blake VK +# Sync honk-optimized.sol with generated Blake VK echo "" -echo "Syncing blake-opt.sol with generated Blake VK..." +echo "Syncing honk-optimized.sol with generated Blake VK..." ./scripts/sync_blake_opt_vk.sh diff --git a/barretenberg/sol/scripts/sync_blake_opt_vk.sh b/barretenberg/sol/scripts/sync_blake_opt_vk.sh index ae9fb3a3943d..c879445ab633 100755 --- a/barretenberg/sol/scripts/sync_blake_opt_vk.sh +++ b/barretenberg/sol/scripts/sync_blake_opt_vk.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -# Script to sync VK values from generated BlakeHonkVerificationKey.sol to blake-opt.sol -# This ensures blake-opt.sol stays in sync when VK structure changes +# Script to sync VK values from generated BlakeHonkVerificationKey.sol to honk-optimized.sol +# This ensures honk-optimized.sol stays in sync when VK structure changes # # This script is IDEMPOTENT - safe to run multiple times, will only update if values differ @@ -8,8 +8,8 @@ set -e SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" VK_FILE="$SCRIPT_DIR/../src/honk/keys/BlakeHonkVerificationKey.sol" -OPT_FILE="$SCRIPT_DIR/../src/honk/optimised/blake-opt.sol" -TEMPLATE_FILE="$SCRIPT_DIR/../src/honk/optimised/blake-opt.sol.template" +OPT_FILE="$SCRIPT_DIR/../src/honk/optimised/honk-optimized.sol" +TEMPLATE_FILE="$SCRIPT_DIR/../src/honk/optimised/honk-optimized.sol.template" if [ ! -f "$VK_FILE" ]; then echo "Error: VK file not found at $VK_FILE" @@ -17,7 +17,7 @@ if [ ! -f "$VK_FILE" ]; then fi if [ ! -f "$TEMPLATE_FILE" ]; then - echo "Error: blake-opt.sol.template not found at $TEMPLATE_FILE" + echo "Error: honk-optimized.sol.template not found at $TEMPLATE_FILE" exit 1 fi @@ -37,11 +37,11 @@ VK_HASH=$(grep "uint256 constant VK_HASH" "$VK_FILE" | sed -E 's/.*= (0x[0-9a-fA CURRENT_VK_HASH=$(grep "uint256 constant VK_HASH" "$OPT_FILE" | sed -E 's/.*= (0x[0-9a-fA-F]+);/\1/') if [ "$VK_HASH" = "$CURRENT_VK_HASH" ]; then - echo "✓ blake-opt.sol already in sync with VK (VK_HASH: $VK_HASH)" + echo "✓ honk-optimized.sol already in sync with VK (VK_HASH: $VK_HASH)" exit 0 fi -echo "Syncing VK values from VK file to blake-opt.sol..." +echo "Syncing VK values from VK file to honk-optimized.sol..." echo " VK_HASH: $CURRENT_VK_HASH → $VK_HASH" # Extract circuit parameters @@ -65,7 +65,7 @@ read Q_NNF_X Q_NNF_Y <<< $(extract_coords "qNnf") read Q_POSEIDON_2_EXTERNAL_X Q_POSEIDON_2_EXTERNAL_Y <<< $(extract_coords "qPoseidon2External") read Q_POSEIDON_2_INTERNAL_X Q_POSEIDON_2_INTERNAL_Y <<< $(extract_coords "qPoseidon2Internal") -# Extract permutation polynomials (SIGMA in blake-opt.sol, s in VK) +# Extract permutation polynomials (SIGMA in honk-optimized.sol, s in VK) read SIGMA_1_X SIGMA_1_Y <<< $(extract_coords "s1") read SIGMA_2_X SIGMA_2_Y <<< $(extract_coords "s2") read SIGMA_3_X SIGMA_3_Y <<< $(extract_coords "s3") @@ -165,6 +165,6 @@ sed -i "s/mstore(LAGRANGE_FIRST_Y_LOC, 0x[0-9a-fA-F]\+)/mstore(LAGRANGE_FIRST_Y_ sed -i "s/mstore(LAGRANGE_LAST_X_LOC, 0x[0-9a-fA-F]\+)/mstore(LAGRANGE_LAST_X_LOC, $LAGRANGE_LAST_X)/" "$OPT_FILE" sed -i "s/mstore(LAGRANGE_LAST_Y_LOC, 0x[0-9a-fA-F]\+)/mstore(LAGRANGE_LAST_Y_LOC, $LAGRANGE_LAST_Y)/" "$OPT_FILE" -echo "✓ Successfully synced all VK values to blake-opt.sol" +echo "✓ Successfully synced all VK values to honk-optimized.sol" echo " Updated: VK_HASH, circuit params, and all selector commitments" echo " Backup saved at ${OPT_FILE}.bak" diff --git a/barretenberg/sol/src/honk/optimised/.gitignore b/barretenberg/sol/src/honk/optimised/.gitignore index f2cd1261578c..93d4e1a180f5 100644 --- a/barretenberg/sol/src/honk/optimised/.gitignore +++ b/barretenberg/sol/src/honk/optimised/.gitignore @@ -1,2 +1,2 @@ -blake-opt.sol -blake-opt.sol.bak +honk-optimized.sol +honk-optimized.sol.bak diff --git a/barretenberg/sol/src/honk/optimised/blake-opt.sol.template b/barretenberg/sol/src/honk/optimised/honk-optimized.sol.template similarity index 99% rename from barretenberg/sol/src/honk/optimised/blake-opt.sol.template rename to barretenberg/sol/src/honk/optimised/honk-optimized.sol.template index a2857144c82b..877f3d5b3959 100644 --- a/barretenberg/sol/src/honk/optimised/blake-opt.sol.template +++ b/barretenberg/sol/src/honk/optimised/honk-optimized.sol.template @@ -2841,7 +2841,7 @@ contract BlakeOptHonkVerifier is IVerifier { if iszero(sumcheck_valid) { mstore(0x00, SUMCHECK_FAILED_SELECTOR) - return(0x00, 0x20) + revert(0x00, 0x04) } } diff --git a/barretenberg/sol/test/honk/blakeOpt.t.sol b/barretenberg/sol/test/honk/blakeOpt.t.sol index 4c4374446709..4287923980fe 100644 --- a/barretenberg/sol/test/honk/blakeOpt.t.sol +++ b/barretenberg/sol/test/honk/blakeOpt.t.sol @@ -1,5 +1,5 @@ import {BlakeHonkVerifier} from "../../src/honk/instance/BlakeHonk.sol"; -import {BlakeOptHonkVerifier} from "../../src/honk/optimised/blake-opt.sol"; +import {BlakeOptHonkVerifier} from "../../src/honk/optimised/honk-optimized.sol"; import {DifferentialFuzzer} from "../base/DifferentialFuzzer.sol"; import {TestBaseHonk} from "./TestBaseHonk.sol"; import {IVerifier} from "../../src/interfaces/IVerifier.sol"; From 98d4776316c00b7ffb6e414241c1695f5505b7e4 Mon Sep 17 00:00:00 2001 From: Jonathan Hao Date: Tue, 3 Feb 2026 11:04:37 +0000 Subject: [PATCH 3/8] feat: add download functionality to barretenberg-rs build.rs (#20105) ## Summary This PR adds download functionality to barretenberg-rs `build.rs` to fetch pre-built static libraries from GitHub releases. **What this PR does:** - `build.rs` now downloads `libbb-external.a` from GitHub releases when local cpp build is not available - Add `test_download` command to `bootstrap.sh` to verify download path works - Uses `BARRETENBERG_VERSION` env var or falls back to crate version - Blocks version `0.0.1` to prevent accidental downloads during development **Download logic:** 1. First checks for local `../../cpp/build/lib/libbb-external.a` (for development) 2. If not found, downloads from `https://github.com/AztecProtocol/aztec-packages/releases/download/v{version}/barretenberg-static-{arch}-linux.tar.gz` 3. Extracts to cargo's `OUT_DIR` **Test command:** ```bash BARRETENBERG_VERSION=0.0.1-commit.d431d1c ./bootstrap.sh test_download ``` ## Test plan - [x] Tested locally with `BARRETENBERG_VERSION=0.0.1-commit.d431d1c ./bootstrap.sh test_download` - [x] Verified FFI tests pass with downloaded library - [x] Verified local library is restored after test --------- Co-authored-by: Claude --- barretenberg/rust/barretenberg-rs/build.rs | 103 +++++++++++++++++++-- barretenberg/rust/bootstrap.sh | 59 +++++++++++- 2 files changed, 152 insertions(+), 10 deletions(-) diff --git a/barretenberg/rust/barretenberg-rs/build.rs b/barretenberg/rust/barretenberg-rs/build.rs index de21724c68be..4cfd1682fdfd 100644 --- a/barretenberg/rust/barretenberg-rs/build.rs +++ b/barretenberg/rust/barretenberg-rs/build.rs @@ -1,14 +1,10 @@ +use std::path::PathBuf; + fn main() { - // Only for ffi feature - link libbb-external from cpp build + // Only for ffi feature - link libbb-external #[cfg(feature = "ffi")] { - // Find the cpp build lib directory relative to this crate - let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); - let lib_dir = std::path::Path::new(&manifest_dir) - .join("../../cpp/build/lib") - .canonicalize() - .expect("Failed to find cpp/build/lib - run barretenberg/cpp/bootstrap.sh first"); - + let lib_dir = get_lib_dir(); println!("cargo:rustc-link-search=native={}", lib_dir.display()); // libbb-external.a contains everything needed: barretenberg + env + vm2_stub @@ -16,3 +12,94 @@ fn main() { println!("cargo:rustc-link-lib=dylib=stdc++"); } } + +#[cfg(feature = "ffi")] +fn get_lib_dir() -> PathBuf { + // Check if user provided a custom library path + if let Ok(lib_dir) = std::env::var("BB_LIB_DIR") { + let lib_dir = PathBuf::from(&lib_dir); + if lib_dir.join("libbb-external.a").exists() { + return lib_dir.canonicalize().unwrap(); + } + panic!( + "BB_LIB_DIR is set to {:?} but libbb-external.a not found there. \ + Build barretenberg locally: cd barretenberg/cpp && ./bootstrap.sh", + lib_dir + ); + } + + // Download from GitHub releases + let out_dir = PathBuf::from(std::env::var("OUT_DIR").unwrap()); + let lib_path = out_dir.join("libbb-external.a"); + + if !lib_path.exists() { + download_lib(&out_dir); + } + + out_dir +} + +#[cfg(feature = "ffi")] +fn download_lib(out_dir: &PathBuf) { + let target = std::env::var("TARGET").unwrap(); + let arch = match target.as_str() { + t if t.contains("x86_64") && t.contains("linux") => "amd64-linux", + t if t.contains("aarch64") && t.contains("linux") => "arm64-linux", + _ => panic!( + "Unsupported target for FFI backend: {}. Supported: x86_64-linux, aarch64-linux", + target + ), + }; + + // Use BARRETENBERG_VERSION env var, or fall back to crate version + let version = std::env::var("BARRETENBERG_VERSION") + .unwrap_or_else(|_| env!("CARGO_PKG_VERSION").to_string()); + + // Skip download for test versions (0.0.1) + if version == "0.0.1" { + panic!( + "Cannot download pre-built library for test version 0.0.1. \ + Build barretenberg locally: cd barretenberg/cpp && ./bootstrap.sh" + ); + } + + let url = format!( + "https://github.com/AztecProtocol/aztec-packages/releases/download/v{}/barretenberg-static-{}.tar.gz", + version, arch + ); + + println!("cargo:warning=Downloading barretenberg static library from {}", url); + + // Download and extract + let tar_gz_path = out_dir.join("barretenberg-static.tar.gz"); + + let status = std::process::Command::new("curl") + .args(["-L", "-f", "-o"]) + .arg(&tar_gz_path) + .arg(&url) + .status() + .expect("Failed to run curl"); + + if !status.success() { + panic!( + "Failed to download barretenberg static library from {}. \ + Make sure version v{} exists as a GitHub release.", + url, version + ); + } + + let status = std::process::Command::new("tar") + .args(["-xzf"]) + .arg(&tar_gz_path) + .arg("-C") + .arg(out_dir) + .status() + .expect("Failed to run tar"); + + if !status.success() { + panic!("Failed to extract barretenberg static library"); + } + + // Clean up tar.gz + std::fs::remove_file(&tar_gz_path).ok(); +} diff --git a/barretenberg/rust/bootstrap.sh b/barretenberg/rust/bootstrap.sh index 4d11c656c172..2b8d2c351576 100755 --- a/barretenberg/rust/bootstrap.sh +++ b/barretenberg/rust/bootstrap.sh @@ -38,7 +38,62 @@ function test { denoise "cargo test --release" # Run FFI backend tests (requires libbb-external.a from cpp build) - RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" denoise "cargo test --release --features ffi" + # BB_LIB_DIR tells build.rs to use local lib instead of downloading + BB_LIB_DIR="../cpp/build/lib" RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" denoise "cargo test --release --features ffi" +} + +function test_download { + echo_header "barretenberg-rs download test" + + # Ensure Cargo is in PATH + if [ -f "$HOME/.cargo/env" ]; then + source "$HOME/.cargo/env" + fi + + # Test that build.rs can download pre-built libraries from GitHub releases + # Hide the local library to force download path + local lib_path="../cpp/build/lib/libbb-external.a" + if [ -f "$lib_path" ]; then + mv "$lib_path" "$lib_path.bak" + trap "mv '$lib_path.bak' '$lib_path' 2>/dev/null" EXIT + fi + + # Clean cargo cache to force rebuild + cargo clean -p barretenberg-rs 2>/dev/null || true + + # Build with a known release version + local version=${BARRETENBERG_VERSION:-$(gh release list --repo AztecProtocol/aztec-packages --limit 1 --json tagName --jq '.[0].tagName' | sed 's/^v//')} + echo "Testing download with version: $version" + + # Retry logic for network flakiness (GitHub releases can be flaky) + local max_retries=3 + local retry=0 + local success=false + while [ $retry -lt $max_retries ]; do + if BARRETENBERG_VERSION=$version RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" \ + cargo test --release --features ffi -p barretenberg-rs 2>&1; then + success=true + break + fi + retry=$((retry + 1)) + if [ $retry -lt $max_retries ]; then + echo "Attempt $retry failed, retrying in 5 seconds..." + sleep 5 + # Clean to force re-download + cargo clean -p barretenberg-rs 2>/dev/null || true + fi + done + + if [ "$success" = false ]; then + echo "Download test failed after $max_retries attempts" + exit 1 + fi + + # Restore the local library (trap handles this, but be explicit) + if [ -f "$lib_path.bak" ]; then + mv "$lib_path.bak" "$lib_path" + trap - EXIT + fi } case "$cmd" in @@ -58,7 +113,7 @@ case "$cmd" in bench|bench_cmds) # Empty handling just to make this command valid. ;; - test|test_cmds) + test|test_cmds|test_download) $cmd ;; *) From 971e41bc033f8d3f9e95969f2583881bb3b4b750 Mon Sep 17 00:00:00 2001 From: Jonathan Hao Date: Tue, 3 Feb 2026 11:38:22 +0000 Subject: [PATCH 4/8] fix: set BB_LIB_DIR in build function (ffi feature is on by default) The ffi feature is enabled by default in Cargo.toml, so the build function needs BB_LIB_DIR to point to the local cpp library. Without this, build.rs tries to download from GitHub releases which fails for development versions. --- barretenberg/rust/bootstrap.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/barretenberg/rust/bootstrap.sh b/barretenberg/rust/bootstrap.sh index 2b8d2c351576..7f524878acdf 100755 --- a/barretenberg/rust/bootstrap.sh +++ b/barretenberg/rust/bootstrap.sh @@ -13,7 +13,8 @@ function build { (cd ../ts && yarn generate) # Build all targets - denoise "cargo build --release" + # BB_LIB_DIR tells build.rs to use local lib instead of downloading (ffi feature is on by default) + BB_LIB_DIR="../cpp/build/lib" denoise "cargo build --release" # Upload build artifacts and generated source files to cache cache_upload barretenberg-rs-$hash.tar.gz target/release barretenberg-rs/src/generated_types.rs barretenberg-rs/src/api.rs From 9ec064ed65dd9626dbb11de4c0d5f0562991b652 Mon Sep 17 00:00:00 2001 From: Jonathan Hao Date: Tue, 3 Feb 2026 11:45:01 +0000 Subject: [PATCH 5/8] Revert barretenberg-rs download functionality This reverts commits: - 971e41bc03 fix: set BB_LIB_DIR in build function - 98d4776316 feat: add download functionality to barretenberg-rs build.rs Will re-land with fixes in a separate PR. --- barretenberg/rust/barretenberg-rs/build.rs | 103 ++------------------- barretenberg/rust/bootstrap.sh | 62 +------------ 2 files changed, 11 insertions(+), 154 deletions(-) diff --git a/barretenberg/rust/barretenberg-rs/build.rs b/barretenberg/rust/barretenberg-rs/build.rs index 4cfd1682fdfd..de21724c68be 100644 --- a/barretenberg/rust/barretenberg-rs/build.rs +++ b/barretenberg/rust/barretenberg-rs/build.rs @@ -1,10 +1,14 @@ -use std::path::PathBuf; - fn main() { - // Only for ffi feature - link libbb-external + // Only for ffi feature - link libbb-external from cpp build #[cfg(feature = "ffi")] { - let lib_dir = get_lib_dir(); + // Find the cpp build lib directory relative to this crate + let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); + let lib_dir = std::path::Path::new(&manifest_dir) + .join("../../cpp/build/lib") + .canonicalize() + .expect("Failed to find cpp/build/lib - run barretenberg/cpp/bootstrap.sh first"); + println!("cargo:rustc-link-search=native={}", lib_dir.display()); // libbb-external.a contains everything needed: barretenberg + env + vm2_stub @@ -12,94 +16,3 @@ fn main() { println!("cargo:rustc-link-lib=dylib=stdc++"); } } - -#[cfg(feature = "ffi")] -fn get_lib_dir() -> PathBuf { - // Check if user provided a custom library path - if let Ok(lib_dir) = std::env::var("BB_LIB_DIR") { - let lib_dir = PathBuf::from(&lib_dir); - if lib_dir.join("libbb-external.a").exists() { - return lib_dir.canonicalize().unwrap(); - } - panic!( - "BB_LIB_DIR is set to {:?} but libbb-external.a not found there. \ - Build barretenberg locally: cd barretenberg/cpp && ./bootstrap.sh", - lib_dir - ); - } - - // Download from GitHub releases - let out_dir = PathBuf::from(std::env::var("OUT_DIR").unwrap()); - let lib_path = out_dir.join("libbb-external.a"); - - if !lib_path.exists() { - download_lib(&out_dir); - } - - out_dir -} - -#[cfg(feature = "ffi")] -fn download_lib(out_dir: &PathBuf) { - let target = std::env::var("TARGET").unwrap(); - let arch = match target.as_str() { - t if t.contains("x86_64") && t.contains("linux") => "amd64-linux", - t if t.contains("aarch64") && t.contains("linux") => "arm64-linux", - _ => panic!( - "Unsupported target for FFI backend: {}. Supported: x86_64-linux, aarch64-linux", - target - ), - }; - - // Use BARRETENBERG_VERSION env var, or fall back to crate version - let version = std::env::var("BARRETENBERG_VERSION") - .unwrap_or_else(|_| env!("CARGO_PKG_VERSION").to_string()); - - // Skip download for test versions (0.0.1) - if version == "0.0.1" { - panic!( - "Cannot download pre-built library for test version 0.0.1. \ - Build barretenberg locally: cd barretenberg/cpp && ./bootstrap.sh" - ); - } - - let url = format!( - "https://github.com/AztecProtocol/aztec-packages/releases/download/v{}/barretenberg-static-{}.tar.gz", - version, arch - ); - - println!("cargo:warning=Downloading barretenberg static library from {}", url); - - // Download and extract - let tar_gz_path = out_dir.join("barretenberg-static.tar.gz"); - - let status = std::process::Command::new("curl") - .args(["-L", "-f", "-o"]) - .arg(&tar_gz_path) - .arg(&url) - .status() - .expect("Failed to run curl"); - - if !status.success() { - panic!( - "Failed to download barretenberg static library from {}. \ - Make sure version v{} exists as a GitHub release.", - url, version - ); - } - - let status = std::process::Command::new("tar") - .args(["-xzf"]) - .arg(&tar_gz_path) - .arg("-C") - .arg(out_dir) - .status() - .expect("Failed to run tar"); - - if !status.success() { - panic!("Failed to extract barretenberg static library"); - } - - // Clean up tar.gz - std::fs::remove_file(&tar_gz_path).ok(); -} diff --git a/barretenberg/rust/bootstrap.sh b/barretenberg/rust/bootstrap.sh index 7f524878acdf..4d11c656c172 100755 --- a/barretenberg/rust/bootstrap.sh +++ b/barretenberg/rust/bootstrap.sh @@ -13,8 +13,7 @@ function build { (cd ../ts && yarn generate) # Build all targets - # BB_LIB_DIR tells build.rs to use local lib instead of downloading (ffi feature is on by default) - BB_LIB_DIR="../cpp/build/lib" denoise "cargo build --release" + denoise "cargo build --release" # Upload build artifacts and generated source files to cache cache_upload barretenberg-rs-$hash.tar.gz target/release barretenberg-rs/src/generated_types.rs barretenberg-rs/src/api.rs @@ -39,62 +38,7 @@ function test { denoise "cargo test --release" # Run FFI backend tests (requires libbb-external.a from cpp build) - # BB_LIB_DIR tells build.rs to use local lib instead of downloading - BB_LIB_DIR="../cpp/build/lib" RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" denoise "cargo test --release --features ffi" -} - -function test_download { - echo_header "barretenberg-rs download test" - - # Ensure Cargo is in PATH - if [ -f "$HOME/.cargo/env" ]; then - source "$HOME/.cargo/env" - fi - - # Test that build.rs can download pre-built libraries from GitHub releases - # Hide the local library to force download path - local lib_path="../cpp/build/lib/libbb-external.a" - if [ -f "$lib_path" ]; then - mv "$lib_path" "$lib_path.bak" - trap "mv '$lib_path.bak' '$lib_path' 2>/dev/null" EXIT - fi - - # Clean cargo cache to force rebuild - cargo clean -p barretenberg-rs 2>/dev/null || true - - # Build with a known release version - local version=${BARRETENBERG_VERSION:-$(gh release list --repo AztecProtocol/aztec-packages --limit 1 --json tagName --jq '.[0].tagName' | sed 's/^v//')} - echo "Testing download with version: $version" - - # Retry logic for network flakiness (GitHub releases can be flaky) - local max_retries=3 - local retry=0 - local success=false - while [ $retry -lt $max_retries ]; do - if BARRETENBERG_VERSION=$version RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" \ - cargo test --release --features ffi -p barretenberg-rs 2>&1; then - success=true - break - fi - retry=$((retry + 1)) - if [ $retry -lt $max_retries ]; then - echo "Attempt $retry failed, retrying in 5 seconds..." - sleep 5 - # Clean to force re-download - cargo clean -p barretenberg-rs 2>/dev/null || true - fi - done - - if [ "$success" = false ]; then - echo "Download test failed after $max_retries attempts" - exit 1 - fi - - # Restore the local library (trap handles this, but be explicit) - if [ -f "$lib_path.bak" ]; then - mv "$lib_path.bak" "$lib_path" - trap - EXIT - fi + RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" denoise "cargo test --release --features ffi" } case "$cmd" in @@ -114,7 +58,7 @@ case "$cmd" in bench|bench_cmds) # Empty handling just to make this command valid. ;; - test|test_cmds|test_download) + test|test_cmds) $cmd ;; *) From 383e7085a90a28e51b4a9b7153fc1021b0d6d73d Mon Sep 17 00:00:00 2001 From: Jonathan Hao Date: Tue, 3 Feb 2026 12:31:57 +0000 Subject: [PATCH 6/8] feat: re-land barretenberg-rs download functionality with fixes (#20128) ## Summary Re-lands the download functionality from #20105 with the following fixes: - Check for all 0.x.x development versions, not just 0.0.1 - Set BB_LIB_DIR in build function (ffi feature is on by default) The build.rs now properly detects development versions and requires either BARRETENBERG_VERSION to be set to a released version, or BB_LIB_DIR to point to a local build. ## Test plan - CI will verify the build and tests pass --- barretenberg/rust/barretenberg-rs/build.rs | 106 +++++++++++++++++++-- barretenberg/rust/bootstrap.sh | 64 ++++++++++++- 2 files changed, 159 insertions(+), 11 deletions(-) diff --git a/barretenberg/rust/barretenberg-rs/build.rs b/barretenberg/rust/barretenberg-rs/build.rs index de21724c68be..fdedb61d7977 100644 --- a/barretenberg/rust/barretenberg-rs/build.rs +++ b/barretenberg/rust/barretenberg-rs/build.rs @@ -1,14 +1,10 @@ +use std::path::PathBuf; + fn main() { - // Only for ffi feature - link libbb-external from cpp build + // Only for ffi feature - link libbb-external #[cfg(feature = "ffi")] { - // Find the cpp build lib directory relative to this crate - let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); - let lib_dir = std::path::Path::new(&manifest_dir) - .join("../../cpp/build/lib") - .canonicalize() - .expect("Failed to find cpp/build/lib - run barretenberg/cpp/bootstrap.sh first"); - + let lib_dir = get_lib_dir(); println!("cargo:rustc-link-search=native={}", lib_dir.display()); // libbb-external.a contains everything needed: barretenberg + env + vm2_stub @@ -16,3 +12,97 @@ fn main() { println!("cargo:rustc-link-lib=dylib=stdc++"); } } + +#[cfg(feature = "ffi")] +fn get_lib_dir() -> PathBuf { + // Check if user provided a custom library path + if let Ok(lib_dir) = std::env::var("BB_LIB_DIR") { + let lib_dir = PathBuf::from(&lib_dir); + if lib_dir.join("libbb-external.a").exists() { + return lib_dir.canonicalize().unwrap(); + } + panic!( + "BB_LIB_DIR is set to {:?} but libbb-external.a not found there. \ + Build barretenberg locally: cd barretenberg/cpp && ./bootstrap.sh", + lib_dir + ); + } + + // Download from GitHub releases + let out_dir = PathBuf::from(std::env::var("OUT_DIR").unwrap()); + let lib_path = out_dir.join("libbb-external.a"); + + if !lib_path.exists() { + download_lib(&out_dir); + } + + out_dir +} + +#[cfg(feature = "ffi")] +fn download_lib(out_dir: &PathBuf) { + let target = std::env::var("TARGET").unwrap(); + let arch = match target.as_str() { + t if t.contains("x86_64") && t.contains("linux") => "amd64-linux", + t if t.contains("aarch64") && t.contains("linux") => "arm64-linux", + _ => panic!( + "Unsupported target for FFI backend: {}. Supported: x86_64-linux, aarch64-linux", + target + ), + }; + + // Use BARRETENBERG_VERSION env var, or fall back to crate version + let version = std::env::var("BARRETENBERG_VERSION") + .unwrap_or_else(|_| env!("CARGO_PKG_VERSION").to_string()); + + // Skip download for development versions (0.x.x without BARRETENBERG_VERSION override) + // Real releases use the aztec-packages version (e.g., 4.0.0) set via BARRETENBERG_VERSION + if version.starts_with("0.") && std::env::var("BARRETENBERG_VERSION").is_err() { + panic!( + "Cannot download pre-built library for development version {}. \ + Either set BARRETENBERG_VERSION to a released version, or \ + set BB_LIB_DIR to point to a local build: cd barretenberg/cpp && ./bootstrap.sh", + version + ); + } + + let url = format!( + "https://github.com/AztecProtocol/aztec-packages/releases/download/v{}/barretenberg-static-{}.tar.gz", + version, arch + ); + + println!("cargo:warning=Downloading barretenberg static library from {}", url); + + // Download and extract + let tar_gz_path = out_dir.join("barretenberg-static.tar.gz"); + + let status = std::process::Command::new("curl") + .args(["-L", "-f", "-o"]) + .arg(&tar_gz_path) + .arg(&url) + .status() + .expect("Failed to run curl"); + + if !status.success() { + panic!( + "Failed to download barretenberg static library from {}. \ + Make sure version v{} exists as a GitHub release.", + url, version + ); + } + + let status = std::process::Command::new("tar") + .args(["-xzf"]) + .arg(&tar_gz_path) + .arg("-C") + .arg(out_dir) + .status() + .expect("Failed to run tar"); + + if !status.success() { + panic!("Failed to extract barretenberg static library"); + } + + // Clean up tar.gz + std::fs::remove_file(&tar_gz_path).ok(); +} diff --git a/barretenberg/rust/bootstrap.sh b/barretenberg/rust/bootstrap.sh index 4d11c656c172..7b632ad0a8f6 100755 --- a/barretenberg/rust/bootstrap.sh +++ b/barretenberg/rust/bootstrap.sh @@ -13,7 +13,9 @@ function build { (cd ../ts && yarn generate) # Build all targets - denoise "cargo build --release" + # BB_LIB_DIR tells build.rs to use local lib instead of downloading (ffi feature is on by default) + # Must use absolute path since build.rs runs from a different directory + BB_LIB_DIR="$(cd ../cpp/build/lib && pwd)" denoise "cargo build --release" # Upload build artifacts and generated source files to cache cache_upload barretenberg-rs-$hash.tar.gz target/release barretenberg-rs/src/generated_types.rs barretenberg-rs/src/api.rs @@ -38,7 +40,63 @@ function test { denoise "cargo test --release" # Run FFI backend tests (requires libbb-external.a from cpp build) - RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" denoise "cargo test --release --features ffi" + # BB_LIB_DIR tells build.rs to use local lib instead of downloading + # Must use absolute path since build.rs runs from a different directory + BB_LIB_DIR="$(cd ../cpp/build/lib && pwd)" RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" denoise "cargo test --release --features ffi" +} + +function test_download { + echo_header "barretenberg-rs download test" + + # Ensure Cargo is in PATH + if [ -f "$HOME/.cargo/env" ]; then + source "$HOME/.cargo/env" + fi + + # Test that build.rs can download pre-built libraries from GitHub releases + # Hide the local library to force download path + local lib_path="../cpp/build/lib/libbb-external.a" + if [ -f "$lib_path" ]; then + mv "$lib_path" "$lib_path.bak" + trap "mv '$lib_path.bak' '$lib_path' 2>/dev/null" EXIT + fi + + # Clean cargo cache to force rebuild + cargo clean -p barretenberg-rs 2>/dev/null || true + + # Build with a known release version + local version=${BARRETENBERG_VERSION:-$(gh release list --repo AztecProtocol/aztec-packages --limit 1 --json tagName --jq '.[0].tagName' | sed 's/^v//')} + echo "Testing download with version: $version" + + # Retry logic for network flakiness (GitHub releases can be flaky) + local max_retries=3 + local retry=0 + local success=false + while [ $retry -lt $max_retries ]; do + if BARRETENBERG_VERSION=$version RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" \ + cargo test --release --features ffi -p barretenberg-rs 2>&1; then + success=true + break + fi + retry=$((retry + 1)) + if [ $retry -lt $max_retries ]; then + echo "Attempt $retry failed, retrying in 5 seconds..." + sleep 5 + # Clean to force re-download + cargo clean -p barretenberg-rs 2>/dev/null || true + fi + done + + if [ "$success" = false ]; then + echo "Download test failed after $max_retries attempts" + exit 1 + fi + + # Restore the local library (trap handles this, but be explicit) + if [ -f "$lib_path.bak" ]; then + mv "$lib_path.bak" "$lib_path" + trap - EXIT + fi } case "$cmd" in @@ -58,7 +116,7 @@ case "$cmd" in bench|bench_cmds) # Empty handling just to make this command valid. ;; - test|test_cmds) + test|test_cmds|test_download) $cmd ;; *) From 603414d14f717f6af9b63591c08300b1d810bc92 Mon Sep 17 00:00:00 2001 From: Jonathan Hao Date: Tue, 3 Feb 2026 14:23:03 +0000 Subject: [PATCH 7/8] fix: skip FFI feature in PipeBackend tests The ffi feature is enabled by default, so 'cargo test --release' tries to build FFI and fails without BB_LIB_DIR. Use --no-default-features to only run the PipeBackend tests, which spawn the bb binary instead. --- barretenberg/rust/bootstrap.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/barretenberg/rust/bootstrap.sh b/barretenberg/rust/bootstrap.sh index 7b632ad0a8f6..7c5c885eb2e3 100755 --- a/barretenberg/rust/bootstrap.sh +++ b/barretenberg/rust/bootstrap.sh @@ -37,7 +37,8 @@ function test { fi # Run PipeBackend tests (spawns bb binary) - denoise "cargo test --release" + # Use --no-default-features to skip FFI (which requires libbb-external.a) + denoise "cargo test --release --no-default-features --features native" # Run FFI backend tests (requires libbb-external.a from cpp build) # BB_LIB_DIR tells build.rs to use local lib instead of downloading From 8b115bd2d823943982f1cee62df82967db60a428 Mon Sep 17 00:00:00 2001 From: Jonathan Hao Date: Tue, 3 Feb 2026 14:23:03 +0000 Subject: [PATCH 8/8] fix: skip FFI feature in PipeBackend tests The ffi feature is enabled by default, so 'cargo test --release' tries to build FFI and fails without BB_LIB_DIR. Use --no-default-features to only run the PipeBackend tests, which spawn the bb binary instead. --- barretenberg/rust/scripts/run_test.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/barretenberg/rust/scripts/run_test.sh b/barretenberg/rust/scripts/run_test.sh index de7156544bfd..02cf98907bd4 100755 --- a/barretenberg/rust/scripts/run_test.sh +++ b/barretenberg/rust/scripts/run_test.sh @@ -8,5 +8,11 @@ if [ -f "$HOME/.cargo/env" ]; then source "$HOME/.cargo/env" fi -# Run all tests (FFI is enabled by default, links to cpp/build/lib automatically) -denoise "cargo test --release" +# Run PipeBackend tests (spawns bb binary) +# Use --no-default-features to skip FFI (which requires libbb-external.a) +denoise "cargo test --release --no-default-features --features native" + +# Run FFI backend tests (requires libbb-external.a from cpp build) +# BB_LIB_DIR tells build.rs to use local lib instead of downloading +# Must use absolute path since build.rs runs from a different directory +BB_LIB_DIR="$(cd ../cpp/build/lib && pwd)" RUSTFLAGS="-C link-arg=-Wl,--allow-multiple-definition" denoise "cargo test --release --features ffi"