fixed bug where range constraining connected witnesses threw an error (…

…#369) * fixed bug where range constraining connected witnesses threw an error * can now apply multiple overlapping ranges to the same witness (or copies of the witness) --------- Co-authored-by: codygunton <[email protected]>
AztecProtocol · Apr 20, 2023 · a38e361 · a38e361
1 parent 9fb4b7a
commit a38e361
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 2 deletions.
diff --git a/cpp/src/barretenberg/plonk/composer/ultra_composer.cpp b/cpp/src/barretenberg/plonk/composer/ultra_composer.cpp
@@ -1216,9 +1216,44 @@ void UltraComposer::create_new_range_constraint(const uint32_t variable_index,
         range_lists.insert({ target_range, create_range_list(target_range) });
     }
 
+    const auto existing_tag = real_variable_tags[real_variable_index[variable_index]];
     auto& list = range_lists[target_range];
-    assign_tag(variable_index, list.range_tag);
-    list.variable_indices.emplace_back(variable_index);
+
+    // If the variable's tag matches the target range list's tag, do nothing.
+    if (existing_tag != list.range_tag) {
+        // If the variable is 'untagged' (i.e., it has the dummy tag), assign it the appropriate tag.
+        // Otherwise, find the range for which the variable has already been tagged.
+        if (existing_tag != DUMMY_TAG) {
+            bool found_tag = false;
+            for (const auto& r : range_lists) {
+                if (r.second.range_tag == existing_tag) {
+                    found_tag = true;
+                    if (r.first < target_range) {
+                        // The variable already has a more restrictive range check, so do nothing.
+                        return;
+                    } else {
+                        // The range constraint we are trying to impose is more restrictive than the existing range
+                        // constraint. It would be difficult to remove an existing range check. Instead deep-copy the
+                        // variable and apply a range check to new variable
+                        const uint32_t copied_witness = add_variable(get_variable(variable_index));
+                        create_add_gate({ .a = variable_index,
+                                          .b = copied_witness,
+                                          .c = zero_idx,
+                                          .a_scaling = 1,
+                                          .b_scaling = -1,
+                                          .c_scaling = 0,
+                                          .const_scaling = 0 });
+                        // Recurse with new witness that has no tag attached.
+                        create_new_range_constraint(copied_witness, target_range, msg);
+                        return;
+                    }
+                }
+            }
+            ASSERT(found_tag == true);
+        }
+        assign_tag(variable_index, list.range_tag);
+        list.variable_indices.emplace_back(variable_index);
+    }
 }
 
 void UltraComposer::process_range_list(const RangeList& list)

diff --git a/cpp/src/barretenberg/plonk/composer/ultra_composer.hpp b/cpp/src/barretenberg/plonk/composer/ultra_composer.hpp
@@ -425,6 +425,10 @@ class UltraComposer : public ComposerBase {
     void assign_tag(const uint32_t variable_index, const uint32_t tag)
     {
         ASSERT(tag <= current_tag);
+        // If we've already assigned this tag to this variable, return (can happen due to copy constraints)
+        if (real_variable_tags[real_variable_index[variable_index]] == tag) {
+            return;
+        }
         ASSERT(real_variable_tags[real_variable_index[variable_index]] == DUMMY_TAG);
         real_variable_tags[real_variable_index[variable_index]] = tag;
     }

diff --git a/cpp/src/barretenberg/plonk/composer/ultra_composer.test.cpp b/cpp/src/barretenberg/plonk/composer/ultra_composer.test.cpp
@@ -758,4 +758,39 @@ TYPED_TEST(ultra_composer, ram)
     TestFixture::prove_and_verify(composer, /*expected_result=*/true);
 }
 
+TYPED_TEST(ultra_composer, range_checks_on_duplicates)
+{
+    UltraComposer composer = UltraComposer();
+
+    uint32_t a = composer.add_variable(100);
+    uint32_t b = composer.add_variable(100);
+    uint32_t c = composer.add_variable(100);
+    uint32_t d = composer.add_variable(100);
+
+    composer.assert_equal(a, b);
+    composer.assert_equal(a, c);
+    composer.assert_equal(a, d);
+
+    composer.create_new_range_constraint(a, 1000);
+    composer.create_new_range_constraint(b, 1001);
+    composer.create_new_range_constraint(c, 999);
+    composer.create_new_range_constraint(d, 1000);
+
+    composer.create_big_add_gate(
+        {
+            a,
+            b,
+            c,
+            d,
+            0,
+            0,
+            0,
+            0,
+            0,
+        },
+        false);
+
+    TestFixture::prove_and_verify(composer, /*expected_result=*/true);
+}
+
 } // namespace proof_system::plonk::test_ultra_composer