Skip to content

Commit f245aed

Browse files
authored
fix(bb): port secp256r1 unique-lookup-index soundness fix to public v5-next (#24842)
Brings public `v5-next` up to `private/v5-next` (`eb52040bd1`), porting the barretenberg secp256r1 fixed-base lookup-table soundness fix. ## Net change (6 files, all under `barretenberg/cpp/`) - Assign a unique `table_index` to each secp256r1 fixed-base lookup table, and add finalize-time guards that reject duplicate or zero table indices, with a new circuit-checker test (`Secp256r1FixedBaseTablesGetUniquePositiveIndices`, `FinalizationRejectsDuplicateTableIndices`, `FinalizationRejectsZeroTableIndex`). - Refresh the pinned Chonk IVC inputs hash to match the circuit change. ## Notes for reviewers - The commit set also includes the routine `chore: sync public-v5-next with upstream v5-next` merge nodes that sit between the two tips; those carry no net file change. The only net delta is the fix + the chonk hash bump. - Must merge via a **merge commit** (branch ruleset allows `merge` only). The public→private sync will reconcile `v5-next` afterward, so private stays a fast-forward of public.
2 parents b89c48b + eb52040 commit f245aed

6 files changed

Lines changed: 65 additions & 14 deletions

File tree

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
30325f8d3e30771a
1+
f19b76f7fa0ff0cd

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include <gtest/gtest.h>
66
#include <unordered_map>
7+
#include <unordered_set>
78

89
using namespace bb;
910

@@ -121,6 +122,47 @@ TEST_F(UltraCircuitBuilderLookup, DifferentTablesGetUniqueIndices)
121122
EXPECT_EQ(builder.get_num_lookup_tables(), 3UL);
122123
}
123124

