add from_witness_index_unsafe, turn members private

Author: iakovenkos

barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp

@@ -44,7 +44,7 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val
     if (infinite.is_constant()) {
         builder.fix_witness(input.result_infinite, infinite.get_value());
     } else {
-        builder.assert_equal(infinite.witness_index, input.result_infinite);
+        builder.assert_equal(infinite.get_normalized_witness_index(), input.result_infinite);
     }
 }
 

barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp

@@ -49,7 +49,8 @@ void create_multi_scalar_mul_constraint(Builder& builder,
     if (output_point.is_point_at_infinity().is_constant()) {
         builder.fix_witness(input.out_point_is_infinite, output_point.is_point_at_infinity().get_value());
     } else {
-        builder.assert_equal(output_point.is_point_at_infinity().witness_index, input.out_point_is_infinite);
+        builder.assert_equal(output_point.is_point_at_infinity().get_normalized_witness_index(),
+                             input.out_point_is_infinite);
     }
     if (output_point.x.is_constant()) {
         builder.fix_witness(input.out_point_x, output_point.x.get_value());

barretenberg/cpp/src/barretenberg/stdlib/encryption/schnorr/schnorr.test.cpp

@@ -123,7 +123,7 @@ TEST(stdlib_schnorr, schnorr_signature_verification_result)
     stdlib::schnorr_signature_bits sig = stdlib::schnorr_convert_signature(&builder, signature);
     byte_array_ct message(&builder, longer_string);
     bool_ct signature_result = schnorr_signature_verification_result(message, pub_key, sig);
-    EXPECT_EQ(signature_result.witness_bool, true);
+    EXPECT_EQ(signature_result.get_normalized_witness_index(), true);
 
     info("num gates = ", builder.get_estimated_num_finalized_gates());
 
@@ -166,7 +166,7 @@ TEST(stdlib_schnorr, signature_verification_result_failure)
     stdlib::schnorr_signature_bits sig = stdlib::schnorr_convert_signature(&builder, signature);
     byte_array_ct message(&builder, message_string);
     bool_ct signature_result = schnorr_signature_verification_result(message, pub_key2_ct, sig);
-    EXPECT_EQ(signature_result.witness_bool, false);
+    EXPECT_EQ(signature_result.get_value(), false);
 
     info("num gates = ", builder.get_estimated_num_finalized_gates());
 

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

@@ -1111,9 +1111,9 @@ template <typename TestType> class stdlib_biggroup : public testing::Test {
             // scalar = -naf[254] + \sum_{i=0}^{253}(1-2*naf[i]) 2^{253-i}
             fr reconstructed_val(0);
             for (size_t i = 0; i < length; i++) {
-                reconstructed_val += (fr(1) - fr(2) * fr(naf[i].witness_bool)) * fr(uint256_t(1) << (length - 1 - i));
+                reconstructed_val += (fr(1) - fr(2) * fr(naf[i].get_value())) * fr(uint256_t(1) << (length - 1 - i));
             };
-            reconstructed_val -= fr(naf[length].witness_bool);
+            reconstructed_val -= fr(naf[length].get_value());
             EXPECT_EQ(scalar_val, reconstructed_val);
         }
 

barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_impl.hpp

@@ -124,11 +124,11 @@ element<C, Fq, Fr, G> element<C, Fq, Fr, G>::operator+(const element& other) con
     // n.b. can likely optimize this
     bool_ct result_is_infinity = infinity_predicate && (!lhs_infinity && !rhs_infinity);
     if constexpr (IsUltraBuilder<C>) {
-        result_is_infinity.get_context()->update_used_witnesses(result_is_infinity.witness_index);
+        result_is_infinity.get_context()->update_used_witnesses(result_is_infinity.get_normalized_witness_index());
     }
     result_is_infinity = result_is_infinity || (lhs_infinity && rhs_infinity);
     if constexpr (IsUltraBuilder<C>) {
-        result_is_infinity.get_context()->update_used_witnesses(result_is_infinity.witness_index);
+        result_is_infinity.get_context()->update_used_witnesses(result_is_infinity.get_normalized_witness_index());
     }
     result.set_point_at_infinity(result_is_infinity);
 
@@ -202,11 +202,11 @@ element<C, Fq, Fr, G> element<C, Fq, Fr, G>::operator-(const element& other) con
     // n.b. can likely optimize this
     bool_ct result_is_infinity = infinity_predicate && (!lhs_infinity && !rhs_infinity);
     if constexpr (IsUltraBuilder<C>) {
-        result_is_infinity.get_context()->update_used_witnesses(result_is_infinity.witness_index);
+        result_is_infinity.get_context()->update_used_witnesses(result_is_infinity.get_normalized_witness_index());
     }
     result_is_infinity = result_is_infinity || (lhs_infinity && rhs_infinity);
     if constexpr (IsUltraBuilder<C>) {
-        result_is_infinity.get_context()->update_used_witnesses(result_is_infinity.witness_index);
+        result_is_infinity.get_context()->update_used_witnesses(result_is_infinity.get_normalized_witness_index());
     }
     result.set_point_at_infinity(result_is_infinity);
     result.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag()));

barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_nafs.hpp

@@ -510,24 +510,10 @@ std::vector<bool_t<C>> element<C, Fq, Fr, G>::compute_naf(const Fr& scalar, cons
         // if the next entry is false, we need to flip the sign of the current entry. i.e. make negative
         // This is a VERY hacky workaround to ensure that UltraBuilder will apply a basic
         // range constraint per bool, and not a full 1-bit range gate
-        if (next_entry == false) {
-            bool_ct bit(ctx, true);
-            bit.context = ctx;
-            bit.witness_index = witness_t<C>(ctx, true).witness_index; // flip sign
-            bit.witness_bool = true;
-            ctx->create_new_range_constraint(
-                bit.witness_index, 1, "biggroup_nafs: compute_naf extracted too many bits in non-next_entry case");
-
-            naf_entries[num_rounds - i - 1] = bit;
-        } else {
-            bool_ct bit(ctx, false);
-            bit.witness_index = witness_t<C>(ctx, false).witness_index; // don't flip sign
-            bit.witness_bool = false;
-            ctx->create_new_range_constraint(
-                bit.witness_index, 1, "biggroup_nafs: compute_naf extracted too many bits in next_entry case");
+        bool_ct bit(witness_t<C>(ctx, !next_entry));
+
+        naf_entries[num_rounds - i - 1] = bit;
 
-            naf_entries[num_rounds - i - 1] = bit;
-        }
         // We need to manually propagate the origin tag
         naf_entries[num_rounds - i - 1].set_origin_tag(scalar.get_origin_tag());
     }

barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp

@@ -79,6 +79,24 @@ bool_t<Builder>::bool_t(bool_t<Builder>&& other)
     , tag(other.tag)
 {}
 
+/**
+ * @brief Create a `bool_t` from a witness index that is **known** to contain a constrained bool value.
+ * @warning The witness value **is not** constrained to be boolean. We simply perform an out-of-circuit sanity check.
+ */
+template <typename Builder>
+bool_t<Builder> bool_t<Builder>::from_witness_index_unsafe(Builder* ctx, const uint32_t witness_index)
+{
+    ASSERT(witness_index != IS_CONSTANT);
+    bool_t<Builder> result(ctx);
+    result.witness_index = witness_index;
+    bb::fr value = ctx->get_variable(witness_index);
+    // It does not create a constraint.
+    BB_ASSERT_EQ(value * value - value, 0, "bool_t: creating a witness bool from a non-boolean value");
+    result.witness_bool = (ctx->get_variable(witness_index) == 1);
+    result.witness_inverted = false;
+    return result;
+}
+
 /**
  * @brief Assigns a native `bool` to `bool_t` object.
  */

barretenberg/cpp/src/barretenberg/stdlib/primitives/bool/bool.hpp

