improve documentation and fix issue in non-ultra variants of the

composer

barretenberg/cpp/src/barretenberg/plonk/composer/splitting_tmp/ultra_plonk_composer.test.cpp

@@ -914,4 +914,4 @@ TEST(ultra_plonk_composer_splitting_tmp, ram)
     EXPECT_EQ(result, true);
 }
 
-} // namespace proof_system::plonk
+} // namespace proof_system::plonk

barretenberg/cpp/src/barretenberg/plonk/composer/ultra_composer.cpp

@@ -1263,7 +1263,9 @@ void UltraComposer::process_range_list(RangeList& list)
 
     ASSERT(list.variable_indices.size() > 0);
 
-    // replace witness index in variable_indices with the real variable index (i.e. not an index that is a copied)
+    // replace witness index in variable_indices with the real variable index i.e. if a copy constraint has been
+    // applied on a variable after it was range constrained, this makes sure the indices in list point to the updated
+    // index in the range list so the set equivalence does not fail
     for (uint32_t& x : list.variable_indices) {
         x = real_variable_index[x];
     }
@@ -1273,7 +1275,7 @@ void UltraComposer::process_range_list(RangeList& list)
     list.variable_indices.erase(back_iterator, list.variable_indices.end());
 
     // go over variables
-    // for each variable, create mirror variable with same value - with tau tag
+    // iterate over each variable and create mirror variable with same value - with tau tag
     // need to make sure that, in original list, increments of at most 3
     std::vector<uint64_t> sorted_list;
     sorted_list.reserve(list.variable_indices.size());

barretenberg/cpp/src/barretenberg/plonk/composer/ultra_composer.hpp

@@ -127,6 +127,8 @@ class UltraComposer : public ComposerBase {
         uint64_t target_range;
         uint32_t range_tag;
         uint32_t tau_tag;
+        // contains the list of variable indices that are range constrained to target_range
+        // as ordered in the vector of variable indoces from the composer
         std::vector<uint32_t> variable_indices;
     };
 

barretenberg/cpp/src/barretenberg/plonk/composer/ultra_composer.test.cpp

@@ -793,15 +793,11 @@ TYPED_TEST(ultra_composer, range_checks_on_duplicates)
     TestFixture::prove_and_verify(composer, /*expected_result=*/true);
 }
 
-// Issue appearing in under-constrained circuit i.e. in which variables:
-// - do not appear in any gate of the circuit
-// - are smaller than 2^14 (hence not sliced and reconstructed through addition gates)
-// - BUT have range constraints applied on them (which, by default, do not add gates to the circuit
-// but check sets are correctly sorted)
-// The test further shows that the witnesses have different indices hence the problem is not caused by
-// the range constraint being applied to the same witness twice.
-// TODO: look further into the problem and exemplify it better
-TEST(ultra_composer, range_constraint_fails)
+// Ensure copy constraints added on variables smaller than 2^14, which have been previously
+// range constrained, do not break the set equivalence checks because of indices mismatch.
+// 2^14 is DEFAULT_PLOOKUP_RANGE_BITNUM i.e. the maximum size before a variable gets sliced
+// before range constraints are applied to it.
+TEST(ultra_composer, range_constraint_small_variable)
 {
     auto composer = UltraComposer();
     uint16_t mask = (1 << 8) - 1;

barretenberg/cpp/src/barretenberg/stdlib/primitives/logic/logic.cpp

@@ -13,7 +13,8 @@ namespace proof_system::plonk::stdlib {
  * @brief A logical AND or XOR over a variable number of bits.
  *
  * @details Defaults to basic Composer method if not using plookup-compatible composer. If the left and right operands
- * are larger than num_bit, the result will be truncated to num_bits.
+ * are larger than num_bit, the result will be truncated to num_bits. However, the two operands could be
+ * range-constrained to num_bits before the call which would remove the need to range constrain inside this function.
  *
  * @tparam Composer
  * @param a
@@ -105,14 +106,10 @@ field_t<Composer> logic<Composer>::create_logic_constraint(
         // If the composer doesn't have lookups we call the expensive logic constraint gate
         // which creates constraints for each bit. We only create constraints up to num_bits.
         Composer* ctx = a.get_context();
-        uint256_t left = a.get_value();
-        uint256_t right = b.get_value();
-        left = left & ((uint256_t(1) << num_bits) - 1);
-        right = right & ((uint256_t(1) << num_bits) - 1);
-        field_pt a_chunk = witness_pt(ctx, left);
-        field_pt b_chunk = witness_pt(ctx, right);
+        field_pt a_slice = a.slice(static_cast<uint8_t>(num_bits - 1), 0)[1];
+        field_pt b_slice = b.slice(static_cast<uint8_t>(num_bits - 1), 0)[1];
         auto accumulator_triple = ctx->create_logic_constraint(
-            a_chunk.normalize().get_witness_index(), b_chunk.normalize().get_witness_index(), num_bits, is_xor_gate);
+            a_slice.normalize().get_witness_index(), b_slice.normalize().get_witness_index(), num_bits, is_xor_gate);
         auto out_idx = accumulator_triple.out[accumulator_triple.out.size() - 1];
         return field_t<Composer>::from_witness_index(ctx, out_idx);
     }