From 7e33fd3df24e1dc2722c122177b0407749ce8f88 Mon Sep 17 00:00:00 2001 From: Khashayar Barooti Date: Tue, 2 Sep 2025 10:34:18 +0000 Subject: [PATCH 1/8] small refactor --- .../barretenberg/dsl/acir_format/block_constraint.cpp | 10 +++------- .../barretenberg/stdlib/primitives/databus/databus.cpp | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index fc069279eafc..83d47611256a 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -179,14 +179,10 @@ void process_call_data_operations(Builder& builder, BB_ASSERT_EQ(op.access_type, 0); field_ct value = poly_to_field_ct(op.value, builder); field_ct index = poly_to_field_ct(op.index, builder); - fr w_value = 0; - if (has_valid_witness_assignments) { - // If witness are assigned, we use the correct value for w - w_value = index.get_value(); + value.assert_equal(calldata_array[index]); + if (!has_valid_witness_assignments) { + index.is_zero(); } - field_ct w = field_ct::from_witness(&builder, w_value); - value.assert_equal(calldata_array[w]); - w.assert_equal(index); } }; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.cpp index 1d03dcbdaab9..0dfb2601a39e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.cpp @@ -53,7 +53,7 @@ field_t databus::bus_vector::operator[](const field_pt& index) if (index.is_constant()) { index_witness_idx = context->put_constant_variable(index.get_value()); } else { - index_witness_idx = index.normalize().get_witness_index(); + index_witness_idx = index.get_normalized_witness_index(); } // Read from the bus vector at the specified index. Creates a single read gate From d6ac2ddf762e3223240e7e1fd74422775068d5ef Mon Sep 17 00:00:00 2001 From: Khashayar Barooti Date: Tue, 2 Sep 2025 16:54:52 +0000 Subject: [PATCH 2/8] changed to use set_variable --- .../dsl/acir_format/block_constraint.cpp | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index 83d47611256a..04b29674c6e6 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -118,16 +118,12 @@ void process_ROM_operations(Builder& builder, // For a ROM table, constant read should be optimized out: // The rom_table won't work with a constant read because the table may not be initialized ASSERT(op.index.q_l != 0); - // We create a new witness w to avoid issues with non-valid witness assignements: - // if witness are not assigned, then w will be zero and table[w] will work - fr w_value = 0; - if (has_valid_witness_assignments) { - // If witness are assigned, we use the correct value for w - w_value = index.get_value(); + + if (!has_valid_witness_assignments) { + // If witness are not assigned, we set the index value to 0 + builder.set_variable(index.witness_index, 0); } - field_ct w = field_ct::from_witness(&builder, w_value); - value.assert_equal(table[w]); - w.assert_equal(index); + value.assert_equal(table[index]); } } @@ -145,11 +141,10 @@ void process_RAM_operations(Builder& builder, field_ct value = poly_to_field_ct(op.value, builder); field_ct index = poly_to_field_ct(op.index, builder); - // We create a new witness w to avoid issues with non-valid witness assignements. - // If witness are not assigned, then index will be zero and table[index] won't hit bounds check. - fr index_value = has_valid_witness_assignments ? index.get_value() : 0; - // Create new witness and ensure equal to index. - field_ct::from_witness(&builder, index_value).assert_equal(index); + if (!has_valid_witness_assignments) { + // If witness are not assigned, we set the index value to 0 + builder.set_variable(index.witness_index, 0); + } if (op.access_type == 0) { value.assert_equal(table.read(index)); @@ -179,10 +174,11 @@ void process_call_data_operations(Builder& builder, BB_ASSERT_EQ(op.access_type, 0); field_ct value = poly_to_field_ct(op.value, builder); field_ct index = poly_to_field_ct(op.index, builder); - value.assert_equal(calldata_array[index]); if (!has_valid_witness_assignments) { - index.is_zero(); + // If witness are not assigned, we set the index value to 0 + builder.set_variable(index.witness_index, 0); } + value.assert_equal(calldata_array[index]); } }; From e8662d028316595c11d98b4b7a39f5df676e969f Mon Sep 17 00:00:00 2001 From: Khashayar Barooti Date: Wed, 3 Sep 2025 10:22:32 +0000 Subject: [PATCH 3/8] added tests for constants and unnormalized indexes --- .../primitives/databus/databus.test.cpp | 58 ++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp index a878728e49ec..deb0420d3bcd 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp @@ -78,6 +78,62 @@ TEST(Databus, CallDataAndReturnData) EXPECT_TRUE(CircuitChecker::check(builder)); } +TEST(Databus, ConstantEntryAccess) +{ + + Builder builder; + databus_ct databus; + fr value_0 = 13; + fr value_1 = 12; + auto constant_0 = witness_ct::create_constant_witness(&builder, value_0); + auto constant_1 = witness_ct::create_constant_witness(&builder, value_1); + databus.return_data.set_values({ constant_0, constant_1 }); + field_ct idx_0(witness_ct(&builder, 0)); + field_ct idx_1(witness_ct(&builder, 1)); + + field_ct read_result_0 = databus.return_data[idx_0]; + field_ct read_result_1 = databus.return_data[idx_1]; + + EXPECT_EQ(value_0, read_result_0.get_value()); + EXPECT_EQ(value_1, read_result_1.get_value()); + EXPECT_TRUE(CircuitChecker::check(builder)); +} + +TEST(Databus, ConstantAndUnnormalizedIndex) +{ + Builder builder; + databus_ct databus; + std::array raw_calldata_values = { 54, 32, 30 }; + std::array raw_returndata_values = { 54, 32, 116 }; + // make the two first elements constants and the second two elements non-normalized + // Populate the calldata in the databus + std::vector calldata_values; + for (auto& value : raw_calldata_values) { + calldata_values.emplace_back(witness_ct(&builder, value)); + } + databus.calldata.set_values(calldata_values); + + // Populate the return data in the databus std::vector return_data_values; + std::vector returndata_values; + for (auto& value : raw_returndata_values) { + returndata_values.emplace_back(witness_ct(&builder, value)); + } + databus.return_data.set_values(returndata_values); + + // constant first index + field_ct idx_0(witness_ct::create_constant_witness(&builder, 0)); + field_ct idx_1(witness_ct(&builder, 1)); + // un-normalized index (with multiplicative constant 2) + field_ct idx_2 = idx_1 + idx_1; + field_ct sum = databus.calldata[idx_0] + databus.calldata[idx_1] + databus.calldata[idx_2]; + + databus.return_data[0].assert_equal(databus.calldata[0]); + databus.return_data[1].assert_equal(databus.calldata[1]); + databus.return_data[2].assert_equal(sum); + + EXPECT_TRUE(CircuitChecker::check(builder)); +} + /** * @brief A failure test demonstrating that trying to prove (via a databus read) that an erroneous value is present in * the databus will result in an invalid witness. @@ -179,4 +235,4 @@ TEST(Databus, DuplicateRead) databus.return_data[idx_1]; EXPECT_TRUE(CircuitChecker::check(builder)); -} \ No newline at end of file +} From 5f47ccc94cacc8be431e82537d68b3c5e4a5b6e6 Mon Sep 17 00:00:00 2001 From: Khashayar Barooti Date: Wed, 3 Sep 2025 10:28:27 +0000 Subject: [PATCH 4/8] typo --- .../src/barretenberg/stdlib/primitives/databus/databus.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp index deb0420d3bcd..fe05e7946ae4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp @@ -99,7 +99,7 @@ TEST(Databus, ConstantEntryAccess) EXPECT_TRUE(CircuitChecker::check(builder)); } -TEST(Databus, ConstantAndUnnormalizedIndex) +TEST(Databus, ConstantAndUnnormalizedIndices) { Builder builder; databus_ct databus; From 5c79d7aec5135dabc73c1a837e56a5b3962eb28b Mon Sep 17 00:00:00 2001 From: Khashayar Barooti Date: Wed, 3 Sep 2025 10:50:54 +0000 Subject: [PATCH 5/8] fixed indices in tests --- .../primitives/databus/databus.test.cpp | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp index fe05e7946ae4..34dc760fffad 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp @@ -99,6 +99,34 @@ TEST(Databus, ConstantEntryAccess) EXPECT_TRUE(CircuitChecker::check(builder)); } +TEST(Databus, UnnormalizedEntryAccess) +{ + + Builder builder; + databus_ct databus; + std::array raw_calldata_entries = { 3, 2, 1 }; + std::array raw_returndata_entries = { 6, 4, 2 }; + std::vector calldata_entries; + for (fr entry : raw_calldata_entries) { + field_ct entry_witness = witness_ct(&builder, entry); + // add the value to itself to make it unnormalized + calldata_entries.emplace_back(entry_witness + entry_witness); + } + std::vector returndata_entries; + for (fr entry : raw_returndata_entries) { + returndata_entries.emplace_back(witness_ct(&builder, entry)); + } + databus.calldata.set_values(calldata_entries); + databus.return_data.set_values(returndata_entries); + field_ct idx_0 = witness_ct(&builder, 0); + field_ct idx_1 = witness_ct(&builder, 1); + field_ct idx_2 = witness_ct(&builder, 2); + databus.return_data[idx_0].assert_equal(databus.calldata[idx_0]); + databus.return_data[idx_1].assert_equal(databus.calldata[idx_1]); + databus.return_data[idx_2].assert_equal(databus.calldata[idx_2]); + EXPECT_TRUE(CircuitChecker::check(builder)); +} + TEST(Databus, ConstantAndUnnormalizedIndices) { Builder builder; @@ -127,9 +155,9 @@ TEST(Databus, ConstantAndUnnormalizedIndices) field_ct idx_2 = idx_1 + idx_1; field_ct sum = databus.calldata[idx_0] + databus.calldata[idx_1] + databus.calldata[idx_2]; - databus.return_data[0].assert_equal(databus.calldata[0]); - databus.return_data[1].assert_equal(databus.calldata[1]); - databus.return_data[2].assert_equal(sum); + databus.return_data[idx_0].assert_equal(databus.calldata[idx_0]); + databus.return_data[idx_1].assert_equal(databus.calldata[idx_1]); + databus.return_data[idx_2].assert_equal(sum); EXPECT_TRUE(CircuitChecker::check(builder)); } From e9d0d5a9fb3b31f3a9011459461f05225fd52a59 Mon Sep 17 00:00:00 2001 From: Khashayar Barooti Date: Wed, 3 Sep 2025 10:53:44 +0000 Subject: [PATCH 6/8] small test change --- .../stdlib/primitives/databus/databus.test.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp index 34dc760fffad..61c456361341 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp @@ -105,25 +105,26 @@ TEST(Databus, UnnormalizedEntryAccess) Builder builder; databus_ct databus; std::array raw_calldata_entries = { 3, 2, 1 }; - std::array raw_returndata_entries = { 6, 4, 2 }; + std::array raw_returndata_entries = { 3, 2, 1 }; std::vector calldata_entries; for (fr entry : raw_calldata_entries) { + calldata_entries.emplace_back(witness_ct(&builder, entry)); field_ct entry_witness = witness_ct(&builder, entry); - // add the value to itself to make it unnormalized - calldata_entries.emplace_back(entry_witness + entry_witness); } std::vector returndata_entries; for (fr entry : raw_returndata_entries) { - returndata_entries.emplace_back(witness_ct(&builder, entry)); + field_ct entry_witness = witness_ct(&builder, entry); + // add the value to itself to make it unnormalized + returndata_entries.emplace_back(entry_witness + entry_witness); } databus.calldata.set_values(calldata_entries); databus.return_data.set_values(returndata_entries); field_ct idx_0 = witness_ct(&builder, 0); field_ct idx_1 = witness_ct(&builder, 1); field_ct idx_2 = witness_ct(&builder, 2); - databus.return_data[idx_0].assert_equal(databus.calldata[idx_0]); - databus.return_data[idx_1].assert_equal(databus.calldata[idx_1]); - databus.return_data[idx_2].assert_equal(databus.calldata[idx_2]); + databus.return_data[idx_0].assert_equal(databus.calldata[idx_0] + databus.calldata[idx_0]); + databus.return_data[idx_1].assert_equal(databus.calldata[idx_1] + databus.calldata[idx_1]); + databus.return_data[idx_2].assert_equal(databus.calldata[idx_2] + databus.calldata[idx_2]); EXPECT_TRUE(CircuitChecker::check(builder)); } From c8a2339c8446d1506adaec106245ac08b9e94c5b Mon Sep 17 00:00:00 2001 From: Khashayar Barooti Date: Wed, 3 Sep 2025 13:25:42 +0000 Subject: [PATCH 7/8] comments updated --- .../stdlib/primitives/databus/databus.test.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp index 61c456361341..76b222aaa5bf 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp @@ -78,6 +78,10 @@ TEST(Databus, CallDataAndReturnData) EXPECT_TRUE(CircuitChecker::check(builder)); } +/** + * @brief An expository test demonstrating the functionality of the databus in a small use case when the entries are + * constant witnesses + */ TEST(Databus, ConstantEntryAccess) { @@ -99,6 +103,10 @@ TEST(Databus, ConstantEntryAccess) EXPECT_TRUE(CircuitChecker::check(builder)); } +/** + * @brief An expository test demonstrating the functionality of the databus in a small use case when the entries of the + * bus_vector are not normalized + */ TEST(Databus, UnnormalizedEntryAccess) { @@ -114,7 +122,7 @@ TEST(Databus, UnnormalizedEntryAccess) std::vector returndata_entries; for (fr entry : raw_returndata_entries) { field_ct entry_witness = witness_ct(&builder, entry); - // add the value to itself to make it unnormalized + // add the value to itself to make it unnormalized (the multiplicative constant will be 2) returndata_entries.emplace_back(entry_witness + entry_witness); } databus.calldata.set_values(calldata_entries); @@ -128,13 +136,16 @@ TEST(Databus, UnnormalizedEntryAccess) EXPECT_TRUE(CircuitChecker::check(builder)); } +/** + * @brief An expository test demonstrating the functionality of the databus in a small use case where the indices are + * constant and/or unnormalized + */ TEST(Databus, ConstantAndUnnormalizedIndices) { Builder builder; databus_ct databus; std::array raw_calldata_values = { 54, 32, 30 }; std::array raw_returndata_values = { 54, 32, 116 }; - // make the two first elements constants and the second two elements non-normalized // Populate the calldata in the databus std::vector calldata_values; for (auto& value : raw_calldata_values) { @@ -142,7 +153,7 @@ TEST(Databus, ConstantAndUnnormalizedIndices) } databus.calldata.set_values(calldata_values); - // Populate the return data in the databus std::vector return_data_values; + // Populate the return data in the databus std::vector returndata_values; for (auto& value : raw_returndata_values) { returndata_values.emplace_back(witness_ct(&builder, value)); From 5de83a566bde7a2f4f517bf89a98a55fddfe8b1c Mon Sep 17 00:00:00 2001 From: Khashayar Barooti Date: Fri, 5 Sep 2025 09:54:11 +0000 Subject: [PATCH 8/8] better comments --- .../barretenberg/dsl/acir_format/block_constraint.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index 04b29674c6e6..05d45d6d21bb 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -119,8 +119,9 @@ void process_ROM_operations(Builder& builder, // The rom_table won't work with a constant read because the table may not be initialized ASSERT(op.index.q_l != 0); + // In case of invalid witness assignment, we set the value of index value to zero to not hit out of bound in + // ROM table if (!has_valid_witness_assignments) { - // If witness are not assigned, we set the index value to 0 builder.set_variable(index.witness_index, 0); } value.assert_equal(table[index]); @@ -140,9 +141,9 @@ void process_RAM_operations(Builder& builder, for (auto& op : constraint.trace) { field_ct value = poly_to_field_ct(op.value, builder); field_ct index = poly_to_field_ct(op.index, builder); - + // In case of invalid witness assignment, we set the value of index value to zero to not hit out of bound in + // RAM table if (!has_valid_witness_assignments) { - // If witness are not assigned, we set the index value to 0 builder.set_variable(index.witness_index, 0); } @@ -174,8 +175,9 @@ void process_call_data_operations(Builder& builder, BB_ASSERT_EQ(op.access_type, 0); field_ct value = poly_to_field_ct(op.value, builder); field_ct index = poly_to_field_ct(op.index, builder); + // In case of invalid witness assignment, we set the value of index value to zero to not hit out of bound in + // calldata-array if (!has_valid_witness_assignments) { - // If witness are not assigned, we set the index value to 0 builder.set_variable(index.witness_index, 0); } value.assert_equal(calldata_array[index]);