@@ -70,6 +70,8 @@ template <typename Builder> class bool_t {
     bool_t& operator=(const bool_t& other);
     bool_t& operator=(bool_t&& other);
 
+    static bool_t from_witness_index_unsafe(Builder* ctx, uint32_t witness_index);
+
     // bitwise operations
     bool_t operator&(const bool_t& other) const;
     bool_t operator|(const bool_t& other) const;
@@ -109,6 +111,13 @@ template <typename Builder> class bool_t {
     bool get_value() const { return witness_bool ^ witness_inverted; }
 
     bool is_constant() const { return witness_index == IS_CONSTANT; }
+    bool is_inverted() const
+    {
+        if (is_constant()) {
+            ASSERT(!witness_inverted);
+        }
+        return witness_inverted;
+    }
 
     bool_t normalize() const;
 
@@ -127,11 +136,16 @@ template <typename Builder> class bool_t {
         context->fix_witness(witness_index, get_value());
         unset_free_witness_tag();
     }
+
+  private:
     mutable Builder* context = nullptr;
     mutable bool witness_bool = false;
     mutable bool witness_inverted = false;
     mutable uint32_t witness_index = IS_CONSTANT;
     mutable OriginTag tag{};
+
+    template <typename, typename> friend class bigfield;
+    template <typename> friend class field_t;
 };
 
 template <typename T> inline std::ostream& operator<<(std::ostream& os, bool_t<T> const& v)

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

@@ -102,11 +102,31 @@ template <class Builder_> class BoolTest : public ::testing::Test {
         EXPECT_TRUE(a_true.get_value());
         EXPECT_FALSE(a_false.get_value());
         EXPECT_TRUE(a_true.is_constant() && a_false.is_constant());
-        EXPECT_TRUE(!a_true.witness_inverted && !a_false.witness_inverted);
+        EXPECT_TRUE(!a_true.is_inverted() && !a_false.is_inverted());
         // No gates have been added
         EXPECT_TRUE(num_gates_start == builder.get_estimated_num_finalized_gates());
     }
 
+    void test_construct_from_witness_index()
+    {
+        Builder builder = Builder();
+        size_t num_gates_start = builder.get_estimated_num_finalized_gates();
+        const size_t witness_idx_zero = builder.add_variable(bb::fr(0));
+        const size_t witness_idx_one = builder.add_variable(bb::fr(1));
+        const size_t non_bool_witness_idx = builder.add_variable(bb::fr(15));
+
+        bool_ct bool_witness = bool_ct::from_witness_index_unsafe(&builder, witness_idx_zero);
+        EXPECT_EQ(bool_witness.get_value(), false);
+
+        bool_witness = bool_ct::from_witness_index_unsafe(&builder, witness_idx_one);
+        EXPECT_EQ(bool_witness.get_value(), true);
+        // No gates are added.
+        EXPECT_EQ(builder.get_estimated_num_finalized_gates() - num_gates_start, 0);
+
+        // Out-of-circuit failure when witness points to a non-bool value.
+        EXPECT_THROW_OR_ABORT(bool_witness = bool_ct::from_witness_index_unsafe(&builder, non_bool_witness_idx),
+                              "bool_t: creating a witness bool from a non-boolean value");
+    }
     void test_construct_from_witness()
     {
         Builder builder = Builder();
@@ -117,7 +137,7 @@ template <class Builder_> class BoolTest : public ::testing::Test {
         EXPECT_TRUE(a_true.get_value());
         EXPECT_FALSE(a_false.get_value());
         EXPECT_TRUE(!a_true.is_constant() && !a_false.is_constant());
-        EXPECT_TRUE(!a_true.witness_inverted && !a_false.witness_inverted);
+        EXPECT_TRUE(!a_true.is_inverted() && !a_false.is_inverted());
         // Each witness bool must be constrained => expect 2 gates being added
         EXPECT_TRUE(builder.get_estimated_num_finalized_gates() - num_gates_start == 2);
         EXPECT_TRUE(CircuitChecker::check(builder));
@@ -217,13 +237,13 @@ template <class Builder_> class BoolTest : public ::testing::Test {
                     if (!result_is_constant && a.is_constant() && !b.is_constant()) {
                         // we only add gates if the value `true` is not flipped to `false` and we need to add a new
                         // constant == 1, which happens iff `b` is not inverted.
-                        EXPECT_EQ(diff, static_cast<size_t>(!b.witness_inverted));
+                        EXPECT_EQ(diff, static_cast<size_t>(!b.is_inverted()));
                     }
 
                     if (!result_is_constant && !a.is_constant() && b.is_constant()) {
                         // we only add gates if the value `true` is not flipped to `false` and we need to add a new
                         // constant == 1, which happens iff `a` is inverted.
-                        EXPECT_EQ(diff, static_cast<size_t>(a.witness_inverted));
+                        EXPECT_EQ(diff, static_cast<size_t>(a.is_inverted()));
                     }
                     EXPECT_EQ(CircuitChecker::check(builder), expected);
                 }
@@ -283,9 +303,9 @@ template <class Builder_> class BoolTest : public ::testing::Test {
             if (!a.is_constant()) {
                 EXPECT_EQ(c.get_origin_tag(), submitted_value_origin_tag);
             }
-            EXPECT_EQ(c.witness_inverted, false);
+            EXPECT_EQ(c.is_inverted(), false);
             size_t diff = builder.get_estimated_num_finalized_gates() - num_gates_start;
-            // Note that although `normalize()` returns value, it flips the `witness_inverted` flag of `a` if it was
+            // Note that although `normalize()` returns value, it flips the `is_inverted()` flag of `a` if it was
             // `true`.
             EXPECT_EQ(diff, static_cast<size_t>(!a.is_constant() && a_raw.is_inverted));
             EXPECT_TRUE(CircuitChecker::check(builder));

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

@@ -164,7 +164,7 @@ template <typename Builder> class stdlib_field : public testing::Test {
 
             if (!expected_to_be_constant) {
                 EXPECT_TRUE(CircuitChecker::check(builder));
-                EXPECT_TRUE(converted.witness_index == field_elt.witness_index);
+                EXPECT_TRUE(converted.get_normalized_witness_index() == field_elt.witness_index);
             }
         }
         // Check that the conversion aborts in the case of random field elements.