fix(bb): assign unique secp256r1 lookup table indices

(cherry picked from commit c6ab912)

Author: AztecBot

barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder_lookup.test.cpp

@@ -4,6 +4,7 @@
 
 #include <gtest/gtest.h>
 #include <unordered_map>
+#include <unordered_set>
 
 using namespace bb;
 
@@ -121,6 +122,40 @@ TEST_F(UltraCircuitBuilderLookup, DifferentTablesGetUniqueIndices)
     EXPECT_EQ(builder.get_num_lookup_tables(), 3UL);
 }
 
+TEST_F(UltraCircuitBuilderLookup, Secp256r1FixedBaseTablesGetUniquePositiveIndices)
+{
+    Builder builder;
+    std::unordered_set<size_t> table_indices;
+
+    const auto first_id = static_cast<size_t>(plookup::BasicTableId::SECP256R1_FIXED_BASE_XLO_0);
+    const auto end_id = static_cast<size_t>(plookup::BasicTableId::SECP256R1_FIXED_BASE_END);
+    for (size_t id = first_id; id < end_id; ++id) {
+        const auto& table = builder.get_table(static_cast<plookup::BasicTableId>(id));
+        EXPECT_GT(table.table_index, 0UL);
+        EXPECT_TRUE(table_indices.insert(table.table_index).second);
+    }
+
+    EXPECT_NO_THROW(builder.finalize_circuit());
+}
+
+TEST_F(UltraCircuitBuilderLookup, FinalizationRejectsDuplicateTableIndices)
+{
+    Builder builder;
+    builder.get_table(plookup::BasicTableId::UINT_XOR_SLICE_6_ROTATE_0);
+    builder.get_table(plookup::BasicTableId::UINT_AND_SLICE_6_ROTATE_0);
+    builder.get_lookup_tables()[1].table_index = builder.get_lookup_tables()[0].table_index;
+
+    EXPECT_THROW_OR_ABORT(builder.finalize_circuit(), "Lookup table indices must be unique within a circuit");
+}
+
+TEST_F(UltraCircuitBuilderLookup, FinalizationRejectsZeroTableIndex)
+{
+    Builder builder;
+    builder.get_table(plookup::BasicTableId::UINT_XOR_SLICE_6_ROTATE_0).table_index = 0;
+
+    EXPECT_THROW_OR_ABORT(builder.finalize_circuit(), "Lookup table indices must be positive");
+}
+
 // Verifies correct behavior when key_b_index is not provided (2-to-1 lookup without second index)
 TEST_F(UltraCircuitBuilderLookup, NoKeyBIndex)
 {

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/plookup_tables/plookup_tables.cpp

@@ -285,22 +285,22 @@ BasicTable create_basic_table(const BasicTableId id, const size_t index)
     if (id_var >= static_cast<size_t>(SECP256R1_FIXED_BASE_XLO_0) &&
         id_var < static_cast<size_t>(SECP256R1_FIXED_BASE_XHI_0)) {
         return secp256r1_fixed_base::table::generate_basic_table_runtime<secp256r1_fixed_base::table::AXIS_XLO>(
-            id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_XLO_0));
+            id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_XLO_0), index);
     }
     if (id_var >= static_cast<size_t>(SECP256R1_FIXED_BASE_XHI_0) &&
         id_var < static_cast<size_t>(SECP256R1_FIXED_BASE_YLO_0)) {
         return secp256r1_fixed_base::table::generate_basic_table_runtime<secp256r1_fixed_base::table::AXIS_XHI>(
-            id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_XHI_0));
+            id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_XHI_0), index);
     }
     if (id_var >= static_cast<size_t>(SECP256R1_FIXED_BASE_YLO_0) &&
         id_var < static_cast<size_t>(SECP256R1_FIXED_BASE_YHI_0)) {
         return secp256r1_fixed_base::table::generate_basic_table_runtime<secp256r1_fixed_base::table::AXIS_YLO>(
-            id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_YLO_0));
+            id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_YLO_0), index);
     }
     if (id_var >= static_cast<size_t>(SECP256R1_FIXED_BASE_YHI_0) &&
         id_var < static_cast<size_t>(SECP256R1_FIXED_BASE_END)) {
         return secp256r1_fixed_base::table::generate_basic_table_runtime<secp256r1_fixed_base::table::AXIS_YHI>(
-            id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_YHI_0));
+            id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_YHI_0), index);
     }
     switch (id) {
     case AES_SPARSE_MAP: {

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/plookup_tables/secp256r1_fixed_base.cpp

@@ -143,17 +143,18 @@ template <table::AxisIndex axis> generate_fn_ptr generate_fn_for_window(size_t w
 }
 } // namespace
 
-template <table::AxisIndex axis> BasicTable table::generate_basic_table_runtime(BasicTableId id, size_t window_idx)
+template <table::AxisIndex axis>
+BasicTable table::generate_basic_table_runtime(BasicTableId id, size_t window_idx, size_t table_index)
 {
     BB_ASSERT_LT(window_idx, NUM_WINDOWS);
-    return generate_fn_for_window<axis>(window_idx)(id, window_idx);
+    return generate_fn_for_window<axis>(window_idx)(id, table_index);
 }
 
 // Explicit instantiations for the four axes.
-template BasicTable table::generate_basic_table_runtime<table::AXIS_XLO>(BasicTableId, size_t);
-template BasicTable table::generate_basic_table_runtime<table::AXIS_XHI>(BasicTableId, size_t);
-template BasicTable table::generate_basic_table_runtime<table::AXIS_YLO>(BasicTableId, size_t);
-template BasicTable table::generate_basic_table_runtime<table::AXIS_YHI>(BasicTableId, size_t);
+template BasicTable table::generate_basic_table_runtime<table::AXIS_XLO>(BasicTableId, size_t, size_t);
+template BasicTable table::generate_basic_table_runtime<table::AXIS_XHI>(BasicTableId, size_t, size_t);
+template BasicTable table::generate_basic_table_runtime<table::AXIS_YLO>(BasicTableId, size_t, size_t);
+template BasicTable table::generate_basic_table_runtime<table::AXIS_YHI>(BasicTableId, size_t, size_t);
 
 namespace {
 // Returns `&table::get_values<axis, window_idx>` for a runtime (axis, window_idx). The per-axis 32-entry

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/plookup_tables/secp256r1_fixed_base.hpp

@@ -116,10 +116,11 @@ class table : public Secp256r1FixedBaseParams {
     static BasicTable generate_basic_table(BasicTableId id, size_t table_index);
 
     /**
-     * @brief Runtime dispatch helper used by plookup_tables.cpp::create_basic_table. Given an axis and a
-     * runtime window_idx ∈ [0, NUM_WINDOWS), instantiate the corresponding generator.
+     * @brief Runtime dispatch helper used by plookup_tables.cpp::create_basic_table. Selects table contents using
+     * window_idx and assigns the independently supplied circuit-local table_index.
      */
-    template <AxisIndex axis> static BasicTable generate_basic_table_runtime(BasicTableId id, size_t table_index);
+    template <AxisIndex axis>
+    static BasicTable generate_basic_table_runtime(BasicTableId id, size_t window_idx, size_t table_index);
 
     /**
      * @brief Construct one of the 10 MultiTables described in the file-header docstring.

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp

@@ -51,6 +51,13 @@ template <typename ExecutionTrace> void UltraCircuitBuilder_<ExecutionTrace>::fi
      * our circuit is finalized, and we must not to execute these functions again.
      */
     if (!this->circuit_finalized) {
+        std::unordered_set<size_t> table_indices;
+        for (const auto& table : lookup_tables) {
+            BB_ASSERT_GT(table.table_index, 0U, "Lookup table indices must be positive");
+            BB_ASSERT(table_indices.insert(table.table_index).second,
+                      "Lookup table indices must be unique within a circuit");
+        }
+
         process_non_native_field_multiplications();
 #ifndef ULTRA_FUZZ
         this->rom_ram_logic.process_ROM_arrays(this);