125+
// Every basic table reachable through any MultiTable must receive a unique, positive circuit-local
126+
// table_index. The LogDeriv lookup relation identifies a table solely by table_index (not BasicTableId),
127+
// so a generator that stores anything other than the builder-assigned index silently collapses distinct tables to one
128+
// identity. This sweeps the whole table space so any such generator is caught.
129+
TEST_F(UltraCircuitBuilderLookup, AllMultiTableBasicTablesGetUniquePositiveIndices)
130+
{
131+
Builder builder;
132+
for (size_t mt = 0; mt < static_cast<size_t>(plookup::MultiTableId::NUM_MULTI_TABLES); ++mt) {
133+
const auto& multitable = plookup::get_multitable(static_cast<plookup::MultiTableId>(mt));
134+
for (const auto id : multitable.basic_table_ids) {
135+
builder.get_table(id);
136+
}
137+
}
138+
139+
std::unordered_set<size_t> seen;
140+
for (const auto& table : builder.get_lookup_tables()) {
141+
EXPECT_GT(table.table_index, 0UL) << "non-positive index for basic table id " << static_cast<size_t>(table.id);
142+
EXPECT_TRUE(seen.insert(table.table_index).second)
143+
<< "duplicate table_index " << table.table_index << " for basic table id " << static_cast<size_t>(table.id);
144+
}
145+
EXPECT_NO_THROW(builder.finalize_circuit());
146+
}
147+
148+
TEST_F(UltraCircuitBuilderLookup, FinalizationRejectsDuplicateTableIndices)
149+
{
150+
Builder builder;
151+
builder.get_table(plookup::BasicTableId::UINT_XOR_SLICE_6_ROTATE_0);
152+
builder.get_table(plookup::BasicTableId::UINT_AND_SLICE_6_ROTATE_0);
153+
builder.get_lookup_tables()[1].table_index = builder.get_lookup_tables()[0].table_index;
154+
155+
EXPECT_THROW_OR_ABORT(builder.finalize_circuit(), "Lookup table indices must be unique within a circuit");
156+
}
157+
158+
TEST_F(UltraCircuitBuilderLookup, FinalizationRejectsZeroTableIndex)
159+
{
160+
Builder builder;
161+
builder.get_table(plookup::BasicTableId::UINT_XOR_SLICE_6_ROTATE_0).table_index = 0;
162+
163+
EXPECT_THROW_OR_ABORT(builder.finalize_circuit(), "Lookup table indices must be positive");
164+
}
165+
124166
// Verifies correct behavior when key_b_index is not provided (2-to-1 lookup without second index)
125167
TEST_F(UltraCircuitBuilderLookup, NoKeyBIndex)
126168
{

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,22 +285,22 @@ BasicTable create_basic_table(const BasicTableId id, const size_t index)
285285
if (id_var >= static_cast<size_t>(SECP256R1_FIXED_BASE_XLO_0) &&
286286
id_var < static_cast<size_t>(SECP256R1_FIXED_BASE_XHI_0)) {
287287
return secp256r1_fixed_base::table::generate_basic_table_runtime<secp256r1_fixed_base::table::AXIS_XLO>(
288-
id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_XLO_0));
288+
id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_XLO_0), index);
289289
}
290290
if (id_var >= static_cast<size_t>(SECP256R1_FIXED_BASE_XHI_0) &&
291291
id_var < static_cast<size_t>(SECP256R1_FIXED_BASE_YLO_0)) {
292292
return secp256r1_fixed_base::table::generate_basic_table_runtime<secp256r1_fixed_base::table::AXIS_XHI>(
293-
id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_XHI_0));
293+
id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_XHI_0), index);
294294
}
295295
if (id_var >= static_cast<size_t>(SECP256R1_FIXED_BASE_YLO_0) &&
296296
id_var < static_cast<size_t>(SECP256R1_FIXED_BASE_YHI_0)) {
297297
return secp256r1_fixed_base::table::generate_basic_table_runtime<secp256r1_fixed_base::table::AXIS_YLO>(
298-
id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_YLO_0));
298+
id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_YLO_0), index);
299299
}
300300
if (id_var >= static_cast<size_t>(SECP256R1_FIXED_BASE_YHI_0) &&
301301
id_var < static_cast<size_t>(SECP256R1_FIXED_BASE_END)) {
302302
return secp256r1_fixed_base::table::generate_basic_table_runtime<secp256r1_fixed_base::table::AXIS_YHI>(
303-
id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_YHI_0));
303+
id, id_var - static_cast<size_t>(SECP256R1_FIXED_BASE_YHI_0), index);
304304
}
305305
switch (id) {
306306
case AES_SPARSE_MAP: {

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,17 +143,18 @@ template <table::AxisIndex axis> generate_fn_ptr generate_fn_for_window(size_t w
143143
}
144144
} // namespace
145145

146-
template <table::AxisIndex axis> BasicTable table::generate_basic_table_runtime(BasicTableId id, size_t window_idx)
146+
template <table::AxisIndex axis>
147+
BasicTable table::generate_basic_table_runtime(BasicTableId id, size_t window_idx, size_t table_index)
147148
{
148149
BB_ASSERT_LT(window_idx, NUM_WINDOWS);
149-
return generate_fn_for_window<axis>(window_idx)(id, window_idx);
150+
return generate_fn_for_window<axis>(window_idx)(id, table_index);
150151
}
151152

152153
// Explicit instantiations for the four axes.
153-
template BasicTable table::generate_basic_table_runtime<table::AXIS_XLO>(BasicTableId, size_t);
154-
template BasicTable table::generate_basic_table_runtime<table::AXIS_XHI>(BasicTableId, size_t);
155-
template BasicTable table::generate_basic_table_runtime<table::AXIS_YLO>(BasicTableId, size_t);
156-
template BasicTable table::generate_basic_table_runtime<table::AXIS_YHI>(BasicTableId, size_t);
154+
template BasicTable table::generate_basic_table_runtime<table::AXIS_XLO>(BasicTableId, size_t, size_t);
155+
template BasicTable table::generate_basic_table_runtime<table::AXIS_XHI>(BasicTableId, size_t, size_t);
156+
template BasicTable table::generate_basic_table_runtime<table::AXIS_YLO>(BasicTableId, size_t, size_t);
157+
template BasicTable table::generate_basic_table_runtime<table::AXIS_YHI>(BasicTableId, size_t, size_t);
157158

158159
namespace {
159160
// 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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,11 @@ class table : public Secp256r1FixedBaseParams {
116116
static BasicTable generate_basic_table(BasicTableId id, size_t table_index);
117117

118118
/**
119-
* @brief Runtime dispatch helper used by plookup_tables.cpp::create_basic_table. Given an axis and a
120-
* runtime window_idx ∈ [0, NUM_WINDOWS), instantiate the corresponding generator.
119+
* @brief Runtime dispatch helper used by plookup_tables.cpp::create_basic_table. Selects table contents using
120+
* window_idx and assigns the independently supplied circuit-local table_index.
121121
*/
122-
template <AxisIndex axis> static BasicTable generate_basic_table_runtime(BasicTableId id, size_t table_index);
122+
template <AxisIndex axis>
123+
static BasicTable generate_basic_table_runtime(BasicTableId id, size_t window_idx, size_t table_index);
123124

124125
/**
125126
* @brief Construct one of the 10 MultiTables described in the file-header docstring.

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ template <typename ExecutionTrace> void UltraCircuitBuilder_<ExecutionTrace>::fi
5151
* our circuit is finalized, and we must not to execute these functions again.
5252
*/
5353
if (!this->circuit_finalized) {
54+
std::unordered_set<size_t> table_indices;
55+
for (const auto& table : lookup_tables) {
56+
BB_ASSERT_GT(table.table_index, 0U, "Lookup table indices must be positive");
57+
BB_ASSERT(table_indices.insert(table.table_index).second,
58+
"Lookup table indices must be unique within a circuit");
59+
}
60+
5461
process_non_native_field_multiplications();
5562
#ifndef ULTRA_FUZZ
5663
this->rom_ram_logic.process_ROM_arrays(this);

0 commit comments

Comments
 (0)