chore: databus internal audit - small cleanups and tests coverage (#16703)

a small PR doing the following as a part of the `databus` audit
- added tests to check the handling of constant or unnormalized indexes 
- removed some ugliness with dangling witnesses and replaced them with
`set_variable`
- some minor cleanups

Author: kashbrti

barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp

@@ -118,16 +118,13 @@ 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();
+
+        // 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) {
+            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]);
     }
 }
 
@@ -144,12 +141,11 @@ 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);
-
-        // 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);
+        // 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) {
+            builder.set_variable(index.witness_index, 0);
+        }
 
         if (op.access_type == 0) {
             value.assert_equal(table.read(index));
@@ -179,14 +175,12 @@ 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();
+            // 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) {
+                builder.set_variable(index.witness_index, 0);
             }
-            field_ct w = field_ct::from_witness(&builder, w_value);
-            value.assert_equal(calldata_array[w]);
-            w.assert_equal(index);
+            value.assert_equal(calldata_array[index]);
         }
     };
 

barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.cpp

@@ -53,7 +53,7 @@ field_t<Builder> databus<Builder>::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

barretenberg/cpp/src/barretenberg/stdlib/primitives/databus/databus.test.cpp

@@ -78,6 +78,102 @@ 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)
+{
+
+    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));
+}
+
+/**
+ * @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)
+{
+
+    Builder builder;
+    databus_ct databus;
+    std::array<fr, 3> raw_calldata_entries = { 3, 2, 1 };
+    std::array<fr, 3> raw_returndata_entries = { 3, 2, 1 };
+    std::vector<field_ct> calldata_entries;
+    for (fr entry : raw_calldata_entries) {
+        calldata_entries.emplace_back(witness_ct(&builder, entry));
+        field_ct entry_witness = witness_ct(&builder, entry);
+    }
+    std::vector<field_ct> 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 (the multiplicative constant will be 2)
+        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.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));
+}
+
+/**
+ * @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<fr, 3> raw_calldata_values = { 54, 32, 30 };
+    std::array<fr, 3> raw_returndata_values = { 54, 32, 116 };
+    // Populate the calldata in the databus
+    std::vector<field_ct> 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<field_ct> 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[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));
+}
+
 /**
  * @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 +275,4 @@ TEST(Databus, DuplicateRead)
     databus.return_data[idx_1];
 
     EXPECT_TRUE(CircuitChecker::check(builder));
-}
+}